From aa46d181254e8e36aedd5ee90964b9072de4dea7 Mon Sep 17 00:00:00 2001
From: Rossen Stoyanchev <rstoyanchev@vmware.com>
Date: Mon, 15 Aug 2011 12:48:33 +0000
Subject: [PATCH] SPR-6464 FlashMap polish

---
 .../springframework/web/servlet/FlashMap.java | 38 ++++++++++---------
 .../web/servlet/view/RedirectView.java        |  2 +-
 .../web/servlet/FlashMapTests.java            | 31 +++++++--------
 .../support/DefaultFlashMapManagerTests.java  |  4 +-
 4 files changed, 40 insertions(+), 35 deletions(-)

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 1da97cdd3c..038c6815ac 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
@@ -37,7 +37,7 @@ public class FlashMap extends HashMap<String, Object> implements Comparable<Flas
 
 	private static final long serialVersionUID = 1L;
 	
-	private String expectedUrlPath;
+	private String expectedRequestUri;
 	
 	private final Map<String, String> expectedRequestParameters = new LinkedHashMap<String, String>();
 	
@@ -57,24 +57,27 @@ public class FlashMap extends HashMap<String, Object> implements Comparable<Flas
 	 * @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 FlashMap setExpectedUrl(HttpServletRequest request, String url) {
+	public FlashMap setExpectedRequestUri(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);
+		String path = extractRequestUri(url);
+		this.expectedRequestUri = path.startsWith("/") ? path : normalizeRelativePath(request, path);
 		return this;
 	}
 
