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 a89bf9051b3..1da97cdd3cc 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,13 +16,14 @@ package org.springframework.web.servlet; +import java.util.HashMap; 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; @@ -32,13 +33,13 @@ import org.springframework.web.util.UrlPathHelper; * @author Rossen Stoyanchev * @since 3.1 */ -public class FlashMap extends ModelMap implements Comparable { +public class FlashMap extends HashMap implements Comparable { private static final long serialVersionUID = 1L; private String expectedUrlPath; - private Map expectedRequestParameters = new LinkedHashMap(); + private final Map expectedRequestParameters = new LinkedHashMap(); private UrlPathHelper urlPathHelper = new UrlPathHelper(); @@ -47,52 +48,62 @@ public class FlashMap extends ModelMap implements Comparable { private int timeToLive; /** - * 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 + * Provide a URL to identify the target request for this FlashMap. + * Only the path of the provided URL will be used for matching purposes. + * If the URL is absolute or has a query string, the URL path is + * extracted. Or if the URL is relative, it is appended to the current + * request URI and normalized. + * + * @param request the current request, used to normalize relative URLs + * @param url an absolute URL, a URL path, or a relative URL, never {@code null} */ - public void setExpectedUrlPath(HttpServletRequest request, String url) { - this.expectedUrlPath = (url != null) ? normalizeRelativeUrlPath(request, extractUrlPath(url)) : null; + public FlashMap setExpectedUrl(HttpServletRequest request, String url) { + Assert.notNull(url, "Expected URL must not be null"); + String urlPath = extractUrlPath(url); + this.expectedUrlPath = urlPath.startsWith("/") ? urlPath : normalizeRelativeUrl(request, urlPath); + return this; } private String extractUrlPath(String url) { int index = url.indexOf("://"); if (index != -1) { - index = url.indexOf("/", index + 3); + index = url.indexOf('/', index + 3); url = (index != -1) ? url.substring(index) : ""; } - index = url.indexOf("?"); + index = url.indexOf('?'); return (index != -1) ? url.substring(0, index) : url; } - 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; + private String normalizeRelativeUrl(HttpServletRequest request, String relativeUrl) { + String requestUri = this.urlPathHelper.getRequestUri(request); + relativeUrl = requestUri.substring(0, requestUri.lastIndexOf('/') + 1) + relativeUrl; + return StringUtils.cleanPath(relativeUrl); } - + + /** + * Add a request parameter pair to help identify the request this FlashMap + * should be made available to. If expected parameters are not set, the + * FlashMap instance will match to requests with any parameters. + * + * @param name the name of the expected parameter (never {@code null}) + * @param value the value for the expected parameter (never {@code null}) + */ + public FlashMap setExpectedRequestParam(String name, String value) { + this.expectedRequestParameters.put(name, value.toString()); + return this; + } + /** * 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. + * should be made available to. If expected parameters are not set, the + * 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 void setExpectedRequestParameters(Map params) { - this.expectedRequestParameters = new LinkedHashMap(); + public FlashMap setExpectedRequestParams(Map params) { if (params != null) { for (String name : params.keySet()) { Object value = params.get(name); @@ -101,12 +112,13 @@ public class FlashMap extends ModelMap implements Comparable { } } } + return this; } - + /** * Whether this FlashMap matches to the given request by checking - * expectations provided via {@link #setExpectedUrlPath} and - * {@link #setExpectedRequestParameters}. + * expectations provided via {@link #setExpectedUrl} and + * {@link #setExpectedRequestParams}. * * @param request the current request * diff --git a/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/mvc/method/support/RedirectResponse.java b/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/mvc/method/support/RedirectResponse.java index 3998c844a49..4ff908bd893 100644 --- a/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/mvc/method/support/RedirectResponse.java +++ b/org.springframework.web.servlet/src/main/java/org/springframework/web/servlet/mvc/method/support/RedirectResponse.java @@ -16,8 +16,12 @@ package org.springframework.web.servlet.mvc.method.support; +import java.util.Collection; + import javax.servlet.http.HttpServletRequest; +import org.springframework.core.Conventions; +import org.springframework.util.Assert; import org.springframework.web.context.request.NativeWebRequest; import org.springframework.web.method.support.ModelAndViewContainer; import org.springframework.web.servlet.FlashMap; @@ -92,7 +96,7 @@ public class RedirectResponse { * target controller method after the redirect. */ public RedirectResponse flashAttribute(String name, Object value) { - getFlashMap().addAttribute(name, value); + getFlashMap().put(name, value); return this; } @@ -102,9 +106,12 @@ public class RedirectResponse { * The name of the attribute is selected using a * {@link org.springframework.core.Conventions#getVariableName generated name}. */ - public RedirectResponse flashAttribute(Object value) { - getFlashMap().addAttribute(value); - return this; + public RedirectResponse flashAttribute(Object attributeValue) { + Assert.notNull(attributeValue, "Model object must not be null"); + if (attributeValue instanceof Collection && ((Collection) attributeValue).isEmpty()) { + return this; + } + return flashAttribute(Conventions.getVariableName(attributeValue), attributeValue); } private FlashMap getFlashMap() { 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 dae1cd48291..f6aff1a0d87 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 @@ -264,8 +264,7 @@ public class RedirectView extends AbstractUrlBasedView { FlashMap flashMap = RequestContextUtils.getFlashMap(request); if (!CollectionUtils.isEmpty(flashMap)) { - flashMap.setExpectedUrlPath(request, targetUrl.toString()); - flashMap.setExpectedRequestParameters(model); + flashMap.setExpectedUrl(request, targetUrl.toString()).setExpectedRequestParams(model); } if (this.exposeModelAttributes) { diff --git a/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/FlashMapTests.java b/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/FlashMapTests.java index 20f1639527c..20688098c24 100644 --- a/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/FlashMapTests.java +++ b/org.springframework.web.servlet/src/test/java/org/springframework/web/servlet/FlashMapTests.java @@ -44,14 +44,14 @@ public class FlashMapTests { @Test public void matchUrlPath() { FlashMap flashMap = new FlashMap(); - flashMap.setExpectedUrlPath(null, "/yes"); + flashMap.setExpectedUrl(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?"); + flashMap.setExpectedUrl(null, "/thats it?"); assertTrue(flashMap.matches(new MockHttpServletRequest("GET", "/thats it"))); assertTrue(flashMap.matches(new MockHttpServletRequest("GET", "/thats%20it"))); @@ -61,45 +61,45 @@ public class FlashMapTests { public void matchRelativeUrlPath() { FlashMap flashMap = new FlashMap(); - flashMap.setExpectedUrlPath(new MockHttpServletRequest("GET", "/oh/no"), "yes"); + flashMap.setExpectedUrl(new MockHttpServletRequest("GET", "/oh/no"), "yes"); assertTrue(flashMap.matches(new MockHttpServletRequest("GET", "/oh/yes"))); - flashMap.setExpectedUrlPath(new MockHttpServletRequest("GET", "/oh/not/again"), "../ok"); + flashMap.setExpectedUrl(new MockHttpServletRequest("GET", "/oh/not/again"), "../ok"); assertTrue(flashMap.matches(new MockHttpServletRequest("GET", "/oh/ok"))); - flashMap.setExpectedUrlPath(new MockHttpServletRequest("GET", "/yes/it/is"), ".."); + flashMap.setExpectedUrl(new MockHttpServletRequest("GET", "/yes/it/is"), ".."); assertTrue(flashMap.matches(new MockHttpServletRequest("GET", "/yes"))); - flashMap.setExpectedUrlPath(new MockHttpServletRequest("GET", "/yes/it/is"), "../"); + flashMap.setExpectedUrl(new MockHttpServletRequest("GET", "/yes/it/is"), "../"); assertTrue(flashMap.matches(new MockHttpServletRequest("GET", "/yes"))); - flashMap.setExpectedUrlPath(new MockHttpServletRequest("GET", "/thats it/really"), "./"); + flashMap.setExpectedUrl(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"); + flashMap.setExpectedUrl(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/"); + flashMap.setExpectedUrl(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"); + flashMap.setExpectedUrl(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"); + flashMap.setExpectedUrl(null, "http://example.com/yes?a=1"); assertTrue(flashMap.matches(new MockHttpServletRequest("GET", "/yes"))); } @@ -109,7 +109,7 @@ public class FlashMapTests { String parameterName = "numero"; FlashMap flashMap = new FlashMap(); - flashMap.setExpectedRequestParameters(new ModelMap(parameterName, "uno")); + flashMap.setExpectedRequestParam(parameterName, "uno"); MockHttpServletRequest request = new MockHttpServletRequest(); assertFalse(flashMap.matches(request)); @@ -151,18 +151,18 @@ public class FlashMapTests { FlashMap flashMap2 = new FlashMap(); assertEquals(0, flashMap1.compareTo(flashMap2)); - flashMap1.setExpectedUrlPath(null, "/path1"); + flashMap1.setExpectedUrl(null, "/path1"); assertEquals(-1, flashMap1.compareTo(flashMap2)); assertEquals(1, flashMap2.compareTo(flashMap1)); - flashMap2.setExpectedUrlPath(null, "/path2"); + flashMap2.setExpectedUrl(null, "/path2"); assertEquals(0, flashMap1.compareTo(flashMap2)); - flashMap1.setExpectedRequestParameters(new ModelMap("id", "1")); + flashMap1.setExpectedRequestParam("id", "1"); assertEquals(-1, flashMap1.compareTo(flashMap2)); assertEquals(1, flashMap2.compareTo(flashMap1)); - flashMap2.setExpectedRequestParameters(new ModelMap("id", "2")); + flashMap2.setExpectedRequestParam("id", "2"); assertEquals(0, flashMap1.compareTo(flashMap2)); } 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 96accc2cb53..1d3e8bde70a 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 @@ -79,10 +79,10 @@ public class DefaultFlashMapManagerTests { FlashMap emptyFlashMap = new FlashMap(); FlashMap oneFlashMap = new FlashMap(); - oneFlashMap.setExpectedUrlPath(null, "/one"); + oneFlashMap.setExpectedUrl(null, "/one"); FlashMap oneOtherFlashMap = new FlashMap(); - oneOtherFlashMap.setExpectedUrlPath(null, "/one/other"); + oneOtherFlashMap.setExpectedUrl(null, "/one/other"); List allMaps = createFlashMapsSessionAttribute(); allMaps.add(emptyFlashMap);