diff --git a/spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClassParser.java b/spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClassParser.java index 3904bd82d7..6a180e6858 100644 --- a/spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClassParser.java +++ b/spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClassParser.java @@ -32,6 +32,8 @@ import java.util.Map; import java.util.Set; 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; @@ -68,8 +70,7 @@ import org.springframework.core.type.StandardAnnotationMetadata; import org.springframework.core.type.classreading.MetadataReader; import org.springframework.core.type.classreading.MetadataReaderFactory; import org.springframework.core.type.filter.AssignableTypeFilter; -import org.springframework.util.LinkedMultiValueMap; -import org.springframework.util.MultiValueMap; +import org.springframework.util.Assert; import org.springframework.util.StringUtils; /** @@ -103,6 +104,8 @@ class ConfigurationClassParser { }; + private final Log logger = LogFactory.getLog(getClass()); + private final MetadataReaderFactory metadataReaderFactory; private final ProblemReporter problemReporter; @@ -122,8 +125,7 @@ class ConfigurationClassParser { private final Map knownSuperclasses = new HashMap(); - private final MultiValueMap propertySources = - new LinkedMultiValueMap(); + private final Set propertySourceNames = new LinkedHashSet(); private final ImportStack importStack = new ImportStack(); @@ -242,12 +244,15 @@ class ConfigurationClassParser { // Process any @PropertySource annotations for (AnnotationAttributes propertySource : AnnotationConfigUtils.attributesForRepeatable( sourceClass.getMetadata(), PropertySources.class, org.springframework.context.annotation.PropertySource.class)) { - processPropertySource(propertySource); + if (!(this.environment instanceof ConfigurableEnvironment)) { + logger.warn("Ignoring @PropertySource annotation on " + + sourceClass.getMetadata().getClassName() + + "Reason: Environment must implement ConfigurableEnvironment"); + } else { + processPropertySource(propertySource); + } } - // Register PropertySources with Environment - registerPropertySources(); - // Process any @ComponentScan annotations AnnotationAttributes componentScan = AnnotationConfigUtils.attributesFor(sourceClass.getMetadata(), ComponentScan.class); if (componentScan != null) { @@ -315,71 +320,69 @@ class ConfigurationClassParser { /** * Process the given @PropertySource annotation metadata. * @param propertySource metadata for the @PropertySource annotation found + * @throws IOException if loading a property source failed */ - private void processPropertySource(AnnotationAttributes propertySource) { + private void processPropertySource(AnnotationAttributes propertySource) throws IOException { String name = propertySource.getString("name"); String[] locations = propertySource.getStringArray("value"); boolean ignoreResourceNotFound = propertySource.getBoolean("ignoreResourceNotFound"); - if (locations.length == 0) { - throw new IllegalArgumentException("At least one @PropertySource(value) location is required"); - } + Assert.isTrue(locations.length > 0, "At least one @PropertySource(value) location is required"); for (String location : locations) { - this.propertySources.add((StringUtils.hasText(name) ? name : this.propertySources.size()), - new PropertySourceDescriptor(location, ignoreResourceNotFound)); + try { + processPropertySourceLocation(name, location); + } + catch (IllegalArgumentException ex) { + // from resolveRequiredPlaceholders + if (!ignoreResourceNotFound) { + throw ex; + } + } + catch (FileNotFoundException ex) { + // from ResourcePropertySource constructor + if (!ignoreResourceNotFound) { + throw ex; + } + } } } - /** - * 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(); + private void processPropertySourceLocation(String name, String location) throws IOException, FileNotFoundException { + 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); + } - String lastName = null; - for (Map.Entry> entry : this.propertySources.entrySet()) { - Object key = entry.getKey(); - String name = (key instanceof String ? (String) key : null); - - List descriptors = entry.getValue(); - List resources = new ArrayList(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 (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); + 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( + propertySource.resourceNamedPropertySource()); } else { - envPropertySources.addLast(ps); + if (existing instanceof AnnotationPropertySource) { + existing = ((AnnotationPropertySource) existing).resourceNamedPropertySource(); + } + CompositePropertySource composite = new CompositePropertySource(name); + composite.addPropertySource(propertySource.resourceNamedPropertySource()); + composite.addPropertySource(existing); + propertySources.replace(name, composite); } - lastName = name; } + else { + if(this.propertySourceNames.isEmpty()) { + propertySources.addLast(propertySource); + } else { + String firstProcessed = this.propertySourceNames.iterator().next(); + propertySources.addBefore(firstProcessed, propertySource); + } + } + this.propertySourceNames.add(name); } /** @@ -529,17 +532,6 @@ class ConfigurationClassParser { return this.configurationClasses.keySet(); } - private PropertySource collatePropertySources(String name, List propertySources) { - if (propertySources.size() == 1) { - return propertySources.get(0).withName(name); - } - CompositePropertySource result = new CompositePropertySource(name); - for (int i = propertySources.size() - 1; i >= 0; i--) { - result.addPropertySource(propertySources.get(i)); - } - return result; - } - ImportRegistry getImportRegistry() { return this.importStack; @@ -825,19 +817,6 @@ 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}. */ @@ -852,4 +831,34 @@ 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 source) { + super(name, source); + this.resourceName = null; + } + + public AnnotationPropertySource resourceNamedPropertySource() { + if (this.resourceName == null) { + return this; + } + return new AnnotationPropertySource(this.resourceName, getSource()); + } + + } + } diff --git a/spring-context/src/test/java/org/springframework/context/annotation/PropertySourceAnnotationTests.java b/spring-context/src/test/java/org/springframework/context/annotation/PropertySourceAnnotationTests.java index 7f8ac023af..6f901970d9 100644 --- a/spring-context/src/test/java/org/springframework/context/annotation/PropertySourceAnnotationTests.java +++ b/spring-context/src/test/java/org/springframework/context/annotation/PropertySourceAnnotationTests.java @@ -210,6 +210,14 @@ public class PropertySourceAnnotationTests { assertThat(ctx.getEnvironment().containsProperty("from.p2"), is(true)); } + @Test + public void withSameSourceImportedInDifferentOrder() throws Exception { + AnnotationConfigApplicationContext ctx =new AnnotationConfigApplicationContext(ConfigWithSameSourceImportedInDifferentOrder.class); + assertThat(ctx.getEnvironment().containsProperty("from.p1"), is(true)); + assertThat(ctx.getEnvironment().containsProperty("from.p2"), is(true)); + assertThat(ctx.getEnvironment().getProperty("testbean.name"), equalTo("p2TestBean")); + } + @Configuration @PropertySource(value="classpath:${unresolvable}/p1.properties") @@ -354,4 +362,22 @@ public class PropertySourceAnnotationTests { static class ConfigWithEmptyResourceLocations { } + @Import({ ConfigImportedWithSameSourceImportedInDifferentOrder.class }) + @PropertySources({ + @PropertySource("classpath:org/springframework/context/annotation/p1.properties"), + @PropertySource("classpath:org/springframework/context/annotation/p2.properties") + }) + @Configuration + public static class ConfigWithSameSourceImportedInDifferentOrder { + + } + + @Configuration + @PropertySources({ + @PropertySource("classpath:org/springframework/context/annotation/p2.properties"), + @PropertySource("classpath:org/springframework/context/annotation/p1.properties") + }) + public static class ConfigImportedWithSameSourceImportedInDifferentOrder { + } + } diff --git a/spring-core/src/main/java/org/springframework/core/env/CompositePropertySource.java b/spring-core/src/main/java/org/springframework/core/env/CompositePropertySource.java index f05d0ce4cd..5177b8dc45 100644 --- a/spring-core/src/main/java/org/springframework/core/env/CompositePropertySource.java +++ b/spring-core/src/main/java/org/springframework/core/env/CompositePropertySource.java @@ -16,7 +16,9 @@ package org.springframework.core.env; +import java.util.ArrayList; import java.util.LinkedHashSet; +import java.util.List; import java.util.Set; /** @@ -25,6 +27,7 @@ import java.util.Set; * share the same name, e.g. when multiple values are supplied to {@code @PropertySource}. * * @author Chris Beams + * @author Phillip Webb * @since 3.1.1 */ public class CompositePropertySource extends PropertySource { @@ -56,6 +59,13 @@ public class CompositePropertySource extends PropertySource { this.propertySources.add(propertySource); } + public void addFirstPropertySource(PropertySource propertySource) { + List> exisiting = new ArrayList>(this.propertySources); + this.propertySources.clear(); + this.propertySources.add(propertySource); + this.propertySources.addAll(exisiting); + } + @Override public String toString() { return String.format("%s [name='%s', propertySources=%s]", diff --git a/spring-core/src/main/java/org/springframework/core/io/support/ResourcePropertySource.java b/spring-core/src/main/java/org/springframework/core/io/support/ResourcePropertySource.java index cec785b3d6..10a293961c 100644 --- a/spring-core/src/main/java/org/springframework/core/io/support/ResourcePropertySource.java +++ b/spring-core/src/main/java/org/springframework/core/io/support/ResourcePropertySource.java @@ -115,7 +115,7 @@ public class ResourcePropertySource extends PropertiesPropertySource { this(new DefaultResourceLoader().getResource(location)); } - private ResourcePropertySource(String name, Map source) { + protected ResourcePropertySource(String name, Map source) { super(name, source); } @@ -137,7 +137,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() */ - private static String getNameForResource(Resource resource) { + protected static String getNameForResource(Resource resource) { String name = resource.getDescription(); if (!StringUtils.hasText(name)) { name = resource.getClass().getSimpleName() + "@" + System.identityHashCode(resource); diff --git a/spring-core/src/test/java/org/springframework/core/env/CompositePropertySourceTests.java b/spring-core/src/test/java/org/springframework/core/env/CompositePropertySourceTests.java new file mode 100644 index 0000000000..3ec7202394 --- /dev/null +++ b/spring-core/src/test/java/org/springframework/core/env/CompositePropertySourceTests.java @@ -0,0 +1,46 @@ +/* + * 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.core.env; + +import java.util.Collections; + +import org.junit.Test; + +import static org.hamcrest.Matchers.*; +import static org.junit.Assert.*; + +/** + * Tests for {@link CompositePropertySource}. + * @author Phillip Webb + */ +public class CompositePropertySourceTests { + + @Test + public void addFirst() { + PropertySource p1 = new MapPropertySource("p1", Collections.emptyMap()); + PropertySource p2 = new MapPropertySource("p2", Collections.emptyMap()); + PropertySource p3 = new MapPropertySource("p3", Collections.emptyMap()); + CompositePropertySource composite = new CompositePropertySource("c"); + composite.addPropertySource(p2); + composite.addPropertySource(p3); + composite.addPropertySource(p1); + composite.addFirstPropertySource(p1); + assertThat(composite.toString(), containsString("MapPropertySource [name='p1'], " + + "MapPropertySource [name='p2'], MapPropertySource [name='p3']")); + } + +}