Skip to content

Commit 37d547c

Browse files
cbeamscbeams
committed
Sync with 3.1.x
* 3.1.x: Warn re Environment construction and instance vars Disallow empty @propertysource(value = {}) Fix @propertysource bug with multiple values final preparations for 3.1.1 release added "receive-timeout" attribute to "jms:listener-container" element
2 parents 624ba72 + 7535e24 commit 37d547c

File tree

11 files changed

+218
-36
lines changed

11 files changed

+218
-36
lines changed

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

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
import org.springframework.beans.factory.support.BeanDefinitionRegistry;
3939
import org.springframework.beans.factory.support.BeanNameGenerator;
4040
import org.springframework.core.annotation.AnnotationAttributes;
41+
import org.springframework.core.env.CompositePropertySource;
4142
import org.springframework.core.env.Environment;
4243
import org.springframework.core.env.PropertySource;
4344
import org.springframework.core.io.ResourceLoader;
@@ -179,13 +180,30 @@ protected AnnotationMetadata doProcessConfigurationClass(
179180
if (propertySource != null) {
180181
String name = propertySource.getString("name");
181182
String[] locations = propertySource.getStringArray("value");
183+
int nLocations = locations.length;
184+
if (nLocations == 0) {
185+
throw new IllegalArgumentException("At least one @PropertySource(value) location is required");
186+
}
187+
for (int i = 0; i < nLocations; i++) {
188+
locations[0] = this.environment.resolveRequiredPlaceholders(locations[0]);
189+
}
182190
ClassLoader classLoader = this.resourceLoader.getClassLoader();
183-
for (String location : locations) {
184-
location = this.environment.resolveRequiredPlaceholders(location);
185-
ResourcePropertySource ps = StringUtils.hasText(name) ?
186-
new ResourcePropertySource(name, location, classLoader) :
187-
new ResourcePropertySource(location, classLoader);
188-
this.propertySources.push(ps);
191+
if (!StringUtils.hasText(name)) {
192+
for (String location : locations) {
193+
this.propertySources.push(new ResourcePropertySource(location, classLoader));
194+
}
195+
}
196+
else {
197+
if (nLocations == 1) {
198+
this.propertySources.push(new ResourcePropertySource(name, locations[0], classLoader));
199+
}
200+
else {
201+
CompositePropertySource ps = new CompositePropertySource(name);
202+
for (String location : locations) {
203+
ps.addPropertySource(new ResourcePropertySource(location, classLoader));
204+
}
205+
this.propertySources.push(ps);
206+
}
189207
}
190208
}
191209

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

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,25 @@ public void withResolvablePlaceholder() {
118118
System.clearProperty("path.to.properties");
119119
}
120120

121+
/**
122+
* Corner bug reported in SPR-9127.
123+
*/
124+
@Test
125+
public void withNameAndMultipleResourceLocations() {
126+
AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext();
127+
ctx.register(ConfigWithNameAndMultipleResourceLocations.class);
128+
ctx.refresh();
129+
assertThat(ctx.getEnvironment().containsProperty("from.p1"), is(true));
130+
assertThat(ctx.getEnvironment().containsProperty("from.p2"), is(true));
131+
}
132+
133+
@Test(expected=IllegalArgumentException.class)
134+
public void withEmptyResourceLocations() {
135+
AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext();
136+
ctx.register(ConfigWithEmptyResourceLocations.class);
137+
ctx.refresh();
138+
}
139+
121140

122141
@Configuration
123142
@PropertySource(value="classpath:${unresolvable}/p1.properties")
@@ -178,4 +197,21 @@ public TestBean testBean() {
178197
@PropertySource("classpath:org/springframework/context/annotation/p2.properties")
179198
static class P2Config {
180199
}
200+
201+
202+
@Configuration
203+
@PropertySource(
204+
name = "psName",
205+
value = {
206+
"classpath:org/springframework/context/annotation/p1.properties",
207+
"classpath:org/springframework/context/annotation/p2.properties"
208+
})
209+
static class ConfigWithNameAndMultipleResourceLocations {
210+
}
211+
212+
213+
@Configuration
214+
@PropertySource(value = {})
215+
static class ConfigWithEmptyResourceLocations {
216+
}
181217
}

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1,2 @@
1-
testbean.name=p1TestBean
1+
testbean.name=p1TestBean
2+
from.p1=p1Value

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1,2 @@
1-
testbean.name=p2TestBean
1+
testbean.name=p2TestBean
2+
from.p2=p2Value

‎spring-core/src/main/java/org/springframework/core/env/AbstractEnvironment.java

Lines changed: 56 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2011 the original author or authors.
2+
* Copyright 2002-2012 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -16,22 +16,23 @@
1616

1717
package org.springframework.core.env;
1818

19-
import static java.lang.String.format;
20-
import static org.springframework.util.StringUtils.commaDelimitedListToStringArray;
21-
import static org.springframework.util.StringUtils.trimAllWhitespace;
22-
2319
import java.security.AccessControlException;
20+
2421
import java.util.Collections;
2522
import java.util.LinkedHashSet;
2623
import java.util.Map;
2724
import java.util.Set;
2825

2926
import org.apache.commons.logging.Log;
3027
import org.apache.commons.logging.LogFactory;
28+
3129
import org.springframework.core.convert.support.ConfigurableConversionService;
3230
import org.springframework.util.Assert;
3331
import org.springframework.util.StringUtils;
3432

33+
import static java.lang.String.*;
34+
import static org.springframework.util.StringUtils.*;
35+
3536
/**
3637
* Abstract base class for {@link Environment} implementations. Supports the notion of
3738
* reserved default profile names and enables specifying active and default profiles
@@ -89,19 +90,39 @@ public abstract class AbstractEnvironment implements ConfigurableEnvironment {
8990
protected final Log logger = LogFactory.getLog(getClass());
9091

9192
private Set<String> activeProfiles = new LinkedHashSet<String>();
92-
private Set<String> defaultProfiles = new LinkedHashSet<String>(this.getReservedDefaultProfiles());
9393

94-
private final MutablePropertySources propertySources = new MutablePropertySources(logger);
95-
private final ConfigurablePropertyResolver propertyResolver = new PropertySourcesPropertyResolver(propertySources);
94+
private Set<String> defaultProfiles =
95+
new LinkedHashSet<String>(this.getReservedDefaultProfiles());
9696

97+
private final MutablePropertySources propertySources =
98+
new MutablePropertySources(this.logger);
9799

100+
private final ConfigurablePropertyResolver propertyResolver =
101+
new PropertySourcesPropertyResolver(this.propertySources);
102+
103+
104+
/**
105+
* Create a new {@code Environment} instance, calling back to
106+
* {@link #customizePropertySources(MutablePropertySources)} during construction to
107+
* allow subclasses to contribute or manipulate {@link PropertySource} instances as
108+
* appropriate.
109+
* @see #customizePropertySources(MutablePropertySources)
110+
*/
98111
public AbstractEnvironment() {
99112
String name = this.getClass().getSimpleName();
100-
logger.debug(String.format("Initializing new %s", name));
101-
this.customizePropertySources(propertySources);
102-
logger.debug(String.format("Initialized %s with PropertySources %s", name, propertySources));
113+
if (this.logger.isDebugEnabled()) {
114+
this.logger.debug(format("Initializing new %s", name));
115+
}
116+
117+
this.customizePropertySources(this.propertySources);
118+
119+
if (this.logger.isDebugEnabled()) {
120+
this.logger.debug(format(
121+
"Initialized %s with PropertySources %s", name, this.propertySources));
122+
}
103123
}
104124

125+
105126
/**
106127
* Customize the set of {@link PropertySource} objects to be searched by this
107128
* {@code Environment} during calls to {@link #getProperty(String)} and related
@@ -163,6 +184,17 @@ public AbstractEnvironment() {
163184
* env.getPropertySources().addLast(new PropertySourceX(...));
164185
* </pre>
165186
*
187+
* <h2>A warning about instance variable access</h2>
188+
* Instance variables declared in subclasses and having default initial values should
189+
* <em>not</em> be accessed from within this method. Due to Java object creation
190+
* lifecycle constraints, any initial value will not yet be assigned when this
191+
* callback is invoked by the {@link #AbstractEnvironment()} constructor, which may
192+
* lead to a {@code NullPointerException} or other problems. If you need to access
193+
* default values of instance variables, leave this method as a no-op and perform
194+
* property source manipulation and instance variable access directly within the
195+
* subclass constructor. Note that <em>assigning</em> values to instance variables is
196+
* not problematic; it is only attempting to read default values that must be avoided.
197+
*
166198
* @see MutablePropertySources
167199
* @see PropertySourcesPropertyResolver
168200
* @see org.springframework.context.ApplicationContextInitializer
@@ -217,7 +249,9 @@ public void setActiveProfiles(String... profiles) {
217249
}
218250

219251
public void addActiveProfile(String profile) {
220-
logger.debug(String.format("Activating profile '%s'", profile));
252+
if (this.logger.isDebugEnabled()) {
253+
this.logger.debug(format("Activating profile '%s'", profile));
254+
}
221255
this.validateProfile(profile);
222256
this.activeProfiles.add(profile);
223257
}
@@ -312,8 +346,10 @@ protected String getSystemAttribute(String variableName) {
312346
}
313347
catch (AccessControlException ex) {
314348
if (logger.isInfoEnabled()) {
315-
logger.info(format("Caught AccessControlException when accessing system environment variable " +
316-
"[%s]; its value will be returned [null]. Reason: %s", variableName, ex.getMessage()));
349+
logger.info(format("Caught AccessControlException when " +
350+
"accessing system environment variable [%s]; its " +
351+
"value will be returned [null]. Reason: %s",
352+
variableName, ex.getMessage()));
317353
}
318354
return null;
319355
}
@@ -338,8 +374,10 @@ protected String getSystemAttribute(String propertyName) {
338374
}
339375
catch (AccessControlException ex) {
340376
if (logger.isInfoEnabled()) {
341-
logger.info(format("Caught AccessControlException when accessing system property " +
342-
"[%s]; its value will be returned [null]. Reason: %s", propertyName, ex.getMessage()));
377+
logger.info(format("Caught AccessControlException when " +
378+
"accessing system property [%s]; its value will be " +
379+
"returned [null]. Reason: %s",
380+
propertyName, ex.getMessage()));
343381
}
344382
return null;
345383
}
@@ -428,7 +466,8 @@ public void setValueSeparator(String valueSeparator) {
428466
@Override
429467
public String toString() {
430468
return format("%s {activeProfiles=%s, defaultProfiles=%s, propertySources=%s}",
431-
getClass().getSimpleName(), this.activeProfiles, this.defaultProfiles, this.propertySources);
469+
getClass().getSimpleName(), this.activeProfiles, this.defaultProfiles,
470+
this.propertySources);
432471
}
433472

434473
}

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

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
/*
2+
* Copyright 2002-2012 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package org.springframework.core.env;
18+
19+
import java.util.LinkedHashSet;
20+
import java.util.Set;
21+
22+
/**
23+
* Composite {@link PropertySource} implementation that iterates over a set of
24+
* {@link PropertySource} instances. Necessary in cases where multiple property sources
25+
* share the same name, e.g. when multiple values are supplied to {@code @PropertySource}.
26+
*
27+
* @author Chris Beams
28+
* @since 3.1.1
29+
*/
30+
public class CompositePropertySource extends PropertySource<Object> {
31+
32+
private Set<PropertySource<?>> propertySources = new LinkedHashSet<PropertySource<?>>();
33+
34+
35+
/**
36+
* Create a new {@code CompositePropertySource}.
37+
*
38+
* @param name the name of the property source
39+
*/
40+
public CompositePropertySource(String name) {
41+
super(name);
42+
}
43+
44+
45+
@Override
46+
public Object getProperty(String name) {
47+
for (PropertySource<?> propertySource : this.propertySources) {
48+
Object candidate = propertySource.getProperty(name);
49+
if (candidate != null) {
50+
return candidate;
51+
}
52+
}
53+
return null;
54+
}
55+
56+
public void addPropertySource(PropertySource<?> propertySource) {
57+
this.propertySources.add(propertySource);
58+
}
59+
60+
@Override
61+
public String toString() {
62+
return String.format("%s [name='%s', propertySources=%s]",
63+
this.getClass().getSimpleName(), this.name, this.propertySources);
64+
}
65+
}

‎spring-jms/src/main/java/org/springframework/jms/config/JcaListenerContainerParser.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2010 the original author or authors.
2+
* Copyright 2002-2012 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -108,6 +108,7 @@ protected BeanDefinition parseContainer(Element listenerEle, Element containerEl
108108
return containerDef;
109109
}
110110

111+
@Override
111112
protected boolean indicatesPubSub(BeanDefinition containerDef) {
112113
BeanDefinition configDef =
113114
(BeanDefinition) containerDef.getPropertyValues().getPropertyValue("activationSpecConfig").getValue();

‎spring-jms/src/main/java/org/springframework/jms/config/JmsListenerContainerParser.java

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2010 the original author or authors.
2+
* Copyright 2002-2012 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -47,6 +47,8 @@ class JmsListenerContainerParser extends AbstractListenerContainerParser {
4747

4848
private static final String CACHE_ATTRIBUTE = "cache";
4949

50+
private static final String RECEIVE_TIMEOUT_ATTRIBUTE = "receive-timeout";
51+
5052

5153
protected BeanDefinition parseContainer(Element listenerEle, Element containerEle, ParserContext parserContext) {
5254
RootBeanDefinition containerDef = new RootBeanDefinition();
@@ -156,6 +158,13 @@ else if ("simple102".equals(containerType)) {
156158
}
157159
}
158160

161+
String receiveTimeout = containerEle.getAttribute(RECEIVE_TIMEOUT_ATTRIBUTE);
162+
if (StringUtils.hasText(receiveTimeout)) {
163+
if (containerType.startsWith("default")) {
164+
containerDef.getPropertyValues().add("receiveTimeout", new Integer(receiveTimeout));
165+
}
166+
}
167+
159168
String phase = containerEle.getAttribute(PHASE_ATTRIBUTE);
160169
if (StringUtils.hasText(phase)) {
161170
containerDef.getPropertyValues().add("phase", phase);
@@ -164,10 +173,12 @@ else if ("simple102".equals(containerType)) {
164173
return containerDef;
165174
}
166175

176+
@Override
167177
protected boolean indicatesPubSub(BeanDefinition containerDef) {
168178
return indicatesPubSubConfig(containerDef);
169179
}
170180

181+
@Override
171182
protected boolean indicatesJms102(BeanDefinition containerDef) {
172183
return containerDef.getBeanClassName().endsWith("102");
173184
}

‎spring-jms/src/main/resources/org/springframework/jms/config/spring-jms-3.1.xsd

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -233,6 +233,14 @@
233233
]]></xsd:documentation>
234234
</xsd:annotation>
235235
</xsd:attribute>
236+
<xsd:attribute name="receive-timeout" type="xsd:int">
237+
<xsd:annotation>
238+
<xsd:documentation><![CDATA[
239+
The timeout to use for receive calls (in milliseconds).
240+
The default is 1000 ms (1 sec); -1 indicates no timeout at all.
241+
]]></xsd:documentation>
242+
</xsd:annotation>
243+
</xsd:attribute>
236244
<xsd:attribute name="phase" type="xsd:string">
237245
<xsd:annotation>
238246
<xsd:documentation><![CDATA[

0 commit comments

Comments
 (0)