From 8e754e9065b5703f93909f480933488a2b180429 Mon Sep 17 00:00:00 2001 From: Dave Syer Date: Thu, 14 Jun 2012 11:40:28 +0100 Subject: [PATCH] SPR-9498: don't make assumptions about equality if ConversionService has failed The failure of the conversion service is not fatal, but the check that was in there (line 248) was inadequate to detect the cases that could already be handled by the default property editors. This code path was also not tested anywhere in spring-beans tests until now. --- .../beans/TypeConverterDelegate.java | 3 +- .../beans/BeanWrapperTests.java | 29 ++++++++++++++++++- 2 files changed, 30 insertions(+), 2 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 9776b460b7..64f437320f 100644 --- a/spring-beans/src/main/java/org/springframework/beans/TypeConverterDelegate.java +++ b/spring-beans/src/main/java/org/springframework/beans/TypeConverterDelegate.java @@ -44,6 +44,7 @@ import org.springframework.util.StringUtils; * * @author Juergen Hoeller * @author Rob Harrop + * @author Dave Syer * @since 2.0 * @see BeanWrapperImpl * @see SimpleTypeConverter @@ -244,7 +245,7 @@ class TypeConverterDelegate { } if (firstAttemptEx != null) { - if (editor == null && convertedValue == newValue) { + if (editor == null && convertedValue == newValue && requiredType!=null && !ClassUtils.isAssignableValue(requiredType, convertedValue)) { throw firstAttemptEx; } logger.debug("Original ConversionService attempt failed - ignored since " + 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 49c301284a..f6e68234bb 100644 --- a/spring-beans/src/test/java/org/springframework/beans/BeanWrapperTests.java +++ b/spring-beans/src/test/java/org/springframework/beans/BeanWrapperTests.java @@ -43,14 +43,16 @@ import java.util.TreeMap; import java.util.TreeSet; import org.apache.commons.logging.LogFactory; -import org.junit.Ignore; import org.junit.Test; import org.springframework.beans.factory.annotation.Autowire; import org.springframework.beans.propertyeditors.CustomNumberEditor; import org.springframework.beans.propertyeditors.StringArrayPropertyEditor; import org.springframework.beans.propertyeditors.StringTrimmerEditor; import org.springframework.beans.support.DerivedFromProtectedBaseBean; +import org.springframework.core.convert.ConversionFailedException; +import org.springframework.core.convert.TypeDescriptor; import org.springframework.core.convert.support.DefaultConversionService; +import org.springframework.core.convert.support.GenericConversionService; import org.springframework.util.StopWatch; import org.springframework.util.StringUtils; @@ -66,9 +68,34 @@ import test.beans.TestBean; * @author Alef Arendsen * @author Arjen Poutsma * @author Chris Beams + * @author Dave Syer */ public final class BeanWrapperTests { + @Test + public void testNullNestedTypeDescriptorWithNoConversionService() { + Foo foo = new Foo(); + BeanWrapperImpl wrapper = new BeanWrapperImpl(foo); + wrapper.setAutoGrowNestedPaths(true); + wrapper.setPropertyValue("listOfMaps[0]['luckyNumber']", "9"); + assertEquals("9", foo.listOfMaps.get(0).get("luckyNumber")); + } + + @Test + public void testNullNestedTypeDescriptorWithBadConversionService() { + Foo foo = new Foo(); + BeanWrapperImpl wrapper = new BeanWrapperImpl(foo); + wrapper.setConversionService(new GenericConversionService() { + @Override + public Object convert(Object source, TypeDescriptor sourceType, TypeDescriptor targetType) { + throw new ConversionFailedException(sourceType, targetType, source, null); + } + }); + wrapper.setAutoGrowNestedPaths(true); + wrapper.setPropertyValue("listOfMaps[0]['luckyNumber']", "9"); + assertEquals("9", foo.listOfMaps.get(0).get("luckyNumber")); + } + @Test public void testNullNestedTypeDescriptor() { Foo foo = new Foo();