From 955665b419ec237029abfa0824ca8a47d410a423 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Tue, 17 Jul 2018 17:01:34 +0200 Subject: [PATCH] Consistent processing of binding/validation failures for data classes Includes an extension of SmartValidator for candidate value validation, as well as nullability refinements in Validator and BindingResult. Issue: SPR-16840 Issue: SPR-16841 Issue: SPR-16854 --- .../validation/AbstractBindingResult.java | 7 +- .../validation/BindException.java | 2 +- .../validation/BindingResult.java | 2 +- .../validation/DataBinder.java | 15 +- .../validation/SmartValidator.java | 28 ++- .../validation/ValidationUtils.java | 40 ++-- .../springframework/validation/Validator.java | 11 +- .../SpringValidatorAdapter.java | 45 +++-- .../support/WebExchangeBindException.java | 2 +- .../ModelAttributeMethodProcessor.java | 177 ++++++++++++++++-- .../support/InvocableHandlerMethod.java | 6 +- ...nnotationControllerHandlerMethodTests.java | 105 ++++++++--- 12 files changed, 341 insertions(+), 99 deletions(-) diff --git a/spring-context/src/main/java/org/springframework/validation/AbstractBindingResult.java b/spring-context/src/main/java/org/springframework/validation/AbstractBindingResult.java index 82a92aba86..ece1663b27 100644 --- a/spring-context/src/main/java/org/springframework/validation/AbstractBindingResult.java +++ b/spring-context/src/main/java/org/springframework/validation/AbstractBindingResult.java @@ -222,7 +222,7 @@ public abstract class AbstractBindingResult extends AbstractErrors implements Bi if (fieldError != null) { Object value = fieldError.getRejectedValue(); // Do not apply formatting on binding failures like type mismatches. - return (fieldError.isBindingFailure() ? value : formatFieldValue(field, value)); + return (fieldError.isBindingFailure() || getTarget() == null ? value : formatFieldValue(field, value)); } else if (getTarget() != null) { Object value = getActualFieldValue(fixedField(field)); @@ -321,9 +321,8 @@ public abstract class AbstractBindingResult extends AbstractErrors implements Bi @Override public String[] resolveMessageCodes(String errorCode, @Nullable String field) { - Class fieldType = (getTarget() != null ? getFieldType(field) : null); return getMessageCodesResolver().resolveMessageCodes( - errorCode, getObjectName(), fixedField(field), fieldType); + errorCode, getObjectName(), fixedField(field), getFieldType(field)); } @Override @@ -332,7 +331,7 @@ public abstract class AbstractBindingResult extends AbstractErrors implements Bi } @Override - public void recordFieldValue(String field, Class type, Object value) { + public void recordFieldValue(String field, Class type, @Nullable Object value) { this.fieldTypes.put(field, type); this.fieldValues.put(field, value); } diff --git a/spring-context/src/main/java/org/springframework/validation/BindException.java b/spring-context/src/main/java/org/springframework/validation/BindException.java index d31ee8e552..f9111ecd22 100644 --- a/spring-context/src/main/java/org/springframework/validation/BindException.java +++ b/spring-context/src/main/java/org/springframework/validation/BindException.java @@ -275,7 +275,7 @@ public class BindException extends Exception implements BindingResult { } @Override - public void recordFieldValue(String field, Class type, Object value) { + public void recordFieldValue(String field, Class type, @Nullable Object value) { this.bindingResult.recordFieldValue(field, type, value); } diff --git a/spring-context/src/main/java/org/springframework/validation/BindingResult.java b/spring-context/src/main/java/org/springframework/validation/BindingResult.java index ebe5594eee..6227396994 100644 --- a/spring-context/src/main/java/org/springframework/validation/BindingResult.java +++ b/spring-context/src/main/java/org/springframework/validation/BindingResult.java @@ -143,7 +143,7 @@ public interface BindingResult extends Errors { * @param value the original value * @since 5.0.4 */ - default void recordFieldValue(String field, Class type, Object value) { + default void recordFieldValue(String field, Class type, @Nullable Object value) { } /** diff --git a/spring-context/src/main/java/org/springframework/validation/DataBinder.java b/spring-context/src/main/java/org/springframework/validation/DataBinder.java index 4f01b64d7d..ab62c932ba 100644 --- a/spring-context/src/main/java/org/springframework/validation/DataBinder.java +++ b/spring-context/src/main/java/org/springframework/validation/DataBinder.java @@ -853,8 +853,12 @@ public class DataBinder implements PropertyEditorRegistry, TypeConverter { * @see #getBindingResult() */ public void validate() { + Object target = getTarget(); + Assert.state(target != null, "No target to validate"); + BindingResult bindingResult = getBindingResult(); + // Call each validator with the same binding result for (Validator validator : this.validators) { - validator.validate(getTarget(), getBindingResult()); + validator.validate(target, bindingResult); } } @@ -862,16 +866,21 @@ public class DataBinder implements PropertyEditorRegistry, TypeConverter { * Invoke the specified Validators, if any, with the given validation hints. *

Note: Validation hints may get ignored by the actual target Validator. * @param validationHints one or more hint objects to be passed to a {@link SmartValidator} + * @since 3.1 * @see #setValidator(Validator) * @see SmartValidator#validate(Object, Errors, Object...) */ public void validate(Object... validationHints) { + Object target = getTarget(); + Assert.state(target != null, "No target to validate"); + BindingResult bindingResult = getBindingResult(); + // Call each validator with the same binding result for (Validator validator : getValidators()) { if (!ObjectUtils.isEmpty(validationHints) && validator instanceof SmartValidator) { - ((SmartValidator) validator).validate(getTarget(), getBindingResult(), validationHints); + ((SmartValidator) validator).validate(target, bindingResult, validationHints); } else if (validator != null) { - validator.validate(getTarget(), getBindingResult()); + validator.validate(target, bindingResult); } } } diff --git a/spring-context/src/main/java/org/springframework/validation/SmartValidator.java b/spring-context/src/main/java/org/springframework/validation/SmartValidator.java index 16f7c7580e..d59026319f 100644 --- a/spring-context/src/main/java/org/springframework/validation/SmartValidator.java +++ b/spring-context/src/main/java/org/springframework/validation/SmartValidator.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2012 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. @@ -39,11 +39,29 @@ public interface SmartValidator extends Validator { *

Note: Validation hints may get ignored by the actual target {@code Validator}, * in which case this method should behave just like its regular * {@link #validate(Object, Errors)} sibling. - * @param target the object that is to be validated (can be {@code null}) - * @param errors contextual state about the validation process (never {@code null}) + * @param target the object that is to be validated + * @param errors contextual state about the validation process * @param validationHints one or more hint objects to be passed to the validation engine - * @see ValidationUtils + * @see javax.validation.Validator#validate(Object, Class[]) */ - void validate(@Nullable Object target, Errors errors, Object... validationHints); + void validate(Object target, Errors errors, Object... validationHints); + + /** + * Validate the supplied value for the specified field on the target type, + * reporting the same validation errors as if the value would be bound to + * the field on an instance of the target class. + * @param targetType the target type + * @param fieldName the name of the field + * @param value the candidate value + * @param errors contextual state about the validation process + * @param validationHints one or more hint objects to be passed to the validation engine + * @since 5.1 + * @see javax.validation.Validator#validateValue(Class, String, Object, Class[]) + */ + default void validateValue( + Class targetType, String fieldName, @Nullable Object value, Errors errors, Object... validationHints) { + + throw new IllegalArgumentException("Cannot validate individual value for " + targetType); + } } diff --git a/spring-context/src/main/java/org/springframework/validation/ValidationUtils.java b/spring-context/src/main/java/org/springframework/validation/ValidationUtils.java index 3ab00680c0..716bb1acdd 100644 --- a/spring-context/src/main/java/org/springframework/validation/ValidationUtils.java +++ b/spring-context/src/main/java/org/springframework/validation/ValidationUtils.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. @@ -45,30 +45,30 @@ public abstract class ValidationUtils { /** * Invoke the given {@link Validator} for the supplied object and * {@link Errors} instance. - * @param validator the {@code Validator} to be invoked (must not be {@code null}) - * @param obj the object to bind the parameters to - * @param errors the {@link Errors} instance that should store the errors (must not be {@code null}) - * @throws IllegalArgumentException if either of the {@code Validator} or {@code Errors} arguments is - * {@code null}, or if the supplied {@code Validator} does not {@link Validator#supports(Class) support} - * the validation of the supplied object's type + * @param validator the {@code Validator} to be invoked + * @param target the object to bind the parameters to + * @param errors the {@link Errors} instance that should store the errors + * @throws IllegalArgumentException if either of the {@code Validator} or {@code Errors} + * arguments is {@code null}, or if the supplied {@code Validator} does not + * {@link Validator#supports(Class) support} the validation of the supplied object's type */ - public static void invokeValidator(Validator validator, Object obj, Errors errors) { - invokeValidator(validator, obj, errors, (Object[]) null); + public static void invokeValidator(Validator validator, Object target, Errors errors) { + invokeValidator(validator, target, errors, (Object[]) null); } /** * Invoke the given {@link Validator}/{@link SmartValidator} for the supplied object and * {@link Errors} instance. - * @param validator the {@code Validator} to be invoked (must not be {@code null}) - * @param obj the object to bind the parameters to - * @param errors the {@link Errors} instance that should store the errors (must not be {@code null}) + * @param validator the {@code Validator} to be invoked + * @param target the object to bind the parameters to + * @param errors the {@link Errors} instance that should store the errors * @param validationHints one or more hint objects to be passed to the validation engine - * @throws IllegalArgumentException if either of the {@code Validator} or {@code Errors} arguments is - * {@code null}, or if the supplied {@code Validator} does not {@link Validator#supports(Class) support} - * the validation of the supplied object's type + * @throws IllegalArgumentException if either of the {@code Validator} or {@code Errors} + * arguments is {@code null}, or if the supplied {@code Validator} does not + * {@link Validator#supports(Class) support} the validation of the supplied object's type */ public static void invokeValidator( - Validator validator, @Nullable Object obj, Errors errors, @Nullable Object... validationHints) { + Validator validator, Object target, Errors errors, @Nullable Object... validationHints) { Assert.notNull(validator, "Validator must not be null"); Assert.notNull(errors, "Errors object must not be null"); @@ -76,16 +76,16 @@ public abstract class ValidationUtils { if (logger.isDebugEnabled()) { logger.debug("Invoking validator [" + validator + "]"); } - if (obj != null && !validator.supports(obj.getClass())) { + if (!validator.supports(target.getClass())) { throw new IllegalArgumentException( - "Validator [" + validator.getClass() + "] does not support [" + obj.getClass() + "]"); + "Validator [" + validator.getClass() + "] does not support [" + target.getClass() + "]"); } if (!ObjectUtils.isEmpty(validationHints) && validator instanceof SmartValidator) { - ((SmartValidator) validator).validate(obj, errors, validationHints); + ((SmartValidator) validator).validate(target, errors, validationHints); } else { - validator.validate(obj, errors); + validator.validate(target, errors); } if (logger.isDebugEnabled()) { diff --git a/spring-context/src/main/java/org/springframework/validation/Validator.java b/spring-context/src/main/java/org/springframework/validation/Validator.java index d5511c602b..d3379ddc03 100644 --- a/spring-context/src/main/java/org/springframework/validation/Validator.java +++ b/spring-context/src/main/java/org/springframework/validation/Validator.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2012 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. @@ -16,8 +16,6 @@ package org.springframework.validation; -import org.springframework.lang.Nullable; - /** * A validator for application-specific objects. * @@ -61,6 +59,7 @@ import org.springframework.lang.Nullable; * application. * * @author Rod Johnson + * @see SmartValidator * @see Errors * @see ValidationUtils */ @@ -87,10 +86,10 @@ public interface Validator { * typically has (or would) return {@code true}. *

The supplied {@link Errors errors} instance can be used to report * any resulting validation errors. - * @param target the object that is to be validated (can be {@code null}) - * @param errors contextual state about the validation process (never {@code null}) + * @param target the object that is to be validated + * @param errors contextual state about the validation process * @see ValidationUtils */ - void validate(@Nullable Object target, Errors errors); + void validate(Object target, Errors errors); } diff --git a/spring-context/src/main/java/org/springframework/validation/beanvalidation/SpringValidatorAdapter.java b/spring-context/src/main/java/org/springframework/validation/beanvalidation/SpringValidatorAdapter.java index 3f6463bc8a..b06c276684 100644 --- a/spring-context/src/main/java/org/springframework/validation/beanvalidation/SpringValidatorAdapter.java +++ b/spring-context/src/main/java/org/springframework/validation/beanvalidation/SpringValidatorAdapter.java @@ -50,13 +50,17 @@ import org.springframework.validation.SmartValidator; * while also exposing the original JSR-303 Validator interface itself. * *

Can be used as a programmatic wrapper. Also serves as base class for - * {@link CustomValidatorBean} and {@link LocalValidatorFactoryBean}. + * {@link CustomValidatorBean} and {@link LocalValidatorFactoryBean}, + * and as the primary implementation of the {@link SmartValidator} interface. * *

As of Spring Framework 5.0, this adapter is fully compatible with * Bean Validation 1.1 as well as 2.0. * * @author Juergen Hoeller * @since 3.0 + * @see SmartValidator + * @see CustomValidatorBean + * @see LocalValidatorFactoryBean */ public class SpringValidatorAdapter implements SmartValidator, javax.validation.Validator { @@ -99,28 +103,45 @@ public class SpringValidatorAdapter implements SmartValidator, javax.validation. } @Override - public void validate(@Nullable Object target, Errors errors) { + public void validate(Object target, Errors errors) { if (this.targetValidator != null) { processConstraintViolations(this.targetValidator.validate(target), errors); } } @Override - public void validate(@Nullable Object target, Errors errors, @Nullable Object... validationHints) { + public void validate(Object target, Errors errors, Object... validationHints) { if (this.targetValidator != null) { - Set> groups = new LinkedHashSet<>(); - if (validationHints != null) { - for (Object hint : validationHints) { - if (hint instanceof Class) { - groups.add((Class) hint); - } - } - } processConstraintViolations( - this.targetValidator.validate(target, ClassUtils.toClassArray(groups)), errors); + this.targetValidator.validate(target, asValidationGroups(validationHints)), errors); } } + @SuppressWarnings("unchecked") + @Override + public void validateValue( + Class targetType, String fieldName, @Nullable Object value, Errors errors, Object... validationHints) { + + if (this.targetValidator != null) { + processConstraintViolations(this.targetValidator.validateValue( + (Class) targetType, fieldName, value, asValidationGroups(validationHints)), errors); + } + } + + /** + * Turn the specified validation hints into JSR-303 validation groups. + * @since 5.1 + */ + private Class[] asValidationGroups(Object... validationHints) { + Set> groups = new LinkedHashSet<>(4); + for (Object hint : validationHints) { + if (hint instanceof Class) { + groups.add((Class) hint); + } + } + return ClassUtils.toClassArray(groups); + } + /** * Process the given JSR-303 ConstraintViolations, adding corresponding errors to * the provided Spring {@link Errors} object. diff --git a/spring-web/src/main/java/org/springframework/web/bind/support/WebExchangeBindException.java b/spring-web/src/main/java/org/springframework/web/bind/support/WebExchangeBindException.java index 288d0daf3d..dd6dcb6e8e 100644 --- a/spring-web/src/main/java/org/springframework/web/bind/support/WebExchangeBindException.java +++ b/spring-web/src/main/java/org/springframework/web/bind/support/WebExchangeBindException.java @@ -261,7 +261,7 @@ public class WebExchangeBindException extends ServerWebInputException implements } @Override - public void recordFieldValue(String field, Class type, Object value) { + public void recordFieldValue(String field, Class type, @Nullable Object value) { this.bindingResult.recordFieldValue(field, type, value); } diff --git a/spring-web/src/main/java/org/springframework/web/method/annotation/ModelAttributeMethodProcessor.java b/spring-web/src/main/java/org/springframework/web/method/annotation/ModelAttributeMethodProcessor.java index 67c4d42bdc..e74b7d4fcf 100644 --- a/spring-web/src/main/java/org/springframework/web/method/annotation/ModelAttributeMethodProcessor.java +++ b/spring-web/src/main/java/org/springframework/web/method/annotation/ModelAttributeMethodProcessor.java @@ -19,8 +19,14 @@ package org.springframework.web.method.annotation; import java.beans.ConstructorProperties; import java.lang.annotation.Annotation; import java.lang.reflect.Constructor; +import java.lang.reflect.Field; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.HashSet; +import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.Set; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -33,10 +39,11 @@ 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.SmartValidator; +import org.springframework.validation.Validator; import org.springframework.validation.annotation.Validated; import org.springframework.web.bind.WebDataBinder; import org.springframework.web.bind.annotation.ModelAttribute; @@ -189,7 +196,7 @@ public class ModelAttributeMethodProcessor implements HandlerMethodArgumentResol * @return the created model attribute (never {@code null}) * @throws BindException in case of constructor argument binding failure * @throws Exception in case of constructor invocation failure - * @see #constructAttribute(Constructor, String, WebDataBinderFactory, NativeWebRequest) + * @see #constructAttribute(Constructor, String, MethodParameter, WebDataBinderFactory, NativeWebRequest) * @see BeanUtils#findPrimaryConstructor(Class) */ protected Object createAttribute(String attributeName, MethodParameter parameter, @@ -214,7 +221,7 @@ public class ModelAttributeMethodProcessor implements HandlerMethodArgumentResol } } - Object attribute = constructAttribute(ctor, attributeName, binderFactory, webRequest); + Object attribute = constructAttribute(ctor, attributeName, parameter, binderFactory, webRequest); if (parameter != nestedParameter) { attribute = Optional.of(attribute); } @@ -233,11 +240,17 @@ public class ModelAttributeMethodProcessor implements HandlerMethodArgumentResol * @return the created model attribute (never {@code null}) * @throws BindException in case of constructor argument binding failure * @throws Exception in case of constructor invocation failure - * @since 5.0 + * @since 5.1 */ - protected Object constructAttribute(Constructor ctor, String attributeName, + @SuppressWarnings("deprecation") + protected Object constructAttribute(Constructor ctor, String attributeName, MethodParameter parameter, WebDataBinderFactory binderFactory, NativeWebRequest webRequest) throws Exception { + Object constructed = constructAttribute(ctor, attributeName, binderFactory, webRequest); + if (constructed != null) { + return constructed; + } + if (ctor.getParameterCount() == 0) { // A single default constructor -> clearly a standard JavaBeans arrangement. return BeanUtils.instantiateClass(ctor); @@ -256,6 +269,7 @@ public class ModelAttributeMethodProcessor implements HandlerMethodArgumentResol String fieldDefaultPrefix = binder.getFieldDefaultPrefix(); String fieldMarkerPrefix = binder.getFieldMarkerPrefix(); boolean bindingFailure = false; + Set failedParams = new HashSet<>(4); for (int i = 0; i < paramNames.length; i++) { String paramName = paramNames[i]; @@ -272,7 +286,7 @@ public class ModelAttributeMethodProcessor implements HandlerMethodArgumentResol } } try { - MethodParameter methodParam = new MethodParameter(ctor, i); + MethodParameter methodParam = new FieldAwareConstructorParameter(ctor, i, paramName); if (value == null && methodParam.isOptional()) { args[i] = (methodParam.getParameterType() == Optional.class ? Optional.empty() : null); } @@ -282,25 +296,43 @@ public class ModelAttributeMethodProcessor implements HandlerMethodArgumentResol } catch (TypeMismatchException ex) { ex.initPropertyName(paramName); + args[i] = value; + failedParams.add(paramName); + binder.getBindingResult().recordFieldValue(paramName, paramType, value); binder.getBindingErrorProcessor().processPropertyAccessException(ex, binder.getBindingResult()); bindingFailure = true; - 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]); + BindingResult result = binder.getBindingResult(); + for (int i = 0; i < paramNames.length; i++) { + String paramName = paramNames[i]; + if (!failedParams.contains(paramName)) { + result.recordFieldValue(paramName, paramTypes[i], args[i]); + validateValueIfApplicable(binder, parameter, ctor.getDeclaringClass(), paramName, args[i]); } } - throw new BindException(binder.getBindingResult()); + throw new BindException(result); } return BeanUtils.instantiateClass(ctor, args); } + /** + * Construct a new attribute instance with the given constructor. + * @since 5.0 + * @deprecated as of 5.1, in favor of + * {@link #constructAttribute(Constructor, String, MethodParameter, WebDataBinderFactory, NativeWebRequest)} + */ + @Deprecated + @Nullable + protected Object constructAttribute(Constructor ctor, String attributeName, + WebDataBinderFactory binderFactory, NativeWebRequest webRequest) throws Exception { + + return null; + } + /** * Extension point to bind the request to the target object. * @param binder the data binder instance to use for the binding @@ -317,20 +349,72 @@ public class ModelAttributeMethodProcessor implements HandlerMethodArgumentResol * and custom annotations whose name starts with "Valid". * @param binder the DataBinder to be used * @param parameter the method parameter declaration + * @see WebDataBinder#validate(Object...) + * @see SmartValidator#validate(Object, Errors, Object...) */ protected void validateIfApplicable(WebDataBinder binder, MethodParameter parameter) { - Annotation[] annotations = parameter.getParameterAnnotations(); - for (Annotation ann : annotations) { - Validated validatedAnn = AnnotationUtils.getAnnotation(ann, Validated.class); - if (validatedAnn != null || ann.annotationType().getSimpleName().startsWith("Valid")) { - Object hints = (validatedAnn != null ? validatedAnn.value() : AnnotationUtils.getValue(ann)); - Object[] validationHints = (hints instanceof Object[] ? (Object[]) hints : new Object[] {hints}); + for (Annotation ann : parameter.getParameterAnnotations()) { + Object[] validationHints = determineValidationHints(ann); + if (validationHints != null) { binder.validate(validationHints); break; } } } + /** + * Validate the specified candidate value if applicable. + *

The default implementation checks for {@code @javax.validation.Valid}, + * Spring's {@link org.springframework.validation.annotation.Validated}, + * and custom annotations whose name starts with "Valid". + * @param binder the DataBinder to be used + * @param parameter the method parameter declaration + * @param targetType the target type + * @param fieldName the name of the field + * @param value the candidate value + * @since 5.1 + * @see #validateIfApplicable(WebDataBinder, MethodParameter) + * @see SmartValidator#validateValue(Class, String, Object, Errors, Object...) + */ + protected void validateValueIfApplicable(WebDataBinder binder, MethodParameter parameter, + Class targetType, String fieldName, @Nullable Object value) { + + for (Annotation ann : parameter.getParameterAnnotations()) { + Object[] validationHints = determineValidationHints(ann); + if (validationHints != null) { + for (Validator validator : binder.getValidators()) { + if (validator instanceof SmartValidator) { + try { + ((SmartValidator) validator).validateValue(targetType, fieldName, value, + binder.getBindingResult(), validationHints); + } + catch (IllegalArgumentException ex) { + // No corresponding field on the target class... + } + } + } + break; + } + } + } + + /** + * Determine any validation triggered by the given annotation. + * @param ann the annotation (potentially a validation annotation) + * @return the validation hints to apply (possibly an empty array), + * or {@code null} if this annotation does not trigger any validation + * @since 5.1 + */ + @Nullable + private Object[] determineValidationHints(Annotation ann) { + Validated validatedAnn = AnnotationUtils.getAnnotation(ann, Validated.class); + if (validatedAnn != null || ann.annotationType().getSimpleName().startsWith("Valid")) { + Object hints = (validatedAnn != null ? validatedAnn.value() : AnnotationUtils.getValue(ann)); + return (hints instanceof Object[] ? (Object[]) hints : new Object[]{hints}); + } + return null; + } + /** * Whether to raise a fatal bind exception on validation errors. *

The default implementation delegates to {@link #isBindExceptionRequired(MethodParameter)}. @@ -380,4 +464,61 @@ public class ModelAttributeMethodProcessor implements HandlerMethodArgumentResol } } + + /** + * {@link MethodParameter} subclass which detects field annotations as well. + * @since 5.1 + */ + private static class FieldAwareConstructorParameter extends MethodParameter { + + private final String parameterName; + + @Nullable + private volatile Annotation[] combinedAnnotations; + + public FieldAwareConstructorParameter(Constructor constructor, int parameterIndex, String parameterName) { + super(constructor, parameterIndex); + this.parameterName = parameterName; + } + + @Override + public Annotation[] getParameterAnnotations() { + Annotation[] anns = this.combinedAnnotations; + if (anns == null) { + anns = super.getParameterAnnotations(); + try { + Field field = getDeclaringClass().getDeclaredField(this.parameterName); + Annotation[] fieldAnns = field.getAnnotations(); + if (fieldAnns.length > 0) { + List merged = new ArrayList<>(anns.length + fieldAnns.length); + merged.addAll(Arrays.asList(anns)); + for (Annotation fieldAnn : fieldAnns) { + boolean existingType = false; + for (Annotation ann : anns) { + if (ann.annotationType() == fieldAnn.annotationType()) { + existingType = true; + break; + } + } + if (!existingType) { + merged.add(fieldAnn); + } + } + anns = merged.toArray(new Annotation[0]); + } + } + catch (NoSuchFieldException | SecurityException ex) { + // ignore + } + this.combinedAnnotations = anns; + } + return anns; + } + + @Override + public String getParameterName() { + return this.parameterName; + } + } + } diff --git a/spring-web/src/main/java/org/springframework/web/method/support/InvocableHandlerMethod.java b/spring-web/src/main/java/org/springframework/web/method/support/InvocableHandlerMethod.java index 4a6e9bf7d2..153024cb74 100644 --- a/spring-web/src/main/java/org/springframework/web/method/support/InvocableHandlerMethod.java +++ b/spring-web/src/main/java/org/springframework/web/method/support/InvocableHandlerMethod.java @@ -160,9 +160,9 @@ public class InvocableHandlerMethod extends HandlerMethod { } catch (Exception ex) { // Leave stack trace for later, e.g. AbstractHandlerExceptionResolver - String message = ex.getMessage(); - if (!message.contains(parameter.getExecutable().toGenericString())) { - if (logger.isDebugEnabled()) { + if (logger.isDebugEnabled()) { + String message = ex.getMessage(); + if (message != null && !message.contains(parameter.getExecutable().toGenericString())) { logger.debug(formatArgumentError(parameter, message)); } } diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ServletAnnotationControllerHandlerMethodTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ServletAnnotationControllerHandlerMethodTests.java index 8317b8b2bb..4cf8a5307d 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ServletAnnotationControllerHandlerMethodTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ServletAnnotationControllerHandlerMethodTests.java @@ -32,6 +32,7 @@ import java.nio.charset.StandardCharsets; import java.security.Principal; import java.text.SimpleDateFormat; import java.time.Instant; +import java.time.LocalDate; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; @@ -72,6 +73,7 @@ import org.springframework.beans.propertyeditors.CustomDateEditor; import org.springframework.context.annotation.AnnotationConfigUtils; import org.springframework.core.MethodParameter; import org.springframework.core.convert.converter.Converter; +import org.springframework.format.annotation.DateTimeFormat; import org.springframework.format.support.DefaultFormattingConversionService; import org.springframework.format.support.FormattingConversionServiceFactoryBean; import org.springframework.http.HttpEntity; @@ -1863,7 +1865,7 @@ public class ServletAnnotationControllerHandlerMethodTests extends AbstractServl request.addParameter("param1", "value1"); MockHttpServletResponse response = new MockHttpServletResponse(); getServlet().service(request, response); - assertEquals("value1-null-null", response.getContentAsString()); + assertEquals("1:value1-null-null", response.getContentAsString()); } @Test @@ -1875,7 +1877,7 @@ public class ServletAnnotationControllerHandlerMethodTests extends AbstractServl request.addParameter("param2", "x"); MockHttpServletResponse response = new MockHttpServletResponse(); getServlet().service(request, response); - assertEquals("value1-x-null", response.getContentAsString()); + assertEquals("1:value1-x-null", response.getContentAsString()); } @Test @@ -1884,10 +1886,21 @@ public class ServletAnnotationControllerHandlerMethodTests extends AbstractServl MockHttpServletRequest request = new MockHttpServletRequest("GET", "/bind"); request.addParameter("param2", "true"); - request.addParameter("param3", "3"); + request.addParameter("param3", "0"); MockHttpServletResponse response = new MockHttpServletResponse(); getServlet().service(request, response); - assertEquals("null-true-3", response.getContentAsString()); + assertEquals("1:null-true-0", response.getContentAsString()); + } + + @Test + public void dataClassBindingWithValidationErrorAndConversionError() throws Exception { + initServletWithControllers(ValidatedDataClassController.class); + + MockHttpServletRequest request = new MockHttpServletRequest("GET", "/bind"); + request.addParameter("param2", "x"); + MockHttpServletResponse response = new MockHttpServletResponse(); + getServlet().service(request, response); + assertEquals("2:null-x-null", response.getContentAsString()); } @Test @@ -1965,6 +1978,17 @@ public class ServletAnnotationControllerHandlerMethodTests extends AbstractServl assertEquals("value1-false-0", response.getContentAsString()); } + @Test + public void dataClassBindingWithLocalDate() throws Exception { + initServletWithControllers(DateClassController.class); + + MockHttpServletRequest request = new MockHttpServletRequest("GET", "/bind"); + request.addParameter("date", "2010-01-01"); + MockHttpServletResponse response = new MockHttpServletResponse(); + getServlet().service(request, response); + assertEquals("2010-01-01", response.getContentAsString()); + } + @Controller static class ControllerWithEmptyValueMapping { @@ -2915,14 +2939,12 @@ public class ServletAnnotationControllerHandlerMethodTests extends AbstractServl } @Override - public Object read(Class clazz, HttpInputMessage inputMessage) - throws IOException, HttpMessageNotReadableException { - throw new HttpMessageNotReadableException("Could not read"); + public Object read(Class clazz, HttpInputMessage inputMessage) { + throw new HttpMessageNotReadableException("Could not read", inputMessage); } @Override - public void write(Object o, @Nullable MediaType contentType, HttpOutputMessage outputMessage) - throws IOException, HttpMessageNotWritableException { + public void write(Object o, @Nullable MediaType contentType, HttpOutputMessage outputMessage) { throw new UnsupportedOperationException("Not implemented"); } } @@ -3672,28 +3694,13 @@ public class ServletAnnotationControllerHandlerMethodTests extends AbstractServl @RequestMapping("/bind") public BindStatusView handle(@Valid DataClass data, BindingResult result) { if (result.hasErrors()) { - return new BindStatusView(result.getFieldValue("param1") + "-" + + return new BindStatusView(result.getErrorCount() + ":" + result.getFieldValue("param1") + "-" + result.getFieldValue("param2") + "-" + result.getFieldValue("param3")); } return new BindStatusView(data.param1 + "-" + data.param2 + "-" + data.param3); } } - @RestController - public static class OptionalDataClassController { - - @RequestMapping("/bind") - public String handle(Optional optionalData, BindingResult result) { - if (result.hasErrors()) { - assertNotNull(optionalData); - assertFalse(optionalData.isPresent()); - 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; @@ -3714,4 +3721,52 @@ public class ServletAnnotationControllerHandlerMethodTests extends AbstractServl } } + @RestController + public static class OptionalDataClassController { + + @RequestMapping("/bind") + public String handle(Optional optionalData, BindingResult result) { + if (result.hasErrors()) { + assertNotNull(optionalData); + assertFalse(optionalData.isPresent()); + return result.getFieldValue("param1") + "-" + result.getFieldValue("param2") + "-" + + result.getFieldValue("param3"); + } + return optionalData.map(data -> data.param1 + "-" + data.param2 + "-" + data.param3).orElse(""); + } + } + + public static class DateClass { + + @DateTimeFormat(pattern = "yyyy-MM-dd") + public LocalDate date; + + public DateClass(LocalDate date) { + this.date = date; + } + } + + @RestController + public static class DateClassController { + + @InitBinder + public void initBinder(WebDataBinder binder) { + binder.initDirectFieldAccess(); + binder.setConversionService(new DefaultFormattingConversionService()); + } + + @RequestMapping("/bind") + public String handle(DateClass data, BindingResult result) { + if (result.hasErrors()) { + return result.getFieldError().toString(); + } + assertNotNull(data); + assertNotNull(data.date); + assertEquals(2010, data.date.getYear()); + assertEquals(1, data.date.getMonthValue()); + assertEquals(1, data.date.getDayOfMonth()); + return result.getFieldValue("date").toString(); + } + } + }