Skip to content

Commit 958dab7

Browse files
sbrannensbrannen
committed
Merge branch '6.2.x'
2 parents 6878587 + ebb44a8 commit 958dab7

File tree

2 files changed

+143
-36
lines changed

2 files changed

+143
-36
lines changed

‎spring-context/src/main/java/org/springframework/context/support/PropertySourcesPlaceholderConfigurer.java

Lines changed: 73 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@
2626
import org.springframework.beans.factory.config.ConfigurableListableBeanFactory;
2727
import org.springframework.beans.factory.config.PlaceholderConfigurerSupport;
2828
import org.springframework.context.EnvironmentAware;
29-
import org.springframework.core.env.CompositePropertySource;
3029
import org.springframework.core.env.ConfigurableEnvironment;
3130
import org.springframework.core.env.ConfigurablePropertyResolver;
3231
import org.springframework.core.env.Environment;
@@ -131,22 +130,10 @@ public void postProcessBeanFactory(ConfigurableListableBeanFactory beanFactory)
131130
if (this.propertySources == null) {
132131
this.propertySources = new MutablePropertySources();
133132
if (this.environment != null) {
134-
PropertySource<?> environmentPropertySource;
135-
if (this.environment instanceof ConfigurableEnvironment configurableEnvironment) {
136-
environmentPropertySource = new CompositePropertySource(ENVIRONMENT_PROPERTIES_PROPERTY_SOURCE_NAME,
137-
configurableEnvironment.getPropertySources());
138-
}
139-
else {
140-
// Fallback code path that should never apply in a regular scenario, since the
141-
// Environment in the ApplicationContext should always be a ConfigurableEnvironment.
142-
environmentPropertySource =
143-
new PropertySource<>(ENVIRONMENT_PROPERTIES_PROPERTY_SOURCE_NAME, this.environment) {
144-
@Override
145-
public @Nullable Object getProperty(String key) {
146-
return super.source.getProperty(key);
147-
}
148-
};
149-
}
133+
PropertySource<?> environmentPropertySource =
134+
(this.environment instanceof ConfigurableEnvironment configurableEnvironment ?
135+
new ConfigurableEnvironmentPropertySource(configurableEnvironment) :
136+
new FallbackEnvironmentPropertySource(this.environment));
150137
this.propertySources.addLast(environmentPropertySource);
151138
}
152139
try {
@@ -229,4 +216,73 @@ public PropertySources getAppliedPropertySources() throws IllegalStateException
229216
return this.appliedPropertySources;
230217
}
231218

219+
220+
/**
221+
* Custom {@link PropertySource} that delegates to the
222+
* {@link ConfigurableEnvironment#getPropertySources() PropertySources} in a
223+
* {@link ConfigurableEnvironment}.
224+
* @since 6.2.7
225+
*/
226+
private static class ConfigurableEnvironmentPropertySource extends PropertySource<ConfigurableEnvironment> {
227+
228+
private final PropertySources propertySources;
229+
230+
231+
ConfigurableEnvironmentPropertySource(ConfigurableEnvironment environment) {
232+
super(ENVIRONMENT_PROPERTIES_PROPERTY_SOURCE_NAME, environment);
233+
this.propertySources = environment.getPropertySources();
234+
}
235+
236+
237+
@Override
238+
public @Nullable Object getProperty(String name) {
239+
for (PropertySource<?> propertySource : this.propertySources) {
240+
Object candidate = propertySource.getProperty(name);
241+
if (candidate != null) {
242+
return candidate;
243+
}
244+
}
245+
return null;
246+
}
247+
248+
@Override
249+
public boolean containsProperty(String name) {
250+
for (PropertySource<?> propertySource : this.propertySources) {
251+
if (propertySource.containsProperty(name)) {
252+
return true;
253+
}
254+
}
255+
return false;
256+
}
257+
258+
@Override
259+
public String toString() {
260+
return "ConfigurableEnvironmentPropertySource {propertySources=" + this.propertySources + "}";
261+
}
262+
}
263+
264+
/**
265+
* Fallback {@link PropertySource} that delegates to a raw {@link Environment}.
266+
* <p>Should never apply in a regular scenario, since the {@code Environment}
267+
* in an {@code ApplicationContext} should always be a {@link ConfigurableEnvironment}.
268+
* @since 6.2.7
269+
*/
270+
private static class FallbackEnvironmentPropertySource extends PropertySource<Environment> {
271+
272+
FallbackEnvironmentPropertySource(Environment environment) {
273+
super(ENVIRONMENT_PROPERTIES_PROPERTY_SOURCE_NAME, environment);
274+
}
275+
276+
277+
@Override
278+
public @Nullable Object getProperty(String name) {
279+
return super.source.getProperty(name);
280+
}
281+
282+
@Override
283+
public String toString() {
284+
return "FallbackEnvironmentPropertySource {environment=" + super.source + "}";
285+
}
286+
}
287+
232288
}

‎spring-context/src/test/java/org/springframework/context/support/PropertySourcesPlaceholderConfigurerTests.java

Lines changed: 70 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,16 @@
1616

1717
package org.springframework.context.support;
1818

19+
import java.util.ArrayList;
20+
import java.util.Collections;
21+
import java.util.List;
1922
import java.util.Optional;
2023
import java.util.Properties;
2124

2225
import org.junit.jupiter.api.Nested;
2326
import org.junit.jupiter.api.Test;
27+
import org.junit.jupiter.params.ParameterizedTest;
28+
import org.junit.jupiter.params.provider.ValueSource;
2429

