From c6f59c0ebe3c59c4f1d4767778b709e2d4291276 Mon Sep 17 00:00:00 2001 From: Yanming Zhou Date: Mon, 11 Mar 2024 11:39:18 +0800 Subject: [PATCH] Include errors from HandlerMethodValidationException for DefaultErrorAttributes Fix GH-39858 --- .../error/DefaultErrorAttributes.java | 16 ++++++ .../servlet/error/DefaultErrorAttributes.java | 37 +++++++++++-- .../error/DefaultErrorAttributesTests.java | 43 +++++++++++++++ .../error/DefaultErrorAttributesTests.java | 52 +++++++++++++++++-- 4 files changed, 141 insertions(+), 7 deletions(-) diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/reactive/error/DefaultErrorAttributes.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/reactive/error/DefaultErrorAttributes.java index 564f16e8fa86..779ae8db878d 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/reactive/error/DefaultErrorAttributes.java +++ b/spring-boot-project/spring-boot/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,14 @@ private String determineMessage(Throwable error, MergedAnnotation 1) ? "errors" : "error"); + } if (error instanceof ResponseStatusException responseStatusException) { return responseStatusException.getReason(); } @@ -151,6 +161,12 @@ private void handleException(Map errorAttributes, Throwable erro errorAttributes.put("errors", result.getAllErrors()); } } + if (error instanceof MethodValidationResult result) { + if (result.hasErrors()) { + errorAttributes.put("errors", + result.getAllErrors().stream().filter(ObjectError.class::isInstance).toList()); + } + } } @Override diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/servlet/error/DefaultErrorAttributes.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/servlet/error/DefaultErrorAttributes.java index c523627c1fcb..046c95b3143b 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/servlet/error/DefaultErrorAttributes.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/servlet/error/DefaultErrorAttributes.java @@ -20,6 +20,7 @@ import java.io.StringWriter; import java.util.Date; import java.util.LinkedHashMap; +import java.util.List; import java.util.Map; import jakarta.servlet.RequestDispatcher; @@ -29,6 +30,7 @@ import org.springframework.boot.web.error.ErrorAttributeOptions; import org.springframework.boot.web.error.ErrorAttributeOptions.Include; +import org.springframework.context.MessageSourceResolvable; import org.springframework.core.Ordered; import org.springframework.core.annotation.Order; import org.springframework.http.HttpStatus; @@ -36,6 +38,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 +65,7 @@ * @author Vedran Pavic * @author Scott Frederick * @author Moritz Halbritter + * @author Yanming Zhou * @since 2.0.0 * @see ErrorAttributes */ @@ -149,13 +153,20 @@ private void addErrorDetails(Map errorAttributes, WebRequest web } private void addErrorMessage(Map 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 errorAttributes, WebRequest webRequest, Throwable error) { @@ -193,6 +204,17 @@ private void addBindingResultErrorMessage(Map errorAttributes, B errorAttributes.put("errors", result.getAllErrors()); } + private void addMethodValidationResultErrorMessage(Map errorAttributes, + MethodValidationResult result) { + List errors = result.getAllErrors() + .stream() + .filter(ObjectError.class::isInstance) + .toList(); + errorAttributes.put("message", + "Validation failed for method='" + result.getMethod() + "'. " + "Error count: " + errors.size()); + errorAttributes.put("errors", errors); + } + private BindingResult extractBindingResult(Throwable error) { if (error instanceof BindingResult bindingResult) { return bindingResult; @@ -200,6 +222,13 @@ private BindingResult extractBindingResult(Throwable error) { return null; } + private MethodValidationResult extractMethodValidationResult(Throwable error) { + if (error instanceof MethodValidationResult methodValidationResult) { + return methodValidationResult; + } + return null; + } + private void addStackTrace(Map errorAttributes, Throwable error) { StringWriter stackTrace = new StringWriter(); error.printStackTrace(new PrintWriter(stackTrace)); diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/reactive/error/DefaultErrorAttributesTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/reactive/error/DefaultErrorAttributesTests.java index 9fa2d22fcd39..62d3c6f6a7a8 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/reactive/error/DefaultErrorAttributesTests.java +++ b/spring-boot-project/spring-boot/src/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,45 @@ 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 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 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"); + assertThat(attributes).containsEntry("errors", + methodValidationResult.getAllErrors().stream().filter(ObjectError.class::isInstance).toList()); + } + @Test void extractBindingResultErrorsExcludeMessageAndErrors() throws Exception { Method method = getClass().getDeclaredMethod("method", String.class); diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/servlet/error/DefaultErrorAttributesTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/servlet/error/DefaultErrorAttributesTests.java index 1eaaed469fe7..fd3e5c65a8a9 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/servlet/error/DefaultErrorAttributesTests.java +++ b/spring-boot-project/spring-boot/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 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> errorsSupplier, + String expectedMessage, Exception ex, ErrorAttributeOptions options) { this.request.setAttribute("jakarta.servlet.error.exception", ex); Map 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");