Fix issue with generic @RequestBody arguments

The original commit c9b7b1 ensured the ability to read parameterized
type @RequestBody arguments via GenericHttpMessageConverter (e.g.
application/json and List<String>). However, it also affected the
ability to read @RequestBody arguments that happen are parameterized
but aren't treated as such (e.g. application/x-www-form-urlencoded and
MultiValueMap<String, String>). This commit corrects the issue.

Issue: SPR-9570

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

@@ -17,6 +17,9 @@
 package org.springframework.web.servlet.mvc.method.annotation;
 
 import java.io.IOException;
+import java.lang.reflect.Array;
+import java.lang.reflect.GenericArrayType;
+import java.lang.reflect.ParameterizedType;
 import java.lang.reflect.Type;
 import java.util.ArrayList;
 import java.util.Collections;
@@ -107,15 +110,15 @@ public abstract class AbstractMessageConverterMethodArgumentResolver implements
 	 * @throws HttpMediaTypeNotSupportedException if no suitable message converter is found
 	 */
 	@SuppressWarnings("unchecked")
-	protected <T> Object readWithMessageConverters(HttpInputMessage inputMessage, MethodParameter methodParam,
-			Type paramType) throws IOException, HttpMediaTypeNotSupportedException {
+	protected <T> Object readWithMessageConverters(HttpInputMessage inputMessage,
+			MethodParameter methodParam, Type paramType) throws IOException, HttpMediaTypeNotSupportedException {
 
 				MediaType contentType = inputMessage.getHeaders().getContentType();
 				if (contentType == null) {
 					contentType = MediaType.APPLICATION_OCTET_STREAM;
 				}
 
-				Class<T> paramClass = (paramType instanceof Class) ? (Class) paramType : null;
+				Class<T> paramClass = getParamClass(paramType);
 
 				for (HttpMessageConverter<?> messageConverter : this.messageConverters) {
 					if (messageConverter instanceof GenericHttpMessageConverter) {
@@ -142,6 +145,26 @@ public abstract class AbstractMessageConverterMethodArgumentResolver implements
 				throw new HttpMediaTypeNotSupportedException(contentType, allSupportedMediaTypes);
 			}
 
+	private Class getParamClass(Type paramType) {
+		if (paramType instanceof Class) {
+			return (Class) paramType;
+		}
+		else if (paramType instanceof GenericArrayType) {
+			Type componentType = ((GenericArrayType) paramType).getGenericComponentType();
+			if (componentType instanceof Class) {
+				// Surely, there should be a nicer way to determine the array type
+				return Array.newInstance((Class<?>) componentType, 0).getClass();
+			}
+		}
+		else if (paramType instanceof ParameterizedType) {
+			ParameterizedType parameterizedType = (ParameterizedType) paramType;
+			if (parameterizedType.getRawType() instanceof Class) {
+				return (Class) parameterizedType.getRawType();
+			}
+		}
+		return null;
+	}
+
 	/**
 	 * Creates a new {@link HttpInputMessage} from the given {@link NativeWebRequest}.
 	 *

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

@@ -17,8 +17,6 @@
 package org.springframework.web.servlet.mvc.method.annotation;
 
 import java.io.IOException;
-import java.lang.reflect.Array;
-import java.lang.reflect.GenericArrayType;
 import java.lang.reflect.ParameterizedType;
 import java.lang.reflect.Type;
 import java.util.List;
@@ -89,23 +87,11 @@ public class HttpEntityMethodProcessor extends AbstractMessageConverterMethodPro
 		Assert.isAssignable(HttpEntity.class, parameter.getParameterType());
 		ParameterizedType type = (ParameterizedType) parameter.getGenericParameterType();
 		if (type.getActualTypeArguments().length == 1) {
-			Type typeArgument = type.getActualTypeArguments()[0];
-			if (typeArgument instanceof Class) {
-				return (Class<?>) typeArgument;
-			}
-			else if (typeArgument instanceof GenericArrayType) {
-				Type componentType = ((GenericArrayType) typeArgument).getGenericComponentType();
-				if (componentType instanceof Class) {
-					// Surely, there should be a nicer way to determine the array type
-					return Array.newInstance((Class<?>) componentType, 0).getClass();
-				}
-			}
-			else if (typeArgument instanceof ParameterizedType) {
-				return typeArgument;
-			}
+			return type.getActualTypeArguments()[0];
 		}
-		throw new IllegalArgumentException("HttpEntity parameter (" + parameter.getParameterName() + ") "
-				+ "in method " + parameter.getMethod() + "is not parameterized");
+		throw new IllegalArgumentException("HttpEntity parameter ("
+				+ parameter.getParameterName() + ") in method " + parameter.getMethod()
+				+ " is not parameterized or has more than one parameter");
 	}
 
 	public void handleReturnValue(

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

@@ -190,10 +190,9 @@ public class RequestResponseBodyMethodProcessorMockTests {
 	}
 
 	@Test(expected = HttpMediaTypeNotSupportedException.class)
-	public void resolveArgumentNotReadable() throws Exception {
+	public void resolveArgumentCannotRead() throws Exception {
 		MediaType contentType = MediaType.TEXT_PLAIN;
 		servletRequest.addHeader("Content-Type", contentType.toString());
-		servletRequest.setContent(new byte[] {});
 
 		expect(messageConverter.canRead(String.class, contentType)).andReturn(false);
 		replay(messageConverter);
@@ -201,19 +200,25 @@ public class RequestResponseBodyMethodProcessorMockTests {
 		processor.resolveArgument(paramRequestBodyString, mavContainer, webRequest, null);
 	}
 
-	@Test(expected = HttpMediaTypeNotSupportedException.class)
+	@Test
 	public void resolveArgumentNoContentType() throws Exception {
-		servletRequest.setContent(new byte[] {});
-		processor.resolveArgument(paramRequestBodyString, mavContainer, webRequest, null);
-	}
-
-	@Test(expected = HttpMessageNotReadableException.class)
-	public void resolveArgumentRequiredNoContent() throws Exception {
-		processor.resolveArgument(paramRequestBodyString, mavContainer, webRequest, null);
+		expect(messageConverter.canRead(String.class, MediaType.APPLICATION_OCTET_STREAM)).andReturn(false);
+		replay(messageConverter);
+		try {
+			processor.resolveArgument(paramRequestBodyString, mavContainer, webRequest, null);
+			fail("Expected exception");
+		}
+		catch (HttpMediaTypeNotSupportedException ex) {
+		}
+		verify(messageConverter);
 	}
 
 	@Test
 	public void resolveArgumentNotRequiredNoContent() throws Exception {
+		servletRequest.setContent(null);
+		assertNull(processor.resolveArgument(paramStringNotRequired, mavContainer, webRequest, new ValidatingBinderFactory()));
+
+		servletRequest.setContent(new byte[0]);
 		assertNull(processor.resolveArgument(paramStringNotRequired, mavContainer, webRequest, new ValidatingBinderFactory()));
 	}
 

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

@@ -21,17 +21,21 @@ import static org.junit.Assert.assertNotNull;
 
 import java.lang.reflect.Method;
 import java.util.ArrayList;
+import java.util.Collection;
 import java.util.List;
 
 import org.junit.Before;
 import org.junit.Test;
 import org.springframework.core.MethodParameter;
+import org.springframework.http.MediaType;
 import org.springframework.http.converter.ByteArrayHttpMessageConverter;
 import org.springframework.http.converter.HttpMessageConverter;
 import org.springframework.http.converter.StringHttpMessageConverter;
 import org.springframework.http.converter.json.MappingJackson2HttpMessageConverter;
+import org.springframework.http.converter.xml.XmlAwareFormHttpMessageConverter;
 import org.springframework.mock.web.MockHttpServletRequest;
 import org.springframework.mock.web.MockHttpServletResponse;
+import org.springframework.util.MultiValueMap;
 import org.springframework.validation.beanvalidation.LocalValidatorFactoryBean;
 import org.springframework.web.bind.WebDataBinder;
 import org.springframework.web.bind.annotation.RequestBody;
@@ -52,6 +56,8 @@ public class RequestResponseBodyMethodProcessorTests {
 
 	private MethodParameter paramGenericList;
 	private MethodParameter paramSimpleBean;
+	private MethodParameter paramMultiValueMap;
+	private MethodParameter paramString;
 	private MethodParameter returnTypeString;
 
 	private ModelAndViewContainer mavContainer;
@@ -65,9 +71,13 @@ public class RequestResponseBodyMethodProcessorTests {
 	@Before
 	public void setUp() throws Exception {
 
-		Method method = getClass().getMethod("handle", List.class, SimpleBean.class);
+		Method method = getClass().getMethod("handle",
+				List.class, SimpleBean.class, MultiValueMap.class, String.class);
+
 		paramGenericList = new MethodParameter(method, 0);
 		paramSimpleBean = new MethodParameter(method, 1);
+		paramMultiValueMap = new MethodParameter(method, 2);
+		paramString = new MethodParameter(method, 3);
 		returnTypeString = new MethodParameter(method, -1);
 
 		mavContainer = new ModelAndViewContainer();
@@ -79,10 +89,10 @@ public class RequestResponseBodyMethodProcessorTests {
 
 
 	@Test
-	public void resolveGenericArgument() throws Exception {
+	public void resolveArgumentParameterizedType() throws Exception {
 		String content = "[{\"name\" : \"Jad\"}, {\"name\" : \"Robert\"}]";
 		this.servletRequest.setContent(content.getBytes("UTF-8"));
-		this.servletRequest.setContentType("application/json");
+		this.servletRequest.setContentType(MediaType.APPLICATION_JSON_VALUE);
 
 		List<HttpMessageConverter<?>> converters = new ArrayList<HttpMessageConverter<?>>();
 		converters.add(new MappingJackson2HttpMessageConverter());
@@ -98,7 +108,26 @@ public class RequestResponseBodyMethodProcessorTests {
 	}
 
 	@Test
-	public void resolveArgument() throws Exception {
+	public void resolveArgumentRawTypeFromParameterizedType() throws Exception {
+		String content = "fruit=apple&vegetable=kale";
+		this.servletRequest.setContent(content.getBytes("UTF-8"));
+		this.servletRequest.setContentType(MediaType.APPLICATION_FORM_URLENCODED_VALUE);
+
+		List<HttpMessageConverter<?>> converters = new ArrayList<HttpMessageConverter<?>>();
+		converters.add(new XmlAwareFormHttpMessageConverter());
+		RequestResponseBodyMethodProcessor processor = new RequestResponseBodyMethodProcessor(converters);
+
+		@SuppressWarnings("unchecked")
+		MultiValueMap<String, String> result = (MultiValueMap<String, String>) processor.resolveArgument(
+				paramMultiValueMap, mavContainer, webRequest, new ValidatingBinderFactory());
+
+		assertNotNull(result);
+		assertEquals("apple", result.getFirst("fruit"));
+		assertEquals("kale", result.getFirst("vegetable"));
+	}
+
+	@Test
+	public void resolveArgumentClassJson() throws Exception {
 		String content = "{\"name\" : \"Jad\"}";
 		this.servletRequest.setContent(content.getBytes("UTF-8"));
 		this.servletRequest.setContentType("application/json");
@@ -114,6 +143,23 @@ public class RequestResponseBodyMethodProcessorTests {
 		assertEquals("Jad", result.getName());
 	}
 
+	@Test
+	public void resolveArgumentClassString() throws Exception {
+		String content = "foobarbaz";
+		this.servletRequest.setContent(content.getBytes("UTF-8"));
+		this.servletRequest.setContentType("application/json");
+
+		List<HttpMessageConverter<?>> converters = new ArrayList<HttpMessageConverter<?>>();
+		converters.add(new StringHttpMessageConverter());
+		RequestResponseBodyMethodProcessor processor = new RequestResponseBodyMethodProcessor(converters);
+
+		String result = (String) processor.resolveArgument(
+				paramString, mavContainer, webRequest, new ValidatingBinderFactory());
+
+		assertNotNull(result);
+		assertEquals("foobarbaz", result);
+	}
+
 	// SPR-9160
 
 	@Test
@@ -158,7 +204,12 @@ public class RequestResponseBodyMethodProcessorTests {
 	}
 
 
-	public String handle(@RequestBody List<SimpleBean> list, @RequestBody SimpleBean simpleBean) {
+	public String handle(
+			@RequestBody List<SimpleBean> list,
+			@RequestBody SimpleBean simpleBean,
+			@RequestBody MultiValueMap<String, String> multiValueMap,
+			@RequestBody String string) {
+
 		return null;
 	}