Polish synchronization of model attributes with the session.

org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/mvc/method/annotation/support/ServletModelAttributeMethodProcessor.java

@@ -50,10 +50,9 @@ import org.springframework.web.servlet.mvc.method.annotation.ServletRequestDataB
 public class ServletModelAttributeMethodProcessor extends ModelAttributeMethodProcessor {
 
 	/**
-	 * @param annotationNotRequired if {@code true}, any non-simple type 
+	 * @param annotationNotRequired if "true", non-simple method arguments and
-	 * argument or return value is regarded as a model attribute even without 
+	 * return values are considered model attributes with or without a 
-	 * the presence of a {@code @ModelAttribute} annotation in which case the 
+	 * {@code @ModelAttribute} annotation.
-	 * attribute name is derived from the model attribute's type.
 	 */
 	public ServletModelAttributeMethodProcessor(boolean annotationNotRequired) {
 		super(annotationNotRequired);

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

@@ -33,9 +33,7 @@ import org.springframework.validation.BindingResult;
 import org.springframework.web.HttpSessionRequiredException;
 import org.springframework.web.bind.WebDataBinder;
 import org.springframework.web.bind.annotation.ModelAttribute;
-import org.springframework.web.bind.annotation.RequestMapping;
 import org.springframework.web.bind.annotation.SessionAttributes;
-import org.springframework.web.bind.support.SessionStatus;
 import org.springframework.web.bind.support.WebDataBinderFactory;
 import org.springframework.web.context.request.NativeWebRequest;
 import org.springframework.web.method.HandlerMethod;
@@ -43,16 +41,12 @@ import org.springframework.web.method.support.InvocableHandlerMethod;
 import org.springframework.web.method.support.ModelAndViewContainer;
 
 /**
- * Provides methods to initialize the {@link Model} before a controller method 
+ * Provides methods to initialize the {@link Model} before controller method 
- * invocation and to update it after the controller method has been invoked.
+ * invocation and to update it afterwards. On initialization, the model is 
- * 
+ * populated with attributes from the session or by invoking 
- * <p>On initialization the model may be populated with session attributes 
+ * {@code @ModelAttribute} methods. On update, model attributes are 
- * stored during a previous request as a result of a {@link SessionAttributes} 
+ * synchronized with the session -- either adding or removing them. 
- * annotation. @{@link ModelAttribute} methods in the same controller may 
+ * Also {@link BindingResult} attributes where missing.
- * also be invoked to populate the model.
- * 
- * <p>On update attributes may be removed from or stored in the session.
- * {@link BindingResult} attributes may also be added as necessary.
  * 
  * @author Rossen Stoyanchev
  * @since 3.1
@@ -66,10 +60,10 @@ public final class ModelFactory {
 	private final SessionAttributesHandler sessionAttributesHandler;
 	
 	/**
-	 * Create a ModelFactory instance with the provided {@link ModelAttribute} methods.
+	 * Create a new instance with the given {@code @ModelAttribute} methods.
-	 * @param attributeMethods {@link ModelAttribute} methods to initialize model instances with
+	 * @param attributeMethods for model initialization
-	 * @param binderFactory used to add {@link BindingResult} attributes to the model
+	 * @param binderFactory for adding {@link BindingResult} attributes
-	 * @param sessionAttributesHandler used to access handler-specific session attributes
+	 * @param sessionAttributesHandler for access to session attributes
 	 */
 	public ModelFactory(List<InvocableHandlerMethod> attributeMethods,
 						WebDataBinderFactory binderFactory,
@@ -82,29 +76,40 @@ public final class ModelFactory {
 	/**
 	 * Populate the model in the following order:
 	 * <ol>
-	 * 	<li>Retrieve "remembered" (i.e. previously stored) controller-specific session attributes
+	 * 	<li>Retrieve "known" session attributes -- i.e. attributes listed via 
-	 * 	<li>Invoke @{@link ModelAttribute} methods
+	 * 	{@link SessionAttributes @SessionAttributes} and previously stored in 
-	 * 	<li>Check the session for any controller-specific attributes not yet "remembered".
+	 * 	the in the model at least once
+	 * 	<li>Invoke {@link ModelAttribute @ModelAttribute} methods
+	 * 	<li>Find method arguments eligible as session attributes and retrieve
+	 * 	them if they're not	already	present in the model 
 	 * </ol>
 	 * @param request the current request
-	 * @param mavContainer contains the model to initialize 
+	 * @param mavContainer contains the model to be initialized 
-	 * @param handlerMethod the @{@link RequestMapping} method for which the model is initialized
+	 * @param handlerMethod the method for which the model is initialized
-	 * @throws Exception may arise from the invocation of @{@link ModelAttribute} methods
+	 * @throws Exception may arise from {@code @ModelAttribute} methods
 	 */
 	public void initModel(NativeWebRequest request, ModelAndViewContainer mavContainer, HandlerMethod handlerMethod)
 			throws Exception {
 
-		Map<String, ?> sessionAttrs = this.sessionAttributesHandler.retrieveAttributes(request);
+		Map<String, ?> attributesInSession = this.sessionAttributesHandler.retrieveAttributes(request);
-		mavContainer.mergeAttributes(sessionAttrs);
+		mavContainer.mergeAttributes(attributesInSession);
 
 		invokeModelAttributeMethods(request, mavContainer);
 
-		checkHandlerSessionAttributes(request, mavContainer, handlerMethod);
+		for (String name : findSessionAttributeArguments(handlerMethod)) {
+			if (!mavContainer.containsAttribute(name)) {
+				Object value = this.sessionAttributesHandler.retrieveAttribute(request, name);
+				if (value == null) {
+					throw new HttpSessionRequiredException("Expected session attribute '" + name + "'");
+				}
+				mavContainer.addAttribute(name, value);
+			}
+		}
 	}
 
 	/**
-	 * Invoke model attribute methods to populate the model. 
+	 * Invoke model attribute methods to populate the model. Attributes are 
-	 * If two methods return the same attribute, the attribute from the first method is added.
+	 * added only if not already present in the model.
 	 */
 	private void invokeModelAttributeMethods(NativeWebRequest request, ModelAndViewContainer mavContainer)
 			throws Exception {
@@ -127,41 +132,29 @@ public final class ModelFactory {
 	}
 	
 	/**
-	 * Checks for @{@link ModelAttribute} arguments in the signature of the 
+	 * Return all {@code @ModelAttribute} arguments declared as session 
-	 * {@link RequestMapping} method that are declared as session attributes 
+	 * attributes via {@code @SessionAttributes}.
-	 * via @{@link SessionAttributes} but are not already in the model. 
-	 * Those attributes may have been outside of this controller. 
-	 * Try to locate the attributes in the session or raise an exception. 
-	 * 
-	 * @throws HttpSessionRequiredException raised if an attribute declared
-	 * as session attribute is missing.
 	 */
-	private void checkHandlerSessionAttributes(NativeWebRequest request, 
+	private List<String> findSessionAttributeArguments(HandlerMethod handlerMethod) {
-											   ModelAndViewContainer mavContainer, 
+		List<String> result = new ArrayList<String>();
-											   HandlerMethod handlerMethod) throws HttpSessionRequiredException {
+		for (MethodParameter param : handlerMethod.getMethodParameters()) {
-		for (MethodParameter parameter : handlerMethod.getMethodParameters()) {
+			if (param.hasParameterAnnotation(ModelAttribute.class)) {
-			if (parameter.hasParameterAnnotation(ModelAttribute.class)) {
+				String name = getNameForParameter(param);
-				String attrName = getNameForParameter(parameter);
+				if (this.sessionAttributesHandler.isHandlerSessionAttribute(name, param.getParameterType())) {
-				if (!mavContainer.containsAttribute(attrName)) {
+					result.add(name);
-					if (sessionAttributesHandler.isHandlerSessionAttribute(attrName, parameter.getParameterType())) {
-						Object attrValue = sessionAttributesHandler.retrieveAttribute(request, attrName);
-						if (attrValue == null){
-							throw new HttpSessionRequiredException(
-									"Session attribute '" + attrName + "' not found in session: " + handlerMethod);
-						}
-						mavContainer.addAttribute(attrName, attrValue);
-					}
 				}
 			}
 		}
+		return result;
 	}
 	
 	/**
-	 * Derive the model attribute name for the given return value using one of the following:
+	 * Derive the model attribute name for the given return value using 
+	 * one of the following:
 	 * <ol>
-	 * <li>The method {@link ModelAttribute} annotation value 
+	 * 	<li>The method {@code ModelAttribute} annotation value 
-	 * <li>The name of the return type
+	 * 	<li>The declared return type if it is other than {@code Object}
-	 * <li>The name of the return value type if the method return type is {@code Object} 
+	 * 	<li>The actual return value type
 	 * </ol>
 	 * @param returnValue the value returned from a method invocation
 	 * @param returnType the return type of the method
@@ -180,12 +173,12 @@ public final class ModelFactory {
 	}
 
 	/**
-	 * Derives the model attribute name for the given method parameter using one of the following:
+	 * Derives the model attribute name for a method parameter based on:
 	 * <ol>
-	 * <li>The parameter {@link ModelAttribute} annotation value
+	 * 	<li>The parameter {@code @ModelAttribute} annotation value
-	 * <li>The name of the parameter type 
+	 * 	<li>The parameter type 
 	 * </ol>
-	 * @return the method parameter model name, never {@code null} or an empty string
+	 * @return the derived name; never {@code null} or an empty string
 	 */
 	public static String getNameForParameter(MethodParameter parameter) {
 		ModelAttribute annot = parameter.getParameterAnnotation(ModelAttribute.class);
@@ -194,11 +187,11 @@ public final class ModelFactory {
 	}
 	
 	/**
-	 * Updates the model by cleaning handler session attributes depending on {@link SessionStatus#isComplete()},
+	 * Synchronize model attributes with the session. Add {@link BindingResult}
-	 * promotes model attributes to the session, and adds {@link BindingResult} attributes where missing.
+	 * attributes where necessary.
 	 * @param request the current request
-	 * @param mavContainer the {@link ModelAndViewContainer} for the current request
+	 * @param mavContainer contains the model to update
-	 * @throws Exception if the process of creating {@link BindingResult} attributes causes an error
+	 * @throws Exception if creating BindingResult attributes fails
 	 */
 	public void updateModel(NativeWebRequest request, ModelAndViewContainer mavContainer) throws Exception {
 		
@@ -234,7 +227,7 @@ public final class ModelFactory {
 	}
 
 	/**
-	 * Whether the given attribute requires a {@link BindingResult} added to the model.
+	 * Whether the given attribute requires a {@link BindingResult} in the model.
 	 */
 	private boolean isBindingCandidate(String attributeName, Object value) {
 		if (attributeName.startsWith(BindingResult.MODEL_KEY_PREFIX)) {

org.springframework.web/src/main/java/org/springframework/web/method/annotation/SessionAttributesHandler.java

@@ -31,16 +31,15 @@ import org.springframework.web.bind.support.SessionStatus;
 import org.springframework.web.context.request.WebRequest;
 
 /**
- * Manages handler-specific session attributes declared via @{@link SessionAttributes}. 
+ * Manages controller-specific session attributes declared via 
- * Actual storage is performed through an instance of {@link SessionAttributeStore}.
+ * {@link SessionAttributes @SessionAttributes}. Actual storage is 
+ * performed via {@link SessionAttributeStore}.
  * 
- * <p>A typical scenario begins with a controller adding attributes to the 
+ * <p>When a controller annotated with {@code @SessionAttributes} adds 
- * {@link org.springframework.ui.Model Model}. At the end of the request, model 
+ * attributes to its model, those attributes are checked against names and 
- * attributes are checked to see if any of them match the names and types declared 
+ * types specified via {@code @SessionAttributes}. Matching model attributes 
- * via @{@link SessionAttributes}. Matching model attributes are "promoted" to 
+ * are saved in the HTTP session and remain there until the controller calls 
- * the session and remain there until the controller calls 
+ * {@link SessionStatus#setComplete()}.
- * {@link SessionStatus#setComplete()} to indicate the session attributes are
- * no longer needed and can be removed. 
  * 
  * @author Rossen Stoyanchev
  * @since 3.1
@@ -56,11 +55,9 @@ public class SessionAttributesHandler {
 	private final SessionAttributeStore sessionAttributeStore;
 
 	/**
-	 * Creates a {@link SessionAttributesHandler} instance for the specified handler type 
+	 * Creates a new instance for a controller type. Session attribute names/types
-	 * Inspects the given handler type for the presence of an @{@link SessionAttributes} 
+	 * are extracted from a type-level {@code @SessionAttributes} if found.
-	 * and stores that information to identify model attribute that need to be stored, 
+	 * @param handlerType the controller type
-	 * retrieved, or removed from the session.
-	 * @param handlerType the handler type to inspect for a {@link SessionAttributes} annotation
 	 * @param sessionAttributeStore used for session access
 	 */
 	public SessionAttributesHandler(Class<?> handlerType, SessionAttributeStore sessionAttributeStore) {
@@ -75,25 +72,24 @@ public class SessionAttributesHandler {
 	}
 
 	/**
-	 * Whether the controller represented by this handler has declared session 
+	 * Whether the controller represented by this instance has declared session 
-	 * attribute names or types of interest via @{@link SessionAttributes}. 
+	 * attribute names or types of interest via {@link SessionAttributes}. 
 	 */
 	public boolean hasSessionAttributes() {
 		return ((this.attributeNames.size() > 0) || (this.attributeTypes.size() > 0)); 
 	}
 	
 	/**
-	 * Whether the controller represented by this instance has declared a specific 
+	 * Whether the attribute name and/or type match those specified in the
-	 * attribute as a session attribute via @{@link SessionAttributes}. 
+	 * controller's {@code @SessionAttributes} annotation. 
 	 * 
-	 * <p>Attributes successfully resolved through this method are "remembered" and
+	 * <p>Attributes successfully resolved through this method are "remembered"
-	 * used by calls to {@link #retrieveAttributes(WebRequest)} and 
+	 * and used in {@link #retrieveAttributes(WebRequest)} and 
-	 * {@link #cleanupAttributes(WebRequest)}. In other words unless attributes 
+	 * {@link #cleanupAttributes(WebRequest)}. In other words, retrieval and 
-	 * have been resolved and stored before, retrieval and cleanup have no impact.
+	 * cleanup only affect attributes previously resolved through here.
 	 * 
-	 * @param attributeName the attribute name to check, must not be null
+	 * @param attributeName the attribute name to check; must not be null
-	 * @param attributeType the type for the attribute, not required but should be provided when
+	 * @param attributeType the type for the attribute; or {@code null}
-	 * available as session attributes of interest can be matched by type
 	 */
 	public boolean isHandlerSessionAttribute(String attributeName, Class<?> attributeType) {
 		Assert.notNull(attributeName, "Attribute name must not be null");
@@ -108,7 +104,7 @@ public class SessionAttributesHandler {
 
 	/**
 	 * Stores a subset of the given attributes in the session. Attributes not 
-	 * declared as session attributes via @{@link SessionAttributes} are ignored. 
+	 * declared as session attributes via {@code @SessionAttributes} are ignored. 
 	 * @param request the current request
 	 * @param attributes candidate attributes for session storage
 	 */
@@ -124,8 +120,9 @@ public class SessionAttributesHandler {
 	}
 	
 	/**
-	 * Retrieves "remembered" (i.e. previously stored) session attributes 
+	 * Retrieve "known" attributes from the session -- i.e. attributes listed 
-	 * for the controller represented by this handler.
+	 * in {@code @SessionAttributes} and previously stored in the in the model 
+	 * at least once. 
 	 * @param request the current request
 	 * @return a map with handler session attributes; possibly empty.
 	 */
@@ -141,8 +138,9 @@ public class SessionAttributesHandler {
 	}
 
 	/**
-	 * Cleans "remembered" (i.e. previously stored) session attributes  
+	 * Cleans "known" attributes from the session - i.e. attributes listed
-	 * for the controller represented by this handler.
+	 * in {@code @SessionAttributes} and previously stored in the in the model 
+	 * at least once.
 	 * @param request the current request
 	 */
 	public void cleanupAttributes(WebRequest request) {

org.springframework.web/src/main/java/org/springframework/web/method/annotation/support/ModelAttributeMethodProcessor.java

@@ -58,10 +58,9 @@ public class ModelAttributeMethodProcessor implements HandlerMethodArgumentResol
 	private final boolean annotationNotRequired;
 	
 	/**
-	 * @param annotationNotRequired if {@code true}, any non-simple type 
+	 * @param annotationNotRequired if "true", non-simple method arguments and
-	 * argument or return value is regarded as a model attribute even without 
+	 * return values are considered model attributes with or without a 
-	 * the presence of a {@code @ModelAttribute} annotation in which case the 
+	 * {@code @ModelAttribute} annotation.
-	 * attribute name is derived from the model attribute's type.
 	 */
 	public ModelAttributeMethodProcessor(boolean annotationNotRequired) {
 		this.annotationNotRequired = annotationNotRequired;