Add ModelFactory test for HttpSessionRequiredException

Author: rstoyanchev

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

@@ -33,6 +33,7 @@
 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;
@@ -41,10 +42,11 @@
 import org.springframework.web.method.support.ModelAndViewContainer;
 
 /**
- * Provides methods to create and update a model in the context of a given request.
- *  
+ * Contains methods for creating and updating a model. A {@link ModelFactory} is associated with a specific controller
+ * through knowledge of its @{@link ModelAttribute} methods and @{@link SessionAttributes}. 
+ * 
  * <p>{@link #initModel(NativeWebRequest, ModelAndViewContainer, HandlerMethod)} populates the model
- * with handler session attributes and attributes from model attribute methods.
+ * with handler session attributes and by invoking model attribute methods.
  * 
  * <p>{@link #updateModel(NativeWebRequest, ModelAndViewContainer, SessionStatus)} updates
  * the model (usually after the {@link RequestMapping} method has been called) promoting attributes
@@ -70,19 +72,19 @@ public final class ModelFactory {
 	public ModelFactory(List<InvocableHandlerMethod> attributeMethods,
 						WebDataBinderFactory binderFactory,
 						SessionAttributesHandler sessionHandler) {
-		this.attributeMethods = attributeMethods;
+		this.attributeMethods = (attributeMethods != null) ? attributeMethods : new ArrayList<InvocableHandlerMethod>();
 		this.binderFactory = binderFactory;
 		this.sessionHandler = sessionHandler;
 	}
 
 	/**
 	 * Populate the model for a request with attributes obtained in the following order:
 	 * <ol>
-	 * <li>Add known (i.e. previously accessed) handler session attributes
-	 * <li>Invoke model attribute methods
-	 * <li>Check if any {@link ModelAttribute}-annotated arguments need to be added from the session
+	 * <li>Retrieve "known" (i.e. have been in the model in prior requests) handler session attributes from the session 
+	 * <li>Create attributes by invoking model attribute methods
+	 * <li>Check for not yet known handler session attributes in the session  
 	 * </ol>
-	 * <p>As a general rule model attributes are added only once.
+	 * <p>As a general rule model attributes are added only once following the above order.
 	 * 
 	 * @param request the current request
 	 * @param mavContainer the {@link ModelAndViewContainer} to add model attributes to
@@ -97,7 +99,7 @@ public void initModel(NativeWebRequest request, ModelAndViewContainer mavContain
 
 		invokeAttributeMethods(request, mavContainer);
 
-		addSessionAttributesByName(request, mavContainer, requestMethod);
+		checkMissingSessionAttributes(request, mavContainer, requestMethod);
 	}
 
 	/**
@@ -123,21 +125,26 @@ private void invokeAttributeMethods(NativeWebRequest request, ModelAndViewContai
 	}
 
 	/**
-	 * Check if {@link ModelAttribute}-annotated arguments are handler session attributes and add them from the session. 
+	 * Checks if any {@link ModelAttribute}-annotated handler method arguments are eligible as handler session 
+	 * attributes, as defined by @{@link SessionAttributes}, and are not yet present in the model. 
+	 * If so, attempts to retrieve them from the session and add them to the model.
+	 * 
+	 * @throws HttpSessionRequiredException raised if a handler session attribute could is missing 
 	 */
