diff --git a/spring-context/src/main/java/org/springframework/context/support/PropertySourcesPlaceholderConfigurer.java b/spring-context/src/main/java/org/springframework/context/support/PropertySourcesPlaceholderConfigurer.java index 0da9f005e9..94ed316e2d 100644 --- a/spring-context/src/main/java/org/springframework/context/support/PropertySourcesPlaceholderConfigurer.java +++ b/spring-context/src/main/java/org/springframework/context/support/PropertySourcesPlaceholderConfigurer.java @@ -26,7 +26,6 @@ import org.springframework.beans.factory.BeanInitializationException; import org.springframework.beans.factory.config.ConfigurableListableBeanFactory; import org.springframework.beans.factory.config.PlaceholderConfigurerSupport; import org.springframework.context.EnvironmentAware; -import org.springframework.core.env.CompositePropertySource; import org.springframework.core.env.ConfigurableEnvironment; import org.springframework.core.env.ConfigurablePropertyResolver; import org.springframework.core.env.Environment; @@ -131,22 +130,10 @@ public class PropertySourcesPlaceholderConfigurer extends PlaceholderConfigurerS if (this.propertySources == null) { this.propertySources = new MutablePropertySources(); if (this.environment != null) { - PropertySource environmentPropertySource; - if (this.environment instanceof ConfigurableEnvironment configurableEnvironment) { - environmentPropertySource = new CompositePropertySource(ENVIRONMENT_PROPERTIES_PROPERTY_SOURCE_NAME, - configurableEnvironment.getPropertySources()); - } - else { - // Fallback code path that should never apply in a regular scenario, since the - // Environment in the ApplicationContext should always be a ConfigurableEnvironment. - environmentPropertySource = - new PropertySource<>(ENVIRONMENT_PROPERTIES_PROPERTY_SOURCE_NAME, this.environment) { - @Override - public @Nullable Object getProperty(String key) { - return super.source.getProperty(key); - } - }; - } + PropertySource environmentPropertySource = + (this.environment instanceof ConfigurableEnvironment configurableEnvironment ? + new ConfigurableEnvironmentPropertySource(configurableEnvironment) : + new FallbackEnvironmentPropertySource(this.environment)); this.propertySources.addLast(environmentPropertySource); } try { @@ -229,4 +216,73 @@ public class PropertySourcesPlaceholderConfigurer extends PlaceholderConfigurerS return this.appliedPropertySources; } + + /** + * Custom {@link PropertySource} that delegates to the + * {@link ConfigurableEnvironment#getPropertySources() PropertySources} in a + * {@link ConfigurableEnvironment}. + * @since 6.2.7 + */ + private static class ConfigurableEnvironmentPropertySource extends PropertySource { + + private final PropertySources propertySources; + + + ConfigurableEnvironmentPropertySource(ConfigurableEnvironment environment) { + super(ENVIRONMENT_PROPERTIES_PROPERTY_SOURCE_NAME, environment); + this.propertySources = environment.getPropertySources(); + } + + + @Override + public @Nullable Object getProperty(String name) { + for (PropertySource propertySource : this.propertySources) { + Object candidate = propertySource.getProperty(name); + if (candidate != null) { + return candidate; + } + } + return null; + } + + @Override + public boolean containsProperty(String name) { + for (PropertySource propertySource : this.propertySources) { + if (propertySource.containsProperty(name)) { + return true; + } + } + return false; + } + + @Override + public String toString() { + return "ConfigurableEnvironmentPropertySource {propertySources=" + this.propertySources + "}"; + } + } + + /** + * Fallback {@link PropertySource} that delegates to a raw {@link Environment}. + *

Should never apply in a regular scenario, since the {@code Environment} + * in an {@code ApplicationContext} should always be a {@link ConfigurableEnvironment}. + * @since 6.2.7 + */ + private static class FallbackEnvironmentPropertySource extends PropertySource { + + FallbackEnvironmentPropertySource(Environment environment) { + super(ENVIRONMENT_PROPERTIES_PROPERTY_SOURCE_NAME, environment); + } + + + @Override + public @Nullable Object getProperty(String name) { + return super.source.getProperty(name); + } + + @Override + public String toString() { + return "FallbackEnvironmentPropertySource {environment=" + super.source + "}"; + } + } + } diff --git a/spring-context/src/test/java/org/springframework/context/support/PropertySourcesPlaceholderConfigurerTests.java b/spring-context/src/test/java/org/springframework/context/support/PropertySourcesPlaceholderConfigurerTests.java index 9fee2b56e9..24325f9552 100644 --- a/spring-context/src/test/java/org/springframework/context/support/PropertySourcesPlaceholderConfigurerTests.java +++ b/spring-context/src/test/java/org/springframework/context/support/PropertySourcesPlaceholderConfigurerTests.java @@ -16,11 +16,16 @@ package org.springframework.context.support; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; import java.util.Optional; import java.util.Properties; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import org.springframework.beans.factory.BeanCreationException; import org.springframework.beans.factory.BeanDefinitionStoreException; @@ -32,6 +37,7 @@ import org.springframework.context.annotation.AnnotationConfigApplicationContext import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.core.convert.support.DefaultConversionService; +import org.springframework.core.env.EnumerablePropertySource; import org.springframework.core.env.MutablePropertySources; import org.springframework.core.env.PropertySource; import org.springframework.core.env.StandardEnvironment; @@ -90,14 +96,29 @@ class PropertySourcesPlaceholderConfigurerTests { assertThat(bf.getBean(TestBean.class).getName()).isEqualTo("foo"); } - @Test - void localPropertiesOverrideFalse() { - localPropertiesOverride(false); - } + @ParameterizedTest + @ValueSource(booleans = {true, false}) + void localPropertiesOverride(boolean override) { + DefaultListableBeanFactory bf = new DefaultListableBeanFactory(); + bf.registerBeanDefinition("testBean", + genericBeanDefinition(TestBean.class) + .addPropertyValue("name", "${foo}") + .getBeanDefinition()); - @Test - void localPropertiesOverrideTrue() { - localPropertiesOverride(true); + PropertySourcesPlaceholderConfigurer ppc = new PropertySourcesPlaceholderConfigurer(); + + ppc.setLocalOverride(override); + ppc.setProperties(new Properties() {{ + setProperty("foo", "local"); + }}); + ppc.setEnvironment(new MockEnvironment().withProperty("foo", "enclosing")); + ppc.postProcessBeanFactory(bf); + if (override) { + assertThat(bf.getBean(TestBean.class).getName()).isEqualTo("local"); + } + else { + assertThat(bf.getBean(TestBean.class).getName()).isEqualTo("enclosing"); + } } @Test @@ -283,28 +304,58 @@ class PropertySourcesPlaceholderConfigurerTests { assertThat(bf.getBean(TestBean.class).getName()).isEqualTo("bar"); } - @SuppressWarnings("serial") - private void localPropertiesOverride(boolean override) { + @Test // gh-34861 + void withEnumerableAndNonEnumerablePropertySourcesInTheEnvironmentAndLocalProperties() { DefaultListableBeanFactory bf = new DefaultListableBeanFactory(); bf.registerBeanDefinition("testBean", genericBeanDefinition(TestBean.class) - .addPropertyValue("name", "${foo}") + .addPropertyValue("name", "${foo:bogus}") + .addPropertyValue("jedi", "${local:false}") .getBeanDefinition()); - PropertySourcesPlaceholderConfigurer ppc = new PropertySourcesPlaceholderConfigurer(); + // 1) MockPropertySource is an EnumerablePropertySource. + MockPropertySource mockPropertySource = new MockPropertySource("mockPropertySource") + .withProperty("foo", "${bar}"); - ppc.setLocalOverride(override); + // 2) PropertySource is not an EnumerablePropertySource. + PropertySource rawPropertySource = new PropertySource<>("rawPropertySource", new Object()) { + @Override + public Object getProperty(String key) { + return ("bar".equals(key) ? "quux" : null); + } + }; + + MockEnvironment env = new MockEnvironment(); + env.getPropertySources().addFirst(mockPropertySource); + env.getPropertySources().addLast(rawPropertySource); + + PropertySourcesPlaceholderConfigurer ppc = new PropertySourcesPlaceholderConfigurer(); + ppc.setEnvironment(env); + // 3) Local properties are stored in a PropertiesPropertySource which is an EnumerablePropertySource. ppc.setProperties(new Properties() {{ - setProperty("foo", "local"); + setProperty("local", "true"); }}); - ppc.setEnvironment(new MockEnvironment().withProperty("foo", "enclosing")); ppc.postProcessBeanFactory(bf); - if (override) { - assertThat(bf.getBean(TestBean.class).getName()).isEqualTo("local"); - } - else { - assertThat(bf.getBean(TestBean.class).getName()).isEqualTo("enclosing"); + + // Verify all properties can be resolved via the Environment. + assertThat(env.getProperty("foo")).isEqualTo("quux"); + assertThat(env.getProperty("bar")).isEqualTo("quux"); + + // Verify that placeholder resolution works. + TestBean testBean = bf.getBean(TestBean.class); + assertThat(testBean.getName()).isEqualTo("quux"); + assertThat(testBean.isJedi()).isTrue(); + + // Verify that the presence of a non-EnumerablePropertySource does not prevent + // accessing EnumerablePropertySources via getAppliedPropertySources(). + List propertyNames = new ArrayList<>(); + for (PropertySource propertySource : ppc.getAppliedPropertySources()) { + if (propertySource instanceof EnumerablePropertySource enumerablePropertySource) { + Collections.addAll(propertyNames, enumerablePropertySource.getPropertyNames()); + } } + // Should not contain "foo" or "bar" from the Environment. + assertThat(propertyNames).containsOnly("local"); } @Test