Polish "Consider HandlerMethodValidationException in DefaultErrorAttributes"

See gh-39865
This commit is contained in:
Andy Wilkinson 2024-04-22 15:34:43 +01:00
parent 20e9ff9f3d
commit 82b218937c
4 changed files with 76 additions and 122 deletions

View File

@ -1,5 +1,5 @@
/*
* Copyright 2012-2023 the original author or authors.
* Copyright 2012-2024 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@ -20,6 +20,7 @@ import java.io.PrintWriter;
import java.io.StringWriter;
import java.util.Date;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.Optional;
@ -47,8 +48,8 @@ import org.springframework.web.server.ServerWebExchange;
* <li>error - The error reason</li>
* <li>exception - The class name of the root exception (if configured)</li>
* <li>message - The exception message (if configured)</li>
* <li>errors - Any {@link ObjectError}s from a {@link BindingResult} exception (if
* configured)</li>
* <li>errors - Any {@link ObjectError}s from a {@link BindingResult} or
* {@link MethodValidationResult} exception (if configured)</li>
* <li>trace - The exception stack trace (if configured)</li>
* <li>path - The URL path when the exception was raised</li>
* <li>requestId - Unique ID associated with the current request</li>
@ -95,9 +96,8 @@ public class DefaultErrorAttributes implements ErrorAttributes {
HttpStatus errorStatus = determineHttpStatus(error, responseStatusAnnotation);
errorAttributes.put("status", errorStatus.value());
errorAttributes.put("error", errorStatus.getReasonPhrase());
errorAttributes.put("message", determineMessage(error, responseStatusAnnotation));
errorAttributes.put("requestId", request.exchange().getRequest().getId());
handleException(errorAttributes, determineException(error), includeStackTrace);
handleException(errorAttributes, error, responseStatusAnnotation, includeStackTrace);
return errorAttributes;
}
@ -111,35 +111,6 @@ public class DefaultErrorAttributes implements ErrorAttributes {
return responseStatusAnnotation.getValue("code", HttpStatus.class).orElse(HttpStatus.INTERNAL_SERVER_ERROR);
}
private String determineMessage(Throwable error, MergedAnnotation<ResponseStatus> responseStatusAnnotation) {
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();
}
String reason = responseStatusAnnotation.getValue("reason", String.class).orElse("");
if (StringUtils.hasText(reason)) {
return reason;
}
return (error.getMessage() != null) ? error.getMessage() : "";
}
private Throwable determineException(Throwable error) {
if (error instanceof ResponseStatusException) {
return (error.getCause() != null) ? error.getCause() : error;
}
return error;
}
private void addStackTrace(Map<String, Object> errorAttributes, Throwable error) {
StringWriter stackTrace = new StringWriter();
error.printStackTrace(new PrintWriter(stackTrace));
@ -147,24 +118,46 @@ public class DefaultErrorAttributes implements ErrorAttributes {
errorAttributes.put("trace", stackTrace.toString());
}
private void handleException(Map<String, Object> errorAttributes, Throwable error, boolean includeStackTrace) {
errorAttributes.put("exception", error.getClass().getName());
private void handleException(Map<String, Object> errorAttributes, Throwable error,
MergedAnnotation<ResponseStatus> responseStatusAnnotation, boolean includeStackTrace) {
Throwable exception;
if (error instanceof BindingResult bindingResult) {
errorAttributes.put("message", error.getMessage());
errorAttributes.put("errors", bindingResult.getAllErrors());
exception = error;
}
else if (error instanceof MethodValidationResult methodValidationResult) {
addMessageAndErrorsFromMethodValidationResult(errorAttributes, methodValidationResult);
exception = error;
}
else if (error instanceof ResponseStatusException responseStatusException) {
errorAttributes.put("message", responseStatusException.getReason());
exception = (responseStatusException.getCause() != null) ? responseStatusException.getCause() : error;
}
else {
exception = error;
String reason = responseStatusAnnotation.getValue("reason", String.class).orElse("");
String message = StringUtils.hasText(reason) ? reason : error.getMessage();
errorAttributes.put("message", (message != null) ? message : "");
}
errorAttributes.put("exception", exception.getClass().getName());
if (includeStackTrace) {
addStackTrace(errorAttributes, error);
}
if (error instanceof BindingResult result) {
if (result.hasErrors()) {
errorAttributes.put("errors", result.getAllErrors());
}
}
if (error instanceof MethodValidationResult result) {
if (result.hasErrors()) {
errorAttributes.put("errors",
result.getAllErrors().stream().filter(ObjectError.class::isInstance).toList());
}
addStackTrace(errorAttributes, exception);
}
}
private void addMessageAndErrorsFromMethodValidationResult(Map<String, Object> errorAttributes,
MethodValidationResult result) {
List<ObjectError> errors = result.getAllErrors()
.stream()
.filter(ObjectError.class::isInstance)
.map(ObjectError.class::cast)
.toList();
errorAttributes.put("message",
"Validation failed for method='" + result.getMethod() + "'. Error count: " + errors.size());
errorAttributes.put("errors", errors);
}
@Override
public Throwable getError(ServerRequest request) {
Optional<Object> error = request.attribute(ERROR_INTERNAL_ATTRIBUTE);

View File

@ -1,5 +1,5 @@
/*
* Copyright 2012-2022 the original author or authors.
* Copyright 2012-2024 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@ -30,7 +30,6 @@ import jakarta.servlet.http.HttpServletResponse;
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;
@ -53,8 +52,8 @@ import org.springframework.web.servlet.ModelAndView;
* <li>error - The error reason</li>
* <li>exception - The class name of the root exception (if configured)</li>
* <li>message - The exception message (if configured)</li>
* <li>errors - Any {@link ObjectError}s from a {@link BindingResult} exception (if
* configured)</li>
* <li>errors - Any {@link ObjectError}s from a {@link BindingResult} or
* {@link MethodValidationResult} exception (if configured)</li>
* <li>trace - The exception stack trace (if configured)</li>
* <li>path - The URL path when the exception was raised</li>
* </ul>
@ -149,20 +148,19 @@ public class DefaultErrorAttributes implements ErrorAttributes, HandlerException
}
private void addErrorMessage(Map<String, Object> errorAttributes, WebRequest webRequest, Throwable error) {
MethodValidationResult methodValidationResult = extractMethodValidationResult(error);
if (methodValidationResult != null) {
addMethodValidationResultErrorMessage(errorAttributes, methodValidationResult);
BindingResult bindingResult = extractBindingResult(error);
if (bindingResult != null) {
addMessageAndErrorsFromBindingResult(errorAttributes, bindingResult);
}
else {
BindingResult bindingResult = extractBindingResult(error);
if (bindingResult != null) {
addBindingResultErrorMessage(errorAttributes, bindingResult);
MethodValidationResult methodValidationResult = extractMethodValidationResult(error);
if (methodValidationResult != null) {
addMessageAndErrorsFromMethodValidationResult(errorAttributes, methodValidationResult);
}
else {
addExceptionErrorMessage(errorAttributes, webRequest, error);
}
}
}
private void addExceptionErrorMessage(Map<String, Object> errorAttributes, WebRequest webRequest, Throwable error) {
@ -194,20 +192,24 @@ public class DefaultErrorAttributes implements ErrorAttributes, HandlerException
return "No message available";
}
private void addBindingResultErrorMessage(Map<String, Object> errorAttributes, BindingResult result) {
errorAttributes.put("message", "Validation failed for object='" + result.getObjectName() + "'. "
+ "Error count: " + result.getErrorCount());
errorAttributes.put("errors", result.getAllErrors());
private void addMessageAndErrorsFromBindingResult(Map<String, Object> errorAttributes, BindingResult result) {
addMessageAndErrorsForValidationFailure(errorAttributes, "object='" + result.getObjectName() + "'",
result.getAllErrors());
}
private void addMethodValidationResultErrorMessage(Map<String, Object> errorAttributes,
private void addMessageAndErrorsFromMethodValidationResult(Map<String, Object> errorAttributes,
MethodValidationResult result) {
List<? extends MessageSourceResolvable> errors = result.getAllErrors()
List<ObjectError> errors = result.getAllErrors()
.stream()
.filter(ObjectError.class::isInstance)
.map(ObjectError.class::cast)
.toList();
errorAttributes.put("message",
"Validation failed for method='" + result.getMethod() + "'. " + "Error count: " + errors.size());
addMessageAndErrorsForValidationFailure(errorAttributes, "method='" + result.getMethod() + "'", errors);
}
private void addMessageAndErrorsForValidationFailure(Map<String, Object> errorAttributes, String validated,
List<ObjectError> errors) {
errorAttributes.put("message", "Validation failed for " + validated + ". Error count: " + errors.size());
errorAttributes.put("errors", errors);
}

View File

@ -1,5 +1,5 @@
/*
* Copyright 2012-2023 the original author or authors.
* Copyright 2012-2024 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@ -255,36 +255,16 @@ class DefaultErrorAttributesTests {
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));
}
};
MethodValidationResult methodValidationResult = MethodValidationResult.create(target, method,
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");
"Validation failed for method='public java.lang.String java.lang.String.substring(int)'. Error count: 1");
assertThat(attributes).containsEntry("errors",
methodValidationResult.getAllErrors().stream().filter(ObjectError.class::isInstance).toList());
}

View File

@ -1,5 +1,5 @@
/*
* Copyright 2012-2023 the original author or authors.
* Copyright 2012-2024 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@ -21,7 +21,6 @@ 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;
@ -213,42 +212,22 @@ class DefaultErrorAttributesTests {
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));
}
};
MethodValidationResult methodValidationResult = MethodValidationResult.create(target, method,
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,
testErrors(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,
testErrors(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) {
private void testErrors(List<? extends MessageSourceResolvable> errors, 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)) {
@ -258,7 +237,7 @@ class DefaultErrorAttributesTests {
assertThat(attributes).doesNotContainKey("message");
}
if (options.isIncluded(Include.BINDING_ERRORS)) {
assertThat(attributes).containsEntry("errors", errorsSupplier.get());
assertThat(attributes).containsEntry("errors", errors);
}
else {
assertThat(attributes).doesNotContainKey("errors");