SPR-6464 Add getInputFlashMap and getOutputFlashMap methods to RequestContextUtils

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

@@ -41,11 +41,11 @@ public class FlashMap extends HashMap<String, Object> implements Comparable<Flas
 	
 	private final Map<String, String> expectedRequestParameters = new LinkedHashMap<String, String>();
 	
-	private UrlPathHelper urlPathHelper = new UrlPathHelper();
-	
 	private long expirationStartTime;
 	
 	private int timeToLive;
+
+	private final UrlPathHelper urlPathHelper = new UrlPathHelper();
 	
 	/**
 	 * Provide a URL to identify the target request for this FlashMap.

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

@@ -22,10 +22,12 @@ import org.springframework.web.servlet.support.RequestContextUtils;
 
 /**
  * A strategy interface for maintaining {@link FlashMap} instances in some 
- * underlying storage until the next request. The most common use case is
+ * underlying storage until the next request. 
- * a redirect. For example redirecting from a POST that creates a resource 
+ * 
- * to the page that shows the created resource and passing along a 
+ * <p>The most common use case for using flash storage is a redirect. 
- * success message that needs to be shown once only.
+ * For example creating a resource in a POST request and then redirecting
+ * to the page that shows the resource. Flash storage may be used to 
+ * pass along a success message.
  * 
  * @author Rossen Stoyanchev
  * @since 3.1
@@ -33,48 +35,41 @@ import org.springframework.web.servlet.support.RequestContextUtils;
  * @see FlashMap
  */
 public interface FlashMapManager {
-	
+
 	/**
-	 * Request attribute to hold the current request FlashMap. 
+	 * Request attribute holding the read-only Map with flash attributes saved 
-	 * @see RequestContextUtils#getFlashMap
+	 * during the previous request.
+	 * @see RequestContextUtils#getInputFlashMap(HttpServletRequest)
 	 */
-	public static final String CURRENT_FLASH_MAP_ATTRIBUTE = DispatcherServlet.class.getName() + ".CURRENT_FLASH_MAP";
+	public static final String INPUT_FLASH_MAP_ATTRIBUTE = FlashMapManager.class.getName() + ".INPUT_FLASH_MAP";
-	
+
 	/**
-	 * Request attribute to hold the FlashMap from the previous request. 
+	 * Request attribute holding the {@link FlashMap} to add attributes to during 
-	 * Access to the previous FlashMap should generally not be needed 
+	 * the current request.
-	 * since its content is exposed as attributes of the current 
+	 * @see RequestContextUtils#getOutputFlashMap(HttpServletRequest)
-	 * request. However, it may be useful to expose previous request
-	 * flash attributes in other ways such as in the model of annotated 
-	 * controllers.
 	 */
-	public static final String PREVIOUS_FLASH_MAP_ATTRIBUTE = DispatcherServlet.class.getName() + ".PREVIOUS_FLASH_MAP";
+	public static final String OUTPUT_FLASH_MAP_ATTRIBUTE = FlashMapManager.class.getName() + ".OUTPUT_FLASH_MAP";
 
 	/**
 	 * Perform flash storage tasks at the start of a new request:
 	 * <ul>
-	 * 	<li>Create a new FlashMap and make it available to the current request 
+	 * 	<li>Create a FlashMap and make it available under the request attribute 
-	 * 	under the request attribute {@link #CURRENT_FLASH_MAP_ATTRIBUTE}.
+	 * 	{@link #OUTPUT_FLASH_MAP_ATTRIBUTE}.
-	 * 	<li>Locate the FlashMap saved on the previous request and expose its 
+	 * 	<li>Locate the FlashMap saved during the previous request and make it
-	 * 	contents as attributes in the current request, also exposing the 
+	 * 	available under the request attribute {@link #INPUT_FLASH_MAP_ATTRIBUTE}.
-	 *  previous FlashMap under {@link #PREVIOUS_FLASH_MAP_ATTRIBUTE}.
+	 * 	<li>Remove expired FlashMap instances.
-	 * 	<li>Check for and remove expired FlashMap instances.
 	 * </ul>
-	 * 
+	 * <p>If the {@link #OUTPUT_FLASH_MAP_ATTRIBUTE} request attribute exists
-	 * <p>If the {@link #CURRENT_FLASH_MAP_ATTRIBUTE} request attribute exists
+	 * return "false" immediately.
-	 * in the current request, this method should return "false" immediately.
 	 * 
 	 * @param request the current request
-	 * 
 	 * @return "true" if flash storage tasks were performed; "false" otherwise.
 	 */
 	boolean requestStarted(HttpServletRequest request);
 
 	/**
-	 * Access the current FlashMap through the request attribute
+	 * Access the FlashMap with attributes added during the current request and
-	 * {@link #CURRENT_FLASH_MAP_ATTRIBUTE} and if it is not empty, save it 
+	 * if it is not empty, save it in the underlying storage. 
-	 * in the underlying storage. 
-	 * 
 	 * <p>If the call to {@link #requestStarted} returned "false", this 
 	 * method is not invoked.
 	 */

org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerAdapter.java

@@ -516,10 +516,8 @@ public class RequestMappingHandlerAdapter extends AbstractHandlerMethodAdapter i
 		ServletInvocableHandlerMethod requestMappingMethod = createRequestMappingMethod(handlerMethod);
 		ModelFactory modelFactory = getModelFactory(handlerMethod);
 
-		FlashMap previousFlashMap = (FlashMap) request.getAttribute(FlashMapManager.PREVIOUS_FLASH_MAP_ATTRIBUTE);
-		
 		ModelAndViewContainer mavContainer = new ModelAndViewContainer();
-		mavContainer.addAllAttributes(previousFlashMap);
+		mavContainer.addAllAttributes(RequestContextUtils.getInputFlashMap(request));
 		modelFactory.initModel(webRequest, mavContainer, requestMappingMethod);
 		
 		SessionStatus sessionStatus = new SimpleSessionStatus();
@@ -539,8 +537,8 @@ public class RequestMappingHandlerAdapter extends AbstractHandlerMethodAdapter i
 			}
 			if (model instanceof RedirectModel) {
 				RedirectModel redirectModel = (RedirectModel) model;
-				FlashMap currentFlashMap = RequestContextUtils.getFlashMap(request);
+				FlashMap flashMap = RequestContextUtils.getOutputFlashMap(request);
-				currentFlashMap.putAll(redirectModel.getFlashAttributes());
+				flashMap.putAll(redirectModel.getFlashAttributes());
 			}
 			return mav;				
 		}

