Skip to content

Commit 41b755e

Browse files
Renato Mamelirenatomameli
Renato Mameli
authored and
renatomameli
committed
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 <[email protected]>
1 parent a265a13 commit 41b755e

File tree

4 files changed

+146
-16
lines changed

4 files changed

+146
-16
lines changed

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

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@
1919
import java.lang.annotation.Annotation;
2020
import java.lang.reflect.AnnotatedElement;
2121
import java.lang.reflect.Method;
22-
import java.lang.reflect.Parameter;
2322
import java.util.Collections;
2423
import java.util.LinkedHashMap;
2524
import java.util.List;
@@ -30,7 +29,9 @@
3029
import org.jspecify.annotations.Nullable;
3130

3231
import org.springframework.context.EmbeddedValueResolverAware;
32+
import org.springframework.core.MethodParameter;
3333
import org.springframework.core.annotation.AnnotatedElementUtils;
34+
import org.springframework.core.annotation.AnnotatedMethod;
3435
import org.springframework.core.annotation.MergedAnnotation;
3536
import org.springframework.core.annotation.MergedAnnotationPredicates;
3637
import org.springframework.core.annotation.MergedAnnotations;
@@ -372,13 +373,17 @@ protected void registerHandlerMethod(Object handler, Method method, RequestMappi
372373

373374
private void updateConsumesCondition(RequestMappingInfo info, Method method) {
374375
ConsumesRequestCondition condition = info.getConsumesCondition();
375-
if (!condition.isEmpty()) {
376-
for (Parameter parameter : method.getParameters()) {
377-
MergedAnnotation<RequestBody> annot = MergedAnnotations.from(parameter).get(RequestBody.class);
378-
if (annot.isPresent()) {
379-
condition.setBodyRequired(annot.getBoolean("required"));
380-
break;
381-
}
376+
if (condition.isEmpty()) {
377+
return;
378+
}
379+
380+
AnnotatedMethod annotatedMethod = new AnnotatedMethod(method);
381+
382+
for (MethodParameter parameter : annotatedMethod.getMethodParameters()) {
383+
RequestBody requestBody = parameter.getParameterAnnotation(RequestBody.class);
384+
if (requestBody != null) {
385+
condition.setBodyRequired(requestBody.required());
386+
break;
382387
}
383388
}
384389
}

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

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -323,6 +323,48 @@ void httpExchangeWithCustomHeaders() {
323323
.containsExactly("h1=hv1", "!h2");
324324
}
325325

326+
@Test
327+
void requestBodyAnnotationFromInterfaceIsRespected() {
328+
this.handlerMapping.afterPropertiesSet();
329+
330+
RequestMappingHandlerMapping mapping = new RequestMappingHandlerMapping();
331+
mapping.setApplicationContext(new StaticWebApplicationContext());
332+
mapping.afterPropertiesSet();
333+
334+
Class<InterfaceControllerImpl> clazz = InterfaceControllerImpl.class;
335+
Method method = ReflectionUtils.findMethod(clazz, "post", Foo.class);
336+
assertThat(method).isNotNull();
337+
338+
RequestMappingInfo info = mapping.getMappingForMethod(method, clazz);
339+
assertThat(info).isNotNull();
340+
341+
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);
345+
}
346+
347+
@Test
348+
void requestBodyAnnotationFromImplementationOverridesInterface() {
349+
this.handlerMapping.afterPropertiesSet();
350+
351+
RequestMappingHandlerMapping mapping = new RequestMappingHandlerMapping();
352+
mapping.setApplicationContext(new StaticWebApplicationContext());
353+
mapping.afterPropertiesSet();
354+
355+
Class<InterfaceControllerImplOverridesRequestBody> clazz = InterfaceControllerImplOverridesRequestBody.class;
356+
Method method = ReflectionUtils.findMethod(clazz, "post", Foo.class);
357+
assertThat(method).isNotNull();
358+
359+
RequestMappingInfo info = mapping.getMappingForMethod(method, clazz);
360+
assertThat(info).isNotNull();
361+
362+
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);
366+
}
367+
326368
private RequestMappingInfo assertComposedAnnotationMapping(RequestMethod requestMethod) {
327369
String methodName = requestMethod.name().toLowerCase();
328370
String path = "/" + methodName;
@@ -501,6 +543,22 @@ static class MethodLevelOverriddenHttpExchangeAnnotationsController implements S
501543
public void post() {}
502544
}
503545

546+
@Controller
547+
@RequestMapping(value = "/controller", consumes = { "application/json" })
548+
interface InterfaceController {
549+
@PostMapping("/postMapping")
550+
void post(@RequestBody(required = false) Foo foo);
551+
}
552+
553+
static class InterfaceControllerImpl implements InterfaceController {
554+
@Override
555+
public void post(Foo foo) {}
556+
}
557+
558+
static class InterfaceControllerImplOverridesRequestBody implements InterfaceController {
559+
@Override
560+
public void post(@RequestBody(required = true) Foo foo) {}
561+
}
504562

505563
@HttpExchange
506564
@Target(ElementType.TYPE)

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

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@
1919
import java.lang.annotation.Annotation;
2020
import java.lang.reflect.AnnotatedElement;
2121
import java.lang.reflect.Method;
22-
import java.lang.reflect.Parameter;
2322
import java.util.Collections;
2423
import java.util.LinkedHashMap;
2524
import java.util.List;
@@ -31,7 +30,9 @@
3130
import org.jspecify.annotations.Nullable;
3231

3332
import org.springframework.context.EmbeddedValueResolverAware;
33+
import org.springframework.core.MethodParameter;
3434
import org.springframework.core.annotation.AnnotatedElementUtils;
35+
import org.springframework.core.annotation.AnnotatedMethod;
3536
import org.springframework.core.annotation.MergedAnnotation;
3637
import org.springframework.core.annotation.MergedAnnotationPredicates;
3738
import org.springframework.core.annotation.MergedAnnotations;
@@ -410,13 +411,17 @@ protected void registerHandlerMethod(Object handler, Method method, RequestMappi
410411

411412
private void updateConsumesCondition(RequestMappingInfo info, Method method) {
412413
ConsumesRequestCondition condition = info.getConsumesCondition();
413-
if (!condition.isEmpty()) {
414-
for (Parameter parameter : method.getParameters()) {
415-
MergedAnnotation<RequestBody> annot = MergedAnnotations.from(parameter).get(RequestBody.class);
416-
if (annot.isPresent()) {
417-
condition.setBodyRequired(annot.getBoolean("required"));
418-
break;
419-
}
414+
if (condition.isEmpty()) {
415+
return;
416+
}
417+
418+
AnnotatedMethod annotatedMethod = new AnnotatedMethod(method);
419+
420+
for (MethodParameter parameter : annotatedMethod.getMethodParameters()) {
421+
RequestBody requestBody = parameter.getParameterAnnotation(RequestBody.class);
422+
if (requestBody != null) {
423+
condition.setBodyRequired(requestBody.required());
424+
break;
420425
}
421426
}
422427
}

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

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -386,6 +386,52 @@ void httpExchangeWithCustomHeaders() throws Exception {
386386
.containsExactly("h1=hv1", "!h2");
387387
}
388388

389+
@Test
390+
void requestBodyAnnotationFromInterfaceIsRespected() throws Exception {
391+
RequestMappingHandlerMapping mapping = createMapping();
392+
393+
Class<?> controllerClass = InterfaceControllerImpl.class;
394+
Method method = controllerClass.getDeclaredMethod("post", Foo.class);
395+
396+
RequestMappingInfo info = mapping.getMappingForMethod(method, controllerClass);
397+
assertThat(info).isNotNull();
398+
399+
mapping.registerHandlerMethod(new InterfaceControllerImpl(), method, info);
400+
401+
assertThat(info.getConsumesCondition()).isNotNull();
402+
assertThat(info.getConsumesCondition().isBodyRequired()).isFalse();
403+
assertThat(info.getConsumesCondition().getConsumableMediaTypes()).containsOnly(MediaType.APPLICATION_JSON);
404+
405+
MockHttpServletRequest request = new MockHttpServletRequest("POST", "/controller/postMapping");
406+
initRequestPath(mapping, request);
407+
408+
RequestMappingInfo matchingInfo = info.getMatchingCondition(request);
409+
assertThat(matchingInfo).isNotNull();
410+
}
411+
412+
@Test
413+
void requestBodyAnnotationFromImplementationOverridesInterface() throws Exception {
414+
RequestMappingHandlerMapping mapping = createMapping();
415+
416+
Class<?> controllerClass = InterfaceControllerImplOverridesRequestBody.class;
417+
Method method = controllerClass.getDeclaredMethod("post", Foo.class);
418+
419+
RequestMappingInfo info = mapping.getMappingForMethod(method, controllerClass);
420+
assertThat(info).isNotNull();
421+
422+
mapping.registerHandlerMethod(new InterfaceControllerImplOverridesRequestBody(), method, info);
423+
424+
assertThat(info.getConsumesCondition()).isNotNull();
425+
assertThat(info.getConsumesCondition().isBodyRequired()).isTrue();
426+
assertThat(info.getConsumesCondition().getConsumableMediaTypes()).containsOnly(MediaType.APPLICATION_JSON);
427+
428+
MockHttpServletRequest request = new MockHttpServletRequest("POST", "/controller/postMapping");
429+
initRequestPath(mapping, request);
430+
431+
RequestMappingInfo matchingInfo = info.getMatchingCondition(request);
432+
assertThat(matchingInfo).isNull();
433+
}
434+
389435
private static RequestMappingHandlerMapping createMapping() {
390436
RequestMappingHandlerMapping mapping = new RequestMappingHandlerMapping();
391437
mapping.setApplicationContext(new StaticWebApplicationContext());
@@ -571,6 +617,22 @@ static class MethodLevelOverriddenHttpExchangeAnnotationsController implements S
571617
public void post() {}
572618
}
573619

620+
@RestController
621+
@RequestMapping(value = "/controller", consumes = { "application/json" })
622+
interface InterfaceController {
623+
@PostMapping("/postMapping")
624+
void post(@RequestBody(required = false) Foo foo);
625+
}
626+
627+
static class InterfaceControllerImpl implements InterfaceController {
628+
@Override
629+
public void post(Foo foo) {}
630+
}
631+
632+
static class InterfaceControllerImplOverridesRequestBody implements InterfaceController {
633+
@Override
634+
public void post(@RequestBody(required = true) Foo foo) {}
635+
}
574636

575637
@HttpExchange
576638
@Target(ElementType.TYPE)

0 commit comments

Comments
 (0)