Constructor-provided field values get recorded for failed binding result

Also, TypeMismatchExceptions get registered via BindingErrorProcessor.

Issue: SPR-16449

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2017 the original author or authors.
+ * Copyright 2002-2018 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.
@@ -19,6 +19,7 @@ package org.springframework.beans;
 import java.beans.PropertyChangeEvent;
 
 import org.springframework.lang.Nullable;
+import org.springframework.util.Assert;
 import org.springframework.util.ClassUtils;
 
 /**
@@ -36,6 +37,9 @@ public class TypeMismatchException extends PropertyAccessException {
 	public static final String ERROR_CODE = "typeMismatch";
 
 
+	@Nullable
+	private String propertyName;
+
 	@Nullable
 	private transient Object value;
 
@@ -69,6 +73,7 @@ public class TypeMismatchException extends PropertyAccessException {
 				(propertyChangeEvent.getPropertyName() != null ?
 				 " for property '" + propertyChangeEvent.getPropertyName() + "'" : ""),
 				cause);
+		this.propertyName = propertyChangeEvent.getPropertyName();
 		this.value = propertyChangeEvent.getNewValue();
 		this.requiredType = requiredType;
 	}
@@ -77,6 +82,7 @@ public class TypeMismatchException extends PropertyAccessException {
 	 * Create a new TypeMismatchException without PropertyChangeEvent.
 	 * @param value the offending value that couldn't be converted (may be {@code null})
 	 * @param requiredType the required target type (or {@code null} if not known)
+	 * @see #initPropertyName
 	 */
 	public TypeMismatchException(@Nullable Object value, @Nullable Class<?> requiredType) {
 		this(value, requiredType, null);
@@ -87,6 +93,7 @@ public class TypeMismatchException extends PropertyAccessException {
 	 * @param value the offending value that couldn't be converted (may be {@code null})
 	 * @param requiredType the required target type (or {@code null} if not known)
 	 * @param cause the root cause (may be {@code null})
+	 * @see #initPropertyName
 	 */
 	public TypeMismatchException(@Nullable Object value, @Nullable Class<?> requiredType, @Nullable Throwable cause) {
 		super("Failed to convert value of type '" + ClassUtils.getDescriptiveType(value) + "'" +
@@ -97,6 +104,27 @@ public class TypeMismatchException extends PropertyAccessException {
 	}
 
 
+	/**
+	 * Initialize this exception's property name for exposure through {@link #getPropertyName()},
+	 * as an alternative to having it initialized via a {@link PropertyChangeEvent}.
+	 * @param propertyName the property name to expose
+	 * @since 5.0.4
+	 * @see #TypeMismatchException(Object, Class)
+	 * @see #TypeMismatchException(Object, Class, Throwable)
+	 */
+	public void initPropertyName(String propertyName) {
+		Assert.state(this.propertyName == null, "Property name already initialized");
+		this.propertyName = propertyName;
+	}
+
+	/**
+	 * Return the name of the affected property, if available.
+	 */
+	@Nullable
+	public String getPropertyName() {
+		return this.propertyName;
+	}
+
 	/**
 	 * Return the offending value (may be {@code null}).
 	 */

spring-context/src/main/java/org/springframework/validation/AbstractBindingResult.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2016 the original author or authors.
+ * Copyright 2002-2018 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.
@@ -19,6 +19,7 @@ package org.springframework.validation;
 import java.beans.PropertyEditor;
 import java.io.Serializable;
 import java.util.Collections;
+import java.util.HashMap;
 import java.util.HashSet;
 import java.util.LinkedHashMap;
 import java.util.LinkedList;
@@ -51,6 +52,10 @@ public abstract class AbstractBindingResult extends AbstractErrors implements Bi
 
 	private final List<ObjectError> errors = new LinkedList<>();
 
+	private final Map<String, Class<?>> fieldTypes = new HashMap<>(0);
+
+	private final Map<String, Object> fieldValues = new HashMap<>(0);
+
 	private final Set<String> suppressedFields = new HashSet<>();
 
 
@@ -63,6 +68,7 @@ public abstract class AbstractBindingResult extends AbstractErrors implements Bi
 		this.objectName = objectName;
 	}
 
+
 	/**
 	 * Set the strategy to use for resolving errors into message codes.
 	 * Default is DefaultMessageCodesResolver.
@@ -90,7 +96,6 @@ public abstract class AbstractBindingResult extends AbstractErrors implements Bi
 		return this.objectName;
 	}
 
-
 	@Override
 	public void reject(String errorCode, @Nullable Object[] errorArgs, @Nullable String defaultMessage) {
 		addError(new ObjectError(getObjectName(), resolveMessageCodes(errorCode), errorArgs, defaultMessage));
@@ -115,11 +120,6 @@ public abstract class AbstractBindingResult extends AbstractErrors implements Bi
 		addError(fe);
 	}
 
-	@Override
-	public void addError(ObjectError error) {
-		this.errors.add(error);
-	}
-
 	@Override
 	public void addAllErrors(Errors errors) {
 		if (!errors.getObjectName().equals(getObjectName())) {
@@ -128,19 +128,6 @@ public abstract class AbstractBindingResult extends AbstractErrors implements Bi
 		this.errors.addAll(errors.getAllErrors());
 	}
 
-	@Override
-	public String[] resolveMessageCodes(String errorCode) {
-		return getMessageCodesResolver().resolveMessageCodes(errorCode, getObjectName());
-	}
-
-	@Override
-	public String[] resolveMessageCodes(String errorCode, @Nullable String field) {
-		Class<?> fieldType = getFieldType(field);
-		return getMessageCodesResolver().resolveMessageCodes(
-				errorCode, getObjectName(), fixedField(field), fieldType);
-	}
-
-
 	@Override
 	public boolean hasErrors() {
 		return !this.errors.isEmpty();
@@ -231,14 +218,19 @@ public abstract class AbstractBindingResult extends AbstractErrors implements Bi
 	@Nullable
 	public Object getFieldValue(String field) {
 		FieldError fieldError = getFieldError(field);
-		// Use rejected value in case of error, current bean property value else.
-		Object value = (fieldError != null ? fieldError.getRejectedValue() :
-				getActualFieldValue(fixedField(field)));
-		// Apply formatting, but not on binding failures like type mismatches.
-		if (fieldError == null || !fieldError.isBindingFailure()) {
-			value = formatFieldValue(field, value);
+		// Use rejected value in case of error, current field value otherwise.
+		if (fieldError != null) {
+			Object value = fieldError.getRejectedValue();
+			// Do not apply formatting on binding failures like type mismatches.
+			return (fieldError.isBindingFailure() ? value : formatFieldValue(field, value));
+		}
+		else if (getTarget() != null) {
+			Object value = getActualFieldValue(fixedField(field));
+			return formatFieldValue(field, value);
+		}
+		else {
+			return this.fieldValues.get(field);
 		}
-		return value;
 	}
 
 	/**
@@ -250,11 +242,13 @@ public abstract class AbstractBindingResult extends AbstractErrors implements Bi
 	@Override
 	@Nullable
 	public Class<?> getFieldType(@Nullable String field) {
-		Object value = getActualFieldValue(fixedField(field));
-		if (value != null) {
-			return value.getClass();
+		if (getTarget() != null) {
+			Object value = getActualFieldValue(fixedField(field));
+			if (value != null) {
+				return value.getClass();
+			}
 		}
-		return null;
+		return this.fieldTypes.get(field);
 	}
 
 
@@ -287,7 +281,7 @@ public abstract class AbstractBindingResult extends AbstractErrors implements Bi
 	@Override
 	@Nullable
 	public Object getRawFieldValue(String field) {
-		return getActualFieldValue(fixedField(field));
+		return (getTarget() != null ? getActualFieldValue(fixedField(field)) : null);
 	}
 
 	/**
@@ -320,6 +314,29 @@ public abstract class AbstractBindingResult extends AbstractErrors implements Bi
 		return null;
 	}
 
+	@Override
+	public String[] resolveMessageCodes(String errorCode) {
+		return getMessageCodesResolver().resolveMessageCodes(errorCode, getObjectName());
+	}
+
+	@Override
+	public String[] resolveMessageCodes(String errorCode, @Nullable String field) {
+		Class<?> fieldType = (getTarget() != null ? getFieldType(field) : null);
+		return getMessageCodesResolver().resolveMessageCodes(
+				errorCode, getObjectName(), fixedField(field), fieldType);
+	}
+
+	@Override
+	public void addError(ObjectError error) {
+		this.errors.add(error);
+	}
+
+	@Override
+	public void recordFieldValue(String field, Class<?> type, Object value) {
+		this.fieldTypes.put(field, type);
+		this.fieldValues.put(field, value);
+	}
+
 	/**
 	 * Mark the specified disallowed field as suppressed.
 	 * <p>The data binder invokes this for each field value that was
@@ -333,7 +350,7 @@ public abstract class AbstractBindingResult extends AbstractErrors implements Bi
 
 	/**
 	 * Return the list of fields that were suppressed during the bind process.
-	 * <p>Can be used to determine whether any field values were targetting
+	 * <p>Can be used to determine whether any field values were targeting
 	 * disallowed fields.
 	 * @see DataBinder#setAllowedFields
 	 */

spring-context/src/main/java/org/springframework/validation/AbstractPropertyBindingResult.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2017 the original author or authors.
+ * Copyright 2002-2018 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.
@@ -71,7 +71,7 @@ public abstract class AbstractPropertyBindingResult extends AbstractBindingResul
 	 */
 	@Override
 	public PropertyEditorRegistry getPropertyEditorRegistry() {
-		return getPropertyAccessor();
+		return (getTarget() != null ? getPropertyAccessor() : null);
 	}
 
 	/**
@@ -90,7 +90,8 @@ public abstract class AbstractPropertyBindingResult extends AbstractBindingResul
 	@Override
 	@Nullable
 	public Class<?> getFieldType(@Nullable String field) {
-		return getPropertyAccessor().getPropertyType(fixedField(field));
+		return (getTarget() != null ? getPropertyAccessor().getPropertyType(fixedField(field)) :
+				super.getFieldType(field));
 	}
 
 	/**
@@ -161,7 +162,7 @@ public abstract class AbstractPropertyBindingResult extends AbstractBindingResul
 		PropertyEditor editor = super.findEditor(field, valueTypeForLookup);
 		if (editor == null && this.conversionService != null) {
 			TypeDescriptor td = null;
-			if (field != null) {
+			if (field != null && getTarget() != null) {
 				TypeDescriptor ptd = getPropertyAccessor().getPropertyTypeDescriptor(fixedField(field));
 				if (ptd != null && (valueType == null || valueType.isAssignableFrom(ptd.getType()))) {
 					td = ptd;

spring-context/src/main/java/org/springframework/validation/BindException.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2017 the original author or authors.
+ * Copyright 2002-2018 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.
@@ -259,11 +259,6 @@ public class BindException extends Exception implements BindingResult {
 		return this.bindingResult.getPropertyEditorRegistry();
 	}
 
-	@Override
-	public void addError(ObjectError error) {
-		this.bindingResult.addError(error);
-	}
-
 	@Override
 	public String[] resolveMessageCodes(String errorCode) {
 		return this.bindingResult.resolveMessageCodes(errorCode);
@@ -274,6 +269,16 @@ public class BindException extends Exception implements BindingResult {
 		return this.bindingResult.resolveMessageCodes(errorCode, field);
 	}
 
+	@Override
+	public void addError(ObjectError error) {
+		this.bindingResult.addError(error);
+	}
+
+	@Override
+	public void recordFieldValue(String field, Class<?> type, Object value) {
+		this.bindingResult.recordFieldValue(field, type, value);
+	}
+
 	@Override
 	public void recordSuppressedField(String field) {
 		this.bindingResult.recordSuppressedField(field);

spring-context/src/main/java/org/springframework/validation/BindingResult.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2017 the original author or authors.
+ * Copyright 2002-2018 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.
@@ -82,8 +82,7 @@ public interface BindingResult extends Errors {
 	 * Extract the raw field value for the given field.
 	 * Typically used for comparison purposes.
 	 * @param field the field to check
-	 * @return the current value of the field in its raw form,
-	 * or {@code null} if not known
+	 * @return the current value of the field in its raw form, or {@code null} if not known
 	 */
 	@Nullable
 	Object getRawFieldValue(String field);
@@ -107,15 +106,6 @@ public interface BindingResult extends Errors {
 	@Nullable
 	PropertyEditorRegistry getPropertyEditorRegistry();
 
-	/**
-	 * Add a custom {@link ObjectError} or {@link FieldError} to the errors list.
-	 * <p>Intended to be used by cooperating strategies such as {@link BindingErrorProcessor}.
-	 * @see ObjectError
-	 * @see FieldError
-	 * @see BindingErrorProcessor
-	 */
-	void addError(ObjectError error);
-
 	/**
 	 * Resolve the given error code into message codes.
 	 * <p>Calls the configured {@link MessageCodesResolver} with appropriate parameters.
@@ -133,13 +123,37 @@ public interface BindingResult extends Errors {
 	 */
 	String[] resolveMessageCodes(String errorCode, String field);
 
+	/**
+	 * Add a custom {@link ObjectError} or {@link FieldError} to the errors list.
+	 * <p>Intended to be used by cooperating strategies such as {@link BindingErrorProcessor}.
+	 * @see ObjectError
+	 * @see FieldError
+	 * @see BindingErrorProcessor
+	 */
+	void addError(ObjectError error);
+
+	/**
+	 * Record the given value for the specified field.
+	 * <p>To be used when a target object cannot be constructed, making
+	 * the original field values available through {@link #getFieldValue}.
+	 * In case of a registered error, the rejected value will be exposed
+	 * for each affected field.
+	 * @param field the field to record the value for
+	 * @param type the type of the field
+	 * @param value the original value
+	 * @since 5.0.4
+	 */
+	default void recordFieldValue(String field, Class<?> type, Object value) {
+	}
+
 	/**
 	 * Mark the specified disallowed field as suppressed.
 	 * <p>The data binder invokes this for each field value that was
 	 * detected to target a disallowed field.
 	 * @see DataBinder#setAllowedFields
 	 */
-	void recordSuppressedField(String field);
+	default void recordSuppressedField(String field) {
+	}
 
 	/**
 	 * Return the list of fields that were suppressed during the bind process.
@@ -147,6 +161,8 @@ public interface BindingResult extends Errors {
 	 * disallowed fields.
 	 * @see DataBinder#setAllowedFields
 	 */
-	String[] getSuppressedFields();
+	default String[] getSuppressedFields() {
+		return new String[0];
+	}
 
 }

spring-web/src/main/java/org/springframework/web/bind/support/WebExchangeBindException.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2017 the original author or authors.
+ * Copyright 2002-2018 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.
@@ -245,11 +245,6 @@ public class WebExchangeBindException extends ServerWebInputException implements
 		return this.bindingResult.getPropertyEditorRegistry();
 	}
 
-	@Override
-	public void addError(ObjectError error) {
-		this.bindingResult.addError(error);
-	}
-
 	@Override
 	public String[] resolveMessageCodes(String errorCode) {
 		return this.bindingResult.resolveMessageCodes(errorCode);
@@ -260,6 +255,16 @@ public class WebExchangeBindException extends ServerWebInputException implements
 		return this.bindingResult.resolveMessageCodes(errorCode, field);
 	}
 
+	@Override
+	public void addError(ObjectError error) {
+		this.bindingResult.addError(error);
+	}
+
+	@Override
+	public void recordFieldValue(String field, Class<?> type, Object value) {
+		this.bindingResult.recordFieldValue(field, type, value);
+	}
+
 	@Override
 	public void recordSuppressedField(String field) {
 		this.bindingResult.recordSuppressedField(field);

spring-web/src/main/java/org/springframework/web/method/annotation/ModelAttributeMethodProcessor.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2017 the original author or authors.
+ * Copyright 2002-2018 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.
@@ -33,10 +33,10 @@ import org.springframework.core.ParameterNameDiscoverer;
 import org.springframework.core.annotation.AnnotationUtils;
 import org.springframework.lang.Nullable;
 import org.springframework.util.Assert;
+import org.springframework.validation.AbstractBindingResult;
 import org.springframework.validation.BindException;
 import org.springframework.validation.BindingResult;
 import org.springframework.validation.Errors;
-import org.springframework.validation.FieldError;
 import org.springframework.validation.annotation.Validated;
 import org.springframework.web.bind.WebDataBinder;
 import org.springframework.web.bind.annotation.ModelAttribute;
@@ -281,16 +281,23 @@ public class ModelAttributeMethodProcessor implements HandlerMethodArgumentResol
 				}
 			}
 			catch (TypeMismatchException ex) {
+				ex.initPropertyName(paramName);
+				binder.getBindingErrorProcessor().processPropertyAccessException(ex, binder.getBindingResult());
 				bindingFailure = true;
-				binder.getBindingResult().addError(new FieldError(
-						binder.getObjectName(), paramNames[i], ex.getValue(), true,
-						new String[] {ex.getErrorCode()}, null, ex.getLocalizedMessage()));
+				args[i] = value;
 			}
 		}
 
 		if (bindingFailure) {
+			if (binder.getBindingResult() instanceof AbstractBindingResult) {
+				AbstractBindingResult result = (AbstractBindingResult) binder.getBindingResult();
+				for (int i = 0; i < paramNames.length; i++) {
+					result.recordFieldValue(paramNames[i], paramTypes[i], args[i]);
+				}
+			}
 			throw new BindException(binder.getBindingResult());
 		}
