Use the type of the actual return value in @MVC

The new @MVC support classes select a HandlerMethodArgumentResolver
and a HandlerMethodReturnValueHandler statically, i.e. based on
the signature of the method, which means that a controller method
can't declare a more general return type like Object but actually
return a more specific one, e.g.  String vs RedirectView, and
expect the right handler to be used.

The fix ensures that a HandlerMethodReturnValueHandler is selected
based on the actual return value type, which is something that was
supported with the old @MVC support classes. One consequence
of the change is the selected HandlerMethodReturnValueHandler can
no longer be cached but that matches the behavior of the old
@MVC support classes.

Issues: SPR-9218

spring-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,13 @@ 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, 
+ * 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.  
+ * initialized through the bean factory.
  *
  * @author Arjen Poutsma
  * @author Rossen Stoyanchev
@@ -49,7 +49,7 @@ public class HandlerMethod {
 	private final Object bean;
 
 	private final Method method;
-	
+
 	private final BeanFactory beanFactory;
 
 	private MethodParameter[] parameters;
@@ -87,7 +87,7 @@ public class HandlerMethod {
 	}
 
 	/**
-	 * Constructs a new handler method with the given bean name and method. The bean name will be lazily 
+	 * 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
@@ -120,7 +120,7 @@ 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() {
@@ -132,7 +132,7 @@ public class HandlerMethod {
 			return ClassUtils.getUserClass(bean.getClass());
 		}
 	}
-	
+
 	/**
 	 * 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()}.
@@ -149,7 +149,7 @@ public class HandlerMethod {
 			int parameterCount = this.bridgedMethod.getParameterTypes().length;
 			MethodParameter[] p = new MethodParameter[parameterCount];
 			for (int i = 0; i < parameterCount; i++) {
-				p[i] = new HandlerMethodParameter(this.bridgedMethod, i);
+				p[i] = new HandlerMethodParameter(i);
 			}
 			this.parameters = p;
 		}
@@ -157,10 +157,17 @@ public class HandlerMethod {
 	}
 
 	/**
-	 * 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 +178,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 +188,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() {
@@ -192,7 +199,7 @@ public class HandlerMethod {
 		}
 		return new HandlerMethod(handler, method);
 	}
-	
+
 	@Override
 	public boolean equals(Object o) {
 		if (this == o) {
@@ -216,33 +223,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();
+		}
+	}
+
 }

spring-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;
 	}
 
 	/**

spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ExceptionHandlerExceptionResolver.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.

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

@@ -107,7 +107,7 @@ public class ServletInvocableHandlerMethod extends InvocableHandlerMethod {
 		mavContainer.setRequestHandled(false);
 
 		try {
-			returnValueHandlers.handleReturnValue(returnValue, getReturnType(), mavContainer, request);
+			returnValueHandlers.handleReturnValue(returnValue, getReturnValueType(returnValue), mavContainer, request);
 		} catch (Exception ex) {
 			if (logger.isTraceEnabled()) {
 				logger.trace(getReturnValueHandlingErrorMessage("Error handling return value", returnValue), ex);

spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ServletInvocableHandlerMethodTests.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.
@@ -15,9 +15,10 @@
  */
 
 package org.springframework.web.servlet.mvc.method.annotation;
-import static org.junit.Assert.*;
 import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
 
 import java.lang.reflect.Method;
 
@@ -30,17 +31,20 @@ import org.springframework.http.HttpStatus;
 import org.springframework.http.converter.HttpMessageNotWritableException;
 import org.springframework.mock.web.MockHttpServletRequest;
 import org.springframework.mock.web.MockHttpServletResponse;
+import org.springframework.web.bind.annotation.RequestParam;
 import org.springframework.web.bind.annotation.ResponseStatus;
 import org.springframework.web.context.request.NativeWebRequest;
 import org.springframework.web.context.request.ServletWebRequest;
+import org.springframework.web.method.annotation.RequestParamMethodArgumentResolver;
 import org.springframework.web.method.support.HandlerMethodArgumentResolverComposite;
 import org.springframework.web.method.support.HandlerMethodReturnValueHandler;
 import org.springframework.web.method.support.HandlerMethodReturnValueHandlerComposite;
 import org.springframework.web.method.support.ModelAndViewContainer;
+import org.springframework.web.servlet.view.RedirectView;
 
 /**
  * Test fixture with {@link ServletInvocableHandlerMethod}.
- * 
+ *
  * @author Rossen Stoyanchev
  */
 public class ServletInvocableHandlerMethodTests {
@@ -53,6 +57,8 @@ public class ServletInvocableHandlerMethodTests {
 
 	private ServletWebRequest webRequest;
 
+	private MockHttpServletRequest request;
+
 	private MockHttpServletResponse response;
 
 	@Before
@@ -60,8 +66,9 @@ public class ServletInvocableHandlerMethodTests {
 		returnValueHandlers = new HandlerMethodReturnValueHandlerComposite();
 		argumentResolvers = new HandlerMethodArgumentResolverComposite();
 		mavContainer = new ModelAndViewContainer();
+		request = new MockHttpServletRequest();
 		response = new MockHttpServletResponse();
-		webRequest = new ServletWebRequest(new MockHttpServletRequest(), response);
+		webRequest = new ServletWebRequest(request, response);
 	}
 
 	@Test
@@ -92,29 +99,45 @@ public class ServletInvocableHandlerMethodTests {
 		webRequest.getNativeRequest(MockHttpServletRequest.class).addHeader("If-Modified-Since", 10 * 1000 * 1000);
 		int lastModifiedTimestamp = 1000 * 1000;
 		webRequest.checkNotModified(lastModifiedTimestamp);
-		
+
 		ServletInvocableHandlerMethod handlerMethod = getHandlerMethod("notModified");
 		handlerMethod.invokeAndHandle(webRequest, mavContainer);
 
 		assertTrue("Null return value + 'not modified' request should result in 'request handled'",
 				mavContainer.isRequestHandled());
 	}
-	
-	@Test
+
+	@Test(expected=HttpMessageNotWritableException.class)
 	public void exceptionWhileHandlingReturnValue() throws Exception {
 		returnValueHandlers.addHandler(new ExceptionRaisingReturnValueHandler());
 
 		ServletInvocableHandlerMethod handlerMethod = getHandlerMethod("handle");
-		try {
-			handlerMethod.invokeAndHandle(webRequest, mavContainer);
-			fail("Expected exception");
-		} catch (HttpMessageNotWritableException ex) {
-			// Expected..
-			// Allow HandlerMethodArgumentResolver exceptions to propagate..
-		}
+		handlerMethod.invokeAndHandle(webRequest, mavContainer);
+		fail("Expected exception");
 	}
 
-	private ServletInvocableHandlerMethod getHandlerMethod(String methodName, Class<?>... argTypes) 
+	@Test
+	public void dynamicReturnValue() throws Exception {
+		argumentResolvers.addResolver(new RequestParamMethodArgumentResolver(null, false));
+		returnValueHandlers.addHandler(new ViewMethodReturnValueHandler());
+		returnValueHandlers.addHandler(new ViewNameMethodReturnValueHandler());
+
+		// Invoke without a request parameter (String return value)
+		ServletInvocableHandlerMethod handlerMethod = getHandlerMethod("dynamicReturnValue", String.class);
+		handlerMethod.invokeAndHandle(webRequest, mavContainer);
+
+		assertNotNull(mavContainer.getView());
+		assertEquals(RedirectView.class, mavContainer.getView().getClass());
+
+		// Invoke with a request parameter (RedirectView return value)
+		request.setParameter("param", "value");
+		handlerMethod.invokeAndHandle(webRequest, mavContainer);
+
+		assertEquals("view", mavContainer.getViewName());
+	}
+
+
+	private ServletInvocableHandlerMethod getHandlerMethod(String methodName, Class<?>... argTypes)
 			throws NoSuchMethodException {
 		Method method = Handler.class.getDeclaredMethod(methodName, argTypes);
 		ServletInvocableHandlerMethod handlerMethod = new ServletInvocableHandlerMethod(new Handler(), method);
@@ -133,13 +156,16 @@ public class ServletInvocableHandlerMethodTests {
 		@ResponseStatus(value = HttpStatus.BAD_REQUEST, reason = "400 Bad Request")
 		public void responseStatus() {
 		}
-		
+
 		public void httpServletResponse(HttpServletResponse response) {
 		}
-		
+
 		public void notModified() {
 		}
-		
+
+		public Object dynamicReturnValue(@RequestParam(required=false) String param) {
+			return (param != null) ? "view" : new RedirectView("redirectView");
+		}
 	}
 
 	private static class ExceptionRaisingReturnValueHandler implements HandlerMethodReturnValueHandler {

src/dist/changelog.txt

@@ -5,8 +5,11 @@ http://www.springsource.org
 Changes in version 3.2 M1
 -------------------------------------
 
-* fix issue with parsing invalid Content-Type or Accept headers
-
+* better handling on failure to parse invalid 'Content-Type' or 'Accept' headers
+* handle a controller method's return value based on the actual returned value (vs declared type)
+* fix issue with combining identical controller and method level request mapping paths
+* fix concurrency issue in AnnotationMethodHandlerExceptionResolver
+* fix case-sensitivity issue with some containers on access to 'Content-Disposition' header
 
 Changes in version 3.1.1 (2012-02-16)
 -------------------------------------