Revised ResourcePropertySource in order to avoid ConfigurationClassProcessor's AnnotationPropertySource subclass

Issue: SPR-12115

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

@@ -34,6 +34,7 @@ import java.util.Stack;
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
+
 import org.springframework.beans.BeanUtils;
 import org.springframework.beans.factory.Aware;
 import org.springframework.beans.factory.BeanClassLoaderAware;
@@ -244,13 +245,13 @@ class ConfigurationClassParser {
 		// Process any @PropertySource annotations
 		for (AnnotationAttributes propertySource : AnnotationConfigUtils.attributesForRepeatable(
 				sourceClass.getMetadata(), PropertySources.class, org.springframework.context.annotation.PropertySource.class)) {
-			if (!(this.environment instanceof ConfigurableEnvironment)) {
+			if (this.environment instanceof ConfigurableEnvironment) {
-				logger.warn("Ignoring @PropertySource annotation on "
-						+ sourceClass.getMetadata().getClassName()
-						+ "Reason: Environment must implement ConfigurableEnvironment");
-			} else {
 				processPropertySource(propertySource);
 			}
+			else {
+				logger.warn("Ignoring @PropertySource annotation on [" + sourceClass.getMetadata().getClassName() +
+						"]. Reason: Environment must implement ConfigurableEnvironment");
+			}
 		}
 
 		// Process any @ComponentScan annotations
@@ -329,7 +330,11 @@ class ConfigurationClassParser {
 		Assert.isTrue(locations.length > 0, "At least one @PropertySource(value) location is required");
 		for (String location : locations) {
 			try {
-				processPropertySourceLocation(name, location);
+				String resolvedLocation = this.environment.resolveRequiredPlaceholders(location);
+				Resource resource = this.resourceLoader.getResource(resolvedLocation);
+				ResourcePropertySource rps = (StringUtils.hasText(name) ?
+						new ResourcePropertySource(name, resource) : new ResourcePropertySource(resource));
+				addPropertySource(rps);
 			}
 			catch (IllegalArgumentException ex) {
 				// from resolveRequiredPlaceholders
@@ -346,38 +351,30 @@ class ConfigurationClassParser {
 		}
 	}
 
-	private void processPropertySourceLocation(String name, String location) throws IOException, FileNotFoundException {
+	private void addPropertySource(ResourcePropertySource propertySource) {
-		String resolvedLocation = this.environment.resolveRequiredPlaceholders(location);
-		Resource resource = this.resourceLoader.getResource(resolvedLocation);
-		AnnotationPropertySource propertySource = (StringUtils.hasText(name) ?
-				new AnnotationPropertySource(name, resource) : new AnnotationPropertySource(resource));
-		addPropertySource(propertySource);
-	}
-
-	private void addPropertySource(AnnotationPropertySource propertySource) {
 		String name = propertySource.getName();
 		MutablePropertySources propertySources = ((ConfigurableEnvironment) this.environment).getPropertySources();
 		if (propertySources.contains(name) && this.propertySourceNames.contains(name)) {
 			// We've already added a version, we need to extend it
 			PropertySource<?> existing = propertySources.get(name);
 			if (existing instanceof CompositePropertySource) {
-				((CompositePropertySource) existing).addFirstPropertySource(
+				((CompositePropertySource) existing).addFirstPropertySource(propertySource.withResourceName());
-						propertySource.resourceNamedPropertySource());
 			}
 			else {
-				if (existing instanceof AnnotationPropertySource) {
+				if (existing instanceof ResourcePropertySource) {
-					existing = ((AnnotationPropertySource) existing).resourceNamedPropertySource();
+					existing = ((ResourcePropertySource) existing).withResourceName();
 				}
 				CompositePropertySource composite = new CompositePropertySource(name);
-				composite.addPropertySource(propertySource.resourceNamedPropertySource());
+				composite.addPropertySource(propertySource.withResourceName());
 				composite.addPropertySource(existing);
 				propertySources.replace(name, composite);
 			}
 		}
 		else {
-			if(this.propertySourceNames.isEmpty()) {
+			if (this.propertySourceNames.isEmpty()) {
 				propertySources.addLast(propertySource);
-			} else {
+			}
+			else {
 				String firstProcessed = this.propertySourceNames.iterator().next();
 				propertySources.addBefore(firstProcessed, propertySource);
 			}
@@ -831,34 +828,4 @@ class ConfigurationClassParser {
 		}
 	}
 
-
-	private static class AnnotationPropertySource extends ResourcePropertySource {
-
-		/** The resource name or null if the same as getName() */
-		private final String resourceName;
-
-		public AnnotationPropertySource(Resource resource) throws IOException {
-			super(resource);
-			this.resourceName = null;
-		}
-
-		public AnnotationPropertySource(String name, Resource resource) throws IOException {
-			super(name, resource);
-			this.resourceName = getNameForResource(resource);
-		}
-
-		protected AnnotationPropertySource(String name, Map<String, Object> source) {
-			super(name, source);
-			this.resourceName = null;
-		}
-
-		public AnnotationPropertySource resourceNamedPropertySource() {
-			if (this.resourceName == null) {
-				return this;
-			}
-			return new AnnotationPropertySource(this.resourceName, getSource());
-		}
-
-	}
-
 }

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

@@ -362,6 +362,7 @@ public class PropertySourceAnnotationTests {
 	static class ConfigWithEmptyResourceLocations {
 	}
 
+
 	@Import({ ConfigImportedWithSameSourceImportedInDifferentOrder.class })
 	@PropertySources({
 		@PropertySource("classpath:org/springframework/context/annotation/p1.properties"),
@@ -372,6 +373,7 @@ public class PropertySourceAnnotationTests {
 
 	}
 
+
 	@Configuration
 	@PropertySources({
 		@PropertySource("classpath:org/springframework/context/annotation/p2.properties"),

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

@@ -55,15 +55,24 @@ public class CompositePropertySource extends PropertySource<Object> {
 		return null;
 	}
 
+	/**
+	 * Add the given {@link PropertySource} to the end of the chain.
+	 * @param propertySource the PropertySource to add
+	 */
 	public void addPropertySource(PropertySource<?> propertySource) {
 		this.propertySources.add(propertySource);
 	}
 
+	/**
+	 * Add the given {@link PropertySource} to the start of the chain.
+	 * @param propertySource the PropertySource to add
+	 * @since 4.1
+	 */
 	public void addFirstPropertySource(PropertySource<?> propertySource) {
-		List<PropertySource<?>> exisiting = new ArrayList<PropertySource<?>>(this.propertySources);
+		List<PropertySource<?>> existing = new ArrayList<PropertySource<?>>(this.propertySources);
 		this.propertySources.clear();
 		this.propertySources.add(propertySource);
-		this.propertySources.addAll(exisiting);
+		this.propertySources.addAll(existing);
 	}
 
 	@Override

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

@@ -42,12 +42,17 @@ import org.springframework.util.StringUtils;
  */
 public class ResourcePropertySource extends PropertiesPropertySource {
 
+	/** The original resource name, if different from the given name */
+	private final String resourceName;
+
+
 	/**
 	 * Create a PropertySource having the given name based on Properties
 	 * loaded from the given encoded resource.
 	 */
 	public ResourcePropertySource(String name, EncodedResource resource) throws IOException {
 		super(name, PropertiesLoaderUtils.loadProperties(resource));
+		this.resourceName = getNameForResource(resource.getResource());
 	}
 
 	/**
@@ -56,7 +61,8 @@ public class ResourcePropertySource extends PropertiesPropertySource {
 	 * {@link Resource#getDescription() description} of the given resource.
 	 */
 	public ResourcePropertySource(EncodedResource resource) throws IOException {
-		this(getNameForResource(resource.getResource()), resource);
+		super(getNameForResource(resource.getResource()), PropertiesLoaderUtils.loadProperties(resource));
+		this.resourceName = null;
 	}
 
 	/**
@@ -65,6 +71,7 @@ public class ResourcePropertySource extends PropertiesPropertySource {
 	 */
 	public ResourcePropertySource(String name, Resource resource) throws IOException {
 		super(name, PropertiesLoaderUtils.loadProperties(new EncodedResource(resource)));
+		this.resourceName = getNameForResource(resource);
 	}
 
 	/**
@@ -73,7 +80,8 @@ public class ResourcePropertySource extends PropertiesPropertySource {
 	 * {@link Resource#getDescription() description} of the given resource.
 	 */
 	public ResourcePropertySource(Resource resource) throws IOException {
-		this(getNameForResource(resource), resource);
+		super(getNameForResource(resource), PropertiesLoaderUtils.loadProperties(new EncodedResource(resource)));
+		this.resourceName = null;
 	}
 
 	/**
@@ -115,20 +123,47 @@ public class ResourcePropertySource extends PropertiesPropertySource {
 		this(new DefaultResourceLoader().getResource(location));
 	}
 
-	protected ResourcePropertySource(String name, Map<String, Object> source) {
+	private ResourcePropertySource(String name, String resourceName, Map<String, Object> source) {
 		super(name, source);
+		this.resourceName = resourceName;
 	}
 
 
 	/**
 	 * Return a potentially adapted variant of this {@link ResourcePropertySource},
 	 * overriding the previously given (or derived) name with the specified name.
+	 * @since 4.0.4
 	 */
 	public ResourcePropertySource withName(String name) {
 		if (this.name.equals(name)) {
 			return this;
 		}
-		return new ResourcePropertySource(name, this.source);
+		// Store the original resource name if necessary...
+		if (this.resourceName != null) {
+			if (this.resourceName.equals(name)) {
+				return new ResourcePropertySource(this.resourceName, null, this.source);
+			}
+			else {
+				return new ResourcePropertySource(name, this.resourceName, this.source);
+			}
+		}
+		else {
+			// Current name is resource name -> preserve it in the extra field...
+			return new ResourcePropertySource(name, this.name, this.source);
+		}
+	}
+
+	/**
+	 * Return a potentially adapted variant of this {@link ResourcePropertySource},
+	 * overriding the previously given name (if any) with the original resource name
+	 * (equivalent to the name generated by the name-less constructor variants).
+	 * @since 4.1
+	 */
+	public ResourcePropertySource withResourceName() {
+		if (this.resourceName == null) {
+			return this;
+		}
+		return new ResourcePropertySource(this.resourceName, null, this.source);
 	}
 
 
@@ -137,7 +172,7 @@ public class ResourcePropertySource extends PropertiesPropertySource {
 	 * empty, return the class name of the resource plus its identity hash code.
 	 * @see org.springframework.core.io.Resource#getDescription()
 	 */
-	protected static String getNameForResource(Resource resource) {
+	private static String getNameForResource(Resource resource) {
 		String name = resource.getDescription();
 		if (!StringUtils.hasText(name)) {
 			name = resource.getClass().getSimpleName() + "@" + System.identityHashCode(resource);

spring-core/src/test/java/org/springframework/core/env/CompositePropertySourceTests.java

@@ -24,6 +24,7 @@ import static org.junit.Assert.*;
 
 /**
  * Tests for {@link CompositePropertySource}.
+ *
  * @author Phillip Webb
  */
 public class CompositePropertySourceTests {