Polish

spring-web/src/main/java/org/springframework/web/method/annotation/ModelAttributeMethodProcessor.java

@@ -38,23 +38,24 @@ import org.springframework.web.method.support.HandlerMethodReturnValueHandler;
 import org.springframework.web.method.support.ModelAndViewContainer;
 
 /**
- * Resolves method arguments annotated with {@code @ModelAttribute} and handles
- * return values from methods annotated with {@code @ModelAttribute}.
+ * Resolve {@code @ModelAttribute} annotated method arguments and handle
+ * return values from {@code @ModelAttribute} annotated methods.
  *
- * <p>Model attributes are obtained from the model or if not found possibly
- * created with a default constructor if it is available. Once created, the
- * attributed is populated with request data via data binding and also
- * validation may be applied if the argument is annotated with
- * {@code @javax.validation.Valid}.
+ * <p>Model attributes are obtained from the model or created with a default
+ * constructor (and then added to the model). Once created the attribute is
+ * populated via data binding to Servlet request parameters. Validation may be
+ * applied if the argument is annotated with {@code @javax.validation.Valid}.
+ * or {@link @Validated}.
  *
- * <p>When this handler is created with {@code annotationNotRequired=true},
+ * <p>When this handler is created with {@code annotationNotRequired=true}
  * any non-simple type argument and return value is regarded as a model
  * attribute with or without the presence of an {@code @ModelAttribute}.
  *
  * @author Rossen Stoyanchev
  * @since 3.1
  */
