Backport fixes in HandlerMethod and sub-classes

Issue: SPR-9747, SPR-9748, SPR-9218, SPR-8946, SPR-9159
Backport Issue: SPR-9622

org.springframework.web/src/main/java/org/springframework/web/method/HandlerMethod.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2011 the original author or authors.
+ * Copyright 2002-2012 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.
@@ -29,13 +29,15 @@ import org.springframework.util.Assert;
 import org.springframework.util.ClassUtils;
 
 /**
- * Encapsulates information about a bean method consisting of a {@linkplain #getMethod() method} and a 
- * {@linkplain #getBean() bean}. Provides convenient access to method parameters, the method return value, 
- * method annotations.
+ * Encapsulates information about a bean method consisting of a
+ * {@linkplain #getMethod() method} and a {@linkplain #getBean() bean}. Provides
+ * convenient access to method parameters, the method return value, method
+ * annotations.
  *
- * <p>The class may be created with a bean instance or with a bean name (e.g. lazy bean, prototype bean).
- * Use {@link #createWithResolvedBean()} to obtain an {@link HandlerMethod} instance with a bean instance
- * initialized through the bean factory.  
+ * <p>The class may be created with a bean instance or with a bean name (e.g. lazy
+ * bean, prototype bean). Use {@link #createWithResolvedBean()} to obtain an
+ * {@link HandlerMethod} instance with a bean instance initialized through the
+ * bean factory.
  *
  * @author Arjen Poutsma
  * @author Rossen Stoyanchev
@@ -44,26 +46,25 @@ import org.springframework.util.ClassUtils;
 public class HandlerMethod {
 
 	/** Logger that is available to subclasses */
-	protected final Log logger = LogFactory.getLog(getClass());
+	protected final Log logger = LogFactory.getLog(HandlerMethod.class);
 
 	private final Object bean;
 
 	private final Method method;
-	
+
 	private final BeanFactory beanFactory;
 
 	private MethodParameter[] parameters;
 
 	private final Method bridgedMethod;
 
