SPR-6464 FlashMap polish

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();
 	}

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) {

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");

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);