request handler methods with @ModelAttribute annotation always return a model attribute (SPR-4867)

org.springframework.web.servlet/src/main/java/org/springframework/web/bind/annotation/support/HandlerMethodInvoker.java

@@ -136,10 +136,10 @@ public class HandlerMethodInvoker {
 				Object attrValue = doInvokeMethod(attributeMethodToInvoke, handler, args);
 				String attrName = AnnotationUtils.findAnnotation(attributeMethodToInvoke, ModelAttribute.class).value();
 				if ("".equals(attrName)) {
-					Class resolvedType =
+					Class resolvedType = GenericTypeResolver.resolveReturnType(
-							GenericTypeResolver.resolveReturnType(attributeMethodToInvoke, handler.getClass());
+							attributeMethodToInvoke, handler.getClass());
-					attrName =
+					attrName = Conventions.getVariableNameForReturnType(
-							Conventions.getVariableNameForReturnType(attributeMethodToInvoke, resolvedType, attrValue);
+							attributeMethodToInvoke, resolvedType, attrValue);
 				}
 				implicitModel.addAttribute(attrName, attrValue);
 			}
@@ -156,10 +156,8 @@ public class HandlerMethodInvoker {
 	}
 
 	@SuppressWarnings("unchecked")
-	private Object[] resolveHandlerArguments(Method handlerMethod,
+	private Object[] resolveHandlerArguments(Method handlerMethod, Object handler,
-			Object handler,
+			NativeWebRequest webRequest, ExtendedModelMap implicitModel) throws Exception {
-			NativeWebRequest webRequest,
-			ExtendedModelMap implicitModel) throws Exception {
 
 		Class[] paramTypes = handlerMethod.getParameterTypes();
 		Object[] args = new Object[paramTypes.length];
@@ -446,7 +444,6 @@ public class HandlerMethodInvoker {
 			throws Exception {
 
 		HttpInputMessage inputMessage = createHttpInputMessage(webRequest);
-
 		Class paramType = methodParam.getParameterType();
 		MediaType contentType = inputMessage.getHeaders().getContentType();
 		if (contentType == null) {
@@ -463,16 +460,14 @@ public class HandlerMethodInvoker {
 				}
 			}
 		}
-		
 		throw new HttpMediaTypeNotSupportedException(contentType, allSupportedMediaTypes);
 	}
 
 	/**
-	 * Returns a {@link HttpInputMessage} for the given {@link NativeWebRequest}.
+	 * Return a {@link HttpInputMessage} for the given {@link NativeWebRequest}.
 	 * Throws an UnsupportedOperationException by default.
 	 */
 	protected HttpInputMessage createHttpInputMessage(NativeWebRequest webRequest) throws Exception {
-
 		throw new UnsupportedOperationException("@RequestBody not supported");
 	}
 
@@ -585,10 +580,8 @@ public class HandlerMethodInvoker {
 	}
 
 	@SuppressWarnings("unchecked")
-	public final void updateModelAttributes(Object handler,
+	public final void updateModelAttributes(Object handler, Map<String, Object> mavModel,
-			Map mavModel,
+			ExtendedModelMap implicitModel, NativeWebRequest webRequest) throws Exception {
-			ExtendedModelMap implicitModel,
-			NativeWebRequest webRequest) throws Exception {
 
 		if (this.methodResolver.hasSessionAttributes() && this.sessionStatus.isComplete()) {
 			for (String attrName : this.methodResolver.getActualSessionAttributeNames()) {
@@ -692,11 +685,22 @@ public class HandlerMethodInvoker {
 	}
 
 	protected Object resolveStandardArgument(Class parameterType, NativeWebRequest webRequest) throws Exception {
-
 		if (WebRequest.class.isAssignableFrom(parameterType)) {
 			return webRequest;
 		}
 		return WebArgumentResolver.UNRESOLVED;
 	}
 
+	protected final void addReturnValueAsModelAttribute(
+			Method handlerMethod, Class handlerType, Object returnValue, ExtendedModelMap implicitModel) {
+
+		ModelAttribute attr = AnnotationUtils.findAnnotation(handlerMethod, ModelAttribute.class);
+		String attrName = (attr != null ? attr.value() : "");
+		if ("".equals(attrName)) {
+			Class resolvedType = GenericTypeResolver.resolveReturnType(handlerMethod, handlerType);
+			attrName = Conventions.getVariableNameForReturnType(handlerMethod, resolvedType, returnValue);
+		}
+		implicitModel.addAttribute(attrName, returnValue);
+	}
+
 }

org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/mvc/annotation/AnnotationMethodHandlerAdapter.java

@@ -45,8 +45,6 @@ import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 
 import org.springframework.beans.BeanUtils;
-import org.springframework.core.Conventions;
-import org.springframework.core.GenericTypeResolver;
 import org.springframework.core.LocalVariableTableParameterNameDiscoverer;
 import org.springframework.core.MethodParameter;
 import org.springframework.core.ParameterNameDiscoverer;
@@ -662,12 +660,16 @@ public class AnnotationMethodHandlerAdapter extends WebContentGenerator implemen
 			else if (returnValue instanceof Model) {
 				return new ModelAndView().addAllObjects(implicitModel).addAllObjects(((Model) returnValue).asMap());
 			}
-			else if (returnValue instanceof Map) {
-				return new ModelAndView().addAllObjects(implicitModel).addAllObjects((Map) returnValue);
-			}
 			else if (returnValue instanceof View) {
 				return new ModelAndView((View) returnValue).addAllObjects(implicitModel);
 			}
+			else if (handlerMethod.isAnnotationPresent(ModelAttribute.class)) {
+				addReturnValueAsModelAttribute(handlerMethod, handlerType, returnValue, implicitModel);
+				return new ModelAndView().addAllObjects(implicitModel);
+			}
+			else if (returnValue instanceof Map) {
+				return new ModelAndView().addAllObjects(implicitModel).addAllObjects((Map) returnValue);
+			}
 			else if (returnValue instanceof String) {
 				return new ModelAndView((String) returnValue).addAllObjects(implicitModel);
 			}
@@ -683,14 +685,8 @@ public class AnnotationMethodHandlerAdapter extends WebContentGenerator implemen
 			}
 			else if (!BeanUtils.isSimpleProperty(returnValue.getClass())) {
 				// Assume a single model attribute...
-				ModelAttribute attr = AnnotationUtils.findAnnotation(handlerMethod, ModelAttribute.class);
+				addReturnValueAsModelAttribute(handlerMethod, handlerType, returnValue, implicitModel);
-				String attrName = (attr != null ? attr.value() : "");
+				return new ModelAndView().addAllObjects(implicitModel);
-				ModelAndView mav = new ModelAndView().addAllObjects(implicitModel);
-				if ("".equals(attrName)) {
-					Class resolvedType = GenericTypeResolver.resolveReturnType(handlerMethod, handlerType);
-					attrName = Conventions.getVariableNameForReturnType(handlerMethod, resolvedType, returnValue);
-				}
-				return mav.addObject(attrName, returnValue);
 			}
 			else {
 				throw new IllegalArgumentException("Invalid handler method return value: " + returnValue);
@@ -726,6 +722,7 @@ public class AnnotationMethodHandlerAdapter extends WebContentGenerator implemen
 		}
 	}
 
+
 	/**
 	 * Comparator capable of sorting {@link RequestMappingInfo}s (RHIs) so that sorting a list with this comparator will
 	 * result in: <ul> <li>RHIs with {@linkplain RequestMappingInfo#matchedPaths better matched paths} take prescedence

org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/mvc/annotation/ServletAnnotationControllerTests.java

@@ -346,7 +346,7 @@ public class ServletAnnotationControllerTests {
 		request.addParameter("age", "value2");
 		MockHttpServletResponse response = new MockHttpServletResponse();
 		servlet.service(request, response);
-		assertEquals("myView-name1-typeMismatch-tb1-myValue", response.getContentAsString());
+		assertEquals("myPath-name1-typeMismatch-tb1-myValue-yourValue", response.getContentAsString());
 	}
 
 	@Test
@@ -360,8 +360,7 @@ public class ServletAnnotationControllerTests {
 				DefaultAdvisorAutoProxyCreator autoProxyCreator = new DefaultAdvisorAutoProxyCreator();
 				autoProxyCreator.setBeanFactory(wac.getBeanFactory());
 				wac.getBeanFactory().addBeanPostProcessor(autoProxyCreator);
-				wac.getBeanFactory()
+				wac.getBeanFactory().registerSingleton("advisor", new DefaultPointcutAdvisor(new SimpleTraceInterceptor()));
-						.registerSingleton("advisor", new DefaultPointcutAdvisor(new SimpleTraceInterceptor()));
 				wac.refresh();
 				return wac;
 			}
@@ -1108,11 +1107,12 @@ public class ServletAnnotationControllerTests {
 		}
 
 		@RequestMapping("/myPath.do")
+		@ModelAttribute("yourKey")
 		public String myHandle(@ModelAttribute("myCommand") TestBean tb, BindingResult errors, Model model) {
 			if (!model.containsAttribute("myKey")) {
 				model.addAttribute("myKey", "myValue");
 			}
-			return "myView";
+			return "yourValue";
 		}
 	}
 
@@ -1379,9 +1379,9 @@ public class ServletAnnotationControllerTests {
 					}
 					List<TestBean> testBeans = (List<TestBean>) model.get("testBeanList");
 					if (errors.hasFieldErrors("age")) {
-						response.getWriter()
+						response.getWriter().write(viewName + "-" + tb.getName() + "-" +
-								.write(viewName + "-" + tb.getName() + "-" + errors.getFieldError("age").getCode() +
+								errors.getFieldError("age").getCode() + "-" + testBeans.get(0).getName() + "-" +
-										"-" + testBeans.get(0).getName() + "-" + model.get("myKey"));
+								model.get("myKey") + (model.containsKey("yourKey") ? "-" + model.get("yourKey") : ""));
 					}
 					else {
 						response.getWriter().write(viewName + "-" + tb.getName() + "-" + tb.getAge() + "-" +
@@ -1472,6 +1472,7 @@ public class ServletAnnotationControllerTests {
 		}
 	}
 
+
 	@Controller
 	public static class PathOrderingController {
 
@@ -1486,6 +1487,7 @@ public class ServletAnnotationControllerTests {
 		}
 	}
 
+
 	@Controller
 	public static class RequestBodyController {
 
@@ -1495,6 +1497,7 @@ public class ServletAnnotationControllerTests {
 		}
 	}
 
+
 	public static class MyMessageConverter implements HttpMessageConverter {
 
 		public boolean supports(Class clazz) {
@@ -1516,5 +1519,4 @@ public class ServletAnnotationControllerTests {
 		}
 	}
 
-
 }