SPR-6464 FlashMap polish
This commit is contained in:
parent
224bce1b64
commit
aa46d18125
|
|
@ -37,7 +37,7 @@ public class FlashMap extends HashMap<String, Object> implements Comparable<Flas
|
||||||
|
|
||||||
private static final long serialVersionUID = 1L;
|
private static final long serialVersionUID = 1L;
|
||||||
|
|
||||||
private String expectedUrlPath;
|
private String expectedRequestUri;
|
||||||
|
|
||||||
private final Map<String, String> expectedRequestParameters = new LinkedHashMap<String, String>();
|
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 request the current request, used to normalize relative URLs
|
||||||
* @param url an absolute URL, a URL path, or a relative URL, never {@code null}
|
* @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");
|
Assert.notNull(url, "Expected URL must not be null");
|
||||||
String urlPath = extractUrlPath(url);
|
String path = extractRequestUri(url);
|
||||||
this.expectedUrlPath = urlPath.startsWith("/") ? urlPath : normalizeRelativeUrl(request, urlPath);
|
this.expectedRequestUri = path.startsWith("/") ? path : normalizeRelativePath(request, path);
|
||||||
return this;
|
return this;
|
||||||
}
|
}
|
||||||
|
|
||||||
private String extractUrlPath(String url) {
|
private String extractRequestUri(String url) {
|
||||||
int index = url.indexOf("://");
|
int index = url.indexOf("?");
|
||||||
if (index != -1) {
|
if (index != -1) {
|
||||||
index = url.indexOf('/', index + 3);
|
url = url.substring(0, index);
|
||||||
url = (index != -1) ? url.substring(index) : "";
|
|
||||||
}
|
}
|
||||||
index = url.indexOf('?');
|
index = url.indexOf("://");
|
||||||
return (index != -1) ? url.substring(0, index) : url;
|
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);
|
String requestUri = this.urlPathHelper.getRequestUri(request);
|
||||||
relativeUrl = requestUri.substring(0, requestUri.lastIndexOf('/') + 1) + relativeUrl;
|
relativeUrl = requestUri.substring(0, requestUri.lastIndexOf('/') + 1) + relativeUrl;
|
||||||
return StringUtils.cleanPath(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
|
* Whether this FlashMap matches to the given request by checking
|
||||||
* expectations provided via {@link #setExpectedUrl} and
|
* expectations provided via {@link #setExpectedRequestUri} and
|
||||||
* {@link #setExpectedRequestParams}.
|
* {@link #setExpectedRequestParams}.
|
||||||
*
|
*
|
||||||
* @param request the current request
|
* @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.
|
* @return "true" if the expectations match or there are no expectations.
|
||||||
*/
|
*/
|
||||||
public boolean matches(HttpServletRequest request) {
|
public boolean matches(HttpServletRequest request) {
|
||||||
if (this.expectedUrlPath != null) {
|
if (this.expectedRequestUri != null) {
|
||||||
if (!matchPathsIgnoreTrailingSlash(this.urlPathHelper.getRequestUri(request), this.expectedUrlPath)) {
|
String requestUri = this.urlPathHelper.getRequestUri(request);
|
||||||
|
if (!matchPathsIgnoreTrailingSlash(requestUri, this.expectedRequestUri)) {
|
||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
@ -178,8 +182,8 @@ public class FlashMap extends HashMap<String, Object> implements Comparable<Flas
|
||||||
* current request via {@link FlashMap#matches}.
|
* current request via {@link FlashMap#matches}.
|
||||||
*/
|
*/
|
||||||
public int compareTo(FlashMap other) {
|
public int compareTo(FlashMap other) {
|
||||||
int thisUrlPath = (this.expectedUrlPath != null) ? 1 : 0;
|
int thisUrlPath = (this.expectedRequestUri != null) ? 1 : 0;
|
||||||
int otherUrlPath = (other.expectedUrlPath != null) ? 1 : 0;
|
int otherUrlPath = (other.expectedRequestUri != null) ? 1 : 0;
|
||||||
if (thisUrlPath != otherUrlPath) {
|
if (thisUrlPath != otherUrlPath) {
|
||||||
return otherUrlPath - thisUrlPath;
|
return otherUrlPath - thisUrlPath;
|
||||||
}
|
}
|
||||||
|
|
@ -192,7 +196,7 @@ public class FlashMap extends HashMap<String, Object> implements Comparable<Flas
|
||||||
public String toString() {
|
public String toString() {
|
||||||
StringBuilder result = new StringBuilder();
|
StringBuilder result = new StringBuilder();
|
||||||
result.append("[Attributes=").append(super.toString());
|
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("]");
|
result.append(", expectedRequestParameters=" + this.expectedRequestParameters.toString()).append("]");
|
||||||
return result.toString();
|
return result.toString();
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -264,7 +264,7 @@ public class RedirectView extends AbstractUrlBasedView {
|
||||||
|
|
||||||
FlashMap flashMap = RequestContextUtils.getFlashMap(request);
|
FlashMap flashMap = RequestContextUtils.getFlashMap(request);
|
||||||
if (!CollectionUtils.isEmpty(flashMap)) {
|
if (!CollectionUtils.isEmpty(flashMap)) {
|
||||||
flashMap.setExpectedUrlPath(request, targetUrl.toString());
|
flashMap.setExpectedRequestUri(request, targetUrl.toString());
|
||||||
}
|
}
|
||||||
|
|
||||||
if (this.exposeModelAttributes) {
|
if (this.exposeModelAttributes) {
|
||||||
|
|
|
||||||
|
|
@ -22,8 +22,6 @@ import static org.junit.Assert.assertTrue;
|
||||||
|
|
||||||
import org.junit.Test;
|
import org.junit.Test;
|
||||||
import org.springframework.mock.web.MockHttpServletRequest;
|
import org.springframework.mock.web.MockHttpServletRequest;
|
||||||
import org.springframework.ui.ModelMap;
|
|
||||||
import org.springframework.web.servlet.FlashMap;
|
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Test fixture for {@link FlashMap} tests.
|
* Test fixture for {@link FlashMap} tests.
|
||||||
|
|
@ -44,14 +42,14 @@ public class FlashMapTests {
|
||||||
@Test
|
@Test
|
||||||
public void matchUrlPath() {
|
public void matchUrlPath() {
|
||||||
FlashMap flashMap = new FlashMap();
|
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")));
|
||||||
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", "/yes/but")));
|
||||||
assertFalse(flashMap.matches(new MockHttpServletRequest("GET", "/no")));
|
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 it")));
|
||||||
assertTrue(flashMap.matches(new MockHttpServletRequest("GET", "/thats%20it")));
|
assertTrue(flashMap.matches(new MockHttpServletRequest("GET", "/thats%20it")));
|
||||||
|
|
@ -61,45 +59,48 @@ public class FlashMapTests {
|
||||||
public void matchRelativeUrlPath() {
|
public void matchRelativeUrlPath() {
|
||||||
FlashMap flashMap = new FlashMap();
|
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")));
|
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")));
|
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")));
|
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")));
|
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")));
|
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
|
@Test
|
||||||
public void matchAbsoluteUrlPath() {
|
public void matchAbsoluteUrlPath() {
|
||||||
FlashMap flashMap = new FlashMap();
|
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", "")));
|
||||||
assertTrue(flashMap.matches(new MockHttpServletRequest("GET", "/")));
|
assertTrue(flashMap.matches(new MockHttpServletRequest("GET", "/")));
|
||||||
assertFalse(flashMap.matches(new MockHttpServletRequest("GET", "/no")));
|
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", "")));
|
||||||
assertTrue(flashMap.matches(new MockHttpServletRequest("GET", "/")));
|
assertTrue(flashMap.matches(new MockHttpServletRequest("GET", "/")));
|
||||||
assertFalse(flashMap.matches(new MockHttpServletRequest("GET", "/no")));
|
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")));
|
||||||
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", "/no")));
|
||||||
assertFalse(flashMap.matches(new MockHttpServletRequest("GET", "/yes/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")));
|
assertTrue(flashMap.matches(new MockHttpServletRequest("GET", "/yes")));
|
||||||
}
|
}
|
||||||
|
|
@ -151,11 +152,11 @@ public class FlashMapTests {
|
||||||
FlashMap flashMap2 = new FlashMap();
|
FlashMap flashMap2 = new FlashMap();
|
||||||
assertEquals(0, flashMap1.compareTo(flashMap2));
|
assertEquals(0, flashMap1.compareTo(flashMap2));
|
||||||
|
|
||||||
flashMap1.setExpectedUrl(null, "/path1");
|
flashMap1.setExpectedRequestUri(null, "/path1");
|
||||||
assertEquals(-1, flashMap1.compareTo(flashMap2));
|
assertEquals(-1, flashMap1.compareTo(flashMap2));
|
||||||
assertEquals(1, flashMap2.compareTo(flashMap1));
|
assertEquals(1, flashMap2.compareTo(flashMap1));
|
||||||
|
|
||||||
flashMap2.setExpectedUrl(null, "/path2");
|
flashMap2.setExpectedRequestUri(null, "/path2");
|
||||||
assertEquals(0, flashMap1.compareTo(flashMap2));
|
assertEquals(0, flashMap1.compareTo(flashMap2));
|
||||||
|
|
||||||
flashMap1.setExpectedRequestParam("id", "1");
|
flashMap1.setExpectedRequestParam("id", "1");
|
||||||
|
|
|
||||||
|
|
@ -79,10 +79,10 @@ public class DefaultFlashMapManagerTests {
|
||||||
FlashMap emptyFlashMap = new FlashMap();
|
FlashMap emptyFlashMap = new FlashMap();
|
||||||
|
|
||||||
FlashMap oneFlashMap = new FlashMap();
|
FlashMap oneFlashMap = new FlashMap();
|
||||||
oneFlashMap.setExpectedUrl(null, "/one");
|
oneFlashMap.setExpectedRequestUri(null, "/one");
|
||||||
|
|
||||||
FlashMap oneOtherFlashMap = new FlashMap();
|
FlashMap oneOtherFlashMap = new FlashMap();
|
||||||
oneOtherFlashMap.setExpectedUrl(null, "/one/other");
|
oneOtherFlashMap.setExpectedRequestUri(null, "/one/other");
|
||||||
|
|
||||||
List<FlashMap> allMaps = createFlashMapsSessionAttribute();
|
List<FlashMap> allMaps = createFlashMapsSessionAttribute();
|
||||||
allMaps.add(emptyFlashMap);
|
allMaps.add(emptyFlashMap);
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue