From 41b755e4679afcb43e1da8503236260e2a72f19f Mon Sep 17 00:00:00 2001 From: Renato Mameli Date: Fri, 20 Jun 2025 10:23:17 +0200 Subject: [PATCH] Fix consumes handling for interface @RequestBody Previously, @RequestBody(required = false) annotations declared on interface methods were ignored when resolving the consumes condition. This caused mappings to incorrectly require a request body with a Content-Type such as application/json, even when no body was provided. This change uses AnnotatedMethod to retrieve parameter annotations from both the implementation and its interfaces, ensuring that the required flag is respected and body presence is evaluated correctly. Signed-off-by: Renato Mameli --- .../RequestMappingHandlerMapping.java | 21 ++++--- .../RequestMappingHandlerMappingTests.java | 58 +++++++++++++++++ .../RequestMappingHandlerMapping.java | 21 ++++--- .../RequestMappingHandlerMappingTests.java | 62 +++++++++++++++++++ 4 files changed, 146 insertions(+), 16 deletions(-) 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 90e2873bcfdf..f52d7419811f 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 @@ -19,7 +19,6 @@ import java.lang.annotation.Annotation; import java.lang.reflect.AnnotatedElement; import java.lang.reflect.Method; -import java.lang.reflect.Parameter; import java.util.Collections; import java.util.LinkedHashMap; import java.util.List; @@ -30,7 +29,9 @@ import org.jspecify.annotations.Nullable; import org.springframework.context.EmbeddedValueResolverAware; +import org.springframework.core.MethodParameter; import org.springframework.core.annotation.AnnotatedElementUtils; +import org.springframework.core.annotation.AnnotatedMethod; import org.springframework.core.annotation.MergedAnnotation; import org.springframework.core.annotation.MergedAnnotationPredicates; import org.springframework.core.annotation.MergedAnnotations; @@ -372,13 +373,17 @@ protected void registerHandlerMethod(Object handler, Method method, RequestMappi private void updateConsumesCondition(RequestMappingInfo info, Method method) { ConsumesRequestCondition condition = info.getConsumesCondition(); - if (!condition.isEmpty()) { - for (Parameter parameter : method.getParameters()) { - MergedAnnotation annot = MergedAnnotations.from(parameter).get(RequestBody.class); - if (annot.isPresent()) { - condition.setBodyRequired(annot.getBoolean("required")); - break; - } + 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; } } } 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 25ce9902c1ec..8ad4d6f77aec 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 @@ -323,6 +323,48 @@ void httpExchangeWithCustomHeaders() { .containsExactly("h1=hv1", "!h2"); } + @Test + void requestBodyAnnotationFromInterfaceIsRespected() { + this.handlerMapping.afterPropertiesSet(); + + RequestMappingHandlerMapping mapping = new RequestMappingHandlerMapping(); + mapping.setApplicationContext(new StaticWebApplicationContext()); + mapping.afterPropertiesSet(); + + Class clazz = InterfaceControllerImpl.class; + Method method = ReflectionUtils.findMethod(clazz, "post", Foo.class); + assertThat(method).isNotNull(); + + RequestMappingInfo info = mapping.getMappingForMethod(method, clazz); + assertThat(info).isNotNull(); + + mapping.registerHandlerMethod(new InterfaceControllerImpl(), method, info); + assertThat(info.getConsumesCondition()).isNotNull(); + assertThat(info.getConsumesCondition().isBodyRequired()).isFalse(); + assertThat(info.getConsumesCondition().getConsumableMediaTypes()).containsOnly(MediaType.APPLICATION_JSON); + } + + @Test + void requestBodyAnnotationFromImplementationOverridesInterface() { + this.handlerMapping.afterPropertiesSet(); + + RequestMappingHandlerMapping mapping = new RequestMappingHandlerMapping(); + mapping.setApplicationContext(new StaticWebApplicationContext()); + mapping.afterPropertiesSet(); + + Class clazz = InterfaceControllerImplOverridesRequestBody.class; + Method method = ReflectionUtils.findMethod(clazz, "post", Foo.class); + assertThat(method).isNotNull(); + + RequestMappingInfo info = mapping.getMappingForMethod(method, clazz); + assertThat(info).isNotNull(); + + mapping.registerHandlerMethod(new InterfaceControllerImplOverridesRequestBody(), method, info); + assertThat(info.getConsumesCondition()).isNotNull(); + assertThat(info.getConsumesCondition().isBodyRequired()).isTrue(); + assertThat(info.getConsumesCondition().getConsumableMediaTypes()).containsOnly(MediaType.APPLICATION_JSON); + } + private RequestMappingInfo assertComposedAnnotationMapping(RequestMethod requestMethod) { String methodName = requestMethod.name().toLowerCase(); String path = "/" + methodName; @@ -501,6 +543,22 @@ static class MethodLevelOverriddenHttpExchangeAnnotationsController implements S public void post() {} } + @Controller + @RequestMapping(value = "/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) {} + } @HttpExchange @Target(ElementType.TYPE) 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 f3ef8c65635d..d921665b1280 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 @@ -19,7 +19,6 @@ import java.lang.annotation.Annotation; import java.lang.reflect.AnnotatedElement; import java.lang.reflect.Method; -import java.lang.reflect.Parameter; import java.util.Collections; import java.util.LinkedHashMap; import java.util.List; @@ -31,7 +30,9 @@ import org.jspecify.annotations.Nullable; import org.springframework.context.EmbeddedValueResolverAware; +import org.springframework.core.MethodParameter; import org.springframework.core.annotation.AnnotatedElementUtils; +import org.springframework.core.annotation.AnnotatedMethod; import org.springframework.core.annotation.MergedAnnotation; import org.springframework.core.annotation.MergedAnnotationPredicates; import org.springframework.core.annotation.MergedAnnotations; @@ -410,13 +411,17 @@ protected void registerHandlerMethod(Object handler, Method method, RequestMappi private void updateConsumesCondition(RequestMappingInfo info, Method method) { ConsumesRequestCondition condition = info.getConsumesCondition(); - if (!condition.isEmpty()) { - for (Parameter parameter : method.getParameters()) { - MergedAnnotation annot = MergedAnnotations.from(parameter).get(RequestBody.class); - if (annot.isPresent()) { - condition.setBodyRequired(annot.getBoolean("required")); - break; - } + 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; } } } 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 d1e106da00bc..85ac6ab72cf8 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 @@ -386,6 +386,52 @@ void httpExchangeWithCustomHeaders() throws Exception { .containsExactly("h1=hv1", "!h2"); } + @Test + void requestBodyAnnotationFromInterfaceIsRespected() throws Exception { + RequestMappingHandlerMapping mapping = createMapping(); + + Class controllerClass = InterfaceControllerImpl.class; + Method method = controllerClass.getDeclaredMethod("post", Foo.class); + + RequestMappingInfo info = mapping.getMappingForMethod(method, controllerClass); + assertThat(info).isNotNull(); + + mapping.registerHandlerMethod(new InterfaceControllerImpl(), method, info); + + assertThat(info.getConsumesCondition()).isNotNull(); + assertThat(info.getConsumesCondition().isBodyRequired()).isFalse(); + assertThat(info.getConsumesCondition().getConsumableMediaTypes()).containsOnly(MediaType.APPLICATION_JSON); + + MockHttpServletRequest request = new MockHttpServletRequest("POST", "/controller/postMapping"); + initRequestPath(mapping, request); + + RequestMappingInfo matchingInfo = info.getMatchingCondition(request); + assertThat(matchingInfo).isNotNull(); + } + + @Test + void requestBodyAnnotationFromImplementationOverridesInterface() throws Exception { + RequestMappingHandlerMapping mapping = createMapping(); + + Class controllerClass = InterfaceControllerImplOverridesRequestBody.class; + Method method = controllerClass.getDeclaredMethod("post", Foo.class); + + RequestMappingInfo info = mapping.getMappingForMethod(method, controllerClass); + assertThat(info).isNotNull(); + + mapping.registerHandlerMethod(new InterfaceControllerImplOverridesRequestBody(), method, info); + + assertThat(info.getConsumesCondition()).isNotNull(); + assertThat(info.getConsumesCondition().isBodyRequired()).isTrue(); + assertThat(info.getConsumesCondition().getConsumableMediaTypes()).containsOnly(MediaType.APPLICATION_JSON); + + MockHttpServletRequest request = new MockHttpServletRequest("POST", "/controller/postMapping"); + initRequestPath(mapping, request); + + RequestMappingInfo matchingInfo = info.getMatchingCondition(request); + assertThat(matchingInfo).isNull(); + } + private static RequestMappingHandlerMapping createMapping() { RequestMappingHandlerMapping mapping = new RequestMappingHandlerMapping(); mapping.setApplicationContext(new StaticWebApplicationContext()); @@ -571,6 +617,22 @@ static class MethodLevelOverriddenHttpExchangeAnnotationsController implements S public void post() {} } + @RestController + @RequestMapping(value = "/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) {} + } @HttpExchange @Target(ElementType.TYPE)