Skip to content

Commit 7a79da5

Browse files
rstoyanchevrstoyanchev
committed
Add default web handling of method validation errors
Closes gh-30644
1 parent a481c76 commit 7a79da5

File tree

10 files changed

+338
-62
lines changed

10 files changed

+338
-62
lines changed

‎spring-web/src/main/java/org/springframework/web/method/annotation/HandlerMethodValidationException.java

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
/*
2+
* Copyright 2002-2023 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package org.springframework.web.method.annotation;
18+
19+
import java.lang.reflect.Method;
20+
import java.util.List;
21+
import java.util.Locale;
22+
23+
import org.springframework.context.MessageSource;
24+
import org.springframework.http.HttpStatus;
25+
import org.springframework.validation.beanvalidation.MethodValidationResult;
26+
import org.springframework.validation.beanvalidation.ParameterValidationResult;
27+
import org.springframework.web.server.ResponseStatusException;
28+
import org.springframework.web.util.BindErrorUtils;
29+
30+
/**
31+
* {@link ResponseStatusException} that is also {@link MethodValidationResult}.
32+
* Raised by {@link HandlerMethodValidator} in case of method validation errors
33+
* on a web controller method.
34+
*
35+
* <p>The {@link #getStatusCode()} is 400 for input validation errors, and 500
36+
* for validation errors on a return value.
37+
*
38+
* @author Rossen Stoyanchev
39+
* @since 6.1
40+
*/
41+
@SuppressWarnings("serial")
42+
public class HandlerMethodValidationException extends ResponseStatusException implements MethodValidationResult {
43+
44+
private final MethodValidationResult validationResult;
45+
46+
47+
public HandlerMethodValidationException(MethodValidationResult validationResult) {
48+
super(initHttpStatus(validationResult), "Validation failure", null, null, null);
49+
this.validationResult = validationResult;
50+
}
51+
52+
private static HttpStatus initHttpStatus(MethodValidationResult validationResult) {
53+
return (!validationResult.isForReturnValue() ? HttpStatus.BAD_REQUEST : HttpStatus.INTERNAL_SERVER_ERROR);
54+
}
55+
56+
57+
@Override
58+
public Object[] getDetailMessageArguments(MessageSource messageSource, Locale locale) {
59+
return new Object[] { BindErrorUtils.resolveAndJoin(getAllErrors(), messageSource, locale) };
60+
}
61+
62+
@Override
63+
public Object[] getDetailMessageArguments() {
64+
return new Object[] { BindErrorUtils.resolveAndJoin(getAllErrors()) };
65+
}
66+
67+
@Override
68+
public Object getTarget() {
69+
return this.validationResult.getTarget();
70+
}
71+
72+
@Override
73+
public Method getMethod() {
74+
return this.validationResult.getMethod();
75+
}
76+
77+
@Override
78+
public boolean isForReturnValue() {
79+
return this.validationResult.isForReturnValue();
80+
}
81+
82+
@Override
83+
public List<ParameterValidationResult> getAllValidationResults() {
84+
return this.validationResult.getAllValidationResults();
85+
}
86+
87+
}

‎spring-web/src/main/java/org/springframework/web/method/annotation/HandlerMethodValidator.java

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@
2727
import org.springframework.validation.BindingResult;
2828
import org.springframework.validation.MessageCodesResolver;
2929
import org.springframework.validation.beanvalidation.MethodValidationAdapter;
30-
import org.springframework.validation.beanvalidation.MethodValidationException;
3130
import org.springframework.validation.beanvalidation.MethodValidationResult;
3231
import org.springframework.validation.beanvalidation.MethodValidator;
3332
import org.springframework.validation.beanvalidation.ParameterErrors;
@@ -91,7 +90,7 @@ public void applyArgumentValidation(
9190
}
9291
}
9392

94-
throw new MethodValidationException(result);
93+
throw new HandlerMethodValidationException(result);
9594
}
9695

9796
@Override
@@ -109,7 +108,7 @@ public void applyReturnValueValidation(
109108

110109
MethodValidationResult result = validateReturnValue(target, method, returnType, returnValue, groups);
111110
if (result.hasErrors()) {
112-
throw new MethodValidationException(result);
111+
throw new HandlerMethodValidationException(result);
113112
}
114113
}
115114

‎spring-web/src/test/java/org/springframework/web/ErrorResponseExceptionTests.java

Lines changed: 58 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -20,13 +20,11 @@
2020
import java.util.Collections;
2121
import java.util.List;
2222
import java.util.Locale;
23-
import java.util.Map;
24-
import java.util.function.BiFunction;
2523

2624
import org.junit.jupiter.api.Test;
2725