org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/support/DefaultFlashMapManager.java

@@ -19,6 +19,7 @@ package org.springframework.web.servlet.support;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
+import java.util.Map;
 import java.util.concurrent.CopyOnWriteArrayList;
 
 import javax.servlet.http.HttpServletRequest;
@@ -29,7 +30,6 @@ import org.apache.commons.logging.LogFactory;
 import org.springframework.util.CollectionUtils;
 import org.springframework.web.servlet.FlashMap;
 import org.springframework.web.servlet.FlashMapManager;
-import org.springframework.web.util.WebUtils;
 
 /**
  * A {@link FlashMapManager} that saves and retrieves FlashMap instances in the 
@@ -58,48 +58,34 @@ public class DefaultFlashMapManager implements FlashMapManager {
 	/**
 	 * {@inheritDoc}
 	 * 
-	 * <p>This method never creates an HTTP session. The current FlashMap is 
+	 * <p>This method never creates an HTTP session. The new FlashMap created 
-	 * exposed as a request attribute only and is not saved in the session 
+	 * for the current request is exposed as a request attribute only and is 
-	 * until {@link #requestCompleted}.
+	 * not saved in the session until {@link #requestCompleted} is called.
 	 */
 	public boolean requestStarted(HttpServletRequest request) {
-		if (request.getAttribute(CURRENT_FLASH_MAP_ATTRIBUTE) != null) {
+		if (request.getAttribute(OUTPUT_FLASH_MAP_ATTRIBUTE) != null) {
 			return false;
 		}
 
-		FlashMap currentFlashMap = new FlashMap();
+		FlashMap outputFlashMap = new FlashMap();
-		request.setAttribute(CURRENT_FLASH_MAP_ATTRIBUTE, currentFlashMap);
+		request.setAttribute(OUTPUT_FLASH_MAP_ATTRIBUTE, outputFlashMap);
 		
-		FlashMap previousFlashMap = lookupPreviousFlashMap(request);
+		Map<String, ?> inputFlashMap = getFlashMap(request);
-		if (previousFlashMap != null) {
+		if (inputFlashMap != null) {
-			WebUtils.exposeRequestAttributes(request, previousFlashMap);
+			request.setAttribute(INPUT_FLASH_MAP_ATTRIBUTE, inputFlashMap);
-			request.setAttribute(PREVIOUS_FLASH_MAP_ATTRIBUTE, previousFlashMap);
 		}
 		
-		// Remove expired flash maps
+		removeExpiredFlashMaps(request);
-		List<FlashMap> allMaps = retrieveFlashMaps(request, false);
-		if (allMaps != null && !allMaps.isEmpty()) {
-			List<FlashMap> expiredMaps = new ArrayList<FlashMap>();
-			for (FlashMap flashMap : allMaps) {
-				if (flashMap.isExpired()) {
-					if (logger.isDebugEnabled()) {
-						logger.debug("Removing expired FlashMap: " + flashMap);
-					}
-					expiredMaps.add(flashMap);
-				}
-			}
-			allMaps.removeAll(expiredMaps);
-		}
 		
 		return true;
 	}
 
 	/**
-	 * Return the FlashMap from the previous request.
+	 * Return the flash attributes saved during the previous request if any.
 	 * 
-	 * @return the FlashMap from the previous request; or {@code null} if none.
+	 * @return a read-only Map; or {@code null} if not found.
 	 */
-	private FlashMap lookupPreviousFlashMap(HttpServletRequest request) {
+	private Map<String, ?> getFlashMap(HttpServletRequest request) {
 		List<FlashMap> allMaps = retrieveFlashMaps(request, false);
 		if (CollectionUtils.isEmpty(allMaps)) {
 			return null;
@@ -123,7 +109,7 @@ public class DefaultFlashMapManager implements FlashMapManager {
 			Collections.sort(matches);
 			FlashMap match = matches.remove(0);
 			allMaps.remove(match);
-			return match;
+			return Collections.unmodifiableMap(match);
 		}
 		
 		return null;
@@ -156,16 +142,32 @@ public class DefaultFlashMapManager implements FlashMapManager {
 		return allMaps;
 	}
 
+	private void removeExpiredFlashMaps(HttpServletRequest request) {
+		List<FlashMap> allMaps = retrieveFlashMaps(request, false);
+		if (allMaps != null && !allMaps.isEmpty()) {
+			List<FlashMap> expiredMaps = new ArrayList<FlashMap>();
+			for (FlashMap flashMap : allMaps) {
+				if (flashMap.isExpired()) {
+					if (logger.isDebugEnabled()) {
+						logger.debug("Removing expired FlashMap: " + flashMap);
+					}
+					expiredMaps.add(flashMap);
+				}
+			}
+			allMaps.removeAll(expiredMaps);
+		}
+	}
+	
 	/**
 	 * {@inheritDoc}
 	 * 
 	 * <p>The HTTP session is not created if the current FlashMap instance is empty.
 	 */
 	public void requestCompleted(HttpServletRequest request) {
-		FlashMap flashMap = (FlashMap) request.getAttribute(CURRENT_FLASH_MAP_ATTRIBUTE);
+		FlashMap flashMap = (FlashMap) request.getAttribute(OUTPUT_FLASH_MAP_ATTRIBUTE);
 		if (flashMap == null) {
 			throw new IllegalStateException(
-					"Did not find a FlashMap exposed as the request attribute " + CURRENT_FLASH_MAP_ATTRIBUTE);
+					"Did not find a FlashMap exposed as the request attribute " + OUTPUT_FLASH_MAP_ATTRIBUTE);
 		}
 		
 		if (!flashMap.isEmpty()) {

org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/support/RequestContextUtils.java

@@ -17,6 +17,7 @@
 package org.springframework.web.servlet.support;
 
 import java.util.Locale;
+import java.util.Map;
 
 import javax.servlet.ServletContext;
 import javax.servlet.ServletRequest;
@@ -27,8 +28,8 @@ import org.springframework.ui.context.ThemeSource;
 import org.springframework.web.context.WebApplicationContext;
 import org.springframework.web.context.support.WebApplicationContextUtils;
 import org.springframework.web.servlet.DispatcherServlet;
-import org.springframework.web.servlet.FlashMapManager;
 import org.springframework.web.servlet.FlashMap;
+import org.springframework.web.servlet.FlashMapManager;
 import org.springframework.web.servlet.LocaleResolver;
 import org.springframework.web.servlet.ThemeResolver;
 
@@ -156,12 +157,22 @@ public abstract class RequestContextUtils {
 	}
 
 	/**
-	 * Retrieves the flash map to use for the current request.
+	 * Return a read-only Map with flash attributes saved during the previous request. 
+	 * @param request the current request
+	 * @return a read-only Map, or {@code null}
+	 */
+	@SuppressWarnings("unchecked")
+	public static Map<String, ?> getInputFlashMap(HttpServletRequest request) {
+		return (Map<String, ?>) request.getAttribute(FlashMapManager.INPUT_FLASH_MAP_ATTRIBUTE);
+	}
+
+	/**
+	 * Return a FlashMap to add attributes to during the current request.
 	 * @param request current HTTP request
 	 * @return the flash map for the current request; never {@code null}.
 	 */
-	public static FlashMap getFlashMap(HttpServletRequest request) {
+	public static FlashMap getOutputFlashMap(HttpServletRequest request) {
-		return (FlashMap) request.getAttribute(FlashMapManager.CURRENT_FLASH_MAP_ATTRIBUTE);
+		return (FlashMap) request.getAttribute(FlashMapManager.OUTPUT_FLASH_MAP_ATTRIBUTE);
 	}
 
 }

org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/view/RedirectView.java

@@ -262,7 +262,7 @@ public class RedirectView extends AbstractUrlBasedView {
 			model = removeKeys(model, uriTemplate.getVariableNames());
 		}
 
-		FlashMap flashMap = RequestContextUtils.getFlashMap(request);
+		FlashMap flashMap = RequestContextUtils.getOutputFlashMap(request);
 		if (!CollectionUtils.isEmpty(flashMap)) {
 			flashMap.setExpectedRequestUri(request, targetUrl.toString());
 		}

org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ServletAnnotationControllerHandlerMethodTests.java

@@ -1469,7 +1469,7 @@ public class ServletAnnotationControllerHandlerMethodTests extends AbstractServl
 		
 		assertEquals(200, response.getStatus());
 		assertEquals("messages/new", response.getForwardedUrl());
-		assertTrue(RequestContextUtils.getFlashMap(request).isEmpty());
+		assertTrue(RequestContextUtils.getOutputFlashMap(request).isEmpty());
 
 		// POST -> success
 		request = new MockHttpServletRequest("POST", "/messages");
@@ -1480,7 +1480,7 @@ public class ServletAnnotationControllerHandlerMethodTests extends AbstractServl
 
 		assertEquals(200, response.getStatus());
 		assertEquals("/messages/1?name=value", response.getRedirectedUrl());
-		assertEquals("yay!", RequestContextUtils.getFlashMap(request).get("successMessage"));
+		assertEquals("yay!", RequestContextUtils.getOutputFlashMap(request).get("successMessage"));
 
 		// GET after POST
 		request = new MockHttpServletRequest("GET", "/messages/1");
@@ -1491,7 +1491,7 @@ public class ServletAnnotationControllerHandlerMethodTests extends AbstractServl
 
 		assertEquals(200, response.getStatus());
 		assertEquals("Got: yay!", response.getContentAsString());
-		assertTrue(RequestContextUtils.getFlashMap(request).isEmpty());
+		assertTrue(RequestContextUtils.getOutputFlashMap(request).isEmpty());
 	}
 
 	/*

org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/support/DefaultFlashMapManagerTests.java

@@ -54,7 +54,7 @@ public class DefaultFlashMapManagerTests {
 		boolean initialized = this.flashMapManager.requestStarted(this.request);
 
 		assertTrue("Current FlashMap not initialized on first call", initialized);
-		assertNotNull("Current FlashMap not found", RequestContextUtils.getFlashMap(request));
+		assertNotNull("Current FlashMap not found", RequestContextUtils.getOutputFlashMap(request));
 		
 		initialized = this.flashMapManager.requestStarted(this.request);
 		
@@ -62,38 +62,39 @@ public class DefaultFlashMapManagerTests {
 	}
 
 	@Test
-	public void lookupPreviousFlashMap() {
+	public void lookupInputFlashMap() {
 		FlashMap flashMap = new FlashMap();
+		flashMap.put("key", "value");
 
 		List<FlashMap> allMaps = createFlashMapsSessionAttribute();
 		allMaps.add(flashMap);
 
 		this.flashMapManager.requestStarted(this.request);
 		
-		assertSame(flashMap, request.getAttribute(DefaultFlashMapManager.PREVIOUS_FLASH_MAP_ATTRIBUTE));
+		assertEquals(flashMap, request.getAttribute(DefaultFlashMapManager.INPUT_FLASH_MAP_ATTRIBUTE));
-		assertEquals("Previous FlashMap should have been removed", 0, allMaps.size());
+		assertEquals("Input FlashMap should have been removed", 0, allMaps.size());
 	}
 	
 	@Test
-	public void lookupPreviousFlashMapExpectedUrlPath() {
+	public void lookupInputFlashMapExpectedUrlPath() {
 		FlashMap emptyFlashMap = new FlashMap();
 
 		FlashMap oneFlashMap = new FlashMap();
 		oneFlashMap.setExpectedRequestUri(null, "/one");
 
-		FlashMap oneOtherFlashMap = new FlashMap();
+		FlashMap secondFlashMap = new FlashMap();
-		oneOtherFlashMap.setExpectedRequestUri(null, "/one/other");
+		secondFlashMap.setExpectedRequestUri(null, "/one/two");
 		
 		List<FlashMap> allMaps = createFlashMapsSessionAttribute();
 		allMaps.add(emptyFlashMap);
 		allMaps.add(oneFlashMap);
-		allMaps.add(oneOtherFlashMap);
+		allMaps.add(secondFlashMap);
 		Collections.shuffle(allMaps);
 
 		this.request.setRequestURI("/one");
 		this.flashMapManager.requestStarted(this.request);
 		
-		assertSame(oneFlashMap, request.getAttribute(DefaultFlashMapManager.PREVIOUS_FLASH_MAP_ATTRIBUTE));
+		assertEquals(oneFlashMap, request.getAttribute(DefaultFlashMapManager.INPUT_FLASH_MAP_ATTRIBUTE));
 	}
 
 	@Test
@@ -116,7 +117,7 @@ public class DefaultFlashMapManagerTests {
 	public void saveFlashMap() throws InterruptedException {
 		FlashMap flashMap = new FlashMap();
 		flashMap.put("name", "value");
-		request.setAttribute(DefaultFlashMapManager.CURRENT_FLASH_MAP_ATTRIBUTE, flashMap);
+		request.setAttribute(DefaultFlashMapManager.OUTPUT_FLASH_MAP_ATTRIBUTE, flashMap);
 
 		this.flashMapManager.setFlashMapTimeout(0);
 		this.flashMapManager.requestCompleted(this.request);
@@ -132,7 +133,7 @@ public class DefaultFlashMapManagerTests {
 
 	@Test
 	public void saveFlashMapIsEmpty() throws InterruptedException {
-		request.setAttribute(DefaultFlashMapManager.CURRENT_FLASH_MAP_ATTRIBUTE, new FlashMap());
+		request.setAttribute(DefaultFlashMapManager.OUTPUT_FLASH_MAP_ATTRIBUTE, new FlashMap());
 		this.flashMapManager.requestCompleted(this.request);
 
 		assertNull(getFlashMapsSessionAttribute());

org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/view/RedirectViewTests.java

@@ -115,7 +115,7 @@ public class RedirectViewTests {
 		MockHttpServletResponse response = new MockHttpServletResponse();
 		FlashMap flashMap = new FlashMap();
 		flashMap.put("successMessage", "yay!");
-		request.setAttribute(FlashMapManager.CURRENT_FLASH_MAP_ATTRIBUTE, flashMap);
+		request.setAttribute(FlashMapManager.OUTPUT_FLASH_MAP_ATTRIBUTE, flashMap);
 		rv.render(new ModelMap("id", "1"), request, response);
 		assertEquals(303, response.getStatus());
 		assertEquals("http://url.somewhere.com/path?id=1", response.getHeader("Location"));