-public class ModelAttributeMethodProcessor implements HandlerMethodArgumentResolver, HandlerMethodReturnValueHandler {
+public class ModelAttributeMethodProcessor
+		implements HandlerMethodArgumentResolver, HandlerMethodReturnValueHandler {
 
 	protected final Log logger = LogFactory.getLog(getClass());
 
@@ -62,6 +63,7 @@ public class ModelAttributeMethodProcessor implements HandlerMethodArgumentResol
 
 
 	/**
+	 * Class constructor.
 	 * @param annotationNotRequired if "true", non-simple method arguments and
 	 * return values are considered model attributes with or without a
 	 * {@code @ModelAttribute} annotation.
@@ -72,20 +74,14 @@ public class ModelAttributeMethodProcessor implements HandlerMethodArgumentResol
 
 
 	/**
-	 * Returns {@code true} if the parameter is annotated with {@link ModelAttribute}
-	 * or in default resolution mode, and also if it is not a simple type.
+	 * Returns {@code true} if the parameter is annotated with
+	 * {@link ModelAttribute} or, if in default resolution mode, for any
+	 * method parameter that is not a simple type.
 	 */
 	@Override
 	public boolean supportsParameter(MethodParameter parameter) {
-		if (parameter.hasParameterAnnotation(ModelAttribute.class)) {
-			return true;
-		}
-		else if (this.annotationNotRequired) {
-			return !BeanUtils.isSimpleProperty(parameter.getParameterType());
-		}
-		else {
-			return false;
-		}
+		return (parameter.hasParameterAnnotation(ModelAttribute.class) ||
+				(this.annotationNotRequired && !BeanUtils.isSimpleProperty(parameter.getParameterType())));
 	}
 
 	/**
@@ -102,8 +98,8 @@ public class ModelAttributeMethodProcessor implements HandlerMethodArgumentResol
 			NativeWebRequest webRequest, WebDataBinderFactory binderFactory) throws Exception {
 
 		String name = ModelFactory.getNameForParameter(parameter);
-		Object attribute = (mavContainer.containsAttribute(name) ?
-				mavContainer.getModel().get(name) : createAttribute(name, parameter, binderFactory, webRequest));
+		Object attribute = (mavContainer.containsAttribute(name) ? mavContainer.getModel().get(name) :
+				createAttribute(name, parameter, binderFactory, webRequest));
 
 		WebDataBinder binder = binderFactory.createBinder(webRequest, attribute, name);
 		if (binder.getTarget() != null) {
@@ -182,19 +178,13 @@ public class ModelAttributeMethodProcessor implements HandlerMethodArgumentResol
 
 	/**
 	 * Return {@code true} if there is a method-level {@code @ModelAttribute}
-	 * or if it is a non-simple type when {@code annotationNotRequired=true}.
+	 * or, in default resolution mode, for any return value type that is not
+	 * a simple type.
 	 */
 	@Override
 	public boolean supportsReturnType(MethodParameter returnType) {
-		if (returnType.getMethodAnnotation(ModelAttribute.class) != null) {
-			return true;
-		}
-		else if (this.annotationNotRequired) {
-			return !BeanUtils.isSimpleProperty(returnType.getParameterType());
-		}
-		else {
-			return false;
-		}
+		return (returnType.getMethodAnnotation(ModelAttribute.class) != null ||
+				this.annotationNotRequired && !BeanUtils.isSimpleProperty(returnType.getParameterType()));
 	}
 
 	/**

spring-web/src/main/java/org/springframework/web/method/annotation/ModelFactory.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2014 the original author or authors.
+ * Copyright 2002-2016 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.
@@ -45,14 +45,15 @@ import org.springframework.web.method.support.InvocableHandlerMethod;
 import org.springframework.web.method.support.ModelAndViewContainer;
 
 /**
- * Provides methods to initialize the {@link Model} before controller method
- * invocation and to update it afterwards.
+ * Assist with initialization of the {@link Model} before controller method
+ * invocation and with updates to it after the invocation.
  *
- * <p>On initialization, the model is populated with attributes from the session
- * and by invoking methods annotated with {@code @ModelAttribute}.
+ * <p>On initialization the model is populated with attributes temporarily
+ * stored in the session and through the invocation of {@code @ModelAttribute}
+ * methods.
  *
- * <p>On update, model attributes are synchronized with the session and also
- * {@link BindingResult} attributes are added where missing.
+ * <p>On update model attributes are synchronized with the session and also
+ * {@link BindingResult} attributes are added if missing.
  *
  * @author Rossen Stoyanchev
  * @since 3.1
@@ -61,6 +62,7 @@ public final class ModelFactory {
 
 	private static final Log logger = LogFactory.getLog(ModelFactory.class);
 
+
 	private final List<ModelMethod> modelMethods = new ArrayList<ModelMethod>();
 
 	private final WebDataBinderFactory dataBinderFactory;
@@ -70,22 +72,23 @@ public final class ModelFactory {
 
 	/**
 	 * Create a new instance with the given {@code @ModelAttribute} methods.
-	 * @param invocableMethods the {@code @ModelAttribute} methods to invoke
-	 * @param dataBinderFactory for preparation of {@link BindingResult} attributes
-	 * @param sessionAttributesHandler for access to session attributes
+	 * @param handlerMethods the {@code @ModelAttribute} methods to invoke
+	 * @param binderFactory for preparation of {@link BindingResult} attributes
+	 * @param attributeHandler for access to session attributes
 	 */
-	public ModelFactory(List<InvocableHandlerMethod> invocableMethods, WebDataBinderFactory dataBinderFactory,
-			SessionAttributesHandler sessionAttributesHandler) {
+	public ModelFactory(List<InvocableHandlerMethod> handlerMethods,
+			WebDataBinderFactory binderFactory, SessionAttributesHandler attributeHandler) {
 
-		if (invocableMethods != null) {
-			for (InvocableHandlerMethod method : invocableMethods) {
-				this.modelMethods.add(new ModelMethod(method));
+		if (handlerMethods != null) {
+			for (InvocableHandlerMethod handlerMethod : handlerMethods) {
+				this.modelMethods.add(new ModelMethod(handlerMethod));
 			}
 		}
-		this.dataBinderFactory = dataBinderFactory;
-		this.sessionAttributesHandler = sessionAttributesHandler;
+		this.dataBinderFactory = binderFactory;
+		this.sessionAttributesHandler = attributeHandler;
 	}
 
+
 	/**
 	 * Populate the model in the following order:
 	 * <ol>
@@ -96,25 +99,26 @@ public final class ModelFactory {
 	 * 	an exception if necessary.
 	 * </ol>
 	 * @param request the current request
-	 * @param mavContainer a container with the model to be initialized
+	 * @param container a container with the model to be initialized
 	 * @param handlerMethod the method for which the model is initialized
 	 * @throws Exception may arise from {@code @ModelAttribute} methods
 	 */
-	public void initModel(NativeWebRequest request, ModelAndViewContainer mavContainer, HandlerMethod handlerMethod)
-			throws Exception {
+	public void initModel(NativeWebRequest request, ModelAndViewContainer container,
+			HandlerMethod handlerMethod) throws Exception {
 
 		Map<String, ?> sessionAttributes = this.sessionAttributesHandler.retrieveAttributes(request);
-		mavContainer.mergeAttributes(sessionAttributes);
+		container.mergeAttributes(sessionAttributes);
 
-		invokeModelAttributeMethods(request, mavContainer);
+		invokeModelAttributeMethods(request, container);
 
 		for (String name : findSessionAttributeArguments(handlerMethod)) {
-			if (!mavContainer.containsAttribute(name)) {
+			if (!container.containsAttribute(name)) {
 				Object value = this.sessionAttributesHandler.retrieveAttribute(request, name);
 				if (value == null) {
-					throw new HttpSessionRequiredException("Expected session attribute '" + name + "'");
+					throw new HttpSessionRequiredException(
+							"Expected session attribute '" + name + "'");
 				}
-				mavContainer.addAttribute(name, value);
+				container.addAttribute(name, value);
 			}
 		}
 	}
@@ -123,30 +127,31 @@ public final class ModelFactory {
 	 * Invoke model attribute methods to populate the model.
 	 * Attributes are added only if not already present in the model.
 	 */
-	private void invokeModelAttributeMethods(NativeWebRequest request, ModelAndViewContainer mavContainer)
-			throws Exception {
+	private void invokeModelAttributeMethods(NativeWebRequest request,
+			ModelAndViewContainer container) throws Exception {
 
 		while (!this.modelMethods.isEmpty()) {
-			InvocableHandlerMethod attrMethod = getNextModelMethod(mavContainer).getHandlerMethod();
-			String modelName = attrMethod.getMethodAnnotation(ModelAttribute.class).value();
-			if (mavContainer.containsAttribute(modelName)) {
+			InvocableHandlerMethod modelMethod = getNextModelMethod(container).getHandlerMethod();
+			ModelAttribute annot = modelMethod.getMethodAnnotation(ModelAttribute.class);
+			String modelName = annot.value();
+			if (container.containsAttribute(modelName)) {
 				continue;
 			}
 
-			Object returnValue = attrMethod.invokeForRequest(request, mavContainer);
+			Object returnValue = modelMethod.invokeForRequest(request, container);
 
-			if (!attrMethod.isVoid()){
-				String returnValueName = getNameForReturnValue(returnValue, attrMethod.getReturnType());
-				if (!mavContainer.containsAttribute(returnValueName)) {
-					mavContainer.addAttribute(returnValueName, returnValue);
+			if (!modelMethod.isVoid()){
+				String returnValueName = getNameForReturnValue(returnValue, modelMethod.getReturnType());
+				if (!container.containsAttribute(returnValueName)) {
+					container.addAttribute(returnValueName, returnValue);
 				}
 			}
 		}
 	}
 
-	private ModelMethod getNextModelMethod(ModelAndViewContainer mavContainer) {
+	private ModelMethod getNextModelMethod(ModelAndViewContainer container) {
 		for (ModelMethod modelMethod : this.modelMethods) {
-			if (modelMethod.checkDependencies(mavContainer)) {
+			if (modelMethod.checkDependencies(container)) {
 				if (logger.isTraceEnabled()) {
 					logger.trace("Selected @ModelAttribute method " + modelMethod);
 				}
@@ -157,7 +162,7 @@ public final class ModelFactory {
 		ModelMethod modelMethod = this.modelMethods.get(0);
 		if (logger.isTraceEnabled()) {
 			logger.trace("Selected @ModelAttribute method (not present: " +
-					modelMethod.getUnresolvedDependencies(mavContainer)+ ") " + modelMethod);
+					modelMethod.getUnresolvedDependencies(container)+ ") " + modelMethod);
 		}
 		this.modelMethods.remove(modelMethod);
 		return modelMethod;
@@ -171,7 +176,8 @@ public final class ModelFactory {
 		for (MethodParameter parameter : handlerMethod.getMethodParameters()) {
 			if (parameter.hasParameterAnnotation(ModelAttribute.class)) {
 				String name = getNameForParameter(parameter);
-				if (this.sessionAttributesHandler.isHandlerSessionAttribute(name, parameter.getParameterType())) {
+				Class<?> paramType = parameter.getParameterType();
+				if (this.sessionAttributesHandler.isHandlerSessionAttribute(name, paramType)) {
 					result.add(name);
 				}
 			}
@@ -189,8 +195,8 @@ public final class ModelFactory {
 	 */
 	public static String getNameForParameter(MethodParameter parameter) {
 		ModelAttribute annot = parameter.getParameterAnnotation(ModelAttribute.class);
-		String attrName = (annot != null) ? annot.value() : null;
-		return StringUtils.hasText(attrName) ? attrName :  Conventions.getVariableNameForParameter(parameter);
+		String name = (annot != null) ? annot.value() : null;
+		return StringUtils.hasText(name) ? name : Conventions.getVariableNameForParameter(parameter);
 	}
 
 	/**
@@ -211,7 +217,8 @@ public final class ModelFactory {
 		}
 		else {
 			Method method = returnType.getMethod();
-			Class<?> resolvedType = GenericTypeResolver.resolveReturnType(method, returnType.getContainingClass());
+			Class<?> containingClass = returnType.getContainingClass();
+			Class<?> resolvedType = GenericTypeResolver.resolveReturnType(method, containingClass);
 			return Conventions.getVariableNameForReturnType(method, resolvedType, returnValue);
 		}
 	}
@@ -220,18 +227,18 @@ public final class ModelFactory {
 	 * Promote model attributes listed as {@code @SessionAttributes} to the session.
 	 * Add {@link BindingResult} attributes where necessary.
 	 * @param request the current request
-	 * @param mavContainer contains the model to update
+	 * @param container contains the model to update
 	 * @throws Exception if creating BindingResult attributes fails
 	 */
-	public void updateModel(NativeWebRequest request, ModelAndViewContainer mavContainer) throws Exception {
-		ModelMap defaultModel = mavContainer.getDefaultModel();
-		if (mavContainer.getSessionStatus().isComplete()){
+	public void updateModel(NativeWebRequest request, ModelAndViewContainer container) throws Exception {
+		ModelMap defaultModel = container.getDefaultModel();
+		if (container.getSessionStatus().isComplete()){
 			this.sessionAttributesHandler.cleanupAttributes(request);
 		}
 		else {
 			this.sessionAttributesHandler.storeAttributes(request, defaultModel);
 		}
-		if (!mavContainer.isRequestHandled() && mavContainer.getModel() == defaultModel) {
+		if (!container.isRequestHandled() && container.getModel() == defaultModel) {
 			updateBindingResult(request, defaultModel);
 		}
 	}
@@ -248,7 +255,7 @@ public final class ModelFactory {
 				String bindingResultKey = BindingResult.MODEL_KEY_PREFIX + name;
 
 				if (!model.containsAttribute(bindingResultKey)) {
-					WebDataBinder dataBinder = dataBinderFactory.createBinder(request, value, name);
+					WebDataBinder dataBinder = this.dataBinderFactory.createBinder(request, value, name);
 					model.put(bindingResultKey, dataBinder.getBindingResult());
 				}
 			}

spring-web/src/test/java/org/springframework/web/method/annotation/ModelAttributeMethodProcessorTests.java

@@ -24,6 +24,7 @@ import org.junit.Before;
 import org.junit.Test;
 
 import org.springframework.core.MethodParameter;
+import org.springframework.core.annotation.SynthesizingMethodParameter;
 import org.springframework.mock.web.test.MockHttpServletRequest;
 import org.springframework.tests.sample.beans.TestBean;
 import org.springframework.validation.BindException;
@@ -51,56 +52,53 @@ import static org.mockito.BDDMockito.*;
  */
 public class ModelAttributeMethodProcessorTests {
 
+	private NativeWebRequest request;
+
+	private ModelAndViewContainer container;
+
 	private ModelAttributeMethodProcessor processor;
 
 	private MethodParameter paramNamedValidModelAttr;
-
 	private MethodParameter paramErrors;
-
 	private MethodParameter paramInt;
-
 	private MethodParameter paramModelAttr;
-
 	private MethodParameter paramNonSimpleType;
 
 	private MethodParameter returnParamNamedModelAttr;
-
 	private MethodParameter returnParamNonSimpleType;
 
-	private ModelAndViewContainer mavContainer;
-
-	private NativeWebRequest webRequest;
 
 	@Before
 	public void setUp() throws Exception {
-		processor = new ModelAttributeMethodProcessor(false);
+		this.request = new ServletWebRequest(new MockHttpServletRequest());
+		this.container = new ModelAndViewContainer();
+		this.processor = new ModelAttributeMethodProcessor(false);
 
 		Method method = ModelAttributeHandler.class.getDeclaredMethod("modelAttribute",
 				TestBean.class, Errors.class, int.class, TestBean.class, TestBean.class);
 
-		paramNamedValidModelAttr = new MethodParameter(method, 0);
-		paramErrors = new MethodParameter(method, 1);
-		paramInt = new MethodParameter(method, 2);
-		paramModelAttr = new MethodParameter(method, 3);
-		paramNonSimpleType = new MethodParameter(method, 4);
+		this.paramNamedValidModelAttr = new SynthesizingMethodParameter(method, 0);
+		this.paramErrors = new SynthesizingMethodParameter(method, 1);
+		this.paramInt = new SynthesizingMethodParameter(method, 2);
+		this.paramModelAttr = new SynthesizingMethodParameter(method, 3);
+		this.paramNonSimpleType = new SynthesizingMethodParameter(method, 4);
 
-		returnParamNamedModelAttr = new MethodParameter(getClass().getDeclaredMethod("annotatedReturnValue"), -1);
-		returnParamNonSimpleType = new MethodParameter(getClass().getDeclaredMethod("notAnnotatedReturnValue"), -1);
+		method = getClass().getDeclaredMethod("annotatedReturnValue");
+		this.returnParamNamedModelAttr = new MethodParameter(method, -1);
 
-		mavContainer = new ModelAndViewContainer();
-
-		webRequest = new ServletWebRequest(new MockHttpServletRequest());
+		method = getClass().getDeclaredMethod("notAnnotatedReturnValue");
+		this.returnParamNonSimpleType = new MethodParameter(method, -1);
 	}
 
+
 	@Test
 	public void supportedParameters() throws Exception {
-		// Only @ModelAttribute arguments
-		assertTrue(processor.supportsParameter(paramNamedValidModelAttr));
-		assertTrue(processor.supportsParameter(paramModelAttr));
+		assertTrue(this.processor.supportsParameter(this.paramNamedValidModelAttr));
+		assertTrue(this.processor.supportsParameter(this.paramModelAttr));
 
-		assertFalse(processor.supportsParameter(paramErrors));
-		assertFalse(processor.supportsParameter(paramInt));
-		assertFalse(processor.supportsParameter(paramNonSimpleType));
+		assertFalse(this.processor.supportsParameter(this.paramErrors));
+		assertFalse(this.processor.supportsParameter(this.paramInt));
+		assertFalse(this.processor.supportsParameter(this.paramNonSimpleType));
 	}
 
 	@Test
@@ -108,135 +106,127 @@ public class ModelAttributeMethodProcessorTests {
 		processor = new ModelAttributeMethodProcessor(true);
 
 		// Only non-simple types, even if not annotated
-		assertTrue(processor.supportsParameter(paramNamedValidModelAttr));
-		assertTrue(processor.supportsParameter(paramErrors));
-		assertTrue(processor.supportsParameter(paramModelAttr));
-		assertTrue(processor.supportsParameter(paramNonSimpleType));
+		assertTrue(this.processor.supportsParameter(this.paramNamedValidModelAttr));
+		assertTrue(this.processor.supportsParameter(this.paramErrors));
+		assertTrue(this.processor.supportsParameter(this.paramModelAttr));
+		assertTrue(this.processor.supportsParameter(this.paramNonSimpleType));
 
-		assertFalse(processor.supportsParameter(paramInt));
+		assertFalse(this.processor.supportsParameter(this.paramInt));
 	}
 
 	@Test
 	public void supportedReturnTypes() throws Exception {
 		processor = new ModelAttributeMethodProcessor(false);
-		assertTrue(processor.supportsReturnType(returnParamNamedModelAttr));
-		assertFalse(processor.supportsReturnType(returnParamNonSimpleType));
+		assertTrue(this.processor.supportsReturnType(returnParamNamedModelAttr));
+		assertFalse(this.processor.supportsReturnType(returnParamNonSimpleType));
 	}
 
 	@Test
 	public void supportedReturnTypesInDefaultResolutionMode() throws Exception {
 		processor = new ModelAttributeMethodProcessor(true);
-		assertTrue(processor.supportsReturnType(returnParamNamedModelAttr));
-		assertTrue(processor.supportsReturnType(returnParamNonSimpleType));
+		assertTrue(this.processor.supportsReturnType(returnParamNamedModelAttr));
+		assertTrue(this.processor.supportsReturnType(returnParamNonSimpleType));
 	}
 
 	@Test
 	public void bindExceptionRequired() throws Exception {
-		assertTrue(processor.isBindExceptionRequired(null, paramNonSimpleType));
-	}
-
-	@Test
-	public void bindExceptionNotRequired() throws Exception {
-		assertFalse(processor.isBindExceptionRequired(null, paramNamedValidModelAttr));
+		assertTrue(this.processor.isBindExceptionRequired(null, this.paramNonSimpleType));
+		assertFalse(this.processor.isBindExceptionRequired(null, this.paramNamedValidModelAttr));
 	}
 
 	@Test
-	public void resovleArgumentFromModel() throws Exception {
-		getAttributeFromModel("attrName", paramNamedValidModelAttr);
-		getAttributeFromModel("testBean", paramModelAttr);
-		getAttributeFromModel("testBean", paramNonSimpleType);
-	}
-
-	private void getAttributeFromModel(String expectedAttributeName, MethodParameter param) throws Exception {
-		Object target = new TestBean();
-		mavContainer.addAttribute(expectedAttributeName, target);
-
-		WebDataBinder dataBinder = new WebRequestDataBinder(target);
-		WebDataBinderFactory factory = mock(WebDataBinderFactory.class);
-		given(factory.createBinder(webRequest, target, expectedAttributeName)).willReturn(dataBinder);
-
-		processor.resolveArgument(param, mavContainer, webRequest, factory);
-		verify(factory).createBinder(webRequest, target, expectedAttributeName);
+	public void resolveArgumentFromModel() throws Exception {
+		testGetAttributeFromModel("attrName", this.paramNamedValidModelAttr);
+		testGetAttributeFromModel("testBean", this.paramModelAttr);
+		testGetAttributeFromModel("testBean", this.paramNonSimpleType);
 	}
 
 	@Test
 	public void resovleArgumentViaDefaultConstructor() throws Exception {
 		WebDataBinder dataBinder = new WebRequestDataBinder(null);
-
 		WebDataBinderFactory factory = mock(WebDataBinderFactory.class);
-		given(factory.createBinder((NativeWebRequest) anyObject(), notNull(), eq("attrName"))).willReturn(dataBinder);
+		given(factory.createBinder(anyObject(), notNull(), eq("attrName"))).willReturn(dataBinder);
 
-		processor.resolveArgument(paramNamedValidModelAttr, mavContainer, webRequest, factory);
-
-		verify(factory).createBinder((NativeWebRequest) anyObject(), notNull(), eq("attrName"));
+		this.processor.resolveArgument(this.paramNamedValidModelAttr, this.container, this.request, factory);
+		verify(factory).createBinder(anyObject(), notNull(), eq("attrName"));
 	}
 
 	@Test
 	public void resolveArgumentValidation() throws Exception {
 		String name = "attrName";
 		Object target = new TestBean();
-		mavContainer.addAttribute(name, target);
+		this.container.addAttribute(name, target);
 
 		StubRequestDataBinder dataBinder = new StubRequestDataBinder(target, name);
-		WebDataBinderFactory binderFactory = mock(WebDataBinderFactory.class);
-		given(binderFactory.createBinder(webRequest, target, name)).willReturn(dataBinder);
+		WebDataBinderFactory factory = mock(WebDataBinderFactory.class);
+		given(factory.createBinder(this.request, target, name)).willReturn(dataBinder);
 
-		processor.resolveArgument(paramNamedValidModelAttr, mavContainer, webRequest, binderFactory);
+		this.processor.resolveArgument(this.paramNamedValidModelAttr, this.container, this.request, factory);
 
 		assertTrue(dataBinder.isBindInvoked());
 		assertTrue(dataBinder.isValidateInvoked());
 	}
 
 	@Test(expected = BindException.class)
-	public void resovleArgumentBindException() throws Exception {
+	public void resolveArgumentBindException() throws Exception {
 		String name = "testBean";
 		Object target = new TestBean();
-		mavContainer.getModel().addAttribute(target);
+		this.container.getModel().addAttribute(target);
 
 		StubRequestDataBinder dataBinder = new StubRequestDataBinder(target, name);
 		dataBinder.getBindingResult().reject("error");
-
 		WebDataBinderFactory binderFactory = mock(WebDataBinderFactory.class);
-		given(binderFactory.createBinder(webRequest, target, name)).willReturn(dataBinder);
+		given(binderFactory.createBinder(this.request, target, name)).willReturn(dataBinder);
 
-		processor.resolveArgument(paramNonSimpleType, mavContainer, webRequest, binderFactory);
-		verify(binderFactory).createBinder(webRequest, target, name);
+		this.processor.resolveArgument(this.paramNonSimpleType, this.container, this.request, binderFactory);
+		verify(binderFactory).createBinder(this.request, target, name);
 	}
 
 	@Test  // SPR-9378
 	public void resolveArgumentOrdering() throws Exception {
 		String name = "testBean";
 		Object testBean = new TestBean(name);
-		mavContainer.addAttribute(name, testBean);
-		mavContainer.addAttribute(BindingResult.MODEL_KEY_PREFIX + name, testBean);
+		this.container.addAttribute(name, testBean);
+		this.container.addAttribute(BindingResult.MODEL_KEY_PREFIX + name, testBean);
 
 		Object anotherTestBean = new TestBean();
-		mavContainer.addAttribute("anotherTestBean", anotherTestBean);
+		this.container.addAttribute("anotherTestBean", anotherTestBean);
 
 		StubRequestDataBinder dataBinder = new StubRequestDataBinder(testBean, name);
 		WebDataBinderFactory binderFactory = mock(WebDataBinderFactory.class);
-		given(binderFactory.createBinder(webRequest, testBean, name)).willReturn(dataBinder);
+		given(binderFactory.createBinder(this.request, testBean, name)).willReturn(dataBinder);
 
-		processor.resolveArgument(paramModelAttr, mavContainer, webRequest, binderFactory);
+		this.processor.resolveArgument(this.paramModelAttr, this.container, this.request, binderFactory);
 
-		assertSame("Resolved attribute should be updated to be last in the order",
-				testBean, mavContainer.getModel().values().toArray()[1]);
-		assertSame("BindingResult of resolved attribute should be last in the order",
-				dataBinder.getBindingResult(), mavContainer.getModel().values().toArray()[2]);
+		Object[] values = this.container.getModel().values().toArray();
+		assertSame("Resolved attribute should be updated to be last", testBean, values[1]);
+		assertSame("BindingResult of resolved attr should be last", dataBinder.getBindingResult(), values[2]);
 	}
 
 	@Test
 	public void handleAnnotatedReturnValue() throws Exception {
-		processor.handleReturnValue("expected", returnParamNamedModelAttr, mavContainer, webRequest);
-		assertEquals("expected", mavContainer.getModel().get("modelAttrName"));
+		this.processor.handleReturnValue("expected", this.returnParamNamedModelAttr, this.container, this.request);
+		assertEquals("expected", this.container.getModel().get("modelAttrName"));
 	}
 
 	@Test
 	public void handleNotAnnotatedReturnValue() throws Exception {
 		TestBean testBean = new TestBean("expected");
-		processor.handleReturnValue(testBean, returnParamNonSimpleType, mavContainer, webRequest);
+		this.processor.handleReturnValue(testBean, this.returnParamNonSimpleType, this.container, this.request);
+		assertSame(testBean, this.container.getModel().get("testBean"));
+	}
+
+
+	private void testGetAttributeFromModel(String expectedAttrName, MethodParameter param) throws Exception {
+		Object target = new TestBean();
+		this.container.addAttribute(expectedAttrName, target);
 
-		assertSame(testBean, mavContainer.getModel().get("testBean"));
+		WebDataBinder dataBinder = new WebRequestDataBinder(target);
+		WebDataBinderFactory factory = mock(WebDataBinderFactory.class);
+		given(factory.createBinder(this.request, target, expectedAttrName)).willReturn(dataBinder);
+
+		this.processor.resolveArgument(param, this.container, this.request, factory);
+		verify(factory).createBinder(this.request, target, expectedAttrName);
 	}
 
 
@@ -246,6 +236,7 @@ public class ModelAttributeMethodProcessorTests {
 
 		private boolean validateInvoked;
 
+
 		public StubRequestDataBinder(Object target, String objectName) {
 			super(target, objectName);
 		}
@@ -285,13 +276,17 @@ public class ModelAttributeMethodProcessorTests {
 	private static class ModelAttributeHandler {
 
 		@SuppressWarnings("unused")
-		public void modelAttribute(@ModelAttribute("attrName") @Valid TestBean annotatedAttr, Errors errors,
-				int intArg, @ModelAttribute TestBean defaultNameAttr, TestBean notAnnotatedAttr) {
+		public void modelAttribute(
+				@ModelAttribute("attrName") @Valid TestBean annotatedAttr,
+				Errors errors,
+				int intArg,
+				@ModelAttribute TestBean defaultNameAttr,
+				TestBean notAnnotatedAttr) {
 		}
 	}
 
 
-	@ModelAttribute("modelAttrName")
+	@ModelAttribute("modelAttrName") @SuppressWarnings("unused")
 	private String annotatedReturnValue() {
 		return null;
 	}

spring-web/src/test/java/org/springframework/web/method/annotation/ModelFactoryTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2014 the original author or authors.
+ * Copyright 2002-2016 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.
@@ -16,19 +16,12 @@
 
 package org.springframework.web.method.annotation;
 
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertNull;
-import static org.junit.Assert.assertSame;
-import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.fail;
-import static org.mockito.BDDMockito.given;
-import static org.mockito.BDDMockito.mock;
-
 import java.lang.reflect.Method;
-import java.util.Arrays;
+import java.util.Collections;
 
 import org.junit.Before;
 import org.junit.Test;
+
 import org.springframework.core.LocalVariableTableParameterNameDiscoverer;
 import org.springframework.mock.web.test.MockHttpServletRequest;
 import org.springframework.ui.Model;
@@ -43,10 +36,19 @@ import org.springframework.web.bind.support.SessionAttributeStore;
 import org.springframework.web.bind.support.WebDataBinderFactory;
 import org.springframework.web.context.request.NativeWebRequest;
 import org.springframework.web.context.request.ServletWebRequest;
+import org.springframework.web.method.HandlerMethod;
 import org.springframework.web.method.support.HandlerMethodArgumentResolverComposite;
 import org.springframework.web.method.support.InvocableHandlerMethod;
 import org.springframework.web.method.support.ModelAndViewContainer;
 
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertSame;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+import static org.mockito.BDDMockito.given;
+import static org.mockito.BDDMockito.mock;
+
 
 /**
  * Text fixture for {@link ModelFactory} tests.
@@ -55,103 +57,92 @@ import org.springframework.web.method.support.ModelAndViewContainer;
  */
 public class ModelFactoryTests {
 
-	private TestController controller = new TestController();
-
-	private InvocableHandlerMethod handleMethod;
+	private NativeWebRequest webRequest;
 
-	private InvocableHandlerMethod handleSessionAttrMethod;
+	private SessionAttributesHandler attributeHandler;
 
-	private SessionAttributesHandler sessionAttrsHandler;
+	private SessionAttributeStore attributeStore;
 
-	private SessionAttributeStore sessionAttributeStore;
+	private TestController controller = new TestController();
 
-	private NativeWebRequest webRequest;
+	private ModelAndViewContainer mavContainer;
 
 
 	@Before
 	public void setUp() throws Exception {
-		this.controller = new TestController();
-
-		Method method = TestController.class.getDeclaredMethod("handle");
-		this.handleMethod = new InvocableHandlerMethod(this.controller, method);
-
-		method = TestController.class.getDeclaredMethod("handleSessionAttr", String.class);
-		this.handleSessionAttrMethod = new InvocableHandlerMethod(this.controller, method);
-
-		this.sessionAttributeStore = new DefaultSessionAttributeStore();
-		this.sessionAttrsHandler = new SessionAttributesHandler(TestController.class, this.sessionAttributeStore);
 		this.webRequest = new ServletWebRequest(new MockHttpServletRequest());
+		this.attributeStore = new DefaultSessionAttributeStore();
+		this.attributeHandler = new SessionAttributesHandler(TestController.class, this.attributeStore);
+		this.controller = new TestController();
+		this.mavContainer = new ModelAndViewContainer();
 	}
 
 
 	@Test
 	public void modelAttributeMethod() throws Exception {
 		ModelFactory modelFactory = createModelFactory("modelAttr", Model.class);
-		ModelAndViewContainer mavContainer = new ModelAndViewContainer();
-		modelFactory.initModel(this.webRequest, mavContainer, this.handleMethod);
+		HandlerMethod handlerMethod = createHandlerMethod("handle");
+		modelFactory.initModel(this.webRequest, this.mavContainer, handlerMethod);
 
-		assertEquals(Boolean.TRUE, mavContainer.getModel().get("modelAttr"));
+		assertEquals(Boolean.TRUE, this.mavContainer.getModel().get("modelAttr"));
 	}
 
 	@Test
 	public void modelAttributeMethodWithExplicitName() throws Exception {
 		ModelFactory modelFactory = createModelFactory("modelAttrWithName");
-		ModelAndViewContainer mavContainer = new ModelAndViewContainer();
-		modelFactory.initModel(this.webRequest, mavContainer, this.handleMethod);
+		HandlerMethod handlerMethod = createHandlerMethod("handle");
+		modelFactory.initModel(this.webRequest, this.mavContainer, handlerMethod);
 
-		assertEquals(Boolean.TRUE, mavContainer.getModel().get("name"));
+		assertEquals(Boolean.TRUE, this.mavContainer.getModel().get("name"));
 	}
 
 	@Test
 	public void modelAttributeMethodWithNameByConvention() throws Exception {
 		ModelFactory modelFactory = createModelFactory("modelAttrConvention");
-		ModelAndViewContainer mavContainer = new ModelAndViewContainer();
-		modelFactory.initModel(this.webRequest, mavContainer, this.handleMethod);
+		HandlerMethod handlerMethod = createHandlerMethod("handle");
+		modelFactory.initModel(this.webRequest, this.mavContainer, handlerMethod);
 
-		assertEquals(Boolean.TRUE, mavContainer.getModel().get("boolean"));
+		assertEquals(Boolean.TRUE, this.mavContainer.getModel().get("boolean"));
 	}
 
 	@Test
 	public void modelAttributeMethodWithNullReturnValue() throws Exception {
 		ModelFactory modelFactory = createModelFactory("nullModelAttr");
-		ModelAndViewContainer mavContainer = new ModelAndViewContainer();
-		modelFactory.initModel(this.webRequest, mavContainer, this.handleMethod);
+		HandlerMethod handlerMethod = createHandlerMethod("handle");
+		modelFactory.initModel(this.webRequest, this.mavContainer, handlerMethod);
 
-		assertTrue(mavContainer.containsAttribute("name"));
-		assertNull(mavContainer.getModel().get("name"));
+		assertTrue(this.mavContainer.containsAttribute("name"));
+		assertNull(this.mavContainer.getModel().get("name"));
 	}
 
 	@Test
 	public void sessionAttribute() throws Exception {
-		this.sessionAttributeStore.storeAttribute(this.webRequest, "sessionAttr", "sessionAttrValue");
-
-		// Resolve successfully handler session attribute once
-		assertTrue(sessionAttrsHandler.isHandlerSessionAttribute("sessionAttr", null));
+		this.attributeStore.storeAttribute(this.webRequest, "sessionAttr", "sessionAttrValue");
 
 		ModelFactory modelFactory = createModelFactory("modelAttr", Model.class);
-		ModelAndViewContainer mavContainer = new ModelAndViewContainer();
-		modelFactory.initModel(this.webRequest, mavContainer, this.handleMethod);
+		HandlerMethod handlerMethod = createHandlerMethod("handle");
+		modelFactory.initModel(this.webRequest, this.mavContainer, handlerMethod);
 
-		assertEquals("sessionAttrValue", mavContainer.getModel().get("sessionAttr"));
+		assertEquals("sessionAttrValue", this.mavContainer.getModel().get("sessionAttr"));
 	}
 
 	@Test
 	public void sessionAttributeNotPresent() throws Exception {
-		ModelFactory modelFactory = new ModelFactory(null, null, this.sessionAttrsHandler);
-
+		ModelFactory modelFactory = new ModelFactory(null, null, this.attributeHandler);
+		HandlerMethod handlerMethod = createHandlerMethod("handleSessionAttr", String.class);
 		try {
-			modelFactory.initModel(this.webRequest, new ModelAndViewContainer(), this.handleSessionAttrMethod);
+			modelFactory.initModel(this.webRequest, this.mavContainer, handlerMethod);
 			fail("Expected HttpSessionRequiredException");
 		}
 		catch (HttpSessionRequiredException e) {
 			// expected
 		}
 
-		this.sessionAttributeStore.storeAttribute(this.webRequest, "sessionAttr", "sessionAttrValue");
-		ModelAndViewContainer mavContainer = new ModelAndViewContainer();
-		modelFactory.initModel(this.webRequest, mavContainer, this.handleSessionAttrMethod);
+		// Now add attribute and try again
+		this.attributeStore.storeAttribute(this.webRequest, "sessionAttr", "sessionAttrValue");
 
-		assertEquals("sessionAttrValue", mavContainer.getModel().get("sessionAttr"));
+		modelFactory.initModel(this.webRequest, this.mavContainer, handlerMethod);
+		assertEquals("sessionAttrValue", this.mavContainer.getModel().get("sessionAttr"));
 	}
 
 	@Test
@@ -165,11 +156,12 @@ public class ModelFactoryTests {
 		WebDataBinderFactory binderFactory = mock(WebDataBinderFactory.class);
 		given(binderFactory.createBinder(this.webRequest, command, commandName)).willReturn(dataBinder);
 
-		ModelFactory modelFactory = new ModelFactory(null, binderFactory, this.sessionAttrsHandler);
+		ModelFactory modelFactory = new ModelFactory(null, binderFactory, this.attributeHandler);
 		modelFactory.updateModel(this.webRequest, container);
 
 		assertEquals(command, container.getModel().get(commandName));
-		assertSame(dataBinder.getBindingResult(), container.getModel().get(bindingResultKey(commandName)));
+		String bindingResultKey = BindingResult.MODEL_KEY_PREFIX + commandName;
+		assertSame(dataBinder.getBindingResult(), container.getModel().get(bindingResultKey));
 		assertEquals(2, container.getModel().size());
 	}
 
@@ -184,11 +176,11 @@ public class ModelFactoryTests {
 		WebDataBinderFactory binderFactory = mock(WebDataBinderFactory.class);
 		given(binderFactory.createBinder(this.webRequest, attribute, attributeName)).willReturn(dataBinder);
 
-		ModelFactory modelFactory = new ModelFactory(null, binderFactory, this.sessionAttrsHandler);
+		ModelFactory modelFactory = new ModelFactory(null, binderFactory, this.attributeHandler);
 		modelFactory.updateModel(this.webRequest, container);
 
 		assertEquals(attribute, container.getModel().get(attributeName));
-		assertEquals(attribute, this.sessionAttributeStore.retrieveAttribute(this.webRequest, attributeName));
+		assertEquals(attribute, this.attributeStore.retrieveAttribute(this.webRequest, attributeName));
 	}
 
 	@Test
@@ -198,9 +190,7 @@ public class ModelFactoryTests {
 		ModelAndViewContainer container = new ModelAndViewContainer();
 		container.addAttribute(attributeName, attribute);
 
-		// Store and resolve once (to be "remembered")
-		this.sessionAttributeStore.storeAttribute(this.webRequest, attributeName, attribute);
-		this.sessionAttrsHandler.isHandlerSessionAttribute(attributeName, null);
+		this.attributeStore.storeAttribute(this.webRequest, attributeName, attribute);
 
 		WebDataBinder dataBinder = new WebDataBinder(attribute, attributeName);
 		WebDataBinderFactory binderFactory = mock(WebDataBinderFactory.class);
@@ -208,11 +198,11 @@ public class ModelFactoryTests {
 
 		container.getSessionStatus().setComplete();
 
-		ModelFactory modelFactory = new ModelFactory(null, binderFactory, this.sessionAttrsHandler);
+		ModelFactory modelFactory = new ModelFactory(null, binderFactory, this.attributeHandler);
 		modelFactory.updateModel(this.webRequest, container);
 
 		assertEquals(attribute, container.getModel().get(attributeName));
-		assertNull(this.sessionAttributeStore.retrieveAttribute(this.webRequest, attributeName));
+		assertNull(this.attributeStore.retrieveAttribute(this.webRequest, attributeName));
 	}
 
 	// SPR-12542
@@ -233,33 +223,33 @@ public class ModelFactoryTests {
 		WebDataBinderFactory binderFactory = mock(WebDataBinderFactory.class);
 		given(binderFactory.createBinder(this.webRequest, attribute, attributeName)).willReturn(dataBinder);
 
-		ModelFactory modelFactory = new ModelFactory(null, binderFactory, this.sessionAttrsHandler);
+		ModelFactory modelFactory = new ModelFactory(null, binderFactory, this.attributeHandler);
 		modelFactory.updateModel(this.webRequest, container);
 
 		assertEquals(queryParam, container.getModel().get(queryParamName));
 		assertEquals(1, container.getModel().size());
-		assertEquals(attribute, this.sessionAttributeStore.retrieveAttribute(this.webRequest, attributeName));
+		assertEquals(attribute, this.attributeStore.retrieveAttribute(this.webRequest, attributeName));
 	}
 
 
-	private String bindingResultKey(String key) {
-		return BindingResult.MODEL_KEY_PREFIX + key;
-	}
-
-	private ModelFactory createModelFactory(String methodName, Class<?>... parameterTypes) throws Exception{
-		Method method = TestController.class.getMethod(methodName, parameterTypes);
+	private ModelFactory createModelFactory(String methodName, Class<?>... parameterTypes) throws Exception {
+		HandlerMethodArgumentResolverComposite resolvers = new HandlerMethodArgumentResolverComposite();
+		resolvers.addResolver(new ModelMethodProcessor());
 
-		HandlerMethodArgumentResolverComposite argResolvers = new HandlerMethodArgumentResolverComposite();
-		argResolvers.addResolver(new ModelMethodProcessor());
+		InvocableHandlerMethod modelMethod = createHandlerMethod(methodName, parameterTypes);
+		modelMethod.setHandlerMethodArgumentResolvers(resolvers);
+		modelMethod.setDataBinderFactory(null);
+		modelMethod.setParameterNameDiscoverer(new LocalVariableTableParameterNameDiscoverer());
 
-		InvocableHandlerMethod handlerMethod = new InvocableHandlerMethod(this.controller, method);
-		handlerMethod.setHandlerMethodArgumentResolvers(argResolvers);
-		handlerMethod.setDataBinderFactory(null);
-		handlerMethod.setParameterNameDiscoverer(new LocalVariableTableParameterNameDiscoverer());
+		return new ModelFactory(Collections.singletonList(modelMethod), null, this.attributeHandler);
+	}
 
-		return new ModelFactory(Arrays.asList(handlerMethod), null, this.sessionAttrsHandler);
+	private InvocableHandlerMethod createHandlerMethod(String methodName, Class<?>... paramTypes) throws Exception {
+		Method method = this.controller.getClass().getMethod(methodName, paramTypes);
+		return new InvocableHandlerMethod(this.controller, method);
 	}
 
+
 	@SessionAttributes("sessionAttr") @SuppressWarnings("unused")
 	private static class TestController {