Minor refactoring in PathPattern

Rename getPathRemaining to matchStartOfPath since the method does
match and to be more clear about what the method and the return value
intuitively follows.

Remove matchStart which matches the start of the pattern (rather than
the start of the path). It is a use case that does not come up in
request mapping.
This commit is contained in:
Rossen Stoyanchev 2017-08-02 15:05:28 +02:00
parent c060f4f615
commit dccedd5ad5
5 changed files with 49 additions and 184 deletions

View File

@ -177,14 +177,35 @@ public class PathPattern implements Comparable<PathPattern> {
}
/**
* Match the beginning of the given path and return the remaining portion of
* the path not covered by this pattern. This is useful for matching through
* nested routes where the path is matched incrementally at each level.
* Match this pattern to the given URI path and return extracted URI template
* variables as well as path parameters (matrix variables).
* @param pathContainer the candidate path to attempt to match against
* @return info object with the extracted variables
* @throws IllegalStateException if the path does not match the pattern
*/
public PathMatchInfo matchAndExtract(PathContainer pathContainer) {
MatchingContext matchingContext = new MatchingContext(pathContainer, true);
if (this.head != null && this.head.matches(0, matchingContext)) {
return matchingContext.getPathMatchResult();
}
else if (!hasLength(pathContainer)) {
return PathMatchInfo.EMPTY;
}
else {
throw new IllegalStateException(
"Pattern \"" + this + "\" is not a match for \"" + pathContainer.value() + "\"");
}
}
/**
* Match the beginning of the given path and return the remaining portion
* not covered by this pattern. This is useful for matching nested routes
* where the path is matched incrementally at each level.
* @param pathContainer the candidate path to attempt to match against
* @return info object with the match result or {@code null} for no match
*/
@Nullable
public PathRemainingMatchInfo getPathRemaining(PathContainer pathContainer) {
public PathRemainingMatchInfo matchStartOfPath(PathContainer pathContainer) {
if (this.head == null) {
return new PathRemainingMatchInfo(pathContainer);
}
@ -205,49 +226,12 @@ public class PathPattern implements Comparable<PathPattern> {
}
else {
info = new PathRemainingMatchInfo(pathContainer.subPath(matchingContext.remainingPathIndex),
matchingContext.getPathMatchResult());
matchingContext.getPathMatchResult());
}
return info;
}
}
/**
* @param pathContainer the path to check against the pattern
* @return true if the pattern matches as much of the path as is supplied
*/
public boolean matchStart(PathContainer pathContainer) {
if (this.head == null) {
return !hasLength(pathContainer);
}
else if (!hasLength(pathContainer)) {
return true;
}
MatchingContext matchingContext = new MatchingContext(pathContainer, false);
matchingContext.setMatchStartMatching(true);
return this.head.matches(0, matchingContext);
}
/**
* Match this pattern to the given URI path and extract URI template
* variables as well as path parameters (matrix variables).
* @param pathContainer the candidate path to attempt to match against
* @return info object with the extracted variables
* @throws IllegalStateException if the path does not match the pattern
*/
public PathMatchInfo matchAndExtract(PathContainer pathContainer) {
MatchingContext matchingContext = new MatchingContext(pathContainer, true);
if (this.head != null && this.head.matches(0, matchingContext)) {
return matchingContext.getPathMatchResult();
}
else if (!hasLength(pathContainer)) {
return PathMatchInfo.EMPTY;
}
else {
throw new IllegalStateException(
"Pattern \"" + this + "\" is not a match for \"" + pathContainer.value() + "\"");
}
}
/**
* Determine the pattern-mapped part for the given path.
* <p>For example: <ul>
@ -266,7 +250,7 @@ public class PathPattern implements Comparable<PathPattern> {
// TODO: implement extractPathWithinPattern for PathContainer
// TODO: also see this from PathPattern javadoc + align behavior with getPathRemaining instead:
// TODO: align behavior with matchStartOfPath with regards to this:
// As per the contract on {@link PathMatcher}, this method will trim leading/trailing
// separators. It will also remove duplicate separators in the returned path.

View File

@ -320,40 +320,40 @@ public class PathPatternTests {
@Test
public void pathRemainingCornerCases_spr15336() {
// No match when the literal path element is a longer form of the segment in the pattern
assertNull(parse("/foo").getPathRemaining(toPathContainer("/footastic/bar")));
assertNull(parse("/f?o").getPathRemaining(toPathContainer("/footastic/bar")));
assertNull(parse("/f*o*p").getPathRemaining(toPathContainer("/flooptastic/bar")));
assertNull(parse("/{abc}abc").getPathRemaining(toPathContainer("/xyzabcbar/bar")));
assertNull(parse("/foo").matchStartOfPath(toPathContainer("/footastic/bar")));
assertNull(parse("/f?o").matchStartOfPath(toPathContainer("/footastic/bar")));
assertNull(parse("/f*o*p").matchStartOfPath(toPathContainer("/flooptastic/bar")));
assertNull(parse("/{abc}abc").matchStartOfPath(toPathContainer("/xyzabcbar/bar")));
// With a /** on the end have to check if there is any more data post
// 'the match' it starts with a separator
assertNull(parse("/resource/**").getPathRemaining(toPathContainer("/resourceX")));
assertEquals("",parse("/resource/**").getPathRemaining(toPathContainer("/resource")).getPathRemaining().value());
assertNull(parse("/resource/**").matchStartOfPath(toPathContainer("/resourceX")));
assertEquals("",parse("/resource/**").matchStartOfPath(toPathContainer("/resource")).getPathRemaining().value());
// Similar to above for the capture-the-rest variant
assertNull(parse("/resource/{*foo}").getPathRemaining(toPathContainer("/resourceX")));
assertEquals("",parse("/resource/{*foo}").getPathRemaining(toPathContainer("/resource")).getPathRemaining().value());
assertNull(parse("/resource/{*foo}").matchStartOfPath(toPathContainer("/resourceX")));
assertEquals("",parse("/resource/{*foo}").matchStartOfPath(toPathContainer("/resource")).getPathRemaining().value());
PathPattern.PathRemainingMatchInfo pri = parse("/aaa/{bbb}/c?d/e*f/*/g").getPathRemaining(toPathContainer("/aaa/b/ccd/ef/x/g/i"));
PathPattern.PathRemainingMatchInfo pri = parse("/aaa/{bbb}/c?d/e*f/*/g").matchStartOfPath(toPathContainer("/aaa/b/ccd/ef/x/g/i"));
assertNotNull(pri);
assertEquals("/i",pri.getPathRemaining().value());
assertEquals("b",pri.getUriVariables().get("bbb"));
pri = parse("/aaa/{bbb}/c?d/e*f/*/g/").getPathRemaining(toPathContainer("/aaa/b/ccd/ef/x/g/i"));
pri = parse("/aaa/{bbb}/c?d/e*f/*/g/").matchStartOfPath(toPathContainer("/aaa/b/ccd/ef/x/g/i"));
assertNotNull(pri);
assertEquals("i",pri.getPathRemaining().value());
assertEquals("b",pri.getUriVariables().get("bbb"));
pri = parse("/{aaa}_{bbb}/e*f/{x}/g").getPathRemaining(toPathContainer("/aa_bb/ef/x/g/i"));
pri = parse("/{aaa}_{bbb}/e*f/{x}/g").matchStartOfPath(toPathContainer("/aa_bb/ef/x/g/i"));
assertNotNull(pri);
assertEquals("/i",pri.getPathRemaining().value());
assertEquals("aa",pri.getUriVariables().get("aaa"));
assertEquals("bb",pri.getUriVariables().get("bbb"));
assertEquals("x",pri.getUriVariables().get("x"));
assertNull(parse("/a/b").getPathRemaining(toPathContainer("")));
assertEquals("/a/b",parse("").getPathRemaining(toPathContainer("/a/b")).getPathRemaining().value());
assertEquals("",parse("").getPathRemaining(toPathContainer("")).getPathRemaining().value());
assertNull(parse("/a/b").matchStartOfPath(toPathContainer("")));
assertEquals("/a/b",parse("").matchStartOfPath(toPathContainer("/a/b")).getPathRemaining().value());
assertEquals("",parse("").matchStartOfPath(toPathContainer("")).getPathRemaining().value());
}
@Test
@ -576,114 +576,6 @@ public class PathPatternTests {
assertEquals("def",pri.getUriVariables().get("foo"));
}
@Test
public void matchStart() {
PathPatternParser ppp = new PathPatternParser();
ppp.setMatchOptionalTrailingSeparator(false);
PathPattern pp = ppp.parse("test");
assertFalse(pp.matchStart(PathContainer.parsePath("test/")));
checkStartNoMatch("test/*/","test//");
checkStartMatches("test/*","test/abc");
checkStartMatches("test/*/def","test/abc/def");
checkStartNoMatch("test/*/def","test//");
checkStartNoMatch("test/*/def","test//def");
checkStartMatches("test/{a}_{b}/foo", "test/a_b");
checkStartMatches("test/?/abc", "test/a");
checkStartMatches("test/{*foobar}", "test/");
checkStartMatches("test/*/bar", "test/a");
checkStartMatches("test/{foo}/bar", "test/abc");
checkStartMatches("test//foo", "test//");
checkStartMatches("test/foo", "test/");
checkStartMatches("test/*", "test/");
checkStartMatches("test", "test");
checkStartNoMatch("test", "tes");
checkStartMatches("test/", "test");
// test exact matching
checkStartMatches("test", "test");
checkStartMatches("/test", "/test");
checkStartNoMatch("/test.jpg", "test.jpg");
checkStartNoMatch("test", "/test");
checkStartNoMatch("/test", "test");
// test matching with ?'s
checkStartMatches("t?st", "test");
checkStartMatches("??st", "test");
checkStartMatches("tes?", "test");
checkStartMatches("te??", "test");
checkStartMatches("?es?", "test");
checkStartNoMatch("tes?", "tes");
checkStartNoMatch("tes?", "testt");
checkStartNoMatch("tes?", "tsst");
// test matching with *'s
checkStartMatches("*", "test");
checkStartMatches("test*", "test");
checkStartMatches("test*", "testTest");
checkStartMatches("test/*", "test/Test");
checkStartMatches("test/*", "test/t");
checkStartMatches("test/*", "test/");
checkStartMatches("*test*", "AnothertestTest");
checkStartMatches("*test", "Anothertest");
checkStartMatches("*.*", "test.");
checkStartMatches("*.*", "test.test");
checkStartMatches("*.*", "test.test.test");
checkStartMatches("test*aaa", "testblaaaa");
checkStartNoMatch("test*", "tst");
checkStartMatches("test*", "test/"); // trailing slash is optional
checkStartMatches("test*", "test");
checkStartNoMatch("test*", "tsttest");
checkStartNoMatch("test*", "test/t");
checkStartMatches("test/*", "test");
checkStartMatches("test/t*.txt", "test");
checkStartNoMatch("*test*", "tsttst");
checkStartNoMatch("*test", "tsttst");
checkStartNoMatch("*.*", "tsttst");
checkStartNoMatch("test*aaa", "test");
checkStartNoMatch("test*aaa", "testblaaab");
// test matching with ?'s and /'s
checkStartMatches("/?", "/a");
checkStartMatches("/?/a", "/a/a");
checkStartMatches("/a/?", "/a/b");
checkStartMatches("/??/a", "/aa/a");
checkStartMatches("/a/??", "/a/bb");
checkStartMatches("/?", "/a");
checkStartMatches("/**", "/testing/testing");
checkStartMatches("/*/**", "/testing/testing");
checkStartMatches("test*/**", "test/");
checkStartMatches("test*/**", "test/t");
checkStartMatches("/bla*bla/test", "/blaXXXbla/test");
checkStartMatches("/*bla/test", "/XXXbla/test");
checkStartNoMatch("/bla*bla/test", "/blaXXXbl/test");
checkStartNoMatch("/*bla/test", "XXXblab/test");
checkStartNoMatch("/*bla/test", "XXXbl/test");
checkStartNoMatch("/????", "/bala/bla");
checkStartMatches("/*bla*/*/bla/**",
"/XXXblaXXXX/testing/bla/testing/testing/");
checkStartMatches("/*bla*/*/bla/*",
"/XXXblaXXXX/testing/bla/testing");
checkStartMatches("/*bla*/*/bla/**",
"/XXXblaXXXX/testing/bla/testing/testing");
checkStartMatches("/*bla*/*/bla/**",
"/XXXblaXXXX/testing/bla/testing/testing.jpg");
checkStartMatches("/abc/{foo}", "/abc/def");
checkStartMatches("/abc/{foo}", "/abc/def/"); // trailing slash is optional
checkStartMatches("/abc/{foo}/", "/abc/def/");
checkStartNoMatch("/abc/{foo}/", "/abc/def/ghi");
checkStartMatches("/abc/{foo}/", "/abc/def");
checkStartMatches("", "");
checkStartMatches("", null);
checkStartMatches("/abc", null);
}
@Test
public void caseSensitivity() {
PathPatternParser pp = new PathPatternParser();
@ -1258,20 +1150,6 @@ public class PathPatternTests {
assertTrue(p.matches(pc));
}
private void checkStartNoMatch(String uriTemplate, String path) {
PathPatternParser p = new PathPatternParser();
p.setMatchOptionalTrailingSeparator(true);
PathPattern pattern = p.parse(uriTemplate);
assertFalse(pattern.matchStart(toPathContainer(path)));
}
private void checkStartMatches(String uriTemplate, String path) {
PathPatternParser p = new PathPatternParser();
p.setMatchOptionalTrailingSeparator(true);
PathPattern pattern = p.parse(uriTemplate);
assertTrue(pattern.matchStart(toPathContainer(path)));
}
private void checkNoMatch(String uriTemplate, String path) {
PathPatternParser p = new PathPatternParser();
PathPattern pattern = p.parse(uriTemplate);
@ -1309,11 +1187,11 @@ public class PathPatternTests {
}
private PathRemainingMatchInfo getPathRemaining(String pattern, String path) {
return parse(pattern).getPathRemaining(toPathContainer(path));
return parse(pattern).matchStartOfPath(toPathContainer(path));
}
private PathRemainingMatchInfo getPathRemaining(PathPattern pattern, String path) {
return pattern.getPathRemaining(toPathContainer(path));
return pattern.matchStartOfPath(toPathContainer(path));
}
private PathPattern.PathMatchInfo matchAndExtract(PathPattern p, String path) {

View File

@ -356,7 +356,7 @@ public abstract class RequestPredicates {
@Override
public Optional<ServerRequest> nest(ServerRequest request) {
return Optional.ofNullable(this.pattern.getPathRemaining(request.pathContainer()))
return Optional.ofNullable(this.pattern.matchStartOfPath(request.pathContainer()))
.map(info -> {
mergeTemplateVariables(request, info.getUriVariables());
return new SubPathServerRequestWrapper(request, info);

View File

@ -33,6 +33,7 @@ import org.springframework.http.InvalidMediaTypeException;
import org.springframework.http.MediaType;
import org.springframework.http.server.PathContainer;
import org.springframework.http.server.reactive.ServerHttpRequest;
import org.springframework.util.Assert;
import org.springframework.util.MultiValueMap;
import org.springframework.web.method.HandlerMethod;
import org.springframework.web.reactive.HandlerMapping;
@ -113,6 +114,8 @@ public abstract class RequestMappingInfoHandlerMapping extends AbstractHandlerMe
else {
bestPattern = patterns.iterator().next();
PathPattern.PathMatchInfo result = bestPattern.matchAndExtract(lookupPath);
Assert.notNull(result, () ->
"Expected bestPattern: " + bestPattern + " to match lookupPath " + lookupPath);
uriVariables = result.getUriVariables();
matrixVariables = result.getMatrixVariables();
}

View File

@ -86,7 +86,7 @@ public class WebFluxConfigurationSupportTests {
@Test
public void requestMappingHandlerMapping() throws Exception {
ApplicationContext context = loadConfig(WebFluxConfig.class);
final Field trailingSlashField = ReflectionUtils.findField(PathPatternParser.class, "matchOptionalTrailingSlash");
final Field trailingSlashField = ReflectionUtils.findField(PathPatternParser.class, "matchOptionalTrailingSeparator");
ReflectionUtils.makeAccessible(trailingSlashField);
String name = "requestMappingHandlerMapping";
@ -111,7 +111,7 @@ public class WebFluxConfigurationSupportTests {
@Test
public void customPathMatchConfig() throws Exception {
ApplicationContext context = loadConfig(CustomPatchMatchConfig.class);
final Field trailingSlashField = ReflectionUtils.findField(PathPatternParser.class, "matchOptionalTrailingSlash");
final Field trailingSlashField = ReflectionUtils.findField(PathPatternParser.class, "matchOptionalTrailingSeparator");
ReflectionUtils.makeAccessible(trailingSlashField);
String name = "requestMappingHandlerMapping";