Skip to content

Commit

Permalink
Include errors from HandlerMethodValidationException for DefaultError…
Browse files Browse the repository at this point in the history 
…Attributes

Fix spring-projectsGH-39858
  • Loading branch information
@quaff
quaff committed Mar 11, 2024
1 parent 8b4f411 commit bac3d75
Show file tree
Hide file tree
Showing 4 changed files with 130 additions and 7 deletions.
12 changes: 12 additions & 0 deletions ...oot/src/main/java/org/springframework/boot/web/reactive/error/DefaultErrorAttributes.java
View file
Expand Up @@ -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;
Expand All @@ -58,6 +59,7 @@
* @author Michele Mancioppi
* @author Scott Frederick
* @author Moritz Halbritter
* @author Yanming Zhou
* @since 2.0.0
* @see ErrorAttributes
*/
Expand Down Expand Up @@ -117,6 +119,11 @@ 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();
}
Expand Down Expand Up @@ -151,6 +158,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
Expand Down
31 changes: 27 additions & 4 deletions ...boot/src/main/java/org/springframework/boot/web/servlet/error/DefaultErrorAttributes.java
View file
Expand Up @@ -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;
Expand All @@ -62,6 +63,7 @@
* @author Vedran Pavic
* @author Scott Frederick
* @author Moritz Halbritter
* @author Yanming Zhou
* @since 2.0.0
* @see ErrorAttributes
*/
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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));
Expand Down
42 changes: 42 additions & 0 deletions ...rc/test/java/org/springframework/boot/web/reactive/error/DefaultErrorAttributesTests.java
View file
Expand Up @@ -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;
Expand All @@ -51,6 +54,7 @@
* @author Stephane Nicoll
* @author Scott Frederick
* @author Moritz Halbritter
* @author Yanming Zhou
*/
class DefaultErrorAttributesTests {

Expand Down Expand Up @@ -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);
Expand Down
52 changes: 49 additions & 3 deletions ...src/test/java/org/springframework/boot/web/servlet/error/DefaultErrorAttributesTests.java
View file
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -48,6 +54,7 @@
* @author Vedran Pavic
* @author Scott Frederick
* @author Moritz Halbritter
* @author Yanming Zhou
*/
class DefaultErrorAttributesTests {

Expand Down Expand Up @@ -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");
Expand Down

0 comments on commit bac3d75

Please sign in to comment.