diff --git a/spring-cloud-context/src/main/java/org/springframework/cloud/bootstrap/config/PropertySourceBootstrapConfiguration.java b/spring-cloud-context/src/main/java/org/springframework/cloud/bootstrap/config/PropertySourceBootstrapConfiguration.java index d9e37684..4e5d6ed3 100644 --- a/spring-cloud-context/src/main/java/org/springframework/cloud/bootstrap/config/PropertySourceBootstrapConfiguration.java +++ b/spring-cloud-context/src/main/java/org/springframework/cloud/bootstrap/config/PropertySourceBootstrapConfiguration.java @@ -102,9 +102,9 @@ public class PropertySourceBootstrapConfiguration implements if (source == null || source.size() == 0) { continue; } - List sourceList = new ArrayList<>(); - for (PropertySource p : source) { - sourceList.add(new BootstrapPropertySource(p)); + List> sourceList = new ArrayList<>(); + for (PropertySource p : source) { + sourceList.add(new BootstrapPropertySource<>(p)); } logger.info("Located property source: " + sourceList); composite.addAll(sourceList); @@ -114,7 +114,7 @@ public class PropertySourceBootstrapConfiguration implements MutablePropertySources propertySources = environment.getPropertySources(); String logConfig = environment.resolvePlaceholders("${logging.config:}"); LogFile logFile = LogFile.get(environment); - for (PropertySource p : environment.getPropertySources()) { + for (PropertySource p : environment.getPropertySources()) { if (p.getName().startsWith(BOOTSTRAP_PROPERTY_SOURCE_NAME)) { propertySources.remove(p.getName()); } @@ -166,13 +166,13 @@ public class PropertySourceBootstrapConfiguration implements private void insertPropertySources(MutablePropertySources propertySources, List> composite) { MutablePropertySources incoming = new MutablePropertySources(); - List> reversedComposite = new ArrayList(composite); + List> reversedComposite = new ArrayList<>(composite); // Reverse the list so that when we call addFirst below we are maintaining the - // same order of PropertySournces + // same order of PropertySources // Wherever we call addLast we can use the order in the List since the first item // will end up before the rest Collections.reverse(reversedComposite); - for (PropertySource p : reversedComposite) { + for (PropertySource p : reversedComposite) { incoming.addFirst(p); } PropertySourceBootstrapProperties remoteProperties = new PropertySourceBootstrapProperties(); @@ -180,31 +180,31 @@ public class PropertySourceBootstrapConfiguration implements Bindable.ofInstance(remoteProperties)); if (!remoteProperties.isAllowOverride() || (!remoteProperties.isOverrideNone() && remoteProperties.isOverrideSystemProperties())) { - for (PropertySource p : reversedComposite) { + for (PropertySource p : reversedComposite) { propertySources.addFirst(p); } return; } if (remoteProperties.isOverrideNone()) { - for (PropertySource p : composite) { + for (PropertySource p : composite) { propertySources.addLast(p); } return; } if (propertySources.contains(SYSTEM_ENVIRONMENT_PROPERTY_SOURCE_NAME)) { if (!remoteProperties.isOverrideSystemProperties()) { - for (PropertySource p : composite) { + for (PropertySource p : reversedComposite) { propertySources.addAfter(SYSTEM_ENVIRONMENT_PROPERTY_SOURCE_NAME, p); } } else { - for (PropertySource p : reversedComposite) { + for (PropertySource p : composite) { propertySources.addBefore(SYSTEM_ENVIRONMENT_PROPERTY_SOURCE_NAME, p); } } } else { - for (PropertySource p : composite) { + for (PropertySource p : composite) { propertySources.addLast(p); } } @@ -276,7 +276,7 @@ class BootstrapPropertySource extends EnumerablePropertySource { @Override public Object getProperty(String name) { - return p.getProperty(name); + return this.p.getProperty(name); } @Override @@ -285,9 +285,10 @@ class BootstrapPropertySource extends EnumerablePropertySource { if (!(this.p instanceof EnumerablePropertySource)) { throw new IllegalStateException( "Failed to enumerate property names due to non-enumerable property source: " - + p); + + this.p); } - names.addAll(Arrays.asList(((EnumerablePropertySource) p).getPropertyNames())); + names.addAll( + Arrays.asList(((EnumerablePropertySource) this.p).getPropertyNames())); return StringUtils.toStringArray(names); } diff --git a/spring-cloud-context/src/main/java/org/springframework/cloud/context/refresh/ContextRefresher.java b/spring-cloud-context/src/main/java/org/springframework/cloud/context/refresh/ContextRefresher.java index 8e1e8069..9332a030 100644 --- a/spring-cloud-context/src/main/java/org/springframework/cloud/context/refresh/ContextRefresher.java +++ b/spring-cloud-context/src/main/java/org/springframework/cloud/context/refresh/ContextRefresher.java @@ -129,6 +129,8 @@ public class ContextRefresher { else { if (targetName != null) { target.addAfter(targetName, source); + // update targetName to preserve ordering + targetName = name; } else { // targetName was null so we are at the start of the list diff --git a/spring-cloud-context/src/test/java/org/springframework/cloud/bootstrap/BootstrapOrderingCustomOverrideSystemPropertiesIntegrationTests.java b/spring-cloud-context/src/test/java/org/springframework/cloud/bootstrap/BootstrapOrderingCustomOverrideSystemPropertiesIntegrationTests.java new file mode 100644 index 00000000..318ec78e --- /dev/null +++ b/spring-cloud-context/src/test/java/org/springframework/cloud/bootstrap/BootstrapOrderingCustomOverrideSystemPropertiesIntegrationTests.java @@ -0,0 +1,98 @@ +/* + * Copyright 2012-2019 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.cloud.bootstrap; + +import java.util.ArrayList; +import java.util.Collection; +import java.util.HashMap; +import java.util.Map; + +import org.junit.Test; +import org.junit.runner.RunWith; + +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.autoconfigure.EnableAutoConfiguration; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.cloud.bootstrap.BootstrapOrderingCustomPropertySourceIntegrationTests.Application; +import org.springframework.cloud.bootstrap.config.PropertySourceLocator; +import org.springframework.context.annotation.Configuration; +import org.springframework.core.env.ConfigurableEnvironment; +import org.springframework.core.env.Environment; +import org.springframework.core.env.MapPropertySource; +import org.springframework.core.env.MutablePropertySources; +import org.springframework.core.env.PropertySource; +import org.springframework.test.context.junit4.SpringRunner; + +import static java.util.Collections.singletonMap; +import static org.assertj.core.api.Assertions.assertThat; + +@RunWith(SpringRunner.class) +@SpringBootTest(classes = Application.class, properties = { + "spring.cloud.bootstrap.name:ordering" }) +public class BootstrapOrderingCustomOverrideSystemPropertiesIntegrationTests { + + @Autowired + private ConfigurableEnvironment environment; + + @Test + public void orderingIsCorrect() { + MutablePropertySources propertySources = environment.getPropertySources(); + PropertySource test1 = propertySources + .get("bootstrapProperties-testBootstrap1"); + PropertySource test2 = propertySources + .get("bootstrapProperties-testBootstrap2"); + PropertySource test3 = propertySources + .get("bootstrapProperties-testBootstrap3"); + int index1 = propertySources.precedenceOf(test1); + int index2 = propertySources.precedenceOf(test2); + int index3 = propertySources.precedenceOf(test3); + assertThat(index1).as("source1 index not less then source2").isLessThan(index2); + assertThat(index2).as("source2 index not less then source3").isLessThan(index3); + } + + @EnableAutoConfiguration + @Configuration + protected static class Application { + + } + + @Configuration + // This is added to bootstrap context as a source in ordering.properties + protected static class PropertySourceConfiguration implements PropertySourceLocator { + + @Override + public PropertySource locate(Environment environment) { + throw new UnsupportedOperationException(); + } + + @Override + public Collection> locateCollection(Environment environment) { + ArrayList> sources = new ArrayList<>(); + sources.add(new MapPropertySource("testBootstrap1", + singletonMap("key1", "value1"))); + sources.add(new MapPropertySource("testBootstrap2", + singletonMap("key2", "value2"))); + Map map = new HashMap<>(); + map.put("key3", "value3"); + map.put("spring.cloud.config.override-system-properties", "false"); + sources.add(new MapPropertySource("testBootstrap3", map)); + return sources; + } + + } + +} diff --git a/spring-cloud-context/src/test/java/org/springframework/cloud/context/refresh/ContextRefresherOrderingIntegrationTests.java b/spring-cloud-context/src/test/java/org/springframework/cloud/context/refresh/ContextRefresherOrderingIntegrationTests.java new file mode 100644 index 00000000..b35386f1 --- /dev/null +++ b/spring-cloud-context/src/test/java/org/springframework/cloud/context/refresh/ContextRefresherOrderingIntegrationTests.java @@ -0,0 +1,128 @@ +/* + * Copyright 2013-2020 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.cloud.context.refresh; + +import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; +import java.util.concurrent.atomic.AtomicBoolean; + +import org.junit.AfterClass; +import org.junit.BeforeClass; +import org.junit.Test; +import org.junit.runner.RunWith; + +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.SpringBootConfiguration; +import org.springframework.boot.autoconfigure.EnableAutoConfiguration; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.cloud.bootstrap.config.PropertySourceLocator; +import org.springframework.context.annotation.Configuration; +import org.springframework.core.env.ConfigurableEnvironment; +import org.springframework.core.env.Environment; +import org.springframework.core.env.MapPropertySource; +import org.springframework.core.env.MutablePropertySources; +import org.springframework.core.env.PropertySource; +import org.springframework.test.annotation.DirtiesContext; +import org.springframework.test.context.junit4.SpringRunner; + +import static java.util.Collections.singletonMap; +import static org.assertj.core.api.Assertions.assertThat; + +@RunWith(SpringRunner.class) +@SpringBootTest +@DirtiesContext +public class ContextRefresherOrderingIntegrationTests { + + @Autowired + private ConfigurableEnvironment environment; + + @Autowired + private ContextRefresher refresher; + + private static String original; + + @BeforeClass + public static void beforeClass() { + original = System.getProperty("spring.cloud.bootstrap.sources"); + System.setProperty("spring.cloud.bootstrap.sources", + "org.springframework.cloud.context.refresh.ContextRefresherOrderingIntegrationTests.PropertySourceConfiguration"); + } + + @AfterClass + public static void afterClass() { + if (original != null) { + System.setProperty("spring.cloud.bootstrap.sources", original); + } + else { + System.clearProperty("spring.cloud.bootstrap.sources"); + } + } + + @Test + public void orderingIsCorrect() { + refresher.refresh(); + MutablePropertySources propertySources = environment.getPropertySources(); + PropertySource test1 = propertySources + .get("bootstrapProperties-testContextRefresherOrdering1"); + PropertySource test2 = propertySources + .get("bootstrapProperties-testContextRefresherOrdering2"); + PropertySource test3 = propertySources + .get("bootstrapProperties-testContextRefresherOrdering3"); + int index1 = propertySources.precedenceOf(test1); + int index2 = propertySources.precedenceOf(test2); + int index3 = propertySources.precedenceOf(test3); + assertThat(index1).as("source1 index not less then source2").isLessThan(index2); + assertThat(index2).as("source2 index not less then source3").isLessThan(index3); + } + + @SpringBootConfiguration + @EnableAutoConfiguration + protected static class Application { + + } + + @Configuration + // This is added to bootstrap context as a source in + // contextrefresherordering.properties + protected static class PropertySourceConfiguration implements PropertySourceLocator { + + private static AtomicBoolean first = new AtomicBoolean(true); + + @Override + public PropertySource locate(Environment environment) { + throw new UnsupportedOperationException(); + } + + @Override + public Collection> locateCollection(Environment environment) { + if (first.compareAndSet(true, false)) { + return Collections.emptyList(); + } + ArrayList> sources = new ArrayList<>(); + sources.add(new MapPropertySource("testContextRefresherOrdering1", + singletonMap("key1", "value1"))); + sources.add(new MapPropertySource("testContextRefresherOrdering2", + singletonMap("key2", "value2"))); + sources.add(new MapPropertySource("testContextRefresherOrdering3", + singletonMap("key3", "value3"))); + return sources; + } + + } + +} diff --git a/spring-cloud-context/src/test/resources/ordering.properties b/spring-cloud-context/src/test/resources/ordering.properties new file mode 100644 index 00000000..7ace5026 --- /dev/null +++ b/spring-cloud-context/src/test/resources/ordering.properties @@ -0,0 +1 @@ +spring.main.sources:org.springframework.cloud.bootstrap.BootstrapOrderingCustomOverrideSystemPropertiesIntegrationTests.PropertySourceConfiguration