Skip to content

Commit ab24dda

Browse files
committed
Revised @propertysource parsing for consistent PropertySource naming, avoiding accidental overriding by name
Issue: SPR-11637
1 parent ce4912b commit ab24dda

File tree

4 files changed

+74
-38
lines changed

4 files changed

+74
-38
lines changed

spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClassParser.java

Lines changed: 9 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,8 @@ public int compare(DeferredImportSelectorHolder o1, DeferredImportSelectorHolder
116116

117117
private final Map<String, ConfigurationClass> knownSuperclasses = new HashMap<String, ConfigurationClass>();
118118

119-
private final MultiValueMap<String, PropertySource<?>> propertySources = new LinkedMultiValueMap<String, PropertySource<?>>();
119+
private final MultiValueMap<String, ResourcePropertySource> propertySources =
120+
new LinkedMultiValueMap<String, ResourcePropertySource>();
120121

121122
private final ImportStack importStack = new ImportStack();
122123

@@ -310,16 +311,10 @@ private void processPropertySource(AnnotationAttributes propertySource) throws I
310311
}
311312
for (String location : locations) {
312313
try {
313-
Resource resource = this.resourceLoader.getResource(
314-
this.environment.resolveRequiredPlaceholders(location));
315-
if (!StringUtils.hasText(name) || this.propertySources.containsKey(name)) {
316-
// We need to ensure unique names when the property source will ultimately end up in a composite
317-
ResourcePropertySource ps = new ResourcePropertySource(resource);
318-
this.propertySources.add((StringUtils.hasText(name) ? name : ps.getName()), ps);
319-
}
320-
else {
321-
this.propertySources.add(name, new ResourcePropertySource(name, resource));
322-
}
314+
String resolvedLocation = this.environment.resolveRequiredPlaceholders(location);
315+
Resource resource = this.resourceLoader.getResource(resolvedLocation);
316+
ResourcePropertySource ps = new ResourcePropertySource(resource);
317+
this.propertySources.add((StringUtils.hasText(name) ? name : ps.getName()), ps);
323318
}
324319
catch (IllegalArgumentException ex) {
325320
// from resolveRequiredPlaceholders
@@ -482,15 +477,15 @@ public Set<ConfigurationClass> getConfigurationClasses() {
482477

483478
public List<PropertySource<?>> getPropertySources() {
484479
List<PropertySource<?>> propertySources = new LinkedList<PropertySource<?>>();
485-
for (Map.Entry<String, List<PropertySource<?>>> entry : this.propertySources.entrySet()) {
480+
for (Map.Entry<String, List<ResourcePropertySource>> entry : this.propertySources.entrySet()) {
486481
propertySources.add(0, collatePropertySources(entry.getKey(), entry.getValue()));
487482
}
488483
return propertySources;
489484
}
490485

491-
private PropertySource<?> collatePropertySources(String name, List<PropertySource<?>> propertySources) {
486+
private PropertySource<?> collatePropertySources(String name, List<ResourcePropertySource> propertySources) {
492487
if (propertySources.size() == 1) {
493-
return propertySources.get(0);
488+
return propertySources.get(0).withName(name);
494489
}
495490
CompositePropertySource result = new CompositePropertySource(name);
496491
for (int i = propertySources.size() - 1; i >= 0; i--) {

spring-context/src/test/java/org/springframework/context/annotation/PropertySourceAnnotationTests.java

Lines changed: 35 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -124,9 +124,14 @@ public void withResolvablePlaceholder() {
124124
System.clearProperty("path.to.properties");
125125
}
126126

127-
/**
128-
* SPR-10820
129-
*/
127+
@Test(expected = IllegalArgumentException.class)
128+
public void withEmptyResourceLocations() {
129+
AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext();
130+
ctx.register(ConfigWithEmptyResourceLocations.class);
131+
ctx.refresh();
132+
}
133+
134+
// SPR-10820
130135
@Test
131136
public void orderingWithAndWithoutNameAndMultipleResourceLocations() {
132137
// p2 should 'win' as it was registered last
@@ -136,13 +141,6 @@ public void orderingWithAndWithoutNameAndMultipleResourceLocations() {
136141
assertThat(ctxWithName.getEnvironment().getProperty("testbean.name"), equalTo("p2TestBean"));
137142
}
138143

139-
@Test(expected=IllegalArgumentException.class)
140-
public void withEmptyResourceLocations() {
141-
AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext();
142-
ctx.register(ConfigWithEmptyResourceLocations.class);
143-
ctx.refresh();
144-
}
145-
146144
@Test
147145
public void withNameAndMultipleResourceLocations() {
148146
AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(ConfigWithNameAndMultipleResourceLocations.class);
@@ -170,6 +168,15 @@ public void withPropertySources() {
170168
assertThat(ctx.getEnvironment().getProperty("testbean.name"), equalTo("p2TestBean"));
171169
}
172170

171+
@Test
172+
public void withNamedPropertySources() {
173+
AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(ConfigWithNamedPropertySources.class);
174+
assertThat(ctx.getEnvironment().containsProperty("from.p1"), is(true));
175+
assertThat(ctx.getEnvironment().containsProperty("from.p2"), is(true));
176+
// p2 should 'win' as it was registered last
177+
assertThat(ctx.getEnvironment().getProperty("testbean.name"), equalTo("p2TestBean"));
178+
}
179+
173180
@Test
174181
public void withMissingPropertySource() {
175182
thrown.expect(BeanDefinitionStoreException.class);
@@ -269,29 +276,38 @@ static class ConfigWithMultipleResourceLocations {
269276

270277
@Configuration
271278
@PropertySources({
272-
@PropertySource(name = "psName", value="classpath:org/springframework/context/annotation/p1.properties"),
273-
@PropertySource(name = "psName", value="classpath:org/springframework/context/annotation/p2.properties")
279+
@PropertySource("classpath:org/springframework/context/annotation/p1.properties"),
280+
@PropertySource("classpath:org/springframework/context/annotation/p2.properties"),
274281
})
275282
static class ConfigWithPropertySources {
276283
}
277284

278285

279286
@Configuration
280287
@PropertySources({
281-
@PropertySource(name = "psName", value="classpath:org/springframework/context/annotation/p1.properties"),
282-
@PropertySource(name = "psName", value="classpath:org/springframework/context/annotation/missing.properties"),
283-
@PropertySource(name = "psName", value="classpath:org/springframework/context/annotation/p2.properties")
288+
@PropertySource(name = "psName", value = "classpath:org/springframework/context/annotation/p1.properties"),
289+
@PropertySource(name = "psName", value = "classpath:org/springframework/context/annotation/p2.properties"),
290+
})
291+
static class ConfigWithNamedPropertySources {
292+
}
293+
294+
295+
@Configuration
296+
@PropertySources({
297+
@PropertySource(name = "psName", value = "classpath:org/springframework/context/annotation/p1.properties"),
298+
@PropertySource(name = "psName", value = "classpath:org/springframework/context/annotation/missing.properties"),
299+
@PropertySource(name = "psName", value = "classpath:org/springframework/context/annotation/p2.properties")
284300
})
285301
static class ConfigWithMissingPropertySource {
286302
}
287303

288304

289305
@Configuration
290306
@PropertySources({
291-
@PropertySource(name = "psName", value="classpath:org/springframework/context/annotation/p1.properties"),
292-
@PropertySource(name = "psName", value="classpath:org/springframework/context/annotation/missing.properties", ignoreResourceNotFound=true),
293-
@PropertySource(name = "psName", value="classpath:${myPath}/missing.properties", ignoreResourceNotFound=true),
294-
@PropertySource(name = "psName", value="classpath:org/springframework/context/annotation/p2.properties")
307+
@PropertySource(name = "psName", value = "classpath:org/springframework/context/annotation/p1.properties"),
308+
@PropertySource(name = "psName", value = "classpath:org/springframework/context/annotation/missing.properties", ignoreResourceNotFound=true),
309+
@PropertySource(name = "psName", value = "classpath:${myPath}/missing.properties", ignoreResourceNotFound=true),
310+
@PropertySource(name = "psName", value = "classpath:org/springframework/context/annotation/p2.properties")
295311
})
296312
static class ConfigWithIgnoredPropertySource {
297313
}

spring-core/src/main/java/org/springframework/core/env/PropertiesPropertySource.java

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2013 the original author or authors.
2+
* Copyright 2002-2014 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -30,13 +30,18 @@
3030
* {@link Properties#getProperty} and {@link Properties#setProperty}.
3131
*
3232
* @author Chris Beams
33+
* @author Juergen Hoeller
3334
* @since 3.1
3435
*/
3536
public class PropertiesPropertySource extends MapPropertySource {
3637

37-
@SuppressWarnings({ "unchecked", "rawtypes" })
38+
@SuppressWarnings({"unchecked", "rawtypes"})
3839
public PropertiesPropertySource(String name, Properties source) {
3940
super(name, (Map) source);
4041
}
4142

43+
protected PropertiesPropertySource(String name, Map<String, Object> source) {
44+
super(name, source);
45+
}
46+
4247
}

spring-core/src/main/java/org/springframework/core/io/support/ResourcePropertySource.java

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2013 the original author or authors.
2+
* Copyright 2002-2014 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -17,6 +17,7 @@
1717
package org.springframework.core.io.support;
1818

1919
import java.io.IOException;
20+
import java.util.Map;
2021
import java.util.Properties;
2122

2223
import org.springframework.core.env.PropertiesPropertySource;
@@ -36,6 +37,8 @@
3637
* @author Chris Beams
3738
* @author Juergen Hoeller
3839
* @since 3.1
40+
* @see org.springframework.core.io.Resource
41+
* @see org.springframework.core.io.support.EncodedResource
3942
*/
4043
public class ResourcePropertySource extends PropertiesPropertySource {
4144

@@ -112,10 +115,27 @@ public ResourcePropertySource(String location) throws IOException {
112115
this(new DefaultResourceLoader().getResource(location));
113116
}
114117

118+
private ResourcePropertySource(String name, Map<String, Object> source) {
119+
super(name, source);
120+
}
121+
122+
123+
/**
124+
* Return a potentially adapted variant of this {@link ResourcePropertySource},
125+
* overriding the previously given (or derived) name with the specified name.
126+
*/
127+
public ResourcePropertySource withName(String name) {
128+
if (this.name.equals(name)) {
129+
return this;
130+
}
131+
return new ResourcePropertySource(name, this.source);
132+
}
133+
115134

116135
/**
117-
* Return the description string for the resource, and if empty returns
118-
* the class name of the resource plus its identity hash code.
136+
* Return the description String for the given Resource; it the description is
137+
* empty, return the class name of the resource plus its identity hash code.
138+
* @see org.springframework.core.io.Resource#getDescription()
119139
*/
120140
private static String getNameForResource(Resource resource) {
121141
String name = resource.getDescription();

0 commit comments

Comments
 (0)