diff --git a/spring-boot/src/main/java/org/springframework/boot/bind/RelaxedDataBinder.java b/spring-boot/src/main/java/org/springframework/boot/bind/RelaxedDataBinder.java index 7801d66b465..db4afff4a83 100644 --- a/spring-boot/src/main/java/org/springframework/boot/bind/RelaxedDataBinder.java +++ b/spring-boot/src/main/java/org/springframework/boot/bind/RelaxedDataBinder.java @@ -21,6 +21,7 @@ import java.util.ArrayList; import java.util.Collection; import java.util.Collections; import java.util.LinkedHashMap; +import java.util.LinkedList; import java.util.List; import java.util.Map; @@ -46,6 +47,8 @@ import org.springframework.validation.DataBinder; */ public class RelaxedDataBinder extends DataBinder { + private static final Object BLANK = new Object(); + private String namePrefix; private boolean ignoreNestedProperties; @@ -112,11 +115,10 @@ public class RelaxedDataBinder extends DataBinder { @Override protected void doBind(MutablePropertyValues propertyValues) { - propertyValues = modifyProperties(propertyValues, getTarget()); // Harmless additional property editor comes in very handy sometimes... getPropertyEditorRegistry().registerCustomEditor(InetAddress.class, new InetAddressEditor()); - super.doBind(propertyValues); + super.doBind(modifyProperties(propertyValues, getTarget())); } /** @@ -129,22 +131,52 @@ public class RelaxedDataBinder extends DataBinder { */ private MutablePropertyValues modifyProperties(MutablePropertyValues propertyValues, Object target) { - propertyValues = getPropertyValuesForNamePrefix(propertyValues); - if (target instanceof MapHolder) { propertyValues = addMapPrefix(propertyValues); } - BeanWrapper wrapper = new BeanWrapperImpl(target); wrapper.setConversionService(new RelaxedConversionService(getConversionService())); wrapper.setAutoGrowNestedPaths(true); - - List list = propertyValues.getPropertyValueList(); - for (int i = 0; i < list.size(); i++) { - modifyProperty(propertyValues, wrapper, list.get(i), i); + List sortedValues = new ArrayList(); + List sortedNames = getSortedPropertyNames(propertyValues); + for (String name : sortedNames) { + sortedValues.add(modifyProperty(wrapper, + propertyValues.getPropertyValue(name))); + } + return new MutablePropertyValues(sortedValues); + } + + private List getSortedPropertyNames(MutablePropertyValues propertyValues) { + List names = new LinkedList(); + for (PropertyValue propertyValue : propertyValues.getPropertyValueList()) { + names.add(propertyValue.getName()); + } + sortPropertyNames(names); + return names; + } + + /** + * Sort by name so that parent properties get processed first (e.g. 'foo.bar' before + * 'foo.bar.spam'). Don't use Collections.sort() because the order might be + * significant for other property names (it shouldn't be but who knows what people + * might be relying on, e.g. HSQL has a JDBCXADataSource where "databaseName" is a + * synonym for "url"). + * @param names the names to sort + */ + private void sortPropertyNames(List names) { + for (String name : new ArrayList(names)) { + int propertyIndex = names.indexOf(name); + BeanPath path = new BeanPath(name); + for (String prefix : path.prefixes()) { + int prefixIndex = names.indexOf(prefix); + if (prefixIndex >= propertyIndex) { + // The child property has a parent in the list in the wrong order + names.remove(name); + names.add(prefixIndex, name); + } + } } - return propertyValues; } private MutablePropertyValues addMapPrefix(MutablePropertyValues propertyValues) { @@ -175,14 +207,13 @@ public class RelaxedDataBinder extends DataBinder { return rtn; } - private void modifyProperty(MutablePropertyValues propertyValues, BeanWrapper target, - PropertyValue propertyValue, int index) { - String oldName = propertyValue.getName(); - String name = normalizePath(target, oldName); - if (!name.equals(oldName)) { - propertyValues.setPropertyValueAt( - new PropertyValue(name, propertyValue.getValue()), index); + private PropertyValue modifyProperty(BeanWrapper target, PropertyValue propertyValue) { + String name = propertyValue.getName(); + String normalizedName = normalizePath(target, name); + if (!normalizedName.equals(name)) { + return new PropertyValue(normalizedName, propertyValue.getValue()); } + return propertyValue; } /** @@ -214,15 +245,9 @@ public class RelaxedDataBinder extends DataBinder { String name = path.prefix(index); TypeDescriptor descriptor = wrapper.getPropertyTypeDescriptor(name); if (descriptor == null || descriptor.isMap()) { - if (descriptor != null) { - TypeDescriptor valueDescriptor = descriptor.getMapValueTypeDescriptor(); - if (valueDescriptor != null) { - Class valueType = valueDescriptor.getObjectType(); - if (valueType != null - && CharSequence.class.isAssignableFrom(valueType)) { - path.collapseKeys(index); - } - } + if (isMapValueStringType(descriptor) + || isBlanked(wrapper, name, path.name(index))) { + path.collapseKeys(index); } path.mapIndex(index); extendMapIfNecessary(wrapper, path, index); @@ -231,16 +256,43 @@ public class RelaxedDataBinder extends DataBinder { extendCollectionIfNecessary(wrapper, path, index); } else if (descriptor.getType().equals(Object.class)) { + if (isBlanked(wrapper, name, path.name(index))) { + path.collapseKeys(index); + } path.mapIndex(index); - String next = path.prefix(index + 1); - if (wrapper.getPropertyValue(next) == null) { - wrapper.setPropertyValue(next, new LinkedHashMap()); + if (path.isLastNode(index)) { + wrapper.setPropertyValue(path.toString(), BLANK); + } + else { + String next = path.prefix(index + 1); + if (wrapper.getPropertyValue(next) == null) { + wrapper.setPropertyValue(next, new LinkedHashMap()); + } } } - return initializePath(wrapper, path, index); } + private boolean isMapValueStringType(TypeDescriptor descriptor) { + if (descriptor == null || descriptor.getMapValueTypeDescriptor() == null) { + return false; + } + Class valueType = descriptor.getMapValueTypeDescriptor().getObjectType(); + return (valueType != null && CharSequence.class.isAssignableFrom(valueType)); + } + + @SuppressWarnings("rawtypes") + private boolean isBlanked(BeanWrapper wrapper, String propertyName, String key) { + Object value = (wrapper.isReadableProperty(propertyName) ? wrapper + .getPropertyValue(propertyName) : null); + if (value instanceof Map) { + if (((Map) value).get(key) == BLANK) { + return true; + } + } + return false; + } + private void extendCollectionIfNecessary(BeanWrapper wrapper, BeanPath path, int index) { String name = path.prefix(index); TypeDescriptor elementDescriptor = wrapper.getPropertyTypeDescriptor(name) @@ -282,6 +334,9 @@ public class RelaxedDataBinder extends DataBinder { if (descriptor.isCollection()) { extend = new ArrayList(); } + if (descriptor.getType().equals(Object.class) && path.isLastNode(index)) { + extend = BLANK; + } wrapper.setPropertyValue(extensionName, extend); } @@ -382,6 +437,18 @@ public class RelaxedDataBinder extends DataBinder { this.nodes = splitPath(path); } + public List prefixes() { + List prefixes = new ArrayList(); + for (int index = 1; index < this.nodes.size(); index++) { + prefixes.add(prefix(index)); + } + return prefixes; + } + + public boolean isLastNode(int index) { + return index >= this.nodes.size() - 1; + } + private List splitPath(String path) { List nodes = new ArrayList(); for (String name : StringUtils.delimitedListToStringArray(path, ".")) { diff --git a/spring-boot/src/test/java/org/springframework/boot/bind/RelaxedDataBinderTests.java b/spring-boot/src/test/java/org/springframework/boot/bind/RelaxedDataBinderTests.java index 9fe61f508a5..8a9d0f578dc 100644 --- a/spring-boot/src/test/java/org/springframework/boot/bind/RelaxedDataBinderTests.java +++ b/spring-boot/src/test/java/org/springframework/boot/bind/RelaxedDataBinderTests.java @@ -421,6 +421,48 @@ public class RelaxedDataBinderTests { assertEquals("123", target.get("value")); } + @Test + public void testBindMapWithClashInProperties() throws Exception { + Map target = new LinkedHashMap(); + BindingResult result = bind(target, "vanilla.spam: bar\n" + + "vanilla.spam.value: 123", "vanilla"); + assertEquals(0, result.getErrorCount()); + assertEquals(2, target.size()); + assertEquals("bar", target.get("spam")); + assertEquals("123", target.get("spam.value")); + } + + @Test + public void testBindMapWithDeepClashInProperties() throws Exception { + Map target = new LinkedHashMap(); + BindingResult result = bind(target, "vanilla.spam.foo: bar\n" + + "vanilla.spam.foo.value: 123", "vanilla"); + assertEquals(0, result.getErrorCount()); + @SuppressWarnings("unchecked") + Map map = (Map) target.get("spam"); + assertEquals("123", map.get("foo.value")); + } + + @Test + public void testBindMapWithDifferentDeepClashInProperties() throws Exception { + Map target = new LinkedHashMap(); + BindingResult result = bind(target, "vanilla.spam.bar: bar\n" + + "vanilla.spam.bar.value: 123", "vanilla"); + assertEquals(0, result.getErrorCount()); + @SuppressWarnings("unchecked") + Map map = (Map) target.get("spam"); + assertEquals("123", map.get("bar.value")); + } + + @Test + public void testBindShallowMap() throws Exception { + Map target = new LinkedHashMap(); + BindingResult result = bind(target, "vanilla.spam: bar\n" + "vanilla.value: 123", + "vanilla"); + assertEquals(0, result.getErrorCount()); + assertEquals("123", target.get("value")); + } + @Test public void testBindMapNestedMap() throws Exception { Map target = new LinkedHashMap();