+
 		return BeanUtils.instantiateClass(ctor, args);
 	}
 

spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ServletAnnotationControllerHandlerMethodTests.java

@@ -144,7 +144,9 @@ import org.springframework.web.servlet.View;
 import org.springframework.web.servlet.ViewResolver;
 import org.springframework.web.servlet.mvc.annotation.ModelAndViewResolver;
 import org.springframework.web.servlet.mvc.support.RedirectAttributes;
+import org.springframework.web.servlet.support.RequestContext;
 import org.springframework.web.servlet.support.RequestContextUtils;
+import org.springframework.web.servlet.view.AbstractView;
 import org.springframework.web.servlet.view.InternalResourceViewResolver;
 
 import static org.junit.Assert.*;
@@ -1837,7 +1839,7 @@ public class ServletAnnotationControllerHandlerMethodTests extends AbstractServl
 		request.addParameter("param1", "value1");
 		MockHttpServletResponse response = new MockHttpServletResponse();
 		getServlet().service(request, response);
-		assertTrue(response.getContentAsString().contains("field 'param2'"));
+		assertEquals("value1-null-null", response.getContentAsString());
 	}
 
 	@Test
@@ -1845,10 +1847,11 @@ public class ServletAnnotationControllerHandlerMethodTests extends AbstractServl
 		initServletWithControllers(ValidatedDataClassController.class);
 
 		MockHttpServletRequest request = new MockHttpServletRequest("GET", "/bind");
