From 6418b54f81a9e56242fb78fda4bf95e7b3d4c572 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Mon, 18 May 2015 23:53:39 +0200 Subject: [PATCH] DataBinder tries ConversionService if PropertyEditor could not produce required type Issue: SPR-13042 --- .../beans/TypeConverterDelegate.java | 37 +++++++++------- .../validation/DataBinderTests.java | 42 +++++++++++++++++++ 2 files changed, 65 insertions(+), 14 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 ff4cfa681a..149d7ec3e6 100644 --- a/spring-beans/src/main/java/org/springframework/beans/TypeConverterDelegate.java +++ b/spring-beans/src/main/java/org/springframework/beans/TypeConverterDelegate.java @@ -159,29 +159,28 @@ class TypeConverterDelegate { public T convertIfNecessary(String propertyName, Object oldValue, Object newValue, Class requiredType, TypeDescriptor typeDescriptor) throws IllegalArgumentException { - Object convertedValue = newValue; - // Custom editor for this type? PropertyEditor editor = this.propertyEditorRegistry.findCustomEditor(requiredType, propertyName); - ConversionFailedException firstAttemptEx = null; + ConversionFailedException conversionAttemptEx = null; // No custom editor but custom ConversionService specified? ConversionService conversionService = this.propertyEditorRegistry.getConversionService(); - if (editor == null && conversionService != null && convertedValue != null && typeDescriptor != null) { + if (editor == null && conversionService != null && newValue != null && typeDescriptor != null) { TypeDescriptor sourceTypeDesc = TypeDescriptor.forObject(newValue); - TypeDescriptor targetTypeDesc = typeDescriptor; - if (conversionService.canConvert(sourceTypeDesc, targetTypeDesc)) { + if (conversionService.canConvert(sourceTypeDesc, typeDescriptor)) { try { - return (T) conversionService.convert(convertedValue, sourceTypeDesc, targetTypeDesc); + return (T) conversionService.convert(newValue, sourceTypeDesc, typeDescriptor); } catch (ConversionFailedException ex) { // fallback to default conversion logic below - firstAttemptEx = ex; + conversionAttemptEx = ex; } } } + Object convertedValue = newValue; + // Value not of required type? if (editor != null || (requiredType != null && !ClassUtils.isAssignableValue(requiredType, convertedValue))) { if (requiredType != null && Collection.class.isAssignableFrom(requiredType) && convertedValue instanceof String) { @@ -233,7 +232,7 @@ class TypeConverterDelegate { return (T) convertedValue.toString(); } else if (convertedValue instanceof String && !requiredType.isInstance(convertedValue)) { - if (firstAttemptEx == null && !requiredType.isInterface() && !requiredType.isEnum()) { + if (conversionAttemptEx == null && !requiredType.isInterface() && !requiredType.isEnum()) { try { Constructor strCtor = requiredType.getConstructor(String.class); return BeanUtils.instantiateClass(strCtor, convertedValue); @@ -272,9 +271,19 @@ class TypeConverterDelegate { } if (!ClassUtils.isAssignableValue(requiredType, convertedValue)) { - if (firstAttemptEx != null) { - throw firstAttemptEx; + if (conversionAttemptEx != null) { + // Original exception from former ConversionService call above... + throw conversionAttemptEx; } + else if (conversionService != null) { + // ConversionService not tried before, probably custom editor found + // but editor couldn't produce the required type... + TypeDescriptor sourceTypeDesc = TypeDescriptor.forObject(newValue); + if (conversionService.canConvert(sourceTypeDesc, typeDescriptor)) { + return (T) conversionService.convert(newValue, sourceTypeDesc, typeDescriptor); + } + } + // Definitely doesn't match: throw IllegalArgumentException/IllegalStateException StringBuilder msg = new StringBuilder(); msg.append("Cannot convert value of type [").append(ClassUtils.getDescriptiveType(newValue)); @@ -295,12 +304,12 @@ class TypeConverterDelegate { } } - if (firstAttemptEx != null) { + if (conversionAttemptEx != null) { if (editor == null && !standardConversion && requiredType != null && !Object.class.equals(requiredType)) { - throw firstAttemptEx; + throw conversionAttemptEx; } logger.debug("Original ConversionService attempt failed - ignored since " + - "PropertyEditor based conversion eventually succeeded", firstAttemptEx); + "PropertyEditor based conversion eventually succeeded", conversionAttemptEx); } return (T) convertedValue; diff --git a/spring-context/src/test/java/org/springframework/validation/DataBinderTests.java b/spring-context/src/test/java/org/springframework/validation/DataBinderTests.java index 291f8582af..aecc61995a 100644 --- a/spring-context/src/test/java/org/springframework/validation/DataBinderTests.java +++ b/spring-context/src/test/java/org/springframework/validation/DataBinderTests.java @@ -43,12 +43,15 @@ import org.springframework.beans.NotWritablePropertyException; import org.springframework.beans.NullValueInNestedPathException; import org.springframework.beans.propertyeditors.CustomCollectionEditor; import org.springframework.beans.propertyeditors.CustomNumberEditor; +import org.springframework.beans.propertyeditors.StringTrimmerEditor; import org.springframework.context.i18n.LocaleContextHolder; import org.springframework.context.support.ResourceBundleMessageSource; import org.springframework.context.support.StaticMessageSource; +import org.springframework.core.convert.converter.Converter; import org.springframework.core.convert.support.DefaultConversionService; import org.springframework.format.Formatter; import org.springframework.format.number.NumberStyleFormatter; +import org.springframework.format.support.DefaultFormattingConversionService; import org.springframework.format.support.FormattingConversionService; import org.springframework.tests.sample.beans.BeanWithObjectProperty; import org.springframework.tests.sample.beans.DerivedTestBean; @@ -592,6 +595,19 @@ public class DataBinderTests { assertEquals("test", binder.getBindingResult().getFieldValue("name")); } + @Test + public void testConversionWithInappropriateStringEditor() { + DataBinder dataBinder = new DataBinder(null); + DefaultFormattingConversionService conversionService = new DefaultFormattingConversionService(); + dataBinder.setConversionService(conversionService); + dataBinder.registerCustomEditor(String.class, new StringTrimmerEditor(true)); + + NameBean bean = new NameBean("Fred"); + assertEquals("ConversionService should have invoked toString()", "Fred", dataBinder.convertIfNecessary(bean, String.class)); + conversionService.addConverter(new NameBeanConverter()); + assertEquals("Type converter should have been used", "[Fred]", dataBinder.convertIfNecessary(bean, String.class)); + } + @Test public void testBindingWithAllowedFields() throws Exception { TestBean rod = new TestBean(); @@ -2087,4 +2103,30 @@ public class DataBinderTests { } } + + public static class NameBean { + + private final String name; + + public NameBean(String name) { + this.name = name; + } + public String getName() { + return name; + } + @Override + public String toString() { + return name; + } + } + + + public static class NameBeanConverter implements Converter { + + @Override + public String convert(NameBean source) { + return "[" + source.getName() + "]"; + } + } + }