From 41ade68b50b39485b3cf02b9c6fb8eb74962146b Mon Sep 17 00:00:00 2001 From: Chris Beams Date: Thu, 16 Feb 2012 16:43:28 +0100 Subject: [PATCH] Fix @PropertySource bug with multiple values Prior to this commit, specifying a named @PropertySource with multiple values would not work as expected. e.g.: @PropertySource( name = "ps", value = { "classpath:a.properties", "classpath:b.properties" }) In this scenario, the implementation would register a.properties with the name "ps", and subsequently register b.properties with the name "ps", overwriting the entry for a.properties. To fix this behavior, a CompositePropertySource type has been introduced which accepts a single name and a set of PropertySource objects to iterate over. ConfigurationClassParser's @PropertySource parsing routine has been updated to use this composite approach when necessary, i.e. when both an explicit name and more than one location have been specified. Note that if no explicit name is specified, the generated property source names are enough to distinguish the instances and avoid overwriting each other; this is why the composite wrapper is not used in these cases. Issue: SPR-9127 --- .../annotation/ConfigurationClassParser.java | 27 ++++++-- .../PropertySourceAnnotationTests.java | 34 ++++++++++ .../context/annotation/p1.properties | 3 +- .../context/annotation/p2.properties | 3 +- .../core/env/CompositePropertySource.java | 65 +++++++++++++++++++ 5 files changed, 124 insertions(+), 8 deletions(-) create mode 100644 org.springframework.core/src/main/java/org/springframework/core/env/CompositePropertySource.java diff --git a/org.springframework.context/src/main/java/org/springframework/context/annotation/ConfigurationClassParser.java b/org.springframework.context/src/main/java/org/springframework/context/annotation/ConfigurationClassParser.java index 1ad4838ed88..3b5d703e520 100644 --- a/org.springframework.context/src/main/java/org/springframework/context/annotation/ConfigurationClassParser.java +++ b/org.springframework.context/src/main/java/org/springframework/context/annotation/ConfigurationClassParser.java @@ -38,6 +38,7 @@ import org.springframework.beans.factory.support.BeanDefinitionReader; import org.springframework.beans.factory.support.BeanDefinitionRegistry; import org.springframework.beans.factory.support.BeanNameGenerator; import org.springframework.core.annotation.AnnotationAttributes; +import org.springframework.core.env.CompositePropertySource; import org.springframework.core.env.Environment; import org.springframework.core.env.PropertySource; import org.springframework.core.io.ResourceLoader; @@ -179,13 +180,27 @@ class ConfigurationClassParser { if (propertySource != null) { String name = propertySource.getString("name"); String[] locations = propertySource.getStringArray("value"); + int nLocations = locations.length; + for (int i = 0; i < nLocations; i++) { + locations[0] = this.environment.resolveRequiredPlaceholders(locations[0]); + } ClassLoader classLoader = this.resourceLoader.getClassLoader(); - for (String location : locations) { - location = this.environment.resolveRequiredPlaceholders(location); - ResourcePropertySource ps = StringUtils.hasText(name) ? - new ResourcePropertySource(name, location, classLoader) : - new ResourcePropertySource(location, classLoader); - this.propertySources.push(ps); + if (!StringUtils.hasText(name)) { + for (String location : locations) { + this.propertySources.push(new ResourcePropertySource(location, classLoader)); + } + } + else { + if (nLocations == 1) { + this.propertySources.push(new ResourcePropertySource(name, locations[0], classLoader)); + } + else { + CompositePropertySource ps = new CompositePropertySource(name); + for (String location : locations) { + ps.addPropertySource(new ResourcePropertySource(location, classLoader)); + } + this.propertySources.push(ps); + } } } diff --git a/org.springframework.context/src/test/java/org/springframework/context/annotation/PropertySourceAnnotationTests.java b/org.springframework.context/src/test/java/org/springframework/context/annotation/PropertySourceAnnotationTests.java index 133b9639670..5b487591b7c 100644 --- a/org.springframework.context/src/test/java/org/springframework/context/annotation/PropertySourceAnnotationTests.java +++ b/org.springframework.context/src/test/java/org/springframework/context/annotation/PropertySourceAnnotationTests.java @@ -118,6 +118,29 @@ public class PropertySourceAnnotationTests { System.clearProperty("path.to.properties"); } + /** + * Corner bug reported in SPR-9127. + */ + @Test + public void withNameAndMultipleResourceLocations() { + AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(); + ctx.register(ConfigWithNameAndMultipleResourceLocations.class); + ctx.refresh(); + assertThat(ctx.getEnvironment().containsProperty("from.p1"), is(true)); + assertThat(ctx.getEnvironment().containsProperty("from.p2"), is(true)); + } + + + @Configuration + @PropertySource( + name = "psName", + value = { + "classpath:org/springframework/context/annotation/p1.properties", + "classpath:org/springframework/context/annotation/p2.properties" + }) + static class ConfigWithNameAndMultipleResourceValues { + } + @Configuration @PropertySource(value="classpath:${unresolvable}/p1.properties") @@ -178,4 +201,15 @@ public class PropertySourceAnnotationTests { @PropertySource("classpath:org/springframework/context/annotation/p2.properties") static class P2Config { } + + + @Configuration + @PropertySource( + name = "psName", + value = { + "classpath:org/springframework/context/annotation/p1.properties", + "classpath:org/springframework/context/annotation/p2.properties" + }) + static class ConfigWithNameAndMultipleResourceLocations { + } } diff --git a/org.springframework.context/src/test/java/org/springframework/context/annotation/p1.properties b/org.springframework.context/src/test/java/org/springframework/context/annotation/p1.properties index 55a32e793f9..39d7cef12b7 100644 --- a/org.springframework.context/src/test/java/org/springframework/context/annotation/p1.properties +++ b/org.springframework.context/src/test/java/org/springframework/context/annotation/p1.properties @@ -1 +1,2 @@ -testbean.name=p1TestBean \ No newline at end of file +testbean.name=p1TestBean +from.p1=p1Value \ No newline at end of file diff --git a/org.springframework.context/src/test/java/org/springframework/context/annotation/p2.properties b/org.springframework.context/src/test/java/org/springframework/context/annotation/p2.properties index 141de759548..34049d52a5e 100644 --- a/org.springframework.context/src/test/java/org/springframework/context/annotation/p2.properties +++ b/org.springframework.context/src/test/java/org/springframework/context/annotation/p2.properties @@ -1 +1,2 @@ -testbean.name=p2TestBean \ No newline at end of file +testbean.name=p2TestBean +from.p2=p2Value \ No newline at end of file diff --git a/org.springframework.core/src/main/java/org/springframework/core/env/CompositePropertySource.java b/org.springframework.core/src/main/java/org/springframework/core/env/CompositePropertySource.java new file mode 100644 index 00000000000..229c9cd2289 --- /dev/null +++ b/org.springframework.core/src/main/java/org/springframework/core/env/CompositePropertySource.java @@ -0,0 +1,65 @@ +/* + * Copyright 2002-2012 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.LinkedHashSet; +import java.util.Set; + +/** + * Composite {@link PropertySource} implementation that iterates over a set of + * {@link PropertySource} instances. Necessary in cases where multiple property sources + * share the same name, e.g. when multiple values are supplied to {@code @PropertySource}. + * + * @author Chris Beams + * @since 3.1.1 + */ +public class CompositePropertySource extends PropertySource { + + private Set> propertySources = new LinkedHashSet>(); + + + /** + * Create a new {@code CompositePropertySource}. + * + * @param name the name of the property source + */ + public CompositePropertySource(String name) { + super(name); + } + + + @Override + public Object getProperty(String name) { + for (PropertySource propertySource : this.propertySources) { + Object candidate = propertySource.getProperty(name); + if (candidate != null) { + return candidate; + } + } + return null; + } + + public void addPropertySource(PropertySource propertySource) { + this.propertySources.add(propertySource); + } + + @Override + public String toString() { + return String.format("%s [name='%s', propertySources=%s]", + this.getClass().getSimpleName(), this.name, this.propertySources); + } +}