+		request.addParameter("param1", "value1");
 		request.addParameter("param2", "x");
 		MockHttpServletResponse response = new MockHttpServletResponse();
 		getServlet().service(request, response);
-		assertTrue(response.getContentAsString().contains("field 'param2'"));
+		assertEquals("value1-x-null", response.getContentAsString());
 	}
 
 	@Test
@@ -1860,7 +1863,7 @@ public class ServletAnnotationControllerHandlerMethodTests extends AbstractServl
 		request.addParameter("param3", "3");
 		MockHttpServletResponse response = new MockHttpServletResponse();
 		getServlet().service(request, response);
-		assertTrue(response.getContentAsString().contains("field 'param1'"));
+		assertEquals("null-true-3", response.getContentAsString());
 	}
 
 	@Test
@@ -1881,10 +1884,11 @@ public class ServletAnnotationControllerHandlerMethodTests extends AbstractServl
 		initServletWithControllers(OptionalDataClassController.class);
 
 		MockHttpServletRequest request = new MockHttpServletRequest("GET", "/bind");
+		request.addParameter("param1", "value1");
 		request.addParameter("param2", "x");
 		MockHttpServletResponse response = new MockHttpServletResponse();
 		getServlet().service(request, response);
-		assertTrue(response.getContentAsString().contains("field 'param2'"));
+		assertEquals("value1-x-null", response.getContentAsString());
 	}
 
 	@Test