2826
import org.springframework.beans.testfixture.beans.TestBean;
29-
import org.springframework.context.MessageSource;
27+
import org.springframework.context.MessageSourceResolvable;
3028
import org.springframework.context.support.StaticMessageSource;
3129
import org.springframework.core.MethodParameter;
3230
import org.springframework.http.HttpHeaders;
@@ -36,9 +34,9 @@
3634
import org.springframework.http.ProblemDetail;
3735
import org.springframework.lang.Nullable;
3836
import org.springframework.util.LinkedMultiValueMap;
39-
import org.springframework.validation.BindException;
37+
import org.springframework.validation.BeanPropertyBindingResult;
4038
import org.springframework.validation.BindingResult;
41-
import org.springframework.validation.ObjectError;
39+
import org.springframework.validation.beanvalidation.MethodValidationResult;
4240
import org.springframework.web.bind.MethodArgumentNotValidException;
4341
import org.springframework.web.bind.MissingMatrixVariableException;
4442
import org.springframework.web.bind.MissingPathVariableException;
@@ -48,6 +46,7 @@
4846
import org.springframework.web.bind.UnsatisfiedServletRequestParameterException;
4947
import org.springframework.web.bind.support.WebExchangeBindException;
5048
import org.springframework.web.context.request.async.AsyncRequestTimeoutException;
49+
import org.springframework.web.method.annotation.HandlerMethodValidationException;
5150
import org.springframework.web.multipart.support.MissingServletRequestPartException;
5251
import org.springframework.web.server.MethodNotAllowedException;
5352
import org.springframework.web.server.MissingRequestValueException;
@@ -59,6 +58,9 @@
5958
import org.springframework.web.util.BindErrorUtils;
6059

6160
import static org.assertj.core.api.Assertions.assertThat;
61+
import static org.mockito.BDDMockito.mock;
62+
import static org.mockito.BDDMockito.reset;
63+
import static org.mockito.BDDMockito.when;
6264

