Browse Source

Lazily initialize ProblemDetail for picking up actual status code

Closes gh-35829

(cherry picked from commit 3026f0a49b)
pull/35859/head
Juergen Hoeller 4 weeks ago
parent
commit
59025c5b96
  1. 14
      spring-web/src/main/java/org/springframework/web/bind/ServletRequestBindingException.java
  2. 40
      spring-web/src/test/java/org/springframework/web/ErrorResponseExceptionTests.java
  3. 1
      src/checkstyle/checkstyle-suppressions.xml

14
spring-web/src/main/java/org/springframework/web/bind/ServletRequestBindingException.java

@ -25,8 +25,8 @@ import org.springframework.lang.Nullable;
import org.springframework.web.ErrorResponse; import org.springframework.web.ErrorResponse;
/** /**
* Fatal binding exception, thrown when we want to * Fatal binding exception, thrown when we want to treat binding exceptions
* treat binding exceptions as unrecoverable. * as unrecoverable.
* *
* <p>Extends ServletException for convenient throwing in any Servlet resource * <p>Extends ServletException for convenient throwing in any Servlet resource
* (such as a Filter), and NestedServletException for proper root cause handling * (such as a Filter), and NestedServletException for proper root cause handling
@ -38,13 +38,14 @@ import org.springframework.web.ErrorResponse;
@SuppressWarnings("serial") @SuppressWarnings("serial")
public class ServletRequestBindingException extends ServletException implements ErrorResponse { public class ServletRequestBindingException extends ServletException implements ErrorResponse {
private final ProblemDetail body = ProblemDetail.forStatus(getStatusCode());
private final String messageDetailCode; private final String messageDetailCode;
@Nullable @Nullable
private final Object[] messageDetailArguments; private final Object[] messageDetailArguments;
@Nullable
private ProblemDetail body;
/** /**
* Constructor with a message only. * Constructor with a message only.
@ -108,7 +109,10 @@ public class ServletRequestBindingException extends ServletException implements
} }
@Override @Override
public ProblemDetail getBody() { public synchronized ProblemDetail getBody() {
if (this.body == null) {
this.body = ProblemDetail.forStatus(getStatusCode());
}
return this.body; return this.body;
} }

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

@ -78,7 +78,6 @@ class ErrorResponseExceptionTests {
@Test @Test
void httpMediaTypeNotSupportedException() { void httpMediaTypeNotSupportedException() {
List<MediaType> mediaTypes = List<MediaType> mediaTypes =
Arrays.asList(MediaType.APPLICATION_JSON, MediaType.APPLICATION_CBOR); Arrays.asList(MediaType.APPLICATION_JSON, MediaType.APPLICATION_CBOR);
@ -96,7 +95,6 @@ class ErrorResponseExceptionTests {
@Test @Test
void httpMediaTypeNotSupportedExceptionWithParseError() { void httpMediaTypeNotSupportedExceptionWithParseError() {
ErrorResponse ex = new HttpMediaTypeNotSupportedException( ErrorResponse ex = new HttpMediaTypeNotSupportedException(
"Could not parse Accept header: Invalid mime type \"foo\": does not contain '/'"); "Could not parse Accept header: Invalid mime type \"foo\": does not contain '/'");
@ -109,7 +107,6 @@ class ErrorResponseExceptionTests {
@Test @Test
void httpMediaTypeNotAcceptableException() { void httpMediaTypeNotAcceptableException() {
List<MediaType> mediaTypes = Arrays.asList(MediaType.APPLICATION_JSON, MediaType.APPLICATION_CBOR); List<MediaType> mediaTypes = Arrays.asList(MediaType.APPLICATION_JSON, MediaType.APPLICATION_CBOR);
HttpMediaTypeNotAcceptableException ex = new HttpMediaTypeNotAcceptableException(mediaTypes); HttpMediaTypeNotAcceptableException ex = new HttpMediaTypeNotAcceptableException(mediaTypes);
@ -123,7 +120,6 @@ class ErrorResponseExceptionTests {
@Test @Test
void httpMediaTypeNotAcceptableExceptionWithParseError() { void httpMediaTypeNotAcceptableExceptionWithParseError() {
ErrorResponse ex = new HttpMediaTypeNotAcceptableException( ErrorResponse ex = new HttpMediaTypeNotAcceptableException(
"Could not parse Accept header: Invalid mime type \"foo\": does not contain '/'"); "Could not parse Accept header: Invalid mime type \"foo\": does not contain '/'");
@ -136,7 +132,6 @@ class ErrorResponseExceptionTests {
@Test @Test
void asyncRequestTimeoutException() { void asyncRequestTimeoutException() {
ErrorResponse ex = new AsyncRequestTimeoutException(); ErrorResponse ex = new AsyncRequestTimeoutException();
assertDetailMessageCode(ex, null, null); assertDetailMessageCode(ex, null, null);
@ -148,7 +143,6 @@ class ErrorResponseExceptionTests {
@Test @Test
void httpRequestMethodNotSupportedException() { void httpRequestMethodNotSupportedException() {
HttpRequestMethodNotSupportedException ex = HttpRequestMethodNotSupportedException ex =
new HttpRequestMethodNotSupportedException("PUT", Arrays.asList("GET", "POST")); new HttpRequestMethodNotSupportedException("PUT", Arrays.asList("GET", "POST"));
@ -162,7 +156,6 @@ class ErrorResponseExceptionTests {
@Test @Test
void missingRequestHeaderException() { void missingRequestHeaderException() {
MissingRequestHeaderException ex = new MissingRequestHeaderException("Authorization", this.methodParameter); MissingRequestHeaderException ex = new MissingRequestHeaderException("Authorization", this.methodParameter);
assertStatus(ex, HttpStatus.BAD_REQUEST); assertStatus(ex, HttpStatus.BAD_REQUEST);
@ -174,7 +167,6 @@ class ErrorResponseExceptionTests {
@Test @Test
void missingServletRequestParameterException() { void missingServletRequestParameterException() {
MissingServletRequestParameterException ex = new MissingServletRequestParameterException("query", "String"); MissingServletRequestParameterException ex = new MissingServletRequestParameterException("query", "String");
assertStatus(ex, HttpStatus.BAD_REQUEST); assertStatus(ex, HttpStatus.BAD_REQUEST);
@ -186,10 +178,8 @@ class ErrorResponseExceptionTests {
@Test @Test
void missingMatrixVariableException() { void missingMatrixVariableException() {
MissingMatrixVariableException ex = new MissingMatrixVariableException("region", this.methodParameter); MissingMatrixVariableException ex = new MissingMatrixVariableException("region", this.methodParameter);
assertStatus(ex, HttpStatus.BAD_REQUEST); assertStatus(ex, HttpStatus.BAD_REQUEST);
assertDetail(ex, "Required path parameter 'region' is not present."); assertDetail(ex, "Required path parameter 'region' is not present.");
assertDetailMessageCode(ex, null, new Object[] {ex.getVariableName()}); assertDetailMessageCode(ex, null, new Object[] {ex.getVariableName()});
@ -199,7 +189,6 @@ class ErrorResponseExceptionTests {
@Test @Test
void missingPathVariableException() { void missingPathVariableException() {
MissingPathVariableException ex = new MissingPathVariableException("id", this.methodParameter); MissingPathVariableException ex = new MissingPathVariableException("id", this.methodParameter);
assertStatus(ex, HttpStatus.INTERNAL_SERVER_ERROR); assertStatus(ex, HttpStatus.INTERNAL_SERVER_ERROR);
@ -210,8 +199,18 @@ class ErrorResponseExceptionTests {
} }
@Test @Test
void missingRequestCookieException() { void missingPathVariableExceptionAfterConversion() {
MissingPathVariableException ex = new MissingPathVariableException("id", this.methodParameter, true);
assertStatus(ex, HttpStatus.BAD_REQUEST);
assertDetail(ex, "Required path variable 'id' is not present.");
assertDetailMessageCode(ex, null, new Object[] {ex.getVariableName()});
assertThat(ex.getHeaders().isEmpty()).isTrue();
}
@Test
void missingRequestCookieException() {
MissingRequestCookieException ex = new MissingRequestCookieException("oreo", this.methodParameter); MissingRequestCookieException ex = new MissingRequestCookieException("oreo", this.methodParameter);
assertStatus(ex, HttpStatus.BAD_REQUEST); assertStatus(ex, HttpStatus.BAD_REQUEST);
@ -223,7 +222,6 @@ class ErrorResponseExceptionTests {
@Test @Test
void unsatisfiedServletRequestParameterException() { void unsatisfiedServletRequestParameterException() {
UnsatisfiedServletRequestParameterException ex = new UnsatisfiedServletRequestParameterException( UnsatisfiedServletRequestParameterException ex = new UnsatisfiedServletRequestParameterException(
new String[] { "foo=bar", "bar=baz" }, Collections.singletonMap("q", new String[] {"1"})); new String[] { "foo=bar", "bar=baz" }, Collections.singletonMap("q", new String[] {"1"}));
@ -236,7 +234,6 @@ class ErrorResponseExceptionTests {
@Test @Test
void missingServletRequestPartException() { void missingServletRequestPartException() {
MissingServletRequestPartException ex = new MissingServletRequestPartException("file"); MissingServletRequestPartException ex = new MissingServletRequestPartException("file");
assertStatus(ex, HttpStatus.BAD_REQUEST); assertStatus(ex, HttpStatus.BAD_REQUEST);
@ -248,7 +245,6 @@ class ErrorResponseExceptionTests {
@Test @Test
void methodArgumentNotValidException() { void methodArgumentNotValidException() {
ValidationTestHelper testHelper = new ValidationTestHelper(MethodArgumentNotValidException.class); ValidationTestHelper testHelper = new ValidationTestHelper(MethodArgumentNotValidException.class);
BindingResult result = testHelper.bindingResult(); BindingResult result = testHelper.bindingResult();
@ -280,7 +276,6 @@ class ErrorResponseExceptionTests {
@Test @Test
void unsupportedMediaTypeStatusException() { void unsupportedMediaTypeStatusException() {
List<MediaType> mediaTypes = List<MediaType> mediaTypes =
Arrays.asList(MediaType.APPLICATION_JSON, MediaType.APPLICATION_CBOR); Arrays.asList(MediaType.APPLICATION_JSON, MediaType.APPLICATION_CBOR);
@ -298,7 +293,6 @@ class ErrorResponseExceptionTests {
@Test @Test
void unsupportedMediaTypeStatusExceptionWithParseError() { void unsupportedMediaTypeStatusExceptionWithParseError() {
ErrorResponse ex = new UnsupportedMediaTypeStatusException( ErrorResponse ex = new UnsupportedMediaTypeStatusException(
"Could not parse Accept header: Invalid mime type \"foo\": does not contain '/'"); "Could not parse Accept header: Invalid mime type \"foo\": does not contain '/'");
@ -311,7 +305,6 @@ class ErrorResponseExceptionTests {
@Test @Test
void notAcceptableStatusException() { void notAcceptableStatusException() {
List<MediaType> mediaTypes = Arrays.asList(MediaType.APPLICATION_JSON, MediaType.APPLICATION_CBOR); List<MediaType> mediaTypes = Arrays.asList(MediaType.APPLICATION_JSON, MediaType.APPLICATION_CBOR);
NotAcceptableStatusException ex = new NotAcceptableStatusException(mediaTypes); NotAcceptableStatusException ex = new NotAcceptableStatusException(mediaTypes);
@ -325,7 +318,6 @@ class ErrorResponseExceptionTests {
@Test @Test
void notAcceptableStatusExceptionWithParseError() { void notAcceptableStatusExceptionWithParseError() {
ErrorResponse ex = new NotAcceptableStatusException( ErrorResponse ex = new NotAcceptableStatusException(
"Could not parse Accept header: Invalid mime type \"foo\": does not contain '/'"); "Could not parse Accept header: Invalid mime type \"foo\": does not contain '/'");
@ -338,7 +330,6 @@ class ErrorResponseExceptionTests {
@Test @Test
void serverErrorException() { void serverErrorException() {
ServerErrorException ex = new ServerErrorException("Failure", null); ServerErrorException ex = new ServerErrorException("Failure", null);
assertStatus(ex, HttpStatus.INTERNAL_SERVER_ERROR); assertStatus(ex, HttpStatus.INTERNAL_SERVER_ERROR);
@ -350,7 +341,6 @@ class ErrorResponseExceptionTests {
@Test @Test
void missingRequestValueException() { void missingRequestValueException() {
MissingRequestValueException ex = MissingRequestValueException ex =
new MissingRequestValueException("foo", String.class, "header", this.methodParameter); new MissingRequestValueException("foo", String.class, "header", this.methodParameter);
@ -363,7 +353,6 @@ class ErrorResponseExceptionTests {
@Test @Test
void unsatisfiedRequestParameterException() { void unsatisfiedRequestParameterException() {
UnsatisfiedRequestParameterException ex = UnsatisfiedRequestParameterException ex =
new UnsatisfiedRequestParameterException( new UnsatisfiedRequestParameterException(
Arrays.asList("foo=bar", "bar=baz"), Arrays.asList("foo=bar", "bar=baz"),
@ -378,7 +367,6 @@ class ErrorResponseExceptionTests {
@Test @Test
void webExchangeBindException() { void webExchangeBindException() {
ValidationTestHelper testHelper = new ValidationTestHelper(WebExchangeBindException.class); ValidationTestHelper testHelper = new ValidationTestHelper(WebExchangeBindException.class);
BindingResult result = testHelper.bindingResult(); BindingResult result = testHelper.bindingResult();
@ -393,7 +381,6 @@ class ErrorResponseExceptionTests {
@Test @Test
void methodNotAllowedException() { void methodNotAllowedException() {
List<HttpMethod> supportedMethods = Arrays.asList(HttpMethod.GET, HttpMethod.POST); List<HttpMethod> supportedMethods = Arrays.asList(HttpMethod.GET, HttpMethod.POST);
MethodNotAllowedException ex = new MethodNotAllowedException(HttpMethod.PUT, supportedMethods); MethodNotAllowedException ex = new MethodNotAllowedException(HttpMethod.PUT, supportedMethods);
@ -407,7 +394,6 @@ class ErrorResponseExceptionTests {
@Test @Test
void methodNotAllowedExceptionWithoutSupportedMethods() { void methodNotAllowedExceptionWithoutSupportedMethods() {
MethodNotAllowedException ex = new MethodNotAllowedException(HttpMethod.PUT, Collections.emptyList()); MethodNotAllowedException ex = new MethodNotAllowedException(HttpMethod.PUT, Collections.emptyList());
assertStatus(ex, HttpStatus.METHOD_NOT_ALLOWED); assertStatus(ex, HttpStatus.METHOD_NOT_ALLOWED);
@ -417,9 +403,8 @@ class ErrorResponseExceptionTests {
assertThat(ex.getHeaders()).isEmpty(); assertThat(ex.getHeaders()).isEmpty();
} }
@Test // gh-30300 @Test // gh-30300
void responseStatusException() { void responseStatusException() {
Locale locale = Locale.UK; Locale locale = Locale.UK;
LocaleContextHolder.setLocale(locale); LocaleContextHolder.setLocale(locale);
@ -519,7 +504,6 @@ class ErrorResponseExceptionTests {
assertThat(BindErrorUtils.resolve(errors, this.messageSource, Locale.UK)).hasSize(4) assertThat(BindErrorUtils.resolve(errors, this.messageSource, Locale.UK)).hasSize(4)
.containsValues("Bean A message", "Bean B message", "name is required", "age is below minimum"); .containsValues("Bean A message", "Bean B message", "name is required", "age is below minimum");
} }
} }
} }

1
src/checkstyle/checkstyle-suppressions.xml

@ -128,6 +128,7 @@
<suppress files="PatternParseException" checks="JavadocVariable"/> <suppress files="PatternParseException" checks="JavadocVariable"/>
<suppress files="web[\\/]reactive[\\/]socket[\\/]CloseStatus" checks="JavadocStyle"/> <suppress files="web[\\/]reactive[\\/]socket[\\/]CloseStatus" checks="JavadocStyle"/>
<suppress files="RestClientResponseException" checks="MutableException"/> <suppress files="RestClientResponseException" checks="MutableException"/>
<suppress files="ServletRequestBindingException" checks="MutableException"/>
<!-- spring-webflux --> <!-- spring-webflux -->
<suppress files="src[\\/]test[\\/]java[\\/]org[\\/]springframework[\\/]web[\\/]reactive[\\/]resource[\\/]GzipSupport" checks="IllegalImport" id="bannedJUnitJupiterImports"/> <suppress files="src[\\/]test[\\/]java[\\/]org[\\/]springframework[\\/]web[\\/]reactive[\\/]resource[\\/]GzipSupport" checks="IllegalImport" id="bannedJUnitJupiterImports"/>

Loading…
Cancel
Save