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

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,48 +76,75 @@ 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));
-						}
-					}
-				}
-			}
-			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));
+			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;
 					}
-					value = source.getProperty(propertyName.toUpperCase());
-					if (value != null && !this.propertyValues.containsKey(propertyName)) {
+					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));
-						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<PropertySource<?>> extractSources(CompositePropertySource composite) {
+		Field field = ReflectionUtils.findField(CompositePropertySource.class,
+				"propertySources");
+		field.setAccessible(true);
+		try {
+			@SuppressWarnings("unchecked")
+			Collection<PropertySource<?>> collection = (Collection<PropertySource<?>>) field
+					.get(composite);
+			return collection;
+		}
+		catch (Exception e) {
+			throw new IllegalStateException(
+					"Cannot extract property sources from composite", e);
+		}
 	}
 
 	@Override

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

+/*
+ * 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<Foo> 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>(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<String, Object> map = new HashMap<String, Object>();
+
+		public Map<String, Object> getMap() {
+			return this.map;
+		}
+
+		public void setMap(Map<String, Object> map) {
+			this.map = map;
+		}
+	}
+
+}

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(