Skip to content

Commit 32ede50

Browse files
author
Dave Syer
committed
Extract property sources from composite when binding
Often this change will not be important because you are binding to a bean with strongly typed properties. A bean with a Map property, on the other hand, won't oytherwise be able to reason about the permitted keys so it will miss any non-enumerable property sources, including composites whose nested sources are themselves enumerable. Fixed gh-1294
1 parent f5c8a88 commit 32ede50

File tree

3 files changed

+201
-34
lines changed

3 files changed

+201
-34
lines changed

‎spring-boot/src/main/java/org/springframework/boot/bind/PropertySourcesPropertyValues.java

Lines changed: 64 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
package org.springframework.boot.bind;
1818

19+
import java.lang.reflect.Field;
1920
import java.util.Arrays;
2021
import java.util.Collection;
2122
import java.util.Map;
@@ -24,12 +25,14 @@
2425
import org.springframework.beans.MutablePropertyValues;
2526
import org.springframework.beans.PropertyValue;
2627
import org.springframework.beans.PropertyValues;
28+
import org.springframework.core.env.CompositePropertySource;
2729
import org.springframework.core.env.EnumerablePropertySource;
2830
import org.springframework.core.env.PropertySource;
2931
import org.springframework.core.env.PropertySources;
3032
import org.springframework.core.env.PropertySourcesPropertyResolver;
3133
import org.springframework.core.env.StandardEnvironment;
3234
import org.springframework.util.PatternMatchUtils;
35+
import org.springframework.util.ReflectionUtils;
3336
import org.springframework.validation.DataBinder;
3437