@@ -3626,18 +3630,20 @@ public class ServletAnnotationControllerHandlerMethodTests extends AbstractServl
 
 		@InitBinder
 		public void initBinder(WebDataBinder binder) {
+			binder.initDirectFieldAccess();
+			binder.setConversionService(new DefaultFormattingConversionService());
 			LocalValidatorFactoryBean vf = new LocalValidatorFactoryBean();
 			vf.afterPropertiesSet();
 			binder.setValidator(vf);
-			binder.setConversionService(new DefaultFormattingConversionService());
 		}
 
 		@RequestMapping("/bind")
-		public String handle(@Valid DataClass data, BindingResult result) {
+		public BindStatusView handle(@Valid DataClass data, BindingResult result) {
 			if (result.hasErrors()) {
-				return result.toString();
+				return new BindStatusView(result.getFieldValue("param1") + "-" +
+						result.getFieldValue("param2") + "-" + result.getFieldValue("param3"));
 			}
-			return data.param1 + "-" + data.param2 + "-" + data.param3;
+			return new BindStatusView(data.param1 + "-" + data.param2 + "-" + data.param3);
 		}
 	}
 
@@ -3649,10 +3655,31 @@ public class ServletAnnotationControllerHandlerMethodTests extends AbstractServl
 			if (result.hasErrors()) {
 				assertNotNull(optionalData);
 				assertFalse(optionalData.isPresent());
-				return result.toString();
+				return result.getFieldValue("param1") + "-" + result.getFieldValue("param2") + "-" +
+						result.getFieldValue("param3");
 			}
 			return optionalData.map(data -> data.param1 + "-" + data.param2 + "-" + data.param3).orElse("");
 		}
 	}
 
+	public static class BindStatusView extends AbstractView {
+
+		private final String content;
+
+		public BindStatusView(String content) {
+			this.content = content;
+		}
+
+		@Override
+		protected void renderMergedOutputModel(
+				Map<String, Object> model, HttpServletRequest request, HttpServletResponse response) throws Exception {
+			RequestContext rc = new RequestContext(request, model);
+			rc.getBindStatus("dataClass");
+			rc.getBindStatus("dataClass.param1");
+			rc.getBindStatus("dataClass.param2");
+			rc.getBindStatus("dataClass.param3");
+			response.getWriter().write(this.content);
+		}
+	}
+
 }