From 7a79da589ae1e8dd431f40fe44880ac064c87fe1 Mon Sep 17 00:00:00 2001 From: rstoyanchev Date: Fri, 30 Jun 2023 16:09:33 +0100 Subject: [PATCH] Add default web handling of method validation errors Closes gh-30644 --- .../HandlerMethodValidationException.java | 87 ++++++++++++++ .../annotation/HandlerMethodValidator.java | 5 +- .../web/ErrorResponseExceptionTests.java | 107 ++++++++++-------- .../ResponseEntityExceptionHandler.java | 48 +++++++- .../annotation/MethodValidationTests.java | 4 +- .../ResponseEntityExceptionHandlerTests.java | 19 ++++ .../ResponseEntityExceptionHandler.java | 51 +++++++++ .../DefaultHandlerExceptionResolver.java | 57 +++++++++- .../annotation/MethodValidationTests.java | 5 +- .../ResponseEntityExceptionHandlerTests.java | 17 ++- 10 files changed, 338 insertions(+), 62 deletions(-) create mode 100644 spring-web/src/main/java/org/springframework/web/method/annotation/HandlerMethodValidationException.java diff --git a/spring-web/src/main/java/org/springframework/web/method/annotation/HandlerMethodValidationException.java b/spring-web/src/main/java/org/springframework/web/method/annotation/HandlerMethodValidationException.java new file mode 100644 index 000000000000..b1ffa481bde4 --- /dev/null +++ b/spring-web/src/main/java/org/springframework/web/method/annotation/HandlerMethodValidationException.java @@ -0,0 +1,87 @@ +/* + * Copyright 2002-2023 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. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.web.method.annotation; + +import java.lang.reflect.Method; +import java.util.List; +import java.util.Locale; + +import org.springframework.context.MessageSource; +import org.springframework.http.HttpStatus; +import org.springframework.validation.beanvalidation.MethodValidationResult; +import org.springframework.validation.beanvalidation.ParameterValidationResult; +import org.springframework.web.server.ResponseStatusException; +import org.springframework.web.util.BindErrorUtils; + +/** + * {@link ResponseStatusException} that is also {@link MethodValidationResult}. + * Raised by {@link HandlerMethodValidator} in case of method validation errors + * on a web controller method. + * + *

The {@link #getStatusCode()} is 400 for input validation errors, and 500 + * for validation errors on a return value. + * + * @author Rossen Stoyanchev + * @since 6.1 + */ +@SuppressWarnings("serial") +public class HandlerMethodValidationException extends ResponseStatusException implements MethodValidationResult { + + private final MethodValidationResult validationResult; + + + public HandlerMethodValidationException(MethodValidationResult validationResult) { + super(initHttpStatus(validationResult), "Validation failure", null, null, null); + this.validationResult = validationResult; + } + + private static HttpStatus initHttpStatus(MethodValidationResult validationResult) { + return (!validationResult.isForReturnValue() ? HttpStatus.BAD_REQUEST : HttpStatus.INTERNAL_SERVER_ERROR); + } + + + @Override + public Object[] getDetailMessageArguments(MessageSource messageSource, Locale locale) { + return new Object[] { BindErrorUtils.resolveAndJoin(getAllErrors(), messageSource, locale) }; + } + + @Override + public Object[] getDetailMessageArguments() { + return new Object[] { BindErrorUtils.resolveAndJoin(getAllErrors()) }; + } + + @Override + public Object getTarget() { + return this.validationResult.getTarget(); + } + + @Override + public Method getMethod() { + return this.validationResult.getMethod(); + } + + @Override + public boolean isForReturnValue() { + return this.validationResult.isForReturnValue(); + } + + @Override + public List getAllValidationResults() { + return this.validationResult.getAllValidationResults(); + } + +} diff --git a/spring-web/src/main/java/org/springframework/web/method/annotation/HandlerMethodValidator.java b/spring-web/src/main/java/org/springframework/web/method/annotation/HandlerMethodValidator.java index 3e7a1265fae0..36605ff4a549 100644 --- a/spring-web/src/main/java/org/springframework/web/method/annotation/HandlerMethodValidator.java +++ b/spring-web/src/main/java/org/springframework/web/method/annotation/HandlerMethodValidator.java @@ -27,7 +27,6 @@ import org.springframework.validation.BindingResult; import org.springframework.validation.MessageCodesResolver; import org.springframework.validation.beanvalidation.MethodValidationAdapter; -import org.springframework.validation.beanvalidation.MethodValidationException; import org.springframework.validation.beanvalidation.MethodValidationResult; import org.springframework.validation.beanvalidation.MethodValidator; import org.springframework.validation.beanvalidation.ParameterErrors; @@ -91,7 +90,7 @@ public void applyArgumentValidation( } } - throw new MethodValidationException(result); + throw new HandlerMethodValidationException(result); } @Override @@ -109,7 +108,7 @@ public void applyReturnValueValidation( MethodValidationResult result = validateReturnValue(target, method, returnType, returnValue, groups); if (result.hasErrors()) { - throw new MethodValidationException(result); + throw new HandlerMethodValidationException(result); } } diff --git a/spring-web/src/test/java/org/springframework/web/ErrorResponseExceptionTests.java b/spring-web/src/test/java/org/springframework/web/ErrorResponseExceptionTests.java index 2570486832f9..3c5ced38f875 100644 --- a/spring-web/src/test/java/org/springframework/web/ErrorResponseExceptionTests.java +++ b/spring-web/src/test/java/org/springframework/web/ErrorResponseExceptionTests.java @@ -20,13 +20,11 @@ import java.util.Collections; import java.util.List; import java.util.Locale; -import java.util.Map; -import java.util.function.BiFunction; import org.junit.jupiter.api.Test; import org.springframework.beans.testfixture.beans.TestBean; -import org.springframework.context.MessageSource; +import org.springframework.context.MessageSourceResolvable; import org.springframework.context.support.StaticMessageSource; import org.springframework.core.MethodParameter; import org.springframework.http.HttpHeaders; @@ -36,9 +34,9 @@ import org.springframework.http.ProblemDetail; import org.springframework.lang.Nullable; import org.springframework.util.LinkedMultiValueMap; -import org.springframework.validation.BindException; +import org.springframework.validation.BeanPropertyBindingResult; import org.springframework.validation.BindingResult; -import org.springframework.validation.ObjectError; +import org.springframework.validation.beanvalidation.MethodValidationResult; import org.springframework.web.bind.MethodArgumentNotValidException; import org.springframework.web.bind.MissingMatrixVariableException; import org.springframework.web.bind.MissingPathVariableException; @@ -48,6 +46,7 @@ import org.springframework.web.bind.UnsatisfiedServletRequestParameterException; import org.springframework.web.bind.support.WebExchangeBindException; import org.springframework.web.context.request.async.AsyncRequestTimeoutException; +import org.springframework.web.method.annotation.HandlerMethodValidationException; import org.springframework.web.multipart.support.MissingServletRequestPartException; import org.springframework.web.server.MethodNotAllowedException; import org.springframework.web.server.MissingRequestValueException; @@ -59,6 +58,9 @@ import org.springframework.web.util.BindErrorUtils; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.BDDMockito.mock; +import static org.mockito.BDDMockito.reset; +import static org.mockito.BDDMockito.when; /** * Unit tests that verify the HTTP response details exposed by exceptions in the @@ -245,20 +247,35 @@ void missingServletRequestPartException() { @Test void methodArgumentNotValidException() { - MessageSourceTestHelper messageSourceHelper = new MessageSourceTestHelper(MethodArgumentNotValidException.class); - BindingResult bindingResult = messageSourceHelper.initBindingResult(); + ValidationTestHelper testHelper = new ValidationTestHelper(MethodArgumentNotValidException.class); + BindingResult result = testHelper.bindingResult(); - MethodArgumentNotValidException ex = new MethodArgumentNotValidException(this.methodParameter, bindingResult); + MethodArgumentNotValidException ex = new MethodArgumentNotValidException(this.methodParameter, result); assertStatus(ex, HttpStatus.BAD_REQUEST); assertDetail(ex, "Invalid request content."); - messageSourceHelper.assertDetailMessage(ex); - messageSourceHelper.assertErrorMessages( - (source, locale) -> BindErrorUtils.resolve(ex.getAllErrors(), source, locale)); + testHelper.assertMessages(ex, ex.getAllErrors()); assertThat(ex.getHeaders()).isEmpty(); } + @Test + void handlerMethodValidationException() { + MethodValidationResult result = mock(MethodValidationResult.class); + when(result.isForReturnValue()).thenReturn(false); + HandlerMethodValidationException ex = new HandlerMethodValidationException(result); + + assertStatus(ex, HttpStatus.BAD_REQUEST); + assertDetail(ex, "Validation failure"); + + reset(result); + when(result.isForReturnValue()).thenReturn(true); + ex = new HandlerMethodValidationException(result); + + assertStatus(ex, HttpStatus.INTERNAL_SERVER_ERROR); + assertDetail(ex, "Validation failure"); + } + @Test void unsupportedMediaTypeStatusException() { @@ -360,15 +377,14 @@ void unsatisfiedRequestParameterException() { @Test void webExchangeBindException() { - MessageSourceTestHelper messageSourceHelper = new MessageSourceTestHelper(WebExchangeBindException.class); - BindingResult bindingResult = messageSourceHelper.initBindingResult(); + ValidationTestHelper testHelper = new ValidationTestHelper(WebExchangeBindException.class); + BindingResult result = testHelper.bindingResult(); - WebExchangeBindException ex = new WebExchangeBindException(this.methodParameter, bindingResult); + WebExchangeBindException ex = new WebExchangeBindException(this.methodParameter, result); assertStatus(ex, HttpStatus.BAD_REQUEST); assertDetail(ex, "Invalid request content."); - messageSourceHelper.assertDetailMessage(ex); - messageSourceHelper.assertErrorMessages(ex::resolveErrorMessages); + testHelper.assertMessages(ex, ex.getAllErrors()); assertThat(ex.getHeaders()).isEmpty(); } @@ -434,59 +450,52 @@ private void assertDetailMessageCode( private void handle(String arg) {} - private static class MessageSourceTestHelper { + private static class ValidationTestHelper { - private final String code; + private final BindingResult bindingResult; - public MessageSourceTestHelper(Class exceptionType) { - this.code = "problemDetail." + exceptionType.getName(); - } + private final StaticMessageSource messageSource = new StaticMessageSource(); + + public ValidationTestHelper(Class exceptionType) { - public BindingResult initBindingResult() { - BindingResult bindingResult = new BindException(new TestBean(), "myBean"); - bindingResult.reject("bean.invalid.A", "Invalid bean message"); - bindingResult.reject("bean.invalid.B"); - bindingResult.rejectValue("name", "name.required", "must be provided"); - bindingResult.rejectValue("age", "age.min"); - return bindingResult; + this.bindingResult = new BeanPropertyBindingResult(new TestBean(), "myBean"); + this.bindingResult.reject("bean.invalid.A", "Invalid bean message"); + this.bindingResult.reject("bean.invalid.B"); + this.bindingResult.rejectValue("name", "name.required", "must be provided"); + this.bindingResult.rejectValue("age", "age.min"); + + String code = "problemDetail." + exceptionType.getName(); + this.messageSource.addMessage(code, Locale.UK, "Failed because {0}. Also because {1}"); + this.messageSource.addMessage("bean.invalid.A", Locale.UK, "Bean A message"); + this.messageSource.addMessage("bean.invalid.B", Locale.UK, "Bean B message"); + this.messageSource.addMessage("name.required", Locale.UK, "name is required"); + this.messageSource.addMessage("age.min", Locale.UK, "age is below minimum"); } - private void assertDetailMessage(ErrorResponse ex) { + public BindingResult bindingResult() { + return this.bindingResult; + } - StaticMessageSource messageSource = initMessageSource(); + private void assertMessages(ErrorResponse ex, List errors) { - String message = messageSource.getMessage( + String message = this.messageSource.getMessage( ex.getDetailMessageCode(), ex.getDetailMessageArguments(), Locale.UK); assertThat(message).isEqualTo( "Failed because Invalid bean message, and bean.invalid.B.myBean. " + "Also because name: must be provided, and age: age.min.myBean.age"); - message = messageSource.getMessage( - ex.getDetailMessageCode(), ex.getDetailMessageArguments(messageSource, Locale.UK), Locale.UK); + message = this.messageSource.getMessage( + ex.getDetailMessageCode(), ex.getDetailMessageArguments(this.messageSource, Locale.UK), Locale.UK); assertThat(message).isEqualTo( "Failed because Bean A message, and Bean B message. " + "Also because name is required, and age is below minimum"); - } - - private void assertErrorMessages(BiFunction> expectedMessages) { - StaticMessageSource messageSource = initMessageSource(); - Map map = expectedMessages.apply(messageSource, Locale.UK); - assertThat(map).hasSize(4).containsValues( - "Bean A message", "Bean B message", "name is required", "age is below minimum"); + assertThat(BindErrorUtils.resolve(errors, this.messageSource, Locale.UK)).hasSize(4) + .containsValues("Bean A message", "Bean B message", "name is required", "age is below minimum"); } - private StaticMessageSource initMessageSource() { - StaticMessageSource messageSource = new StaticMessageSource(); - messageSource.addMessage(this.code, Locale.UK, "Failed because {0}. Also because {1}"); - messageSource.addMessage("bean.invalid.A", Locale.UK, "Bean A message"); - messageSource.addMessage("bean.invalid.B", Locale.UK, "Bean B message"); - messageSource.addMessage("name.required", Locale.UK, "name is required"); - messageSource.addMessage("age.min", Locale.UK, "age is below minimum"); - return messageSource; - } } } diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/ResponseEntityExceptionHandler.java b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/ResponseEntityExceptionHandler.java index 367bd8de351c..c9bed00458c8 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/ResponseEntityExceptionHandler.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/ResponseEntityExceptionHandler.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2022 the original author or authors. + * Copyright 2002-2023 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. @@ -25,15 +25,18 @@ import org.springframework.context.MessageSource; import org.springframework.context.MessageSourceAware; import org.springframework.http.HttpHeaders; +import org.springframework.http.HttpStatus; import org.springframework.http.HttpStatusCode; import org.springframework.http.ProblemDetail; import org.springframework.http.ResponseEntity; import org.springframework.lang.Nullable; +import org.springframework.validation.beanvalidation.MethodValidationException; import org.springframework.web.ErrorResponse; import org.springframework.web.ErrorResponseException; import org.springframework.web.bind.annotation.ControllerAdvice; import org.springframework.web.bind.annotation.ExceptionHandler; import org.springframework.web.bind.support.WebExchangeBindException; +import org.springframework.web.method.annotation.HandlerMethodValidationException; import org.springframework.web.server.MethodNotAllowedException; import org.springframework.web.server.MissingRequestValueException; import org.springframework.web.server.NotAcceptableStatusException; @@ -97,10 +100,12 @@ protected MessageSource getMessageSource() { MissingRequestValueException.class, UnsatisfiedRequestParameterException.class, WebExchangeBindException.class, + HandlerMethodValidationException.class, ServerWebInputException.class, ServerErrorException.class, ResponseStatusException.class, - ErrorResponseException.class + ErrorResponseException.class, + MethodValidationException.class }) public final Mono> handleException(Exception ex, ServerWebExchange exchange) { if (ex instanceof MethodNotAllowedException theEx) { @@ -121,6 +126,9 @@ else if (ex instanceof UnsatisfiedRequestParameterException theEx) { else if (ex instanceof WebExchangeBindException theEx) { return handleWebExchangeBindException(theEx, theEx.getHeaders(), theEx.getStatusCode(), exchange); } + else if (ex instanceof HandlerMethodValidationException theEx) { + return handleHandlerMethodValidationException(theEx, theEx.getHeaders(), theEx.getStatusCode(), exchange); + } else if (ex instanceof ServerWebInputException theEx) { return handleServerWebInputException(theEx, theEx.getHeaders(), theEx.getStatusCode(), exchange); } @@ -133,6 +141,9 @@ else if (ex instanceof ResponseStatusException theEx) { else if (ex instanceof ErrorResponseException theEx) { return handleErrorResponseException(theEx, theEx.getHeaders(), theEx.getStatusCode(), exchange); } + else if (ex instanceof MethodValidationException theEx) { + return handleMethodValidationException(theEx, HttpStatus.INTERNAL_SERVER_ERROR, exchange); + } else { if (logger.isWarnEnabled()) { logger.warn("Unexpected exception type: " + ex.getClass().getName()); @@ -237,6 +248,23 @@ protected Mono> handleWebExchangeBindException( return handleExceptionInternal(ex, null, headers, status, exchange); } + /** + * Customize the handling of {@link HandlerMethodValidationException}. + *

This method delegates to {@link #handleExceptionInternal}. + * @param ex the exception to handle + * @param headers the headers to use for the response + * @param status the status code to use for the response + * @param exchange the current request and response + * @return a {@code Mono} with the {@code ResponseEntity} for the response + * @since 6.1 + */ + protected Mono> handleHandlerMethodValidationException( + HandlerMethodValidationException ex, HttpHeaders headers, HttpStatusCode status, + ServerWebExchange exchange) { + + return handleExceptionInternal(ex, null, headers, status, exchange); + } + /** * Customize the handling of {@link ServerWebInputException}. *

This method delegates to {@link #handleExceptionInternal}. @@ -301,6 +329,22 @@ protected Mono> handleErrorResponseException( return handleExceptionInternal(ex, null, headers, status, exchange); } + /** + * Customize the handling of {@link MethodValidationException}. + *

This method delegates to {@link #handleExceptionInternal}. + * @param ex the exception to handle + * @param status the status code to use for the response + * @param exchange the current request and response + * @return a {@code Mono} with the {@code ResponseEntity} for the response + * @since 6.1 + */ + protected Mono> handleMethodValidationException( + MethodValidationException ex, HttpStatus status, ServerWebExchange exchange) { + + ProblemDetail body = createProblemDetail(ex, status, "Validation failed", null, null, exchange); + return handleExceptionInternal(ex, body, null, status, exchange); + } + /** * Convenience method to create a {@link ProblemDetail} for any exception * that doesn't implement {@link ErrorResponse}, also performing a diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/MethodValidationTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/MethodValidationTests.java index a2649c6e5273..ee4ad52099e5 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/MethodValidationTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/MethodValidationTests.java @@ -43,7 +43,6 @@ import org.springframework.validation.Validator; import org.springframework.validation.annotation.Validated; import org.springframework.validation.beanvalidation.LocalValidatorFactoryBean; -import org.springframework.validation.beanvalidation.MethodValidationException; import org.springframework.validation.beanvalidation.ParameterValidationResult; import org.springframework.validation.beanvalidation.SpringValidatorAdapter; import org.springframework.web.bind.WebDataBinder; @@ -55,6 +54,7 @@ import org.springframework.web.bind.support.WebExchangeBindException; import org.springframework.web.context.support.GenericWebApplicationContext; import org.springframework.web.method.HandlerMethod; +import org.springframework.web.method.annotation.HandlerMethodValidationException; import org.springframework.web.reactive.HandlerResult; import org.springframework.web.server.ServerWebExchange; import org.springframework.web.testfixture.http.server.reactive.MockServerHttpRequest; @@ -202,7 +202,7 @@ void modelAttributeWithBindingResultAndRequestHeader() { StepVerifier.create(this.handlerAdapter.handle(exchange, hm)) .consumeErrorWith(throwable -> { - MethodValidationException ex = (MethodValidationException) throwable; + HandlerMethodValidationException ex = (HandlerMethodValidationException) throwable; assertThat(this.jakartaValidator.getValidationCount()).isEqualTo(1); assertThat(this.jakartaValidator.getMethodValidationCount()).isEqualTo(1); diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/ResponseEntityExceptionHandlerTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/ResponseEntityExceptionHandlerTests.java index c098c2f088c9..6da9bb5b1179 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/ResponseEntityExceptionHandlerTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/ResponseEntityExceptionHandlerTests.java @@ -37,9 +37,12 @@ import org.springframework.http.ResponseEntity; import org.springframework.util.LinkedMultiValueMap; import org.springframework.validation.BeanPropertyBindingResult; +import org.springframework.validation.beanvalidation.MethodValidationException; +import org.springframework.validation.beanvalidation.MethodValidationResult; import org.springframework.web.ErrorResponse; import org.springframework.web.ErrorResponseException; import org.springframework.web.bind.support.WebExchangeBindException; +import org.springframework.web.method.annotation.HandlerMethodValidationException; import org.springframework.web.server.MethodNotAllowedException; import org.springframework.web.server.MissingRequestValueException; import org.springframework.web.server.NotAcceptableStatusException; @@ -53,6 +56,7 @@ import org.springframework.web.testfixture.server.MockServerWebExchange; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.BDDMockito.mock; /** * Unit tests for {@link ResponseEntityExceptionHandler}. @@ -105,6 +109,21 @@ void handleWebExchangeBindException() { testException(new WebExchangeBindException(null, new BeanPropertyBindingResult(new Object(), "foo"))); } + @Test + public void handlerMethodValidationException() { + testException(new HandlerMethodValidationException(mock(MethodValidationResult.class))); + } + + @Test + public void methodValidationException() { + MethodValidationException ex = new MethodValidationException(mock(MethodValidationResult.class)); + ResponseEntity entity = this.exceptionHandler.handleException(ex, this.exchange).block(); + + assertThat(entity).isNotNull(); + assertThat(entity.getStatusCode()).isEqualTo(HttpStatus.INTERNAL_SERVER_ERROR); + assertThat(entity.getBody()).isInstanceOf(ProblemDetail.class); + } + @Test void handleServerWebInputException() { testException(new ServerWebInputException("")); diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ResponseEntityExceptionHandler.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ResponseEntityExceptionHandler.java index 489ee1434490..fd696be71c01 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ResponseEntityExceptionHandler.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ResponseEntityExceptionHandler.java @@ -34,6 +34,7 @@ import org.springframework.http.converter.HttpMessageNotWritableException; import org.springframework.lang.Nullable; import org.springframework.validation.BindException; +import org.springframework.validation.beanvalidation.MethodValidationException; import org.springframework.web.ErrorResponse; import org.springframework.web.ErrorResponseException; import org.springframework.web.HttpMediaTypeNotAcceptableException; @@ -48,6 +49,7 @@ import org.springframework.web.context.request.ServletWebRequest; import org.springframework.web.context.request.WebRequest; import org.springframework.web.context.request.async.AsyncRequestTimeoutException; +import org.springframework.web.method.annotation.HandlerMethodValidationException; import org.springframework.web.multipart.support.MissingServletRequestPartException; import org.springframework.web.servlet.NoHandlerFoundException; import org.springframework.web.servlet.resource.NoResourceFoundException; @@ -121,6 +123,7 @@ protected MessageSource getMessageSource() { MissingServletRequestPartException.class, ServletRequestBindingException.class, MethodArgumentNotValidException.class, + HandlerMethodValidationException.class, NoHandlerFoundException.class, NoResourceFoundException.class, AsyncRequestTimeoutException.class, @@ -129,6 +132,7 @@ protected MessageSource getMessageSource() { TypeMismatchException.class, HttpMessageNotReadableException.class, HttpMessageNotWritableException.class, + MethodValidationException.class, BindException.class }) @Nullable @@ -157,6 +161,9 @@ else if (ex instanceof ServletRequestBindingException subEx) { else if (ex instanceof MethodArgumentNotValidException subEx) { return handleMethodArgumentNotValid(subEx, subEx.getHeaders(), subEx.getStatusCode(), request); } + else if (ex instanceof HandlerMethodValidationException subEx) { + return handleHandlerMethodValidationException(subEx, subEx.getHeaders(), subEx.getStatusCode(), request); + } else if (ex instanceof NoHandlerFoundException subEx) { return handleNoHandlerFoundException(subEx, subEx.getHeaders(), subEx.getStatusCode(), request); } @@ -185,6 +192,9 @@ else if (ex instanceof HttpMessageNotReadableException theEx) { else if (ex instanceof HttpMessageNotWritableException theEx) { return handleHttpMessageNotWritable(theEx, headers, HttpStatus.INTERNAL_SERVER_ERROR, request); } + else if (ex instanceof MethodValidationException subEx) { + return handleMethodValidationException(subEx, headers, HttpStatus.INTERNAL_SERVER_ERROR, request); + } else if (ex instanceof BindException theEx) { return handleBindException(theEx, headers, HttpStatus.BAD_REQUEST, request); } @@ -335,6 +345,24 @@ protected ResponseEntity handleMethodArgumentNotValid( return handleExceptionInternal(ex, null, headers, status, request); } + /** + * Customize the handling of {@link HandlerMethodValidationException}. + *

This method delegates to {@link #handleExceptionInternal}. + * @param ex the exception to handle + * @param headers the headers to be written to the response + * @param status the selected response status + * @param request the current request + * @return a {@code ResponseEntity} for the response to use, possibly + * {@code null} when the response is already committed + * @since 6.1 + */ + @Nullable + protected ResponseEntity handleHandlerMethodValidationException( + HandlerMethodValidationException ex, HttpHeaders headers, HttpStatusCode status, WebRequest request) { + + return handleExceptionInternal(ex, null, headers, status, request); + } + /** * Customize the handling of {@link NoHandlerFoundException}. *

This method delegates to {@link #handleExceptionInternal}. @@ -521,6 +549,29 @@ protected ResponseEntity handleBindException( return handleExceptionInternal(ex, body, headers, status, request); } + + /** + * Customize the handling of {@link MethodValidationException}. + *

By default this method creates a {@link ProblemDetail} with the status + * and a short detail message, and also looks up an override for the detail + * via {@link MessageSource}, before delegating to + * {@link #handleExceptionInternal}. + * @param ex the exception to handle + * @param headers the headers to use for the response + * @param status the status code to use for the response + * @param request the current request + * @return a {@code ResponseEntity} for the response to use, possibly + * {@code null} when the response is already committed + * @since 6.1 + */ + @Nullable + protected ResponseEntity handleMethodValidationException( + MethodValidationException ex, HttpHeaders headers, HttpStatus status, WebRequest request) { + + ProblemDetail body = createProblemDetail(ex, status, "Validation failed", null, null, request); + return handleExceptionInternal(ex, body, headers, status, request); + } + /** * Convenience method to create a {@link ProblemDetail} for any exception * that doesn't implement {@link ErrorResponse}, also performing a diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/support/DefaultHandlerExceptionResolver.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/support/DefaultHandlerExceptionResolver.java index fa9c853e4fe6..52f946d479d8 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/support/DefaultHandlerExceptionResolver.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/support/DefaultHandlerExceptionResolver.java @@ -33,6 +33,7 @@ import org.springframework.lang.Nullable; import org.springframework.validation.BindException; import org.springframework.validation.BindingResult; +import org.springframework.validation.beanvalidation.MethodValidationException; import org.springframework.web.ErrorResponse; import org.springframework.web.HttpMediaTypeNotAcceptableException; import org.springframework.web.HttpMediaTypeNotSupportedException; @@ -45,6 +46,7 @@ import org.springframework.web.bind.annotation.RequestBody; import org.springframework.web.bind.annotation.RequestPart; import org.springframework.web.context.request.async.AsyncRequestTimeoutException; +import org.springframework.web.method.annotation.HandlerMethodValidationException; import org.springframework.web.multipart.MultipartFile; import org.springframework.web.multipart.support.MissingServletRequestPartException; import org.springframework.web.servlet.ModelAndView; @@ -118,8 +120,8 @@ *
MethodArgumentNotValidException
*
400 (SC_BAD_REQUEST)
* - * - *
BindException
+ * + *
{@link HandlerMethodValidationException}
*
400 (SC_BAD_REQUEST)
* * @@ -134,6 +136,10 @@ *
AsyncRequestTimeoutException
*
503 (SC_SERVICE_UNAVAILABLE)
* + * + *
{@link MethodValidationException}
+ *
500 (SC_INTERNAL_SERVER_ERROR)
+ * * * * @@ -200,6 +206,9 @@ else if (ex instanceof ServletRequestBindingException theEx) { else if (ex instanceof MethodArgumentNotValidException theEx) { mav = handleMethodArgumentNotValidException(theEx, request, response, handler); } + else if (ex instanceof HandlerMethodValidationException theEx) { + mav = handleHandlerMethodValidationException(theEx, request, response, handler); + } else if (ex instanceof NoHandlerFoundException theEx) { mav = handleNoHandlerFoundException(theEx, request, response, handler); } @@ -228,6 +237,9 @@ else if (ex instanceof HttpMessageNotReadableException theEx) { else if (ex instanceof HttpMessageNotWritableException theEx) { return handleHttpMessageNotWritable(theEx, request, response, handler); } + else if (ex instanceof MethodValidationException theEx) { + return handleMethodValidationException(theEx, request, response, handler); + } else if (ex instanceof BindException theEx) { return handleBindException(theEx, request, response, handler); } @@ -399,6 +411,26 @@ protected ModelAndView handleMethodArgumentNotValidException(MethodArgumentNotVa return null; } + /** + * Handle the case where method validation for a controller method failed. + *

The default implementation returns {@code null} in which case the + * exception is handled in {@link #handleErrorResponse}. + * @param ex the exception to be handled + * @param request current HTTP request + * @param response current HTTP response + * @param handler the executed handler + * @return an empty {@code ModelAndView} indicating the exception was handled, or + * {@code null} indicating the exception should be handled in {@link #handleErrorResponse} + * @throws IOException potentially thrown from {@link HttpServletResponse#sendError} + * @since 6.1 + */ + @Nullable + protected ModelAndView handleHandlerMethodValidationException(HandlerMethodValidationException ex, + HttpServletRequest request, HttpServletResponse response, @Nullable Object handler) throws IOException { + + return null; + } + /** * Handle the case where no handler was found during the dispatch. *

The default implementation returns {@code null} in which case the @@ -577,6 +609,27 @@ protected ModelAndView handleHttpMessageNotWritable(HttpMessageNotWritableExcept return new ModelAndView(); } + /** + * Handle the case where method validation failed on a component that is + * not a web controller, e.g. on some underlying service. + *

The default implementation sends an HTTP 500 error, and returns an empty {@code ModelAndView}. + * Alternatively, a fallback view could be chosen, or the HttpMessageNotWritableException could + * be rethrown as-is. + * @param ex the exception to be handled + * @param request current HTTP request + * @param response current HTTP response + * @param handler the executed handler + * @return an empty {@code ModelAndView} indicating the exception was handled + * @throws IOException potentially thrown from {@link HttpServletResponse#sendError} + * @since 6.1 + */ + protected ModelAndView handleMethodValidationException(MethodValidationException ex, + HttpServletRequest request, HttpServletResponse response, @Nullable Object handler) throws IOException { + + sendServerError(ex, request, response); + return new ModelAndView(); + } + /** * Handle the case where an {@linkplain ModelAttribute @ModelAttribute} method * argument has binding or validation errors and is not followed by another diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/MethodValidationTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/MethodValidationTests.java index fb27dfad8b20..dca5849d0d97 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/MethodValidationTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/MethodValidationTests.java @@ -39,7 +39,6 @@ import org.springframework.validation.Validator; import org.springframework.validation.annotation.Validated; import org.springframework.validation.beanvalidation.LocalValidatorFactoryBean; -import org.springframework.validation.beanvalidation.MethodValidationException; import org.springframework.validation.beanvalidation.ParameterValidationResult; import org.springframework.validation.beanvalidation.SpringValidatorAdapter; import org.springframework.web.bind.MethodArgumentNotValidException; @@ -164,9 +163,9 @@ void modelAttributeWithBindingResultAndRequestHeader() { this.request.addParameter("name", "name=Faustino1234"); this.request.addHeader("myHeader", "123"); - MethodValidationException ex = catchThrowableOfType( + HandlerMethodValidationException ex = catchThrowableOfType( () -> this.handlerAdapter.handle(this.request, this.response, hm), - MethodValidationException.class); + HandlerMethodValidationException.class); assertThat(this.jakartaValidator.getValidationCount()).isEqualTo(1); assertThat(this.jakartaValidator.getMethodValidationCount()).isEqualTo(1); diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ResponseEntityExceptionHandlerTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ResponseEntityExceptionHandlerTests.java index b765e931beef..91d84b474f68 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ResponseEntityExceptionHandlerTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ResponseEntityExceptionHandlerTests.java @@ -43,6 +43,8 @@ import org.springframework.stereotype.Controller; import org.springframework.validation.BindException; import org.springframework.validation.MapBindingResult; +import org.springframework.validation.beanvalidation.MethodValidationException; +import org.springframework.validation.beanvalidation.MethodValidationResult; import org.springframework.web.ErrorResponse; import org.springframework.web.HttpMediaTypeNotAcceptableException; import org.springframework.web.HttpMediaTypeNotSupportedException; @@ -58,6 +60,7 @@ import org.springframework.web.context.request.WebRequest; import org.springframework.web.context.request.async.AsyncRequestTimeoutException; import org.springframework.web.context.support.StaticWebApplicationContext; +import org.springframework.web.method.annotation.HandlerMethodValidationException; import org.springframework.web.multipart.support.MissingServletRequestPartException; import org.springframework.web.servlet.DispatcherServlet; import org.springframework.web.servlet.ModelAndView; @@ -69,6 +72,7 @@ import org.springframework.web.testfixture.servlet.MockServletConfig; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.BDDMockito.mock; /** * Unit tests for {@link ResponseEntityExceptionHandler}. @@ -245,6 +249,16 @@ public void methodArgumentNotValid() throws Exception { new MapBindingResult(Collections.emptyMap(), "name"))); } + @Test + public void handlerMethodValidationException() { + testException(new HandlerMethodValidationException(mock(MethodValidationResult.class))); + } + + @Test + public void methodValidationException() { + testException(new MethodValidationException(mock(MethodValidationResult.class))); + } + @Test public void missingServletRequestPart() { testException(new MissingServletRequestPartException("partName")); @@ -351,6 +365,7 @@ public void controllerAdviceWithNestedExceptionWithinDispatcherServlet() throws private ResponseEntity testException(Exception ex) { try { ResponseEntity entity = this.exceptionHandler.handleException(ex, this.request); + assertThat(entity).isNotNull(); // SPR-9653 if (HttpStatus.INTERNAL_SERVER_ERROR.equals(entity.getStatusCode())) { @@ -383,7 +398,7 @@ public void handleRequest() throws Exception { private static class NestedExceptionThrowingController { @RequestMapping("/") - public void handleRequest() throws Exception { + public void handleRequest() { throw new IllegalStateException(new ServletRequestBindingException("message")); } }