Refactoring in MethodValidationResult

Remove throwIfViolationsPresent and replace with static factory
methods on MethodValidationException taking MethodValidationResult,
which makes handling more explicit and allows choice of what
exception to raise.

Update MethodValidationResult to expose the target, the method, and
forReturnValue flag, so the code handling an exception will have
access to all details.

See gh-30644

Author: rstoyanchev

spring-context/src/main/java/org/springframework/validation/beanvalidation/DefaultMethodValidator.java

@@ -46,47 +46,53 @@ public Class<?>[] determineValidationGroups(Object bean, Method method) {
 
 	@Override
 	public void validateArguments(
-			Object target, Method method, @Nullable MethodParameter[] parameters, Object[] arguments,
-			Class<?>[] groups) {
+			Object target, Method method, @Nullable MethodParameter[] parameters,
+			Object[] arguments, Class<?>[] groups) {
 
-		handleArgumentsValidationResult(target, method, arguments, groups,
-				this.adapter.validateMethodArguments(target, method, parameters, arguments, groups));
-	}
-
-	public void validateReturnValue(
-			Object target, Method method, @Nullable MethodParameter returnType, @Nullable Object returnValue,
-			Class<?>[] groups) {
+		MethodValidationResult validationResult =
+				this.adapter.validateMethodArguments(target, method, parameters, arguments, groups);
 
-		handleReturnValueValidationResult(target, method, returnValue, groups,
-				this.adapter.validateMethodReturnValue(target, method, returnType, returnValue, groups));
+		handleArgumentsResult(arguments, groups, validationResult);
 	}
 
 	/**
 	 * Subclasses can override this to handle the result of argument validation.
-	 * By default, {@link MethodValidationResult#throwIfViolationsPresent()} is called.
-	 * @param bean the target Object for method invocation
-	 * @param method the target method
+	 * By default, throws {@link MethodValidationException} if there are errors.
 	 * @param arguments the candidate argument values to validate
 	 * @param groups groups for validation determined via
+	 * @param validationResult the result from validation
 	 */
-	protected void handleArgumentsValidationResult(
-			Object bean, Method method, Object[] arguments, Class<?>[] groups, MethodValidationResult result) {
+	protected void handleArgumentsResult(
+			Object[] arguments, Class<?>[] groups, MethodValidationResult validationResult) {
+
+		if (validationResult.hasViolations()) {
+			throw MethodValidationException.forResult(validationResult);
+		}
+	}
+
+	public void validateReturnValue(
+			Object target, Method method, @Nullable MethodParameter returnType,
+			@Nullable Object returnValue, Class<?>[] groups) {
+
+		MethodValidationResult validationResult =
+				this.adapter.validateMethodReturnValue(target, method, returnType, returnValue, groups);
 
-		result.throwIfViolationsPresent();
+		handleReturnValueResult(returnValue, groups, validationResult);
 	}
 
 	/**
 	 * Subclasses can override this to handle the result of return value validation.
-	 * By default, {@link MethodValidationResult#throwIfViolationsPresent()} is called.
-	 * @param bean the target Object for method invocation
-	 * @param method the target method
+	 * By default, throws {@link MethodValidationException} if there are errors.
 	 * @param returnValue the return value to validate
 	 * @param groups groups for validation determined via
+	 * @param validationResult the result from validation
 	 */
-	protected void handleReturnValueValidationResult(
-			Object bean, Method method, @Nullable Object returnValue, Class<?>[] groups, MethodValidationResult result) {
+	protected void handleReturnValueResult(
+			@Nullable Object returnValue, Class<?>[] groups, MethodValidationResult validationResult) {
 
-		result.throwIfViolationsPresent();
+		if (validationResult.hasViolations()) {
+			throw MethodValidationException.forResult(validationResult);
+		}
 	}
 
 }

spring-context/src/main/java/org/springframework/validation/beanvalidation/MethodValidationAdapter.java

@@ -18,7 +18,6 @@
 
 import java.lang.reflect.Method;
 import java.util.ArrayList;
-import java.util.Collections;
 import java.util.Comparator;
 import java.util.Iterator;
 import java.util.LinkedHashMap;
@@ -74,8 +73,6 @@ public class MethodValidationAdapter {
 
 	private static final Comparator<ParameterValidationResult> RESULT_COMPARATOR = new ResultComparator();
 
-	private static final MethodValidationResult EMPTY_RESULT = new EmptyMethodValidationResult();
-
 
 	private final Supplier<Validator> validator;
 
@@ -232,7 +229,8 @@ public MethodValidationResult validateMethodArguments(
 			Method bridgedMethod = BridgeMethodResolver.findBridgedMethod(mostSpecificMethod);
 			result = execVal.validateParameters(target, bridgedMethod, arguments, groups);
 		}
-		return (result.isEmpty() ? EMPTY_RESULT :
+		return (result.isEmpty() ?
+				MethodValidationException.forEmptyResult(target, method, true) :
 				createException(target, method, result,
 						i -> parameters != null ? parameters[i] : new MethodParameter(method, i),
 						i -> arguments[i],
@@ -257,7 +255,8 @@ public MethodValidationResult validateMethodReturnValue(
 
 		ExecutableValidator execVal = this.validator.get().forExecutables();
 		Set<ConstraintViolation<Object>> result = execVal.validateReturnValue(target, method, returnValue, groups);
-		return (result.isEmpty() ? EMPTY_RESULT :
+		return (result.isEmpty() ?
+				MethodValidationException.forEmptyResult(target, method, true) :
 				createException(target, method, result,
 						i -> returnType != null ? returnType : new MethodParameter(method, -1),
 						i -> returnValue,
@@ -310,7 +309,7 @@ else if (node.getKind().equals(ElementKind.RETURN_VALUE)) {
 		cascadedViolations.forEach((node, builder) -> validatonResultList.add(builder.build()));
 		validatonResultList.sort(RESULT_COMPARATOR);
 
-		return new MethodValidationException(target, method, violations, validatonResultList, forReturnValue);
+		return new MethodValidationException(target, method, forReturnValue, violations, validatonResultList);
 	}
 
 	/**
@@ -533,41 +532,4 @@ private <E> int compareKeys(ParameterErrors errors1, ParameterErrors errors2) {
 		}
 	}
 
-
-	/**
-	 * {@link MethodValidationResult} to use when there are no violations.
-	 */
-	private static final class EmptyMethodValidationResult implements MethodValidationResult {
-
-		@Override
-		public Set<ConstraintViolation<?>> getConstraintViolations() {
-			return Collections.emptySet();
-		}
-
-		@Override
-		public List<ParameterValidationResult> getAllValidationResults() {
-			return Collections.emptyList();
-		}
-
-		@Override
-		public List<ParameterValidationResult> getValueResults() {
-			return Collections.emptyList();
-		}
-
-		@Override
-		public List<ParameterErrors> getBeanResults() {
-			return Collections.emptyList();
-		}
-
-		@Override
-		public void throwIfViolationsPresent() {
-		}
-
-		@Override
-		public String toString() {
-			return "MethodValidationResult (0 violations)";
-		}
-
-	}
-
 }

spring-context/src/main/java/org/springframework/validation/beanvalidation/MethodValidationException.java

@@ -17,6 +17,7 @@
 package org.springframework.validation.beanvalidation;
 
 import java.lang.reflect.Method;
+import java.util.Collections;
 import java.util.List;
 import java.util.Set;
 
@@ -32,7 +33,6 @@
  * {@link org.springframework.context.MessageSourceResolvable} and grouped by
  * method parameter.
  *
- *
  * @author Rossen Stoyanchev
  * @since 6.1
  * @see ParameterValidationResult
@@ -51,49 +51,55 @@ public class MethodValidationException extends ConstraintViolationException impl
 	private final boolean forReturnValue;
 
 
-	public MethodValidationException(
-			Object target, Method method, Set<? extends ConstraintViolation<?>> violations,
-			List<ParameterValidationResult> validationResults, boolean forReturnValue) {
+	/**
+	 * Package private constructor for {@link MethodValidationAdapter}.
+	 */
+	MethodValidationException(
+			Object target, Method method, boolean forReturnValue,
+			Set<? extends ConstraintViolation<?>> violations, List<ParameterValidationResult> results) {
 
 		super(violations);
-		Assert.notEmpty(violations, "'violations' must not be empty");
+
+		Assert.notNull(violations, "'violations' is required");
+		Assert.notNull(results, "'results' is required");
+
 		this.target = target;
 		this.method = method;
-		this.allValidationResults = validationResults;
+		this.allValidationResults = results;
 		this.forReturnValue = forReturnValue;
 	}
 
-
 	/**
-	 * Return the target of the method invocation to which validation was applied.
+	 * Private constructor copying from another {@code MethodValidationResult}.
 	 */
+	private MethodValidationException(MethodValidationResult other) {
+		this(other.getTarget(), other.getMethod(), other.isForReturnValue(),
+				other.getConstraintViolations(), other.getAllValidationResults());
+	}
+
+
+	// re-declare getConstraintViolations as NonNull
+
+	@Override
+	public Set<ConstraintViolation<?>> getConstraintViolations() {
+		return super.getConstraintViolations();
+	}
+
+	@Override
 	public Object getTarget() {
 		return this.target;
 	}
 
-	/**
-	 * Return the method to which validation was applied.
-	 */
+	@Override
 	public Method getMethod() {
 		return this.method;
 	}
 
-	/**
-	 * Whether the violations are for a return value.
-	 * If true the violations are from validating a return value.
-	 * If false the violations are from validating method arguments.
-	 */
+	@Override
 	public boolean isForReturnValue() {
 		return this.forReturnValue;
 	}
 
-	// re-declare parent class method for NonNull treatment of interface
-
-	@Override
-	public Set<ConstraintViolation<?>> getConstraintViolations() {
-		return super.getConstraintViolations();
-	}
-
 	@Override
 	public List<ParameterValidationResult> getAllValidationResults() {
 		return this.allValidationResults;
@@ -114,15 +120,28 @@ public List<ParameterErrors> getBeanResults() {
 				.toList();
 	}
 
-	@Override
-	public void throwIfViolationsPresent() {
-		throw this;
-	}
-
 	@Override
 	public String toString() {
 		return "MethodValidationResult (" + getConstraintViolations().size() + " violations) " +
 				"for " + this.method.toGenericString();
 	}
 
+
+	/**
+	 * Create an exception copying from the given result, or return the same
+	 * instance if it is a {@code MethodValidationException} already.
+	 */
+	public static MethodValidationException forResult(MethodValidationResult result) {
+		return (result instanceof MethodValidationException ex ? ex : new MethodValidationException(result));
+	}
+
+	/**
+	 * Create an exception for validation without errors.
+	 */
+	public static MethodValidationException forEmptyResult(Object target, Method method, boolean forReturnValue) {
+		return new MethodValidationException(
+				target, method, forReturnValue, Collections.emptySet(), Collections.emptyList());
+	}
+
+
 }

spring-context/src/main/java/org/springframework/validation/beanvalidation/MethodValidationInterceptor.java

@@ -104,13 +104,19 @@ public Object invoke(MethodInvocation invocation) throws Throwable {
 		Method method = invocation.getMethod();
 		Class<?>[] groups = determineValidationGroups(invocation);
 
-		this.delegate.validateMethodArguments(target, method, null, invocation.getArguments(), groups)
-				.throwIfViolationsPresent();
+		MethodValidationResult result;
+
+		result = this.delegate.validateMethodArguments(target, method, null, invocation.getArguments(), groups);
+		if (result.hasViolations()) {
+			throw MethodValidationException.forResult(result);
+		}
 
 		Object returnValue = invocation.proceed();
 
-		this.delegate.validateMethodReturnValue(target, method, null, returnValue, groups)
-				.throwIfViolationsPresent();
+		result = this.delegate.validateMethodReturnValue(target, method, null, returnValue, groups);
+		if (result.hasViolations()) {
+			throw MethodValidationException.forResult(result);
+		}
 
 		return returnValue;
 	}

spring-context/src/main/java/org/springframework/validation/beanvalidation/MethodValidationResult.java

@@ -16,6 +16,7 @@
 
 package org.springframework.validation.beanvalidation;
 
+import java.lang.reflect.Method;
 import java.util.List;
 import java.util.Set;
 
@@ -39,6 +40,30 @@
  */
 public interface MethodValidationResult {
 
+	/**
+	 * Return the target of the method invocation to which validation was applied.
+	 */
+	Object getTarget();
+
+	/**
+	 * Return the method to which validation was applied.
+	 */
+	Method getMethod();
+
+	/**
+	 * Whether the violations are for a return value.
+	 * If true the violations are from validating a return value.
+	 * If false the violations are from validating method arguments.
+	 */
+	boolean isForReturnValue();
+
+	/**
+	 * Whether the result contains any {@link ConstraintViolation}s.
+	 */
+	default boolean hasViolations() {
+		return !getConstraintViolations().isEmpty();
+	}
+
 	/**
 	 * Returns the set of constraint violations reported during a validation.
 	 * @return the {@code Set} of {@link ConstraintViolation}s, or an empty Set
@@ -72,11 +97,4 @@ public interface MethodValidationResult {
 	 */
 	List<ParameterErrors> getBeanResults();
 
-	/**
-	 * Check if {@link #getConstraintViolations()} is empty, and if not, raise
-	 * {@link MethodValidationException}.
-	 * @throws MethodValidationException if the result contains any violations
-	 */
-	void throwIfViolationsPresent();
-
 }