From 6bce09831060430b00e6ed93785c7d40ac8529a8 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Wed, 5 Sep 2012 00:59:46 +0200 Subject: [PATCH] refined TypeConverterDelegate's ConversionService exception handling Issue: SPR-9498 --- .../beans/TypeConverterDelegate.java | 30 +++--- .../beans/BeanWrapperTests.java | 93 ++++++++++--------- 2 files changed, 62 insertions(+), 61 deletions(-) diff --git a/spring-beans/src/main/java/org/springframework/beans/TypeConverterDelegate.java b/spring-beans/src/main/java/org/springframework/beans/TypeConverterDelegate.java index b06166d13e..7de77d5e10 100644 --- a/spring-beans/src/main/java/org/springframework/beans/TypeConverterDelegate.java +++ b/spring-beans/src/main/java/org/springframework/beans/TypeConverterDelegate.java @@ -176,11 +176,13 @@ class TypeConverterDelegate { } } if (editor == null) { - editor = findDefaultEditor(requiredType, typeDescriptor); + editor = findDefaultEditor(requiredType); } convertedValue = doConvertValue(oldValue, convertedValue, requiredType, editor); } + boolean standardConversion = false; + if (requiredType != null) { // Try to apply some standard type conversion rules if appropriate. @@ -196,14 +198,17 @@ class TypeConverterDelegate { // Convert elements to target type, if determined. convertedValue = convertToTypedCollection( (Collection) convertedValue, propertyName, requiredType, typeDescriptor); + standardConversion = true; } else if (convertedValue instanceof Map) { // Convert keys and values to respective target type, if determined. convertedValue = convertToTypedMap( (Map) convertedValue, propertyName, requiredType, typeDescriptor); + standardConversion = true; } if (convertedValue.getClass().isArray() && Array.getLength(convertedValue) == 1) { convertedValue = Array.get(convertedValue, 0); + standardConversion = true; } if (String.class.equals(requiredType) && ClassUtils.isPrimitiveOrWrapper(convertedValue.getClass())) { // We can stringify any primitive value... @@ -234,6 +239,7 @@ class TypeConverterDelegate { } convertedValue = attemptToConvertStringToEnum(requiredType, trimmedValue, convertedValue); + standardConversion = true; } } @@ -262,8 +268,7 @@ class TypeConverterDelegate { } if (firstAttemptEx != null) { - if (editor == null && convertedValue == newValue && requiredType != null && - !ClassUtils.isAssignableValue(requiredType, convertedValue)) { + if (editor == null && !standardConversion && requiredType != null && !Object.class.equals(requiredType)) { throw firstAttemptEx; } logger.debug("Original ConversionService attempt failed - ignored since " + @@ -322,16 +327,11 @@ class TypeConverterDelegate { /** * Find a default editor for the given type. * @param requiredType the type to find an editor for - * @param descriptor the JavaBeans descriptor for the property * @return the corresponding editor, or null if none */ - protected PropertyEditor findDefaultEditor(Class requiredType, TypeDescriptor typeDescriptor) { + private PropertyEditor findDefaultEditor(Class requiredType) { PropertyEditor editor = null; - //if (typeDescriptor instanceof PropertyTypeDescriptor) { - //PropertyDescriptor pd = ((PropertyTypeDescriptor) typeDescriptor).getPropertyDescriptor(); - //editor = pd.createPropertyEditor(this.targetObject); - //} - if (editor == null && requiredType != null) { + if (requiredType != null) { // No custom editor -> check BeanWrapperImpl's default editors. editor = this.propertyEditorRegistry.getDefaultEditor(requiredType); if (editor == null && !String.class.equals(requiredType)) { @@ -353,7 +353,7 @@ class TypeConverterDelegate { * @return the new value, possibly the result of type conversion * @throws IllegalArgumentException if type conversion failed */ - protected Object doConvertValue(Object oldValue, Object newValue, Class requiredType, PropertyEditor editor) { + private Object doConvertValue(Object oldValue, Object newValue, Class requiredType, PropertyEditor editor) { Object convertedValue = newValue; boolean sharedEditor = false; @@ -440,7 +440,7 @@ class TypeConverterDelegate { * @param editor the PropertyEditor to use * @return the converted value */ - protected Object doConvertTextValue(Object oldValue, String newTextValue, PropertyEditor editor) { + private Object doConvertTextValue(Object oldValue, String newTextValue, PropertyEditor editor) { try { editor.setValue(oldValue); } @@ -454,7 +454,7 @@ class TypeConverterDelegate { return editor.getValue(); } - protected Object convertToTypedArray(Object input, String propertyName, Class componentType) { + private Object convertToTypedArray(Object input, String propertyName, Class componentType) { if (input instanceof Collection) { // Convert Collection elements to array elements. Collection coll = (Collection) input; @@ -493,7 +493,7 @@ class TypeConverterDelegate { } @SuppressWarnings("unchecked") - protected Collection convertToTypedCollection( + private Collection convertToTypedCollection( Collection original, String propertyName, Class requiredType, TypeDescriptor typeDescriptor) { if (!Collection.class.isAssignableFrom(requiredType)) { @@ -575,7 +575,7 @@ class TypeConverterDelegate { } @SuppressWarnings("unchecked") - protected Map convertToTypedMap( + private Map convertToTypedMap( Map original, String propertyName, Class requiredType, TypeDescriptor typeDescriptor) { if (!Map.class.isAssignableFrom(requiredType)) { diff --git a/spring-beans/src/test/java/org/springframework/beans/BeanWrapperTests.java b/spring-beans/src/test/java/org/springframework/beans/BeanWrapperTests.java index f648d88bea..b584e4a51e 100644 --- a/spring-beans/src/test/java/org/springframework/beans/BeanWrapperTests.java +++ b/spring-beans/src/test/java/org/springframework/beans/BeanWrapperTests.java @@ -72,6 +72,28 @@ import test.beans.TestBean; */ public final class BeanWrapperTests { + @Test + public void testNullNestedTypeDescriptor() { + Foo foo = new Foo(); + BeanWrapperImpl wrapper = new BeanWrapperImpl(foo); + wrapper.setConversionService(new DefaultConversionService()); + wrapper.setAutoGrowNestedPaths(true); + wrapper.setPropertyValue("listOfMaps[0]['luckyNumber']", "9"); + assertEquals("9", foo.listOfMaps.get(0).get("luckyNumber")); + } + + @Test + public void testNullNestedTypeDescriptor2() { + Foo foo = new Foo(); + BeanWrapperImpl wrapper = new BeanWrapperImpl(foo); + wrapper.setConversionService(new DefaultConversionService()); + wrapper.setAutoGrowNestedPaths(true); + Map map = new HashMap(); + map.put("favoriteNumber", "9"); + wrapper.setPropertyValue("list[0]", map); + assertEquals(map, foo.list.get(0)); + } + @Test public void testNullNestedTypeDescriptorWithNoConversionService() { Foo foo = new Foo(); @@ -96,52 +118,6 @@ public final class BeanWrapperTests { assertEquals("9", foo.listOfMaps.get(0).get("luckyNumber")); } - @Test - public void testNullNestedTypeDescriptor() { - Foo foo = new Foo(); - BeanWrapperImpl wrapper = new BeanWrapperImpl(foo); - wrapper.setConversionService(new DefaultConversionService()); - wrapper.setAutoGrowNestedPaths(true); - wrapper.setPropertyValue("listOfMaps[0]['luckyNumber']", "9"); - assertEquals("9", foo.listOfMaps.get(0).get("luckyNumber")); - } - - @Test - public void testNullNestedTypeDescriptor2() { - Foo foo = new Foo(); - BeanWrapperImpl wrapper = new BeanWrapperImpl(foo); - wrapper.setConversionService(new DefaultConversionService()); - wrapper.setAutoGrowNestedPaths(true); - Map map = new HashMap(); - map.put("favoriteNumber", "9"); - wrapper.setPropertyValue("list[0]", map); - assertEquals(map, foo.list.get(0)); - } - - static class Foo { - - private List list; - - private List listOfMaps; - - public List getList() { - return list; - } - - public void setList(List list) { - this.list = list; - } - - public List getListOfMaps() { - return listOfMaps; - } - - public void setListOfMaps(List listOfMaps) { - this.listOfMaps = listOfMaps; - } - - } - @Test public void testIsReadablePropertyNotReadable() { NoRead nr = new NoRead(); @@ -1565,6 +1541,31 @@ public final class BeanWrapperTests { assertEquals(TestEnum.TEST_VALUE, consumer.getEnumValue()); } + + private static class Foo { + + private List list; + + private List listOfMaps; + + public List getList() { + return list; + } + + public void setList(List list) { + this.list = list; + } + + public List getListOfMaps() { + return listOfMaps; + } + + public void setListOfMaps(List listOfMaps) { + this.listOfMaps = listOfMaps; + } + } + + private static class DifferentTestBean extends TestBean { // class to test naming of beans in a BeanWrapper error message }