6365
/**
6466
* Unit tests that verify the HTTP response details exposed by exceptions in the
@@ -245,20 +247,35 @@ void missingServletRequestPartException() {
245247
@Test
246248
void methodArgumentNotValidException() {
247249

248-
MessageSourceTestHelper messageSourceHelper = new MessageSourceTestHelper(MethodArgumentNotValidException.class);
249-
BindingResult bindingResult = messageSourceHelper.initBindingResult();
250+
ValidationTestHelper testHelper = new ValidationTestHelper(MethodArgumentNotValidException.class);
251+
BindingResult result = testHelper.bindingResult();
250252

251-
MethodArgumentNotValidException ex = new MethodArgumentNotValidException(this.methodParameter, bindingResult);
253+
MethodArgumentNotValidException ex = new MethodArgumentNotValidException(this.methodParameter, result);
252254

253255
assertStatus(ex, HttpStatus.BAD_REQUEST);
254256
assertDetail(ex, "Invalid request content.");
255-
messageSourceHelper.assertDetailMessage(ex);
256-
messageSourceHelper.assertErrorMessages(
257-
(source, locale) -> BindErrorUtils.resolve(ex.getAllErrors(), source, locale));
257+
testHelper.assertMessages(ex, ex.getAllErrors());
258258

259259
assertThat(ex.getHeaders()).isEmpty();
260260
}
261261

262+
@Test
263+
void handlerMethodValidationException() {
264+
MethodValidationResult result = mock(MethodValidationResult.class);
265+
when(result.isForReturnValue()).thenReturn(false);
266+
HandlerMethodValidationException ex = new HandlerMethodValidationException(result);
267+
268+
assertStatus(ex, HttpStatus.BAD_REQUEST);
269+
assertDetail(ex, "Validation failure");
270+
271+
reset(result);
272+
when(result.isForReturnValue()).thenReturn(true);
273+
ex = new HandlerMethodValidationException(result);
274+
275+
assertStatus(ex, HttpStatus.INTERNAL_SERVER_ERROR);
276+
assertDetail(ex, "Validation failure");
277+
}
278+
262279
@Test
263280
void unsupportedMediaTypeStatusException() {
264281

@@ -360,15 +377,14 @@ void unsatisfiedRequestParameterException() {
360377
@Test
361378
void webExchangeBindException() {
362379

363-
MessageSourceTestHelper messageSourceHelper = new MessageSourceTestHelper(WebExchangeBindException.class);
364-
BindingResult bindingResult = messageSourceHelper.initBindingResult();
380+
ValidationTestHelper testHelper = new ValidationTestHelper(WebExchangeBindException.class);
381+
BindingResult result = testHelper.bindingResult();
365382

366-
WebExchangeBindException ex = new WebExchangeBindException(this.methodParameter, bindingResult);
383+
WebExchangeBindException ex = new WebExchangeBindException(this.methodParameter, result);
367384

368385
assertStatus(ex, HttpStatus.BAD_REQUEST);
369386
assertDetail(ex, "Invalid request content.");
370-
messageSourceHelper.assertDetailMessage(ex);
371-
messageSourceHelper.assertErrorMessages(ex::resolveErrorMessages);
387+
testHelper.assertMessages(ex, ex.getAllErrors());
372388

373389
assertThat(ex.getHeaders()).isEmpty();
374390
}
@@ -434,59 +450,52 @@ private void assertDetailMessageCode(
434450
private void handle(String arg) {}
435451

436452

437-
private static class MessageSourceTestHelper {
453+
private static class ValidationTestHelper {
438454

439-
private final String code;
455+
private final BindingResult bindingResult;
440456

441-
public MessageSourceTestHelper(Class<? extends ErrorResponse> exceptionType) {
442-
this.code = "problemDetail." + exceptionType.getName();
443-
}
457+
private final StaticMessageSource messageSource = new StaticMessageSource();
458+
459+
public ValidationTestHelper(Class<? extends ErrorResponse> exceptionType) {
444460

445-
public BindingResult initBindingResult() {
446-
BindingResult bindingResult = new BindException(new TestBean(), "myBean");
447-
bindingResult.reject("bean.invalid.A", "Invalid bean message");
448-
bindingResult.reject("bean.invalid.B");
449-
bindingResult.rejectValue("name", "name.required", "must be provided");
450-
bindingResult.rejectValue("age", "age.min");
451-
return bindingResult;
461+
this.bindingResult = new BeanPropertyBindingResult(new TestBean(), "myBean");
462+
this.bindingResult.reject("bean.invalid.A", "Invalid bean message");
463+
this.bindingResult.reject("bean.invalid.B");
464+
this.bindingResult.rejectValue("name", "name.required", "must be provided");
465+
this.bindingResult.rejectValue("age", "age.min");
466+
467+
String code = "problemDetail." + exceptionType.getName();
468+
this.messageSource.addMessage(code, Locale.UK, "Failed because {0}. Also because {1}");
469+
this.messageSource.addMessage("bean.invalid.A", Locale.UK, "Bean A message");
470+
this.messageSource.addMessage("bean.invalid.B", Locale.UK, "Bean B message");
471+
this.messageSource.addMessage("name.required", Locale.UK, "name is required");
472+
this.messageSource.addMessage("age.min", Locale.UK, "age is below minimum");
452473
}
453474

454-
private void assertDetailMessage(ErrorResponse ex) {
475+
public BindingResult bindingResult() {
476+
return this.bindingResult;
477+
}
455478

456-
StaticMessageSource messageSource = initMessageSource();
479+
private void assertMessages(ErrorResponse ex, List<? extends MessageSourceResolvable> errors) {
457480

458-
String message = messageSource.getMessage(
481+
String message = this.messageSource.getMessage(
459482
ex.getDetailMessageCode(), ex.getDetailMessageArguments(), Locale.UK);
460483

461484
assertThat(message).isEqualTo(
462485
"Failed because Invalid bean message, and bean.invalid.B.myBean. " +
463486
"Also because name: must be provided, and age: age.min.myBean.age");
464487

465-
message = messageSource.getMessage(
466-
ex.getDetailMessageCode(), ex.getDetailMessageArguments(messageSource, Locale.UK), Locale.UK);
488+
message = this.messageSource.getMessage(
489+
ex.getDetailMessageCode(), ex.getDetailMessageArguments(this.messageSource, Locale.UK), Locale.UK);
467490

468491
assertThat(message).isEqualTo(
469492
"Failed because Bean A message, and Bean B message. " +
470493
"Also because name is required, and age is below minimum");
471-
}
472-
473-
private void assertErrorMessages(BiFunction<MessageSource, Locale, Map<ObjectError, String>> expectedMessages) {
474-
StaticMessageSource messageSource = initMessageSource();
475-
Map<ObjectError, String> map = expectedMessages.apply(messageSource, Locale.UK);
476494

477-
assertThat(map).hasSize(4).containsValues(
478-
"Bean A message", "Bean B message", "name is required", "age is below minimum");
495+
assertThat(BindErrorUtils.resolve(errors, this.messageSource, Locale.UK)).hasSize(4)
496+
.containsValues("Bean A message", "Bean B message", "name is required", "age is below minimum");
479497
}
480498

481-
private StaticMessageSource initMessageSource() {
482-
StaticMessageSource messageSource = new StaticMessageSource();
483-
messageSource.addMessage(this.code, Locale.UK, "Failed because {0}. Also because {1}");
484-
messageSource.addMessage("bean.invalid.A", Locale.UK, "Bean A message");
485-
messageSource.addMessage("bean.invalid.B", Locale.UK, "Bean B message");
486-
messageSource.addMessage("name.required", Locale.UK, "name is required");
487-
messageSource.addMessage("age.min", Locale.UK, "age is below minimum");
488-
return messageSource;
489-
}
490499
}
491500

492501
}

0 commit comments

Comments
 (0)