From 109e5779e960c76dbb3f60d2bdc40f57e83df587 Mon Sep 17 00:00:00 2001 From: Spencer Gibb Date: Wed, 13 Feb 2019 11:33:06 -0500 Subject: [PATCH] Ignores MapPropertySources not created by boot Prior to this change, any `MapPropertySource` created by an `EnvironmentPostProcessor` would be processed by `BootstrapApplicationListener`. If that property source had a conditional property that it created itself, the 2nd run (bootstrap causes EPPs to run twice) would not have the property and that property source would end up in the main `Environment` effectively deleting a property. This change only deals with `MapPropertySource`s created by boot testing for a specific sub-type. Fixes gh-476 --- .../BootstrapApplicationListener.java | 5 +- ...ironmentPostProcessorIntegrationTests.java | 80 +++++++++++++++++++ ...ionPropertiesRebinderIntegrationTests.java | 4 +- .../test/resources/META-INF/spring.factories | 4 + 4 files changed, 89 insertions(+), 4 deletions(-) create mode 100644 spring-cloud-context/src/test/java/org/springframework/cloud/bootstrap/BootstrapEnvironmentPostProcessorIntegrationTests.java diff --git a/spring-cloud-context/src/main/java/org/springframework/cloud/bootstrap/BootstrapApplicationListener.java b/spring-cloud-context/src/main/java/org/springframework/cloud/bootstrap/BootstrapApplicationListener.java index 53c86cf0..d2a82e86 100644 --- a/spring-cloud-context/src/main/java/org/springframework/cloud/bootstrap/BootstrapApplicationListener.java +++ b/spring-cloud-context/src/main/java/org/springframework/cloud/bootstrap/BootstrapApplicationListener.java @@ -36,6 +36,7 @@ import org.springframework.boot.builder.SpringApplicationBuilder; import org.springframework.boot.context.event.ApplicationEnvironmentPreparedEvent; import org.springframework.boot.context.event.ApplicationFailedEvent; import org.springframework.boot.context.logging.LoggingApplicationListener; +import org.springframework.boot.env.OriginTrackedMapPropertySource; import org.springframework.cloud.bootstrap.encrypt.EnvironmentDecryptApplicationInitializer; import org.springframework.context.ApplicationContextInitializer; import org.springframework.context.ApplicationEvent; @@ -47,7 +48,6 @@ import org.springframework.core.annotation.AnnotationAwareOrderComparator; import org.springframework.core.annotation.Order; import org.springframework.core.env.CompositePropertySource; import org.springframework.core.env.ConfigurableEnvironment; -import org.springframework.core.env.EnumerablePropertySource; import org.springframework.core.env.MapPropertySource; import org.springframework.core.env.MutablePropertySources; import org.springframework.core.env.PropertySource; @@ -444,7 +444,8 @@ public class BootstrapApplicationListener } public void add(PropertySource source) { - if (source instanceof EnumerablePropertySource + // Only add map property sources added by boot, see gh-476 + if (source instanceof OriginTrackedMapPropertySource && !this.names.contains(source.getName())) { this.sources.addPropertySource(source); this.names.add(source.getName()); diff --git a/spring-cloud-context/src/test/java/org/springframework/cloud/bootstrap/BootstrapEnvironmentPostProcessorIntegrationTests.java b/spring-cloud-context/src/test/java/org/springframework/cloud/bootstrap/BootstrapEnvironmentPostProcessorIntegrationTests.java new file mode 100644 index 00000000..0a8c752e --- /dev/null +++ b/spring-cloud-context/src/test/java/org/springframework/cloud/bootstrap/BootstrapEnvironmentPostProcessorIntegrationTests.java @@ -0,0 +1,80 @@ +/* + * Copyright 2013-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 + * + * http://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.HashMap; + +import org.junit.Test; +import org.junit.runner.RunWith; + +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.SpringApplication; +import org.springframework.boot.SpringBootConfiguration; +import org.springframework.boot.autoconfigure.EnableAutoConfiguration; +import org.springframework.boot.env.EnvironmentPostProcessor; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.core.env.ConfigurableEnvironment; +import org.springframework.core.env.MapPropertySource; +import org.springframework.test.context.junit4.SpringRunner; + +import static org.assertj.core.api.Assertions.assertThat; + +// see https://github.com/spring-cloud/spring-cloud-commons/issues/476 +@RunWith(SpringRunner.class) +@SpringBootTest +public class BootstrapEnvironmentPostProcessorIntegrationTests { + + @Autowired + private ConfigurableEnvironment env; + + @Test + public void conditionalValuesFromMapProperySourceCreatedByEPPExist() { + assertThat(this.env.containsProperty("unconditional.property")) + .as("Environment does not contain unconditional.property") + .isTrue(); + assertThat(this.env.getProperty("unconditional.property")) + .as("Environment has wrong value for unconditional.property") + .isEqualTo("unconditional.value"); + assertThat(this.env.containsProperty("conditional.property")) + .as("Environment does not contain conditional.property") + .isTrue(); + assertThat(this.env.getProperty("conditional.property")) + .as("Environment has wrong value for conditional.property") + .isEqualTo("conditional.value"); + } + + @EnableAutoConfiguration + @SpringBootConfiguration + protected static class TestConfig { + } + + public static class TestConditionalEnvironmentPostProcessor implements EnvironmentPostProcessor { + @Override + public void postProcessEnvironment(ConfigurableEnvironment environment, SpringApplication application) { + HashMap map = new HashMap<>(); + + if (!environment.containsProperty("conditional.property")) { + map.put("conditional.property", "conditional.value"); + } + map.put("unconditional.property", "unconditional.value"); + + MapPropertySource propertySource = new MapPropertySource("test-epp-map", map); + environment.getPropertySources().addFirst(propertySource); + } + } + +} diff --git a/spring-cloud-context/src/test/java/org/springframework/cloud/context/properties/ConfigurationPropertiesRebinderIntegrationTests.java b/spring-cloud-context/src/test/java/org/springframework/cloud/context/properties/ConfigurationPropertiesRebinderIntegrationTests.java index 59b22423..70f50df2 100644 --- a/spring-cloud-context/src/test/java/org/springframework/cloud/context/properties/ConfigurationPropertiesRebinderIntegrationTests.java +++ b/spring-cloud-context/src/test/java/org/springframework/cloud/context/properties/ConfigurationPropertiesRebinderIntegrationTests.java @@ -24,6 +24,7 @@ import org.junit.runner.RunWith; import org.springframework.aop.framework.ProxyFactory; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.autoconfigure.ImportAutoConfiguration; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; import org.springframework.boot.autoconfigure.context.PropertyPlaceholderAutoConfiguration; import org.springframework.boot.context.properties.ConfigurationProperties; @@ -35,7 +36,6 @@ import org.springframework.cloud.autoconfigure.RefreshAutoConfiguration; import org.springframework.cloud.context.properties.ConfigurationPropertiesRebinderIntegrationTests.TestConfiguration; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; -import org.springframework.context.annotation.Import; import org.springframework.core.env.ConfigurableEnvironment; import org.springframework.test.annotation.DirtiesContext; import org.springframework.test.context.ActiveProfiles; @@ -117,7 +117,7 @@ public class ConfigurationPropertiesRebinderIntegrationTests { @Configuration @EnableConfigurationProperties - @Import({ RefreshConfiguration.RebinderConfiguration.class, + @ImportAutoConfiguration({ RefreshConfiguration.RebinderConfiguration.class, PropertyPlaceholderAutoConfiguration.class }) protected static class TestConfiguration { diff --git a/spring-cloud-context/src/test/resources/META-INF/spring.factories b/spring-cloud-context/src/test/resources/META-INF/spring.factories index 419787be..8c44cd81 100644 --- a/spring-cloud-context/src/test/resources/META-INF/spring.factories +++ b/spring-cloud-context/src/test/resources/META-INF/spring.factories @@ -2,3 +2,7 @@ org.springframework.cloud.bootstrap.BootstrapConfiguration=\ org.springframework.cloud.bootstrap.TestBootstrapConfiguration,\ org.springframework.cloud.bootstrap.TestHigherPriorityBootstrapConfiguration + + +org.springframework.boot.env.EnvironmentPostProcessor=\ +org.springframework.cloud.bootstrap.BootstrapEnvironmentPostProcessorIntegrationTests.TestConditionalEnvironmentPostProcessor \ No newline at end of file