Skip to content

Commit b1222cc

Browse files
sbrannensbrannen
committed
Polish contribution
See gh-35086
1 parent b901aff commit b1222cc

File tree

4 files changed

+120
-46
lines changed

4 files changed

+120
-46
lines changed

‎spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/RequestMappingHandlerMapping.java

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -376,17 +376,14 @@ protected void registerHandlerMethod(Object handler, Method method, RequestMappi
376376

377377
private void updateConsumesCondition(RequestMappingInfo info, Method method) {
378378
ConsumesRequestCondition condition = info.getConsumesCondition();
379-
if (condition.isEmpty()) {
380-
return;
381-
}
382-
383-
AnnotatedMethod annotatedMethod = new AnnotatedMethod(method);
384-
385-
for (MethodParameter parameter : annotatedMethod.getMethodParameters()) {
386-
RequestBody requestBody = parameter.getParameterAnnotation(RequestBody.class);
387-
if (requestBody != null) {
388-
condition.setBodyRequired(requestBody.required());
389-
break;
379+
if (!condition.isEmpty()) {
380+
AnnotatedMethod annotatedMethod = new AnnotatedMethod(method);
381+
for (MethodParameter parameter : annotatedMethod.getMethodParameters()) {
382+
RequestBody requestBody = parameter.getParameterAnnotation(RequestBody.class);
383+
if (requestBody != null) {
384+
condition.setBodyRequired(requestBody.required());
385+
break;
386+
}
390387
}
391388
}
392389
}

‎spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/RequestMappingHandlerMappingTests.java

Lines changed: 57 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -46,15 +46,20 @@
4646
import org.springframework.web.reactive.result.condition.ConsumesRequestCondition;
4747
import org.springframework.web.reactive.result.condition.MediaTypeExpression;
4848
import org.springframework.web.reactive.result.method.RequestMappingInfo;
49+
import org.springframework.web.server.ServerWebExchange;
4950
import org.springframework.web.service.annotation.HttpExchange;
5051
import org.springframework.web.service.annotation.PostExchange;
5152
import org.springframework.web.service.annotation.PutExchange;
53+
import org.springframework.web.testfixture.http.server.reactive.MockServerHttpRequest;
54+
import org.springframework.web.testfixture.server.MockServerWebExchange;
5255
import org.springframework.web.util.pattern.PathPattern;
5356
import org.springframework.web.util.pattern.PathPatternParser;
5457

5558
import static org.assertj.core.api.Assertions.assertThat;
5659
import static org.assertj.core.api.Assertions.assertThatIllegalStateException;
5760
import static org.mockito.Mockito.mock;
61+
import static org.springframework.web.bind.annotation.RequestMethod.POST;
62+
import static org.springframework.web.reactive.result.method.RequestMappingInfo.paths;
5863

5964
/**
6065
* Tests for {@link RequestMappingHandlerMapping}.
@@ -110,7 +115,7 @@ void resolveRequestMappingViaComposedAnnotation() {
110115
}
111116

112117
@Test // SPR-14988
113-
void getMappingOverridesConsumesFromTypeLevelAnnotation() {
118+
void postMappingOverridesConsumesFromTypeLevelAnnotation() {
114119
RequestMappingInfo requestMappingInfo = assertComposedAnnotationMapping(RequestMethod.POST);
115120

116121
ConsumesRequestCondition condition = requestMappingInfo.getConsumesCondition();
@@ -323,10 +328,13 @@ void httpExchangeWithCustomHeaders() {
323328
.containsExactly("h1=hv1", "!h2");
324329
}
325330

326-
@Test
331+
@Test // gh-35086
327332
void requestBodyAnnotationFromInterfaceIsRespected() {
328333
this.handlerMapping.afterPropertiesSet();
329334

335+
String path = "/controller/postMapping";
336+
MediaType mediaType = MediaType.APPLICATION_JSON;
337+
330338
RequestMappingHandlerMapping mapping = new RequestMappingHandlerMapping();
331339
mapping.setApplicationContext(new StaticWebApplicationContext());
332340
mapping.afterPropertiesSet();
@@ -338,16 +346,37 @@ void requestBodyAnnotationFromInterfaceIsRespected() {
338346
RequestMappingInfo info = mapping.getMappingForMethod(method, clazz);
339347
assertThat(info).isNotNull();
340348

349+
// Original ConsumesCondition
350+
ConsumesRequestCondition consumesCondition = info.getConsumesCondition();
351+
assertThat(consumesCondition).isNotNull();
352+
assertThat(consumesCondition.isBodyRequired()).isTrue();
353+
assertThat(consumesCondition.getConsumableMediaTypes()).containsOnly(mediaType);
354+
341355
mapping.registerHandlerMethod(new InterfaceControllerImpl(), method, info);
342-
assertThat(info.getConsumesCondition()).isNotNull();
343-
assertThat(info.getConsumesCondition().isBodyRequired()).isFalse();
344-
assertThat(info.getConsumesCondition().getConsumableMediaTypes()).containsOnly(MediaType.APPLICATION_JSON);
356+
357+
// Updated ConsumesCondition
358+
consumesCondition = info.getConsumesCondition();
359+
assertThat(consumesCondition).isNotNull();
360+
assertThat(consumesCondition.isBodyRequired()).isFalse();
361+
assertThat(consumesCondition.getConsumableMediaTypes()).containsOnly(mediaType);
362+
363+
MockServerHttpRequest request = MockServerHttpRequest.post(path)
364+
.contentType(mediaType).build();
365+
ServerWebExchange exchange = MockServerWebExchange.from(request);
366+
RequestMappingInfo matchingInfo = info.getMatchingCondition(exchange);
367+
// Since the request has no body AND the required flag is false, the
368+
// ConsumesCondition in the matching condition in an EMPTY_CONDITION.
369+
// In other words, the "consumes" expressions are removed.
370+
assertThat(matchingInfo).isEqualTo(paths(path).methods(POST).build());
345371
}
346372

347-
@Test
373+
@Test // gh-35086
348374
void requestBodyAnnotationFromImplementationOverridesInterface() {
349375
this.handlerMapping.afterPropertiesSet();
350376

377+
String path = "/controller/postMapping";
378+
MediaType mediaType = MediaType.APPLICATION_JSON;
379+
351380
RequestMappingHandlerMapping mapping = new RequestMappingHandlerMapping();
352381
mapping.setApplicationContext(new StaticWebApplicationContext());
353382
mapping.afterPropertiesSet();
@@ -359,10 +388,25 @@ void requestBodyAnnotationFromImplementationOverridesInterface() {
359388
RequestMappingInfo info = mapping.getMappingForMethod(method, clazz);
360389
assertThat(info).isNotNull();
361390

391+
// Original ConsumesCondition
392+
ConsumesRequestCondition consumesCondition = info.getConsumesCondition();
393+
assertThat(consumesCondition).isNotNull();
394+
assertThat(consumesCondition.isBodyRequired()).isTrue();
395+
assertThat(consumesCondition.getConsumableMediaTypes()).containsOnly(mediaType);
396+
362397
mapping.registerHandlerMethod(new InterfaceControllerImplOverridesRequestBody(), method, info);
363-
assertThat(info.getConsumesCondition()).isNotNull();
364-
assertThat(info.getConsumesCondition().isBodyRequired()).isTrue();
365-
assertThat(info.getConsumesCondition().getConsumableMediaTypes()).containsOnly(MediaType.APPLICATION_JSON);
398+
399+
// Updated ConsumesCondition
400+
consumesCondition = info.getConsumesCondition();
401+
assertThat(consumesCondition).isNotNull();
402+
assertThat(consumesCondition.isBodyRequired()).isTrue();
403+
assertThat(consumesCondition.getConsumableMediaTypes()).containsOnly(mediaType);
404+
405+
MockServerHttpRequest request = MockServerHttpRequest.post(path)
406+
.contentType(mediaType).build();
407+
ServerWebExchange exchange = MockServerWebExchange.from(request);
408+
RequestMappingInfo matchingInfo = info.getMatchingCondition(exchange);
409+
assertThat(matchingInfo).isEqualTo(paths(path).methods(POST).consumes(mediaType.toString()).build());
366410
}
367411

368412
private RequestMappingInfo assertComposedAnnotationMapping(RequestMethod requestMethod) {
@@ -544,18 +588,21 @@ public void post() {}
544588
}
545589

546590
@Controller
547-
@RequestMapping(value = "/controller", consumes = { "application/json" })
591+
@RequestMapping(path = "/controller", consumes = "application/json")
548592
interface InterfaceController {
593+
549594
@PostMapping("/postMapping")
550595
void post(@RequestBody(required = false) Foo foo);
551596
}
552597

553598
static class InterfaceControllerImpl implements InterfaceController {
599+
554600
@Override
555601
public void post(Foo foo) {}
556602
}
557603

558604
static class InterfaceControllerImplOverridesRequestBody implements InterfaceController {
605+
559606
@Override
560607
public void post(@RequestBody(required = true) Foo foo) {}
561608
}

‎spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerMapping.java

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -416,17 +416,14 @@ protected void registerHandlerMethod(Object handler, Method method, RequestMappi
416416

417417
private void updateConsumesCondition(RequestMappingInfo info, Method method) {
418418
ConsumesRequestCondition condition = info.getConsumesCondition();
419-
if (condition.isEmpty()) {
420-
return;
421-
}
422-
423-
AnnotatedMethod annotatedMethod = new AnnotatedMethod(method);
424-
425-
for (MethodParameter parameter : annotatedMethod.getMethodParameters()) {
426-
RequestBody requestBody = parameter.getParameterAnnotation(RequestBody.class);
427-
if (requestBody != null) {
428-
condition.setBodyRequired(requestBody.required());
429-
break;
419+
if (!condition.isEmpty()) {
420+
AnnotatedMethod annotatedMethod = new AnnotatedMethod(method);
421+
for (MethodParameter parameter : annotatedMethod.getMethodParameters()) {
422+
RequestBody requestBody = parameter.getParameterAnnotation(RequestBody.class);
423+
if (requestBody != null) {
424+
condition.setBodyRequired(requestBody.required());
425+
break;
426+
}
430427
}
431428
}
432429
}

‎spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerMappingTests.java

Lines changed: 47 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,8 @@
6060
import static org.junit.jupiter.api.Named.named;
6161
import static org.junit.jupiter.params.provider.Arguments.arguments;
6262
import static org.mockito.Mockito.mock;
63+
import static org.springframework.web.bind.annotation.RequestMethod.POST;
64+
import static org.springframework.web.servlet.mvc.method.RequestMappingInfo.paths;
6365

6466
/**
6567
* Tests for {@link RequestMappingHandlerMapping}.
@@ -167,7 +169,7 @@ void resolveRequestMappingViaComposedAnnotation(RequestMappingHandlerMapping map
167169
}
168170

169171
@Test // SPR-14988
170-
void getMappingOverridesConsumesFromTypeLevelAnnotation() {
172+
void postMappingOverridesConsumesFromTypeLevelAnnotation() {
171173
RequestMappingInfo requestMappingInfo = assertComposedAnnotationMapping(RequestMethod.POST);
172174

173175
ConsumesRequestCondition condition = requestMappingInfo.getConsumesCondition();
@@ -386,8 +388,11 @@ void httpExchangeWithCustomHeaders() throws Exception {
386388
.containsExactly("h1=hv1", "!h2");
387389
}
388390

389-
@Test
391+
@Test // gh-35086
390392
void requestBodyAnnotationFromInterfaceIsRespected() throws Exception {
393+
String path = "/controller/postMapping";
394+
MediaType mediaType = MediaType.APPLICATION_JSON;
395+
391396
RequestMappingHandlerMapping mapping = createMapping();
392397

393398
Class<?> controllerClass = InterfaceControllerImpl.class;
@@ -396,21 +401,36 @@ void requestBodyAnnotationFromInterfaceIsRespected() throws Exception {
396401
RequestMappingInfo info = mapping.getMappingForMethod(method, controllerClass);
397402
assertThat(info).isNotNull();
398403

404+
// Original ConsumesCondition
405+
ConsumesRequestCondition consumesCondition = info.getConsumesCondition();
406+
assertThat(consumesCondition).isNotNull();
407+
assertThat(consumesCondition.isBodyRequired()).isTrue();
408+
assertThat(consumesCondition.getConsumableMediaTypes()).containsOnly(mediaType);
409+
399410
mapping.registerHandlerMethod(new InterfaceControllerImpl(), method, info);
400411

401-
assertThat(info.getConsumesCondition()).isNotNull();
402-
assertThat(info.getConsumesCondition().isBodyRequired()).isFalse();
403-
assertThat(info.getConsumesCondition().getConsumableMediaTypes()).containsOnly(MediaType.APPLICATION_JSON);
412+
// Updated ConsumesCondition
413+
consumesCondition = info.getConsumesCondition();
414+
assertThat(consumesCondition).isNotNull();
415+
assertThat(consumesCondition.isBodyRequired()).isFalse();
416+
assertThat(consumesCondition.getConsumableMediaTypes()).containsOnly(mediaType);
404417

405-
MockHttpServletRequest request = new MockHttpServletRequest("POST", "/controller/postMapping");
418+
MockHttpServletRequest request = new MockHttpServletRequest("POST", path);
419+
request.setContentType(mediaType.toString());
406420
initRequestPath(mapping, request);
407421

408422
RequestMappingInfo matchingInfo = info.getMatchingCondition(request);
409-
assertThat(matchingInfo).isNotNull();
423+
// Since the request has no body AND the required flag is false, the
424+
// ConsumesCondition in the matching condition in an EMPTY_CONDITION.
425+
// In other words, the "consumes" expressions are removed.
426+
assertThat(matchingInfo).isEqualTo(paths(path).methods(POST).build());
410427
}
411428

412-
@Test
429+
@Test // gh-35086
413430
void requestBodyAnnotationFromImplementationOverridesInterface() throws Exception {
431+
String path = "/controller/postMapping";
432+
MediaType mediaType = MediaType.APPLICATION_JSON;
433+
414434
RequestMappingHandlerMapping mapping = createMapping();
415435

416436
Class<?> controllerClass = InterfaceControllerImplOverridesRequestBody.class;
@@ -419,19 +439,29 @@ void requestBodyAnnotationFromImplementationOverridesInterface() throws Exceptio
419439
RequestMappingInfo info = mapping.getMappingForMethod(method, controllerClass);
420440
assertThat(info).isNotNull();
421441

442+
// Original ConsumesCondition
443+
ConsumesRequestCondition consumesCondition = info.getConsumesCondition();
444+
assertThat(consumesCondition).isNotNull();
445+
assertThat(consumesCondition.isBodyRequired()).isTrue();
446+
assertThat(consumesCondition.getConsumableMediaTypes()).containsOnly(mediaType);
447+
422448
mapping.registerHandlerMethod(new InterfaceControllerImplOverridesRequestBody(), method, info);
423449

424-
assertThat(info.getConsumesCondition()).isNotNull();
425-
assertThat(info.getConsumesCondition().isBodyRequired()).isTrue();
426-
assertThat(info.getConsumesCondition().getConsumableMediaTypes()).containsOnly(MediaType.APPLICATION_JSON);
450+
// Updated ConsumesCondition
451+
consumesCondition = info.getConsumesCondition();
452+
assertThat(consumesCondition).isNotNull();
453+
assertThat(consumesCondition.isBodyRequired()).isTrue();
454+
assertThat(consumesCondition.getConsumableMediaTypes()).containsOnly(mediaType);
427455

428-
MockHttpServletRequest request = new MockHttpServletRequest("POST", "/controller/postMapping");
456+
MockHttpServletRequest request = new MockHttpServletRequest("POST", path);
457+
request.setContentType(mediaType.toString());
429458
initRequestPath(mapping, request);
430459

431460
RequestMappingInfo matchingInfo = info.getMatchingCondition(request);
432-
assertThat(matchingInfo).isNull();
461+
assertThat(matchingInfo).isEqualTo(paths(path).methods(POST).consumes(mediaType.toString()).build());
433462
}
434463

464+
435465
private static RequestMappingHandlerMapping createMapping() {
436466
RequestMappingHandlerMapping mapping = new RequestMappingHandlerMapping();
437467
mapping.setApplicationContext(new StaticWebApplicationContext());
@@ -618,18 +648,21 @@ public void post() {}
618648
}
619649

620650
@RestController
621-
@RequestMapping(value = "/controller", consumes = { "application/json" })
651+
@RequestMapping(path = "/controller", consumes = "application/json")
622652
interface InterfaceController {
653+
623654
@PostMapping("/postMapping")
624655
void post(@RequestBody(required = false) Foo foo);
625656
}
626657

627658
static class InterfaceControllerImpl implements InterfaceController {
659+
628660
@Override
629661
public void post(Foo foo) {}
630662
}
631663

632664
static class InterfaceControllerImplOverridesRequestBody implements InterfaceController {
665+
633666
@Override
634667
public void post(@RequestBody(required = true) Foo foo) {}
635668
}

0 commit comments

Comments
 (0)