From b6d1fd9d222f324ef9fdee4ae106937653584d6b Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Tue, 1 Aug 2017 22:52:18 +0200 Subject: [PATCH] Minor refactoring in PathPatternParser Remove the separator constructor argument (but preserve internal functionality) now that PathPatternParser is more explicitly purposed for URL paths and in any case the use of an alternate separator would also requires a similar input option on the PathContainer parsing side. --- .../pattern/InternalPathPatternParser.java | 41 +++-------- .../web/util/pattern/PathPatternParser.java | 44 ++++-------- .../util/pattern/PathPatternParserTests.java | 6 -- .../web/util/pattern/PathPatternTests.java | 69 +------------------ 4 files changed, 27 insertions(+), 133 deletions(-) diff --git a/spring-web/src/main/java/org/springframework/web/util/pattern/InternalPathPatternParser.java b/spring-web/src/main/java/org/springframework/web/util/pattern/InternalPathPatternParser.java index 3ec91daa8f8..082a824fe3a 100644 --- a/spring-web/src/main/java/org/springframework/web/util/pattern/InternalPathPatternParser.java +++ b/spring-web/src/main/java/org/springframework/web/util/pattern/InternalPathPatternParser.java @@ -33,19 +33,11 @@ import org.springframework.web.util.pattern.PatternParseException.PatternMessage */ class InternalPathPatternParser { - private PathPatternParser parser; + private final PathPatternParser parser; // The expected path separator to split path elements during parsing - char separator = PathPatternParser.DEFAULT_SEPARATOR; + private final char separator = '/'; - // Is the parser producing case sensitive PathPattern matchers - boolean caseSensitive = true; - - // If true the PathPatterns produced by the parser will allow patterns - // that don't have a trailing slash to match paths that may or may not - // have a trailing slash - private boolean matchOptionalTrailingSlash = false; - // The input data for parsing private char[] pathPatternData = new char[0]; @@ -90,19 +82,8 @@ class InternalPathPatternParser { PathElement currentPE; - /** - * @param separator the path separator to look for when parsing - * @param caseSensitive true if PathPatterns should be sensitive to case - * @param matchOptionalTrailingSlash true if patterns without a trailing slash - * can match paths that do have a trailing slash - */ - public InternalPathPatternParser(char separator, boolean caseSensitive, boolean matchOptionalTrailingSlash) { - this.separator = separator; - this.caseSensitive = caseSensitive; - this.matchOptionalTrailingSlash = matchOptionalTrailingSlash; - this.parser = new PathPatternParser(this.separator); - this.parser.setCaseSensitive(this.caseSensitive); - this.parser.setMatchOptionalTrailingSlash(this.matchOptionalTrailingSlash); + InternalPathPatternParser(PathPatternParser parentParser) { + this.parser = parentParser; } @@ -210,8 +191,8 @@ class InternalPathPatternParser { if (this.pathElementStart != -1) { pushPathElement(createPathElement()); } - return new PathPattern( - pathPattern, this.parser, this.headPE, this.separator, this.caseSensitive, this.matchOptionalTrailingSlash); + return new PathPattern(pathPattern, this.parser, this.headPE, this.separator, + this.parser.isCaseSensitive(), this.parser.isMatchOptionalTrailingSlash()); } /** @@ -347,7 +328,7 @@ class InternalPathPatternParser { // It is a full capture of this element (possibly with constraint), for example: /foo/{abc}/ try { newPE = new CaptureVariablePathElement(this.pathElementStart, getPathElementText(), - this.caseSensitive, this.separator); + this.parser.isCaseSensitive(), this.separator); } catch (PatternSyntaxException pse) { throw new PatternParseException(pse, @@ -364,7 +345,7 @@ class InternalPathPatternParser { PatternMessage.CAPTURE_ALL_IS_STANDALONE_CONSTRUCT); } RegexPathElement newRegexSection = new RegexPathElement(this.pathElementStart, - getPathElementText(), this.caseSensitive, + getPathElementText(), this.parser.isCaseSensitive(), this.pathPatternData, this.separator); for (String variableName : newRegexSection.getVariableNames()) { recordCapturedVariable(this.pathElementStart, variableName); @@ -379,16 +360,16 @@ class InternalPathPatternParser { } else { newPE = new RegexPathElement(this.pathElementStart, getPathElementText(), - this.caseSensitive, this.pathPatternData, this.separator); + this.parser.isCaseSensitive(), this.pathPatternData, this.separator); } } else if (this.singleCharWildcardCount != 0) { newPE = new SingleCharWildcardedPathElement(this.pathElementStart, getPathElementText(), - this.singleCharWildcardCount, this.caseSensitive, this.separator); + this.singleCharWildcardCount, this.parser.isCaseSensitive(), this.separator); } else { newPE = new LiteralPathElement(this.pathElementStart, getPathElementText(), - this.caseSensitive, this.separator); + this.parser.isCaseSensitive(), this.separator); } } diff --git a/spring-web/src/main/java/org/springframework/web/util/pattern/PathPatternParser.java b/spring-web/src/main/java/org/springframework/web/util/pattern/PathPatternParser.java index 4a8761e7975..7dfdb06d1f3 100644 --- a/spring-web/src/main/java/org/springframework/web/util/pattern/PathPatternParser.java +++ b/spring-web/src/main/java/org/springframework/web/util/pattern/PathPatternParser.java @@ -32,34 +32,11 @@ package org.springframework.web.util.pattern; */ public class PathPatternParser { - public final static char DEFAULT_SEPARATOR = '/'; - - - private char separator = DEFAULT_SEPARATOR; - private boolean matchOptionalTrailingSlash = true; private boolean caseSensitive = true; - /** - * Create a path pattern parser that will use the default separator '/' - * when parsing patterns. - * @see #DEFAULT_SEPARATOR - */ - public PathPatternParser() { - } - - /** - * Create a path pattern parser that will use the supplied separator when - * parsing patterns. - * @param separator the separator expected to divide pattern elements - */ - public PathPatternParser(char separator) { - this.separator = separator; - } - - /** * Whether a {@link PathPattern} produced by this parser should should * automatically match request paths with a trailing slash. @@ -75,6 +52,13 @@ public class PathPatternParser { this.matchOptionalTrailingSlash = matchOptionalTrailingSlash; } + /** + * Whether optional trailing slashing match is enabled. + */ + public boolean isMatchOptionalTrailingSlash() { + return this.matchOptionalTrailingSlash; + } + /** * Whether path pattern matching should be case-sensitive. *

The default is {@code true}. @@ -83,6 +67,13 @@ public class PathPatternParser { this.caseSensitive = caseSensitive; } + /** + * Whether case-sensitive pattern matching is enabled. + */ + public boolean isCaseSensitive() { + return this.caseSensitive; + } + /** * Process the path pattern data, a character at a time, breaking it into @@ -95,12 +86,7 @@ public class PathPatternParser { * @throws PatternParseException in case of parse errors */ public PathPattern parse(String pathPattern) throws PatternParseException { - return createDelegate().parse(pathPattern); - } - - private InternalPathPatternParser createDelegate() { - return new InternalPathPatternParser( - this.separator, this.caseSensitive, this.matchOptionalTrailingSlash); + return new InternalPathPatternParser(this).parse(pathPattern); } } diff --git a/spring-web/src/test/java/org/springframework/web/util/pattern/PathPatternParserTests.java b/spring-web/src/test/java/org/springframework/web/util/pattern/PathPatternParserTests.java index f9f9ce211c0..3395d1ed9cf 100644 --- a/spring-web/src/test/java/org/springframework/web/util/pattern/PathPatternParserTests.java +++ b/spring-web/src/test/java/org/springframework/web/util/pattern/PathPatternParserTests.java @@ -114,12 +114,6 @@ public class PathPatternParserTests { pp2 = caseSensitiveParser.parse("/abc"); assertFalse(pp1.equals(pp2)); assertNotEquals(pp1.hashCode(), pp2.hashCode()); - - PathPatternParser alternateSeparatorParser = new PathPatternParser(':'); - pp1 = caseInsensitiveParser.parse("abc"); - pp2 = alternateSeparatorParser.parse("abc"); - assertFalse(pp1.equals(pp2)); - assertNotEquals(pp1.hashCode(), pp2.hashCode()); } @Test diff --git a/spring-web/src/test/java/org/springframework/web/util/pattern/PathPatternTests.java b/spring-web/src/test/java/org/springframework/web/util/pattern/PathPatternTests.java index d6c7a4938ca..4e3fe36a671 100644 --- a/spring-web/src/test/java/org/springframework/web/util/pattern/PathPatternTests.java +++ b/spring-web/src/test/java/org/springframework/web/util/pattern/PathPatternTests.java @@ -24,7 +24,6 @@ import java.util.List; import java.util.Map; import org.hamcrest.Matchers; -import org.junit.Ignore; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; @@ -51,9 +50,6 @@ import static org.junit.Assert.fail; */ public class PathPatternTests { - private char separator = PathPatternParser.DEFAULT_SEPARATOR; - - @Test public void pathContainer() { assertEquals("[/][abc][/][def]",elementsToString(toPathContainer("/abc/def").elements())); @@ -770,69 +766,6 @@ public class PathPatternTests { assertMatches(p,"bAb"); } - @Ignore - @Test - public void alternativeDelimiter() { - try { - this.separator = '.'; - - // test exact matching -// checkMatches("test", "test"); -// checkMatches(".test", ".test"); -// checkNoMatch(".test/jpg", "test/jpg"); -// checkNoMatch("test", ".test"); -// checkNoMatch(".test", "test"); - - // test matching with ?'s - checkMatches("t?st", "test"); - checkMatches("??st", "test"); - checkMatches("tes?", "test"); - checkMatches("te??", "test"); - checkMatches("?es?", "test"); - checkNoMatch("tes?", "tes"); - checkNoMatch("tes?", "testt"); - checkNoMatch("tes?", "tsst"); - - // test matching with *'s - checkMatches("*", "test"); - checkMatches("test*", "test"); - checkMatches("test*", "testTest"); - checkMatches("*test*", "AnothertestTest"); - checkMatches("*test", "Anothertest"); - checkMatches("*/*", "test/"); - checkMatches("*/*", "test/test"); - checkMatches("*/*", "test/test/test"); - checkMatches("test*aaa", "testblaaaa"); - checkNoMatch("test*", "tst"); - checkNoMatch("test*", "tsttest"); - checkNoMatch("*test*", "tsttst"); - checkNoMatch("*test", "tsttst"); - checkNoMatch("*/*", "tsttst"); - checkNoMatch("test*aaa", "test"); - checkNoMatch("test*aaa", "testblaaab"); - - // test matching with ?'s and .'s - checkMatches(".?", ".a"); - checkMatches(".?.a", ".a.a"); - checkMatches(".a.?", ".a.b"); - checkMatches(".??.a", ".aa.a"); - checkMatches(".a.??", ".a.bb"); - checkMatches(".?", ".a"); - - // test matching with **'s - checkMatches(".**", ".testing.testing"); - checkMatches(".*.**", ".testing.testing"); - checkMatches(".bla*bla.test", ".blaXXXbla.test"); - checkMatches(".*bla.test", ".XXXbla.test"); - checkNoMatch(".bla*bla.test", ".blaXXXbl.test"); - checkNoMatch(".*bla.test", "XXXblab.test"); - checkNoMatch(".*bla.test", "XXXbl.test"); - } - finally { - this.separator = PathPatternParser.DEFAULT_SEPARATOR; - } - } - @Test public void extractPathWithinPattern_spr15259() { checkExtractPathWithinPattern("/**","//","/"); @@ -1319,7 +1252,7 @@ public class PathPatternTests { } private void checkMatches(String uriTemplate, String path) { - PathPatternParser parser = new PathPatternParser(this.separator); + PathPatternParser parser = new PathPatternParser(); parser.setMatchOptionalTrailingSlash(true); PathPattern p = parser.parse(uriTemplate); PathContainer pc = toPathContainer(path);