@PropertySource gets parsed as early as possible

Other PropertySources and in particular @ComponentScan can benefit from previously declared property sources on the same configuration class.

Issue: SPR-12110
Issue: SPR-12111

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

@@ -55,7 +55,9 @@ import org.springframework.core.NestedIOException;
 import org.springframework.core.annotation.AnnotationAttributes;
 import org.springframework.core.annotation.AnnotationAwareOrderComparator;
 import org.springframework.core.env.CompositePropertySource;
+import org.springframework.core.env.ConfigurableEnvironment;
 import org.springframework.core.env.Environment;
+import org.springframework.core.env.MutablePropertySources;
 import org.springframework.core.env.PropertySource;
 import org.springframework.core.io.Resource;
 import org.springframework.core.io.ResourceLoader;
@@ -120,8 +122,8 @@ class ConfigurationClassParser {
 
 	private final Map<String, ConfigurationClass> knownSuperclasses = new HashMap<String, ConfigurationClass>();
 
-	private final MultiValueMap<String, ResourcePropertySource> propertySources =
-			new LinkedMultiValueMap<String, ResourcePropertySource>();
+	private final MultiValueMap<Object, PropertySourceDescriptor> propertySources =
+			new LinkedMultiValueMap<Object, PropertySourceDescriptor>();
 
 	private final ImportStack importStack = new ImportStack();
 
@@ -234,16 +236,19 @@ class ConfigurationClassParser {
 	 * @return the superclass, or {@code null} if none found or previously processed
 	 */
 	protected final SourceClass doProcessConfigurationClass(ConfigurationClass configClass, SourceClass sourceClass) throws IOException {
-		// recursively process any member (nested) classes first
+		// Recursively process any member (nested) classes first
 		processMemberClasses(configClass, sourceClass);
 
-		// process any @PropertySource annotations
+		// Process any @PropertySource annotations
 		for (AnnotationAttributes propertySource : AnnotationConfigUtils.attributesForRepeatable(
 				sourceClass.getMetadata(), PropertySources.class, org.springframework.context.annotation.PropertySource.class)) {
 			processPropertySource(propertySource);
 		}
 
-		// process any @ComponentScan annotations
+		// Register PropertySources with Environment
+		registerPropertySources();
+
+		// Process any @ComponentScan annotations
 		AnnotationAttributes componentScan = AnnotationConfigUtils.attributesFor(sourceClass.getMetadata(), ComponentScan.class);
 		if (componentScan != null) {
 			// the config class is annotated with @ComponentScan -> perform the scan immediately
@@ -260,10 +265,10 @@ class ConfigurationClassParser {
 			}
 		}
 
-		// process any @Import annotations
+		// Process any @Import annotations
 		processImports(configClass, sourceClass, getImports(sourceClass), true, false);
 
-		// process any @ImportResource annotations
+		// Process any @ImportResource annotations
 		if (sourceClass.getMetadata().isAnnotated(ImportResource.class.getName())) {
 			AnnotationAttributes importResource = AnnotationConfigUtils.attributesFor(sourceClass.getMetadata(), ImportResource.class);
 			String[] resources = importResource.getStringArray("value");
@@ -274,23 +279,23 @@ class ConfigurationClassParser {
 			}
 		}
 
-		// process individual @Bean methods
+		// Process individual @Bean methods
 		Set<MethodMetadata> beanMethods = sourceClass.getMetadata().getAnnotatedMethods(Bean.class.getName());
 		for (MethodMetadata methodMetadata : beanMethods) {
 			configClass.addBeanMethod(new BeanMethod(methodMetadata, configClass));
 		}
 
-		// process superclass, if any
+		// Process superclass, if any
 		if (sourceClass.getMetadata().hasSuperClass()) {
 			String superclass = sourceClass.getMetadata().getSuperClassName();
 			if (!superclass.startsWith("java") && !this.knownSuperclasses.containsKey(superclass)) {
 				this.knownSuperclasses.put(superclass, configClass);
-				// superclass found, return its annotation metadata and recurse
+				// Superclass found, return its annotation metadata and recurse
 				return sourceClass.getSuperClass();
 			}
 		}
 
-		// no superclass, processing is complete
+		// No superclass, processing is complete
 		return null;
 	}
 
@@ -310,9 +315,8 @@ class ConfigurationClassParser {
 	/**
 	 * Process the given <code>@PropertySource</code> annotation metadata.
 	 * @param propertySource metadata for the <code>@PropertySource</code> annotation found
-	 * @throws IOException if loading a property source failed
 	 */
-	private void processPropertySource(AnnotationAttributes propertySource) throws IOException {
+	private void processPropertySource(AnnotationAttributes propertySource) {
 		String name = propertySource.getString("name");
 		String[] locations = propertySource.getStringArray("value");
 		boolean ignoreResourceNotFound = propertySource.getBoolean("ignoreResourceNotFound");
@@ -320,24 +324,61 @@ class ConfigurationClassParser {
 			throw new IllegalArgumentException("At least one @PropertySource(value) location is required");
 		}
 		for (String location : locations) {
-			try {
-				String resolvedLocation = this.environment.resolveRequiredPlaceholders(location);
-				Resource resource = this.resourceLoader.getResource(resolvedLocation);
-				ResourcePropertySource ps = new ResourcePropertySource(resource);
-				this.propertySources.add((StringUtils.hasText(name) ? name : ps.getName()), ps);
-			}
-			catch (IllegalArgumentException ex) {
-				// from resolveRequiredPlaceholders
-				if (!ignoreResourceNotFound) {
-					throw ex;
+			this.propertySources.add((StringUtils.hasText(name) ? name : this.propertySources.size()),
+					new PropertySourceDescriptor(location, ignoreResourceNotFound));
+		}
+	}
+
+	/**
+	 * Register all discovered property sources with the {@link Environment}.
+	 * @throws IOException in case of property loading failure
+	 */
+	private void registerPropertySources() throws IOException {
+		if (!(this.environment instanceof ConfigurableEnvironment)) {
+			return;
+		}
+		MutablePropertySources envPropertySources = ((ConfigurableEnvironment) this.environment).getPropertySources();
+
+		String lastName = null;
+		for (Map.Entry<Object, List<PropertySourceDescriptor>> entry : this.propertySources.entrySet()) {
+			Object key = entry.getKey();
+			String name = (key instanceof String ? (String) key : null);
+
+			List<PropertySourceDescriptor> descriptors = entry.getValue();
+			List<ResourcePropertySource> resources = new ArrayList<ResourcePropertySource>(descriptors.size());
+
+			for (PropertySourceDescriptor descriptor : descriptors) {
+				try {
+					String resolvedLocation = this.environment.resolveRequiredPlaceholders(descriptor.location);
+					Resource resource = this.resourceLoader.getResource(resolvedLocation);
+					ResourcePropertySource ps = new ResourcePropertySource(resource);
+					resources.add(ps);
+					if (name == null) {
+						name = ps.getName();
+					}
 				}
-			}
-			catch (FileNotFoundException ex) {
-				// from ResourcePropertySource constructor
-				if (!ignoreResourceNotFound) {
-					throw ex;
+				catch (IllegalArgumentException ex) {
+					// from resolveRequiredPlaceholders
+					if (!descriptor.ignoreResourceNotFound) {
+						throw ex;
+					}
+				}
+				catch (FileNotFoundException ex) {
+					// from ResourcePropertySource constructor
+					if (!descriptor.ignoreResourceNotFound) {
+						throw ex;
+					}
 				}
 			}
+
+			PropertySource<?> ps = collatePropertySources(name, resources);
+			if (lastName != null) {
+				envPropertySources.addBefore(lastName, ps);
+			}
+			else {
+				envPropertySources.addLast(ps);
+			}
+			lastName = name;
 		}
 	}
 
@@ -488,18 +529,6 @@ class ConfigurationClassParser {
 		return this.configurationClasses.keySet();
 	}
 
-	public int getPropertySourceCount() {
-		return this.propertySources.size();
-	}
-
-	public List<PropertySource<?>> getPropertySources() {
-		List<PropertySource<?>> propertySources = new LinkedList<PropertySource<?>>();
-		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<ResourcePropertySource> propertySources) {
 		if (propertySources.size() == 1) {
 			return propertySources.get(0).withName(name);
@@ -796,6 +825,19 @@ class ConfigurationClassParser {
 	}
 
 
+	private static class PropertySourceDescriptor {
+
+		public PropertySourceDescriptor(String location, boolean ignoreResourceNotFound) {
+			this.location = location;
+			this.ignoreResourceNotFound = ignoreResourceNotFound;
+		}
+
+		public final String location;
+
+		public final boolean ignoreResourceNotFound;
+	}
+
+
 	/**
 	 * {@link Problem} registered upon detection of a circular {@link Import}.
 	 */

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

@@ -21,7 +21,6 @@ import java.util.Arrays;
 import java.util.HashSet;
 import java.util.LinkedHashMap;
 import java.util.LinkedHashSet;
-import java.util.List;
 import java.util.Map;
 import java.util.Set;
 
@@ -57,10 +56,7 @@ import org.springframework.context.annotation.ConfigurationClassEnhancer.Enhance
 import org.springframework.context.annotation.ConfigurationClassParser.ImportRegistry;
 import org.springframework.core.Ordered;
 import org.springframework.core.PriorityOrdered;
-import org.springframework.core.env.ConfigurableEnvironment;
 import org.springframework.core.env.Environment;
-import org.springframework.core.env.MutablePropertySources;
-import org.springframework.core.env.PropertySource;
 import org.springframework.core.io.DefaultResourceLoader;
 import org.springframework.core.io.ResourceLoader;
 import org.springframework.core.type.AnnotationMetadata;
@@ -307,29 +303,10 @@ public class ConfigurationClassPostProcessor implements BeanDefinitionRegistryPo
 				this.resourceLoader, this.componentScanBeanNameGenerator, registry);
 
 		Set<ConfigurationClass> alreadyParsed = new HashSet<ConfigurationClass>(configCandidates.size());
-		int propertySourceCount = 0;
 		do {
 			parser.parse(configCandidates);
 			parser.validate();
 
-			// Handle any @PropertySource annotations
-			if (parser.getPropertySourceCount() > propertySourceCount) {
-				List<PropertySource<?>> parsedPropertySources = parser.getPropertySources();
-				if (!parsedPropertySources.isEmpty()) {
-					if (!(this.environment instanceof ConfigurableEnvironment)) {
-						logger.warn("Ignoring @PropertySource annotations. " +
-								"Reason: Environment must implement ConfigurableEnvironment");
-					}
-					else {
-						MutablePropertySources envPropertySources = ((ConfigurableEnvironment) this.environment).getPropertySources();
-						for (PropertySource<?> propertySource : parsedPropertySources) {
-							envPropertySources.addLast(propertySource);
-						}
-					}
-				}
-				propertySourceCount = parser.getPropertySourceCount();
-			}
-
 			Set<ConfigurationClass> configClasses = new LinkedHashSet<ConfigurationClass>(parser.getConfigurationClasses());
 			configClasses.removeAll(alreadyParsed);
 

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

@@ -76,6 +76,15 @@ public class PropertySourceAnnotationTests {
 		assertThat(ctx.getBean(TestBean.class).getName(), equalTo("p1TestBean"));
 	}
 
+	@Test
+	public void withTestProfileBeans() {
+		AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext();
+		ctx.register(ConfigWithTestProfileBeans.class);
+		ctx.refresh();
+		assertTrue(ctx.containsBean("testBean"));
+		assertTrue(ctx.containsBean("testProfileBean"));
+	}
+
 	/**
 	 * Tests the LIFO behavior of @PropertySource annotaitons.
 	 * The last one registered should 'win'.
@@ -211,6 +220,7 @@ public class PropertySourceAnnotationTests {
 	@Configuration
 	@PropertySource(value="classpath:${unresolvable:org/springframework/context/annotation}/p1.properties")
 	static class ConfigWithUnresolvablePlaceholderAndDefault {
+
 		@Inject Environment env;
 
 		@Bean
@@ -223,6 +233,7 @@ public class PropertySourceAnnotationTests {
 	@Configuration
 	@PropertySource(value="classpath:${path.to.properties}/p1.properties")
 	static class ConfigWithResolvablePlaceholder {
+
 		@Inject Environment env;
 
 		@Bean
@@ -232,10 +243,10 @@ public class PropertySourceAnnotationTests {
 	}
 
 
-
 	@Configuration
 	@PropertySource(name="p1", value="classpath:org/springframework/context/annotation/p1.properties")
 	static class ConfigWithExplicitName {
+
 		@Inject Environment env;
 
 		@Bean
@@ -248,6 +259,7 @@ public class PropertySourceAnnotationTests {
 	@Configuration
 	@PropertySource("classpath:org/springframework/context/annotation/p1.properties")
 	static class ConfigWithImplicitName {
+
 		@Inject Environment env;
 
 		@Bean
@@ -257,6 +269,20 @@ public class PropertySourceAnnotationTests {
 	}
 
 
+	@Configuration
+	@PropertySource(name="p1", value="classpath:org/springframework/context/annotation/p1.properties")
+	@ComponentScan("org.springframework.context.annotation.spr12111")
+	static class ConfigWithTestProfileBeans {
+
+		@Inject Environment env;
+
+		@Bean @Profile("test")
+		public TestBean testBean() {
+			return new TestBean(env.getProperty("testbean.name"));
+		}
+	}
+
+
 	@Configuration
 	@PropertySource("classpath:org/springframework/context/annotation/p2.properties")
 	static class P2Config {
@@ -287,7 +313,7 @@ public class PropertySourceAnnotationTests {
 	@Configuration
 	@PropertySources({
 		@PropertySource("classpath:org/springframework/context/annotation/p1.properties"),
-		@PropertySource("classpath:org/springframework/context/annotation/p2.properties"),
+		@PropertySource("classpath:${base.package}/p2.properties"),
 	})
 	static class ConfigWithPropertySources {
 	}

spring-context/src/test/java/org/springframework/context/annotation/p1.properties

@@ -1,2 +1,4 @@
 testbean.name=p1TestBean
-from.p1=p1Value
+from.p1=p1Value
+base.package=org/springframework/context/annotation
+spring.profiles.active=test

spring-context/src/test/java/org/springframework/context/annotation/spr12111/TestProfileBean.java

@@ -0,0 +1,25 @@
+/*
+ * 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.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.springframework.context.annotation.spr12111;
+
+import org.springframework.context.annotation.Profile;
+import org.springframework.stereotype.Component;
+
+@Component
+@Profile("test")
+public class TestProfileBean {
+}