-	private void addSessionAttributesByName(NativeWebRequest request, ModelAndViewContainer mavContainer, HandlerMethod requestMethod) {
+	private void checkMissingSessionAttributes(NativeWebRequest request, 
+											   ModelAndViewContainer mavContainer, 
+											   HandlerMethod requestMethod) throws HttpSessionRequiredException {
 		for (MethodParameter parameter : requestMethod.getMethodParameters()) {
 			if (parameter.hasParameterAnnotation(ModelAttribute.class)) {
-				continue;
-			}
-			String attrName = getNameForParameter(parameter);
-			if (!mavContainer.containsAttribute(attrName)) {
-				if (sessionHandler.isHandlerSessionAttribute(attrName, parameter.getParameterType())) {
-					Object attrValue = sessionHandler.retrieveAttribute(request, attrName);
-					if (attrValue == null){
-						new HttpSessionRequiredException("Session attribute '" + attrName + "' not found in session");
+				String name = getNameForParameter(parameter);
+				if (!mavContainer.containsAttribute(name)) {
+					if (sessionHandler.isHandlerSessionAttribute(name, parameter.getParameterType())) {
+						Object attrValue = sessionHandler.retrieveAttribute(request, name);
+						if (attrValue == null){
+							throw new HttpSessionRequiredException("Session attribute '" + name + "' not found in session");
+						}
+						mavContainer.addAttribute(name, attrValue);
 					}
-					mavContainer.addAttribute(attrName, attrValue);
 				}
 			}
 		}

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

@@ -38,9 +38,9 @@
  * Resolves method arguments annotated with @{@link ModelAttribute}. Or if created in default resolution mode,
  * resolves any non-simple type argument even without an @{@link ModelAttribute}. See the constructor for details.
  *
- * <p>A model attribute argument value is obtained from the model or is created using its default constructor.
- * Data binding and optionally validation is then applied through a {@link WebDataBinder} instance. Validation is
- * invoked optionally when the argument is annotated with an {@code @Valid}.
+ * <p>A model attribute argument is obtained from the model or otherwise is created with a default constructor.
+ * Data binding and validation are applied through a {@link WebDataBinder} instance. Validation is applied 
+ * only when the argument is also annotated with {@code @Valid}.
  *
  * <p>Also handles return values from methods annotated with an @{@link ModelAttribute}. The return value is
  * added to the {@link ModelAndViewContainer}.

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

@@ -24,6 +24,7 @@
 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 java.lang.reflect.Method;
 import java.util.Arrays;
@@ -34,6 +35,7 @@
 import org.springframework.mock.web.MockHttpServletRequest;
 import org.springframework.ui.Model;
 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.SessionAttributes;
@@ -59,6 +61,8 @@ public class ModelFactoryTests {
 
 	private InvocableHandlerMethod handleMethod;
 
+	private InvocableHandlerMethod handleSessionAttrMethod;
+
 	private SessionAttributesHandler handlerSessionAttributeStore;
 
 	private SessionAttributeStore sessionAttributeStore;
@@ -69,67 +73,77 @@ public class ModelFactoryTests {
 
 	@Before
 	public void setUp() throws Exception {
-		handleMethod = new InvocableHandlerMethod(handler, handler.getClass().getDeclaredMethod("handle"));
+		Class<?> handlerType = handler.getClass();
+		handleMethod = new InvocableHandlerMethod(handler, handlerType.getDeclaredMethod("handle"));
+		Method method = handlerType.getDeclaredMethod("handleSessionAttr", String.class);
+		handleSessionAttrMethod = new InvocableHandlerMethod(handler, method);
 		sessionAttributeStore = new DefaultSessionAttributeStore();
-		handlerSessionAttributeStore = new SessionAttributesHandler(handler.getClass(), sessionAttributeStore);
+		handlerSessionAttributeStore = new SessionAttributesHandler(handlerType, sessionAttributeStore);
 		mavContainer = new ModelAndViewContainer();
 		webRequest = new ServletWebRequest(new MockHttpServletRequest());
 	}
 
 	@Test
-	public void createModel() throws Exception {
-		ModelFactory modelFactory = createModelFactory("model", Model.class);
+	public void addAttributeToModel() throws Exception {
+		ModelFactory modelFactory = createModelFactory("modelAttr", Model.class);
 		modelFactory.initModel(webRequest, mavContainer, handleMethod);
 
-		assertEquals(Boolean.TRUE, mavContainer.getAttribute("model"));
+		assertEquals(Boolean.TRUE, mavContainer.getAttribute("modelAttr"));
 	}
 
 	@Test
-	public void createModelWithName() throws Exception {
-		ModelFactory modelFactory = createModelFactory("modelWithName");
+	public void returnAttributeWithName() throws Exception {
+		ModelFactory modelFactory = createModelFactory("modelAttrWithName");
 		modelFactory.initModel(webRequest, mavContainer, handleMethod);
 
 		assertEquals(Boolean.TRUE, mavContainer.getAttribute("name"));
 	}
 
 	@Test
-	public void createModelWithDefaultName() throws Exception {
-		ModelFactory modelFactory = createModelFactory("modelWithDefaultName");
+	public void returnAttributeWithNameByConvention() throws Exception {
+		ModelFactory modelFactory = createModelFactory("modelAttrConvention");
 		modelFactory.initModel(webRequest, mavContainer, handleMethod);
 
 		assertEquals(Boolean.TRUE, mavContainer.getAttribute("boolean"));
 	}
 
 	@Test
-	public void createModelWithExistingName() throws Exception {
-		ModelFactory modelFactory = createModelFactory("modelWithName");
-		modelFactory.initModel(webRequest, mavContainer, handleMethod);
-
-		assertEquals(Boolean.TRUE, mavContainer.getAttribute("name"));
-	}
-
-	@Test
-	public void createModelWithNullAttribute() throws Exception {
-		ModelFactory modelFactory = createModelFactory("modelWithNullAttribute");
+	public void returnNullAttributeValue() throws Exception {
+		ModelFactory modelFactory = createModelFactory("nullModelAttr");
 		modelFactory.initModel(webRequest, mavContainer, handleMethod);
 
 		assertTrue(mavContainer.containsAttribute("name"));
 		assertNull(mavContainer.getAttribute("name"));
 	}
 
 	@Test
-	public void createModelExistingSessionAttributes() throws Exception {
-		sessionAttributeStore.storeAttribute(webRequest, "sessAttr", "sessAttrValue");
+	public void retrieveAttributeFromSession() throws Exception {
+		sessionAttributeStore.storeAttribute(webRequest, "sessionAttr", "sessionAttrValue");
 
 		// Resolve successfully handler session attribute once
-		assertTrue(handlerSessionAttributeStore.isHandlerSessionAttribute("sessAttr", null));
+		assertTrue(handlerSessionAttributeStore.isHandlerSessionAttribute("sessionAttr", null));
 
-		ModelFactory modelFactory = createModelFactory("model", Model.class);
+		ModelFactory modelFactory = createModelFactory("modelAttr", Model.class);
 		modelFactory.initModel(webRequest, mavContainer, handleMethod);
 
-		assertEquals("sessAttrValue", mavContainer.getAttribute("sessAttr"));
+		assertEquals("sessionAttrValue", mavContainer.getAttribute("sessionAttr"));
 	}
-	
+
+	@Test
+	public void requiredSessionAttribute() throws Exception {
+		ModelFactory modelFactory = new ModelFactory(null, null, handlerSessionAttributeStore);
+
+		try {
+			modelFactory.initModel(webRequest, mavContainer, handleSessionAttrMethod);
+			fail("Expected HttpSessionRequiredException");
+		} catch (HttpSessionRequiredException e) { }
+		
+		sessionAttributeStore.storeAttribute(webRequest, "sessionAttr", "sessionAttrValue");
+		modelFactory.initModel(webRequest, mavContainer, handleSessionAttrMethod);
+
+		assertEquals("sessionAttrValue", mavContainer.getAttribute("sessionAttr"));
+	}
+
 	@Test
 	public void updateBindingResult() throws Exception {
 		String attrName = "attr1";
@@ -170,35 +184,33 @@ private ModelFactory createModelFactory(String methodName, Class<?>... parameter
 		return new ModelFactory(Arrays.asList(handlerMethod), null, handlerSessionAttributeStore);
 	}
 
-	@SessionAttributes("sessAttr")
+	@SessionAttributes("sessionAttr") @SuppressWarnings("unused")
 	private static class ModelHandler {
 
-		@SuppressWarnings("unused")
 		@ModelAttribute
-		public void model(Model model) {
-			model.addAttribute("model", Boolean.TRUE);
+		public void modelAttr(Model model) {
+			model.addAttribute("modelAttr", Boolean.TRUE);
 		}
 
-		@SuppressWarnings("unused")
 		@ModelAttribute("name")
-		public Boolean modelWithName() {
+		public Boolean modelAttrWithName() {
 			return Boolean.TRUE;
 		}
 
-		@SuppressWarnings("unused")
 		@ModelAttribute
-		public Boolean modelWithDefaultName() {
+		public Boolean modelAttrConvention() {
 			return Boolean.TRUE;
 		}
 
-		@SuppressWarnings("unused")
 		@ModelAttribute("name")
-		public Boolean modelWithNullAttribute() {
+		public Boolean nullModelAttr() {
 			return null;
 		}
 
-		@SuppressWarnings("unused")
 		public void handle() {
 		}
+
+		public void handleSessionAttr(@ModelAttribute("sessionAttr") String sessionAttr) {
+		}
 	}
 }