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 <renatomamel410@gmail.com>
This commit is contained in:
parent
a265a135d5
commit
41b755e467
|
@ -19,7 +19,6 @@ package org.springframework.web.reactive.result.method.annotation;
|
|||
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 java.util.stream.Stream;
|
|||
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 @@ public class RequestMappingHandlerMapping extends RequestMappingInfoHandlerMappi
|
|||
|
||||
private void updateConsumesCondition(RequestMappingInfo info, Method method) {
|
||||
ConsumesRequestCondition condition = info.getConsumesCondition();
|
||||
if (!condition.isEmpty()) {
|
||||
for (Parameter parameter : method.getParameters()) {
|
||||
MergedAnnotation<RequestBody> 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;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
@ -323,6 +323,48 @@ class RequestMappingHandlerMappingTests {
|
|||
.containsExactly("h1=hv1", "!h2");
|
||||
}
|
||||
|
||||
@Test
|
||||
void requestBodyAnnotationFromInterfaceIsRespected() {
|
||||
this.handlerMapping.afterPropertiesSet();
|
||||
|
||||
RequestMappingHandlerMapping mapping = new RequestMappingHandlerMapping();
|
||||
mapping.setApplicationContext(new StaticWebApplicationContext());
|
||||
mapping.afterPropertiesSet();
|
||||
|
||||
Class<InterfaceControllerImpl> 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<InterfaceControllerImplOverridesRequestBody> 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 @@ class RequestMappingHandlerMappingTests {
|
|||
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)
|
||||
|
|
|
@ -19,7 +19,6 @@ package org.springframework.web.servlet.mvc.method.annotation;
|
|||
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 jakarta.servlet.http.HttpServletRequest;
|
|||
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 @@ public class RequestMappingHandlerMapping extends RequestMappingInfoHandlerMappi
|
|||
|
||||
private void updateConsumesCondition(RequestMappingInfo info, Method method) {
|
||||
ConsumesRequestCondition condition = info.getConsumesCondition();
|
||||
if (!condition.isEmpty()) {
|
||||
for (Parameter parameter : method.getParameters()) {
|
||||
MergedAnnotation<RequestBody> 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;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
@ -386,6 +386,52 @@ class RequestMappingHandlerMappingTests {
|
|||
.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 @@ class RequestMappingHandlerMappingTests {
|
|||
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)
|
||||
|
|
Loading…
Reference in New Issue