Revised @PropertySource parsing for consistent PropertySource naming, avoiding accidental overriding by name

Issue: SPR-11637

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

@@ -116,7 +116,8 @@ class ConfigurationClassParser {
 
 	private final Map<String, ConfigurationClass> knownSuperclasses = new HashMap<String, ConfigurationClass>();
 
-	private final MultiValueMap<String, PropertySource<?>> propertySources = new LinkedMultiValueMap<String, PropertySource<?>>();
+	private final MultiValueMap<String, ResourcePropertySource> propertySources =
+			new LinkedMultiValueMap<String, ResourcePropertySource>();
 
 	private final ImportStack importStack = new ImportStack();
 
@@ -310,16 +311,10 @@ class ConfigurationClassParser {
 		}
 		for (String location : locations) {
 			try {
-				Resource resource = this.resourceLoader.getResource(
+				String resolvedLocation = this.environment.resolveRequiredPlaceholders(location);
-						this.environment.resolveRequiredPlaceholders(location));
+				Resource resource = this.resourceLoader.getResource(resolvedLocation);
-				if (!StringUtils.hasText(name) || this.propertySources.containsKey(name)) {
+				ResourcePropertySource ps = new ResourcePropertySource(resource);
-					// We need to ensure unique names when the property source will ultimately end up in a composite
+				this.propertySources.add((StringUtils.hasText(name) ? name : ps.getName()), ps);
-					ResourcePropertySource ps = new ResourcePropertySource(resource);
-					this.propertySources.add((StringUtils.hasText(name) ? name : ps.getName()), ps);
-				}
-				else {
-					this.propertySources.add(name, new ResourcePropertySource(name, resource));
-				}
 			}
 			catch (IllegalArgumentException ex) {
 				// from resolveRequiredPlaceholders
@@ -482,15 +477,15 @@ class ConfigurationClassParser {
 
 	public List<PropertySource<?>> getPropertySources() {
 		List<PropertySource<?>> propertySources = new LinkedList<PropertySource<?>>();
-		for (Map.Entry<String, List<PropertySource<?>>> entry : this.propertySources.entrySet()) {
+		for (Map.Entry<String, List<ResourcePropertySource>> entry : this.propertySources.entrySet()) {
 			propertySources.add(0, collatePropertySources(entry.getKey(), entry.getValue()));
 		}
 		return propertySources;
 	}
 
-	private PropertySource<?> collatePropertySources(String name, List<PropertySource<?>> propertySources) {
+	private PropertySource<?> collatePropertySources(String name, List<ResourcePropertySource> propertySources) {
 		if (propertySources.size() == 1) {
-			return propertySources.get(0);
+			return propertySources.get(0).withName(name);
 		}
 		CompositePropertySource result = new CompositePropertySource(name);
 		for (int i = propertySources.size() - 1; i >= 0; i--) {

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

@@ -124,9 +124,14 @@ public class PropertySourceAnnotationTests {
 		System.clearProperty("path.to.properties");
 	}
 
-	/**
+	@Test(expected = IllegalArgumentException.class)
-	 * SPR-10820
+	public void withEmptyResourceLocations() {
-	 */
+		AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext();
+		ctx.register(ConfigWithEmptyResourceLocations.class);
+		ctx.refresh();
+	}
+
+	// SPR-10820
 	@Test
 	public void orderingWithAndWithoutNameAndMultipleResourceLocations() {
 		// p2 should 'win' as it was registered last
@@ -136,13 +141,6 @@ public class PropertySourceAnnotationTests {
 		assertThat(ctxWithName.getEnvironment().getProperty("testbean.name"), equalTo("p2TestBean"));
 	}
 
-	@Test(expected=IllegalArgumentException.class)
-	public void withEmptyResourceLocations() {
-		AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext();
-		ctx.register(ConfigWithEmptyResourceLocations.class);
-		ctx.refresh();
-	}
-
 	@Test
 	public void withNameAndMultipleResourceLocations() {
 		AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(ConfigWithNameAndMultipleResourceLocations.class);
@@ -170,6 +168,15 @@ public class PropertySourceAnnotationTests {
 		assertThat(ctx.getEnvironment().getProperty("testbean.name"), equalTo("p2TestBean"));
 	}
 
+	@Test
+	public void withNamedPropertySources() {
+		AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(ConfigWithNamedPropertySources.class);
+		assertThat(ctx.getEnvironment().containsProperty("from.p1"), is(true));
+		assertThat(ctx.getEnvironment().containsProperty("from.p2"), is(true));
+		// p2 should 'win' as it was registered last
+		assertThat(ctx.getEnvironment().getProperty("testbean.name"), equalTo("p2TestBean"));
+	}
+
 	@Test
 	public void withMissingPropertySource() {
 		thrown.expect(BeanDefinitionStoreException.class);
@@ -269,8 +276,8 @@ public class PropertySourceAnnotationTests {
 
 	@Configuration
 	@PropertySources({
-		@PropertySource(name = "psName", value="classpath:org/springframework/context/annotation/p1.properties"),
+		@PropertySource("classpath:org/springframework/context/annotation/p1.properties"),
-		@PropertySource(name = "psName", value="classpath:org/springframework/context/annotation/p2.properties")
+		@PropertySource("classpath:org/springframework/context/annotation/p2.properties"),
 	})
 	static class ConfigWithPropertySources {
 	}
@@ -278,9 +285,18 @@ public class PropertySourceAnnotationTests {
 
 	@Configuration
 	@PropertySources({
-		@PropertySource(name = "psName", value="classpath:org/springframework/context/annotation/p1.properties"),
+			@PropertySource(name = "psName", value = "classpath:org/springframework/context/annotation/p1.properties"),
-		@PropertySource(name = "psName", value="classpath:org/springframework/context/annotation/missing.properties"),
+			@PropertySource(name = "psName", value = "classpath:org/springframework/context/annotation/p2.properties"),
-		@PropertySource(name = "psName", value="classpath:org/springframework/context/annotation/p2.properties")
+	})
+	static class ConfigWithNamedPropertySources {
+	}
+
+
+	@Configuration
+	@PropertySources({
+		@PropertySource(name = "psName", value = "classpath:org/springframework/context/annotation/p1.properties"),
+		@PropertySource(name = "psName", value = "classpath:org/springframework/context/annotation/missing.properties"),
+		@PropertySource(name = "psName", value = "classpath:org/springframework/context/annotation/p2.properties")
 	})
 	static class ConfigWithMissingPropertySource {
 	}
@@ -288,10 +304,10 @@ public class PropertySourceAnnotationTests {
 
 	@Configuration
 	@PropertySources({
-		@PropertySource(name = "psName", value="classpath:org/springframework/context/annotation/p1.properties"),
+		@PropertySource(name = "psName", value = "classpath:org/springframework/context/annotation/p1.properties"),
-		@PropertySource(name = "psName", value="classpath:org/springframework/context/annotation/missing.properties", ignoreResourceNotFound=true),
+		@PropertySource(name = "psName", value = "classpath:org/springframework/context/annotation/missing.properties", ignoreResourceNotFound=true),
-		@PropertySource(name = "psName", value="classpath:${myPath}/missing.properties", ignoreResourceNotFound=true),
+		@PropertySource(name = "psName", value = "classpath:${myPath}/missing.properties", ignoreResourceNotFound=true),
-		@PropertySource(name = "psName", value="classpath:org/springframework/context/annotation/p2.properties")
+		@PropertySource(name = "psName", value = "classpath:org/springframework/context/annotation/p2.properties")
 	})
 	static class ConfigWithIgnoredPropertySource {
 	}

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2013 the original author or authors.
+ * Copyright 2002-2014 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.
@@ -30,13 +30,18 @@ import java.util.Properties;
  * {@link Properties#getProperty} and {@link Properties#setProperty}.
  *
  * @author Chris Beams
+ * @author Juergen Hoeller
  * @since 3.1
  */
 public class PropertiesPropertySource extends MapPropertySource {
 
-	@SuppressWarnings({ "unchecked", "rawtypes" })
+	@SuppressWarnings({"unchecked", "rawtypes"})
 	public PropertiesPropertySource(String name, Properties source) {
 		super(name, (Map) source);
 	}
 
+	protected PropertiesPropertySource(String name, Map<String, Object> source) {
+		super(name, source);
+	}
+
 }

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2013 the original author or authors.
+ * Copyright 2002-2014 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.
@@ -17,6 +17,7 @@
 package org.springframework.core.io.support;
 
 import java.io.IOException;
+import java.util.Map;
 import java.util.Properties;
 
 import org.springframework.core.env.PropertiesPropertySource;
@@ -36,6 +37,8 @@ import org.springframework.util.StringUtils;
  * @author Chris Beams
  * @author Juergen Hoeller
  * @since 3.1
+ * @see org.springframework.core.io.Resource
+ * @see org.springframework.core.io.support.EncodedResource
  */
 public class ResourcePropertySource extends PropertiesPropertySource {
 
@@ -112,10 +115,27 @@ public class ResourcePropertySource extends PropertiesPropertySource {
 		this(new DefaultResourceLoader().getResource(location));
 	}
 
+	private ResourcePropertySource(String name, Map<String, Object> source) {
+		super(name, source);
+	}
+
+
+	/**
+	 * Return a potentially adapted variant of this {@link ResourcePropertySource},
+	 * overriding the previously given (or derived) name with the specified name.
+	 */
+	public ResourcePropertySource withName(String name) {
+		if (this.name.equals(name)) {
+			return this;
+		}
+		return new ResourcePropertySource(name, this.source);
+	}
+
 
 	/**
-	 * Return the description string for the resource, and if empty returns
+	 * Return the description String for the given Resource; it the description is
-	 * the class name of the resource plus its identity hash code.
+	 * empty, return the class name of the resource plus its identity hash code.
+	 * @see org.springframework.core.io.Resource#getDescription()
 	 */
 	private static String getNameForResource(Resource resource) {
 		String name = resource.getDescription();