Polishing in Web validation exceptions

- Update method order - Do not automatically create MessageSource arguments in WebExchangeBindException constructor as they're more likely to be created via getDetailMessageArguments with MessageSource passed in. See gh-30644
spring-projects · Jul 3, 2023 · e83594a · e83594a
1 parent a8ea472
commit e83594a
Show file tree

Hide file tree

Showing 2 changed files with 88 additions and 88 deletions.
diff --git a/spring-web/src/main/java/org/springframework/web/bind/MethodArgumentNotValidException.java b/spring-web/src/main/java/org/springframework/web/bind/MethodArgumentNotValidException.java
@@ -64,16 +64,6 @@ public MethodArgumentNotValidException(MethodParameter parameter, BindingResult
 	}
 
 
-	@Override
-	public HttpStatusCode getStatusCode() {
-		return HttpStatus.BAD_REQUEST;
-	}
-
-	@Override
-	public ProblemDetail getBody() {
-		return this.body;
-	}
-
 	/**
 	 * Return the method parameter that failed validation.
 	 */
@@ -82,59 +72,40 @@ public final MethodParameter getParameter() {
 	}
 
 	@Override
-	public String getMessage() {
-		StringBuilder sb = new StringBuilder("Validation failed for argument [")
-				.append(this.parameter.getParameterIndex()).append("] in ")
-				.append(this.parameter.getExecutable().toGenericString());
-		BindingResult bindingResult = getBindingResult();
-		if (bindingResult.getErrorCount() > 1) {
-			sb.append(" with ").append(bindingResult.getErrorCount()).append(" errors");
-		}
-		sb.append(": ");
-		for (ObjectError error : bindingResult.getAllErrors()) {
-			sb.append('[').append(error).append("] ");
-		}
-		return sb.toString();
+	public HttpStatusCode getStatusCode() {
+		return HttpStatus.BAD_REQUEST;
+	}
+
+	@Override
+	public ProblemDetail getBody() {
+		return this.body;
 	}
 
 	@Override
 	public Object[] getDetailMessageArguments() {
 		return new Object[] {
-				join(formatErrors(getGlobalErrors(), null, null)),
-				join(formatErrors(getFieldErrors(), null, null))};
+				join(errorsToStringList(getGlobalErrors())),
+				join(errorsToStringList(getFieldErrors()))};
 	}
 
 	@Override
 	public Object[] getDetailMessageArguments(MessageSource messageSource, Locale locale) {
 		return new Object[] {
-				join(formatErrors(getGlobalErrors(), messageSource, locale)),
-				join(formatErrors(getFieldErrors(), messageSource, locale))};
+				join(errorsToStringList(getGlobalErrors(), messageSource, locale)),
+				join(errorsToStringList(getFieldErrors(), messageSource, locale))};
 	}
 
 	private static String join(List<String> errors) {
 		return String.join(", and ", errors);
 	}
 
-	/**
-	 * Resolve global and field errors to messages with the given
-	 * {@link MessageSource} and {@link Locale}.
-	 * @return a Map with errors as keys and resolved messages as values
-	 * @since 6.0.3
-	 */
-	public Map<ObjectError, String> resolveErrorMessages(MessageSource source, Locale locale) {
-		Map<ObjectError, String> map = new LinkedHashMap<>(getErrorCount());
-		getGlobalErrors().forEach(error -> map.put(error, formatError(error, source, locale)));
-		getFieldErrors().forEach(error -> map.put(error, formatError(error, source, locale)));
-		return map;
-	}
-
 	/**
 	 * Convert each given {@link ObjectError} to a String in single quotes, taking
 	 * either the error's default message, or its error code.
 	 * @since 6.0
 	 */
 	public static List<String> errorsToStringList(List<? extends ObjectError> errors) {
-		return formatErrors(errors, null, null);
+		return errorsToStringList(errors, null, null);
 	}
 
 	/**
@@ -144,12 +115,6 @@ public static List<String> errorsToStringList(List<? extends ObjectError> errors
 	 * @since 6.0
 	 */
 	public static List<String> errorsToStringList(
-			List<? extends ObjectError> errors, @Nullable MessageSource source, Locale locale) {
-
-		return formatErrors(errors, source, locale);
-	}
-
-	public static List<String> formatErrors(
 			List<? extends ObjectError> errors, @Nullable MessageSource messageSource, @Nullable Locale locale) {
 
 		return errors.stream()
@@ -170,4 +135,33 @@ private static String formatError(
 		return (field + message);
 	}
 
+	/**
+	 * Resolve global and field errors to messages with the given
+	 * {@link MessageSource} and {@link Locale}.
+	 * @return a Map with errors as keys and resolved messages as values
+	 * @since 6.0.3
+	 */
+	public Map<ObjectError, String> resolveErrorMessages(MessageSource source, Locale locale) {
+		Map<ObjectError, String> map = new LinkedHashMap<>(getErrorCount());
+		getGlobalErrors().forEach(error -> map.put(error, formatError(error, source, locale)));
+		getFieldErrors().forEach(error -> map.put(error, formatError(error, source, locale)));
+		return map;
+	}
+
+	@Override
+	public String getMessage() {
+		StringBuilder sb = new StringBuilder("Validation failed for argument [")
+				.append(this.parameter.getParameterIndex()).append("] in ")
+				.append(this.parameter.getExecutable().toGenericString());
+		BindingResult bindingResult = getBindingResult();
+		if (bindingResult.getErrorCount() > 1) {
+			sb.append(" with ").append(bindingResult.getErrorCount()).append(" errors");
+		}
+		sb.append(": ");
+		for (ObjectError error : bindingResult.getAllErrors()) {
+			sb.append('[').append(error).append("] ");
+		}
+		return sb.toString();
+	}
+
 }
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
@@ -49,30 +49,67 @@ public class WebExchangeBindException extends ServerWebInputException implements
 
 
 	public WebExchangeBindException(MethodParameter parameter, BindingResult bindingResult) {
-		super("Validation failure", parameter, null, null, initMessageDetailArguments(bindingResult));
+		super("Validation failure", parameter, null, null, null);
 		this.bindingResult = bindingResult;
 		getBody().setDetail("Invalid request content.");
 	}
 
