Skip to content

Commit 2f4453a

Browse files
jhoellerjhoeller
committed
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)
1 parent 65885d1 commit 2f4453a

File tree

3 files changed

+102
-34
lines changed

3 files changed

+102
-34
lines changed

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

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,10 @@
123123
public class AnnotationMethodHandlerAdapter extends PortletContentGenerator
124124
implements HandlerAdapter, Ordered, BeanFactoryAware {
125125

126-
private static final String IMPLICIT_MODEL_ATTRIBUTE = "org.springframework.web.portlet.mvc.ImplicitModel";
126+
public static final String IMPLICIT_MODEL_SESSION_ATTRIBUTE =
127+
AnnotationMethodHandlerAdapter.class.getName() + ".IMPLICIT_MODEL";
128+
129+
public static final String IMPLICIT_MODEL_RENDER_PARAMETER = "implicitModel";
127130

128131

129132
private WebBindingInitializer webBindingInitializer;
@@ -303,10 +306,15 @@ protected ModelAndView doHandle(PortletRequest request, PortletResponse response
303306
if (response instanceof MimeResponse) {
304307
MimeResponse mimeResponse = (MimeResponse) response;
305308
// Detect implicit model from associated action phase.
306-
if (request.getParameter(IMPLICIT_MODEL_ATTRIBUTE) != null) {
309+
if (response instanceof RenderResponse) {
307310
PortletSession session = request.getPortletSession(false);
308311
if (session != null) {
309-
implicitModel = (ExtendedModelMap) session.getAttribute(IMPLICIT_MODEL_ATTRIBUTE);
312+
if (request.getParameter(IMPLICIT_MODEL_RENDER_PARAMETER) != null) {
313+
implicitModel = (ExtendedModelMap) session.getAttribute(IMPLICIT_MODEL_SESSION_ATTRIBUTE);
314+
}
315+
else {
316+
session.removeAttribute(IMPLICIT_MODEL_SESSION_ATTRIBUTE);
317+
}
310318
}
311319
}
312320
if (handler.getClass().getAnnotation(SessionAttributes.class) != null) {
@@ -356,8 +364,8 @@ private ModelAndView invokeHandlerMethod(
356364
if (response instanceof ActionResponse && !implicitModel.isEmpty()) {
357365
ActionResponse actionResponse = (ActionResponse) response;
358366
try {
359-
actionResponse.setRenderParameter(IMPLICIT_MODEL_ATTRIBUTE, Boolean.TRUE.toString());
360-
request.getPortletSession().setAttribute(IMPLICIT_MODEL_ATTRIBUTE, implicitModel);
367+
actionResponse.setRenderParameter(IMPLICIT_MODEL_RENDER_PARAMETER, Boolean.TRUE.toString());
368+
request.getPortletSession().setAttribute(IMPLICIT_MODEL_SESSION_ATTRIBUTE, implicitModel);
361369
}
362370
catch (IllegalStateException ex) {
363371
// 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

Lines changed: 60 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -70,11 +70,14 @@
7070
import org.springframework.validation.Errors;
7171
import org.springframework.web.bind.WebDataBinder;
7272
import org.springframework.web.bind.annotation.CookieValue;
73+
import org.springframework.web.bind.annotation.ExceptionHandler;
7374
import org.springframework.web.bind.annotation.InitBinder;
7475
import org.springframework.web.bind.annotation.ModelAttribute;
7576
import org.springframework.web.bind.annotation.RequestHeader;
7677
import org.springframework.web.bind.annotation.RequestMapping;
7778
import org.springframework.web.bind.annotation.RequestParam;
79+
import org.springframework.web.bind.annotation.SessionAttributes;
80+
import org.springframework.web.bind.support.SessionStatus;
7881
import org.springframework.web.bind.support.WebArgumentResolver;
7982
import org.springframework.web.bind.support.WebBindingInitializer;
8083
import org.springframework.web.context.WebApplicationContext;
@@ -83,6 +86,7 @@
8386
import org.springframework.web.context.support.GenericWebApplicationContext;
8487
import org.springframework.web.portlet.DispatcherPortlet;
8588
import org.springframework.web.portlet.ModelAndView;
89+
import org.springframework.web.portlet.bind.MissingPortletRequestParameterException;
8690
import org.springframework.web.portlet.bind.annotation.ActionMapping;
8791
import org.springframework.web.portlet.bind.annotation.EventMapping;
8892
import org.springframework.web.portlet.bind.annotation.RenderMapping;
@@ -129,6 +133,11 @@ public void adaptedHandleMethods3() throws Exception {
129133
doTestAdaptedHandleMethods(MyAdaptedController3.class);
130134
}
131135

136+
@Test
137+
public void adaptedHandleMethods4() throws Exception {
138+
doTestAdaptedHandleMethods(MyAdaptedController4.class);
139+
}
140+
132141
private void doTestAdaptedHandleMethods(final Class controllerClass) throws Exception {
133142
DispatcherPortlet portlet = new DispatcherPortlet() {
134143
protected ApplicationContext createPortletApplicationContext(ApplicationContext parent) throws BeansException {
@@ -145,12 +154,22 @@ protected ApplicationContext createPortletApplicationContext(ApplicationContext
145154
portlet.processAction(actionRequest, actionResponse);
146155
assertEquals("value", actionResponse.getRenderParameter("test"));
147156

148-
MockRenderRequest request = new MockRenderRequest(PortletMode.EDIT);
157+
MockRenderRequest request = new MockRenderRequest(PortletMode.VIEW);
158+
request.setSession(actionRequest.getPortletSession());
159+
request.setParameters(actionResponse.getRenderParameterMap());
160+
request.addParameter("name", "name1");
161+
request.addParameter("age", "value2");
162+
MockRenderResponse response = new MockRenderResponse();
163+
portlet.render(request, response);
164+
assertEquals("test-name1-typeMismatch", response.getContentAsString());
165+
assertNull(request.getPortletSession().getAttribute("testBean"));
166+
167+
request = new MockRenderRequest(PortletMode.EDIT);
149168
request.addParameter("param1", "value1");
150169
request.addParameter("param2", "2");
151170
request.addProperty("header1", "10");
152171
request.setCookies(new Cookie("cookie1", "3"));
153-
MockRenderResponse response = new MockRenderResponse();
172+
response = new MockRenderResponse();
154173
portlet.render(request, response);
155174
assertEquals("test-value1-2-10-3", response.getContentAsString());
156175

@@ -160,13 +179,6 @@ protected ApplicationContext createPortletApplicationContext(ApplicationContext
160179
response = new MockRenderResponse();
161180
portlet.render(request, response);
162181
assertEquals("test-name1-2", response.getContentAsString());
163-
164-
request = new MockRenderRequest(PortletMode.VIEW);
165-
request.addParameter("name", "name1");
166-
request.addParameter("age", "value2");
167-
response = new MockRenderResponse();
168-
portlet.render(request, response);
169-
assertEquals("test-name1-typeMismatch", response.getContentAsString());
170182
}
171183

172184
@Test
@@ -572,6 +584,7 @@ protected ApplicationContext createPortletApplicationContext(ApplicationContext
572584

573585
request = new MockRenderRequest(PortletMode.VIEW, WindowState.MAXIMIZED);
574586
request.setParameters(actionResponse.getRenderParameterMap());
587+
request.setSession(actionRequest.getPortletSession());
575588
response = new MockRenderResponse();
576589
portlet.render(request, response);
577590
assertEquals("myLargeView-value", response.getContentAsString());
@@ -583,6 +596,7 @@ protected ApplicationContext createPortletApplicationContext(ApplicationContext
583596

584597
request = new MockRenderRequest(PortletMode.VIEW, WindowState.MAXIMIZED);
585598
request.setParameters(actionResponse.getRenderParameterMap());
599+
request.setSession(actionRequest.getPortletSession());
586600
response = new MockRenderResponse();
587601
portlet.render(request, response);
588602
assertEquals("myLargeView-details", response.getContentAsString());
@@ -731,6 +745,43 @@ public void myHandle(TestBean tb, Errors errors, RenderResponse response) throws
731745
}
732746

733747

748+
@Controller
749+
@SessionAttributes("testBean")
750+
private static class MyAdaptedController4 {
751+
752+
@RequestMapping("VIEW")
753+
@ActionMapping
754+
public void myHandle(Model model, ActionResponse response, SessionStatus status) {
755+
TestBean tb = new TestBean();
756+
tb.setJedi(true);
757+
model.addAttribute("testBean", tb);
758+
status.setComplete();
759+
response.setRenderParameter("test", "value");
760+
}
761+
762+
@RequestMapping("EDIT")
763+
@RenderMapping
764+
public void myHandle(@RequestParam("param1") String p1, int param2, RenderResponse response,
765+
@RequestHeader("header1") String h1, @CookieValue("cookie1") String c1) throws IOException {
766+
response.getWriter().write("test-" + p1 + "-" + param2 + "-" + h1 + "-" + c1);
767+
}
768+
769+
@RequestMapping("HELP")
770+
@RenderMapping
771+
public void myHandle(@ModelAttribute("tb") TestBean tb, RenderResponse response) throws IOException {
772+
response.getWriter().write("test-" + tb.getName() + "-" + tb.getAge());
773+
}
774+
775+
@RequestMapping("VIEW")
776+
@RenderMapping
777+
public void myHandle(@ModelAttribute("testBean") TestBean tb, Errors errors, RenderResponse response, PortletSession session) throws IOException {
778+
assertTrue(tb.isJedi());
779+
assertNull(session.getAttribute("testBean"));
780+
response.getWriter().write("test-" + tb.getName() + "-" + errors.getFieldError("age").getCode());
781+
}
782+
}
783+
784+
734785
@Controller
735786
private static class MyFormController {
736787

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

Lines changed: 29 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@
2626
import java.util.ArrayList;
2727
import java.util.Arrays;
2828
import java.util.Collection;
29-
import java.util.HashSet;
3029
import java.util.Iterator;
3130
import java.util.LinkedHashMap;
3231
import java.util.List;
@@ -95,6 +94,8 @@
9594
*/
9695
public class HandlerMethodInvoker {
9796

97+
private static final String MODEL_KEY_PREFIX_STALE = SessionAttributeStore.class.getName() + ".STALE.";
98+
9899
/** We'll create a lot of these objects, so we don't want a new logger every time. */
99100
private static final Log logger = LogFactory.getLog(HandlerMethodInvoker.class);
100101

@@ -197,28 +198,36 @@ public final void updateModelAttributes(Object handler, Map<String, Object> mavM
197198
// Expose model attributes as session attributes, if required.
198199
// Expose BindingResults for all attributes, making custom editors available.
199200
Map<String, Object> model = (mavModel != null ? mavModel : implicitModel);
200-
try {
201-
for (String attrName : new HashSet<String>(model.keySet())) {
202-
Object attrValue = model.get(attrName);
203-
boolean isSessionAttr =
204-
this.methodResolver.isSessionAttribute(attrName, (attrValue != null ? attrValue.getClass() : null));
205-
if (isSessionAttr && !this.sessionStatus.isComplete()) {
206-
this.sessionAttributeStore.storeAttribute(webRequest, attrName, attrValue);
207-
}
208-
if (!attrName.startsWith(BindingResult.MODEL_KEY_PREFIX) &&
209-
(isSessionAttr || isBindingCandidate(attrValue))) {
210-
String bindingResultKey = BindingResult.MODEL_KEY_PREFIX + attrName;
211-
if (mavModel != null && !model.containsKey(bindingResultKey)) {
212-
WebDataBinder binder = createBinder(webRequest, attrValue, attrName);
213-
initBinder(handler, attrName, binder, webRequest);
214-
mavModel.put(bindingResultKey, binder.getBindingResult());
201+
if (model != null) {
202+
try {
203+
String[] originalAttrNames = model.keySet().toArray(new String[model.size()]);
204+
for (String attrName : originalAttrNames) {
205+
Object attrValue = model.get(attrName);
206+
boolean isSessionAttr = this.methodResolver.isSessionAttribute(
207+
attrName, (attrValue != null ? attrValue.getClass() : null));
208+
if (isSessionAttr) {
209+
if (this.sessionStatus.isComplete()) {
210+
implicitModel.put(MODEL_KEY_PREFIX_STALE + attrName, Boolean.TRUE);
211+
}
212+
else if (!implicitModel.containsKey(MODEL_KEY_PREFIX_STALE + attrName)) {
213+
this.sessionAttributeStore.storeAttribute(webRequest, attrName, attrValue);
214+
}
215+
}
216+
if (!attrName.startsWith(BindingResult.MODEL_KEY_PREFIX) &&
217+
(isSessionAttr || isBindingCandidate(attrValue))) {
218+
String bindingResultKey = BindingResult.MODEL_KEY_PREFIX + attrName;
219+
if (mavModel != null && !model.containsKey(bindingResultKey)) {
220+
WebDataBinder binder = createBinder(webRequest, attrValue, attrName);
221+
initBinder(handler, attrName, binder, webRequest);
222+
mavModel.put(bindingResultKey, binder.getBindingResult());
223+
}
215224
}
216225
}
217226
}
218-
}
219-
catch (InvocationTargetException ex) {
220-
// User-defined @InitBinder method threw an exception...
221-
ReflectionUtils.rethrowException(ex.getTargetException());
227+
catch (InvocationTargetException ex) {
228+
// User-defined @InitBinder method threw an exception...
229+
ReflectionUtils.rethrowException(ex.getTargetException());
230+
}
222231
}
223232
}
224233

0 commit comments

Comments
 (0)