revised Portlet SessionStatus.setComplete() to avoid re-exposure of attributes in render phase (SPR-6126); shortened implicit model render parameter name to "implicitModel" (SPR-7149)

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

@@ -123,7 +123,10 @@ import org.springframework.web.servlet.mvc.annotation.ModelAndViewResolver;
 public class AnnotationMethodHandlerAdapter extends PortletContentGenerator
 		implements HandlerAdapter, Ordered, BeanFactoryAware {
 
-	private static final String IMPLICIT_MODEL_ATTRIBUTE = "org.springframework.web.portlet.mvc.ImplicitModel";
+	public static final String IMPLICIT_MODEL_SESSION_ATTRIBUTE =
+			AnnotationMethodHandlerAdapter.class.getName() + ".IMPLICIT_MODEL";
+
+	public static final String IMPLICIT_MODEL_RENDER_PARAMETER = "implicitModel";
 
 
 	private WebBindingInitializer webBindingInitializer;
@@ -303,10 +306,15 @@ public class AnnotationMethodHandlerAdapter extends PortletContentGenerator
 		if (response instanceof MimeResponse) {
 			MimeResponse mimeResponse = (MimeResponse) response;
 			// Detect implicit model from associated action phase.
-			if (request.getParameter(IMPLICIT_MODEL_ATTRIBUTE) != null) {
+			if (response instanceof RenderResponse) {
 				PortletSession session = request.getPortletSession(false);
 				if (session != null) {
-					implicitModel = (ExtendedModelMap) session.getAttribute(IMPLICIT_MODEL_ATTRIBUTE);
+					if (request.getParameter(IMPLICIT_MODEL_RENDER_PARAMETER) != null) {
+						implicitModel = (ExtendedModelMap) session.getAttribute(IMPLICIT_MODEL_SESSION_ATTRIBUTE);
+					}
+					else {
+						session.removeAttribute(IMPLICIT_MODEL_SESSION_ATTRIBUTE);
+					}
 				}
 			}
 			if (handler.getClass().getAnnotation(SessionAttributes.class) != null) {
@@ -356,8 +364,8 @@ public class AnnotationMethodHandlerAdapter extends PortletContentGenerator
 		if (response instanceof ActionResponse && !implicitModel.isEmpty()) {
 			ActionResponse actionResponse = (ActionResponse) response;
 			try {
-				actionResponse.setRenderParameter(IMPLICIT_MODEL_ATTRIBUTE, Boolean.TRUE.toString());
-				request.getPortletSession().setAttribute(IMPLICIT_MODEL_ATTRIBUTE, implicitModel);
+				actionResponse.setRenderParameter(IMPLICIT_MODEL_RENDER_PARAMETER, Boolean.TRUE.toString());
+				request.getPortletSession().setAttribute(IMPLICIT_MODEL_SESSION_ATTRIBUTE, implicitModel);
 			}
 			catch (IllegalStateException ex) {
 				// Probably sendRedirect called... no need to expose model to render phase.

org.springframework.web.portlet/src/test/java/org/springframework/web/portlet/mvc/annotation/Portlet20AnnotationControllerTests.java

@@ -70,11 +70,14 @@ import org.springframework.validation.BindingResult;
 import org.springframework.validation.Errors;
 import org.springframework.web.bind.WebDataBinder;
 import org.springframework.web.bind.annotation.CookieValue;
+import org.springframework.web.bind.annotation.ExceptionHandler;
 import org.springframework.web.bind.annotation.InitBinder;
 import org.springframework.web.bind.annotation.ModelAttribute;
 import org.springframework.web.bind.annotation.RequestHeader;
 import org.springframework.web.bind.annotation.RequestMapping;
 import org.springframework.web.bind.annotation.RequestParam;
+import org.springframework.web.bind.annotation.SessionAttributes;
+import org.springframework.web.bind.support.SessionStatus;
 import org.springframework.web.bind.support.WebArgumentResolver;
 import org.springframework.web.bind.support.WebBindingInitializer;
 import org.springframework.web.context.WebApplicationContext;
@@ -83,6 +86,7 @@ import org.springframework.web.context.request.WebRequest;
 import org.springframework.web.context.support.GenericWebApplicationContext;
 import org.springframework.web.portlet.DispatcherPortlet;
 import org.springframework.web.portlet.ModelAndView;
+import org.springframework.web.portlet.bind.MissingPortletRequestParameterException;
 import org.springframework.web.portlet.bind.annotation.ActionMapping;
 import org.springframework.web.portlet.bind.annotation.EventMapping;
 import org.springframework.web.portlet.bind.annotation.RenderMapping;
@@ -129,6 +133,11 @@ public class Portlet20AnnotationControllerTests {
 		doTestAdaptedHandleMethods(MyAdaptedController3.class);
 	}
 
+	@Test
+	public void adaptedHandleMethods4() throws Exception {
+		doTestAdaptedHandleMethods(MyAdaptedController4.class);
+	}
+
 	private void doTestAdaptedHandleMethods(final Class controllerClass) throws Exception {
 		DispatcherPortlet portlet = new DispatcherPortlet() {
 			protected ApplicationContext createPortletApplicationContext(ApplicationContext parent) throws BeansException {
@@ -145,12 +154,22 @@ public class Portlet20AnnotationControllerTests {
 		portlet.processAction(actionRequest, actionResponse);
 		assertEquals("value", actionResponse.getRenderParameter("test"));
 
-		MockRenderRequest request = new MockRenderRequest(PortletMode.EDIT);
+		MockRenderRequest request = new MockRenderRequest(PortletMode.VIEW);
+		request.setSession(actionRequest.getPortletSession());
+		request.setParameters(actionResponse.getRenderParameterMap());
+		request.addParameter("name", "name1");
+		request.addParameter("age", "value2");
+		MockRenderResponse response = new MockRenderResponse();
+		portlet.render(request, response);
+		assertEquals("test-name1-typeMismatch", response.getContentAsString());
+		assertNull(request.getPortletSession().getAttribute("testBean"));
+
+		request = new MockRenderRequest(PortletMode.EDIT);
 		request.addParameter("param1", "value1");
 		request.addParameter("param2", "2");
 		request.addProperty("header1", "10");
 		request.setCookies(new Cookie("cookie1", "3"));
-		MockRenderResponse response = new MockRenderResponse();
+		response = new MockRenderResponse();
 		portlet.render(request, response);
 		assertEquals("test-value1-2-10-3", response.getContentAsString());
 
@@ -160,13 +179,6 @@ public class Portlet20AnnotationControllerTests {
 		response = new MockRenderResponse();
 		portlet.render(request, response);
 		assertEquals("test-name1-2", response.getContentAsString());
-
-		request = new MockRenderRequest(PortletMode.VIEW);
-		request.addParameter("name", "name1");
-		request.addParameter("age", "value2");
-		response = new MockRenderResponse();
-		portlet.render(request, response);
-		assertEquals("test-name1-typeMismatch", response.getContentAsString());
 	}
 
 	@Test
@@ -572,6 +584,7 @@ public class Portlet20AnnotationControllerTests {
 
 		request = new MockRenderRequest(PortletMode.VIEW, WindowState.MAXIMIZED);
 		request.setParameters(actionResponse.getRenderParameterMap());
+		request.setSession(actionRequest.getPortletSession());
 		response = new MockRenderResponse();
 		portlet.render(request, response);
 		assertEquals("myLargeView-value", response.getContentAsString());
@@ -583,6 +596,7 @@ public class Portlet20AnnotationControllerTests {
 
 		request = new MockRenderRequest(PortletMode.VIEW, WindowState.MAXIMIZED);
 		request.setParameters(actionResponse.getRenderParameterMap());
+		request.setSession(actionRequest.getPortletSession());
 		response = new MockRenderResponse();
 		portlet.render(request, response);
 		assertEquals("myLargeView-details", response.getContentAsString());
@@ -731,6 +745,43 @@ public class Portlet20AnnotationControllerTests {
 	}
 
 
+	@Controller
+	@SessionAttributes("testBean")
+	private static class MyAdaptedController4 {
+
+		@RequestMapping("VIEW")
+		@ActionMapping
+		public void myHandle(Model model, ActionResponse response, SessionStatus status) {
+			TestBean tb = new TestBean();
+			tb.setJedi(true);
+			model.addAttribute("testBean", tb);
+			status.setComplete();
+			response.setRenderParameter("test", "value");
+		}
+
+		@RequestMapping("EDIT")
+		@RenderMapping
+		public void myHandle(@RequestParam("param1") String p1, int param2, RenderResponse response,
+				@RequestHeader("header1") String h1, @CookieValue("cookie1") String c1) throws IOException {
+			response.getWriter().write("test-" + p1 + "-" + param2 + "-" + h1 + "-" + c1);
+		}
+
+		@RequestMapping("HELP")
+		@RenderMapping
+		public void myHandle(@ModelAttribute("tb") TestBean tb, RenderResponse response) throws IOException {
+			response.getWriter().write("test-" + tb.getName() + "-" + tb.getAge());
+		}
+
+		@RequestMapping("VIEW")
+		@RenderMapping
+		public void myHandle(@ModelAttribute("testBean") TestBean tb, Errors errors, RenderResponse response, PortletSession session) throws IOException {
+			assertTrue(tb.isJedi());
+			assertNull(session.getAttribute("testBean"));
+			response.getWriter().write("test-" + tb.getName() + "-" + errors.getFieldError("age").getCode());
+		}
+	}
+
+
 	@Controller
 	private static class MyFormController {
 

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

@@ -26,7 +26,6 @@ import java.lang.reflect.Type;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
-import java.util.HashSet;
 import java.util.Iterator;
 import java.util.LinkedHashMap;
 import java.util.List;
@@ -95,6 +94,8 @@ import org.springframework.web.multipart.MultipartRequest;
  */
 public class HandlerMethodInvoker {
 
+	private static final String MODEL_KEY_PREFIX_STALE = SessionAttributeStore.class.getName() + ".STALE.";
+
 	/** We'll create a lot of these objects, so we don't want a new logger every time. */
 	private static final Log logger = LogFactory.getLog(HandlerMethodInvoker.class);
 
@@ -197,28 +198,36 @@ public class HandlerMethodInvoker {
 		// Expose model attributes as session attributes, if required.
 		// Expose BindingResults for all attributes, making custom editors available.
 		Map<String, Object> model = (mavModel != null ? mavModel : implicitModel);
-		try {
-			for (String attrName : new HashSet<String>(model.keySet())) {
-				Object attrValue = model.get(attrName);
-				boolean isSessionAttr =
-						this.methodResolver.isSessionAttribute(attrName, (attrValue != null ? attrValue.getClass() : null));
-				if (isSessionAttr && !this.sessionStatus.isComplete()) {
-					this.sessionAttributeStore.storeAttribute(webRequest, attrName, attrValue);
-				}
-				if (!attrName.startsWith(BindingResult.MODEL_KEY_PREFIX) &&
-						(isSessionAttr || isBindingCandidate(attrValue))) {
-					String bindingResultKey = BindingResult.MODEL_KEY_PREFIX + attrName;
-					if (mavModel != null && !model.containsKey(bindingResultKey)) {
-						WebDataBinder binder = createBinder(webRequest, attrValue, attrName);
-						initBinder(handler, attrName, binder, webRequest);
-						mavModel.put(bindingResultKey, binder.getBindingResult());
+		if (model != null) {
+			try {
+				String[] originalAttrNames = model.keySet().toArray(new String[model.size()]);
+				for (String attrName : originalAttrNames) {
+					Object attrValue = model.get(attrName);
+					boolean isSessionAttr = this.methodResolver.isSessionAttribute(
+							attrName, (attrValue != null ? attrValue.getClass() : null));
+					if (isSessionAttr) {
+						if (this.sessionStatus.isComplete()) {
+							implicitModel.put(MODEL_KEY_PREFIX_STALE + attrName, Boolean.TRUE);
+						}
+						else if (!implicitModel.containsKey(MODEL_KEY_PREFIX_STALE + attrName)) {
+							this.sessionAttributeStore.storeAttribute(webRequest, attrName, attrValue);
+						}
+					}
+					if (!attrName.startsWith(BindingResult.MODEL_KEY_PREFIX) &&
+							(isSessionAttr || isBindingCandidate(attrValue))) {
+						String bindingResultKey = BindingResult.MODEL_KEY_PREFIX + attrName;
+						if (mavModel != null && !model.containsKey(bindingResultKey)) {
+							WebDataBinder binder = createBinder(webRequest, attrValue, attrName);
+							initBinder(handler, attrName, binder, webRequest);
+							mavModel.put(bindingResultKey, binder.getBindingResult());
+						}
 					}
 				}
 			}
-		}
-		catch (InvocationTargetException ex) {
-			// User-defined @InitBinder method threw an exception...
-			ReflectionUtils.rethrowException(ex.getTargetException());
+			catch (InvocationTargetException ex) {
+				// User-defined @InitBinder method threw an exception...
+				ReflectionUtils.rethrowException(ex.getTargetException());
+			}
 		}
 	}