-	private static Object[] initMessageDetailArguments(BindingResult bindingResult) {
+
+	/**
+	 * Return the BindingResult that this BindException wraps.
+	 * <p>Will typically be a BeanPropertyBindingResult.
+	 * @see BeanPropertyBindingResult
+	 */
+	public final BindingResult getBindingResult() {
+		return this.bindingResult;
+	}
+
+
+	@Override
+	public Object[] getDetailMessageArguments() {
+		return new Object[] {
+				join(MethodArgumentNotValidException.errorsToStringList(getGlobalErrors())),
+				join(MethodArgumentNotValidException.errorsToStringList(getFieldErrors()))};
+	}
+
+	@Override
+	public Object[] getDetailMessageArguments(MessageSource source, Locale locale) {
 		return new Object[] {
-				join(MethodArgumentNotValidException.errorsToStringList(bindingResult.getGlobalErrors())),
-				join(MethodArgumentNotValidException.errorsToStringList(bindingResult.getFieldErrors()))};
+				join(MethodArgumentNotValidException.errorsToStringList(getGlobalErrors(), source, locale)),
+				join(MethodArgumentNotValidException.errorsToStringList(getFieldErrors(), source, locale))
+		};
 	}
 
 	private static String join(List<String> errors) {
 		return String.join(", and ", errors);
 	}
 
 	/**
-	 * Return the BindingResult that this BindException wraps.
-	 * <p>Will typically be a BeanPropertyBindingResult.
-	 * @see BeanPropertyBindingResult
+	 * Resolve global and field errors to messages with the given
+	 * {@link MessageSource} and {@link Locale}.
+	 * @return a Map with errors as key and resolves messages as value
+	 * @since 6.0.3
 	 */
-	public final BindingResult getBindingResult() {
-		return this.bindingResult;
+	public Map<ObjectError, String> resolveErrorMessages(MessageSource messageSource, Locale locale) {
+		Map<ObjectError, String> map = new LinkedHashMap<>();
+		addMessages(map, getGlobalErrors(), messageSource, locale);
+		addMessages(map, getFieldErrors(), messageSource, locale);
+		return map;
 	}
 
+	private static void addMessages(
+			Map<ObjectError, String> map, List<? extends ObjectError> errors,
+			MessageSource messageSource, Locale locale) {
+
+		List<String> messages = MethodArgumentNotValidException.errorsToStringList(errors, messageSource, locale);
+		for (int i = 0; i < errors.size(); i++) {
+			map.put(errors.get(i), messages.get(i));
+		}
+	}
+
+
+	// BindingResult implementation methods
+
 	@Override
 	public String getObjectName() {
 		return this.bindingResult.getObjectName();
@@ -285,6 +322,7 @@ public String[] getSuppressedFields() {
 		return this.bindingResult.getSuppressedFields();
 	}
 
+
 	/**
 	 * Returns diagnostic information about the errors held in this object.
 	 */
@@ -302,38 +340,6 @@ public String getMessage() {
 		return sb.toString();
 	}
 
-	@Override
-	public Object[] getDetailMessageArguments(MessageSource source, Locale locale) {
-		return new Object[] {
-				join(MethodArgumentNotValidException.errorsToStringList(getGlobalErrors(), source, locale)),
-				join(MethodArgumentNotValidException.errorsToStringList(getFieldErrors(), source, locale))
-		};
-	}
-
-	/**
-	 * Resolve global and field errors to messages with the given
-	 * {@link MessageSource} and {@link Locale}.
-	 * @return a Map with errors as key and resolves messages as value
-	 * @since 6.0.3
-	 */
-	public Map<ObjectError, String> resolveErrorMessages(MessageSource messageSource, Locale locale) {
-		Map<ObjectError, String> map = new LinkedHashMap<>();
-		addMessages(map, getGlobalErrors(), messageSource, locale);
-		addMessages(map, getFieldErrors(), messageSource, locale);
-		return map;
-	}
-
-	private static void addMessages(
-			Map<ObjectError, String> map, List<? extends ObjectError> errors,
-			MessageSource messageSource, Locale locale) {
-
-		List<String> messages = MethodArgumentNotValidException.errorsToStringList(errors, messageSource, locale);
-		for (int i = 0; i < errors.size(); i++) {
-			map.put(errors.get(i), messages.get(i));
-		}
-	}
-
-
 	@Override
 	public boolean equals(@Nullable Object other) {
 		return (this == other || this.bindingResult.equals(other));