2530
import org.springframework.beans.factory.BeanCreationException;
2631
import org.springframework.beans.factory.BeanDefinitionStoreException;
@@ -32,6 +37,7 @@
3237
import org.springframework.context.annotation.Bean;
3338
import org.springframework.context.annotation.Configuration;
3439
import org.springframework.core.convert.support.DefaultConversionService;
40+
import org.springframework.core.env.EnumerablePropertySource;
3541
import org.springframework.core.env.MutablePropertySources;
3642
import org.springframework.core.env.PropertySource;
3743
import org.springframework.core.env.StandardEnvironment;
@@ -90,14 +96,29 @@ void localPropertiesViaResource() {
9096
assertThat(bf.getBean(TestBean.class).getName()).isEqualTo("foo");
9197
}
9298

93-
@Test
94-
void localPropertiesOverrideFalse() {
95-
localPropertiesOverride(false);
96-
}
99+
@ParameterizedTest
100+
@ValueSource(booleans = {true, false})
101+
void localPropertiesOverride(boolean override) {
102+
DefaultListableBeanFactory bf = new DefaultListableBeanFactory();
103+
bf.registerBeanDefinition("testBean",
104+
genericBeanDefinition(TestBean.class)
105+
.addPropertyValue("name", "${foo}")
106+
.getBeanDefinition());
97107

98-
@Test
99-
void localPropertiesOverrideTrue() {
100-
localPropertiesOverride(true);
108+
PropertySourcesPlaceholderConfigurer ppc = new PropertySourcesPlaceholderConfigurer();
109+
110+
ppc.setLocalOverride(override);
111+
ppc.setProperties(new Properties() {{
112+
setProperty("foo", "local");
113+
}});
114+
ppc.setEnvironment(new MockEnvironment().withProperty("foo", "enclosing"));
115+
ppc.postProcessBeanFactory(bf);
116+
if (override) {
117+
assertThat(bf.getBean(TestBean.class).getName()).isEqualTo("local");
118+
}
119+
else {
120+
assertThat(bf.getBean(TestBean.class).getName()).isEqualTo("enclosing");
121+
}
101122
}
102123

103124
@Test
@@ -283,28 +304,58 @@ public Object getProperty(String key) {
283304
assertThat(bf.getBean(TestBean.class).getName()).isEqualTo("bar");
284305
}
285306

286-
@SuppressWarnings("serial")
287-
private void localPropertiesOverride(boolean override) {
307+
@Test // gh-34861
308+
void withEnumerableAndNonEnumerablePropertySourcesInTheEnvironmentAndLocalProperties() {
288309
DefaultListableBeanFactory bf = new DefaultListableBeanFactory();
289310
bf.registerBeanDefinition("testBean",
290311
genericBeanDefinition(TestBean.class)
291-
.addPropertyValue("name", "${foo}")
312+
.addPropertyValue("name", "${foo:bogus}")
313+
.addPropertyValue("jedi", "${local:false}")
292314
.getBeanDefinition());
293315

294-
PropertySourcesPlaceholderConfigurer ppc = new PropertySourcesPlaceholderConfigurer();
316+
// 1) MockPropertySource is an EnumerablePropertySource.
317+
MockPropertySource mockPropertySource = new MockPropertySource("mockPropertySource")
318+
.withProperty("foo", "${bar}");
295319

296-
ppc.setLocalOverride(override);
320+
// 2) PropertySource is not an EnumerablePropertySource.
321+
PropertySource<?> rawPropertySource = new PropertySource<>("rawPropertySource", new Object()) {
322+
@Override
323+
public Object getProperty(String key) {
324+
return ("bar".equals(key) ? "quux" : null);
325+
}
326+
};
327+
328+
MockEnvironment env = new MockEnvironment();
329+
env.getPropertySources().addFirst(mockPropertySource);
330+
env.getPropertySources().addLast(rawPropertySource);
331+
332+
PropertySourcesPlaceholderConfigurer ppc = new PropertySourcesPlaceholderConfigurer();
333+
ppc.setEnvironment(env);
334+
// 3) Local properties are stored in a PropertiesPropertySource which is an EnumerablePropertySource.
297335
ppc.setProperties(new Properties() {{
298-
setProperty("foo", "local");
336+
setProperty("local", "true");
299337
}});
300-
ppc.setEnvironment(new MockEnvironment().withProperty("foo", "enclosing"));
301338
ppc.postProcessBeanFactory(bf);
302-
if (override) {
303-
assertThat(bf.getBean(TestBean.class).getName()).isEqualTo("local");
304-
}
305-
else {
306-
assertThat(bf.getBean(TestBean.class).getName()).isEqualTo("enclosing");
339+
340+
// Verify all properties can be resolved via the Environment.
341+
assertThat(env.getProperty("foo")).isEqualTo("quux");
342+
assertThat(env.getProperty("bar")).isEqualTo("quux");
343+
344+
// Verify that placeholder resolution works.
345+
TestBean testBean = bf.getBean(TestBean.class);
346+
assertThat(testBean.getName()).isEqualTo("quux");
347+
assertThat(testBean.isJedi()).isTrue();
348+
349+
// Verify that the presence of a non-EnumerablePropertySource does not prevent
350+
// accessing EnumerablePropertySources via getAppliedPropertySources().
351+
List<String> propertyNames = new ArrayList<>();
352+
for (PropertySource<?> propertySource : ppc.getAppliedPropertySources()) {
353+
if (propertySource instanceof EnumerablePropertySource<?> enumerablePropertySource) {
354+
Collections.addAll(propertyNames, enumerablePropertySource.getPropertyNames());
355+
}
307356
}
357+
// Should not contain "foo" or "bar" from the Environment.
358+
assertThat(propertyNames).containsOnly("local");
308359
}
309360

310361
@Test

0 commit comments

Comments
 (0)