diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/RequestMappingHandlerMapping.java b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/RequestMappingHandlerMapping.java index 6da373e1eea..fa910868172 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/RequestMappingHandlerMapping.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/RequestMappingHandlerMapping.java @@ -376,17 +376,14 @@ public class RequestMappingHandlerMapping extends RequestMappingInfoHandlerMappi private void updateConsumesCondition(RequestMappingInfo info, Method method) { ConsumesRequestCondition condition = info.getConsumesCondition(); - if (condition.isEmpty()) { - return; - } - - AnnotatedMethod annotatedMethod = new AnnotatedMethod(method); - - for (MethodParameter parameter : annotatedMethod.getMethodParameters()) { - RequestBody requestBody = parameter.getParameterAnnotation(RequestBody.class); - if (requestBody != null) { - condition.setBodyRequired(requestBody.required()); - break; + if (!condition.isEmpty()) { + AnnotatedMethod annotatedMethod = new AnnotatedMethod(method); + for (MethodParameter parameter : annotatedMethod.getMethodParameters()) { + RequestBody requestBody = parameter.getParameterAnnotation(RequestBody.class); + if (requestBody != null) { + condition.setBodyRequired(requestBody.required()); + break; + } } } } diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/RequestMappingHandlerMappingTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/RequestMappingHandlerMappingTests.java index 8ad4d6f77ae..98d118a6132 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/RequestMappingHandlerMappingTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/RequestMappingHandlerMappingTests.java @@ -46,15 +46,20 @@ import org.springframework.web.method.HandlerTypePredicate; import org.springframework.web.reactive.result.condition.ConsumesRequestCondition; import org.springframework.web.reactive.result.condition.MediaTypeExpression; import org.springframework.web.reactive.result.method.RequestMappingInfo; +import org.springframework.web.server.ServerWebExchange; import org.springframework.web.service.annotation.HttpExchange; import org.springframework.web.service.annotation.PostExchange; import org.springframework.web.service.annotation.PutExchange; +import org.springframework.web.testfixture.http.server.reactive.MockServerHttpRequest; +import org.springframework.web.testfixture.server.MockServerWebExchange; import org.springframework.web.util.pattern.PathPattern; import org.springframework.web.util.pattern.PathPatternParser; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatIllegalStateException; import static org.mockito.Mockito.mock; +import static org.springframework.web.bind.annotation.RequestMethod.POST; +import static org.springframework.web.reactive.result.method.RequestMappingInfo.paths; /** * Tests for {@link RequestMappingHandlerMapping}. @@ -110,7 +115,7 @@ class RequestMappingHandlerMappingTests { } @Test // SPR-14988 - void getMappingOverridesConsumesFromTypeLevelAnnotation() { + void postMappingOverridesConsumesFromTypeLevelAnnotation() { RequestMappingInfo requestMappingInfo = assertComposedAnnotationMapping(RequestMethod.POST); ConsumesRequestCondition condition = requestMappingInfo.getConsumesCondition(); @@ -323,10 +328,13 @@ class RequestMappingHandlerMappingTests { .containsExactly("h1=hv1", "!h2"); } - @Test + @Test // gh-35086 void requestBodyAnnotationFromInterfaceIsRespected() { this.handlerMapping.afterPropertiesSet(); + String path = "/controller/postMapping"; + MediaType mediaType = MediaType.APPLICATION_JSON; + RequestMappingHandlerMapping mapping = new RequestMappingHandlerMapping(); mapping.setApplicationContext(new StaticWebApplicationContext()); mapping.afterPropertiesSet(); @@ -338,16 +346,37 @@ class RequestMappingHandlerMappingTests { RequestMappingInfo info = mapping.getMappingForMethod(method, clazz); assertThat(info).isNotNull(); + // Original ConsumesCondition + ConsumesRequestCondition consumesCondition = info.getConsumesCondition(); + assertThat(consumesCondition).isNotNull(); + assertThat(consumesCondition.isBodyRequired()).isTrue(); + assertThat(consumesCondition.getConsumableMediaTypes()).containsOnly(mediaType); + mapping.registerHandlerMethod(new InterfaceControllerImpl(), method, info); - assertThat(info.getConsumesCondition()).isNotNull(); - assertThat(info.getConsumesCondition().isBodyRequired()).isFalse(); - assertThat(info.getConsumesCondition().getConsumableMediaTypes()).containsOnly(MediaType.APPLICATION_JSON); + + // Updated ConsumesCondition + consumesCondition = info.getConsumesCondition(); + assertThat(consumesCondition).isNotNull(); + assertThat(consumesCondition.isBodyRequired()).isFalse(); + assertThat(consumesCondition.getConsumableMediaTypes()).containsOnly(mediaType); + + MockServerHttpRequest request = MockServerHttpRequest.post(path) + .contentType(mediaType).build(); + ServerWebExchange exchange = MockServerWebExchange.from(request); + RequestMappingInfo matchingInfo = info.getMatchingCondition(exchange); + // Since the request has no body AND the required flag is false, the + // ConsumesCondition in the matching condition in an EMPTY_CONDITION. + // In other words, the "consumes" expressions are removed. + assertThat(matchingInfo).isEqualTo(paths(path).methods(POST).build()); } - @Test + @Test // gh-35086 void requestBodyAnnotationFromImplementationOverridesInterface() { this.handlerMapping.afterPropertiesSet(); + String path = "/controller/postMapping"; + MediaType mediaType = MediaType.APPLICATION_JSON; + RequestMappingHandlerMapping mapping = new RequestMappingHandlerMapping(); mapping.setApplicationContext(new StaticWebApplicationContext()); mapping.afterPropertiesSet(); @@ -359,10 +388,25 @@ class RequestMappingHandlerMappingTests { RequestMappingInfo info = mapping.getMappingForMethod(method, clazz); assertThat(info).isNotNull(); + // Original ConsumesCondition + ConsumesRequestCondition consumesCondition = info.getConsumesCondition(); + assertThat(consumesCondition).isNotNull(); + assertThat(consumesCondition.isBodyRequired()).isTrue(); + assertThat(consumesCondition.getConsumableMediaTypes()).containsOnly(mediaType); + mapping.registerHandlerMethod(new InterfaceControllerImplOverridesRequestBody(), method, info); - assertThat(info.getConsumesCondition()).isNotNull(); - assertThat(info.getConsumesCondition().isBodyRequired()).isTrue(); - assertThat(info.getConsumesCondition().getConsumableMediaTypes()).containsOnly(MediaType.APPLICATION_JSON); + + // Updated ConsumesCondition + consumesCondition = info.getConsumesCondition(); + assertThat(consumesCondition).isNotNull(); + assertThat(consumesCondition.isBodyRequired()).isTrue(); + assertThat(consumesCondition.getConsumableMediaTypes()).containsOnly(mediaType); + + MockServerHttpRequest request = MockServerHttpRequest.post(path) + .contentType(mediaType).build(); + ServerWebExchange exchange = MockServerWebExchange.from(request); + RequestMappingInfo matchingInfo = info.getMatchingCondition(exchange); + assertThat(matchingInfo).isEqualTo(paths(path).methods(POST).consumes(mediaType.toString()).build()); } private RequestMappingInfo assertComposedAnnotationMapping(RequestMethod requestMethod) { @@ -544,18 +588,21 @@ class RequestMappingHandlerMappingTests { } @Controller - @RequestMapping(value = "/controller", consumes = { "application/json" }) + @RequestMapping(path = "/controller", consumes = "application/json") interface InterfaceController { + @PostMapping("/postMapping") void post(@RequestBody(required = false) Foo foo); } static class InterfaceControllerImpl implements InterfaceController { + @Override public void post(Foo foo) {} } static class InterfaceControllerImplOverridesRequestBody implements InterfaceController { + @Override public void post(@RequestBody(required = true) Foo foo) {} } diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerMapping.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerMapping.java index 893575321c3..2cc4f4676e3 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerMapping.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerMapping.java @@ -416,17 +416,14 @@ public class RequestMappingHandlerMapping extends RequestMappingInfoHandlerMappi private void updateConsumesCondition(RequestMappingInfo info, Method method) { ConsumesRequestCondition condition = info.getConsumesCondition(); - if (condition.isEmpty()) { - return; - } - - AnnotatedMethod annotatedMethod = new AnnotatedMethod(method); - - for (MethodParameter parameter : annotatedMethod.getMethodParameters()) { - RequestBody requestBody = parameter.getParameterAnnotation(RequestBody.class); - if (requestBody != null) { - condition.setBodyRequired(requestBody.required()); - break; + if (!condition.isEmpty()) { + AnnotatedMethod annotatedMethod = new AnnotatedMethod(method); + for (MethodParameter parameter : annotatedMethod.getMethodParameters()) { + RequestBody requestBody = parameter.getParameterAnnotation(RequestBody.class); + if (requestBody != null) { + condition.setBodyRequired(requestBody.required()); + break; + } } } } diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerMappingTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerMappingTests.java index 85ac6ab72cf..89ab9332567 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerMappingTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerMappingTests.java @@ -60,6 +60,8 @@ import static org.assertj.core.api.Assertions.assertThatIllegalStateException; import static org.junit.jupiter.api.Named.named; import static org.junit.jupiter.params.provider.Arguments.arguments; import static org.mockito.Mockito.mock; +import static org.springframework.web.bind.annotation.RequestMethod.POST; +import static org.springframework.web.servlet.mvc.method.RequestMappingInfo.paths; /** * Tests for {@link RequestMappingHandlerMapping}. @@ -167,7 +169,7 @@ class RequestMappingHandlerMappingTests { } @Test // SPR-14988 - void getMappingOverridesConsumesFromTypeLevelAnnotation() { + void postMappingOverridesConsumesFromTypeLevelAnnotation() { RequestMappingInfo requestMappingInfo = assertComposedAnnotationMapping(RequestMethod.POST); ConsumesRequestCondition condition = requestMappingInfo.getConsumesCondition(); @@ -386,8 +388,11 @@ class RequestMappingHandlerMappingTests { .containsExactly("h1=hv1", "!h2"); } - @Test + @Test // gh-35086 void requestBodyAnnotationFromInterfaceIsRespected() throws Exception { + String path = "/controller/postMapping"; + MediaType mediaType = MediaType.APPLICATION_JSON; + RequestMappingHandlerMapping mapping = createMapping(); Class controllerClass = InterfaceControllerImpl.class; @@ -396,21 +401,36 @@ class RequestMappingHandlerMappingTests { RequestMappingInfo info = mapping.getMappingForMethod(method, controllerClass); assertThat(info).isNotNull(); + // Original ConsumesCondition + ConsumesRequestCondition consumesCondition = info.getConsumesCondition(); + assertThat(consumesCondition).isNotNull(); + assertThat(consumesCondition.isBodyRequired()).isTrue(); + assertThat(consumesCondition.getConsumableMediaTypes()).containsOnly(mediaType); + mapping.registerHandlerMethod(new InterfaceControllerImpl(), method, info); - assertThat(info.getConsumesCondition()).isNotNull(); - assertThat(info.getConsumesCondition().isBodyRequired()).isFalse(); - assertThat(info.getConsumesCondition().getConsumableMediaTypes()).containsOnly(MediaType.APPLICATION_JSON); + // Updated ConsumesCondition + consumesCondition = info.getConsumesCondition(); + assertThat(consumesCondition).isNotNull(); + assertThat(consumesCondition.isBodyRequired()).isFalse(); + assertThat(consumesCondition.getConsumableMediaTypes()).containsOnly(mediaType); - MockHttpServletRequest request = new MockHttpServletRequest("POST", "/controller/postMapping"); + MockHttpServletRequest request = new MockHttpServletRequest("POST", path); + request.setContentType(mediaType.toString()); initRequestPath(mapping, request); RequestMappingInfo matchingInfo = info.getMatchingCondition(request); - assertThat(matchingInfo).isNotNull(); + // Since the request has no body AND the required flag is false, the + // ConsumesCondition in the matching condition in an EMPTY_CONDITION. + // In other words, the "consumes" expressions are removed. + assertThat(matchingInfo).isEqualTo(paths(path).methods(POST).build()); } - @Test + @Test // gh-35086 void requestBodyAnnotationFromImplementationOverridesInterface() throws Exception { + String path = "/controller/postMapping"; + MediaType mediaType = MediaType.APPLICATION_JSON; + RequestMappingHandlerMapping mapping = createMapping(); Class controllerClass = InterfaceControllerImplOverridesRequestBody.class; @@ -419,19 +439,29 @@ class RequestMappingHandlerMappingTests { RequestMappingInfo info = mapping.getMappingForMethod(method, controllerClass); assertThat(info).isNotNull(); + // Original ConsumesCondition + ConsumesRequestCondition consumesCondition = info.getConsumesCondition(); + assertThat(consumesCondition).isNotNull(); + assertThat(consumesCondition.isBodyRequired()).isTrue(); + assertThat(consumesCondition.getConsumableMediaTypes()).containsOnly(mediaType); + mapping.registerHandlerMethod(new InterfaceControllerImplOverridesRequestBody(), method, info); - assertThat(info.getConsumesCondition()).isNotNull(); - assertThat(info.getConsumesCondition().isBodyRequired()).isTrue(); - assertThat(info.getConsumesCondition().getConsumableMediaTypes()).containsOnly(MediaType.APPLICATION_JSON); + // Updated ConsumesCondition + consumesCondition = info.getConsumesCondition(); + assertThat(consumesCondition).isNotNull(); + assertThat(consumesCondition.isBodyRequired()).isTrue(); + assertThat(consumesCondition.getConsumableMediaTypes()).containsOnly(mediaType); - MockHttpServletRequest request = new MockHttpServletRequest("POST", "/controller/postMapping"); + MockHttpServletRequest request = new MockHttpServletRequest("POST", path); + request.setContentType(mediaType.toString()); initRequestPath(mapping, request); RequestMappingInfo matchingInfo = info.getMatchingCondition(request); - assertThat(matchingInfo).isNull(); + assertThat(matchingInfo).isEqualTo(paths(path).methods(POST).consumes(mediaType.toString()).build()); } + private static RequestMappingHandlerMapping createMapping() { RequestMappingHandlerMapping mapping = new RequestMappingHandlerMapping(); mapping.setApplicationContext(new StaticWebApplicationContext()); @@ -618,18 +648,21 @@ class RequestMappingHandlerMappingTests { } @RestController - @RequestMapping(value = "/controller", consumes = { "application/json" }) + @RequestMapping(path = "/controller", consumes = "application/json") interface InterfaceController { + @PostMapping("/postMapping") void post(@RequestBody(required = false) Foo foo); } static class InterfaceControllerImpl implements InterfaceController { + @Override public void post(Foo foo) {} } static class InterfaceControllerImplOverridesRequestBody implements InterfaceController { + @Override public void post(@RequestBody(required = true) Foo foo) {} }