From 3d1fa4f6b60f4cbef66df980071ec5747c9183c8 Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Tue, 31 Jan 2012 22:10:55 -0500 Subject: [PATCH] SPR-9060 Revise HTTP Session based FlashMapManager implementation. The "default" FlashMapManager implementation added in 3.1 was invoked after the redirect, which is too late in cases where the HTTP session has not been yet been created since as the response is committed. This change corrects the issue and makes other improvements to the FlashMapManager implementation such as extracting a base AbstractFlashMapManager class and making it easier for other implementations to be added (for example cookie-based). --- .../resources/changelog.txt | 1 + .../web/servlet/DispatcherServlet.java | 35 +- .../web/servlet/DispatcherServlet.properties | 2 +- .../web/servlet/FlashMapManager.java | 63 ++-- .../support/AbstractFlashMapManager.java | 221 ++++++++++++ .../support/DefaultFlashMapManager.java | 266 --------------- .../servlet/support/RequestContextUtils.java | 15 +- .../support/SessionFlashMapManager.java | 55 +++ .../web/servlet/view/RedirectView.java | 8 +- .../ParameterizableViewControllerTests.java | 8 +- .../mvc/UrlFilenameViewControllerTests.java | 6 +- .../RequestMappingHandlerAdapterTests.java | 6 +- .../support/AbstractFlashMapManagerTests.java | 297 ++++++++++++++++ .../support/DefaultFlashMapManagerTests.java | 322 ------------------ .../web/servlet/view/RedirectViewTests.java | 35 +- .../view/RedirectViewUriTemplateTests.java | 7 +- 16 files changed, 684 insertions(+), 663 deletions(-) create mode 100644 org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/support/AbstractFlashMapManager.java delete mode 100644 org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/support/DefaultFlashMapManager.java create mode 100644 org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/support/SessionFlashMapManager.java create mode 100644 org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/support/AbstractFlashMapManagerTests.java delete mode 100644 org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/support/DefaultFlashMapManagerTests.java diff --git a/build-spring-framework/resources/changelog.txt b/build-spring-framework/resources/changelog.txt index c61372a70e7..a5ddc80ba75 100644 --- a/build-spring-framework/resources/changelog.txt +++ b/build-spring-framework/resources/changelog.txt @@ -29,6 +29,7 @@ Changes in version 3.1.1 (2012-02-06) * make flash attributes available in the model of ParameterizableViewController and UrlFilenameViewController * add property to RedirectView to disable expanding URI variables in redirect URL * fix request mapping bug involving direct vs pattern path matches with HTTP methods +* revise the FlashMapManager contract and implemenation to address a flaw in its design Changes in version 3.1 GA (2011-12-12) -------------------------------------- diff --git a/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/DispatcherServlet.java b/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/DispatcherServlet.java index da5bb9b7121..7bac3dfb1ba 100644 --- a/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/DispatcherServlet.java +++ b/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/DispatcherServlet.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2010 the original author or authors. + * Copyright 2002-2012 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. @@ -208,7 +208,27 @@ public class DispatcherServlet extends FrameworkServlet { * @see org.springframework.web.servlet.support.RequestContextUtils#getThemeSource */ public static final String THEME_SOURCE_ATTRIBUTE = DispatcherServlet.class.getName() + ".THEME_SOURCE"; - + + /** + * Name of request attribute that holds a read-only {@code Map} + * with "input" flash attributes saved by a previous request, if any. + * @see org.springframework.web.servlet.support.RequestContextUtils#getInputFlashMap(HttpServletRequest) + */ + public static final String INPUT_FLASH_MAP_ATTRIBUTE = DispatcherServlet.class.getName() + ".INPUT_FLASH_MAP"; + + /** + * Name of request attribute that holds the "output" {@link FlashMap} with + * attributes to save for a subsequent request. + * @see org.springframework.web.servlet.support.RequestContextUtils#getOutputFlashMap(HttpServletRequest) + */ + public static final String OUTPUT_FLASH_MAP_ATTRIBUTE = DispatcherServlet.class.getName() + ".OUTPUT_FLASH_MAP"; + + /** + * Name of request attribute that holds the {@link FlashMapManager}. + * @see org.springframework.web.servlet.support.RequestContextUtils#getFlashMapManager(HttpServletRequest) + */ + public static final String FLASH_MAP_MANAGER_ATTRIBUTE = DispatcherServlet.class.getName() + ".FLASH_MAP_MANAGER"; + /** Log category to use when no mapped handler is found for a request. */ public static final String PAGE_NOT_FOUND_LOG_CATEGORY = "org.springframework.web.servlet.PageNotFound"; @@ -815,20 +835,23 @@ public class DispatcherServlet extends FrameworkServlet { } } - this.flashMapManager.requestStarted(request, response); - // Make framework objects available to handlers and view objects. request.setAttribute(WEB_APPLICATION_CONTEXT_ATTRIBUTE, getWebApplicationContext()); request.setAttribute(LOCALE_RESOLVER_ATTRIBUTE, this.localeResolver); request.setAttribute(THEME_RESOLVER_ATTRIBUTE, this.themeResolver); request.setAttribute(THEME_SOURCE_ATTRIBUTE, getThemeSource()); + request.setAttribute(OUTPUT_FLASH_MAP_ATTRIBUTE, new FlashMap()); + request.setAttribute(FLASH_MAP_MANAGER_ATTRIBUTE, this.flashMapManager); + + Map flashMap = this.flashMapManager.getFlashMapForRequest(request); + if (flashMap != null) { + request.setAttribute(INPUT_FLASH_MAP_ATTRIBUTE, flashMap); + } try { doDispatch(request, response); } finally { - this.flashMapManager.requestCompleted(request, response); - // Restore the original attribute snapshot, in case of an include. if (attributesSnapshot != null) { restoreAttributesAfterInclude(request, attributesSnapshot); diff --git a/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/DispatcherServlet.properties b/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/DispatcherServlet.properties index a3ec101afaf..c8e5ab298cf 100644 --- a/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/DispatcherServlet.properties +++ b/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/DispatcherServlet.properties @@ -21,4 +21,4 @@ org.springframework.web.servlet.RequestToViewNameTranslator=org.springframework. org.springframework.web.servlet.ViewResolver=org.springframework.web.servlet.view.InternalResourceViewResolver -org.springframework.web.servlet.FlashMapManager=org.springframework.web.servlet.support.DefaultFlashMapManager \ No newline at end of file +org.springframework.web.servlet.FlashMapManager=org.springframework.web.servlet.support.SessionFlashMapManager \ No newline at end of file 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 6d765ec8473..8440bd636c5 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 @@ -1,5 +1,5 @@ /* - * Copyright 2002-2011 the original author or authors. + * Copyright 2002-2012 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. @@ -16,22 +16,14 @@ package org.springframework.web.servlet; +import java.util.Map; + import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; /** - * A strategy interface for storing, retrieving, and managing {@code FlashMap} - * instances. See {@link FlashMap} for a general overview of flash attributes. - * - *

A FlashMapManager is invoked at the beginning and at the end of requests. - * For each request it retrieves an "input" FlashMap with attributes passed - * from a previous request (if any) and creates an "output" FlashMap with - * attributes to pass to a subsequent request. "Input" and "output" FlashMap - * instances are exposed as request attributes and are accessible via methods - * in {@code org.springframework.web.servlet.support.RequestContextUtils}. - * - *

Annotated controllers will usually not use this FlashMap directly. - * See {@code org.springframework.web.servlet.mvc.support.RedirectAttributes}. + * A strategy interface for retrieving and saving FlashMap instances. + * See {@link FlashMap} for a general overview of flash attributes. * * @author Rossen Stoyanchev * @since 3.1 @@ -41,42 +33,27 @@ import javax.servlet.http.HttpServletResponse; public interface FlashMapManager { /** - * Name of request attribute that holds a read-only - * {@code Map} with "input" flash attributes if any. - * @see org.springframework.web.servlet.support.RequestContextUtils#getInputFlashMap(HttpServletRequest) - */ - String INPUT_FLASH_MAP_ATTRIBUTE = FlashMapManager.class.getName() + ".INPUT_FLASH_MAP"; - - /** - * Name of request attribute that holds the "output" {@link FlashMap} with - * attributes to save for a subsequent request. - * @see org.springframework.web.servlet.support.RequestContextUtils#getOutputFlashMap(HttpServletRequest) - */ - String OUTPUT_FLASH_MAP_ATTRIBUTE = FlashMapManager.class.getName() + ".OUTPUT_FLASH_MAP"; - - /** - * Perform the following tasks unless the {@link #OUTPUT_FLASH_MAP_ATTRIBUTE} - * request attribute exists: - *

    - *
  1. Find the "input" FlashMap, expose it under the request attribute - * {@link #INPUT_FLASH_MAP_ATTRIBUTE}, and remove it from underlying storage. - *
  2. Create the "output" FlashMap and expose it under the request - * attribute {@link #OUTPUT_FLASH_MAP_ATTRIBUTE}. - *
  3. Clean expired FlashMap instances. - *
+ * Get a Map with flash attributes saved by a previous request. + * See {@link FlashMap} for details on how FlashMap instances + * identifies the target requests they're saved for. + * If found, the Map is removed from the underlying storage. * @param request the current request - * @param response the current response + * @return a read-only Map with flash attributes or {@code null} */ - void requestStarted(HttpServletRequest request, HttpServletResponse response); + Map getFlashMapForRequest(HttpServletRequest request); /** - * Start the expiration period of the "output" FlashMap save it in the - * underlying storage. - *

The "output" FlashMap should not be saved if it is empty or if it was - * not created by the current FlashMapManager instance. + * Save the given FlashMap, in some underlying storage, mark the beginning + * of its expiration period, and remove other expired FlashMap instances. + * The method has no impact if the FlashMap is empty and there are no + * expired FlashMap instances to be removed. + *

Note: Invoke this method prior to a redirect in order + * to allow saving the FlashMap in the HTTP session or perhaps in a response + * cookie before the response is committed. + * @param flashMap the FlashMap to save * @param request the current request * @param response the current response */ - void requestCompleted(HttpServletRequest request, HttpServletResponse response); + void save(FlashMap flashMap, HttpServletRequest request, HttpServletResponse response); } diff --git a/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/support/AbstractFlashMapManager.java b/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/support/AbstractFlashMapManager.java new file mode 100644 index 00000000000..0cfc3b2037d --- /dev/null +++ b/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/support/AbstractFlashMapManager.java @@ -0,0 +1,221 @@ +/* + * Copyright 2002-2012 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 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; +import javax.servlet.http.HttpServletResponse; + +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; +import org.springframework.util.Assert; +import org.springframework.util.CollectionUtils; +import org.springframework.util.MultiValueMap; +import org.springframework.util.ObjectUtils; +import org.springframework.util.StringUtils; +import org.springframework.web.servlet.FlashMap; +import org.springframework.web.servlet.FlashMapManager; +import org.springframework.web.util.UrlPathHelper; + +/** + * A base class for {@link FlashMapManager} implementations. + * + * @author Rossen Stoyanchev + * @since 3.1.1 + */ +public abstract class AbstractFlashMapManager implements FlashMapManager { + + protected final Log logger = LogFactory.getLog(getClass()); + + private int flashMapTimeout = 180; + + private UrlPathHelper urlPathHelper = new UrlPathHelper(); + + /** + * Set the amount of time in seconds after a {@link FlashMap} is saved + * (at request completion) and before it expires. + *

The default value is 180 seconds. + */ + public void setFlashMapTimeout(int flashMapTimeout) { + this.flashMapTimeout = flashMapTimeout; + } + + /** + * Return the amount of time in seconds before a FlashMap expires. + */ + public int getFlashMapTimeout() { + return this.flashMapTimeout; + } + + /** + * Set the UrlPathHelper to use to match FlashMap instances to requests. + */ + public void setUrlPathHelper(UrlPathHelper urlPathHelper) { + Assert.notNull(urlPathHelper, "UrlPathHelper must not be null"); + this.urlPathHelper = urlPathHelper; + } + + /** + * Return the UrlPathHelper implementation to use. + */ + public UrlPathHelper getUrlPathHelper() { + return this.urlPathHelper; + } + + /** + * {@inheritDoc} + *

Does not cause an HTTP session to be created. + */ + public final Map getFlashMapForRequest(HttpServletRequest request) { + List flashMaps = retrieveFlashMaps(request); + if (CollectionUtils.isEmpty(flashMaps)) { + return null; + } + if (logger.isDebugEnabled()) { + logger.debug("Retrieved FlashMap(s): " + flashMaps); + } + List result = new ArrayList(); + for (FlashMap flashMap : flashMaps) { + if (isFlashMapForRequest(flashMap, request)) { + result.add(flashMap); + } + } + if (!result.isEmpty()) { + Collections.sort(result); + if (logger.isDebugEnabled()) { + logger.debug("Found matching FlashMap(s): " + result); + } + FlashMap match = result.remove(0); + flashMaps.remove(match); + return Collections.unmodifiableMap(match); + } + return null; + } + + /** + * Retrieve saved FlashMap instances from underlying storage. + * @param request the current request + * @return a List with FlashMap instances or {@code null} + */ + protected abstract List retrieveFlashMaps(HttpServletRequest request); + + /** + * Whether the given FlashMap matches the current request. + * The default implementation uses the target request path and query params + * saved in the FlashMap. + */ + protected boolean isFlashMapForRequest(FlashMap flashMap, HttpServletRequest request) { + if (flashMap.getTargetRequestPath() != null) { + String requestUri = this.urlPathHelper.getOriginatingRequestUri(request); + if (!requestUri.equals(flashMap.getTargetRequestPath()) + && !requestUri.equals(flashMap.getTargetRequestPath() + "/")) { + return false; + } + } + MultiValueMap targetParams = flashMap.getTargetRequestParams(); + for (String paramName : targetParams.keySet()) { + for (String targetValue : targetParams.get(paramName)) { + if (!ObjectUtils.containsElement(request.getParameterValues(paramName), targetValue)) { + return false; + } + } + } + return true; + } + + /** + * {@inheritDoc} + *

The FlashMap, if not empty, is saved to the HTTP session. + */ + public final void save(FlashMap flashMap, HttpServletRequest request, HttpServletResponse response) { + Assert.notNull(flashMap, "FlashMap must not be null"); + + List flashMaps = retrieveFlashMaps(request); + if (flashMap.isEmpty() && (flashMaps == null)) { + return; + } + synchronized (this) { + boolean update = false; + flashMaps = retrieveFlashMaps(request); + if (!CollectionUtils.isEmpty(flashMaps)) { + update = removeExpired(flashMaps); + } + if (!flashMap.isEmpty()) { + String path = decodeAndNormalizePath(flashMap.getTargetRequestPath(), request); + flashMap.setTargetRequestPath(path); + flashMap.startExpirationPeriod(this.flashMapTimeout); + if (logger.isDebugEnabled()) { + logger.debug("Saving FlashMap=" + flashMap); + } + flashMaps = (flashMaps == null) ? new CopyOnWriteArrayList() : flashMaps; + flashMaps.add(flashMap); + update = true; + } + if (update) { + updateFlashMaps(flashMaps, request, response); + } + } + } + + private String decodeAndNormalizePath(String path, HttpServletRequest request) { + if (path != null) { + path = this.urlPathHelper.decodeRequestString(request, path); + if (path.charAt(0) != '/') { + String requestUri = this.urlPathHelper.getRequestUri(request); + path = requestUri.substring(0, requestUri.lastIndexOf('/') + 1) + path; + path = StringUtils.cleanPath(path); + } + } + return path; + } + + /** + * Update the FlashMap instances in some underlying storage. + * @param flashMaps a non-empty list of FlashMap instances to save + * @param request the current request + * @param response the current response + */ + protected abstract void updateFlashMaps(List flashMaps, HttpServletRequest request, + HttpServletResponse response); + + /** + * Remove expired FlashMap instances from the given List. + */ + protected boolean removeExpired(List flashMaps) { + List expired = new ArrayList(); + for (FlashMap flashMap : flashMaps) { + if (flashMap.isExpired()) { + if (logger.isTraceEnabled()) { + logger.trace("Removing expired FlashMap: " + flashMap); + } + expired.add(flashMap); + } + } + if (expired.isEmpty()) { + return false; + } + else { + return flashMaps.removeAll(expired); + } + } + +} 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 deleted file mode 100644 index f4b1a04a5f1..00000000000 --- a/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/support/DefaultFlashMapManager.java +++ /dev/null @@ -1,266 +0,0 @@ -/* - * 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 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.HttpServletResponse; -import javax.servlet.http.HttpSession; - -import org.apache.commons.logging.Log; -import org.apache.commons.logging.LogFactory; -import org.springframework.util.Assert; -import org.springframework.util.CollectionUtils; -import org.springframework.util.MultiValueMap; -import org.springframework.util.ObjectUtils; -import org.springframework.util.StringUtils; -import org.springframework.web.servlet.FlashMap; -import org.springframework.web.servlet.FlashMapManager; -import org.springframework.web.util.UrlPathHelper; - -/** - * A default {@link FlashMapManager} implementation that stores {@link FlashMap} - * instances in the HTTP session. - * - * @author Rossen Stoyanchev - * @since 3.1 - */ -public class DefaultFlashMapManager implements FlashMapManager { - - private static final String FLASH_MAPS_SESSION_ATTRIBUTE = DefaultFlashMapManager.class.getName() + ".FLASH_MAPS"; - - private static final Log logger = LogFactory.getLog(DefaultFlashMapManager.class); - - private int flashMapTimeout = 180; - - private UrlPathHelper urlPathHelper = new UrlPathHelper(); - - /** - * Set the amount of time in seconds after a {@link FlashMap} is saved - * (at request completion) and before it expires. - *

The default value is 180 seconds. - */ - public void setFlashMapTimeout(int flashMapTimeout) { - this.flashMapTimeout = flashMapTimeout; - } - - /** - * Return the amount of time in seconds before a FlashMap expires. - */ - public int getFlashMapTimeout() { - return flashMapTimeout; - } - - /** - * Set the UrlPathHelper to use to obtain the request URI. - */ - public void setUrlPathHelper(UrlPathHelper urlPathHelper) { - Assert.notNull(urlPathHelper, "UrlPathHelper must not be null"); - this.urlPathHelper = urlPathHelper; - } - - /** - * Return the UrlPathHelper implementation for the request URI. - */ - public UrlPathHelper getUrlPathHelper() { - return urlPathHelper; - } - - /** - * {@inheritDoc} - *

An HTTP session is never created by this method. - */ - public final void requestStarted(HttpServletRequest request, HttpServletResponse response) { - if (request.getAttribute(OUTPUT_FLASH_MAP_ATTRIBUTE) != null) { - return; - } - - FlashMap inputFlashMap = lookupFlashMap(request); - if (inputFlashMap != null) { - request.setAttribute(INPUT_FLASH_MAP_ATTRIBUTE, Collections.unmodifiableMap(inputFlashMap)); - } - - FlashMap outputFlashMap = new FlashMap(this.hashCode()); - request.setAttribute(OUTPUT_FLASH_MAP_ATTRIBUTE, outputFlashMap); - - removeExpiredFlashMaps(request); - } - - /** - * Find the "input" FlashMap for the current request target by matching it - * to the target request information of all stored FlashMap instances. - * @return a FlashMap instance or {@code null} - */ - private FlashMap lookupFlashMap(HttpServletRequest request) { - List allFlashMaps = retrieveFlashMaps(request, false); - if (CollectionUtils.isEmpty(allFlashMaps)) { - return null; - } - if (logger.isDebugEnabled()) { - logger.debug("Retrieved FlashMap(s): " + allFlashMaps); - } - List result = new ArrayList(); - for (FlashMap flashMap : allFlashMaps) { - if (isFlashMapForRequest(flashMap, request)) { - result.add(flashMap); - } - } - if (!result.isEmpty()) { - Collections.sort(result); - if (logger.isDebugEnabled()) { - logger.debug("Found matching FlashMap(s): " + result); - } - FlashMap match = result.remove(0); - allFlashMaps.remove(match); - return match; - } - return null; - } - - /** - * Whether the given FlashMap matches the current request. - * The default implementation uses the target request path and query params - * saved in the FlashMap. - */ - protected boolean isFlashMapForRequest(FlashMap flashMap, HttpServletRequest request) { - if (flashMap.getTargetRequestPath() != null) { - String requestUri = this.urlPathHelper.getOriginatingRequestUri(request); - if (!requestUri.equals(flashMap.getTargetRequestPath()) - && !requestUri.equals(flashMap.getTargetRequestPath() + "/")) { - return false; - } - } - MultiValueMap targetParams = flashMap.getTargetRequestParams(); - for (String paramName : targetParams.keySet()) { - for (String targetValue : targetParams.get(paramName)) { - if (!ObjectUtils.containsElement(request.getParameterValues(paramName), targetValue)) { - return false; - } - } - } - return true; - } - - /** - * Retrieve all FlashMap instances from the current HTTP session. - * If {@code allowCreate} is "true" and no flash maps exist yet, a new list - * is created and stored as a session attribute. - * @param request the current request - * @param allowCreate whether to create the session if necessary - * @return a List to add FlashMap instances to or {@code null} - * assuming {@code allowCreate} is "false". - */ - @SuppressWarnings("unchecked") - protected List retrieveFlashMaps(HttpServletRequest request, boolean allowCreate) { - HttpSession session = request.getSession(allowCreate); - if (session == null) { - return null; - } - List allFlashMaps = (List) session.getAttribute(FLASH_MAPS_SESSION_ATTRIBUTE); - if (allFlashMaps == null && allowCreate) { - synchronized (this) { - allFlashMaps = (List) session.getAttribute(FLASH_MAPS_SESSION_ATTRIBUTE); - if (allFlashMaps == null) { - allFlashMaps = new CopyOnWriteArrayList(); - session.setAttribute(FLASH_MAPS_SESSION_ATTRIBUTE, allFlashMaps); - } - } - } - return allFlashMaps; - } - - /** - * Check and remove expired FlashMaps instances. - */ - protected void removeExpiredFlashMaps(HttpServletRequest request) { - List allMaps = retrieveFlashMaps(request, false); - if (CollectionUtils.isEmpty(allMaps)) { - return; - } - List expiredMaps = new ArrayList(); - for (FlashMap flashMap : allMaps) { - if (flashMap.isExpired()) { - if (logger.isDebugEnabled()) { - logger.debug("Removing expired FlashMap: " + flashMap); - } - expiredMaps.add(flashMap); - } - } - if (!expiredMaps.isEmpty()) { - allMaps.removeAll(expiredMaps); - } - } - - /** - * {@inheritDoc} - *

An HTTP session is never created if the "output" FlashMap is empty. - */ - 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"); - } - if (!flashMap.isEmpty() && flashMap.isCreatedBy(this.hashCode())) { - if (logger.isDebugEnabled()) { - logger.debug("Saving FlashMap=" + flashMap); - } - onSaveFlashMap(flashMap, request, response); - saveFlashMap(flashMap, request, response); - } - } - - /** - * Update a FlashMap before it is stored in the underlying storage. - *

The default implementation starts the expiration period and ensures the - * target request path is decoded and normalized if it is relative. - * @param flashMap the flash map to be saved - * @param request the current request - * @param response the current response - */ - protected void onSaveFlashMap(FlashMap flashMap, HttpServletRequest request, HttpServletResponse response) { - String targetPath = flashMap.getTargetRequestPath(); - flashMap.setTargetRequestPath(decodeAndNormalizePath(targetPath, request)); - flashMap.startExpirationPeriod(this.flashMapTimeout); - } - - /** - * 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); - if (path.charAt(0) != '/') { - String requestUri = this.urlPathHelper.getRequestUri(request); - path = requestUri.substring(0, requestUri.lastIndexOf('/') + 1) + path; - path = StringUtils.cleanPath(path); - } - } - return path; - } - -} 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 a615777b88b..8c284e37edf 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 @@ -1,5 +1,5 @@ /* - * Copyright 2002-2009 the original author or authors. + * Copyright 2002-2012 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. @@ -165,7 +165,7 @@ public abstract class RequestContextUtils { */ @SuppressWarnings("unchecked") public static Map getInputFlashMap(HttpServletRequest request) { - return (Map) request.getAttribute(FlashMapManager.INPUT_FLASH_MAP_ATTRIBUTE); + return (Map) request.getAttribute(DispatcherServlet.INPUT_FLASH_MAP_ATTRIBUTE); } /** @@ -175,7 +175,16 @@ public abstract class RequestContextUtils { * @see FlashMap */ public static FlashMap getOutputFlashMap(HttpServletRequest request) { - return (FlashMap) request.getAttribute(FlashMapManager.OUTPUT_FLASH_MAP_ATTRIBUTE); + return (FlashMap) request.getAttribute(DispatcherServlet.OUTPUT_FLASH_MAP_ATTRIBUTE); + } + + /** + * Return the FlashMapManager instance to save flash attributes with + * before a redirect. + * @param request the current request + */ + public static FlashMapManager getFlashMapManager(HttpServletRequest request) { + return (FlashMapManager) request.getAttribute(DispatcherServlet.FLASH_MAP_MANAGER_ATTRIBUTE); } } diff --git a/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/support/SessionFlashMapManager.java b/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/support/SessionFlashMapManager.java new file mode 100644 index 00000000000..ef6183e952f --- /dev/null +++ b/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/support/SessionFlashMapManager.java @@ -0,0 +1,55 @@ +/* + * Copyright 2002-2012 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 java.util.List; + +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import javax.servlet.http.HttpSession; + +import org.springframework.web.servlet.FlashMap; + +/** + * Stores {@link FlashMap} instances in the HTTP session. + * + * @author Rossen Stoyanchev + * @since 3.1.1 + */ +public class SessionFlashMapManager extends AbstractFlashMapManager{ + + private static final String FLASH_MAPS_SESSION_ATTRIBUTE = SessionFlashMapManager.class.getName() + ".FLASH_MAPS"; + + /** + * Retrieve saved FlashMap instances from the HTTP session. + * @param request the current request + * @return a List with FlashMap instances or {@code null} + */ + @SuppressWarnings("unchecked") + protected List retrieveFlashMaps(HttpServletRequest request) { + HttpSession session = request.getSession(false); + return (session != null) ? (List) session.getAttribute(FLASH_MAPS_SESSION_ATTRIBUTE) : null; + } + + /** + * Save the given FlashMap instances in the HTTP session. + */ + protected void updateFlashMaps(List flashMaps, HttpServletRequest request, HttpServletResponse response) { + request.getSession().setAttribute(FLASH_MAPS_SESSION_ATTRIBUTE, flashMaps); + } + +} 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 0e100ee7f34..0f931d776bb 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 @@ -1,5 +1,5 @@ /* - * Copyright 2002-2011 the original author or authors. + * Copyright 2002-2012 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. @@ -41,6 +41,7 @@ import org.springframework.util.StringUtils; import org.springframework.web.context.ContextLoader; import org.springframework.web.context.WebApplicationContext; import org.springframework.web.servlet.FlashMap; +import org.springframework.web.servlet.FlashMapManager; import org.springframework.web.servlet.HandlerMapping; import org.springframework.web.servlet.SmartView; import org.springframework.web.servlet.View; @@ -273,7 +274,10 @@ public class RedirectView extends AbstractUrlBasedView implements SmartView { flashMap.setTargetRequestPath(uriComponents.getPath()); flashMap.addTargetRequestParams(uriComponents.getQueryParams()); } - + + FlashMapManager flashMapManager = RequestContextUtils.getFlashMapManager(request); + flashMapManager.save(flashMap, request, response); + sendRedirect(request, response, targetUrl.toString(), this.http10Compatible); } diff --git a/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/mvc/ParameterizableViewControllerTests.java b/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/mvc/ParameterizableViewControllerTests.java index 77b90fb0b42..51792e20e65 100644 --- a/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/mvc/ParameterizableViewControllerTests.java +++ b/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/mvc/ParameterizableViewControllerTests.java @@ -16,14 +16,16 @@ package org.springframework.web.servlet.mvc; -import static org.junit.Assert.*; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; import org.junit.Before; import org.junit.Test; import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.mock.web.MockHttpServletResponse; import org.springframework.ui.ModelMap; -import org.springframework.web.servlet.FlashMapManager; +import org.springframework.web.servlet.DispatcherServlet; import org.springframework.web.servlet.ModelAndView; /** @@ -62,7 +64,7 @@ public class ParameterizableViewControllerTests { @Test public void handleRequestWithFlashAttributes() throws Exception { - this.request.setAttribute(FlashMapManager.INPUT_FLASH_MAP_ATTRIBUTE, new ModelMap("name", "value")); + this.request.setAttribute(DispatcherServlet.INPUT_FLASH_MAP_ATTRIBUTE, new ModelMap("name", "value")); ModelAndView mav = this.controller.handleRequest(this.request, new MockHttpServletResponse()); assertEquals(1, mav.getModel().size()); assertEquals("value", mav.getModel().get("name")); diff --git a/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/mvc/UrlFilenameViewControllerTests.java b/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/mvc/UrlFilenameViewControllerTests.java index 58258067f5a..2ac2f6977d0 100644 --- a/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/mvc/UrlFilenameViewControllerTests.java +++ b/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/mvc/UrlFilenameViewControllerTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2007 the original author or authors. + * Copyright 2002-2012 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. @@ -23,7 +23,7 @@ import org.springframework.mock.web.MockHttpServletResponse; import org.springframework.ui.ModelMap; import org.springframework.util.AntPathMatcher; import org.springframework.util.PathMatcher; -import org.springframework.web.servlet.FlashMapManager; +import org.springframework.web.servlet.DispatcherServlet; import org.springframework.web.servlet.HandlerMapping; import org.springframework.web.servlet.ModelAndView; @@ -155,7 +155,7 @@ public class UrlFilenameViewControllerTests extends TestCase { public void testWithFlashAttributes() throws Exception { UrlFilenameViewController ctrl = new UrlFilenameViewController(); MockHttpServletRequest request = new MockHttpServletRequest("GET", "/index"); - request.setAttribute(FlashMapManager.INPUT_FLASH_MAP_ATTRIBUTE, new ModelMap("name", "value")); + request.setAttribute(DispatcherServlet.INPUT_FLASH_MAP_ATTRIBUTE, new ModelMap("name", "value")); MockHttpServletResponse response = new MockHttpServletResponse(); ModelAndView mv = ctrl.handleRequest(request, response); assertEquals("index", mv.getViewName()); diff --git a/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerAdapterTests.java b/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerAdapterTests.java index da2df3d1506..6a0808e8d8f 100644 --- a/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerAdapterTests.java +++ b/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerAdapterTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2011 the original author or authors. + * Copyright 2002-2012 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. @@ -35,8 +35,8 @@ import org.springframework.web.method.annotation.ModelMethodProcessor; import org.springframework.web.method.support.HandlerMethodArgumentResolver; import org.springframework.web.method.support.HandlerMethodReturnValueHandler; import org.springframework.web.method.support.InvocableHandlerMethod; +import org.springframework.web.servlet.DispatcherServlet; import org.springframework.web.servlet.FlashMap; -import org.springframework.web.servlet.FlashMapManager; import org.springframework.web.servlet.ModelAndView; /** @@ -111,7 +111,7 @@ public class RequestMappingHandlerAdapterTests { this.handlerAdapter.setIgnoreDefaultModelOnRedirect(true); this.handlerAdapter.afterPropertiesSet(); - this.request.setAttribute(FlashMapManager.OUTPUT_FLASH_MAP_ATTRIBUTE, new FlashMap()); + this.request.setAttribute(DispatcherServlet.OUTPUT_FLASH_MAP_ATTRIBUTE, new FlashMap()); HandlerMethod handlerMethod = handlerMethod(new RedirectAttributeController(), "handle", Model.class); ModelAndView mav = this.handlerAdapter.handle(request, response, handlerMethod); diff --git a/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/support/AbstractFlashMapManagerTests.java b/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/support/AbstractFlashMapManagerTests.java new file mode 100644 index 00000000000..ccef7060b9b --- /dev/null +++ b/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/support/AbstractFlashMapManagerTests.java @@ -0,0 +1,297 @@ +/* + * Copyright 2002-2012 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.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertSame; +import static org.junit.Assert.assertTrue; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; +import java.util.Map; +import java.util.concurrent.CopyOnWriteArrayList; + +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; + +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; + +/** + * Test fixture for testing {@link AbstractFlashMapManager} methods. + * + * @author Rossen Stoyanchev + */ +public class AbstractFlashMapManagerTests { + + private TestFlashMapManager flashMapManager; + + private MockHttpServletRequest request; + + private MockHttpServletResponse response; + + @Before + public void setup() { + this.flashMapManager = new TestFlashMapManager(); + this.request = new MockHttpServletRequest(); + this.response = new MockHttpServletResponse(); + } + + @Test + public void getFlashMapForRequestByPath() { + FlashMap flashMap = new FlashMap(); + flashMap.put("key", "value"); + flashMap.setTargetRequestPath("/path"); + + this.flashMapManager.setFlashMaps(flashMap); + + this.request.setRequestURI("/path"); + Map inputFlashMap = this.flashMapManager.getFlashMapForRequest(this.request); + + assertEquals(flashMap, inputFlashMap); + assertEquals("Input FlashMap should have been removed", 0, this.flashMapManager.getFlashMaps().size()); + } + + // SPR-8779 + + @Test + public void getFlashMapForRequestByOriginatingPath() { + FlashMap flashMap = new FlashMap(); + flashMap.put("key", "value"); + flashMap.setTargetRequestPath("/accounts"); + + this.flashMapManager.setFlashMaps(flashMap); + + this.request.setAttribute(WebUtils.FORWARD_REQUEST_URI_ATTRIBUTE, "/accounts"); + this.request.setRequestURI("/mvc/accounts"); + Map inputFlashMap = this.flashMapManager.getFlashMapForRequest(this.request); + + assertEquals(flashMap, inputFlashMap); + assertEquals("Input FlashMap should have been removed", 0, this.flashMapManager.getFlashMaps().size()); + } + + @Test + public void getFlashMapForRequestByPathWithTrailingSlash() { + FlashMap flashMap = new FlashMap(); + flashMap.put("key", "value"); + flashMap.setTargetRequestPath("/path"); + + this.flashMapManager.setFlashMaps(flashMap); + + this.request.setRequestURI("/path/"); + Map inputFlashMap = this.flashMapManager.getFlashMapForRequest(this.request); + + assertEquals(flashMap, inputFlashMap); + assertEquals("Input FlashMap should have been removed", 0, this.flashMapManager.getFlashMaps().size()); + } + + @Test + public void getFlashMapForRequestWithParams() { + FlashMap flashMap = new FlashMap(); + flashMap.put("key", "value"); + flashMap.addTargetRequestParam("number", "one"); + + this.flashMapManager.setFlashMaps(flashMap); + + this.request.setParameter("number", (String) null); + Map inputFlashMap = this.flashMapManager.getFlashMapForRequest(this.request); + + assertNull(inputFlashMap); + assertEquals("FlashMap should not have been removed", 1, this.flashMapManager.getFlashMaps().size()); + + this.request.setParameter("number", "two"); + inputFlashMap = this.flashMapManager.getFlashMapForRequest(this.request); + + assertNull(inputFlashMap); + assertEquals("FlashMap should not have been removed", 1, this.flashMapManager.getFlashMaps().size()); + + this.request.setParameter("number", "one"); + inputFlashMap = this.flashMapManager.getFlashMapForRequest(this.request); + + assertEquals(flashMap, inputFlashMap); + assertEquals("Input FlashMap should have been removed", 0, this.flashMapManager.getFlashMaps().size()); + } + + // SPR-8798 + + @Test + public void getFlashMapForRequestWithMultiValueParam() { + FlashMap flashMap = new FlashMap(); + flashMap.put("name", "value"); + flashMap.addTargetRequestParam("id", "1"); + flashMap.addTargetRequestParam("id", "2"); + + this.flashMapManager.setFlashMaps(flashMap); + + this.request.setParameter("id", "1"); + Map inputFlashMap = this.flashMapManager.getFlashMapForRequest(this.request); + + assertNull(inputFlashMap); + assertEquals("FlashMap should not have been removed", 1, this.flashMapManager.getFlashMaps().size()); + + this.request.addParameter("id", "2"); + inputFlashMap = this.flashMapManager.getFlashMapForRequest(this.request); + + assertEquals(flashMap, inputFlashMap); + assertEquals("Input FlashMap should have been removed", 0, this.flashMapManager.getFlashMaps().size()); + } + + @Test + public void getFlashMapForRequestSortOrder() { + FlashMap emptyFlashMap = new FlashMap(); + + FlashMap flashMapOne = new FlashMap(); + flashMapOne.put("key1", "value1"); + flashMapOne.setTargetRequestPath("/one"); + + FlashMap flashMapTwo = new FlashMap(); + flashMapTwo.put("key1", "value1"); + flashMapTwo.put("key2", "value2"); + flashMapTwo.setTargetRequestPath("/one/two"); + + this.flashMapManager.setFlashMaps(emptyFlashMap, flashMapOne, flashMapTwo); + + this.request.setRequestURI("/one/two"); + Map inputFlashMap = this.flashMapManager.getFlashMapForRequest(this.request); + + assertEquals(flashMapTwo, inputFlashMap); + } + + @Test + public void saveFlashMapEmpty() throws InterruptedException { + FlashMap flashMap = new FlashMap(); + + this.flashMapManager.save(flashMap, this.request, this.response); + List allMaps = this.flashMapManager.getFlashMaps(); + + assertNull(allMaps); + } + + @Test + public void saveFlashMap() throws InterruptedException { + FlashMap flashMap = new FlashMap(); + flashMap.put("name", "value"); + + this.flashMapManager.setFlashMapTimeout(-1); // expire immediately so we can check expiration started + this.flashMapManager.save(flashMap, this.request, this.response); + List allMaps = this.flashMapManager.getFlashMaps(); + + assertNotNull(allMaps); + assertSame(flashMap, allMaps.get(0)); + assertTrue(flashMap.isExpired()); + } + + @Test + public void saveFlashMapDecodeTargetPath() throws InterruptedException { + FlashMap flashMap = new FlashMap(); + flashMap.put("key", "value"); + + flashMap.setTargetRequestPath("/once%20upon%20a%20time"); + this.flashMapManager.save(flashMap, this.request, this.response); + + assertEquals("/once upon a time", flashMap.getTargetRequestPath()); + } + + @Test + public void saveFlashMapNormalizeTargetPath() throws InterruptedException { + FlashMap flashMap = new FlashMap(); + flashMap.put("key", "value"); + + flashMap.setTargetRequestPath("."); + this.request.setRequestURI("/once/upon/a/time"); + this.flashMapManager.save(flashMap, this.request, this.response); + + assertEquals("/once/upon/a", flashMap.getTargetRequestPath()); + + flashMap.setTargetRequestPath("./"); + this.request.setRequestURI("/once/upon/a/time"); + this.flashMapManager.save(flashMap, this.request, this.response); + + assertEquals("/once/upon/a/", flashMap.getTargetRequestPath()); + + flashMap.setTargetRequestPath(".."); + this.request.setRequestURI("/once/upon/a/time"); + this.flashMapManager.save(flashMap, this.request, this.response); + + assertEquals("/once/upon", flashMap.getTargetRequestPath()); + + flashMap.setTargetRequestPath("../"); + this.request.setRequestURI("/once/upon/a/time"); + this.flashMapManager.save(flashMap, this.request, this.response); + + assertEquals("/once/upon/", flashMap.getTargetRequestPath()); + + flashMap.setTargetRequestPath("../../only"); + this.request.setRequestURI("/once/upon/a/time"); + this.flashMapManager.save(flashMap, this.request, this.response); + + assertEquals("/once/only", flashMap.getTargetRequestPath()); + } + + @Test + public void saveFlashMapAndRemoveExpired() throws InterruptedException { + List flashMaps = new ArrayList(); + for (int i=0; i < 5; i++) { + FlashMap flashMap = new FlashMap(); + flashMap.startExpirationPeriod(-1); + flashMaps.add(flashMap); + } + this.flashMapManager.setFlashMaps(flashMaps); + this.flashMapManager.save(new FlashMap(), request, response); + + assertEquals("Expired instances should be removed even if the saved FlashMap is empty", + 0, this.flashMapManager.getFlashMaps().size()); + } + + + private static class TestFlashMapManager extends AbstractFlashMapManager { + + private List flashMaps; + + public List getFlashMaps() { + return this.flashMaps; + } + + public void setFlashMaps(FlashMap... flashMaps) { + setFlashMaps(Arrays.asList(flashMaps)); + } + + public void setFlashMaps(List flashMaps) { + this.flashMaps = new CopyOnWriteArrayList(flashMaps); + } + + @Override + protected List retrieveFlashMaps(HttpServletRequest request) { + return this.flashMaps; + } + + @Override + protected void updateFlashMaps(List flashMaps, HttpServletRequest request, + HttpServletResponse response) { + this.flashMaps = flashMaps; + } + } + +} 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 deleted file mode 100644 index eb7414633de..00000000000 --- a/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/support/DefaultFlashMapManagerTests.java +++ /dev/null @@ -1,322 +0,0 @@ -/* - * 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.assertNotNull; -import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertSame; -import static org.junit.Assert.assertTrue; -import static org.springframework.web.servlet.FlashMapManager.INPUT_FLASH_MAP_ATTRIBUTE; -import static org.springframework.web.servlet.FlashMapManager.OUTPUT_FLASH_MAP_ATTRIBUTE; - -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.mock.web.MockHttpServletResponse; -import org.springframework.web.servlet.FlashMap; -import org.springframework.web.util.WebUtils; - -/** - * Test fixture for {@link DefaultFlashMapManager} tests. - * - * @author Rossen Stoyanchev - */ -public class DefaultFlashMapManagerTests { - - private DefaultFlashMapManager flashMapManager; - - 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.response); - FlashMap flashMap = RequestContextUtils.getOutputFlashMap(request); - - assertNotNull("Current FlashMap not found", flashMap); - } - - @Test - public void requestStartedAlready() { - FlashMap flashMap = new FlashMap(); - this.request.setAttribute(OUTPUT_FLASH_MAP_ATTRIBUTE, flashMap); - this.flashMapManager.requestStarted(this.request, this.response); - - assertSame(flashMap, RequestContextUtils.getOutputFlashMap(request)); - } - - @Test - public void lookupFlashMapByPath() { - FlashMap flashMap = new FlashMap(); - flashMap.put("key", "value"); - flashMap.setTargetRequestPath("/path"); - - List allMaps = createFlashMaps(); - allMaps.add(flashMap); - - this.request.setRequestURI("/path"); - this.flashMapManager.requestStarted(this.request, this.response); - - assertEquals(flashMap, RequestContextUtils.getInputFlashMap(this.request)); - assertEquals("Input FlashMap should have been removed", 0, getFlashMaps().size()); - } - - // SPR-8779 - - @Test - public void lookupFlashMapByOriginatingPath() { - FlashMap flashMap = new FlashMap(); - flashMap.put("key", "value"); - flashMap.setTargetRequestPath("/accounts"); - - List allMaps = createFlashMaps(); - allMaps.add(flashMap); - - this.request.setAttribute(WebUtils.FORWARD_REQUEST_URI_ATTRIBUTE, "/accounts"); - this.request.setRequestURI("/mvc/accounts"); - this.flashMapManager.requestStarted(this.request, this.response); - - assertEquals(flashMap, RequestContextUtils.getInputFlashMap(this.request)); - assertEquals("Input FlashMap should have been removed", 0, getFlashMaps().size()); - } - - @Test - public void lookupFlashMapByPathWithTrailingSlash() { - FlashMap flashMap = new FlashMap(); - flashMap.put("key", "value"); - flashMap.setTargetRequestPath("/path"); - - List allMaps = createFlashMaps(); - allMaps.add(flashMap); - - this.request.setRequestURI("/path/"); - this.flashMapManager.requestStarted(this.request, this.response); - - assertEquals(flashMap, RequestContextUtils.getInputFlashMap(this.request)); - assertEquals("Input FlashMap should have been removed", 0, getFlashMaps().size()); - } - - @Test - public void lookupFlashMapWithParams() { - FlashMap flashMap = new FlashMap(); - flashMap.put("key", "value"); - flashMap.addTargetRequestParam("number", "one"); - - List allMaps = createFlashMaps(); - allMaps.add(flashMap); - - this.request.setParameter("number", (String) null); - 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.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.response); - - assertEquals(flashMap, RequestContextUtils.getInputFlashMap(this.request)); - assertEquals("Input FlashMap should have been removed", 0, getFlashMaps().size()); - } - - // SPR-8798 - - @Test - public void lookupFlashMapWithMultiValueParam() { - FlashMap flashMap = new FlashMap(); - flashMap.put("name", "value"); - flashMap.addTargetRequestParam("id", "1"); - flashMap.addTargetRequestParam("id", "2"); - - List allMaps = createFlashMaps(); - allMaps.add(flashMap); - - this.request.setParameter("id", "1"); - 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.response); - - assertEquals(flashMap, RequestContextUtils.getInputFlashMap(this.request)); - assertEquals("Input FlashMap should have been removed", 0, getFlashMaps().size()); - } - - @Test - public void lookupFlashMapSortOrder() { - FlashMap emptyFlashMap = new FlashMap(); - - FlashMap flashMapOne = new FlashMap(); - flashMapOne.put("key1", "value1"); - flashMapOne.setTargetRequestPath("/one"); - - FlashMap flashMapTwo = new FlashMap(); - flashMapTwo.put("key1", "value1"); - flashMapTwo.put("key2", "value2"); - flashMapTwo.setTargetRequestPath("/one/two"); - - List allMaps = createFlashMaps(); - allMaps.add(emptyFlashMap); - allMaps.add(flashMapOne); - allMaps.add(flashMapTwo); - Collections.shuffle(allMaps); - - this.request.setRequestURI("/one/two"); - this.flashMapManager.requestStarted(this.request, this.response); - - assertEquals(flashMapTwo, request.getAttribute(INPUT_FLASH_MAP_ATTRIBUTE)); - } - - @Test - public void removeExpiredFlashMaps() throws InterruptedException { - List allMaps = createFlashMaps(); - for (int i=0; i < 5; i++) { - FlashMap flashMap = new FlashMap(); - allMaps.add(flashMap); - flashMap.startExpirationPeriod(0); - } - Thread.sleep(100); - this.flashMapManager.requestStarted(this.request, this.response); - - assertEquals(0, allMaps.size()); - } - - @Test - public void saveFlashMapWithoutAttributes() throws InterruptedException { - this.flashMapManager.requestStarted(this.request, this.response); - this.flashMapManager.requestCompleted(this.request, this.response); - - assertNull(getFlashMaps()); - } - - @Test - public void saveFlashMapNotCreatedByThisManager() throws InterruptedException { - FlashMap flashMap = new FlashMap(); - this.request.setAttribute(OUTPUT_FLASH_MAP_ATTRIBUTE, flashMap); - this.flashMapManager.requestCompleted(this.request, this.response); - - assertNull(getFlashMaps()); - } - - @Test - public void saveFlashMapWithAttributes() throws InterruptedException { - 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.response); - - Thread.sleep(100); - - List allMaps = getFlashMaps(); - - assertNotNull(allMaps); - assertSame(flashMap, allMaps.get(0)); - assertTrue(flashMap.isExpired()); - } - - @Test - public void decodeTargetPath() throws InterruptedException { - 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.response); - - assertEquals("/once upon a time", flashMap.getTargetRequestPath()); - } - - @Test - public void normalizeTargetPath() throws InterruptedException { - 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.response); - - assertEquals("/once/upon/a", flashMap.getTargetRequestPath()); - - flashMap.setTargetRequestPath("./"); - this.request.setRequestURI("/once/upon/a/time"); - 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.response); - - assertEquals("/once/upon", flashMap.getTargetRequestPath()); - - flashMap.setTargetRequestPath("../"); - this.request.setRequestURI("/once/upon/a/time"); - 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.response); - - assertEquals("/once/only", flashMap.getTargetRequestPath()); - } - - @SuppressWarnings("unchecked") - private List getFlashMaps() { - return (List) this.request.getSession().getAttribute(DefaultFlashMapManager.class.getName() + ".FLASH_MAPS"); - } - - private List createFlashMaps() { - List allMaps = new CopyOnWriteArrayList(); - this.request.getSession().setAttribute(DefaultFlashMapManager.class.getName() + ".FLASH_MAPS", allMaps); - return allMaps; - } - - private void clearFlashMapRequestAttributes() { - request.removeAttribute(INPUT_FLASH_MAP_ATTRIBUTE); - request.removeAttribute(OUTPUT_FLASH_MAP_ATTRIBUTE); - } - -} 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 5f80b80e63e..4ae6a2aa5dc 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-2011 the original author or authors. + * Copyright 2002-2012 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. @@ -48,8 +48,9 @@ import org.springframework.web.servlet.DispatcherServlet; import org.springframework.web.servlet.FlashMap; import org.springframework.web.servlet.FlashMapManager; import org.springframework.web.servlet.View; -import org.springframework.web.servlet.support.RequestDataValueProcessorWrapper; import org.springframework.web.servlet.support.RequestDataValueProcessor; +import org.springframework.web.servlet.support.RequestDataValueProcessorWrapper; +import org.springframework.web.servlet.support.SessionFlashMapManager; import org.springframework.web.util.WebUtils; /** @@ -75,20 +76,29 @@ public class RedirectViewTests { RedirectView rv = new RedirectView(); rv.setUrl("http://url.somewhere.com"); rv.setHttp10Compatible(false); - MockHttpServletRequest request = new MockHttpServletRequest(); + MockHttpServletRequest request = createRequest(); MockHttpServletResponse response = new MockHttpServletResponse(); + request.setAttribute(DispatcherServlet.OUTPUT_FLASH_MAP_ATTRIBUTE, new FlashMap()); + request.setAttribute(DispatcherServlet.FLASH_MAP_MANAGER_ATTRIBUTE, new SessionFlashMapManager()); rv.render(new HashMap(), request, response); assertEquals(303, response.getStatus()); assertEquals("http://url.somewhere.com", response.getHeader("Location")); } + private MockHttpServletRequest createRequest() { + MockHttpServletRequest request = new MockHttpServletRequest(); + request.setAttribute(DispatcherServlet.OUTPUT_FLASH_MAP_ATTRIBUTE, new FlashMap()); + request.setAttribute(DispatcherServlet.FLASH_MAP_MANAGER_ATTRIBUTE, new SessionFlashMapManager()); + return request; + } + @Test public void explicitStatusCodeHttp11() throws Exception { RedirectView rv = new RedirectView(); rv.setUrl("http://url.somewhere.com"); rv.setHttp10Compatible(false); rv.setStatusCode(HttpStatus.MOVED_PERMANENTLY); - MockHttpServletRequest request = new MockHttpServletRequest(); + MockHttpServletRequest request = createRequest(); MockHttpServletResponse response = new MockHttpServletResponse(); rv.render(new HashMap(), request, response); assertEquals(301, response.getStatus()); @@ -100,7 +110,7 @@ public class RedirectViewTests { RedirectView rv = new RedirectView(); rv.setUrl("http://url.somewhere.com"); rv.setStatusCode(HttpStatus.MOVED_PERMANENTLY); - MockHttpServletRequest request = new MockHttpServletRequest(); + MockHttpServletRequest request = createRequest(); MockHttpServletResponse response = new MockHttpServletResponse(); rv.render(new HashMap(), request, response); assertEquals(301, response.getStatus()); @@ -112,7 +122,7 @@ public class RedirectViewTests { RedirectView rv = new RedirectView(); rv.setUrl("http://url.somewhere.com"); rv.setHttp10Compatible(false); - MockHttpServletRequest request = new MockHttpServletRequest(); + MockHttpServletRequest request = createRequest(); request.setAttribute(View.RESPONSE_STATUS_ATTRIBUTE, HttpStatus.CREATED); MockHttpServletResponse response = new MockHttpServletResponse(); rv.render(new HashMap(), request, response); @@ -125,11 +135,11 @@ public class RedirectViewTests { RedirectView rv = new RedirectView(); rv.setUrl("http://url.somewhere.com/path"); rv.setHttp10Compatible(false); - HttpServletRequest request = new MockHttpServletRequest(); + MockHttpServletRequest request = createRequest(); HttpServletResponse response = new MockHttpServletResponse(); FlashMap flashMap = new FlashMap(); flashMap.put("successMessage", "yay!"); - request.setAttribute(FlashMapManager.OUTPUT_FLASH_MAP_ATTRIBUTE, flashMap); + request.setAttribute(DispatcherServlet.OUTPUT_FLASH_MAP_ATTRIBUTE, flashMap); ModelMap model = new ModelMap("id", "1"); rv.render(model, request, response); assertEquals(303, response.getStatus()); @@ -153,7 +163,7 @@ public class RedirectViewTests { rv.setApplicationContext(wac); // Init RedirectView with WebAppCxt rv.setUrl("/path"); - HttpServletRequest request = new MockHttpServletRequest(); + MockHttpServletRequest request = createRequest(); request.setAttribute(DispatcherServlet.WEB_APPLICATION_CONTEXT_ATTRIBUTE, wac); HttpServletResponse response = new MockHttpServletResponse(); @@ -182,7 +192,7 @@ public class RedirectViewTests { RedirectView rv = new RedirectView(); rv.setUrl("/path"); - HttpServletRequest request = new MockHttpServletRequest(); + MockHttpServletRequest request = createRequest(); HttpServletResponse response = new MockHttpServletResponse(); EasyMock.expect(mockProcessor.processUrl(request, "/path")).andReturn("/path?key=123"); @@ -361,6 +371,11 @@ public class RedirectViewTests { expectedUrlForEncoding = "/context" + expectedUrlForEncoding; expect(request.getContextPath()).andReturn("/context"); } + + expect(request.getAttribute(DispatcherServlet.OUTPUT_FLASH_MAP_ATTRIBUTE)).andReturn(new FlashMap()); + + FlashMapManager flashMapManager = new SessionFlashMapManager(); + expect(request.getAttribute(DispatcherServlet.FLASH_MAP_MANAGER_ATTRIBUTE)).andReturn(flashMapManager); HttpServletResponse response = createMock("response", HttpServletResponse.class); expect(response.encodeRedirectURL(expectedUrlForEncoding)).andReturn(expectedUrlForEncoding); diff --git a/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/view/RedirectViewUriTemplateTests.java b/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/view/RedirectViewUriTemplateTests.java index 87b5d5a3b85..96526a174d7 100644 --- a/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/view/RedirectViewUriTemplateTests.java +++ b/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/view/RedirectViewUriTemplateTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2011 the original author or authors. + * Copyright 2002-2012 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. @@ -26,7 +26,10 @@ import org.junit.Test; import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.mock.web.MockHttpServletResponse; import org.springframework.ui.ModelMap; +import org.springframework.web.servlet.DispatcherServlet; +import org.springframework.web.servlet.FlashMap; import org.springframework.web.servlet.HandlerMapping; +import org.springframework.web.servlet.support.SessionFlashMapManager; public class RedirectViewUriTemplateTests { @@ -38,6 +41,8 @@ public class RedirectViewUriTemplateTests { public void setUp() { this.request = new MockHttpServletRequest(); this.response = new MockHttpServletResponse(); + this.request.setAttribute(DispatcherServlet.OUTPUT_FLASH_MAP_ATTRIBUTE, new FlashMap()); + this.request.setAttribute(DispatcherServlet.FLASH_MAP_MANAGER_ATTRIBUTE, new SessionFlashMapManager()); } @Test