3538
/**
@@ -73,48 +76,75 @@ public PropertySourcesPropertyValues(PropertySources propertySources,
7376
.toArray(new String[0]);
7477
String[] exacts = names == null ? new String[0] : names.toArray(new String[0]);
7578
for (PropertySource<?> source : propertySources) {
76-
if (source instanceof EnumerablePropertySource) {
77-
EnumerablePropertySource<?> enumerable = (EnumerablePropertySource<?>) source;
78-
if (enumerable.getPropertyNames().length > 0) {
79-
for (String propertyName : enumerable.getPropertyNames()) {
80-
if (this.NON_ENUMERABLE_ENUMERABLES.contains(source.getName())
81-
&& !PatternMatchUtils.simpleMatch(includes, propertyName)) {
82-
continue;
83-
}
84-
Object value = source.getProperty(propertyName);
85-
try {
86-
value = resolver.getProperty(propertyName);
87-
}
88-
catch (RuntimeException ex) {
89-
// Probably could not resolve placeholders, ignore it here
90-
}
91-
if (!this.propertyValues.containsKey(propertyName)) {
92-
this.propertyValues.put(propertyName, new PropertyValue(
93-
propertyName, value));
94-
}
95-
}
96-
}
97-
}
98-
else {
99-
// We can only do exact matches for non-enumerable property names, but
100-
// that's better than nothing...
101-
for (String propertyName : exacts) {
102-
Object value;
103-
value = resolver.getProperty(propertyName);
104-
if (value != null && !this.propertyValues.containsKey(propertyName)) {
105-
this.propertyValues.put(propertyName, new PropertyValue(
106-
propertyName, value));
79+
processPropertSource(source, resolver, includes, exacts);
80+
}
81+
}
82+
83+
private void processPropertSource(PropertySource<?> source,
84+
PropertySourcesPropertyResolver resolver, String[] includes, String[] exacts) {
85+
if (source instanceof EnumerablePropertySource) {
86+
EnumerablePropertySource<?> enumerable = (EnumerablePropertySource<?>) source;
87+
if (enumerable.getPropertyNames().length > 0) {
88+
for (String propertyName : enumerable.getPropertyNames()) {
89+
if (this.NON_ENUMERABLE_ENUMERABLES.contains(source.getName())
90+
&& !PatternMatchUtils.simpleMatch(includes, propertyName)) {
10791
continue;
10892
}
109-
value = source.getProperty(propertyName.toUpperCase());
110-
if (value != null && !this.propertyValues.containsKey(propertyName)) {
93+
Object value = source.getProperty(propertyName);
94+
try {
95+
value = resolver.getProperty(propertyName);
96+
}
97+
catch (RuntimeException ex) {
98+
// Probably could not resolve placeholders, ignore it here
99+
}
100+
if (!this.propertyValues.containsKey(propertyName)) {
111101
this.propertyValues.put(propertyName, new PropertyValue(
112102
propertyName, value));
113-
continue;
114103
}
115104
}
116105
}
117106
}
107+
else if (source instanceof CompositePropertySource) {
108+
CompositePropertySource composite = (CompositePropertySource) source;
109+
for (PropertySource<?> nested : extractSources(composite)) {
110+
processPropertSource(nested, resolver, includes, exacts);
111+
}
112+
}
113+
else {
114+
// We can only do exact matches for non-enumerable property names, but
115+
// that's better than nothing...
116+
for (String propertyName : exacts) {
117+
Object value;
118+
value = resolver.getProperty(propertyName);
119+
if (value != null && !this.propertyValues.containsKey(propertyName)) {
120+
this.propertyValues.put(propertyName, new PropertyValue(propertyName,
121+
value));
122+
continue;
123+
}
124+
value = source.getProperty(propertyName.toUpperCase());
125+
if (value != null && !this.propertyValues.containsKey(propertyName)) {
126+
this.propertyValues.put(propertyName, new PropertyValue(propertyName,
127+
value));
128+
continue;
129+
}
130+
}
131+
}
132+
}
133+
134+
private Collection<PropertySource<?>> extractSources(CompositePropertySource composite) {
135+
Field field = ReflectionUtils.findField(CompositePropertySource.class,
136+
"propertySources");
137+
field.setAccessible(true);
138+
try {
139+
@SuppressWarnings("unchecked")
140+
Collection<PropertySource<?>> collection = (Collection<PropertySource<?>>) field
141+
.get(composite);
142+
return collection;
143+
}
144+
catch (Exception e) {
145+
throw new IllegalStateException(
146+
"Cannot extract property sources from composite", e);
147+
}
118148
}
119149

120150
@Override

‎spring-boot/src/test/java/org/springframework/boot/bind/PropertiesConfigurationFactoryMapTests.java

Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,125 @@
1+
/*
2+
* Copyright 2012-2013 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.boot.bind;
18+
19+
import java.io.IOException;
20+
import java.util.Collections;
21+
import java.util.HashMap;
22+
import java.util.Map;
23+
24+
import org.junit.Test;
25+
import org.springframework.context.support.StaticMessageSource;
26+
import org.springframework.core.env.CompositePropertySource;
27+
import org.springframework.core.env.MapPropertySource;
28+
import org.springframework.core.env.MutablePropertySources;
29+
import org.springframework.core.io.ByteArrayResource;
30+
import org.springframework.core.io.support.PropertiesLoaderUtils;
31+
import org.springframework.validation.Validator;
32+
33+
import static org.junit.Assert.assertEquals;
34+
35+
/**
36+
* Tests for {@link PropertiesConfigurationFactory}.
37+
*
38+
* @author Dave Syer
39+
*/
40+
public class PropertiesConfigurationFactoryMapTests {
41+
42+
private PropertiesConfigurationFactory<Foo> factory;
43+
44+
private Validator validator;
45+
46+
private boolean ignoreUnknownFields = true;
47+
48+
private String targetName = null;
49+
50+
@Test
51+
public void testValidPropertiesLoadsWithNoErrors() throws Exception {
52+
Foo foo = createFoo("map.name: blah\nmap.bar: blah");
53+
assertEquals("blah", foo.map.get("bar"));
54+
assertEquals("blah", foo.map.get("name"));
55+
}
56+
57+
@Test
58+
public void testBindToNamedTarget() throws Exception {
59+
this.targetName = "foo";
60+
Foo foo = createFoo("hi: hello\nfoo.map.name: foo\nfoo.map.bar: blah");
61+
assertEquals("blah", foo.map.get("bar"));
62+
}
63+
64+
@Test
65+
public void testBindFromPropertySource() throws Exception {
66+
this.targetName = "foo";
67+
setupFactory();
68+
MutablePropertySources sources = new MutablePropertySources();
69+
sources.addFirst(new MapPropertySource("map", Collections.singletonMap(
70+
"foo.map.name", (Object) "blah")));
71+
this.factory.setPropertySources(sources);
72+
this.factory.afterPropertiesSet();
73+
Foo foo = this.factory.getObject();
74+
assertEquals("blah", foo.map.get("name"));
75+
}
76+
77+
@Test
78+
public void testBindFromCompositePropertySource() throws Exception {
79+
this.targetName = "foo";
80+
setupFactory();
81+
MutablePropertySources sources = new MutablePropertySources();
82+
CompositePropertySource composite = new CompositePropertySource("composite");
83+
composite.addPropertySource(new MapPropertySource("map", Collections
84+
.singletonMap("foo.map.name", (Object) "blah")));
85+
sources.addFirst(composite);
86+
this.factory.setPropertySources(sources);
87+
this.factory.afterPropertiesSet();
88+
Foo foo = this.factory.getObject();
89+
assertEquals("blah", foo.map.get("name"));
90+
}
91+
92+
private Foo createFoo(final String values) throws Exception {
93+
setupFactory();
94+
return bindFoo(values);
95+
}
96+
97+
private Foo bindFoo(final String values) throws Exception {
98+
this.factory.setProperties(PropertiesLoaderUtils
99+
.loadProperties(new ByteArrayResource(values.getBytes())));
100+
this.factory.afterPropertiesSet();
101+
return this.factory.getObject();
102+
}
103+
104+
private void setupFactory() throws IOException {
105+
this.factory = new PropertiesConfigurationFactory<Foo>(Foo.class);
106+
this.factory.setValidator(this.validator);
107+
this.factory.setTargetName(this.targetName);
108+
this.factory.setIgnoreUnknownFields(this.ignoreUnknownFields);
109+
this.factory.setMessageSource(new StaticMessageSource());
110+
}
111+
112+
// Foo needs to be public and to have setters for all properties
113+
public static class Foo {
114+
private Map<String, Object> map = new HashMap<String, Object>();
115+
116+
public Map<String, Object> getMap() {
117+
return this.map;
118+
}
119+
120+
public void setMap(Map<String, Object> map) {
121+
this.map = map;
122+
}
123+
}
124+
125+
}

