diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/AbstractMessageConverterMethodArgumentResolver.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/AbstractMessageConverterMethodArgumentResolver.java index 85eee537f4d..9222449adf9 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/AbstractMessageConverterMethodArgumentResolver.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/AbstractMessageConverterMethodArgumentResolver.java @@ -22,10 +22,12 @@ import java.io.PushbackInputStream; import java.lang.annotation.Annotation; import java.lang.reflect.Type; import java.util.ArrayList; +import java.util.Collection; import java.util.Collections; import java.util.EnumSet; import java.util.LinkedHashSet; import java.util.List; +import java.util.Optional; import java.util.Set; import javax.servlet.http.HttpServletRequest; @@ -59,6 +61,7 @@ import org.springframework.web.method.support.HandlerMethodArgumentResolver; * * @author Arjen Poutsma * @author Rossen Stoyanchev + * @author Juergen Hoeller * @since 3.1 */ public abstract class AbstractMessageConverterMethodArgumentResolver implements HandlerMethodArgumentResolver { @@ -128,17 +131,17 @@ public abstract class AbstractMessageConverterMethodArgumentResolver implements * reading from the given request. * @param the expected type of the argument value to be created * @param webRequest the current request - * @param methodParam the method argument + * @param parameter the method parameter descriptor (may be {@code null}) * @param paramType the type of the argument value to be created * @return the created method argument value * @throws IOException if the reading from the request fails * @throws HttpMediaTypeNotSupportedException if no suitable message converter is found */ - protected Object readWithMessageConverters(NativeWebRequest webRequest, MethodParameter methodParam, + protected Object readWithMessageConverters(NativeWebRequest webRequest, MethodParameter parameter, Type paramType) throws IOException, HttpMediaTypeNotSupportedException, HttpMessageNotReadableException { HttpInputMessage inputMessage = createInputMessage(webRequest); - return readWithMessageConverters(inputMessage, methodParam, paramType); + return readWithMessageConverters(inputMessage, parameter, paramType); } /** @@ -146,7 +149,7 @@ public abstract class AbstractMessageConverterMethodArgumentResolver implements * from the given HttpInputMessage. * @param the expected type of the argument value to be created * @param inputMessage the HTTP input message representing the current request - * @param param the method parameter descriptor (may be {@code null}) + * @param parameter the method parameter descriptor (may be {@code null}) * @param targetType the target type, not necessarily the same as the method * parameter type, e.g. for {@code HttpEntity}. * @return the created method argument value @@ -154,7 +157,7 @@ public abstract class AbstractMessageConverterMethodArgumentResolver implements * @throws HttpMediaTypeNotSupportedException if no suitable message converter is found */ @SuppressWarnings("unchecked") - protected Object readWithMessageConverters(HttpInputMessage inputMessage, MethodParameter param, + protected Object readWithMessageConverters(HttpInputMessage inputMessage, MethodParameter parameter, Type targetType) throws IOException, HttpMediaTypeNotSupportedException, HttpMessageNotReadableException { MediaType contentType; @@ -170,11 +173,11 @@ public abstract class AbstractMessageConverterMethodArgumentResolver implements contentType = MediaType.APPLICATION_OCTET_STREAM; } - Class contextClass = (param != null ? param.getContainingClass() : null); + Class contextClass = (parameter != null ? parameter.getContainingClass() : null); Class targetClass = (targetType instanceof Class ? (Class) targetType : null); if (targetClass == null) { - ResolvableType resolvableType = (param != null ? - ResolvableType.forMethodParameter(param) : ResolvableType.forType(targetType)); + ResolvableType resolvableType = (parameter != null ? + ResolvableType.forMethodParameter(parameter) : ResolvableType.forType(targetType)); targetClass = (Class) resolvableType.resolve(); } @@ -193,13 +196,12 @@ public abstract class AbstractMessageConverterMethodArgumentResolver implements logger.debug("Read [" + targetType + "] as \"" + contentType + "\" with [" + converter + "]"); } if (inputMessage.getBody() != null) { - inputMessage = getAdvice().beforeBodyRead(inputMessage, param, targetType, converterType); + inputMessage = getAdvice().beforeBodyRead(inputMessage, parameter, targetType, converterType); body = genericConverter.read(targetType, contextClass, inputMessage); - body = getAdvice().afterBodyRead(body, inputMessage, param, targetType, converterType); + body = getAdvice().afterBodyRead(body, inputMessage, parameter, targetType, converterType); } else { - body = null; - body = getAdvice().handleEmptyBody(body, inputMessage, param, targetType, converterType); + body = getAdvice().handleEmptyBody(null, inputMessage, parameter, targetType, converterType); } break; } @@ -210,13 +212,12 @@ public abstract class AbstractMessageConverterMethodArgumentResolver implements logger.debug("Read [" + targetType + "] as \"" + contentType + "\" with [" + converter + "]"); } if (inputMessage.getBody() != null) { - inputMessage = getAdvice().beforeBodyRead(inputMessage, param, targetType, converterType); + inputMessage = getAdvice().beforeBodyRead(inputMessage, parameter, targetType, converterType); body = ((HttpMessageConverter) converter).read(targetClass, inputMessage); - body = getAdvice().afterBodyRead(body, inputMessage, param, targetType, converterType); + body = getAdvice().afterBodyRead(body, inputMessage, parameter, targetType, converterType); } else { - body = null; - body = getAdvice().handleEmptyBody(body, inputMessage, param, targetType, converterType); + body = getAdvice().handleEmptyBody(null, inputMessage, parameter, targetType, converterType); } break; } @@ -254,12 +255,12 @@ public abstract class AbstractMessageConverterMethodArgumentResolver implements * Spring's {@link org.springframework.validation.annotation.Validated}, * and custom annotations whose name starts with "Valid". * @param binder the DataBinder to be used - * @param methodParam the method parameter - * @see #isBindExceptionRequired + * @param parameter the method parameter descriptor * @since 4.1.5 + * @see #isBindExceptionRequired */ - protected void validateIfApplicable(WebDataBinder binder, MethodParameter methodParam) { - Annotation[] annotations = methodParam.getParameterAnnotations(); + protected void validateIfApplicable(WebDataBinder binder, MethodParameter parameter) { + Annotation[] annotations = parameter.getParameterAnnotations(); for (Annotation ann : annotations) { Validated validatedAnn = AnnotationUtils.getAnnotation(ann, Validated.class); if (validatedAnn != null || ann.annotationType().getSimpleName().startsWith("Valid")) { @@ -274,17 +275,37 @@ public abstract class AbstractMessageConverterMethodArgumentResolver implements /** * Whether to raise a fatal bind exception on validation errors. * @param binder the data binder used to perform data binding - * @param methodParam the method argument + * @param parameter the method parameter descriptor * @return {@code true} if the next method argument is not of type {@link Errors} * @since 4.1.5 */ - protected boolean isBindExceptionRequired(WebDataBinder binder, MethodParameter methodParam) { - int i = methodParam.getParameterIndex(); - Class[] paramTypes = methodParam.getMethod().getParameterTypes(); + protected boolean isBindExceptionRequired(WebDataBinder binder, MethodParameter parameter) { + int i = parameter.getParameterIndex(); + Class[] paramTypes = parameter.getMethod().getParameterTypes(); boolean hasBindingResult = (paramTypes.length > (i + 1) && Errors.class.isAssignableFrom(paramTypes[i + 1])); return !hasBindingResult; } + /** + * Adapt the given argument against the method parameter, if necessary. + * @param arg the resolved argument + * @param parameter the method parameter descriptor + * @return the adapted argument, or the original resolved argument as-is + * @since 4.3.5 + */ + protected Object adaptArgumentIfNecessary(Object arg, MethodParameter parameter) { + if (parameter.getParameterType() == Optional.class) { + if (arg == null || (arg instanceof Collection && ((Collection) arg).isEmpty()) || + (arg instanceof Object[] && ((Object[]) arg).length == 0)) { + return Optional.empty(); + } + else { + return Optional.of(arg); + } + } + return arg; + } + private static class EmptyBodyCheckingHttpInputMessage implements HttpInputMessage { diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestPartMethodArgumentResolver.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestPartMethodArgumentResolver.java index 4cef39e45b5..a46d3a99f38 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestPartMethodArgumentResolver.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestPartMethodArgumentResolver.java @@ -16,9 +16,7 @@ package org.springframework.web.servlet.mvc.method.annotation; -import java.util.Collection; import java.util.List; -import java.util.Optional; import javax.servlet.http.HttpServletRequest; import org.springframework.core.MethodParameter; @@ -156,17 +154,7 @@ public class RequestPartMethodArgumentResolver extends AbstractMessageConverterM throw new MissingServletRequestPartException(name); } } - if (parameter.getParameterType() == Optional.class) { - if (arg == null || (arg instanceof Collection && ((Collection) arg).isEmpty()) || - (arg instanceof Object[] && ((Object[]) arg).length == 0)) { - arg = Optional.empty(); - } - else { - arg = Optional.of(arg); - } - } - - return arg; + return adaptArgumentIfNecessary(arg, parameter); } private String getPartName(MethodParameter methodParam, RequestPart requestPart) { diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestResponseBodyMethodProcessor.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestResponseBodyMethodProcessor.java index e8de9e85925..2f2438c9798 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestResponseBodyMethodProcessor.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestResponseBodyMethodProcessor.java @@ -19,7 +19,6 @@ package org.springframework.web.servlet.mvc.method.annotation; import java.io.IOException; import java.lang.reflect.Type; import java.util.List; - import javax.servlet.http.HttpServletRequest; import org.springframework.core.Conventions; @@ -55,6 +54,7 @@ import org.springframework.web.servlet.mvc.support.DefaultHandlerExceptionResolv * * @author Arjen Poutsma * @author Rossen Stoyanchev + * @author Juergen Hoeller * @since 3.1 */ public class RequestResponseBodyMethodProcessor extends AbstractMessageConverterMethodProcessor { @@ -124,7 +124,8 @@ public class RequestResponseBodyMethodProcessor extends AbstractMessageConverter public Object resolveArgument(MethodParameter parameter, ModelAndViewContainer mavContainer, NativeWebRequest webRequest, WebDataBinderFactory binderFactory) throws Exception { - Object arg = readWithMessageConverters(webRequest, parameter, parameter.getGenericParameterType()); + parameter = parameter.nestedIfOptional(); + Object arg = readWithMessageConverters(webRequest, parameter, parameter.getNestedGenericParameterType()); String name = Conventions.getVariableNameForParameter(parameter); WebDataBinder binder = binderFactory.createBinder(webRequest, arg, name); @@ -136,28 +137,28 @@ public class RequestResponseBodyMethodProcessor extends AbstractMessageConverter } mavContainer.addAttribute(BindingResult.MODEL_KEY_PREFIX + name, binder.getBindingResult()); - return arg; + return adaptArgumentIfNecessary(arg, parameter); } @Override - protected Object readWithMessageConverters(NativeWebRequest webRequest, MethodParameter methodParam, + protected Object readWithMessageConverters(NativeWebRequest webRequest, MethodParameter parameter, Type paramType) throws IOException, HttpMediaTypeNotSupportedException, HttpMessageNotReadableException { HttpServletRequest servletRequest = webRequest.getNativeRequest(HttpServletRequest.class); ServletServerHttpRequest inputMessage = new ServletServerHttpRequest(servletRequest); - Object arg = readWithMessageConverters(inputMessage, methodParam, paramType); + Object arg = readWithMessageConverters(inputMessage, parameter, paramType); if (arg == null) { - if (checkRequired(methodParam)) { + if (checkRequired(parameter)) { throw new HttpMessageNotReadableException("Required request body is missing: " + - methodParam.getMethod().toGenericString()); + parameter.getMethod().toGenericString()); } } return arg; } - protected boolean checkRequired(MethodParameter methodParam) { - return methodParam.getParameterAnnotation(RequestBody.class).required(); + protected boolean checkRequired(MethodParameter parameter) { + return (parameter.getParameterAnnotation(RequestBody.class).required() && !parameter.isOptional()); } @Override diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/RequestResponseBodyMethodProcessorMockTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/RequestResponseBodyMethodProcessorMockTests.java index 3ab0432b331..4529274a517 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/RequestResponseBodyMethodProcessorMockTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/RequestResponseBodyMethodProcessorMockTests.java @@ -21,6 +21,7 @@ import java.nio.charset.StandardCharsets; import java.util.Arrays; import java.util.Collections; import java.util.List; +import java.util.Optional; import javax.validation.Valid; import javax.validation.constraints.NotNull; @@ -61,6 +62,7 @@ import static org.mockito.BDDMockito.*; * * @author Arjen Poutsma * @author Rossen Stoyanchev + * @author Juergen Hoeller */ public class RequestResponseBodyMethodProcessorMockTests { @@ -74,6 +76,7 @@ public class RequestResponseBodyMethodProcessorMockTests { private MethodParameter paramInt; private MethodParameter paramValidBean; private MethodParameter paramStringNotRequired; + private MethodParameter paramOptionalString; private MethodParameter returnTypeString; private MethodParameter returnTypeInt; private MethodParameter returnTypeStringProduces; @@ -102,12 +105,13 @@ public class RequestResponseBodyMethodProcessorMockTests { Method methodHandle1 = getClass().getMethod("handle1", String.class, Integer.TYPE); paramRequestBodyString = new MethodParameter(methodHandle1, 0); paramInt = new MethodParameter(methodHandle1, 1); + paramValidBean = new MethodParameter(getClass().getMethod("handle2", SimpleBean.class), 0); + paramStringNotRequired = new MethodParameter(getClass().getMethod("handle3", String.class), 0); + paramOptionalString = new MethodParameter(getClass().getMethod("handle4", Optional.class), 0); returnTypeString = new MethodParameter(methodHandle1, -1); - returnTypeInt = new MethodParameter(getClass().getMethod("handle2"), -1); - returnTypeStringProduces = new MethodParameter(getClass().getMethod("handle3"), -1); - returnTypeResource = new MethodParameter(getClass().getMethod("handle6"), -1); - paramValidBean = new MethodParameter(getClass().getMethod("handle4", SimpleBean.class), 0); - paramStringNotRequired = new MethodParameter(getClass().getMethod("handle5", String.class), 0); + returnTypeInt = new MethodParameter(getClass().getMethod("handle5"), -1); + returnTypeStringProduces = new MethodParameter(getClass().getMethod("handle6"), -1); + returnTypeResource = new MethodParameter(getClass().getMethod("handle7"), -1); mavContainer = new ModelAndViewContainer(); @@ -204,9 +208,7 @@ public class RequestResponseBodyMethodProcessorMockTests { processor.resolveArgument(paramRequestBodyString, mavContainer, webRequest, null); } - // SPR-9942 - - @Test(expected = HttpMessageNotReadableException.class) + @Test(expected = HttpMessageNotReadableException.class) // SPR-9942 public void resolveArgumentRequiredNoContent() throws Exception { servletRequest.setContentType(MediaType.TEXT_PLAIN_VALUE); servletRequest.setContent(new byte[0]); @@ -215,6 +217,23 @@ public class RequestResponseBodyMethodProcessorMockTests { assertNull(processor.resolveArgument(paramRequestBodyString, mavContainer, webRequest, new ValidatingBinderFactory())); } + @Test + public void resolveArgumentNotGetRequests() throws Exception { + servletRequest.setMethod("GET"); + servletRequest.setContent(new byte[0]); + given(stringMessageConverter.canRead(String.class, MediaType.APPLICATION_OCTET_STREAM)).willReturn(false); + assertNull(processor.resolveArgument(paramStringNotRequired, mavContainer, webRequest, new ValidatingBinderFactory())); + } + + @Test + public void resolveArgumentNotRequiredWithContent() throws Exception { + servletRequest.setContentType("text/plain"); + servletRequest.setContent("body".getBytes()); + given(stringMessageConverter.canRead(String.class, MediaType.TEXT_PLAIN)).willReturn(true); + given(stringMessageConverter.read(eq(String.class), isA(HttpInputMessage.class))).willReturn("body"); + assertEquals("body", processor.resolveArgument(paramStringNotRequired, mavContainer, webRequest, new ValidatingBinderFactory())); + } + @Test public void resolveArgumentNotRequiredNoContent() throws Exception { servletRequest.setContentType("text/plain"); @@ -223,8 +242,7 @@ public class RequestResponseBodyMethodProcessorMockTests { assertNull(processor.resolveArgument(paramStringNotRequired, mavContainer, webRequest, new ValidatingBinderFactory())); } - // SPR-13417 - @Test + @Test // SPR-13417 public void resolveArgumentNotRequiredNoContentNoContentType() throws Exception { servletRequest.setContent(new byte[0]); given(stringMessageConverter.canRead(String.class, MediaType.TEXT_PLAIN)).willReturn(true); @@ -233,11 +251,28 @@ public class RequestResponseBodyMethodProcessorMockTests { } @Test - public void resolveArgumentNotGetRequests() throws Exception { - servletRequest.setMethod("GET"); + public void resolveArgumentOptionalWithContent() throws Exception { + servletRequest.setContentType("text/plain"); + servletRequest.setContent("body".getBytes()); + given(stringMessageConverter.canRead(String.class, MediaType.TEXT_PLAIN)).willReturn(true); + given(stringMessageConverter.read(eq(String.class), isA(HttpInputMessage.class))).willReturn("body"); + assertEquals(Optional.of("body"), processor.resolveArgument(paramOptionalString, mavContainer, webRequest, new ValidatingBinderFactory())); + } + + @Test + public void resolveArgumentOptionalNoContent() throws Exception { + servletRequest.setContentType("text/plain"); servletRequest.setContent(new byte[0]); + given(stringMessageConverter.canRead(String.class, MediaType.TEXT_PLAIN)).willReturn(true); + assertEquals(Optional.empty(), processor.resolveArgument(paramOptionalString, mavContainer, webRequest, new ValidatingBinderFactory())); + } + + @Test + public void resolveArgumentOptionalNoContentNoContentType() throws Exception { + servletRequest.setContent(new byte[0]); + given(stringMessageConverter.canRead(String.class, MediaType.TEXT_PLAIN)).willReturn(true); given(stringMessageConverter.canRead(String.class, MediaType.APPLICATION_OCTET_STREAM)).willReturn(false); - assertNull(processor.resolveArgument(paramStringNotRequired, mavContainer, webRequest, new ValidatingBinderFactory())); + assertEquals(Optional.empty(), processor.resolveArgument(paramOptionalString, mavContainer, webRequest, new ValidatingBinderFactory())); } @Test @@ -311,9 +346,7 @@ public class RequestResponseBodyMethodProcessorMockTests { assertEquals(200, servletResponse.getStatus()); } - // SPR-9841 - - @Test + @Test // SPR-9841 public void handleReturnValueMediaTypeSuffix() throws Exception { String body = "Foo"; MediaType accepted = MediaType.APPLICATION_XHTML_XML; @@ -339,29 +372,37 @@ public class RequestResponseBodyMethodProcessorMockTests { } @SuppressWarnings("unused") - public int handle2() { - return 42; + public void handle2(@Valid @RequestBody SimpleBean b) { } @SuppressWarnings("unused") - @ResponseBody - public String handle3() { - return null; + public void handle3(@RequestBody(required = false) String s) { } @SuppressWarnings("unused") - public void handle4(@Valid @RequestBody SimpleBean b) { + public void handle4(@RequestBody Optional s) { } @SuppressWarnings("unused") - public void handle5(@RequestBody(required=false) String s) { + public int handle5() { + return 42; } @SuppressWarnings("unused") @ResponseBody - public Resource handle6() {return null;} + public String handle6() { + return null; + } + + @SuppressWarnings("unused") + @ResponseBody + public Resource handle7() { + return null; + } + private final class ValidatingBinderFactory implements WebDataBinderFactory { + @Override public WebDataBinder createBinder(NativeWebRequest webRequest, Object target, String objectName) throws Exception { LocalValidatorFactoryBean validator = new LocalValidatorFactoryBean(); @@ -372,6 +413,7 @@ public class RequestResponseBodyMethodProcessorMockTests { } } + @SuppressWarnings("unused") private static class SimpleBean {