From 32ede50d190c2c0bd5fe8dff87d1425d18215f5a Mon Sep 17 00:00:00 2001 From: Dave Syer Date: Fri, 25 Jul 2014 08:31:45 -0700 Subject: [PATCH] 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 --- .../bind/PropertySourcesPropertyValues.java | 102 +++++++++----- ...ropertiesConfigurationFactoryMapTests.java | 125 ++++++++++++++++++ .../PropertySourcesPropertyValuesTests.java | 12 ++ 3 files changed, 203 insertions(+), 36 deletions(-) create mode 100644 spring-boot/src/test/java/org/springframework/boot/bind/PropertiesConfigurationFactoryMapTests.java diff --git a/spring-boot/src/main/java/org/springframework/boot/bind/PropertySourcesPropertyValues.java b/spring-boot/src/main/java/org/springframework/boot/bind/PropertySourcesPropertyValues.java index bc7013e2b79..0ee422f3cc7 100644 --- a/spring-boot/src/main/java/org/springframework/boot/bind/PropertySourcesPropertyValues.java +++ b/spring-boot/src/main/java/org/springframework/boot/bind/PropertySourcesPropertyValues.java @@ -16,6 +16,7 @@ package org.springframework.boot.bind; +import java.lang.reflect.Field; import java.util.Arrays; import java.util.Collection; import java.util.Map; @@ -24,12 +25,14 @@ import java.util.concurrent.ConcurrentHashMap; import org.springframework.beans.MutablePropertyValues; import org.springframework.beans.PropertyValue; import org.springframework.beans.PropertyValues; +import org.springframework.core.env.CompositePropertySource; import org.springframework.core.env.EnumerablePropertySource; import org.springframework.core.env.PropertySource; import org.springframework.core.env.PropertySources; import org.springframework.core.env.PropertySourcesPropertyResolver; import org.springframework.core.env.StandardEnvironment; import org.springframework.util.PatternMatchUtils; +import org.springframework.util.ReflectionUtils; import org.springframework.validation.DataBinder; /** @@ -73,50 +76,77 @@ public class PropertySourcesPropertyValues implements PropertyValues { .toArray(new String[0]); String[] exacts = names == null ? new String[0] : names.toArray(new String[0]); for (PropertySource source : propertySources) { - if (source instanceof EnumerablePropertySource) { - EnumerablePropertySource enumerable = (EnumerablePropertySource) source; - if (enumerable.getPropertyNames().length > 0) { - for (String propertyName : enumerable.getPropertyNames()) { - if (this.NON_ENUMERABLE_ENUMERABLES.contains(source.getName()) - && !PatternMatchUtils.simpleMatch(includes, propertyName)) { - continue; - } - Object value = source.getProperty(propertyName); - try { - value = resolver.getProperty(propertyName); - } - catch (RuntimeException ex) { - // Probably could not resolve placeholders, ignore it here - } - if (!this.propertyValues.containsKey(propertyName)) { - this.propertyValues.put(propertyName, new PropertyValue( - propertyName, value)); - } + processPropertSource(source, resolver, includes, exacts); + } + } + + private void processPropertSource(PropertySource source, + PropertySourcesPropertyResolver resolver, String[] includes, String[] exacts) { + if (source instanceof EnumerablePropertySource) { + EnumerablePropertySource enumerable = (EnumerablePropertySource) source; + if (enumerable.getPropertyNames().length > 0) { + for (String propertyName : enumerable.getPropertyNames()) { + if (this.NON_ENUMERABLE_ENUMERABLES.contains(source.getName()) + && !PatternMatchUtils.simpleMatch(includes, propertyName)) { + continue; + } + Object value = source.getProperty(propertyName); + try { + value = resolver.getProperty(propertyName); + } + catch (RuntimeException ex) { + // Probably could not resolve placeholders, ignore it here + } + if (!this.propertyValues.containsKey(propertyName)) { + this.propertyValues.put(propertyName, new PropertyValue( + propertyName, value)); } } } - else { - // We can only do exact matches for non-enumerable property names, but - // that's better than nothing... - for (String propertyName : exacts) { - Object value; - value = resolver.getProperty(propertyName); - if (value != null && !this.propertyValues.containsKey(propertyName)) { - this.propertyValues.put(propertyName, new PropertyValue( - propertyName, value)); - continue; - } - value = source.getProperty(propertyName.toUpperCase()); - if (value != null && !this.propertyValues.containsKey(propertyName)) { - this.propertyValues.put(propertyName, new PropertyValue( - propertyName, value)); - continue; - } + } + else if (source instanceof CompositePropertySource) { + CompositePropertySource composite = (CompositePropertySource) source; + for (PropertySource nested : extractSources(composite)) { + processPropertSource(nested, resolver, includes, exacts); + } + } + else { + // We can only do exact matches for non-enumerable property names, but + // that's better than nothing... + for (String propertyName : exacts) { + Object value; + value = resolver.getProperty(propertyName); + if (value != null && !this.propertyValues.containsKey(propertyName)) { + this.propertyValues.put(propertyName, new PropertyValue(propertyName, + value)); + continue; + } + value = source.getProperty(propertyName.toUpperCase()); + if (value != null && !this.propertyValues.containsKey(propertyName)) { + this.propertyValues.put(propertyName, new PropertyValue(propertyName, + value)); + continue; } } } } + private Collection> extractSources(CompositePropertySource composite) { + Field field = ReflectionUtils.findField(CompositePropertySource.class, + "propertySources"); + field.setAccessible(true); + try { + @SuppressWarnings("unchecked") + Collection> collection = (Collection>) field + .get(composite); + return collection; + } + catch (Exception e) { + throw new IllegalStateException( + "Cannot extract property sources from composite", e); + } + } + @Override public PropertyValue[] getPropertyValues() { Collection values = this.propertyValues.values(); diff --git a/spring-boot/src/test/java/org/springframework/boot/bind/PropertiesConfigurationFactoryMapTests.java b/spring-boot/src/test/java/org/springframework/boot/bind/PropertiesConfigurationFactoryMapTests.java new file mode 100644 index 00000000000..200092cce3e --- /dev/null +++ b/spring-boot/src/test/java/org/springframework/boot/bind/PropertiesConfigurationFactoryMapTests.java @@ -0,0 +1,125 @@ +/* + * Copyright 2012-2013 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.boot.bind; + +import java.io.IOException; +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; + +import org.junit.Test; +import org.springframework.context.support.StaticMessageSource; +import org.springframework.core.env.CompositePropertySource; +import org.springframework.core.env.MapPropertySource; +import org.springframework.core.env.MutablePropertySources; +import org.springframework.core.io.ByteArrayResource; +import org.springframework.core.io.support.PropertiesLoaderUtils; +import org.springframework.validation.Validator; + +import static org.junit.Assert.assertEquals; + +/** + * Tests for {@link PropertiesConfigurationFactory}. + * + * @author Dave Syer + */ +public class PropertiesConfigurationFactoryMapTests { + + private PropertiesConfigurationFactory factory; + + private Validator validator; + + private boolean ignoreUnknownFields = true; + + private String targetName = null; + + @Test + public void testValidPropertiesLoadsWithNoErrors() throws Exception { + Foo foo = createFoo("map.name: blah\nmap.bar: blah"); + assertEquals("blah", foo.map.get("bar")); + assertEquals("blah", foo.map.get("name")); + } + + @Test + public void testBindToNamedTarget() throws Exception { + this.targetName = "foo"; + Foo foo = createFoo("hi: hello\nfoo.map.name: foo\nfoo.map.bar: blah"); + assertEquals("blah", foo.map.get("bar")); + } + + @Test + public void testBindFromPropertySource() throws Exception { + this.targetName = "foo"; + setupFactory(); + MutablePropertySources sources = new MutablePropertySources(); + sources.addFirst(new MapPropertySource("map", Collections.singletonMap( + "foo.map.name", (Object) "blah"))); + this.factory.setPropertySources(sources); + this.factory.afterPropertiesSet(); + Foo foo = this.factory.getObject(); + assertEquals("blah", foo.map.get("name")); + } + + @Test + public void testBindFromCompositePropertySource() throws Exception { + this.targetName = "foo"; + setupFactory(); + MutablePropertySources sources = new MutablePropertySources(); + CompositePropertySource composite = new CompositePropertySource("composite"); + composite.addPropertySource(new MapPropertySource("map", Collections + .singletonMap("foo.map.name", (Object) "blah"))); + sources.addFirst(composite); + this.factory.setPropertySources(sources); + this.factory.afterPropertiesSet(); + Foo foo = this.factory.getObject(); + assertEquals("blah", foo.map.get("name")); + } + + private Foo createFoo(final String values) throws Exception { + setupFactory(); + return bindFoo(values); + } + + private Foo bindFoo(final String values) throws Exception { + this.factory.setProperties(PropertiesLoaderUtils + .loadProperties(new ByteArrayResource(values.getBytes()))); + this.factory.afterPropertiesSet(); + return this.factory.getObject(); + } + + private void setupFactory() throws IOException { + this.factory = new PropertiesConfigurationFactory(Foo.class); + this.factory.setValidator(this.validator); + this.factory.setTargetName(this.targetName); + this.factory.setIgnoreUnknownFields(this.ignoreUnknownFields); + this.factory.setMessageSource(new StaticMessageSource()); + } + + // Foo needs to be public and to have setters for all properties + public static class Foo { + private Map map = new HashMap(); + + public Map getMap() { + return this.map; + } + + public void setMap(Map map) { + this.map = map; + } + } + +} diff --git a/spring-boot/src/test/java/org/springframework/boot/bind/PropertySourcesPropertyValuesTests.java b/spring-boot/src/test/java/org/springframework/boot/bind/PropertySourcesPropertyValuesTests.java index 50be81c3dbf..096a4540dd3 100644 --- a/spring-boot/src/test/java/org/springframework/boot/bind/PropertySourcesPropertyValuesTests.java +++ b/spring-boot/src/test/java/org/springframework/boot/bind/PropertySourcesPropertyValuesTests.java @@ -20,6 +20,7 @@ import java.util.Collections; import org.junit.Before; import org.junit.Test; +import org.springframework.core.env.CompositePropertySource; import org.springframework.core.env.MapPropertySource; import org.springframework.core.env.MutablePropertySources; import org.springframework.core.env.PropertySource; @@ -66,6 +67,17 @@ public class PropertySourcesPropertyValuesTests { assertEquals("bar", propertyValues.getPropertyValue("foo").getValue()); } + @Test + public void testCompositeValue() { + PropertySource map = this.propertySources.get("map"); + CompositePropertySource composite = new CompositePropertySource("composite"); + composite.addPropertySource(map); + this.propertySources.replace("map", composite); + PropertySourcesPropertyValues propertyValues = new PropertySourcesPropertyValues( + this.propertySources); + assertEquals("bar", propertyValues.getPropertyValue("foo").getValue()); + } + @Test public void testEnumeratedValue() { PropertySourcesPropertyValues propertyValues = new PropertySourcesPropertyValues(