Include errors from HandlerMethodValidationException for DefaultError…

…Attributes Fix GH-39858
spring-projects · Mar 11, 2024 · 6b6f1c8 · 6b6f1c8
1 parent 8b4f411
commit 6b6f1c8
Show file tree

Hide file tree

Showing 4 changed files with 129 additions and 7 deletions.
diff --git a/...oot/src/main/java/org/springframework/boot/web/reactive/error/DefaultErrorAttributes.java b/...oot/src/main/java/org/springframework/boot/web/reactive/error/DefaultErrorAttributes.java
@@ -32,6 +32,7 @@
 import org.springframework.util.StringUtils;
 import org.springframework.validation.BindingResult;
 import org.springframework.validation.ObjectError;
+import org.springframework.validation.method.MethodValidationResult;
 import org.springframework.web.bind.annotation.ResponseStatus;
 import org.springframework.web.reactive.function.server.ServerRequest;
 import org.springframework.web.server.ResponseStatusException;
@@ -58,6 +59,7 @@
  * @author Michele Mancioppi
  * @author Scott Frederick
  * @author Moritz Halbritter
+ * @author Yanming Zhou
  * @since 2.0.0
  * @see ErrorAttributes
  */
@@ -117,6 +119,10 @@ private String determineMessage(Throwable error, MergedAnnotation<ResponseStatus
 		if (error instanceof BindingResult) {
 			return error.getMessage();
 		}
+		if (error instanceof MethodValidationResult methodValidationResult) {
+			return "Validation failed for method: %s, with %d error(s)".formatted(methodValidationResult.getMethod(),
+					methodValidationResult.getAllErrors().size());
+		}
 		if (error instanceof ResponseStatusException responseStatusException) {
 			return responseStatusException.getReason();
 		}
@@ -151,6 +157,11 @@ private void handleException(Map<String, Object> errorAttributes, Throwable erro
 				errorAttributes.put("errors", result.getAllErrors());
 			}
 		}
+		if (error instanceof MethodValidationResult result) {
+			if (result.hasErrors()) {
+				errorAttributes.put("errors", result.getAllErrors());
+			}
+		}
 	}
 
 	@Override

diff --git a/...boot/src/main/java/org/springframework/boot/web/servlet/error/DefaultErrorAttributes.java b/...boot/src/main/java/org/springframework/boot/web/servlet/error/DefaultErrorAttributes.java
@@ -36,6 +36,7 @@
 import org.springframework.util.StringUtils;
 import org.springframework.validation.BindingResult;
 import org.springframework.validation.ObjectError;
+import org.springframework.validation.method.MethodValidationResult;
 import org.springframework.web.context.request.RequestAttributes;
 import org.springframework.web.context.request.WebRequest;
 import org.springframework.web.servlet.HandlerExceptionResolver;
@@ -62,6 +63,7 @@
  * @author Vedran Pavic
  * @author Scott Frederick
  * @author Moritz Halbritter
+ * @author Yanming Zhou
  * @since 2.0.0
  * @see ErrorAttributes
  */
