Skip to content

Commit 3026f0a

Browse files
jhoellerjhoeller
committed
Lazily initialize ProblemDetail for picking up actual status code
Closes gh-35829
1 parent 9fe4e77 commit 3026f0a

File tree

3 files changed

+21
-33
lines changed

3 files changed

+21
-33
lines changed

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

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,8 @@
2525
import org.springframework.web.ErrorResponse;
2626

2727
/**
28-
* Fatal binding exception, thrown when we want to
29-
* treat binding exceptions as unrecoverable.
28+
* Fatal binding exception, thrown when we want to treat binding exceptions
29+
* as unrecoverable.
3030
*
3131
* <p>Extends ServletException for convenient throwing in any Servlet resource
3232
* (such as a Filter), and NestedServletException for proper root cause handling
@@ -38,12 +38,12 @@
3838
@SuppressWarnings("serial")
3939
public class ServletRequestBindingException extends ServletException implements ErrorResponse {
4040

41-
private final ProblemDetail body = ProblemDetail.forStatus(getStatusCode());
42-
4341
private final String messageDetailCode;
4442

4543
private final Object @Nullable [] messageDetailArguments;
4644

45+
private @Nullable ProblemDetail body;
46+
4747

4848
/**
4949
* Constructor with a message only.
@@ -107,7 +107,10 @@ public HttpStatusCode getStatusCode() {
107107
}
108108

109109
@Override
110-
public ProblemDetail getBody() {
110+
public synchronized ProblemDetail getBody() {
111+
if (this.body == null) {
112+
this.body = ProblemDetail.forStatus(getStatusCode());
113+
}
111114
return this.body;
112115
}
113116

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

Lines changed: 12 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,6 @@ class ErrorResponseExceptionTests {
7878

7979
@Test
8080
void httpMediaTypeNotSupportedException() {
81-
8281
List<MediaType> mediaTypes =
8382
Arrays.asList(MediaType.APPLICATION_JSON, MediaType.APPLICATION_CBOR);
8483

@@ -96,7 +95,6 @@ void httpMediaTypeNotSupportedException() {
9695

9796
@Test
9897
void httpMediaTypeNotSupportedExceptionWithParseError() {
99-
10098
ErrorResponse ex = new HttpMediaTypeNotSupportedException(
10199
"Could not parse Accept header: Invalid mime type \"foo\": does not contain '/'");
102100

@@ -109,7 +107,6 @@ void httpMediaTypeNotSupportedExceptionWithParseError() {
109107

110108
@Test
111109
void httpMediaTypeNotAcceptableException() {
112-
113110
List<MediaType> mediaTypes = Arrays.asList(MediaType.APPLICATION_JSON, MediaType.APPLICATION_CBOR);
114111
HttpMediaTypeNotAcceptableException ex = new HttpMediaTypeNotAcceptableException(mediaTypes);
115112

@@ -123,7 +120,6 @@ void httpMediaTypeNotAcceptableException() {
123120

124121
@Test
125122
void httpMediaTypeNotAcceptableExceptionWithParseError() {
126-
127123
ErrorResponse ex = new HttpMediaTypeNotAcceptableException(
128124
"Could not parse Accept header: Invalid mime type \"foo\": does not contain '/'");
129125

@@ -136,7 +132,6 @@ void httpMediaTypeNotAcceptableExceptionWithParseError() {
136132

137133
@Test
138134
void asyncRequestTimeoutException() {
139-
140135
ErrorResponse ex = new AsyncRequestTimeoutException();
141136
assertDetailMessageCode(ex, null, null);
142137

@@ -148,7 +143,6 @@ void asyncRequestTimeoutException() {
148143

149144
@Test
150145
void httpRequestMethodNotSupportedException() {
151-
152146
HttpRequestMethodNotSupportedException ex =
153147
new HttpRequestMethodNotSupportedException("PUT", Arrays.asList("GET", "POST"));
154148

@@ -162,7 +156,6 @@ void httpRequestMethodNotSupportedException() {
162156

163157
@Test
164158
void missingRequestHeaderException() {
165-
166159
MissingRequestHeaderException ex = new MissingRequestHeaderException("Authorization", this.methodParameter);
167160

168161
assertStatus(ex, HttpStatus.BAD_REQUEST);
@@ -174,7 +167,6 @@ void missingRequestHeaderException() {
174167

175168
@Test
176169
void missingServletRequestParameterException() {
177-
178170
MissingServletRequestParameterException ex = new MissingServletRequestParameterException("query", "String");
179171

180172
assertStatus(ex, HttpStatus.BAD_REQUEST);
@@ -186,10 +178,8 @@ void missingServletRequestParameterException() {
186178

187179
@Test
188180
void missingMatrixVariableException() {
189-
190181
MissingMatrixVariableException ex = new MissingMatrixVariableException("region", this.methodParameter);
191182

192-
193183
assertStatus(ex, HttpStatus.BAD_REQUEST);
194184
assertDetail(ex, "Required path parameter 'region' is not present.");
195185
assertDetailMessageCode(ex, null, new Object[] {ex.getVariableName()});
@@ -199,7 +189,6 @@ void missingMatrixVariableException() {
199189

200190
@Test
201191
void missingPathVariableException() {
202-
203192
MissingPathVariableException ex = new MissingPathVariableException("id", this.methodParameter);
204193

205194
assertStatus(ex, HttpStatus.INTERNAL_SERVER_ERROR);
@@ -210,8 +199,18 @@ void missingPathVariableException() {
210199
}
211200

212201
@Test
213-
void missingRequestCookieException() {
202+
void missingPathVariableExceptionAfterConversion() {
203+
MissingPathVariableException ex = new MissingPathVariableException("id", this.methodParameter, true);
204+
205+
assertStatus(ex, HttpStatus.BAD_REQUEST);
206+
assertDetail(ex, "Required path variable 'id' is not present.");
207+
assertDetailMessageCode(ex, null, new Object[] {ex.getVariableName()});
208+
209+
assertThat(ex.getHeaders().isEmpty()).isTrue();
210+
}
214211

212+
@Test
213+
void missingRequestCookieException() {
215214
MissingRequestCookieException ex = new MissingRequestCookieException("oreo", this.methodParameter);
216215

217216
assertStatus(ex, HttpStatus.BAD_REQUEST);
@@ -223,7 +222,6 @@ void missingRequestCookieException() {
223222

224223
@Test
225224
void unsatisfiedServletRequestParameterException() {
226-
227225
UnsatisfiedServletRequestParameterException ex = new UnsatisfiedServletRequestParameterException(
228226
new String[] { "foo=bar", "bar=baz" }, Collections.singletonMap("q", new String[] {"1"}));
229227

@@ -236,7 +234,6 @@ void unsatisfiedServletRequestParameterException() {
236234

237235
@Test
238236
void missingServletRequestPartException() {
239-
240237
MissingServletRequestPartException ex = new MissingServletRequestPartException("file");
241238

242239
assertStatus(ex, HttpStatus.BAD_REQUEST);
@@ -248,7 +245,6 @@ void missingServletRequestPartException() {
248245

249246
@Test
250247
void methodArgumentNotValidException() {
251-
252248
ValidationTestHelper testHelper = new ValidationTestHelper(MethodArgumentNotValidException.class);
253249
BindingResult result = testHelper.bindingResult();
254250

@@ -280,7 +276,6 @@ void handlerMethodValidationException() {
280276

281277
@Test
282278
void unsupportedMediaTypeStatusException() {
283-
284279
List<MediaType> mediaTypes =
285280
Arrays.asList(MediaType.APPLICATION_JSON, MediaType.APPLICATION_CBOR);
286281

@@ -298,7 +293,6 @@ void unsupportedMediaTypeStatusException() {
298293

299294
@Test
300295
void unsupportedMediaTypeStatusExceptionWithParseError() {
301-
302296
ErrorResponse ex = new UnsupportedMediaTypeStatusException(
303297
"Could not parse Accept header: Invalid mime type \"foo\": does not contain '/'");
304298

@@ -311,7 +305,6 @@ void unsupportedMediaTypeStatusExceptionWithParseError() {
311305

312306
@Test
313307
void notAcceptableStatusException() {
314-
315308
List<MediaType> mediaTypes = Arrays.asList(MediaType.APPLICATION_JSON, MediaType.APPLICATION_CBOR);
316309
NotAcceptableStatusException ex = new NotAcceptableStatusException(mediaTypes);
317310

@@ -325,7 +318,6 @@ void notAcceptableStatusException() {
325318

326319
@Test
327320
void notAcceptableStatusExceptionWithParseError() {
328-
329321
ErrorResponse ex = new NotAcceptableStatusException(
330322
"Could not parse Accept header: Invalid mime type \"foo\": does not contain '/'");
331323

@@ -338,7 +330,6 @@ void notAcceptableStatusExceptionWithParseError() {
338330

339331
@Test
340332
void serverErrorException() {
341-
342333
ServerErrorException ex = new ServerErrorException("Failure", null);
343334

344335
assertStatus(ex, HttpStatus.INTERNAL_SERVER_ERROR);
@@ -350,7 +341,6 @@ void serverErrorException() {
350341

351342
@Test
352343
void missingRequestValueException() {
353-
354344
MissingRequestValueException ex =
355345
new MissingRequestValueException("foo", String.class, "header", this.methodParameter);
356346

@@ -363,7 +353,6 @@ void missingRequestValueException() {
363353

364354
@Test
365355
void unsatisfiedRequestParameterException() {
366-
367356
UnsatisfiedRequestParameterException ex =
368357
new UnsatisfiedRequestParameterException(
369358
Arrays.asList("foo=bar", "bar=baz"),
@@ -378,7 +367,6 @@ void unsatisfiedRequestParameterException() {
378367

379368
@Test
380369
void webExchangeBindException() {
381-
382370
ValidationTestHelper testHelper = new ValidationTestHelper(WebExchangeBindException.class);
383371
BindingResult result = testHelper.bindingResult();
384372

@@ -393,7 +381,6 @@ void webExchangeBindException() {
393381

394382
@Test
395383
void methodNotAllowedException() {
396-
397384
List<HttpMethod> supportedMethods = Arrays.asList(HttpMethod.GET, HttpMethod.POST);
398385
MethodNotAllowedException ex = new MethodNotAllowedException(HttpMethod.PUT, supportedMethods);
399386

@@ -407,7 +394,6 @@ void methodNotAllowedException() {
407394

408395
@Test
409396
void methodNotAllowedExceptionWithoutSupportedMethods() {
410-
411397
MethodNotAllowedException ex = new MethodNotAllowedException(HttpMethod.PUT, Collections.emptyList());
412398

413399
assertStatus(ex, HttpStatus.METHOD_NOT_ALLOWED);
@@ -417,9 +403,8 @@ void methodNotAllowedExceptionWithoutSupportedMethods() {
417403
assertThat(ex.getHeaders().isEmpty()).isTrue();
418404
}
419405

420-
@Test // gh-30300
406+
@Test // gh-30300
421407
void responseStatusException() {
422-
423408
Locale locale = Locale.UK;
424409
LocaleContextHolder.setLocale(locale);
425410

@@ -519,7 +504,6 @@ private void assertMessages(ErrorResponse ex, List<? extends MessageSourceResolv
519504
assertThat(BindErrorUtils.resolve(errors, this.messageSource, Locale.UK)).hasSize(4)
520505
.containsValues("Bean A message", "Bean B message", "name is required", "age is below minimum");
521506
}
522-
523507
}
524508

525509
}

‎src/checkstyle/checkstyle-suppressions.xml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,7 @@
113113
<suppress files="PatternParseException" checks="JavadocVariable"/>
114114
<suppress files="web[\\/]reactive[\\/]socket[\\/]CloseStatus" checks="JavadocStyle"/>
115115
<suppress files="RestClientResponseException" checks="MutableException"/>
116+
<suppress files="ServletRequestBindingException" checks="MutableException"/>
116117

117118
<!-- spring-webflux -->
118119
<suppress files="src[\\/]test[\\/]java[\\/]org[\\/]springframework[\\/]web[\\/]reactive[\\/]resource[\\/]GzipSupport" checks="IllegalImport" id="bannedJUnitJupiterImports"/>

0 commit comments

Comments
 (0)