Added new logic to RelaxedDataBinder to deal with nested stuff

* Leverages existing behaviour of BeanWrapperImpl where possible
to autogrow collections and lists
* Logic for scanning and converting bean paths encapsulated in
BeanPath inner class

[Fixes #53947797] [bs-249]
@ConfigurationProperties cannot bind to Map<String,List<Thing>>

spring-bootstrap/src/main/java/org/springframework/bootstrap/bind/RelaxedDataBinder.java

@@ -17,6 +17,7 @@
 package org.springframework.bootstrap.bind;
 
 import java.net.InetAddress;
+import java.util.ArrayList;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
@@ -26,6 +27,7 @@ import org.springframework.beans.BeanWrapperImpl;
 import org.springframework.beans.InvalidPropertyException;
 import org.springframework.beans.MutablePropertyValues;
 import org.springframework.beans.PropertyValue;
+import org.springframework.core.convert.TypeDescriptor;
 import org.springframework.util.StringUtils;
 import org.springframework.validation.DataBinder;
 
@@ -129,66 +131,228 @@ public class RelaxedDataBinder extends DataBinder {
 
 	private void modifyProperty(MutablePropertyValues propertyValues, BeanWrapper target,
 			PropertyValue propertyValue, int index) {
-
-		String name = propertyValue.getName();
-		StringBuilder builder = new StringBuilder();
-		Class<?> type = target.getWrappedClass();
-
-		for (String key : StringUtils.delimitedListToStringArray(name, ".")) {
-			if (builder.length() != 0) {
-				builder.append(".");
-			}
-			String oldKey = key;
-			key = getActualPropertyName(target, builder.toString(), oldKey);
-			builder.append(key);
-			String base = builder.toString();
-			if (!oldKey.equals(key)) {
-				propertyValues.setPropertyValueAt(
-						new PropertyValue(base, propertyValue.getValue()), index);
-			}
-			type = target.getPropertyType(base);
-
-			// Any nested properties that are maps, are assumed to be simple nested
-			// maps of maps...
-			if (type != null && Map.class.isAssignableFrom(type)) {
-				Map<String, Object> nested = new LinkedHashMap<String, Object>();
-				if (target.getPropertyValue(base) != null) {
-					@SuppressWarnings("unchecked")
-					Map<String, Object> existing = (Map<String, Object>) target
-							.getPropertyValue(base);
-					nested = existing;
-				}
-				else {
-					target.setPropertyValue(base, nested);
-				}
-				modifyPopertiesForMap(nested, propertyValues, index, base);
-				break;
-			}
+		String oldName = propertyValue.getName();
+		String name = normalizePath(target, oldName);
+		if (!name.equals(oldName)) {
+			propertyValues.setPropertyValueAt(
+					new PropertyValue(name, propertyValue.getValue()), index);
 		}
 	}
 
-	private void modifyPopertiesForMap(Map<String, Object> target,
-			MutablePropertyValues propertyValues, int index, String base) {
-		PropertyValue propertyValue = propertyValues.getPropertyValueList().get(index);
-		String name = propertyValue.getName();
-		String suffix = name.substring(base.length());
-		Map<String, Object> value = new LinkedHashMap<String, Object>();
-		String[] tree = StringUtils.delimitedListToStringArray(
-				suffix.startsWith(".") ? suffix.substring(1) : suffix, ".");
-		for (int j = 0; j < tree.length - 1; j++) {
-			if (!target.containsKey(tree[j])) {
-				target.put(tree[j], value);
-			}
-			target = value;
-			value = new LinkedHashMap<String, Object>();
-		}
-		String refName = base + suffix.replaceAll("\\.([a-zA-Z0-9]*)", "[$1]");
-		propertyValues.setPropertyValueAt(
-				new PropertyValue(refName, propertyValue.getValue()), index);
+	protected String normalizePath(BeanWrapper wrapper, String path) {
+		return initializePath(wrapper, new BeanPath(path), 0);
+	}
 
+	private static class BeanPath {
+
+		private List<PathNode> nodes;
+
+		public BeanPath(String path) {
+			this.nodes = splitPath(path);
+		}
+
+		public void mapIndex(int index) {
+			PathNode node = this.nodes.get(index);
+			if (node instanceof PropertyNode) {
+				node = ((PropertyNode) node).mapIndex();
+			}
+			this.nodes.set(index, node);
+		}
+
+		public String prefix(int index) {
+			return range(0, index);
+		}
+
+		public void rename(int index, String name) {
+			this.nodes.get(index).name = name;
+		}
+
+		public String name(int index) {
+			if (index < this.nodes.size()) {
+				return this.nodes.get(index).name;
+			}
+			return null;
+		}
+
+		public int length() {
+			return this.nodes.size();
+		}
+
+		private String range(int start, int end) {
+			StringBuilder builder = new StringBuilder();
+			for (int i = start; i < end; i++) {
+				PathNode node = this.nodes.get(i);
+				builder.append(node);
+			}
+			if (builder.toString().startsWith(("."))) {
+				builder.replace(0, 1, "");
+			}
+			return builder.toString();
+		}
+
+		public boolean isArrayIndex(int index) {
+			return this.nodes.get(index) instanceof ArrayIndexNode;
+		}
+
+		public boolean isProperty(int index) {
+			return this.nodes.get(index) instanceof PropertyNode;
+		}
+
+		@Override
+		public String toString() {
+			return prefix(this.nodes.size());
+		}
+
+		private static class PathNode {
+
+			protected String name;
+
+			public PathNode(String name) {
+				this.name = name;
+			}
+
+		}
+
+		private static class ArrayIndexNode extends PathNode {
+
+			public ArrayIndexNode(String name) {
+				super(name);
+			}
+
+			@Override
+			public String toString() {
+				return "[" + this.name + "]";
+			}
+
+		}
+
+		private static class MapIndexNode extends PathNode {
+
+			public MapIndexNode(String name) {
+				super(name);
+			}
+
+			@Override
+			public String toString() {
+				return "[" + this.name + "]";
+			}
+		}
+
+		private static class PropertyNode extends PathNode {
+
+			public PropertyNode(String name) {
+				super(name);
+			}
+
+			public MapIndexNode mapIndex() {
+				return new MapIndexNode(this.name);
+			}
+
+			@Override
+			public String toString() {
+				return "." + this.name;
+			}
+		}
+
+		private List<PathNode> splitPath(String path) {
+			List<PathNode> nodes = new ArrayList<PathNode>();
+			for (String name : StringUtils.delimitedListToStringArray(path, ".")) {
+				for (String sub : StringUtils.delimitedListToStringArray(name, "[")) {
+					if (StringUtils.hasText(sub)) {
+						if (sub.endsWith("]")) {
+							sub = sub.substring(0, sub.length() - 1);
+							if (sub.matches("[0-9]+")) {
+								nodes.add(new ArrayIndexNode(sub));
+							}
+							else {
+								nodes.add(new MapIndexNode(sub));
+							}
+						}
+						else {
+							nodes.add(new PropertyNode(sub));
+						}
+					}
+				}
+			}
+			return nodes;
+		}
+
+	}
+
+	private String initializePath(BeanWrapper wrapper, BeanPath path, int index) {
+		String prefix = path.prefix(index);
+		String key = path.name(index);
+		if (key == null) {
+			return path.toString();
+		}
+		if (path.isProperty(index)) {
+			key = getActualPropertyName(wrapper, prefix, key);
+			path.rename(index, key);
+		}
+		if (index >= path.length() - 1) {
+			return path.toString();
+		}
+		String name = path.prefix(++index);
+		TypeDescriptor descriptor = wrapper.getPropertyTypeDescriptor(name);
+		if (descriptor == null || descriptor.isMap()) {
+			if (descriptor != null) {
+				wrapper.getPropertyValue(name + "[foo]");
+			}
+			path.mapIndex(index);
+			extendMapIfNecessary(wrapper, path, index);
+		}
+		else if (descriptor.isCollection()) {
+			// TODO: test collection extension
+			extendCollectionIfNecessary(wrapper, path, index);
+		}
+		else if (descriptor.getType().equals(Object.class)) {
+			path.mapIndex(index);
+			name = path.prefix(index + 1);
+			if (wrapper.getPropertyValue(name) == null) {
+				wrapper.setPropertyValue(name, new LinkedHashMap<String, Object>());
+			}
+		}
+		if (index < path.length()) {
+			return initializePath(wrapper, path, index);
+		}
+		return path.toString();
+	}
+
+	private void extendCollectionIfNecessary(BeanWrapper wrapper, BeanPath path, int index) {
+		String name = path.prefix(index);
+		TypeDescriptor elementDescriptor = wrapper.getPropertyTypeDescriptor(name)
+				.getElementTypeDescriptor();
+		if (!elementDescriptor.isMap() && !elementDescriptor.isCollection()
+				&& !elementDescriptor.getType().equals(Object.class)) {
+			return;
+		}
+		Object extend = new LinkedHashMap<String, Object>();
+		if (!elementDescriptor.isMap() && path.isArrayIndex(index + 1)) {
+			extend = new ArrayList<Object>();
+		}
+		wrapper.setPropertyValue(path.prefix(index + 1), extend);
+	}
+
+	private void extendMapIfNecessary(BeanWrapper wrapper, BeanPath path, int index) {
+		String name = path.prefix(index);
+		TypeDescriptor parent = wrapper.getPropertyTypeDescriptor(name);
+		if (parent == null) {
+			return;
+		}
+		TypeDescriptor descriptor = parent.getMapValueTypeDescriptor();
+		if (!descriptor.isMap() && !descriptor.isCollection()
+				&& !descriptor.getType().equals(Object.class)) {
+			return;
+		}
+		Object extend = new LinkedHashMap<String, Object>();
+		if (descriptor.isCollection()) {
+			extend = new ArrayList<Object>();
+		}
+		wrapper.setPropertyValue(path.prefix(index + 1), extend);
 	}
 
 	private String getActualPropertyName(BeanWrapper target, String prefix, String name) {
+		prefix = StringUtils.hasText(prefix) ? prefix + "." : "";
 		for (Variation variation : Variation.values()) {
 			for (Manipulation manipulation : Manipulation.values()) {
 				// Apply all manipulations before attempting variations
@@ -199,7 +363,7 @@ public class RelaxedDataBinder extends DataBinder {
 					}
 				}
 				catch (InvalidPropertyException ex) {
-					// swallow and contrinue
+					// swallow and continue
 				}
 			}
 		}

spring-bootstrap/src/test/java/org/springframework/bootstrap/bind/BindingPreparationTests.java

@@ -0,0 +1,240 @@
+/*
+ * 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.bootstrap.bind;
+
+import java.util.ArrayList;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+
+import org.junit.Ignore;
+import org.junit.Test;
+import org.springframework.beans.BeanWrapperImpl;
+import org.springframework.bootstrap.bind.RelaxedDataBinderTests.TargetWithNestedObject;
+import org.springframework.context.expression.MapAccessor;
+import org.springframework.core.convert.TypeDescriptor;
+import org.springframework.expression.Expression;
+import org.springframework.expression.spel.standard.SpelExpressionParser;
+import org.springframework.expression.spel.support.StandardEvaluationContext;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
+
+/**
+ * @author Dave Syer
+ */
+public class BindingPreparationTests {
+
+	@Test
+	public void testBeanWrapperCreatesNewMaps() throws Exception {
+		TargetWithNestedMap target = new TargetWithNestedMap();
+		BeanWrapperImpl wrapper = new BeanWrapperImpl(target);
+		wrapper.setAutoGrowNestedPaths(true);
+		// For a nested map, you only have to get an element of it for it to be created
+		wrapper.getPropertyValue("nested[foo]");
+		assertNotNull(wrapper.getPropertyValue("nested"));
+	}
+
+	@Test
+	public void testBeanWrapperCreatesNewMapEntries() throws Exception {
+		TargetWithNestedMapOfBean target = new TargetWithNestedMapOfBean();
+		BeanWrapperImpl wrapper = new BeanWrapperImpl(target);
+		wrapper.setAutoGrowNestedPaths(true);
+		// For a nested map, you only have to get an element of it for it to be created
+		wrapper.getPropertyValue("nested[foo]");
+		wrapper.setPropertyValue("nested[foo].foo", "bar");
+		assertNotNull(wrapper.getPropertyValue("nested"));
+		assertNotNull(wrapper.getPropertyValue("nested[foo]"));
+	}
+
+	@Test
+	public void testAutoGrowWithFuzzyNameCapitals() throws Exception {
+		TargetWithNestedMap target = new TargetWithNestedMap();
+		BeanWrapperImpl wrapper = new BeanWrapperImpl(target);
+		wrapper.setAutoGrowNestedPaths(true);
+		RelaxedDataBinder binder = new RelaxedDataBinder(target);
+		String result = binder.normalizePath(wrapper, "NESTED[foo][bar]");
+		assertNotNull(wrapper.getPropertyValue("nested"));
+		assertEquals("nested[foo][bar]", result);
+		assertNotNull(wrapper.getPropertyValue("nested[foo][bar]"));
+	}
+
+	@Test
+	public void testAutoGrowWithFuzzyNameUnderscores() throws Exception {
+		TargetWithNestedMap target = new TargetWithNestedMap();
+		BeanWrapperImpl wrapper = new BeanWrapperImpl(target);
+		wrapper.setAutoGrowNestedPaths(true);
+		RelaxedDataBinder binder = new RelaxedDataBinder(target);
+		String result = binder.normalizePath(wrapper, "nes_ted[foo][bar]");
+		assertNotNull(wrapper.getPropertyValue("nested"));
+		assertEquals("nested[foo][bar]", result);
+		assertNotNull(wrapper.getPropertyValue("nested[foo][bar]"));
+	}
+
+	@Test
+	public void testAutoGrowNewNestedMapOfMaps() throws Exception {
+		TargetWithNestedMap target = new TargetWithNestedMap();
+		BeanWrapperImpl wrapper = new BeanWrapperImpl(target);
+		wrapper.setAutoGrowNestedPaths(true);
+		RelaxedDataBinder binder = new RelaxedDataBinder(target);
+		String result = binder.normalizePath(wrapper, "nested[foo][bar]");
+		assertNotNull(wrapper.getPropertyValue("nested"));
+		assertEquals("nested[foo][bar]", result);
+		assertNotNull(wrapper.getPropertyValue("nested[foo][bar]"));
+	}
+
+	@Test
+	public void testAutoGrowNewNestedMapOfBeans() throws Exception {
+		TargetWithNestedMapOfBean target = new TargetWithNestedMapOfBean();
+		BeanWrapperImpl wrapper = new BeanWrapperImpl(target);
+		wrapper.setAutoGrowNestedPaths(true);
+		RelaxedDataBinder binder = new RelaxedDataBinder(target);
+		String result = binder.normalizePath(wrapper, "nested[foo].foo");
+		assertNotNull(wrapper.getPropertyValue("nested"));
+		assertEquals("nested[foo].foo", result);
+		assertNotNull(wrapper.getPropertyValue("nested[foo]"));
+	}
+
+	@Test
+	public void testAutoGrowNewNestedMapOfBeansWithPeriod() throws Exception {
+		TargetWithNestedMapOfBean target = new TargetWithNestedMapOfBean();
+		BeanWrapperImpl wrapper = new BeanWrapperImpl(target);
+		wrapper.setAutoGrowNestedPaths(true);
+		RelaxedDataBinder binder = new RelaxedDataBinder(target);
+		String result = binder.normalizePath(wrapper, "nested.foo.foo");
+		assertNotNull(wrapper.getPropertyValue("nested"));
+		assertEquals("nested[foo].foo", result);
+	}
+
+	@Test
+	public void testAutoGrowNewNestedMapOfListOfString() throws Exception {
+		TargetWithNestedMapOfListOfString target = new TargetWithNestedMapOfListOfString();
+		BeanWrapperImpl wrapper = new BeanWrapperImpl(target);
+		wrapper.setAutoGrowNestedPaths(true);
+		RelaxedDataBinder binder = new RelaxedDataBinder(target);
+		binder.normalizePath(wrapper, "nested[foo][0]");
+		assertNotNull(wrapper.getPropertyValue("nested"));
+		assertNotNull(wrapper.getPropertyValue("nested[foo]"));
+	}
+
+	@Test
+	public void testBeanWrapperCreatesNewNestedMaps() throws Exception {
+		TargetWithNestedMap target = new TargetWithNestedMap();
+		BeanWrapperImpl wrapper = new BeanWrapperImpl(target);
+		wrapper.setAutoGrowNestedPaths(true);
+		// For a nested map, you only have to get an element of it for it to be created
+		wrapper.getPropertyValue("nested[foo]");
+		// To decide what type to create for nested[foo] we need to look ahead and see
+		// what the user is trying to bind it to, e.g. if nested[foo][bar] then it's a map
+		wrapper.setPropertyValue("nested[foo]", new LinkedHashMap<String, Object>());
+		// But it might equally well be a collection, if nested[foo][0]
+		wrapper.setPropertyValue("nested[foo]", new ArrayList<Object>());
+		// Then it would have to be actually bound to get the list to autogrow
+		wrapper.setPropertyValue("nested[foo][0]", "bar");
+		assertNotNull(wrapper.getPropertyValue("nested[foo][0]"));
+	}
+
+	@Test
+	public void testBeanWrapperCreatesNewObjects() throws Exception {
+		TargetWithNestedObject target = new TargetWithNestedObject();
+		BeanWrapperImpl wrapper = new BeanWrapperImpl(target);
+		wrapper.setAutoGrowNestedPaths(true);
+		// For a nested object, you have to set a property for it to be created
+		wrapper.setPropertyValue("nested.foo", "bar");
+		wrapper.getPropertyValue("nested");
+		assertNotNull(wrapper.getPropertyValue("nested"));
+	}
+
+	@Test
+	public void testBeanWrapperLists() throws Exception {
+		TargetWithNestedMapOfListOfString target = new TargetWithNestedMapOfListOfString();
+		BeanWrapperImpl wrapper = new BeanWrapperImpl(target);
+		wrapper.setAutoGrowNestedPaths(true);
+		TypeDescriptor descriptor = wrapper.getPropertyTypeDescriptor("nested");
+		assertTrue(descriptor.isMap());
+		wrapper.getPropertyValue("nested[foo]");
+		assertNotNull(wrapper.getPropertyValue("nested"));
+		// You also need to bind to a value here
+		wrapper.setPropertyValue("nested[foo][0]", "bar");
+		wrapper.getPropertyValue("nested[foo][0]");
+		assertNotNull(wrapper.getPropertyValue("nested[foo]"));
+	}
+
+	@Test
+	@Ignore("Work in progress")
+	public void testExpressionLists() throws Exception {
+		TargetWithNestedMapOfListOfString target = new TargetWithNestedMapOfListOfString();
+		LinkedHashMap<String, List<String>> map = new LinkedHashMap<String, List<String>>();
+		// map.put("foo", Arrays.asList("bar"));
+		target.setNested(map);
+		SpelExpressionParser parser = new SpelExpressionParser();
+		StandardEvaluationContext context = new StandardEvaluationContext(target);
+		context.addPropertyAccessor(new MapAccessor());
+		Expression expression = parser.parseExpression("nested.foo");
+		assertNotNull(expression.getValue(context));
+	}
+
+	public static class TargetWithNestedMap {
+		private Map<String, Object> nested;
+
+		public Map<String, Object> getNested() {
+			return this.nested;
+		}
+
+		public void setNested(Map<String, Object> nested) {
+			this.nested = nested;
+		}
+	}
+
+	public static class TargetWithNestedMapOfListOfString {
+		private Map<String, List<String>> nested;
+
+		public Map<String, List<String>> getNested() {
+			return this.nested;
+		}
+
+		public void setNested(Map<String, List<String>> nested) {
+			this.nested = nested;
+		}
+	}
+
+	public static class TargetWithNestedMapOfBean {
+		private Map<String, VanillaTarget> nested;
+
+		public Map<String, VanillaTarget> getNested() {
+			return this.nested;
+		}
+
+		public void setNested(Map<String, VanillaTarget> nested) {
+			this.nested = nested;
+		}
+	}
+
+	public static class VanillaTarget {
+
+		private String foo;
+
+		public String getFoo() {
+			return this.foo;
+		}
+
+		public void setFoo(String foo) {
+			this.foo = foo;
+		}
+	}
+}

spring-bootstrap/src/test/java/org/springframework/bootstrap/bind/RelaxedDataBinderTests.java

@@ -36,7 +36,6 @@ import javax.validation.ConstraintValidatorContext;
 import javax.validation.Payload;
 import javax.validation.constraints.NotNull;
 
-import org.junit.Ignore;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.ExpectedException;
@@ -76,12 +75,19 @@ public class RelaxedDataBinderTests {
 	}
 
 	@Test
-	public void testBindUnderscore() throws Exception {
+	public void testBindUnderscoreInActualPropertyName() throws Exception {
 		VanillaTarget target = new VanillaTarget();
 		bind(target, "foo-bar: bar");
 		assertEquals("bar", target.getFoo_bar());
 	}
 
+	@Test
+	public void testBindHyphen() throws Exception {
+		VanillaTarget target = new VanillaTarget();
+		bind(target, "foo-baz: bar");
+		assertEquals("bar", target.getFooBaz());
+	}
+
 	@Test
 	public void testBindCamelCase() throws Exception {
 		VanillaTarget target = new VanillaTarget();
@@ -208,7 +214,7 @@ public class RelaxedDataBinderTests {
 	}
 
 	@Test
-	@Ignore("Should be possible but currently not supported")
+	// @Ignore("Should be possible but currently not supported")
 	// FIXME: bind to map containing beans
 	public void testBindNestedMapOfBean() throws Exception {
 		TargetWithNestedMapOfBean target = new TargetWithNestedMapOfBean();
@@ -218,7 +224,7 @@ public class RelaxedDataBinderTests {
 	}
 
 	@Test
-	@Ignore("Should be possible but currently not supported")
+	// @Ignore("Should be possible but currently not supported")
 	// FIXME: bind to map containing beans
 	public void testBindNestedMapOfListOfBean() throws Exception {
 		TargetWithNestedMapOfListOfBean target = new TargetWithNestedMapOfListOfBean();
@@ -265,7 +271,7 @@ public class RelaxedDataBinderTests {
 	}
 
 	@Test
-	public void testBindMapNestedInMap() throws Exception {
+	public void testBindMapNestedMap() throws Exception {
 		Map<String, Object> target = new LinkedHashMap<String, Object>();
 		BindingResult result = bind(target, "spam: bar\n" + "vanilla.foo.value: 123",
 				"vanilla");

spring-cli/src/test/resources/logback.xml

@@ -14,6 +14,6 @@
 		<appender-ref ref="CONSOLE" />
 	</root>
 	
-	<logger name="org.springframework" level="DEBUG"/>
+	<!-- logger name="org.springframework" level="DEBUG"/-->
 	
 </configuration>