DataBinder tries ConversionService if PropertyEditor could not produce required type

Issue: SPR-13042

spring-beans/src/main/java/org/springframework/beans/TypeConverterDelegate.java

@@ -159,29 +159,28 @@ class TypeConverterDelegate {
 	public <T> T convertIfNecessary(String propertyName, Object oldValue, Object newValue,
 			Class<T> 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, typeDescriptor)) {
-			if (conversionService.canConvert(sourceTypeDesc, targetTypeDesc)) {
 				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<T> strCtor = requiredType.getConstructor(String.class);
 							return BeanUtils.instantiateClass(strCtor, convertedValue);
@@ -272,9 +271,19 @@ class TypeConverterDelegate {
 			}
 
 			if (!ClassUtils.isAssignableValue(requiredType, convertedValue)) {
-				if (firstAttemptEx != null) {
+				if (conversionAttemptEx != null) {
-					throw firstAttemptEx;
+					// 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;

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<NameBean, String> {
+
+		@Override
+		public String convert(NameBean source) {
+			return "[" + source.getName() + "]";
+		}
+	}
+
 }