root
/
spring-boot
mirror of https://github.com/spring-projects/spring-boot.git
1
0
Fork 0
Code Issues Actions Packages Projects Releases Wiki Activity

Fix overlapping property name binding to Maps 

Update RelaxedDataBinder so that multiple overlapping nested property
names can be bound to a Map.

Prior to this commit, properties of the following form could not be
bound to Maps:

  foo: baz
  foo.bar: spam

This was due to BeanWrapperImpl throwing an InvalidPropertyException
when binding `map[foo][bar]` because `foo` is already bound to `baz`.

The updated code now detects such cases and instead uses the binding
property `map[foo.bar]`.

Fixes gh-2610
This commit is contained in:
Dave Syer 2015-03-30 13:51:59 -07:00 committed by Phillip Webb
parent e3f203a8a5
commit b3d8e32ccf
2 changed files with 139 additions and 30 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

127
spring-boot/src/main/java/org/springframework/boot/bind/RelaxedDataBinder.java
View File

@ -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<PropertyValue> list = propertyValues.getPropertyValueList();
for (int i = 0; i < list.size(); i++) {
modifyProperty(propertyValues, wrapper, list.get(i), i);
List<PropertyValue> sortedValues = new ArrayList<PropertyValue>();
List<String> sortedNames = getSortedPropertyNames(propertyValues);
for (String name : sortedNames) {
sortedValues.add(modifyProperty(wrapper,
propertyValues.getPropertyValue(name)));
}
return new MutablePropertyValues(sortedValues);
}
private List<String> getSortedPropertyNames(MutablePropertyValues propertyValues) {
List<String> names = new LinkedList<String>();
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<String> names) {
for (String name : new ArrayList<String>(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<String, Object>());
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<String, Object>());
}
}
}
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<Object>();
}
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<String> prefixes() {
List<String> prefixes = new ArrayList<String>();
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<PathNode> splitPath(String path) {
List<PathNode> nodes = new ArrayList<PathNode>();
for (String name : StringUtils.delimitedListToStringArray(path, ".")) {

42
spring-boot/src/test/java/org/springframework/boot/bind/RelaxedDataBinderTests.java
View File

@ -421,6 +421,48 @@ public class RelaxedDataBinderTests {
assertEquals("123", target.get("value"));
}
@Test
public void testBindMapWithClashInProperties() throws Exception {
Map<String, Object> target = new LinkedHashMap<String, Object>();
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<String, Object> target = new LinkedHashMap<String, Object>();
BindingResult result = bind(target, "vanilla.spam.foo: bar\n"
+ "vanilla.spam.foo.value: 123", "vanilla");
assertEquals(0, result.getErrorCount());
@SuppressWarnings("unchecked")
Map<String, Object> map = (Map<String, Object>) target.get("spam");
assertEquals("123", map.get("foo.value"));
}
@Test
public void testBindMapWithDifferentDeepClashInProperties() throws Exception {
Map<String, Object> target = new LinkedHashMap<String, Object>();
BindingResult result = bind(target, "vanilla.spam.bar: bar\n"
+ "vanilla.spam.bar.value: 123", "vanilla");
assertEquals(0, result.getErrorCount());
@SuppressWarnings("unchecked")
Map<String, Object> map = (Map<String, Object>) target.get("spam");
assertEquals("123", map.get("bar.value"));
}
@Test
public void testBindShallowMap() throws Exception {
Map<String, Object> target = new LinkedHashMap<String, Object>();
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<String, Object> target = new LinkedHashMap<String, Object>();