-	private String extractUrlPath(String url) {
-		int index = url.indexOf("://");
+	private String extractRequestUri(String url) {
+		int index = url.indexOf("?");
 		if (index != -1) {
-			index = url.indexOf('/', index + 3);
-			url = (index != -1) ? url.substring(index) : "";
+			url = url.substring(0, index);
 		}
-		index = url.indexOf('?');
-		return (index != -1) ? url.substring(0, index) : url;  
+		index = url.indexOf("://");
+		if (index != -1) {
+			int pathBegin = url.indexOf("/", index + 3);
+			url = (pathBegin != -1 ) ? url.substring(pathBegin) : "";
+		}
+		return url;
 	}
 
-	private String normalizeRelativeUrl(HttpServletRequest request, String relativeUrl) {
+	private String normalizeRelativePath(HttpServletRequest request, String relativeUrl) {
 		String requestUri = this.urlPathHelper.getRequestUri(request);
 		relativeUrl = requestUri.substring(0, requestUri.lastIndexOf('/') + 1) + relativeUrl;
 		return StringUtils.cleanPath(relativeUrl);
@@ -117,7 +120,7 @@ public class FlashMap extends HashMap<String, Object> implements Comparable<Flas
 
 	/**
 	 * Whether this FlashMap matches to the given request by checking 
-	 * expectations provided via {@link #setExpectedUrl} and 
+	 * expectations provided via {@link #setExpectedRequestUri} and 
 	 * {@link #setExpectedRequestParams}.
 	 * 
 	 * @param request the current request
@@ -125,8 +128,9 @@ public class FlashMap extends HashMap<String, Object> implements Comparable<Flas
 	 * @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)) {
+		if (this.expectedRequestUri != null) {
+			String requestUri = this.urlPathHelper.getRequestUri(request);
+			if (!matchPathsIgnoreTrailingSlash(requestUri, this.expectedRequestUri)) {
 				return false;
 			}
 		}
@@ -178,8 +182,8 @@ public class FlashMap extends HashMap<String, Object> implements Comparable<Flas
 	 * 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;
+		int thisUrlPath = (this.expectedRequestUri != null) ? 1 : 0;
+		int otherUrlPath = (other.expectedRequestUri != null) ? 1 : 0;
 		if (thisUrlPath != otherUrlPath) {
 			return otherUrlPath - thisUrlPath;
 		}
@@ -192,7 +196,7 @@ public class FlashMap extends HashMap<String, Object> implements Comparable<Flas
 	public String toString() {
 		StringBuilder result = new StringBuilder();
 		result.append("[Attributes=").append(super.toString());
-		result.append(", expecteUrlPath=").append(this.expectedUrlPath);
+		result.append(", expecteRequestUri=").append(this.expectedRequestUri);
 		result.append(", expectedRequestParameters=" + this.expectedRequestParameters.toString()).append("]");
 		return result.toString();
 	}
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 6b2b4c6997..9eafad72b4 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,7 +264,7 @@ public class RedirectView extends AbstractUrlBasedView {
 
 		FlashMap flashMap = RequestContextUtils.getFlashMap(request);
 		if (!CollectionUtils.isEmpty(flashMap)) {
-			flashMap.setExpectedUrlPath(request, targetUrl.toString());
+			flashMap.setExpectedRequestUri(request, targetUrl.toString());
 		}
 
 		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 20688098c2..bdc0f7682e 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
@@ -22,8 +22,6 @@ 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.
@@ -44,14 +42,14 @@ public class FlashMapTests {
 	@Test
 	public void matchUrlPath() {
 		FlashMap flashMap = new FlashMap();
-		flashMap.setExpectedUrl(null, "/yes");
+		flashMap.setExpectedRequestUri(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.setExpectedUrl(null, "/thats it?");
+		flashMap.setExpectedRequestUri(null, "/thats it?");
 
 		assertTrue(flashMap.matches(new MockHttpServletRequest("GET", "/thats it")));
 		assertTrue(flashMap.matches(new MockHttpServletRequest("GET", "/thats%20it")));
@@ -61,45 +59,48 @@ public class FlashMapTests {
 	public void matchRelativeUrlPath() {
 		FlashMap flashMap = new FlashMap();
 
-		flashMap.setExpectedUrl(new MockHttpServletRequest("GET", "/oh/no"), "yes");
+		flashMap.setExpectedRequestUri(new MockHttpServletRequest("GET", "/oh/no"), "yes");
 		assertTrue(flashMap.matches(new MockHttpServletRequest("GET", "/oh/yes")));
 
-		flashMap.setExpectedUrl(new MockHttpServletRequest("GET", "/oh/not/again"), "../ok");
+		flashMap.setExpectedRequestUri(new MockHttpServletRequest("GET", "/oh/not/again"), "../ok");
 		assertTrue(flashMap.matches(new MockHttpServletRequest("GET", "/oh/ok")));
 
-		flashMap.setExpectedUrl(new MockHttpServletRequest("GET", "/yes/it/is"), "..");
+		flashMap.setExpectedRequestUri(new MockHttpServletRequest("GET", "/yes/it/is"), "..");
 		assertTrue(flashMap.matches(new MockHttpServletRequest("GET", "/yes")));
 
-		flashMap.setExpectedUrl(new MockHttpServletRequest("GET", "/yes/it/is"), "../");
+		flashMap.setExpectedRequestUri(new MockHttpServletRequest("GET", "/yes/it/is"), "../");
 		assertTrue(flashMap.matches(new MockHttpServletRequest("GET", "/yes")));
 
-		flashMap.setExpectedUrl(new MockHttpServletRequest("GET", "/thats it/really"), "./");
+		flashMap.setExpectedRequestUri(new MockHttpServletRequest("GET", "/thats it/really"), "./");
 		assertTrue(flashMap.matches(new MockHttpServletRequest("GET", "/thats%20it")));
+		
+		flashMap.setExpectedRequestUri(new MockHttpServletRequest("GET", "/yes/it/is"), "..?url=http://example.com");
+		assertTrue(flashMap.matches(new MockHttpServletRequest("GET", "/yes")));
 	}
 
 	@Test
 	public void matchAbsoluteUrlPath() {
 		FlashMap flashMap = new FlashMap();
-		flashMap.setExpectedUrl(new MockHttpServletRequest(), "http://example.com");
+		flashMap.setExpectedRequestUri(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.setExpectedUrl(null, "http://example.com/");
+		flashMap.setExpectedRequestUri(null, "http://example.com/");
 
 		assertTrue(flashMap.matches(new MockHttpServletRequest("GET", "")));
 		assertTrue(flashMap.matches(new MockHttpServletRequest("GET", "/")));
 		assertFalse(flashMap.matches(new MockHttpServletRequest("GET", "/no")));
 
-		flashMap.setExpectedUrl(null, "http://example.com/yes");
+		flashMap.setExpectedRequestUri(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.setExpectedUrl(null, "http://example.com/yes?a=1");
+		flashMap.setExpectedRequestUri(null, "http://example.com/yes?a=1");
 
 		assertTrue(flashMap.matches(new MockHttpServletRequest("GET", "/yes")));
 	}
@@ -151,11 +152,11 @@ public class FlashMapTests {
 		FlashMap flashMap2 = new FlashMap();
 		assertEquals(0, flashMap1.compareTo(flashMap2));
 
-		flashMap1.setExpectedUrl(null, "/path1");
+		flashMap1.setExpectedRequestUri(null, "/path1");
 		assertEquals(-1, flashMap1.compareTo(flashMap2));
 		assertEquals(1, flashMap2.compareTo(flashMap1));
 
-		flashMap2.setExpectedUrl(null, "/path2");
+		flashMap2.setExpectedRequestUri(null, "/path2");
 		assertEquals(0, flashMap1.compareTo(flashMap2));
 
 		flashMap1.setExpectedRequestParam("id", "1");
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 1d3e8bde70..28c9052caf 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.setExpectedUrl(null, "/one");
+		oneFlashMap.setExpectedRequestUri(null, "/one");
 
 		FlashMap oneOtherFlashMap = new FlashMap();
-		oneOtherFlashMap.setExpectedUrl(null, "/one/other");
+		oneOtherFlashMap.setExpectedRequestUri(null, "/one/other");
 		
 		List<FlashMap> allMaps = createFlashMapsSessionAttribute();
 		allMaps.add(emptyFlashMap);