@@ -149,13 +151,20 @@ private void addErrorDetails(Map<String, Object> errorAttributes, WebRequest web
 	}
 
 	private void addErrorMessage(Map<String, Object> errorAttributes, WebRequest webRequest, Throwable error) {
-		BindingResult result = extractBindingResult(error);
-		if (result == null) {
-			addExceptionErrorMessage(errorAttributes, webRequest, error);
+		MethodValidationResult methodValidationResult = extractMethodValidationResult(error);
+		if (methodValidationResult != null) {
+			addMethodValidationResultErrorMessage(errorAttributes, methodValidationResult);
 		}
 		else {
-			addBindingResultErrorMessage(errorAttributes, result);
+			BindingResult bindingResult = extractBindingResult(error);
+			if (bindingResult != null) {
+				addBindingResultErrorMessage(errorAttributes, bindingResult);
+			}
+			else {
+				addExceptionErrorMessage(errorAttributes, webRequest, error);
+			}
 		}
+
 	}
 
 	private void addExceptionErrorMessage(Map<String, Object> errorAttributes, WebRequest webRequest, Throwable error) {
@@ -193,13 +202,27 @@ private void addBindingResultErrorMessage(Map<String, Object> errorAttributes, B
 		errorAttributes.put("errors", result.getAllErrors());
 	}
 
+	private void addMethodValidationResultErrorMessage(Map<String, Object> errorAttributes,
+			MethodValidationResult result) {
+		errorAttributes.put("message", "Validation failed for method='" + result.getMethod() + "'. " + "Error count: "
+				+ result.getAllErrors().size());
+		errorAttributes.put("errors", result.getAllErrors());
+	}
+
 	private BindingResult extractBindingResult(Throwable error) {
 		if (error instanceof BindingResult bindingResult) {
 			return bindingResult;
 		}
 		return null;
 	}
 
+	private MethodValidationResult extractMethodValidationResult(Throwable error) {
+		if (error instanceof MethodValidationResult methodValidationResult) {
+			return methodValidationResult;
+		}
+		return null;
+	}
+
 	private void addStackTrace(Map<String, Object> errorAttributes, Throwable error) {
 		StringWriter stackTrace = new StringWriter();
 		error.printStackTrace(new PrintWriter(stackTrace));

diff --git a/...rc/test/java/org/springframework/boot/web/reactive/error/DefaultErrorAttributesTests.java b/...rc/test/java/org/springframework/boot/web/reactive/error/DefaultErrorAttributesTests.java
@@ -35,8 +35,11 @@
 import org.springframework.validation.BindingResult;
 import org.springframework.validation.MapBindingResult;
 import org.springframework.validation.ObjectError;
+import org.springframework.validation.method.MethodValidationResult;
+import org.springframework.validation.method.ParameterValidationResult;
 import org.springframework.web.bind.annotation.ResponseStatus;
 import org.springframework.web.bind.support.WebExchangeBindException;
+import org.springframework.web.method.annotation.HandlerMethodValidationException;
 import org.springframework.web.reactive.function.server.ServerRequest;
 import org.springframework.web.server.ResponseStatusException;
 import org.springframework.web.server.ServerWebExchange;
@@ -51,6 +54,7 @@
  * @author Stephane Nicoll
  * @author Scott Frederick
  * @author Moritz Halbritter
+ * @author Yanming Zhou
  */
 class DefaultErrorAttributesTests {
 
@@ -271,6 +275,44 @@ void extractBindingResultErrors() throws Exception {
 		assertThat(attributes).containsEntry("errors", bindingResult.getAllErrors());
 	}
 
+	@Test
+	void extractMethodValidationResultErrors() throws Exception {
+		Object target = "test";
+		Method method = String.class.getMethod("substring", int.class);
+		MethodParameter parameter = new MethodParameter(method, 0);
+		MethodValidationResult methodValidationResult = new MethodValidationResult() {
+
+			@Override
+			public Object getTarget() {
+				return target;
+			}
+
+			@Override
+			public Method getMethod() {
+				return method;
+			}
+
+			@Override
+			public boolean isForReturnValue() {
+				return false;
+			}
+
+			@Override
+			public List<ParameterValidationResult> getAllValidationResults() {
+				return List.of(new ParameterValidationResult(parameter, -1,
+						List.of(new ObjectError("beginIndex", "beginIndex is negative")), null, null, null));
+			}
+		};
+		HandlerMethodValidationException ex = new HandlerMethodValidationException(methodValidationResult);
+		MockServerHttpRequest request = MockServerHttpRequest.get("/test").build();
+		Map<String, Object> attributes = this.errorAttributes.getErrorAttributes(buildServerRequest(request, ex),
+				ErrorAttributeOptions.of(Include.MESSAGE, Include.BINDING_ERRORS));
+		assertThat(attributes.get("message")).asString()
+			.isEqualTo(
+					"Validation failed for method: public java.lang.String java.lang.String.substring(int), with 1 error(s)");
+		assertThat(attributes).containsEntry("errors", methodValidationResult.getAllErrors());
+	}
+
 	@Test
 	void extractBindingResultErrorsExcludeMessageAndErrors() throws Exception {
 		Method method = getClass().getDeclaredMethod("method", String.class);

diff --git a/...src/test/java/org/springframework/boot/web/servlet/error/DefaultErrorAttributesTests.java b/...src/test/java/org/springframework/boot/web/servlet/error/DefaultErrorAttributesTests.java
@@ -19,13 +19,16 @@
 import java.lang.reflect.Method;
 import java.util.Collections;
 import java.util.Date;
+import java.util.List;
 import java.util.Map;
+import java.util.function.Supplier;
 
 import jakarta.servlet.ServletException;
 import org.junit.jupiter.api.Test;
 
 import org.springframework.boot.web.error.ErrorAttributeOptions;
 import org.springframework.boot.web.error.ErrorAttributeOptions.Include;
+import org.springframework.context.MessageSourceResolvable;
 import org.springframework.core.MethodParameter;
 import org.springframework.http.HttpStatus;
 import org.springframework.mock.web.MockHttpServletRequest;
@@ -34,9 +37,12 @@
 import org.springframework.validation.BindingResult;
 import org.springframework.validation.MapBindingResult;
 import org.springframework.validation.ObjectError;
+import org.springframework.validation.method.MethodValidationResult;
+import org.springframework.validation.method.ParameterValidationResult;
 import org.springframework.web.bind.MethodArgumentNotValidException;
 import org.springframework.web.context.request.ServletWebRequest;
 import org.springframework.web.context.request.WebRequest;
+import org.springframework.web.method.annotation.HandlerMethodValidationException;
 import org.springframework.web.servlet.ModelAndView;
 
 import static org.assertj.core.api.Assertions.assertThat;
@@ -48,6 +54,7 @@
  * @author Vedran Pavic
  * @author Scott Frederick
  * @author Moritz Halbritter
+ * @author Yanming Zhou
  */
 class DefaultErrorAttributesTests {
 
@@ -202,18 +209,57 @@ void withMethodArgumentNotValidExceptionBindingErrors() {
 		testBindingResult(bindingResult, ex, ErrorAttributeOptions.of(Include.MESSAGE, Include.BINDING_ERRORS));
 	}
 
+	@Test
+	void withHandlerMethodValidationExceptionBindingErrors() {
+		Object target = "test";
+		Method method = ReflectionUtils.findMethod(String.class, "substring", int.class);
+		MethodParameter parameter = new MethodParameter(method, 0);
+		MethodValidationResult methodValidationResult = new MethodValidationResult() {
+
+			@Override
+			public Object getTarget() {
+				return target;
+			}
+
+			@Override
+			public Method getMethod() {
+				return method;
+			}
+
+			@Override
+			public boolean isForReturnValue() {
+				return false;
+			}
+
+			@Override
+			public List<ParameterValidationResult> getAllValidationResults() {
+				return List.of(new ParameterValidationResult(parameter, -1,
+						List.of(new ObjectError("beginIndex", "beginIndex is negative")), null, null, null));
+			}
+		};
+		HandlerMethodValidationException ex = new HandlerMethodValidationException(methodValidationResult);
+		testErrorsSupplier(methodValidationResult::getAllErrors,
+				"Validation failed for method='public java.lang.String java.lang.String.substring(int)'. Error count: 1",
+				ex, ErrorAttributeOptions.of(Include.MESSAGE, Include.BINDING_ERRORS));
+	}
+
 	private void testBindingResult(BindingResult bindingResult, Exception ex, ErrorAttributeOptions options) {
+		testErrorsSupplier(bindingResult::getAllErrors, "Validation failed for object='objectName'. Error count: 1", ex,
+				options);
+	}
+
+	private void testErrorsSupplier(Supplier<List<? extends MessageSourceResolvable>> errorsSupplier,
+			String expectedMessage, Exception ex, ErrorAttributeOptions options) {
 		this.request.setAttribute("jakarta.servlet.error.exception", ex);
 		Map<String, Object> attributes = this.errorAttributes.getErrorAttributes(this.webRequest, options);
 		if (options.isIncluded(Include.MESSAGE)) {
-			assertThat(attributes).containsEntry("message",
-					"Validation failed for object='objectName'. Error count: 1");
+			assertThat(attributes).containsEntry("message", expectedMessage);
 		}
 		else {
 			assertThat(attributes).doesNotContainKey("message");
 		}
 		if (options.isIncluded(Include.BINDING_ERRORS)) {
-			assertThat(attributes).containsEntry("errors", bindingResult.getAllErrors());
+			assertThat(attributes).containsEntry("errors", errorsSupplier.get());
 		}
 		else {
 			assertThat(attributes).doesNotContainKey("errors");