Skip to content

Commit 82b2189

Browse files
wilkinsonawilkinsona
committed
Polish "Consider HandlerMethodValidationException in DefaultErrorAttributes"
See gh-39865
1 parent 20e9ff9 commit 82b2189

File tree

4 files changed

+75
-121
lines changed

4 files changed

+75
-121
lines changed

‎spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/reactive/error/DefaultErrorAttributes.java

+40-47
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2012-2023 the original author or authors.
2+
* Copyright 2012-2024 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -20,6 +20,7 @@
2020
import java.io.StringWriter;
2121
import java.util.Date;
2222
import java.util.LinkedHashMap;
23+
import java.util.List;
2324
import java.util.Map;
2425
import java.util.Optional;
2526

@@ -47,8 +48,8 @@
4748
* <li>error - The error reason</li>
4849
* <li>exception - The class name of the root exception (if configured)</li>
4950
* <li>message - The exception message (if configured)</li>
50-
* <li>errors - Any {@link ObjectError}s from a {@link BindingResult} exception (if
51-
* configured)</li>
51+
* <li>errors - Any {@link ObjectError}s from a {@link BindingResult} or
52+
* {@link MethodValidationResult} exception (if configured)</li>
5253
* <li>trace - The exception stack trace (if configured)</li>
5354
* <li>path - The URL path when the exception was raised</li>
5455
* <li>requestId - Unique ID associated with the current request</li>
@@ -95,9 +96,8 @@ private Map<String, Object> getErrorAttributes(ServerRequest request, boolean in
9596
HttpStatus errorStatus = determineHttpStatus(error, responseStatusAnnotation);
9697
errorAttributes.put("status", errorStatus.value());
9798
errorAttributes.put("error", errorStatus.getReasonPhrase());
98-
errorAttributes.put("message", determineMessage(error, responseStatusAnnotation));
9999
errorAttributes.put("requestId", request.exchange().getRequest().getId());
100-
handleException(errorAttributes, determineException(error), includeStackTrace);
100+
handleException(errorAttributes, error, responseStatusAnnotation, includeStackTrace);
101101
return errorAttributes;
102102
}
103103

@@ -111,58 +111,51 @@ private HttpStatus determineHttpStatus(Throwable error, MergedAnnotation<Respons
111111
return responseStatusAnnotation.getValue("code", HttpStatus.class).orElse(HttpStatus.INTERNAL_SERVER_ERROR);
112112
}
113113

114-
private String determineMessage(Throwable error, MergedAnnotation<ResponseStatus> responseStatusAnnotation) {
115-
if (error instanceof BindingResult) {
116-
return error.getMessage();
117-
}
118-
if (error instanceof MethodValidationResult methodValidationResult) {
119-
long errorCount = methodValidationResult.getAllErrors()
120-
.stream()
121-
.filter(ObjectError.class::isInstance)
122-
.count();
123-
return "Validation failed for method: %s, with %d %s".formatted(methodValidationResult.getMethod(),
124-
errorCount, (errorCount > 1) ? "errors" : "error");
125-
}
126-
if (error instanceof ResponseStatusException responseStatusException) {
127-
return responseStatusException.getReason();
128-
}
129-
String reason = responseStatusAnnotation.getValue("reason", String.class).orElse("");
130-
if (StringUtils.hasText(reason)) {
131-
return reason;
132-
}
133-
return (error.getMessage() != null) ? error.getMessage() : "";
134-
}
135-
136-
private Throwable determineException(Throwable error) {
137-
if (error instanceof ResponseStatusException) {
138-
return (error.getCause() != null) ? error.getCause() : error;
139-
}
140-
return error;
141-
}
142-
143114
private void addStackTrace(Map<String, Object> errorAttributes, Throwable error) {
144115
StringWriter stackTrace = new StringWriter();
145116
error.printStackTrace(new PrintWriter(stackTrace));
146117
stackTrace.flush();
147118
errorAttributes.put("trace", stackTrace.toString());
148119
}
149120

