Fixed AbstractMessageConverterMethodArgumentResolver's type variable resolution

Issue: SPR-11225

spring-core/src/main/java/org/springframework/core/ResolvableType.java

@@ -117,10 +117,7 @@ public final class ResolvableType implements Serializable {
 
 
 	/**
-	 * Private constructor used to create a new {@link ResolvableType}.
-	 * @param type the underlying Java type (may only be {@code null} for {@link #NONE})
-	 * @param variableResolver the resolver used for {@link TypeVariable}s (may be {@code null})
-	 * @param componentType an option declared component type for arrays (may be {@code null})
+	 * Private constructor used to create a new {@link ResolvableType} for resolution purposes.
 	 */
 	private ResolvableType(
 			Type type, TypeProvider typeProvider, VariableResolver variableResolver, ResolvableType componentType) {
@@ -132,6 +129,17 @@ public final class ResolvableType implements Serializable {
 		this.resolved = resolveClass();
 	}
 
+	/**
+	 * Private constructor used to create a new {@link ResolvableType} for cache key purposes.
+	 */
+	private ResolvableType(Type type, TypeProvider typeProvider, VariableResolver variableResolver) {
+		this.type = type;
+		this.typeProvider = typeProvider;
+		this.variableResolver = variableResolver;
+		this.componentType = null;
+		this.resolved = null;
+	}
+
 
 	/**
 	 * Return the underling Java {@link Type} being managed. With the exception of
@@ -935,9 +943,21 @@ public final class ResolvableType implements Serializable {
 	 * @see #forMethodParameter(Method, int)
 	 */
 	public static ResolvableType forMethodParameter(MethodParameter methodParameter) {
+		return forMethodParameter(methodParameter, null);
+	}
+
+	/**
+	 * Return a {@link ResolvableType} for the specified {@link MethodParameter},
+	 * overriding the target type to resolve with a specific given type.
+	 * @param methodParameter the source method parameter (must not be {@code null})
+	 * @param targetType the type to resolve (a part of the method parameter's type)
+	 * @return a {@link ResolvableType} for the specified method parameter
+	 * @see #forMethodParameter(Method, int)
+	 */
+	public static ResolvableType forMethodParameter(MethodParameter methodParameter, Type targetType) {
 		Assert.notNull(methodParameter, "MethodParameter must not be null");
 		ResolvableType owner = forType(methodParameter.getContainingClass()).as(methodParameter.getDeclaringClass());
-		return forType(null, new MethodParameterTypeProvider(methodParameter),
+		return forType(targetType, new MethodParameterTypeProvider(methodParameter),
 				owner.asVariableResolver()).getNested(methodParameter.getNestingLevel(),
 				methodParameter.typeIndexesPerLevel);
 	}
@@ -985,8 +1005,8 @@ public final class ResolvableType implements Serializable {
 	}
 
 	/**
-	 * Return a {@link ResolvableType} for the specified {@link Type}. NOTE: The resulting
-	 * {@link ResolvableType} may not be {@link Serializable}.
+	 * Return a {@link ResolvableType} for the specified {@link Type}.
+	 * Note: The resulting {@link ResolvableType} may not be {@link Serializable}.
 	 * @param type the source type or {@code null}
 	 * @return a {@link ResolvableType} for the specified {@link Type}
 	 * @see #forType(Type, ResolvableType)
@@ -997,7 +1017,7 @@ public final class ResolvableType implements Serializable {
 
 	/**
 	 * Return a {@link ResolvableType} for the specified {@link Type} backed by the given
-	 * owner type. NOTE: The resulting {@link ResolvableType} may not be {@link Serializable}.
+	 * owner type. Note: The resulting {@link ResolvableType} may not be {@link Serializable}.
 	 * @param type the source type or {@code null}
 	 * @param owner the owner type used to resolve variables
 	 * @return a {@link ResolvableType} for the specified {@link Type} and owner
@@ -1038,10 +1058,10 @@ public final class ResolvableType implements Serializable {
 			return NONE;
 		}
 		// Check the cache, we may have a ResolvableType that may have already been resolved
-		ResolvableType key = new ResolvableType(type, typeProvider, variableResolver, null);
+		ResolvableType key = new ResolvableType(type, typeProvider, variableResolver);
 		ResolvableType resolvableType = cache.get(key);
 		if (resolvableType == null) {
-			resolvableType = key;
+			resolvableType = new ResolvableType(type, typeProvider, variableResolver, null);
 			cache.put(key, resolvableType);
 		}
 		return resolvableType;

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

@@ -121,14 +121,11 @@ public abstract class AbstractMessageConverterMethodArgumentResolver implements
 		catch (InvalidMediaTypeException ex) {
 			throw new HttpMediaTypeNotSupportedException(ex.getMessage());
 		}
-
 		if (contentType == null) {
 			contentType = MediaType.APPLICATION_OCTET_STREAM;
 		}
 
 		Class<?> contextClass = methodParam.getContainingClass();
-		Class<T> targetClass = (Class<T>) ResolvableType.forType(targetType,
-				ResolvableType.forMethodParameter(methodParam)).resolve();
 
 		for (HttpMessageConverter<?> converter : this.messageConverters) {
 			if (converter instanceof GenericHttpMessageConverter) {
@@ -141,18 +138,18 @@ public abstract class AbstractMessageConverterMethodArgumentResolver implements
 					return genericConverter.read(targetType, contextClass, inputMessage);
 				}
 			}
-			if (targetClass != null) {
-				if (converter.canRead(targetClass, contentType)) {
-					if (logger.isDebugEnabled()) {
-						logger.debug("Reading [" + targetClass.getName() + "] as \"" +
-								contentType + "\" using [" + converter + "]");
-					}
-					return ((HttpMessageConverter<T>) converter).read(targetClass, inputMessage);
+			Class<T> targetClass = (Class<T>)
+					ResolvableType.forMethodParameter(methodParam, targetType).resolve(Object.class);
+			if (converter.canRead(targetClass, contentType)) {
+				if (logger.isDebugEnabled()) {
+					logger.debug("Reading [" + targetClass.getName() + "] as \"" +
+							contentType + "\" using [" + converter + "]");
 				}
+				return ((HttpMessageConverter<T>) converter).read(targetClass, inputMessage);
 			}
 		}
 
-		throw new HttpMediaTypeNotSupportedException(contentType, allSupportedMediaTypes);
+		throw new HttpMediaTypeNotSupportedException(contentType, this.allSupportedMediaTypes);
 	}
 
 	/**

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2012 the original author or authors.
+ * Copyright 2002-2013 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -23,6 +23,9 @@ import java.util.List;
 
 import org.junit.Before;
 import org.junit.Test;
+
+import org.springframework.aop.framework.ProxyFactory;
+import org.springframework.aop.target.SingletonTargetSource;
 import org.springframework.core.MethodParameter;
 import org.springframework.http.MediaType;
 import org.springframework.http.converter.ByteArrayHttpMessageConverter;
@@ -76,9 +79,7 @@ public class RequestResponseBodyMethodProcessorTests {
 
 	@Before
 	public void setUp() throws Exception {
-
-		Method method = getClass().getMethod("handle",
-				List.class, SimpleBean.class, MultiValueMap.class, String.class);
+		Method method = getClass().getMethod("handle", List.class, SimpleBean.class, MultiValueMap.class, String.class);
 
 		paramGenericList = new MethodParameter(method, 0);
 		paramSimpleBean = new MethodParameter(method, 1);
@@ -171,8 +172,7 @@ public class RequestResponseBodyMethodProcessorTests {
 
 	@Test
 	public void resolveArgumentTypeVariable() throws Exception {
-
-		Method method = MySimpleParameterizedController.class.getMethod("handleDto", Identifiable.class);
+		Method method = MyParameterizedController.class.getMethod("handleDto", Identifiable.class);
 		HandlerMethod handlerMethod = new HandlerMethod(new MySimpleParameterizedController(), method);
 		MethodParameter methodParam = handlerMethod.getMethodParameters()[0];
 
@@ -190,6 +190,30 @@ public class RequestResponseBodyMethodProcessorTests {
 		assertEquals("Jad", result.getName());
 	}
 
+	// SPR-11225
+
+	@Test
+	public void resolveArgumentTypeVariableWithNonGenericConverter() throws Exception {
+		Method method = MyParameterizedController.class.getMethod("handleDto", Identifiable.class);
+		HandlerMethod handlerMethod = new HandlerMethod(new MySimpleParameterizedController(), method);
+		MethodParameter methodParam = handlerMethod.getMethodParameters()[0];
+
+		String content = "{\"name\" : \"Jad\"}";
+		this.servletRequest.setContent(content.getBytes("UTF-8"));
+		this.servletRequest.setContentType(MediaType.APPLICATION_JSON_VALUE);
+
+		List<HttpMessageConverter<?>> converters = new ArrayList<HttpMessageConverter<?>>();
+		HttpMessageConverter target = new MappingJackson2HttpMessageConverter();
+		HttpMessageConverter proxy = ProxyFactory.getProxy(HttpMessageConverter.class, new SingletonTargetSource(target));
+		converters.add(proxy);
+		RequestResponseBodyMethodProcessor processor = new RequestResponseBodyMethodProcessor(converters);
+
+		SimpleBean result = (SimpleBean) processor.resolveArgument(methodParam, mavContainer, webRequest, binderFactory);
+
+		assertNotNull(result);
+		assertEquals("Jad", result.getName());
+	}
+
 	// SPR-9160
 
 	@Test
@@ -235,7 +259,6 @@ public class RequestResponseBodyMethodProcessorTests {
 
 	@Test
 	public void supportsReturnTypeResponseBodyOnType() throws Exception {
-
 		Method method = ResponseBodyController.class.getMethod("handle");
 		MethodParameter returnType = new MethodParameter(method, -1);
 
@@ -249,7 +272,6 @@ public class RequestResponseBodyMethodProcessorTests {
 
 	@Test
 	public void supportsReturnTypeRestController() throws Exception {
-
 		Method method = TestRestController.class.getMethod("handle");
 		MethodParameter returnType = new MethodParameter(method, -1);
 
@@ -271,22 +293,31 @@ public class RequestResponseBodyMethodProcessorTests {
 		return null;
 	}
 
+
 	private static abstract class MyParameterizedController<DTO extends Identifiable> {
+
 		@SuppressWarnings("unused")
 		public void handleDto(@RequestBody DTO dto) {}
 	}
 
-	private static class MySimpleParameterizedController extends MyParameterizedController<SimpleBean> { }
+
+	private static class MySimpleParameterizedController extends MyParameterizedController<SimpleBean> {
+	}
+
 
 	private interface Identifiable extends Serializable {
+
 		public Long getId();
+
 		public void setId(Long id);
 	}
 
+
 	@SuppressWarnings({ "serial" })
 	private static class SimpleBean implements Identifiable {
 
 		private Long id;
+
 		private String name;
 
 		@Override
@@ -308,7 +339,9 @@ public class RequestResponseBodyMethodProcessorTests {
 		}
 	}
 
+
 	private final class ValidatingBinderFactory implements WebDataBinderFactory {
+
 		@Override
 		public WebDataBinder createBinder(NativeWebRequest webRequest, Object target, String objectName) throws Exception {
 			LocalValidatorFactoryBean validator = new LocalValidatorFactoryBean();
@@ -319,6 +352,7 @@ public class RequestResponseBodyMethodProcessorTests {
 		}
 	}
 
+
 	@ResponseBody
 	private static class ResponseBodyController {
 
@@ -328,6 +362,7 @@ public class RequestResponseBodyMethodProcessorTests {
 		}
 	}
 
+
 	@RestController
 	private static class TestRestController {
 
@@ -337,4 +372,4 @@ public class RequestResponseBodyMethodProcessorTests {
 		}
 	}
 
-}
+}