‎spring-boot/src/test/java/org/springframework/boot/bind/PropertySourcesPropertyValuesTests.java

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020

2121
import org.junit.Before;
2222
import org.junit.Test;
23+
import org.springframework.core.env.CompositePropertySource;
2324
import org.springframework.core.env.MapPropertySource;
2425
import org.springframework.core.env.MutablePropertySources;
2526
import org.springframework.core.env.PropertySource;
@@ -66,6 +67,17 @@ public void testNonEnumeratedValue() {
6667
assertEquals("bar", propertyValues.getPropertyValue("foo").getValue());
6768
}
6869

70+
@Test
71+
public void testCompositeValue() {
72+
PropertySource<?> map = this.propertySources.get("map");
73+
CompositePropertySource composite = new CompositePropertySource("composite");
74+
composite.addPropertySource(map);
75+
this.propertySources.replace("map", composite);
76+
PropertySourcesPropertyValues propertyValues = new PropertySourcesPropertyValues(
77+
this.propertySources);
78+
assertEquals("bar", propertyValues.getPropertyValue("foo").getValue());
79+
}
80+
6981
@Test
7082
public void testEnumeratedValue() {
7183
PropertySourcesPropertyValues propertyValues = new PropertySourcesPropertyValues(

0 commit comments

Comments
 (0)