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 9dfaf6fec26..a89bf9051b3 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 @@ -16,8 +16,15 @@ package org.springframework.web.servlet; +import java.util.LinkedHashMap; +import java.util.Map; + +import javax.servlet.http.HttpServletRequest; + +import org.springframework.beans.BeanUtils; import org.springframework.ui.ModelMap; -import org.springframework.util.Assert; +import org.springframework.util.StringUtils; +import org.springframework.web.util.UrlPathHelper; /** * Stores attributes that need to be made available in the next request. @@ -25,52 +32,108 @@ import org.springframework.util.Assert; * @author Rossen Stoyanchev * @since 3.1 */ -public class FlashMap extends ModelMap { +public class FlashMap extends ModelMap implements Comparable { private static final long serialVersionUID = 1L; - - private final String key; - - private final String keyParameterName; - + + private String expectedUrlPath; + + private Map expectedRequestParameters = new LinkedHashMap(); + + private UrlPathHelper urlPathHelper = new UrlPathHelper(); + private long expirationStartTime; private int timeToLive; - + /** - * Create a FlashMap with a unique key. + * Provide a URL path to help identify the request this FlashMap should be + * made available to. This will usually be the target URL of a redirect. + * If not set, this FlashMap will match to requests with any URL path. + * + *

If the {@code url} parameter is not a URL path but is an absolute + * or a relative URL, the unnecessary parts of the URL are removed the + * resulting URL path is normalized. + * + * @param request the current request + * @param url a URL path, an absolute URL, or a relative URL */ - public FlashMap(String key, String keyParameterName) { - Assert.notNull("The key is required", key); - Assert.notNull("The key parameter name is required", keyParameterName); - this.key = key; - this.keyParameterName = keyParameterName; + public void setExpectedUrlPath(HttpServletRequest request, String url) { + this.expectedUrlPath = (url != null) ? normalizeRelativeUrlPath(request, extractUrlPath(url)) : null; } - /** - * Create a FlashMap without a key. - */ - public FlashMap() { - this.key = null; - this.keyParameterName = null; + private String extractUrlPath(String url) { + int index = url.indexOf("://"); + if (index != -1) { + index = url.indexOf("/", index + 3); + url = (index != -1) ? url.substring(index) : ""; + } + index = url.indexOf("?"); + return (index != -1) ? url.substring(0, index) : url; } - /** - * Return the key assigned to this FlashMap instance; - * or {@code null} if a unique key has not been assigned. - */ - public String getKey() { - return this.key; + private String normalizeRelativeUrlPath(HttpServletRequest request, String path) { + if (!path.startsWith("/")) { + String requestUri = this.urlPathHelper.getRequestUri(request); + path = requestUri.substring(0, requestUri.lastIndexOf('/') + 1) + path; + path = StringUtils.cleanPath(path); + } + return path; } - + /** - * Return the name of the request parameter to use when appending the flash - * key to a redirect URL. + * Provide request parameter pairs to help identify the request this FlashMap + * should be made available to. If expected parameters are not set, this + * FlashMap instance will match to requests with any parameters. + * + *

Although the provided map contain any Object values, only non-"simple" + * value types as defined in {@link BeanUtils#isSimpleValueType} are used. + * + * @param params a Map with the names and values of expected parameters. */ - public String getKeyParameterName() { - return keyParameterName; + public void setExpectedRequestParameters(Map params) { + this.expectedRequestParameters = new LinkedHashMap(); + if (params != null) { + for (String name : params.keySet()) { + Object value = params.get(name); + if ((value != null) && BeanUtils.isSimpleValueType(value.getClass())) { + this.expectedRequestParameters.put(name, value.toString()); + } + } + } } - + + /** + * Whether this FlashMap matches to the given request by checking + * expectations provided via {@link #setExpectedUrlPath} and + * {@link #setExpectedRequestParameters}. + * + * @param request the current request + * + * @return "true" if the expectations match or there are no expectations. + */ + public boolean matches(HttpServletRequest request) { + if (this.expectedUrlPath != null) { + if (!matchPathsIgnoreTrailingSlash(this.urlPathHelper.getRequestUri(request), this.expectedUrlPath)) { + return false; + } + } + if (this.expectedRequestParameters != null) { + for (Map.Entry entry : this.expectedRequestParameters.entrySet()) { + if (!entry.getValue().equals(request.getParameter(entry.getKey()))) { + return false; + } + } + } + return true; + } + + private boolean matchPathsIgnoreTrailingSlash(String path1, String path2) { + path1 = path1.endsWith("/") ? path1.substring(0, path1.length() - 1) : path1; + path2 = path2.endsWith("/") ? path2.substring(0, path2.length() - 1) : path2; + return path1.equals(path2); + } + /** * Start the expiration period for this instance. After the given number of * seconds calls to {@link #isExpired()} will return "true". @@ -94,4 +157,32 @@ public class FlashMap extends ModelMap { } } + /** + * Compare two FlashMap instances. One instance is preferred over the other + * if it has an expected URL path or if it has a greater number of expected + * request parameters. + * + *

It is expected that both instances have been matched against the + * current request via {@link FlashMap#matches}. + */ + public int compareTo(FlashMap other) { + int thisUrlPath = (this.expectedUrlPath != null) ? 1 : 0; + int otherUrlPath = (other.expectedUrlPath != null) ? 1 : 0; + if (thisUrlPath != otherUrlPath) { + return otherUrlPath - thisUrlPath; + } + else { + return other.expectedRequestParameters.size() - this.expectedRequestParameters.size(); + } + } + + @Override + public String toString() { + StringBuilder result = new StringBuilder(); + result.append("[Attributes=").append(super.toString()); + result.append(", expecteUrlPath=").append(this.expectedUrlPath); + result.append(", expectedRequestParameters=" + this.expectedRequestParameters.toString()).append("]"); + return result.toString(); + } + } 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 98d8778034b..73bac7afaad 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,11 +22,11 @@ import org.springframework.web.servlet.support.RequestContextUtils; /** * A strategy interface for maintaining {@link FlashMap} instances in some - * underlying storage between two requests. This is typically used when - * redirecting from one URL to another. + * 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. * - * TODO ... - * * @author Rossen Stoyanchev * @since 3.1 * @@ -35,13 +35,18 @@ import org.springframework.web.servlet.support.RequestContextUtils; public interface FlashMapManager { /** - * Request attribute to hold the current request FlashMap. + * Request attribute to hold the current request FlashMap. * @see RequestContextUtils#getFlashMap */ public static final String CURRENT_FLASH_MAP_ATTRIBUTE = DispatcherServlet.class.getName() + ".CURRENT_FLASH_MAP"; /** - * Request attribute to hold the FlashMap from the previous request. + * 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. */ public static final String PREVIOUS_FLASH_MAP_ATTRIBUTE = DispatcherServlet.class.getName() + ".PREVIOUS_FLASH_MAP"; @@ -51,21 +56,27 @@ public interface FlashMapManager { *

  • 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. - *
  • Remove expired flash map instances. + * 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. * * + *

    If the {@link #CURRENT_FLASH_MAP_ATTRIBUTE} request attribute exists + * 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 - * if the {@link #CURRENT_FLASH_MAP_ATTRIBUTE} request attribute exists. + * @return "true" if flash storage tasks were performed; "false" otherwise. */ boolean requestStarted(HttpServletRequest request); /** - * Access the current FlashMap through the {@link #CURRENT_FLASH_MAP_ATTRIBUTE} - * request attribute and if not empty, save it in the underlying storage. This - * method should be invoked after {@link #requestStarted} and if it returned "true". + * 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. + * + *

    If the call to {@link #requestStarted} returned "false", this + * method is not invoked. */ void requestCompleted(HttpServletRequest request); 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 68cbf0af6e5..189f3013903 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 @@ -16,58 +16,35 @@ package org.springframework.web.servlet.support; -import java.util.Iterator; -import java.util.Map; -import java.util.Random; -import java.util.concurrent.ConcurrentHashMap; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.concurrent.CopyOnWriteArrayList; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpSession; +import org.apache.commons.logging.Log; +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 default implementation that saves and retrieves FlashMap instances to and - * from the HTTP session. + * A {@link FlashMapManager} that saves and retrieves FlashMap instances in the + * HTTP session. * * @author Rossen Stoyanchev * @since 3.1 */ public class DefaultFlashMapManager implements FlashMapManager { - static final String FLASH_MAPS_SESSION_ATTRIBUTE = DefaultFlashMapManager.class + ".FLASH_MAPS"; + private static final String FLASH_MAPS_SESSION_ATTRIBUTE = DefaultFlashMapManager.class + ".FLASH_MAPS"; - private boolean useUniqueFlashKey = true; - - private String flashKeyParameterName = "_flashKey"; + private static final Log logger = LogFactory.getLog(DefaultFlashMapManager.class); private int flashTimeout = 180; - - private static final Random random = new Random(); - - /** - * Whether each FlashMap instance should be stored with a unique key. - * The unique key needs to be passed as a parameter in the redirect URL - * and then used to look up the FlashMap instance avoiding potential - * issues with concurrent requests. - *

    The default setting is "true". - *

    When set to "false" only one FlashMap is maintained making it - * possible for a second concurrent request (e.g. via Ajax) to "consume" - * the FlashMap inadvertently. - */ - public void setUseUniqueFlashKey(boolean useUniqueFlashKey) { - this.useUniqueFlashKey = useUniqueFlashKey; - } - - /** - * Customize the name of the request parameter to be appended to the - * redirect URL when using a unique flash key. - *

    The default value is "_flashKey". - */ - public void setFlashKeyParameterName(String parameterName) { - this.flashKeyParameterName = parameterName; - } /** * The amount of time in seconds after a request has completed processing @@ -90,92 +67,91 @@ public class DefaultFlashMapManager implements FlashMapManager { return false; } - FlashMap currentFlashMap = - this.useUniqueFlashKey ? - new FlashMap(createFlashKey(request), this.flashKeyParameterName) : new FlashMap(); + FlashMap currentFlashMap = new FlashMap(); request.setAttribute(CURRENT_FLASH_MAP_ATTRIBUTE, currentFlashMap); FlashMap previousFlashMap = lookupPreviousFlashMap(request); if (previousFlashMap != null) { - for (String name : previousFlashMap.keySet()) { - if (request.getAttribute(name) == null) { - request.setAttribute(name, previousFlashMap.get(name)); - } - } - // For exposing flash attributes in other places (e.g. annotated controllers) + WebUtils.exposeRequestAttributes(request, previousFlashMap); request.setAttribute(PREVIOUS_FLASH_MAP_ATTRIBUTE, previousFlashMap); } - // Check and remove expired instances - Map allFlashMaps = retrieveAllFlashMaps(request, false); - if (allFlashMaps != null && !allFlashMaps.isEmpty()) { - Iterator iterator = allFlashMaps.values().iterator(); - while (iterator.hasNext()) { - if (iterator.next().isExpired()) { - iterator.remove(); + // 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); } return true; } /** - * Create a unique flash key. The default implementation uses {@link Random}. - * @return the unique key; never {@code null}. - */ - protected String createFlashKey(HttpServletRequest request) { - return String.valueOf(random.nextInt()); - } - - /** - * Return the FlashMap from the previous request, if available. - * If {@link #useUniqueFlashKey} is "true", a flash key parameter is - * expected to be in the request. Otherwise there can be only one - * FlashMap instance to return. + * Return the FlashMap from the previous request. * * @return the FlashMap from the previous request; or {@code null} if none. */ private FlashMap lookupPreviousFlashMap(HttpServletRequest request) { - Map flashMaps = retrieveAllFlashMaps(request, false); - if (flashMaps != null && !flashMaps.isEmpty()) { - if (this.useUniqueFlashKey) { - String key = request.getParameter(this.flashKeyParameterName); - if (key != null) { - return flashMaps.remove(key); + List allMaps = retrieveFlashMaps(request, false); + if (CollectionUtils.isEmpty(allMaps)) { + return null; + } + + if (logger.isDebugEnabled()) { + logger.debug("Looking up previous FlashMap among available FlashMaps: " + allMaps); + } + + List matches = new ArrayList(); + for (FlashMap flashMap : allMaps) { + if (flashMap.matches(request)) { + if (logger.isDebugEnabled()) { + logger.debug("Matched " + flashMap); } - } - else { - String key = flashMaps.keySet().iterator().next(); - return flashMaps.remove(key); + matches.add(flashMap); } } + + if (!matches.isEmpty()) { + Collections.sort(matches); + return matches.remove(0); + } + return null; } /** - * Retrieve all FlashMap instances from the HTTP session in a thread-safe way. + * Retrieve the list of all FlashMap instances from the HTTP session. * @param request the current request * @param allowCreate whether to create and the FlashMap container if not found * @return a Map with all stored FlashMap instances; or {@code null} */ @SuppressWarnings("unchecked") - private Map retrieveAllFlashMaps(HttpServletRequest request, boolean allowCreate) { + private List retrieveFlashMaps(HttpServletRequest request, boolean allowCreate) { HttpSession session = request.getSession(allowCreate); if (session == null) { return null; } - Map result = (Map) session.getAttribute(FLASH_MAPS_SESSION_ATTRIBUTE); - if (result == null && allowCreate) { + + List allMaps = (List) session.getAttribute(FLASH_MAPS_SESSION_ATTRIBUTE); + if (allMaps == null && allowCreate) { synchronized (DefaultFlashMapManager.class) { - result = (Map) session.getAttribute(FLASH_MAPS_SESSION_ATTRIBUTE); - if (result == null) { - result = new ConcurrentHashMap(5); - session.setAttribute(FLASH_MAPS_SESSION_ATTRIBUTE, result); + allMaps = (List) session.getAttribute(FLASH_MAPS_SESSION_ATTRIBUTE); + if (allMaps == null) { + allMaps = new CopyOnWriteArrayList(); + session.setAttribute(FLASH_MAPS_SESSION_ATTRIBUTE, allMaps); } } } - return result; + + return allMaps; } /** @@ -187,13 +163,16 @@ public class DefaultFlashMapManager implements FlashMapManager { FlashMap flashMap = (FlashMap) request.getAttribute(CURRENT_FLASH_MAP_ATTRIBUTE); if (flashMap == null) { throw new IllegalStateException( - "Did not find current FlashMap exposed as request attribute " + CURRENT_FLASH_MAP_ATTRIBUTE); + "Did not find a FlashMap exposed as the request attribute " + CURRENT_FLASH_MAP_ATTRIBUTE); } + if (!flashMap.isEmpty()) { - Map allFlashMaps = retrieveAllFlashMaps(request, true); + if (logger.isDebugEnabled()) { + logger.debug("Saving FlashMap=" + flashMap); + } + List allFlashMaps = retrieveFlashMaps(request, true); flashMap.startExpirationPeriod(this.flashTimeout); - String key = this.useUniqueFlashKey ? flashMap.getKey() : "flashMap"; - allFlashMaps.put(key, flashMap); + allFlashMaps.add(flashMap); } } 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 2393a7b4104..dae1cd48291 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 @@ -36,7 +36,6 @@ import javax.servlet.http.HttpServletResponse; import org.springframework.beans.BeanUtils; import org.springframework.http.HttpStatus; -import org.springframework.ui.ModelMap; import org.springframework.util.CollectionUtils; import org.springframework.util.ObjectUtils; import org.springframework.web.servlet.FlashMap; @@ -44,7 +43,6 @@ import org.springframework.web.servlet.HandlerMapping; import org.springframework.web.servlet.View; import org.springframework.web.servlet.support.RequestContextUtils; import org.springframework.web.util.UriTemplate; -import org.springframework.web.util.UriUtils; import org.springframework.web.util.WebUtils; /** @@ -263,19 +261,17 @@ public class RedirectView extends AbstractUrlBasedView { targetUrl = new StringBuilder(uriTemplate.expand(vars).toString()); model = removeKeys(model, uriTemplate.getVariableNames()); } + + FlashMap flashMap = RequestContextUtils.getFlashMap(request); + if (!CollectionUtils.isEmpty(flashMap)) { + flashMap.setExpectedUrlPath(request, targetUrl.toString()); + flashMap.setExpectedRequestParameters(model); + } if (this.exposeModelAttributes) { appendQueryProperties(targetUrl, model, enc); } - - FlashMap flashMap = RequestContextUtils.getFlashMap(request); - if (flashMap != null && !flashMap.isEmpty()) { - if (flashMap.getKey() != null) { - ModelMap queryParam = new ModelMap(flashMap.getKeyParameterName(), flashMap.getKey()); - appendQueryProperties(targetUrl, queryParam, enc); - } - } - + return targetUrl.toString(); } @@ -295,11 +291,7 @@ public class RedirectView extends AbstractUrlBasedView { @Override protected URI encodeUri(String uri) { try { - String encoded = UriUtils.encodeUri(uri, encoding); - return new URI(encoded); - } - catch (UnsupportedEncodingException ex) { - throw new IllegalStateException(ex); + return new URI(uri); } catch (URISyntaxException ex) { throw new IllegalArgumentException("Could not create URI from [" + uri + "]: " + ex, ex); 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 c58bf0fda5a..e1de3dfa8bb 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 @@ -133,7 +133,6 @@ import org.springframework.web.context.support.GenericWebApplicationContext; import org.springframework.web.method.HandlerMethod; import org.springframework.web.method.support.HandlerMethodArgumentResolver; import org.springframework.web.multipart.support.StringMultipartFileEditor; -import org.springframework.web.servlet.FlashMap; import org.springframework.web.servlet.ModelAndView; import org.springframework.web.servlet.View; import org.springframework.web.servlet.ViewResolver; @@ -1479,21 +1478,20 @@ public class ServletAnnotationControllerHandlerMethodTests extends AbstractServl response = new MockHttpServletResponse(); getServlet().service(request, response); - FlashMap flashMap = RequestContextUtils.getFlashMap(request); - - assertNotNull(flashMap); assertEquals(200, response.getStatus()); - assertEquals("/messages/1?name=value&_flashKey=" + flashMap.getKey(), response.getRedirectedUrl()); + assertEquals("/messages/1?name=value", response.getRedirectedUrl()); + assertEquals("yay!", RequestContextUtils.getFlashMap(request).get("successMessage")); // GET after POST request = new MockHttpServletRequest("GET", "/messages/1"); + request.addParameter("name", "value"); request.setSession(session); - request.setParameter("_flashKey", String.valueOf(flashMap.getKey())); response = new MockHttpServletResponse(); getServlet().service(request, response); assertEquals(200, response.getStatus()); assertEquals("Got: yay!", response.getContentAsString()); + assertTrue(RequestContextUtils.getFlashMap(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 2f7d3afdbf7..581e115878d 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 @@ -23,14 +23,14 @@ import static org.junit.Assert.assertNull; import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; -import java.util.HashMap; -import java.util.Map; +import java.util.Collections; +import java.util.List; +import java.util.concurrent.CopyOnWriteArrayList; import org.junit.Before; import org.junit.Test; import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.web.servlet.FlashMap; -import org.springframework.web.servlet.FlashMapManager; /** * Test fixture for {@link DefaultFlashMapManager} tests. @@ -50,125 +50,102 @@ public class DefaultFlashMapManagerTests { } @Test - public void requestAlreadyStarted() { - request.setAttribute(FlashMapManager.CURRENT_FLASH_MAP_ATTRIBUTE, new FlashMap()); - boolean actual = this.flashMapManager.requestStarted(this.request); + public void requestStarted() { + boolean initialized = this.flashMapManager.requestStarted(this.request); + + assertTrue("Current FlashMap not initialized on first call", initialized); + assertNotNull("Current FlashMap not found", RequestContextUtils.getFlashMap(request)); - assertFalse(actual); - } - - @Test - public void createFlashMap() { - boolean actual = this.flashMapManager.requestStarted(this.request); - FlashMap flashMap = RequestContextUtils.getFlashMap(this.request); - - assertTrue(actual); - assertNotNull(flashMap); - assertNotNull(flashMap.getKey()); - assertEquals("_flashKey", flashMap.getKeyParameterName()); - } - - @Test - public void createFlashMapWithoutKey() { - this.flashMapManager.setUseUniqueFlashKey(false); - boolean actual = this.flashMapManager.requestStarted(this.request); - FlashMap flashMap = RequestContextUtils.getFlashMap(this.request); - - assertTrue(actual); - assertNotNull(flashMap); - assertNull(flashMap.getKey()); - assertNull(flashMap.getKeyParameterName()); + initialized = this.flashMapManager.requestStarted(this.request); + + assertFalse("Current FlashMap initialized twice", initialized); } @Test public void lookupPreviousFlashMap() { - FlashMap flashMap = new FlashMap("key", "_flashKey"); - flashMap.put("name", "value"); - Map allFlashMaps = new HashMap(); - allFlashMaps.put(flashMap.getKey(), flashMap); - - this.request.getSession().setAttribute(DefaultFlashMapManager.FLASH_MAPS_SESSION_ATTRIBUTE, allFlashMaps); - this.request.addParameter("_flashKey", flashMap.getKey()); + FlashMap flashMap = new FlashMap(); + + List allMaps = createFlashMapsSessionAttribute(); + allMaps.add(flashMap); + this.flashMapManager.requestStarted(this.request); assertSame(flashMap, request.getAttribute(DefaultFlashMapManager.PREVIOUS_FLASH_MAP_ATTRIBUTE)); - assertEquals("value", request.getAttribute("name")); } - + @Test - public void lookupPreviousFlashMapWithoutKey() { - Map allFlashMaps = new HashMap(); - request.getSession().setAttribute(DefaultFlashMapManager.FLASH_MAPS_SESSION_ATTRIBUTE, allFlashMaps); + public void lookupPreviousFlashMapExpectedUrlPath() { + FlashMap emptyFlashMap = new FlashMap(); - FlashMap flashMap = new FlashMap(); - flashMap.put("name", "value"); - allFlashMaps.put("key", flashMap); + FlashMap oneFlashMap = new FlashMap(); + oneFlashMap.setExpectedUrlPath(null, "/one"); + + FlashMap oneOtherFlashMap = new FlashMap(); + oneOtherFlashMap.setExpectedUrlPath(null, "/one/other"); - this.flashMapManager.setUseUniqueFlashKey(false); + List allMaps = createFlashMapsSessionAttribute(); + allMaps.add(emptyFlashMap); + allMaps.add(oneFlashMap); + allMaps.add(oneOtherFlashMap); + Collections.shuffle(allMaps); + + this.request.setRequestURI("/one"); this.flashMapManager.requestStarted(this.request); - assertSame(flashMap, this.request.getAttribute(DefaultFlashMapManager.PREVIOUS_FLASH_MAP_ATTRIBUTE)); - assertEquals("value", this.request.getAttribute("name")); + assertSame(oneFlashMap, request.getAttribute(DefaultFlashMapManager.PREVIOUS_FLASH_MAP_ATTRIBUTE)); } - @SuppressWarnings("static-access") @Test - public void removeExpired() throws InterruptedException { - FlashMap[] flashMapArray = new FlashMap[5]; - flashMapArray[0] = new FlashMap("key0", "_flashKey"); - flashMapArray[1] = new FlashMap("key1", "_flashKey"); - flashMapArray[2] = new FlashMap("key2", "_flashKey"); - flashMapArray[3] = new FlashMap("key3", "_flashKey"); - flashMapArray[4] = new FlashMap("key4", "_flashKey"); - - Map allFlashMaps = new HashMap(); - for (FlashMap flashMap : flashMapArray) { - allFlashMaps.put(flashMap.getKey(), flashMap); + public void removeExpiredFlashMaps() throws InterruptedException { + List allMaps = createFlashMapsSessionAttribute(); + for (int i=0; i < 5; i++) { + FlashMap flashMap = new FlashMap(); + allMaps.add(flashMap); + flashMap.startExpirationPeriod(0); } - flashMapArray[1].startExpirationPeriod(0); - flashMapArray[3].startExpirationPeriod(0); - - Thread.currentThread().sleep(5); + Thread.sleep(5); - MockHttpServletRequest request = new MockHttpServletRequest(); - request.getSession().setAttribute(DefaultFlashMapManager.FLASH_MAPS_SESSION_ATTRIBUTE, allFlashMaps); - request.setParameter("_flashKey", "key0"); - this.flashMapManager.requestStarted(request); + this.flashMapManager.requestStarted(this.request); - assertEquals(2, allFlashMaps.size()); - assertNotNull(allFlashMaps.get("key2")); - assertNotNull(allFlashMaps.get("key4")); + assertEquals(0, allMaps.size()); } - @SuppressWarnings({ "unchecked", "static-access" }) @Test public void saveFlashMap() throws InterruptedException { - FlashMap flashMap = new FlashMap("key", "_flashKey"); + FlashMap flashMap = new FlashMap(); flashMap.put("name", "value"); request.setAttribute(DefaultFlashMapManager.CURRENT_FLASH_MAP_ATTRIBUTE, flashMap); this.flashMapManager.setFlashMapTimeout(0); this.flashMapManager.requestCompleted(this.request); - Thread.currentThread().sleep(1); + Thread.sleep(1); - String sessionKey = DefaultFlashMapManager.FLASH_MAPS_SESSION_ATTRIBUTE; - Map allFlashMaps = (Map) this.request.getSession().getAttribute(sessionKey); + List allMaps = getFlashMapsSessionAttribute(); - assertSame(flashMap, allFlashMaps.get("key")); + assertNotNull(allMaps); + assertSame(flashMap, allMaps.get(0)); assertTrue(flashMap.isExpired()); } @Test - public void saveEmptyFlashMap() throws InterruptedException { - FlashMap flashMap = new FlashMap("key", "_flashKey"); - request.setAttribute(DefaultFlashMapManager.CURRENT_FLASH_MAP_ATTRIBUTE, flashMap); - - this.flashMapManager.setFlashMapTimeout(0); + public void saveFlashMapIsEmpty() throws InterruptedException { + request.setAttribute(DefaultFlashMapManager.CURRENT_FLASH_MAP_ATTRIBUTE, new FlashMap()); this.flashMapManager.requestCompleted(this.request); - assertNull(this.request.getSession().getAttribute(DefaultFlashMapManager.FLASH_MAPS_SESSION_ATTRIBUTE)); + assertNull(getFlashMapsSessionAttribute()); + } + + @SuppressWarnings("unchecked") + private List getFlashMapsSessionAttribute() { + return (List) this.request.getSession().getAttribute(DefaultFlashMapManager.class + ".FLASH_MAPS"); + } + + private List createFlashMapsSessionAttribute() { + List allMaps = new CopyOnWriteArrayList(); + this.request.getSession().setAttribute(DefaultFlashMapManager.class + ".FLASH_MAPS", allMaps); + return allMaps; } } diff --git a/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/support/FlashMapTests.java b/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/support/FlashMapTests.java new file mode 100644 index 00000000000..aea9a138457 --- /dev/null +++ b/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/support/FlashMapTests.java @@ -0,0 +1,169 @@ +/* + * Copyright 2002-2011 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.web.servlet.support; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +import org.junit.Test; +import org.springframework.mock.web.MockHttpServletRequest; +import org.springframework.ui.ModelMap; +import org.springframework.web.servlet.FlashMap; + +/** + * Test fixture for {@link FlashMap} tests. + * + * @author Rossen Stoyanchev + */ +public class FlashMapTests { + + @Test + public void matchAnyUrlPath() { + FlashMap flashMap = new FlashMap(); + + assertTrue(flashMap.matches(new MockHttpServletRequest("GET", ""))); + assertTrue(flashMap.matches(new MockHttpServletRequest("GET", "/"))); + assertTrue(flashMap.matches(new MockHttpServletRequest("GET", "/yes"))); + } + + @Test + public void matchUrlPath() { + FlashMap flashMap = new FlashMap(); + flashMap.setExpectedUrlPath(null, "/yes"); + + assertTrue(flashMap.matches(new MockHttpServletRequest("GET", "/yes"))); + assertTrue(flashMap.matches(new MockHttpServletRequest("GET", "/yes/"))); + assertFalse(flashMap.matches(new MockHttpServletRequest("GET", "/yes/but"))); + assertFalse(flashMap.matches(new MockHttpServletRequest("GET", "/no"))); + + flashMap.setExpectedUrlPath(null, "/thats it?"); + + assertTrue(flashMap.matches(new MockHttpServletRequest("GET", "/thats it"))); + assertTrue(flashMap.matches(new MockHttpServletRequest("GET", "/thats%20it"))); + } + + @Test + public void matchRelativeUrlPath() { + FlashMap flashMap = new FlashMap(); + + flashMap.setExpectedUrlPath(new MockHttpServletRequest("GET", "/oh/no"), "yes"); + assertTrue(flashMap.matches(new MockHttpServletRequest("GET", "/oh/yes"))); + + flashMap.setExpectedUrlPath(new MockHttpServletRequest("GET", "/oh/not/again"), "../ok"); + assertTrue(flashMap.matches(new MockHttpServletRequest("GET", "/oh/ok"))); + + flashMap.setExpectedUrlPath(new MockHttpServletRequest("GET", "/yes/it/is"), ".."); + assertTrue(flashMap.matches(new MockHttpServletRequest("GET", "/yes"))); + + flashMap.setExpectedUrlPath(new MockHttpServletRequest("GET", "/yes/it/is"), "../"); + assertTrue(flashMap.matches(new MockHttpServletRequest("GET", "/yes"))); + + flashMap.setExpectedUrlPath(new MockHttpServletRequest("GET", "/thats it/really"), "./"); + assertTrue(flashMap.matches(new MockHttpServletRequest("GET", "/thats%20it"))); + } + + @Test + public void matchAbsoluteUrlPath() { + FlashMap flashMap = new FlashMap(); + flashMap.setExpectedUrlPath(new MockHttpServletRequest(), "http://example.com"); + + assertTrue(flashMap.matches(new MockHttpServletRequest("GET", ""))); + assertTrue(flashMap.matches(new MockHttpServletRequest("GET", "/"))); + assertFalse(flashMap.matches(new MockHttpServletRequest("GET", "/no"))); + + flashMap.setExpectedUrlPath(null, "http://example.com/"); + + assertTrue(flashMap.matches(new MockHttpServletRequest("GET", ""))); + assertTrue(flashMap.matches(new MockHttpServletRequest("GET", "/"))); + assertFalse(flashMap.matches(new MockHttpServletRequest("GET", "/no"))); + + flashMap.setExpectedUrlPath(null, "http://example.com/yes"); + + assertTrue(flashMap.matches(new MockHttpServletRequest("GET", "/yes"))); + assertTrue(flashMap.matches(new MockHttpServletRequest("GET", "/yes/"))); + assertFalse(flashMap.matches(new MockHttpServletRequest("GET", "/no"))); + assertFalse(flashMap.matches(new MockHttpServletRequest("GET", "/yes/no"))); + + flashMap.setExpectedUrlPath(null, "http://example.com/yes?a=1"); + + assertTrue(flashMap.matches(new MockHttpServletRequest("GET", "/yes"))); + } + + @Test + public void matchExpectedRequestParameter() { + String parameterName = "numero"; + + FlashMap flashMap = new FlashMap(); + flashMap.setExpectedRequestParameters(new ModelMap(parameterName, "uno")); + + MockHttpServletRequest request = new MockHttpServletRequest(); + assertFalse(flashMap.matches(request)); + + request.setParameter(parameterName, "uno"); + assertTrue(flashMap.matches(request)); + + request.setParameter(parameterName, "dos"); + assertFalse(flashMap.matches(request)); + + request.setParameter(parameterName, (String) null); + assertFalse(flashMap.matches(request)); + } + + @Test + public void isExpired() throws InterruptedException { + FlashMap flashMap = new FlashMap(); + flashMap.startExpirationPeriod(0); + + Thread.sleep(1); + + assertTrue(flashMap.isExpired()); + } + + @Test + public void notExpired() throws InterruptedException { + assertFalse(new FlashMap().isExpired()); + + FlashMap flashMap = new FlashMap(); + flashMap.startExpirationPeriod(10); + Thread.sleep(100); + + assertFalse(flashMap.isExpired()); + } + + @Test + public void compareTo() { + FlashMap flashMap1 = new FlashMap(); + FlashMap flashMap2 = new FlashMap(); + assertEquals(0, flashMap1.compareTo(flashMap2)); + + flashMap1.setExpectedUrlPath(null, "/path1"); + assertEquals(-1, flashMap1.compareTo(flashMap2)); + assertEquals(1, flashMap2.compareTo(flashMap1)); + + flashMap2.setExpectedUrlPath(null, "/path2"); + assertEquals(0, flashMap1.compareTo(flashMap2)); + + flashMap1.setExpectedRequestParameters(new ModelMap("id", "1")); + assertEquals(-1, flashMap1.compareTo(flashMap2)); + assertEquals(1, flashMap2.compareTo(flashMap1)); + + flashMap2.setExpectedRequestParameters(new ModelMap("id", "2")); + assertEquals(0, flashMap1.compareTo(flashMap2)); + } + +} 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 8cb68d4da16..e2ec8dbd7a0 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 @@ -1,5 +1,5 @@ /* - * Copyright 2002-2010 the original author or authors. + * Copyright 2002-2011 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -32,6 +32,7 @@ import org.springframework.beans.TestBean; import org.springframework.http.HttpStatus; import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.mock.web.MockHttpServletResponse; +import org.springframework.ui.ModelMap; import org.springframework.web.servlet.FlashMap; import org.springframework.web.servlet.FlashMapManager; import org.springframework.web.servlet.View; @@ -96,30 +97,20 @@ public class RedirectViewTests { @Test public void flashMap() throws Exception { RedirectView rv = new RedirectView(); - rv.setUrl("http://url.somewhere.com"); + rv.setUrl("http://url.somewhere.com/path"); rv.setHttp10Compatible(false); MockHttpServletRequest request = new MockHttpServletRequest(); MockHttpServletResponse response = new MockHttpServletResponse(); - FlashMap flashMap = new FlashMap("key", "_flashKey"); - flashMap.put("name", "value"); + FlashMap flashMap = new FlashMap(); + flashMap.put("successMessage", "yay!"); request.setAttribute(FlashMapManager.CURRENT_FLASH_MAP_ATTRIBUTE, flashMap); - rv.render(new HashMap(), request, response); + rv.render(new ModelMap("id", "1"), request, response); assertEquals(303, response.getStatus()); - assertEquals("http://url.somewhere.com?_flashKey=key", response.getHeader("Location")); - } - - @Test - public void emptyFlashMap() throws Exception { - RedirectView rv = new RedirectView(); - rv.setUrl("http://url.somewhere.com"); - rv.setHttp10Compatible(false); - MockHttpServletRequest request = new MockHttpServletRequest(); - MockHttpServletResponse response = new MockHttpServletResponse(); - FlashMap flashMap = new FlashMap("key", "_flashKey"); - request.setAttribute(FlashMapManager.CURRENT_FLASH_MAP_ATTRIBUTE, flashMap); - rv.render(new HashMap(), request, response); - assertEquals(303, response.getStatus()); - assertEquals("http://url.somewhere.com", response.getHeader("Location")); + assertEquals("http://url.somewhere.com/path?id=1", response.getHeader("Location")); + + MockHttpServletRequest nextRequest = new MockHttpServletRequest("GET", "/path"); + nextRequest.addParameter("id", "1"); + assertTrue(flashMap.matches(nextRequest)); } @Test