From 92f8446eea53a2c05e75e5b3921fc155fc3eb4b0 Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Tue, 10 Jan 2012 10:47:47 -0500 Subject: [PATCH] SPR-8997 Add HttpServletResponse to FlashMapManager contract. This change makes the HttpServletResponse available to the methods of FlashMapManager in addition to the HttpServletRequest. --- .../web/servlet/DispatcherServlet.java | 4 +- .../springframework/web/servlet/FlashMap.java | 2 + .../web/servlet/FlashMapManager.java | 7 ++- .../support/DefaultFlashMapManager.java | 28 +++++++--- .../support/DefaultFlashMapManagerTests.java | 54 ++++++++++--------- 5 files changed, 58 insertions(+), 37 deletions(-) diff --git a/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/DispatcherServlet.java b/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/DispatcherServlet.java index 1875eead59e..da5bb9b7121 100644 --- a/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/DispatcherServlet.java +++ b/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/DispatcherServlet.java @@ -815,7 +815,7 @@ public class DispatcherServlet extends FrameworkServlet { } } - this.flashMapManager.requestStarted(request); + this.flashMapManager.requestStarted(request, response); // Make framework objects available to handlers and view objects. request.setAttribute(WEB_APPLICATION_CONTEXT_ATTRIBUTE, getWebApplicationContext()); @@ -827,7 +827,7 @@ public class DispatcherServlet extends FrameworkServlet { doDispatch(request, response); } finally { - this.flashMapManager.requestCompleted(request); + this.flashMapManager.requestCompleted(request, response); // Restore the original attribute snapshot, in case of an include. if (attributesSnapshot != null) { diff --git a/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/FlashMap.java b/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/FlashMap.java index 0b7bc82ad03..cb6450ca89b 100644 --- a/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/FlashMap.java +++ b/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/FlashMap.java @@ -63,6 +63,8 @@ public final class FlashMap extends HashMap implements Comparabl /** * Create a new instance with an id uniquely identifying the creator of * this FlashMap. + * @param createdBy identifies the FlashMapManager instance that created + * and will manage this FlashMap instance (e.g. via a hashCode) */ public FlashMap(int createdBy) { this.createdBy = createdBy; diff --git a/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/FlashMapManager.java b/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/FlashMapManager.java index 066da4231d8..6d765ec8473 100644 --- a/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/FlashMapManager.java +++ b/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/FlashMapManager.java @@ -17,6 +17,7 @@ package org.springframework.web.servlet; import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; /** * A strategy interface for storing, retrieving, and managing {@code FlashMap} @@ -64,8 +65,9 @@ public interface FlashMapManager { *
  • Clean expired FlashMap instances. * * @param request the current request + * @param response the current response */ - void requestStarted(HttpServletRequest request); + void requestStarted(HttpServletRequest request, HttpServletResponse response); /** * Start the expiration period of the "output" FlashMap save it in the @@ -73,7 +75,8 @@ public interface FlashMapManager { *

    The "output" FlashMap should not be saved if it is empty or if it was * not created by the current FlashMapManager instance. * @param request the current request + * @param response the current response */ - void requestCompleted(HttpServletRequest request); + void requestCompleted(HttpServletRequest request, HttpServletResponse response); } diff --git a/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/support/DefaultFlashMapManager.java b/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/support/DefaultFlashMapManager.java index c10bb36f04d..6d6a978a336 100644 --- a/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/support/DefaultFlashMapManager.java +++ b/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/support/DefaultFlashMapManager.java @@ -22,6 +22,7 @@ import java.util.List; import java.util.concurrent.CopyOnWriteArrayList; import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; import javax.servlet.http.HttpSession; import org.apache.commons.logging.Log; @@ -64,7 +65,7 @@ public class DefaultFlashMapManager implements FlashMapManager { * {@inheritDoc} *

    An HTTP session is never created by this method. */ - public void requestStarted(HttpServletRequest request) { + public final void requestStarted(HttpServletRequest request, HttpServletResponse response) { if (request.getAttribute(OUTPUT_FLASH_MAP_ATTRIBUTE) != null) { return; } @@ -164,9 +165,9 @@ public class DefaultFlashMapManager implements FlashMapManager { } /** - * Iterate all flash maps and remove expired ones. + * Check and remove expired FlashMaps instances. */ - private void removeExpiredFlashMaps(HttpServletRequest request) { + protected void removeExpiredFlashMaps(HttpServletRequest request) { List allMaps = retrieveFlashMaps(request, false); if (CollectionUtils.isEmpty(allMaps)) { return; @@ -189,7 +190,7 @@ public class DefaultFlashMapManager implements FlashMapManager { * {@inheritDoc} *

    An HTTP session is never created if the "output" FlashMap is empty. */ - public void requestCompleted(HttpServletRequest request) { + public void requestCompleted(HttpServletRequest request, HttpServletResponse response) { FlashMap flashMap = (FlashMap) request.getAttribute(OUTPUT_FLASH_MAP_ATTRIBUTE); if (flashMap == null) { throw new IllegalStateException("requestCompleted called but \"output\" FlashMap was never created"); @@ -198,24 +199,35 @@ public class DefaultFlashMapManager implements FlashMapManager { if (logger.isDebugEnabled()) { logger.debug("Saving FlashMap=" + flashMap); } - onSaveFlashMap(flashMap, request); - retrieveFlashMaps(request, true).add(flashMap); + onSaveFlashMap(flashMap, request, response); + saveFlashMap(flashMap, request, response); } } /** - * Update a FlashMap before it is stored in the HTTP Session. + * Update a FlashMap before it is stored in the underlying storage. *

    The default implementation starts the expiration period and ensures the * target request path is decoded and normalized if it is relative. * @param flashMap the flash map to be saved * @param request the current request + * @param response the current response */ - protected void onSaveFlashMap(FlashMap flashMap, HttpServletRequest request) { + protected void onSaveFlashMap(FlashMap flashMap, HttpServletRequest request, HttpServletResponse response) { String targetPath = flashMap.getTargetRequestPath(); flashMap.setTargetRequestPath(decodeAndNormalizePath(targetPath, request)); flashMap.startExpirationPeriod(this.flashTimeout); } + /** + * Save the FlashMap in the underlying storage. + * @param flashMap the FlashMap to save + * @param request the current request + * @param response the current response + */ + protected void saveFlashMap(FlashMap flashMap, HttpServletRequest request, HttpServletResponse response) { + retrieveFlashMaps(request, true).add(flashMap); + } + private String decodeAndNormalizePath(String path, HttpServletRequest request) { if (path != null) { path = this.urlPathHelper.decodeRequestString(request, path); diff --git a/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/support/DefaultFlashMapManagerTests.java b/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/support/DefaultFlashMapManagerTests.java index 61aaa808e6c..eb7414633de 100644 --- a/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/support/DefaultFlashMapManagerTests.java +++ b/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/support/DefaultFlashMapManagerTests.java @@ -32,6 +32,7 @@ import java.util.concurrent.CopyOnWriteArrayList; import org.junit.Before; import org.junit.Test; import org.springframework.mock.web.MockHttpServletRequest; +import org.springframework.mock.web.MockHttpServletResponse; import org.springframework.web.servlet.FlashMap; import org.springframework.web.util.WebUtils; @@ -46,15 +47,18 @@ public class DefaultFlashMapManagerTests { private MockHttpServletRequest request; + private MockHttpServletResponse response; + @Before public void setup() { this.flashMapManager = new DefaultFlashMapManager(); this.request = new MockHttpServletRequest(); + this.response = new MockHttpServletResponse(); } @Test public void requestStarted() { - this.flashMapManager.requestStarted(this.request); + this.flashMapManager.requestStarted(this.request, this.response); FlashMap flashMap = RequestContextUtils.getOutputFlashMap(request); assertNotNull("Current FlashMap not found", flashMap); @@ -64,7 +68,7 @@ public class DefaultFlashMapManagerTests { public void requestStartedAlready() { FlashMap flashMap = new FlashMap(); this.request.setAttribute(OUTPUT_FLASH_MAP_ATTRIBUTE, flashMap); - this.flashMapManager.requestStarted(this.request); + this.flashMapManager.requestStarted(this.request, this.response); assertSame(flashMap, RequestContextUtils.getOutputFlashMap(request)); } @@ -79,7 +83,7 @@ public class DefaultFlashMapManagerTests { allMaps.add(flashMap); this.request.setRequestURI("/path"); - this.flashMapManager.requestStarted(this.request); + this.flashMapManager.requestStarted(this.request, this.response); assertEquals(flashMap, RequestContextUtils.getInputFlashMap(this.request)); assertEquals("Input FlashMap should have been removed", 0, getFlashMaps().size()); @@ -98,7 +102,7 @@ public class DefaultFlashMapManagerTests { this.request.setAttribute(WebUtils.FORWARD_REQUEST_URI_ATTRIBUTE, "/accounts"); this.request.setRequestURI("/mvc/accounts"); - this.flashMapManager.requestStarted(this.request); + this.flashMapManager.requestStarted(this.request, this.response); assertEquals(flashMap, RequestContextUtils.getInputFlashMap(this.request)); assertEquals("Input FlashMap should have been removed", 0, getFlashMaps().size()); @@ -114,7 +118,7 @@ public class DefaultFlashMapManagerTests { allMaps.add(flashMap); this.request.setRequestURI("/path/"); - this.flashMapManager.requestStarted(this.request); + this.flashMapManager.requestStarted(this.request, this.response); assertEquals(flashMap, RequestContextUtils.getInputFlashMap(this.request)); assertEquals("Input FlashMap should have been removed", 0, getFlashMaps().size()); @@ -130,21 +134,21 @@ public class DefaultFlashMapManagerTests { allMaps.add(flashMap); this.request.setParameter("number", (String) null); - this.flashMapManager.requestStarted(this.request); + this.flashMapManager.requestStarted(this.request, this.response); assertNull(RequestContextUtils.getInputFlashMap(this.request)); assertEquals("FlashMap should not have been removed", 1, getFlashMaps().size()); clearFlashMapRequestAttributes(); this.request.setParameter("number", "two"); - this.flashMapManager.requestStarted(this.request); + this.flashMapManager.requestStarted(this.request, this.response); assertNull(RequestContextUtils.getInputFlashMap(this.request)); assertEquals("FlashMap should not have been removed", 1, getFlashMaps().size()); clearFlashMapRequestAttributes(); this.request.setParameter("number", "one"); - this.flashMapManager.requestStarted(this.request); + this.flashMapManager.requestStarted(this.request, this.response); assertEquals(flashMap, RequestContextUtils.getInputFlashMap(this.request)); assertEquals("Input FlashMap should have been removed", 0, getFlashMaps().size()); @@ -163,14 +167,14 @@ public class DefaultFlashMapManagerTests { allMaps.add(flashMap); this.request.setParameter("id", "1"); - this.flashMapManager.requestStarted(this.request); + this.flashMapManager.requestStarted(this.request, this.response); assertNull(RequestContextUtils.getInputFlashMap(this.request)); assertEquals("FlashMap should not have been removed", 1, getFlashMaps().size()); clearFlashMapRequestAttributes(); this.request.addParameter("id", "2"); - this.flashMapManager.requestStarted(this.request); + this.flashMapManager.requestStarted(this.request, this.response); assertEquals(flashMap, RequestContextUtils.getInputFlashMap(this.request)); assertEquals("Input FlashMap should have been removed", 0, getFlashMaps().size()); @@ -196,7 +200,7 @@ public class DefaultFlashMapManagerTests { Collections.shuffle(allMaps); this.request.setRequestURI("/one/two"); - this.flashMapManager.requestStarted(this.request); + this.flashMapManager.requestStarted(this.request, this.response); assertEquals(flashMapTwo, request.getAttribute(INPUT_FLASH_MAP_ATTRIBUTE)); } @@ -210,15 +214,15 @@ public class DefaultFlashMapManagerTests { flashMap.startExpirationPeriod(0); } Thread.sleep(100); - this.flashMapManager.requestStarted(this.request); + this.flashMapManager.requestStarted(this.request, this.response); assertEquals(0, allMaps.size()); } @Test public void saveFlashMapWithoutAttributes() throws InterruptedException { - this.flashMapManager.requestStarted(this.request); - this.flashMapManager.requestCompleted(this.request); + this.flashMapManager.requestStarted(this.request, this.response); + this.flashMapManager.requestCompleted(this.request, this.response); assertNull(getFlashMaps()); } @@ -227,19 +231,19 @@ public class DefaultFlashMapManagerTests { public void saveFlashMapNotCreatedByThisManager() throws InterruptedException { FlashMap flashMap = new FlashMap(); this.request.setAttribute(OUTPUT_FLASH_MAP_ATTRIBUTE, flashMap); - this.flashMapManager.requestCompleted(this.request); + this.flashMapManager.requestCompleted(this.request, this.response); assertNull(getFlashMaps()); } @Test public void saveFlashMapWithAttributes() throws InterruptedException { - this.flashMapManager.requestStarted(this.request); + this.flashMapManager.requestStarted(this.request, this.response); FlashMap flashMap = RequestContextUtils.getOutputFlashMap(this.request); flashMap.put("name", "value"); this.flashMapManager.setFlashMapTimeout(0); - this.flashMapManager.requestCompleted(this.request); + this.flashMapManager.requestCompleted(this.request, this.response); Thread.sleep(100); @@ -252,49 +256,49 @@ public class DefaultFlashMapManagerTests { @Test public void decodeTargetPath() throws InterruptedException { - this.flashMapManager.requestStarted(this.request); + this.flashMapManager.requestStarted(this.request, this.response); FlashMap flashMap = RequestContextUtils.getOutputFlashMap(this.request); flashMap.put("key", "value"); flashMap.setTargetRequestPath("/once%20upon%20a%20time"); - this.flashMapManager.requestCompleted(this.request); + this.flashMapManager.requestCompleted(this.request, this.response); assertEquals("/once upon a time", flashMap.getTargetRequestPath()); } @Test public void normalizeTargetPath() throws InterruptedException { - this.flashMapManager.requestStarted(this.request); + this.flashMapManager.requestStarted(this.request, this.response); FlashMap flashMap = RequestContextUtils.getOutputFlashMap(this.request); flashMap.put("key", "value"); flashMap.setTargetRequestPath("."); this.request.setRequestURI("/once/upon/a/time"); - this.flashMapManager.requestCompleted(this.request); + this.flashMapManager.requestCompleted(this.request, this.response); assertEquals("/once/upon/a", flashMap.getTargetRequestPath()); flashMap.setTargetRequestPath("./"); this.request.setRequestURI("/once/upon/a/time"); - this.flashMapManager.requestCompleted(this.request); + this.flashMapManager.requestCompleted(this.request, this.response); assertEquals("/once/upon/a/", flashMap.getTargetRequestPath()); flashMap.setTargetRequestPath(".."); this.request.setRequestURI("/once/upon/a/time"); - this.flashMapManager.requestCompleted(this.request); + this.flashMapManager.requestCompleted(this.request, this.response); assertEquals("/once/upon", flashMap.getTargetRequestPath()); flashMap.setTargetRequestPath("../"); this.request.setRequestURI("/once/upon/a/time"); - this.flashMapManager.requestCompleted(this.request); + this.flashMapManager.requestCompleted(this.request, this.response); assertEquals("/once/upon/", flashMap.getTargetRequestPath()); flashMap.setTargetRequestPath("../../only"); this.request.setRequestURI("/once/upon/a/time"); - this.flashMapManager.requestCompleted(this.request); + this.flashMapManager.requestCompleted(this.request, this.response); assertEquals("/once/only", flashMap.getTargetRequestPath()); }