+
 	/**
-	 * Constructs a new handler method with the given bean instance and method.
-	 * @param bean the object bean
-	 * @param method the method
+	 * Create an instance from a bean instance and a method.
 	 */
 	public HandlerMethod(Object bean, Method method) {
-		Assert.notNull(bean, "bean must not be null");
-		Assert.notNull(method, "method must not be null");
+		Assert.notNull(bean, "bean is required");
+		Assert.notNull(method, "method is required");
 		this.bean = bean;
 		this.beanFactory = null;
 		this.method = method;
@@ -71,15 +72,12 @@ public class HandlerMethod {
 	}
 
 	/**
-	 * Constructs a new handler method with the given bean instance, method name and parameters.
-	 * @param bean the object bean
-	 * @param methodName the method name
-	 * @param parameterTypes the method parameter types
+	 * Create an instance from a bean instance, method name, and parameter types.
 	 * @throws NoSuchMethodException when the method cannot be found
 	 */
 	public HandlerMethod(Object bean, String methodName, Class<?>... parameterTypes) throws NoSuchMethodException {
-		Assert.notNull(bean, "bean must not be null");
-		Assert.notNull(methodName, "method must not be null");
+		Assert.notNull(bean, "bean is required");
+		Assert.notNull(methodName, "method is required");
 		this.bean = bean;
 		this.beanFactory = null;
 		this.method = bean.getClass().getMethod(methodName, parameterTypes);
@@ -87,24 +85,34 @@ public class HandlerMethod {
 	}
 
 	/**
-	 * Constructs a new handler method with the given bean name and method. The bean name will be lazily 
-	 * initialized when {@link #createWithResolvedBean()} is called.
-	 * @param beanName the bean name
-	 * @param beanFactory the bean factory to use for bean initialization
-	 * @param method the method for the bean
+	 * Create an instance from a bean name, a method, and a {@code BeanFactory}.
+	 * The method {@link #createWithResolvedBean()} may be used later to
+	 * re-create the {@code HandlerMethod} with an initialized the bean.
 	 */
 	public HandlerMethod(String beanName, BeanFactory beanFactory, Method method) {
-		Assert.hasText(beanName, "'beanName' must not be null");
-		Assert.notNull(beanFactory, "'beanFactory' must not be null");
-		Assert.notNull(method, "'method' must not be null");
+		Assert.hasText(beanName, "beanName is required");
+		Assert.notNull(beanFactory, "beanFactory is required");
+		Assert.notNull(method, "method is required");
 		Assert.isTrue(beanFactory.containsBean(beanName),
-				"Bean factory [" + beanFactory + "] does not contain bean " + "with name [" + beanName + "]");
+				"Bean factory [" + beanFactory + "] does not contain bean [" + beanName + "]");
 		this.bean = beanName;
 		this.beanFactory = beanFactory;
 		this.method = method;
 		this.bridgedMethod = BridgeMethodResolver.findBridgedMethod(method);
 	}
 
+	/**
+	 * Create an instance from another {@code HandlerMethod}.
+	 */
+	protected HandlerMethod(HandlerMethod handlerMethod) {
+		Assert.notNull(handlerMethod, "HandlerMethod is required");
+		this.bean = handlerMethod.bean;
+		this.beanFactory = handlerMethod.beanFactory;
+		this.method = handlerMethod.method;
+		this.bridgedMethod = handlerMethod.bridgedMethod;
+		this.parameters = handlerMethod.parameters;
+	}
+
 	/**
 	 * Returns the bean for this handler method.
 	 */
@@ -120,25 +128,22 @@ public class HandlerMethod {
 	}
 
 	/**
-	 * Returns the type of the handler for this handler method. 
+	 * Returns the type of the handler for this handler method.
 	 * Note that if the bean type is a CGLIB-generated class, the original, user-defined class is returned.
 	 */
 	public Class<?> getBeanType() {
-		if (bean instanceof String) {
-			String beanName = (String) bean;
-			return beanFactory.getType(beanName);
-		}
-		else {
-			return ClassUtils.getUserClass(bean.getClass());
-		}
+		Class<?> clazz = (this.bean instanceof String)
+				? this.beanFactory.getType((String) this.bean) : this.bean.getClass();
+
+		return ClassUtils.getUserClass(clazz);
 	}
-	
+
 	/**
 	 * If the bean method is a bridge method, this method returns the bridged (user-defined) method.
 	 * Otherwise it returns the same method as {@link #getMethod()}.
 	 */
 	protected Method getBridgedMethod() {
-		return bridgedMethod;
+		return this.bridgedMethod;
 	}
 
 	/**
@@ -147,20 +152,26 @@ public class HandlerMethod {
 	public MethodParameter[] getMethodParameters() {
 		if (this.parameters == null) {
 			int parameterCount = this.bridgedMethod.getParameterTypes().length;
-			MethodParameter[] p = new MethodParameter[parameterCount];
+			this.parameters = new MethodParameter[parameterCount];
 			for (int i = 0; i < parameterCount; i++) {
-				p[i] = new HandlerMethodParameter(this.bridgedMethod, i);
+				this.parameters[i] = new HandlerMethodParameter(i);
 			}
-			this.parameters = p;
 		}
-		return parameters;
+		return this.parameters;
 	}
 
 	/**
-	 * Returns the method return type, as {@code MethodParameter}.
+	 * Return the HandlerMethod return type.
 	 */
 	public MethodParameter getReturnType() {
-		return new HandlerMethodParameter(this.bridgedMethod, -1);
+		return new HandlerMethodParameter(-1);
+	}
+
+	/**
+	 * Return the actual return value type.
+	 */
+	public MethodParameter getReturnValueType(Object returnValue) {
+		return new ReturnValueMethodParameter(returnValue);
 	}
 
 	/**
@@ -171,8 +182,8 @@ public class HandlerMethod {
 	}
 
 	/**
-	 * Returns a single annotation on the underlying method traversing its super methods if no 
-	 * annotation can be found on the given method itself. 
+	 * Returns a single annotation on the underlying method traversing its super methods if no
+	 * annotation can be found on the given method itself.
 	 * @param annotationType the type of annotation to introspect the method for.
 	 * @return the annotation, or {@code null} if none found
 	 */
@@ -181,7 +192,7 @@ public class HandlerMethod {
 	}
 
 	/**
-	 * If the provided instance contains a bean name rather than an object instance, the bean name is resolved 
+	 * If the provided instance contains a bean name rather than an object instance, the bean name is resolved
 	 * before a {@link HandlerMethod} is created and returned.
 	 */
 	public HandlerMethod createWithResolvedBean() {
@@ -190,9 +201,11 @@ public class HandlerMethod {
 			String beanName = (String) this.bean;
 			handler = this.beanFactory.getBean(beanName);
 		}
-		return new HandlerMethod(handler, method);
+		HandlerMethod handlerMethod = new HandlerMethod(handler, this.method);
+		handlerMethod.parameters = getMethodParameters();
+		return handlerMethod;
 	}
-	
+
 	@Override
 	public boolean equals(Object o) {
 		if (this == o) {
@@ -216,33 +229,41 @@ public class HandlerMethod {
 	}
 
 	/**
-	 * A {@link MethodParameter} that resolves method annotations even when the actual annotations
-	 * are on a bridge method rather than on the current method. Annotations on super types are
-	 * also returned via {@link AnnotationUtils#findAnnotation(Method, Class)}. 
+	 * A MethodParameter with HandlerMethod-specific behavior.
 	 */
 	private class HandlerMethodParameter extends MethodParameter {
-		
-		public HandlerMethodParameter(Method method, int parameterIndex) {
-			super(method, parameterIndex);
+
+		protected HandlerMethodParameter(int index) {
+			super(HandlerMethod.this.bridgedMethod, index);
 		}
 
-		/**
-		 * Return {@link HandlerMethod#getBeanType()} rather than the method's class, which could be
-		 * important for the proper discovery of generic types.
-		 */
 		@Override
 		public Class<?> getDeclaringClass() {
 			return HandlerMethod.this.getBeanType();
 		}
 
-		/**
-		 * Return the method annotation via {@link HandlerMethod#getMethodAnnotation(Class)}, which will find
-		 * the annotation by traversing super-types and handling annotations on bridge methods correctly.
-		 */
 		@Override
 		public <T extends Annotation> T getMethodAnnotation(Class<T> annotationType) {
 			return HandlerMethod.this.getMethodAnnotation(annotationType);
 		}
 	}
 
+	/**
+	 * A MethodParameter for a HandlerMethod return type based on an actual return value.
+	 */
+	private class ReturnValueMethodParameter extends HandlerMethodParameter {
+
+		private final Object returnValue;
+
+		public ReturnValueMethodParameter(Object returnValue) {
+			super(-1);
+			this.returnValue = returnValue;
+		}
+
+		@Override
+		public Class<?> getParameterType() {
+			return (this.returnValue != null) ? this.returnValue.getClass() : super.getParameterType();
+		}
+	}
+
 }

org.springframework.web/src/main/java/org/springframework/web/method/support/HandlerMethodReturnValueHandlerComposite.java

@@ -19,8 +19,6 @@ package org.springframework.web.method.support;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
-import java.util.Map;
-import java.util.concurrent.ConcurrentHashMap;
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
@@ -42,9 +40,6 @@ public class HandlerMethodReturnValueHandlerComposite implements HandlerMethodRe
 	private final List<HandlerMethodReturnValueHandler> returnValueHandlers =
 		new ArrayList<HandlerMethodReturnValueHandler>();
 
-	private final Map<MethodParameter, HandlerMethodReturnValueHandler> returnValueHandlerCache =
-		new ConcurrentHashMap<MethodParameter, HandlerMethodReturnValueHandler>();
-
 	/**
 	 * Return a read-only list with the registered handlers, or an empty list.
 	 */
@@ -78,21 +73,16 @@ public class HandlerMethodReturnValueHandlerComposite implements HandlerMethodRe
 	 * Find a registered {@link HandlerMethodReturnValueHandler} that supports the given return type.
 	 */
 	private HandlerMethodReturnValueHandler getReturnValueHandler(MethodParameter returnType) {
-		HandlerMethodReturnValueHandler result = this.returnValueHandlerCache.get(returnType);
-		if (result == null) {
-			for (HandlerMethodReturnValueHandler returnValueHandler : returnValueHandlers) {
-				if (logger.isTraceEnabled()) {
-					logger.trace("Testing if return value handler [" + returnValueHandler + "] supports [" +
-							returnType.getGenericParameterType() + "]");
-				}
-				if (returnValueHandler.supportsReturnType(returnType)) {
-					result = returnValueHandler;
-					this.returnValueHandlerCache.put(returnType, returnValueHandler);
-					break;
-				}
+		for (HandlerMethodReturnValueHandler returnValueHandler : returnValueHandlers) {
+			if (logger.isTraceEnabled()) {
+				logger.trace("Testing if return value handler [" + returnValueHandler + "] supports [" +
+						returnType.getGenericParameterType() + "]");
+			}
+			if (returnValueHandler.supportsReturnType(returnType)) {
+				return returnValueHandler;
 			}
 		}
-		return result;
+		return null;
 	}
 
 	/**

org.springframework.web/src/main/java/org/springframework/web/method/support/InvocableHandlerMethod.java

@@ -53,15 +53,21 @@ public class InvocableHandlerMethod extends HandlerMethod {
 
 	private ParameterNameDiscoverer parameterNameDiscoverer = new LocalVariableTableParameterNameDiscoverer();
 
+
 	/**
-	 * Constructs a new handler method with the given bean instance and method.
-	 * @param bean the bean instance
-	 * @param method the method
+	 * Creates an instance from the given handler and method.
 	 */
 	public InvocableHandlerMethod(Object bean, Method method) {
 		super(bean, method);
 	}
 
+	/**
+	 * Create an instance from a {@code HandlerMethod}.
+	 */
+	public InvocableHandlerMethod(HandlerMethod handlerMethod) {
+		super(handlerMethod);
+	}
+
 	/**
 	 * Constructs a new handler method with the given bean instance, method name and parameters.
 	 * @param bean the object bean
@@ -106,8 +112,8 @@ public class InvocableHandlerMethod extends HandlerMethod {
 	 * a thrown exception instance. Provided argument values are checked before argument resolvers.
 	 *
 	 * @param request the current request
-	 * @param mavContainer the {@link ModelAndViewContainer} for the current request
-	 * @param providedArgs argument values to try to use without view resolution
+	 * @param mavContainer the ModelAndViewContainer for this request
+	 * @param providedArgs "given" arguments matched by type, not resolved
 	 * @return the raw value returned by the invoked method
 	 * @exception Exception raised if no suitable argument resolver can be found, or the method raised an exception
 	 */
@@ -193,7 +199,7 @@ public class InvocableHandlerMethod extends HandlerMethod {
 	 * Attempt to resolve a method parameter from the list of provided argument values.
 	 */
 	private Object resolveProvidedArgument(MethodParameter parameter, Object... providedArgs) {
-		if (providedArgs == null || parameter.hasParameterAnnotations()) {
+		if (providedArgs == null) {
 			return null;
 		}
 		for (Object providedArg : providedArgs) {

org.springframework.web/src/test/java/org/springframework/web/method/support/InvocableHandlerMethodTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2011 the original author or authors.
+ * Copyright 2002-2012 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.
@@ -36,19 +36,19 @@ import org.springframework.web.context.request.ServletWebRequest;
 
 /**
  * Test fixture for {@link InvocableHandlerMethod} unit tests.
- * 
+ *
  * @author Rossen Stoyanchev
  */
 public class InvocableHandlerMethodTests {
 
-	private InvocableHandlerMethod handleMethod;
+	private InvocableHandlerMethod handlerMethod;
 
 	private NativeWebRequest webRequest;
 
 	@Before
 	public void setUp() throws Exception {
 		Method method = Handler.class.getDeclaredMethod("handle", Integer.class, String.class);
-		this.handleMethod = new InvocableHandlerMethod(new Handler(), method);
+		this.handlerMethod = new InvocableHandlerMethod(new Handler(), method);
 		this.webRequest = new ServletWebRequest(new MockHttpServletRequest(), new MockHttpServletResponse());
 	}
 
@@ -60,14 +60,14 @@ public class InvocableHandlerMethodTests {
 		HandlerMethodArgumentResolverComposite composite = new HandlerMethodArgumentResolverComposite();
 		composite.addResolver(intResolver);
 		composite.addResolver(stringResolver);
-		handleMethod.setHandlerMethodArgumentResolvers(composite);
-		
-		Object returnValue = handleMethod.invokeForRequest(webRequest, null);
-		
+		handlerMethod.setHandlerMethodArgumentResolvers(composite);
+
+		Object returnValue = handlerMethod.invokeForRequest(webRequest, null);
+
 		assertEquals(1, intResolver.getResolvedParameters().size());
 		assertEquals(1, stringResolver.getResolvedParameters().size());
 		assertEquals("99-value", returnValue);
-		
+
 		assertEquals("intArg", intResolver.getResolvedParameters().get(0).getParameterName());
 		assertEquals("stringArg", stringResolver.getResolvedParameters().get(0).getParameterName());
 	}
@@ -80,10 +80,10 @@ public class InvocableHandlerMethodTests {
 		HandlerMethodArgumentResolverComposite composite = new HandlerMethodArgumentResolverComposite();
 		composite.addResolver(intResolver);
 		composite.addResolver(stringResolver);
-		handleMethod.setHandlerMethodArgumentResolvers(composite);
-		
-		Object returnValue = handleMethod.invokeForRequest(webRequest, null);
-		
+		handlerMethod.setHandlerMethodArgumentResolvers(composite);
+
+		Object returnValue = handlerMethod.invokeForRequest(webRequest, null);
+
 		assertEquals(1, intResolver.getResolvedParameters().size());
 		assertEquals(1, stringResolver.getResolvedParameters().size());
 		assertEquals("null-null", returnValue);
@@ -92,7 +92,7 @@ public class InvocableHandlerMethodTests {
 	@Test
 	public void cannotResolveArg() throws Exception {
 		try {
-			handleMethod.invokeForRequest(webRequest, null);
+			handlerMethod.invokeForRequest(webRequest, null);
 			fail("Expected exception");
 		} catch (IllegalStateException ex) {
 			assertTrue(ex.getMessage().contains("No suitable resolver for argument [0] [type=java.lang.Integer]"));
@@ -101,7 +101,7 @@ public class InvocableHandlerMethodTests {
 
 	@Test
 	public void resolveProvidedArg() throws Exception {
-		Object returnValue = handleMethod.invokeForRequest(webRequest, null, 99, "value");
+		Object returnValue = handlerMethod.invokeForRequest(webRequest, null, 99, "value");
 
 		assertEquals(String.class, returnValue.getClass());
 		assertEquals("99-value", returnValue);
@@ -115,21 +115,21 @@ public class InvocableHandlerMethodTests {
 		HandlerMethodArgumentResolverComposite composite = new HandlerMethodArgumentResolverComposite();
 		composite.addResolver(intResolver);
 		composite.addResolver(stringResolver);
-		handleMethod.setHandlerMethodArgumentResolvers(composite);
+		handlerMethod.setHandlerMethodArgumentResolvers(composite);
 
-		Object returnValue = handleMethod.invokeForRequest(webRequest, null, 2, "value2");
+		Object returnValue = handlerMethod.invokeForRequest(webRequest, null, 2, "value2");
 
 		assertEquals("2-value2", returnValue);
 	}
-	
+
 	@Test
 	public void exceptionInResolvingArg() throws Exception {
 		HandlerMethodArgumentResolverComposite composite = new HandlerMethodArgumentResolverComposite();
 		composite.addResolver(new ExceptionRaisingArgumentResolver());
-		handleMethod.setHandlerMethodArgumentResolvers(composite);
-		
+		handlerMethod.setHandlerMethodArgumentResolvers(composite);
+
 		try {
-			handleMethod.invokeForRequest(webRequest, null);
+			handlerMethod.invokeForRequest(webRequest, null);
 			fail("Expected exception");
 		} catch (HttpMessageNotReadableException ex) {
 			// Expected..
@@ -145,10 +145,10 @@ public class InvocableHandlerMethodTests {
 		HandlerMethodArgumentResolverComposite composite = new HandlerMethodArgumentResolverComposite();
 		composite.addResolver(intResolver);
 		composite.addResolver(stringResolver);
-		handleMethod.setHandlerMethodArgumentResolvers(composite);
+		handlerMethod.setHandlerMethodArgumentResolvers(composite);
 
 		try {
-			handleMethod.invokeForRequest(webRequest, null);
+			handlerMethod.invokeForRequest(webRequest, null);
 			fail("Expected exception");
 		} catch (IllegalArgumentException ex) {
 			assertNotNull("Exception not wrapped", ex.getCause());
@@ -200,10 +200,10 @@ public class InvocableHandlerMethodTests {
 		new InvocableHandlerMethod(handler, method).invokeForRequest(webRequest, null);
 		fail("Expected exception");
 	}
-	
+
 	@SuppressWarnings("unused")
 	private static class Handler {
-		
+
 		public String handle(Integer intArg, String stringArg) {
 			return intArg + "-" + stringArg;
 		}
@@ -211,19 +211,19 @@ public class InvocableHandlerMethodTests {
 
 	@SuppressWarnings("unused")
 	private static class ExceptionRaisingHandler {
-		
+
 		private final Throwable t;
 
 		public ExceptionRaisingHandler(Throwable t) {
 			this.t = t;
 		}
-		
+
 		public void raiseException() throws Throwable {
 			throw t;
 		}
-		
+
 	}
-	
+
 	private static class ExceptionRaisingArgumentResolver implements HandlerMethodArgumentResolver {
 
 		public boolean supportsParameter(MethodParameter parameter) {
@@ -235,5 +235,5 @@ public class InvocableHandlerMethodTests {
 			throw new HttpMessageNotReadableException("oops, can't read");
 		}
 	}
-	
+
 }