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 038c6815ac8..4e0dc66ed8d 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 @@ -41,11 +41,11 @@ public class FlashMap extends HashMap implements Comparable expectedRequestParameters = new LinkedHashMap(); - 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. 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 73bac7afaad..f36e73e3625 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 @@ -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 - * a redirect. For example redirecting from a POST that creates a resource - * to the page that shows the created resource and passing along a - * success message that needs to be shown once only. + * underlying storage until the next request. + * + *

The most common use case for using flash storage is a redirect. + * 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. - * @see RequestContextUtils#getFlashMap + * Request attribute holding the read-only Map with flash attributes saved + * 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. - * Access to the previous FlashMap should generally not be needed - * since its content is exposed as attributes of the current - * request. However, it may be useful to expose previous request - * flash attributes in other ways such as in the model of annotated - * controllers. + * Request attribute holding the {@link FlashMap} to add attributes to during + * the current request. + * @see RequestContextUtils#getOutputFlashMap(HttpServletRequest) */ - 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: *

    - *
  • Create a new FlashMap and make it available to the current request - * under the request attribute {@link #CURRENT_FLASH_MAP_ATTRIBUTE}. - *
  • Locate the FlashMap saved on the previous request and expose its - * contents as attributes in the current request, also exposing the - * previous FlashMap under {@link #PREVIOUS_FLASH_MAP_ATTRIBUTE}. - *
  • Check for and remove expired FlashMap instances. + *
  • Create a FlashMap and make it available under the request attribute + * {@link #OUTPUT_FLASH_MAP_ATTRIBUTE}. + *
  • Locate the FlashMap saved during the previous request and make it + * available under the request attribute {@link #INPUT_FLASH_MAP_ATTRIBUTE}. + *
  • Remove expired FlashMap instances. *
- * - *

If the {@link #CURRENT_FLASH_MAP_ATTRIBUTE} request attribute exists - * in the current request, this method should return "false" immediately. + *

If the {@link #OUTPUT_FLASH_MAP_ATTRIBUTE} request attribute exists + * 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 - * {@link #CURRENT_FLASH_MAP_ATTRIBUTE} and if it is not empty, save it - * in the underlying storage. - * + * Access the FlashMap with attributes added during the current request and + * if it is not empty, save it in the underlying storage. *

If the call to {@link #requestStarted} returned "false", this * method is not invoked. */ diff --git a/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerAdapter.java b/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerAdapter.java index 6bc2152b4f6..8fc09573f49 100644 --- a/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerAdapter.java +++ b/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); - currentFlashMap.putAll(redirectModel.getFlashAttributes()); + FlashMap flashMap = RequestContextUtils.getOutputFlashMap(request); + flashMap.putAll(redirectModel.getFlashAttributes()); } return mav; } 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 7b07b73044c..e521b2fd758 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 @@ -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} * - *

This method never creates an HTTP session. The current FlashMap is - * exposed as a request attribute only and is not saved in the session - * until {@link #requestCompleted}. + *

This method never creates an HTTP session. The new FlashMap created + * for the current request is exposed as a request attribute only and is + * 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(); - request.setAttribute(CURRENT_FLASH_MAP_ATTRIBUTE, currentFlashMap); + FlashMap outputFlashMap = new FlashMap(); + request.setAttribute(OUTPUT_FLASH_MAP_ATTRIBUTE, outputFlashMap); - FlashMap previousFlashMap = lookupPreviousFlashMap(request); - if (previousFlashMap != null) { - WebUtils.exposeRequestAttributes(request, previousFlashMap); - request.setAttribute(PREVIOUS_FLASH_MAP_ATTRIBUTE, previousFlashMap); + Map inputFlashMap = getFlashMap(request); + if (inputFlashMap != null) { + request.setAttribute(INPUT_FLASH_MAP_ATTRIBUTE, inputFlashMap); } - // Remove expired flash maps - List allMaps = retrieveFlashMaps(request, false); - if (allMaps != null && !allMaps.isEmpty()) { - List expiredMaps = new ArrayList(); - for (FlashMap flashMap : allMaps) { - if (flashMap.isExpired()) { - if (logger.isDebugEnabled()) { - logger.debug("Removing expired FlashMap: " + flashMap); - } - expiredMaps.add(flashMap); - } - } - allMaps.removeAll(expiredMaps); - } + removeExpiredFlashMaps(request); 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 getFlashMap(HttpServletRequest request) { List 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 allMaps = retrieveFlashMaps(request, false); + if (allMaps != null && !allMaps.isEmpty()) { + List expiredMaps = new ArrayList(); + for (FlashMap flashMap : allMaps) { + if (flashMap.isExpired()) { + if (logger.isDebugEnabled()) { + logger.debug("Removing expired FlashMap: " + flashMap); + } + expiredMaps.add(flashMap); + } + } + allMaps.removeAll(expiredMaps); + } + } + /** * {@inheritDoc} * *

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()) { diff --git a/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/support/RequestContextUtils.java b/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/support/RequestContextUtils.java index 4f4575e01c3..62497fe2636 100644 --- a/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/support/RequestContextUtils.java +++ b/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 getInputFlashMap(HttpServletRequest request) { + return (Map) 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) { - return (FlashMap) request.getAttribute(FlashMapManager.CURRENT_FLASH_MAP_ATTRIBUTE); + public static FlashMap getOutputFlashMap(HttpServletRequest request) { + return (FlashMap) request.getAttribute(FlashMapManager.OUTPUT_FLASH_MAP_ATTRIBUTE); } } diff --git a/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/view/RedirectView.java b/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/view/RedirectView.java index 9eafad72b45..d5e8cd8826f 100644 --- a/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/view/RedirectView.java +++ b/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()); } diff --git a/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ServletAnnotationControllerHandlerMethodTests.java b/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ServletAnnotationControllerHandlerMethodTests.java index 266934a81bb..293d4cc6201 100644 --- a/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ServletAnnotationControllerHandlerMethodTests.java +++ b/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()); } /* 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 28c9052caf8..258c5b2ec54 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 @@ -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 allMaps = createFlashMapsSessionAttribute(); allMaps.add(flashMap); this.flashMapManager.requestStarted(this.request); - assertSame(flashMap, request.getAttribute(DefaultFlashMapManager.PREVIOUS_FLASH_MAP_ATTRIBUTE)); - assertEquals("Previous FlashMap should have been removed", 0, allMaps.size()); + assertEquals(flashMap, request.getAttribute(DefaultFlashMapManager.INPUT_FLASH_MAP_ATTRIBUTE)); + 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(); - oneOtherFlashMap.setExpectedRequestUri(null, "/one/other"); + FlashMap secondFlashMap = new FlashMap(); + secondFlashMap.setExpectedRequestUri(null, "/one/two"); List 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()); diff --git a/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/view/RedirectViewTests.java b/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/view/RedirectViewTests.java index cae4242f14f..30081ae85a4 100644 --- a/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/view/RedirectViewTests.java +++ b/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"));