Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

server.error.include-binding-errors does not recognize MethodValidationResult exceptions #39865

Closed
wants to merge 1 commit into from
Closed

server.error.include-binding-errors does not recognize MethodValidationResult exceptions #39865

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions ...oot/src/main/java/org/springframework/boot/web/reactive/error/DefaultErrorAttributes.java
View file
Open in desktop
Original file line number Diff line number Diff line change
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,14 @@ private String determineMessage(Throwable error, MergedAnnotation<ResponseStatus
if (error instanceof BindingResult) {
return error.getMessage();
}
if (error instanceof MethodValidationResult methodValidationResult) {
long errorCount = methodValidationResult.getAllErrors()
.stream()
.filter(ObjectError.class::isInstance)
.count();
return "Validation failed for method: %s, with %d %s".formatted(methodValidationResult.getMethod(),
errorCount, (errorCount > 1) ? "errors" : "error");
}
if (error instanceof ResponseStatusException responseStatusException) {
return responseStatusException.getReason();
}
Expand Down Expand Up @@ -151,6 +161,12 @@ 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().stream().filter(ObjectError.class::isInstance).toList());
}
}
}

@Override
Expand Down
37 changes: 33 additions & 4 deletions ...boot/src/main/java/org/springframework/boot/web/servlet/error/DefaultErrorAttributes.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -29,13 +30,15 @@

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;
import org.springframework.util.ObjectUtils;
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 +65,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 +153,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 +204,31 @@ private void addBindingResultErrorMessage(Map<String, Object> errorAttributes, B
errorAttributes.put("errors", result.getAllErrors());
}

private void addMethodValidationResultErrorMessage(Map<String, Object> errorAttributes,
MethodValidationResult result) {
List<? extends MessageSourceResolvable> 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;
}
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
43 changes: 43 additions & 0 deletions ...rc/test/java/org/springframework/boot/web/reactive/error/DefaultErrorAttributesTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
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,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<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");
assertThat(attributes).containsEntry("errors",
methodValidationResult.getAllErrors().stream().filter(ObjectError.class::isInstance).toList());
}

@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
Open in desktop
Original file line number Diff line number Diff line change
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