150-
private void handleException(Map<String, Object> errorAttributes, Throwable error, boolean includeStackTrace) {
151-
errorAttributes.put("exception", error.getClass().getName());
152-
if (includeStackTrace) {
153-
addStackTrace(errorAttributes, error);
121+
private void handleException(Map<String, Object> errorAttributes, Throwable error,
122+
MergedAnnotation<ResponseStatus> responseStatusAnnotation, boolean includeStackTrace) {
123+
Throwable exception;
124+
if (error instanceof BindingResult bindingResult) {
125+
errorAttributes.put("message", error.getMessage());
126+
errorAttributes.put("errors", bindingResult.getAllErrors());
127+
exception = error;
154128
}
155-
if (error instanceof BindingResult result) {
156-
if (result.hasErrors()) {
157-
errorAttributes.put("errors", result.getAllErrors());
158-
}
129+
else if (error instanceof MethodValidationResult methodValidationResult) {
130+
addMessageAndErrorsFromMethodValidationResult(errorAttributes, methodValidationResult);
131+
exception = error;
159132
}
160-
if (error instanceof MethodValidationResult result) {
161-
if (result.hasErrors()) {
162-
errorAttributes.put("errors",
163-
result.getAllErrors().stream().filter(ObjectError.class::isInstance).toList());
164-
}
133+
else if (error instanceof ResponseStatusException responseStatusException) {
134+
errorAttributes.put("message", responseStatusException.getReason());
135+
exception = (responseStatusException.getCause() != null) ? responseStatusException.getCause() : error;
136+
}
137+
else {
138+
exception = error;
139+
String reason = responseStatusAnnotation.getValue("reason", String.class).orElse("");
140+
String message = StringUtils.hasText(reason) ? reason : error.getMessage();
141+
errorAttributes.put("message", (message != null) ? message : "");
165142
}
143+
errorAttributes.put("exception", exception.getClass().getName());
144+
if (includeStackTrace) {
145+
addStackTrace(errorAttributes, exception);
146+
}
147+
}
148+
149+
private void addMessageAndErrorsFromMethodValidationResult(Map<String, Object> errorAttributes,
150+
MethodValidationResult result) {
151+
List<ObjectError> errors = result.getAllErrors()
152+
.stream()
153+
.filter(ObjectError.class::isInstance)
154+
.map(ObjectError.class::cast)
155+
.toList();
156+
errorAttributes.put("message",
157+
"Validation failed for method='" + result.getMethod() + "'. Error count: " + errors.size());
158+
errorAttributes.put("errors", errors);
166159
}
167160

168161
@Override

‎spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/servlet/error/DefaultErrorAttributes.java

+21-19
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2012-2022 the original author or authors.
2+
* Copyright 2012-2024 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -30,7 +30,6 @@
3030

3131
import org.springframework.boot.web.error.ErrorAttributeOptions;
3232
import org.springframework.boot.web.error.ErrorAttributeOptions.Include;
33-
import org.springframework.context.MessageSourceResolvable;
3433
import org.springframework.core.Ordered;
3534
import org.springframework.core.annotation.Order;
3635
import org.springframework.http.HttpStatus;
@@ -53,8 +52,8 @@
5352
* <li>error - The error reason</li>
5453
* <li>exception - The class name of the root exception (if configured)</li>
5554
* <li>message - The exception message (if configured)</li>
56-
* <li>errors - Any {@link ObjectError}s from a {@link BindingResult} exception (if
57-
* configured)</li>
55+
* <li>errors - Any {@link ObjectError}s from a {@link BindingResult} or
56+
* {@link MethodValidationResult} exception (if configured)</li>
5857
* <li>trace - The exception stack trace (if configured)</li>
5958
* <li>path - The URL path when the exception was raised</li>
6059
* </ul>
@@ -149,20 +148,19 @@ private void addErrorDetails(Map<String, Object> errorAttributes, WebRequest web
149148
}
150149

151150
private void addErrorMessage(Map<String, Object> errorAttributes, WebRequest webRequest, Throwable error) {
152-
MethodValidationResult methodValidationResult = extractMethodValidationResult(error);
153-
if (methodValidationResult != null) {
154-
addMethodValidationResultErrorMessage(errorAttributes, methodValidationResult);
151+
BindingResult bindingResult = extractBindingResult(error);
152+
if (bindingResult != null) {
153+
addMessageAndErrorsFromBindingResult(errorAttributes, bindingResult);
155154
}
156155
else {
157-
BindingResult bindingResult = extractBindingResult(error);
158-
if (bindingResult != null) {
159-
addBindingResultErrorMessage(errorAttributes, bindingResult);
156+
MethodValidationResult methodValidationResult = extractMethodValidationResult(error);
157+
if (methodValidationResult != null) {
158+
addMessageAndErrorsFromMethodValidationResult(errorAttributes, methodValidationResult);
160159
}
161160
else {
162161
addExceptionErrorMessage(errorAttributes, webRequest, error);
163162
}
164163
}
165-
166164
}
167165

168166
private void addExceptionErrorMessage(Map<String, Object> errorAttributes, WebRequest webRequest, Throwable error) {
@@ -194,20 +192,24 @@ protected String getMessage(WebRequest webRequest, Throwable error) {
194192
return "No message available";
195193
}
196194

197-
private void addBindingResultErrorMessage(Map<String, Object> errorAttributes, BindingResult result) {
198-
errorAttributes.put("message", "Validation failed for object='" + result.getObjectName() + "'. "
199-
+ "Error count: " + result.getErrorCount());
200-
errorAttributes.put("errors", result.getAllErrors());
195+
private void addMessageAndErrorsFromBindingResult(Map<String, Object> errorAttributes, BindingResult result) {
196+
addMessageAndErrorsForValidationFailure(errorAttributes, "object='" + result.getObjectName() + "'",
197+
result.getAllErrors());
201198
}
202199

203-
private void addMethodValidationResultErrorMessage(Map<String, Object> errorAttributes,
200+
private void addMessageAndErrorsFromMethodValidationResult(Map<String, Object> errorAttributes,
204201
MethodValidationResult result) {
205-
List<? extends MessageSourceResolvable> errors = result.getAllErrors()
202+
List<ObjectError> errors = result.getAllErrors()
206203
.stream()
207204
.filter(ObjectError.class::isInstance)
205+
.map(ObjectError.class::cast)
208206
.toList();
209-
errorAttributes.put("message",
210-
"Validation failed for method='" + result.getMethod() + "'. " + "Error count: " + errors.size());
207+
addMessageAndErrorsForValidationFailure(errorAttributes, "method='" + result.getMethod() + "'", errors);
208+
}
209+
210+
private void addMessageAndErrorsForValidationFailure(Map<String, Object> errorAttributes, String validated,
211+
List<ObjectError> errors) {
212+
errorAttributes.put("message", "Validation failed for " + validated + ". Error count: " + errors.size());
211213
errorAttributes.put("errors", errors);
212214
}
213215

‎spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/reactive/error/DefaultErrorAttributesTests.java

+5-25
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2012-2023 the original author or authors.
2+
* Copyright 2012-2024 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -255,36 +255,16 @@ void extractMethodValidationResultErrors() throws Exception {
255255
Object target = "test";
256256
Method method = String.class.getMethod("substring", int.class);
257257
MethodParameter parameter = new MethodParameter(method, 0);
258-
MethodValidationResult methodValidationResult = new MethodValidationResult() {
259-
260-
@Override
261-
public Object getTarget() {
262-
return target;
263-
}
264-
265-
@Override
266-
public Method getMethod() {
267-
return method;
268-
}
269-
270-
@Override
271-
public boolean isForReturnValue() {
272-
return false;
273-
}
274-
275-
@Override
276-
public List<ParameterValidationResult> getAllValidationResults() {
277-
return List.of(new ParameterValidationResult(parameter, -1,
278-
List.of(new ObjectError("beginIndex", "beginIndex is negative")), null, null, null));
279-
}
280-
};
258+
MethodValidationResult methodValidationResult = MethodValidationResult.create(target, method,
259+
List.of(new ParameterValidationResult(parameter, -1,
260+
List.of(new ObjectError("beginIndex", "beginIndex is negative")), null, null, null)));
281261
HandlerMethodValidationException ex = new HandlerMethodValidationException(methodValidationResult);
282262
MockServerHttpRequest request = MockServerHttpRequest.get("/test").build();
283263
Map<String, Object> attributes = this.errorAttributes.getErrorAttributes(buildServerRequest(request, ex),
284264
ErrorAttributeOptions.of(Include.MESSAGE, Include.BINDING_ERRORS));
285265
assertThat(attributes.get("message")).asString()
286266
.isEqualTo(
287-
"Validation failed for method: public java.lang.String java.lang.String.substring(int), with 1 error");
267+
"Validation failed for method='public java.lang.String java.lang.String.substring(int)'. Error count: 1");
288268
assertThat(attributes).containsEntry("errors",
289269
methodValidationResult.getAllErrors().stream().filter(ObjectError.class::isInstance).toList());
290270
}

‎spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/servlet/error/DefaultErrorAttributesTests.java

+9-30
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2012-2023 the original author or authors.
2+
* Copyright 2012-2024 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -21,7 +21,6 @@
2121
import java.util.Date;
2222
import java.util.List;
2323
import java.util.Map;
24-
import java.util.function.Supplier;
2524

2625
import jakarta.servlet.ServletException;
2726
import org.junit.jupiter.api.Test;
@@ -213,42 +212,22 @@ void withHandlerMethodValidationExceptionBindingErrors() {
213212
Object target = "test";
214213
Method method = ReflectionUtils.findMethod(String.class, "substring", int.class);
215214
MethodParameter parameter = new MethodParameter(method, 0);
216-
MethodValidationResult methodValidationResult = new MethodValidationResult() {
217-
218-
@Override
219-
public Object getTarget() {
220-
return target;
221-
}
222-
223-
@Override
224-
public Method getMethod() {
225-
return method;
226-
}
227-
228-
@Override
229-
public boolean isForReturnValue() {
230-
return false;
231-
}
232-
233-
@Override
234-
public List<ParameterValidationResult> getAllValidationResults() {
235-
return List.of(new ParameterValidationResult(parameter, -1,
236-
List.of(new ObjectError("beginIndex", "beginIndex is negative")), null, null, null));
237-
}
238-
};
215+
MethodValidationResult methodValidationResult = MethodValidationResult.create(target, method,
216+
List.of(new ParameterValidationResult(parameter, -1,
217+
List.of(new ObjectError("beginIndex", "beginIndex is negative")), null, null, null)));
239218
HandlerMethodValidationException ex = new HandlerMethodValidationException(methodValidationResult);
240-
testErrorsSupplier(methodValidationResult::getAllErrors,
219+
testErrors(methodValidationResult.getAllErrors(),
241220
"Validation failed for method='public java.lang.String java.lang.String.substring(int)'. Error count: 1",
242221
ex, ErrorAttributeOptions.of(Include.MESSAGE, Include.BINDING_ERRORS));
243222
}
244223

245224
private void testBindingResult(BindingResult bindingResult, Exception ex, ErrorAttributeOptions options) {
246-
testErrorsSupplier(bindingResult::getAllErrors, "Validation failed for object='objectName'. Error count: 1", ex,
225+
testErrors(bindingResult.getAllErrors(), "Validation failed for object='objectName'. Error count: 1", ex,
247226
options);
248227
}
249228

250-
private void testErrorsSupplier(Supplier<List<? extends MessageSourceResolvable>> errorsSupplier,
251-
String expectedMessage, Exception ex, ErrorAttributeOptions options) {
229+
private void testErrors(List<? extends MessageSourceResolvable> errors, String expectedMessage, Exception ex,
230+
ErrorAttributeOptions options) {
252231
this.request.setAttribute("jakarta.servlet.error.exception", ex);
253232
Map<String, Object> attributes = this.errorAttributes.getErrorAttributes(this.webRequest, options);
254233
if (options.isIncluded(Include.MESSAGE)) {
@@ -258,7 +237,7 @@ private void testErrorsSupplier(Supplier<List<? extends MessageSourceResolvable>
258237
assertThat(attributes).doesNotContainKey("message");
259238
}
260239
if (options.isIncluded(Include.BINDING_ERRORS)) {
261-
assertThat(attributes).containsEntry("errors", errorsSupplier.get());
240+
assertThat(attributes).containsEntry("errors", errors);
262241
}
263242
else {
264243
assertThat(attributes).doesNotContainKey("errors");

0 commit comments

Comments
 (0)