From 2f4453a99c904f820c72478b902a62d3a670bbee Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Wed, 9 Jun 2010 20:09:53 +0000 Subject: [PATCH] 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) --- .../AnnotationMethodHandlerAdapter.java | 18 +++-- .../Portlet20AnnotationControllerTests.java | 69 ++++++++++++++++--- .../support/HandlerMethodInvoker.java | 49 +++++++------ 3 files changed, 102 insertions(+), 34 deletions(-) diff --git a/org.springframework.web.portlet/src/main/java/org/springframework/web/portlet/mvc/annotation/AnnotationMethodHandlerAdapter.java b/org.springframework.web.portlet/src/main/java/org/springframework/web/portlet/mvc/annotation/AnnotationMethodHandlerAdapter.java index 249bc8538a..6fbe0333fe 100644 --- a/org.springframework.web.portlet/src/main/java/org/springframework/web/portlet/mvc/annotation/AnnotationMethodHandlerAdapter.java +++ b/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. diff --git a/org.springframework.web.portlet/src/test/java/org/springframework/web/portlet/mvc/annotation/Portlet20AnnotationControllerTests.java b/org.springframework.web.portlet/src/test/java/org/springframework/web/portlet/mvc/annotation/Portlet20AnnotationControllerTests.java index cc50f5d9ce..2ecd287cd6 100644 --- a/org.springframework.web.portlet/src/test/java/org/springframework/web/portlet/mvc/annotation/Portlet20AnnotationControllerTests.java +++ b/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 { diff --git a/org.springframework.web/src/main/java/org/springframework/web/bind/annotation/support/HandlerMethodInvoker.java b/org.springframework.web/src/main/java/org/springframework/web/bind/annotation/support/HandlerMethodInvoker.java index dcb3a0df96..baeb4606f2 100644 --- a/org.springframework.web/src/main/java/org/springframework/web/bind/annotation/support/HandlerMethodInvoker.java +++ b/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 model = (mavModel != null ? mavModel : implicitModel); - try { - for (String attrName : new HashSet(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()); + } } }