SPR-8997 Add HttpServletResponse to FlashMapManager contract.

This change makes the HttpServletResponse available to the methods
of FlashMapManager in addition to the HttpServletRequest.

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) {

org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/FlashMap.java

@@ -63,6 +63,8 @@ public final class FlashMap extends HashMap<String, Object> 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;

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 {
 	 * 	<li>Clean expired FlashMap instances.
 	 * </ol>
 	 * @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 {
 	 * <p>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);
 
 }

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}
 	 * <p>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<FlashMap> allMaps = retrieveFlashMaps(request, false);
 		if (CollectionUtils.isEmpty(allMaps)) {
 			return;
@@ -189,7 +190,7 @@ public class DefaultFlashMapManager implements FlashMapManager {
 	 * {@inheritDoc}
 	 * <p>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);
+			onSaveFlashMap(flashMap, request, response);
-			retrieveFlashMaps(request, true).add(flashMap);
+			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.
 	 * <p>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);

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.requestStarted(this.request, this.response);
-		this.flashMapManager.requestCompleted(this.request);
+		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());
 	}