Consistent processing of empty values and catching of RuntimeExceptions for Formatters

Issue: SPR-14345

Author: jhoeller

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

@@ -590,7 +590,7 @@ private Object convertIfNecessary(String propertyName, Object oldValue, Object n
 					new PropertyChangeEvent(this.rootObject, this.nestedPath + propertyName, oldValue, newValue);
 			throw new ConversionNotSupportedException(pce, requiredType, ex);
 		}
-		catch (IllegalArgumentException ex) {
+		catch (Throwable ex) {
 			PropertyChangeEvent pce =
 					new PropertyChangeEvent(this.rootObject, this.nestedPath + propertyName, oldValue, newValue);
 			throw new TypeMismatchException(pce, requiredType, ex);

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2012 the original author or authors.
+ * Copyright 2002-2016 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -73,7 +73,7 @@ private <T> T doConvert(Object value, Class<T> requiredType, MethodParameter met
 		catch (IllegalStateException ex) {
 			throw new ConversionNotSupportedException(value, requiredType, ex);
 		}
-		catch (IllegalArgumentException ex) {
+		catch (Throwable ex) {
 			throw new TypeMismatchException(value, requiredType, ex);
 		}
 	}

spring-context/src/main/java/org/springframework/format/support/FormatterPropertyEditorAdapter.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2015 the original author or authors.
+ * Copyright 2002-2016 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -23,6 +23,7 @@
 import org.springframework.context.i18n.LocaleContextHolder;
 import org.springframework.format.Formatter;
 import org.springframework.util.Assert;
+import org.springframework.util.StringUtils;
 
 /**
  * Adapter that bridges between {@link Formatter} and {@link PropertyEditor}.
@@ -60,17 +61,23 @@ public Class<?> getFieldType() {
 
 	@Override
 	public void setAsText(String text) throws IllegalArgumentException {
-		try {
-			setValue(this.formatter.parse(text, LocaleContextHolder.getLocale()));
+		if (StringUtils.hasText(text)) {
+			try {
+				setValue(this.formatter.parse(text, LocaleContextHolder.getLocale()));
+			}
+			catch (ParseException ex) {
+				throw new IllegalArgumentException("Parse attempt failed for value [" + text + "]", ex);
+			}
 		}
-		catch (ParseException ex) {
-			throw new IllegalArgumentException("Parse attempt failed for value [" + text + "]", ex);
+		else {
+			setValue(null);
 		}
 	}
 
 	@Override
 	public String getAsText() {
-		return this.formatter.print(getValue(), LocaleContextHolder.getLocale());
+		Object value = getValue();
+		return (value != null ? this.formatter.print(value, LocaleContextHolder.getLocale()) : "");
 	}
 
 }

spring-context/src/main/java/org/springframework/format/support/FormattingConversionService.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2015 the original author or authors.
+ * Copyright 2002-2016 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -194,7 +194,7 @@ public Object convert(Object source, TypeDescriptor sourceType, TypeDescriptor t
 				result = this.parser.parse(text, LocaleContextHolder.getLocale());
 			}
 			catch (ParseException ex) {
-				throw new IllegalArgumentException("Unable to parse '" + text + "'", ex);
+				throw new IllegalArgumentException("Parse attempt failed for value [" + text + "]", ex);
 			}
 			if (result == null) {
 				throw new IllegalStateException("Parsers are not allowed to return null");

spring-context/src/test/java/org/springframework/validation/DataBinderTests.java

@@ -403,11 +403,12 @@ public void testBindingErrorWithFormatter() {
 	}
 
 	@Test
-	public void testBindingErrorWithStringFormatter() {
+	public void testBindingErrorWithParseExceptionFromFormatter() {
 		TestBean tb = new TestBean();
 		DataBinder binder = new DataBinder(tb);
 		FormattingConversionService conversionService = new FormattingConversionService();
 		DefaultConversionService.addDefaultConverters(conversionService);
+
 		conversionService.addFormatter(new Formatter<String>() {
 			@Override
 			public String parse(String text, Locale locale) throws ParseException {
@@ -418,6 +419,35 @@ public String print(String object, Locale locale) {
 				return object;
 			}
 		});
+
+		binder.setConversionService(conversionService);
+		MutablePropertyValues pvs = new MutablePropertyValues();
+		pvs.add("name", "test");
+
+		binder.bind(pvs);
+		assertTrue(binder.getBindingResult().hasFieldErrors("name"));
+		assertEquals("typeMismatch", binder.getBindingResult().getFieldError("name").getCode());
+		assertEquals("test", binder.getBindingResult().getFieldValue("name"));
+	}
+
+	@Test
+	public void testBindingErrorWithRuntimeExceptionFromFormatter() {
+		TestBean tb = new TestBean();
+		DataBinder binder = new DataBinder(tb);
+		FormattingConversionService conversionService = new FormattingConversionService();
+		DefaultConversionService.addDefaultConverters(conversionService);
+
+		conversionService.addFormatter(new Formatter<String>() {
+			@Override
+			public String parse(String text, Locale locale) throws ParseException {
+				throw new RuntimeException(text);
+			}
+			@Override
+			public String print(String object, Locale locale) {
+				return object;
+			}
+		});
+
 		binder.setConversionService(conversionService);
 		MutablePropertyValues pvs = new MutablePropertyValues();
 		pvs.add("name", "test");
@@ -581,9 +611,10 @@ public void testBindingErrorWithCustomFormatter() {
 	}
 
 	@Test
-	public void testBindingErrorWithCustomStringFormatter() {
+	public void testBindingErrorWithParseExceptionFromCustomFormatter() {
 		TestBean tb = new TestBean();
 		DataBinder binder = new DataBinder(tb);
+
 		binder.addCustomFormatter(new Formatter<String>() {
 			@Override
 			public String parse(String text, Locale locale) throws ParseException {
@@ -594,12 +625,39 @@ public String print(String object, Locale locale) {
 				return object;
 			}
 		});
+
 		MutablePropertyValues pvs = new MutablePropertyValues();
 		pvs.add("name", "test");
 
 		binder.bind(pvs);
 		assertTrue(binder.getBindingResult().hasFieldErrors("name"));
 		assertEquals("test", binder.getBindingResult().getFieldValue("name"));
+		assertEquals("typeMismatch", binder.getBindingResult().getFieldError("name").getCode());
+	}
+
+	@Test
+	public void testBindingErrorWithRuntimeExceptionFromCustomFormatter() {
+		TestBean tb = new TestBean();
+		DataBinder binder = new DataBinder(tb);
+
+		binder.addCustomFormatter(new Formatter<String>() {
+			@Override
+			public String parse(String text, Locale locale) throws ParseException {
+				throw new RuntimeException(text);
+			}
+			@Override
+			public String print(String object, Locale locale) {
+				return object;
+			}
+		});
+
+		MutablePropertyValues pvs = new MutablePropertyValues();
+		pvs.add("name", "test");
+
+		binder.bind(pvs);
+		assertTrue(binder.getBindingResult().hasFieldErrors("name"));
+		assertEquals("test", binder.getBindingResult().getFieldValue("name"));
+		assertEquals("typeMismatch", binder.getBindingResult().getFieldError("name").getCode());
 	}
 
 	@Test
@@ -991,7 +1049,7 @@ public String print(Integer object, Locale locale) {
 		}, "age");
 
 		MutablePropertyValues pvs = new MutablePropertyValues();
-		pvs.add("age", "");
+		pvs.add("age", "x");
 		binder.bind(pvs);
 
 		assertEquals("argh", binder.getBindingResult().getFieldValue("age"));

spring-core/src/main/java/org/springframework/core/convert/ConversionFailedException.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2015 the original author or authors.
+ * Copyright 2002-2016 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -26,7 +26,7 @@
  * @since 3.0
  */
 @SuppressWarnings("serial")
-public final class ConversionFailedException extends ConversionException {
+public class ConversionFailedException extends ConversionException {
 
 	private final TypeDescriptor sourceType;
 

spring-core/src/main/java/org/springframework/core/convert/ConverterNotFoundException.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2015 the original author or authors.
+ * Copyright 2002-2016 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -25,15 +25,15 @@
  * @since 3.0
  */
 @SuppressWarnings("serial")
-public final class ConverterNotFoundException extends ConversionException {
+public class ConverterNotFoundException extends ConversionException {
 
 	private final TypeDescriptor sourceType;
 
 	private final TypeDescriptor targetType;
 
 
 	/**
-	 * Creates a new conversion executor not found exception.
+	 * Create a new conversion executor not found exception.
 	 * @param sourceType the source type requested to convert from
 	 * @param targetType the target type requested to convert to
 	 */

spring-core/src/main/java/org/springframework/core/convert/support/ConversionUtils.java

@@ -38,7 +38,7 @@ public static Object invokeConverter(GenericConverter converter, Object source,
 		catch (ConversionFailedException ex) {
 			throw ex;
 		}
-		catch (Exception ex) {
+		catch (Throwable ex) {
 			throw new ConversionFailedException(sourceType, targetType, source, ex);
 		}
 	}