diff --git a/org.springframework.web/src/main/java/org/springframework/web/method/annotation/ModelFactory.java b/org.springframework.web/src/main/java/org/springframework/web/method/annotation/ModelFactory.java index 414e59be911..e9ee775c81d 100644 --- a/org.springframework.web/src/main/java/org/springframework/web/method/annotation/ModelFactory.java +++ b/org.springframework.web/src/main/java/org/springframework/web/method/annotation/ModelFactory.java @@ -33,6 +33,7 @@ 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; @@ -41,10 +42,11 @@ import org.springframework.web.method.support.InvocableHandlerMethod; 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}. + * *

{@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. * *

{@link #updateModel(NativeWebRequest, ModelAndViewContainer, SessionStatus)} updates * the model (usually after the {@link RequestMapping} method has been called) promoting attributes @@ -70,7 +72,7 @@ public final class ModelFactory { public ModelFactory(List attributeMethods, WebDataBinderFactory binderFactory, SessionAttributesHandler sessionHandler) { - this.attributeMethods = attributeMethods; + this.attributeMethods = (attributeMethods != null) ? attributeMethods : new ArrayList(); this.binderFactory = binderFactory; this.sessionHandler = sessionHandler; } @@ -78,11 +80,11 @@ public final class ModelFactory { /** * Populate the model for a request with attributes obtained in the following order: *

    - *
  1. Add known (i.e. previously accessed) handler session attributes - *
  2. Invoke model attribute methods - *
  3. Check if any {@link ModelAttribute}-annotated arguments need to be added from the session + *
  4. Retrieve "known" (i.e. have been in the model in prior requests) handler session attributes from the session + *
  5. Create attributes by invoking model attribute methods + *
  6. Check for not yet known handler session attributes in the session *
- *

As a general rule model attributes are added only once. + *

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 final class ModelFactory { invokeAttributeMethods(request, mavContainer); - addSessionAttributesByName(request, mavContainer, requestMethod); + checkMissingSessionAttributes(request, mavContainer, requestMethod); } /** @@ -123,21 +125,26 @@ public final class ModelFactory { } /** - * 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); } } } diff --git a/org.springframework.web/src/main/java/org/springframework/web/method/annotation/support/ModelAttributeMethodProcessor.java b/org.springframework.web/src/main/java/org/springframework/web/method/annotation/support/ModelAttributeMethodProcessor.java index 7b8a0c2d737..9633857675b 100644 --- a/org.springframework.web/src/main/java/org/springframework/web/method/annotation/support/ModelAttributeMethodProcessor.java +++ b/org.springframework.web/src/main/java/org/springframework/web/method/annotation/support/ModelAttributeMethodProcessor.java @@ -38,9 +38,9 @@ import org.springframework.web.method.support.ModelAndViewContainer; * 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. * - *

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}. + *

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}. * *

Also handles return values from methods annotated with an @{@link ModelAttribute}. The return value is * added to the {@link ModelAndViewContainer}. diff --git a/org.springframework.web/src/test/java/org/springframework/web/method/annotation/ModelFactoryTests.java b/org.springframework.web/src/test/java/org/springframework/web/method/annotation/ModelFactoryTests.java index 7ab27f05402..bfd2e9e5cae 100644 --- a/org.springframework.web/src/test/java/org/springframework/web/method/annotation/ModelFactoryTests.java +++ b/org.springframework.web/src/test/java/org/springframework/web/method/annotation/ModelFactoryTests.java @@ -24,6 +24,7 @@ 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 java.lang.reflect.Method; import java.util.Arrays; @@ -34,6 +35,7 @@ import org.springframework.core.LocalVariableTableParameterNameDiscoverer; 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,48 +73,43 @@ 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")); @@ -118,18 +117,33 @@ public class ModelFactoryTests { } @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 @@ public class ModelFactoryTests { 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) { + } } }