From 5a287455273270a20742f03e4546acde9e857bee Mon Sep 17 00:00:00 2001 From: Dave Syer Date: Wed, 26 Aug 2015 14:41:44 -0700 Subject: [PATCH] Handle unresolved property names with underscore When the configuration property name is not resolved against the target bean's property descriptors and the property name is delimited by `underscore` instead of `dot`, then `RelaxedDataBinder` doesn't bind those properties to the target if the property is of type Map. With thanks to @ilayaperumalg for the original PR. Updated by @dsyer to recognize that the property is of type Map, rather than blindly trying all combinations of property paths. Fixes gh-3836 --- .../boot/bind/RelaxedDataBinder.java | 31 ++++++++----- .../boot/bind/RelaxedDataBinderTests.java | 44 ++++++++++++++----- 2 files changed, 54 insertions(+), 21 deletions(-) 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 96f6b2a7b1..258be9e123 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 @@ -83,8 +83,8 @@ public class RelaxedDataBinder extends DataBinder { * @param namePrefix An optional prefix to be used when reading properties */ public RelaxedDataBinder(Object target, String namePrefix) { - super(wrapTarget(target), (StringUtils.hasLength(namePrefix) ? namePrefix - : DEFAULT_OBJECT_NAME)); + super(wrapTarget(target), + (StringUtils.hasLength(namePrefix) ? namePrefix : DEFAULT_OBJECT_NAME)); this.namePrefix = cleanNamePrefix(namePrefix); } @@ -146,7 +146,8 @@ public class RelaxedDataBinder extends DataBinder { propertyValues = addMapPrefix(propertyValues); } BeanWrapper wrapper = new BeanWrapperImpl(target); - wrapper.setConversionService(new RelaxedConversionService(getConversionService())); + wrapper.setConversionService( + new RelaxedConversionService(getConversionService())); wrapper.setAutoGrowNestedPaths(true); List sortedValues = new ArrayList(); Set modifiedNames = new HashSet(); @@ -214,8 +215,8 @@ public class RelaxedDataBinder extends DataBinder { name = name.substring(candidate.length()); if (!(this.ignoreNestedProperties && name.contains("."))) { PropertyOrigin propertyOrigin = findPropertyOrigin(value); - rtn.addPropertyValue(new OriginCapablePropertyValue(name, value - .getValue(), propertyOrigin)); + rtn.addPropertyValue(new OriginCapablePropertyValue(name, + value.getValue(), propertyOrigin)); } } } @@ -223,7 +224,8 @@ public class RelaxedDataBinder extends DataBinder { return rtn; } - private PropertyValue modifyProperty(BeanWrapper target, PropertyValue propertyValue) { + private PropertyValue modifyProperty(BeanWrapper target, + PropertyValue propertyValue) { String name = propertyValue.getName(); String normalizedName = normalizePath(target, name); if (!normalizedName.equals(name)) { @@ -277,8 +279,8 @@ public class RelaxedDataBinder extends DataBinder { } } }; - beanWrapper.setConversionService(new RelaxedConversionService( - getConversionService())); + beanWrapper.setConversionService( + new RelaxedConversionService(getConversionService())); beanWrapper.registerCustomEditor(InetAddress.class, new InetAddressEditor()); return beanWrapper; @@ -352,8 +354,8 @@ public class RelaxedDataBinder extends DataBinder { @SuppressWarnings("rawtypes") private boolean isBlanked(BeanWrapper wrapper, String propertyName, String key) { - Object value = (wrapper.isReadableProperty(propertyName) ? wrapper - .getPropertyValue(propertyName) : null); + Object value = (wrapper.isReadableProperty(propertyName) + ? wrapper.getPropertyValue(propertyName) : null); if (value instanceof Map) { if (((Map) value).get(key) == BLANK) { return true; @@ -362,7 +364,8 @@ public class RelaxedDataBinder extends DataBinder { return false; } - private void extendCollectionIfNecessary(BeanWrapper wrapper, BeanPath path, int index) { + private void extendCollectionIfNecessary(BeanWrapper wrapper, BeanPath path, + int index) { String name = path.prefix(index); TypeDescriptor elementDescriptor = wrapper.getPropertyTypeDescriptor(name) .getElementTypeDescriptor(); @@ -425,6 +428,12 @@ public class RelaxedDataBinder extends DataBinder { candidate.append(field); String nested = resolvePropertyName(target, prefix, candidate.toString()); if (nested != null) { + Class type = target.getPropertyType(nested); + if (type != null && Map.class.isAssignableFrom(type)) { + // Special case for map property (gh-3836). Maybe could be fixed + // in spring-beans)? + return nested + "[" + name.substring(candidate.length() + 1) + "]"; + } String propertyName = resolvePropertyName(target, joinString(prefix, nested), name.substring(candidate.length() + 1)); 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 71b8a92c5e..4db76f6a6d 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 @@ -16,6 +16,15 @@ package org.springframework.boot.bind; +import static java.lang.annotation.RetentionPolicy.RUNTIME; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.nullValue; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertThat; + import java.lang.annotation.Documented; import java.lang.annotation.ElementType; import java.lang.annotation.Retention; @@ -51,15 +60,6 @@ import org.springframework.validation.DataBinder; import org.springframework.validation.FieldError; import org.springframework.validation.beanvalidation.LocalValidatorFactoryBean; -import static java.lang.annotation.RetentionPolicy.RUNTIME; -import static org.hamcrest.Matchers.equalTo; -import static org.hamcrest.Matchers.nullValue; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertThat; - /** * Tests for {@link RelaxedDataBinder}. * @@ -323,6 +323,14 @@ public class RelaxedDataBinderTests { assertEquals("123", target.getNested().get("value")); } + @Test + public void testBindNestedMapPropsWithUnderscores() throws Exception { + TargetWithNestedMap target = new TargetWithNestedMap(); + bind(target, "nested_foo: bar\n" + "nested_value: 123"); + assertEquals("123", target.getNested().get("value")); + assertEquals("bar", target.getNested().get("foo")); + } + @Test public void testBindNestedUntypedMap() throws Exception { TargetWithNestedUntypedMap target = new TargetWithNestedUntypedMap(); @@ -338,6 +346,22 @@ public class RelaxedDataBinderTests { assertEquals("123", target.getNested().get("value.foo")); } + @Test + public void testBindNestedMapOfStringWithUnderscore() throws Exception { + TargetWithNestedMapOfString target = new TargetWithNestedMapOfString(); + bind(target, "nested_foo: bar\n" + "nested_value_foo: 123"); + assertEquals("bar", target.getNested().get("foo")); + assertEquals("123", target.getNested().get("value_foo")); + } + + @Test + public void testBindNestedMapOfStringWithUnderscoreAndupperCase() throws Exception { + TargetWithNestedMapOfString target = new TargetWithNestedMapOfString(); + bind(target, "NESTED_FOO: bar\n" + "NESTED_VALUE_FOO: 123"); + assertEquals("bar", target.getNested().get("FOO")); + assertEquals("123", target.getNested().get("VALUE_FOO")); + } + @Test public void testBindNestedMapOfStringReferenced() throws Exception { TargetWithNestedMapOfString target = new TargetWithNestedMapOfString(); @@ -686,7 +710,7 @@ public class RelaxedDataBinderTests { } public static class RequiredKeysValidator implements - ConstraintValidator> { + ConstraintValidator> { private String[] fields;