Skip to content

Commit b6d1fd9

Browse files
committed
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.
1 parent 1794f1c commit b6d1fd9

File tree

4 files changed

+27
-133
lines changed

4 files changed

+27
-133
lines changed

spring-web/src/main/java/org/springframework/web/util/pattern/InternalPathPatternParser.java

+11-30
Original file line numberDiff line numberDiff line change
@@ -33,19 +33,11 @@
3333
*/
3434
class InternalPathPatternParser {
3535

36-
private PathPatternParser parser;
36+
private final PathPatternParser parser;
3737

3838
// The expected path separator to split path elements during parsing
39-
char separator = PathPatternParser.DEFAULT_SEPARATOR;
39+
private final char separator = '/';
4040

41-
// Is the parser producing case sensitive PathPattern matchers
42-
boolean caseSensitive = true;
43-
44-
// If true the PathPatterns produced by the parser will allow patterns
45-
// that don't have a trailing slash to match paths that may or may not
46-
// have a trailing slash
47-
private boolean matchOptionalTrailingSlash = false;
48-
4941
// The input data for parsing
5042
private char[] pathPatternData = new char[0];
5143

@@ -90,19 +82,8 @@ class InternalPathPatternParser {
9082
PathElement currentPE;
9183

9284

93-
/**
94-
* @param separator the path separator to look for when parsing
95-
* @param caseSensitive true if PathPatterns should be sensitive to case
96-
* @param matchOptionalTrailingSlash true if patterns without a trailing slash
97-
* can match paths that do have a trailing slash
98-
*/
99-
public InternalPathPatternParser(char separator, boolean caseSensitive, boolean matchOptionalTrailingSlash) {
100-
this.separator = separator;
101-
this.caseSensitive = caseSensitive;
102-
this.matchOptionalTrailingSlash = matchOptionalTrailingSlash;
103-
this.parser = new PathPatternParser(this.separator);
104-
this.parser.setCaseSensitive(this.caseSensitive);
105-
this.parser.setMatchOptionalTrailingSlash(this.matchOptionalTrailingSlash);
85+
InternalPathPatternParser(PathPatternParser parentParser) {
86+
this.parser = parentParser;
10687
}
10788

10889

@@ -210,8 +191,8 @@ else if ((this.pos > (this.variableCaptureStart + 1 + (this.isCaptureTheRestVari
210191
if (this.pathElementStart != -1) {
211192
pushPathElement(createPathElement());
212193
}
213-
return new PathPattern(
214-
pathPattern, this.parser, this.headPE, this.separator, this.caseSensitive, this.matchOptionalTrailingSlash);
194+
return new PathPattern(pathPattern, this.parser, this.headPE, this.separator,
195+
this.parser.isCaseSensitive(), this.parser.isMatchOptionalTrailingSlash());
215196
}
216197

217198
/**
@@ -347,7 +328,7 @@ private PathElement createPathElement() {
347328
// It is a full capture of this element (possibly with constraint), for example: /foo/{abc}/
348329
try {
349330
newPE = new CaptureVariablePathElement(this.pathElementStart, getPathElementText(),
350-
this.caseSensitive, this.separator);
331+
this.parser.isCaseSensitive(), this.separator);
351332
}
352333
catch (PatternSyntaxException pse) {
353334
throw new PatternParseException(pse,
@@ -364,7 +345,7 @@ private PathElement createPathElement() {
364345
PatternMessage.CAPTURE_ALL_IS_STANDALONE_CONSTRUCT);
365346
}
366347
RegexPathElement newRegexSection = new RegexPathElement(this.pathElementStart,
367-
getPathElementText(), this.caseSensitive,
348+
getPathElementText(), this.parser.isCaseSensitive(),
368349
this.pathPatternData, this.separator);
369350
for (String variableName : newRegexSection.getVariableNames()) {
370351
recordCapturedVariable(this.pathElementStart, variableName);
@@ -379,16 +360,16 @@ private PathElement createPathElement() {
379360
}
380361
else {
381362
newPE = new RegexPathElement(this.pathElementStart, getPathElementText(),
382-
this.caseSensitive, this.pathPatternData, this.separator);
363+
this.parser.isCaseSensitive(), this.pathPatternData, this.separator);
383364
}
384365
}
385366
else if (this.singleCharWildcardCount != 0) {
386367
newPE = new SingleCharWildcardedPathElement(this.pathElementStart, getPathElementText(),
387-
this.singleCharWildcardCount, this.caseSensitive, this.separator);
368+
this.singleCharWildcardCount, this.parser.isCaseSensitive(), this.separator);
388369
}
389370
else {
390371
newPE = new LiteralPathElement(this.pathElementStart, getPathElementText(),
391-
this.caseSensitive, this.separator);
372+
this.parser.isCaseSensitive(), this.separator);
392373
}
393374
}
394375

spring-web/src/main/java/org/springframework/web/util/pattern/PathPatternParser.java

+15-29
Original file line numberDiff line numberDiff line change
@@ -32,34 +32,11 @@
3232
*/
3333
public class PathPatternParser {
3434

35-
public final static char DEFAULT_SEPARATOR = '/';
36-
37-
38-
private char separator = DEFAULT_SEPARATOR;
39-
4035
private boolean matchOptionalTrailingSlash = true;
4136

4237
private boolean caseSensitive = true;
4338

4439

45-
/**
46-
* Create a path pattern parser that will use the default separator '/'
47-
* when parsing patterns.
48-
* @see #DEFAULT_SEPARATOR
49-
*/
50-
public PathPatternParser() {
51-
}
52-
53-
/**
54-
* Create a path pattern parser that will use the supplied separator when
55-
* parsing patterns.
56-
* @param separator the separator expected to divide pattern elements
57-
*/
58-
public PathPatternParser(char separator) {
59-
this.separator = separator;
60-
}
61-
62-
6340
/**
6441
* Whether a {@link PathPattern} produced by this parser should should
6542
* automatically match request paths with a trailing slash.
@@ -75,6 +52,13 @@ public void setMatchOptionalTrailingSlash(boolean matchOptionalTrailingSlash) {
7552
this.matchOptionalTrailingSlash = matchOptionalTrailingSlash;
7653
}
7754

55+
/**
56+
* Whether optional trailing slashing match is enabled.
57+
*/
58+
public boolean isMatchOptionalTrailingSlash() {
59+
return this.matchOptionalTrailingSlash;
60+
}
61+
7862
/**
7963
* Whether path pattern matching should be case-sensitive.
8064
* <p>The default is {@code true}.
@@ -83,6 +67,13 @@ public void setCaseSensitive(boolean caseSensitive) {
8367
this.caseSensitive = caseSensitive;
8468
}
8569

70+
/**
71+
* Whether case-sensitive pattern matching is enabled.
72+
*/
73+
public boolean isCaseSensitive() {
74+
return this.caseSensitive;
75+
}
76+
8677

8778
/**
8879
* Process the path pattern data, a character at a time, breaking it into
@@ -95,12 +86,7 @@ public void setCaseSensitive(boolean caseSensitive) {
9586
* @throws PatternParseException in case of parse errors
9687
*/
9788
public PathPattern parse(String pathPattern) throws PatternParseException {
98-
return createDelegate().parse(pathPattern);
99-
}
100-
101-
private InternalPathPatternParser createDelegate() {
102-
return new InternalPathPatternParser(
103-
this.separator, this.caseSensitive, this.matchOptionalTrailingSlash);
89+
return new InternalPathPatternParser(this).parse(pathPattern);
10490
}
10591

10692
}

spring-web/src/test/java/org/springframework/web/util/pattern/PathPatternParserTests.java

-6
Original file line numberDiff line numberDiff line change
@@ -114,12 +114,6 @@ public void equalsAndHashcode() {
114114
pp2 = caseSensitiveParser.parse("/abc");
115115
assertFalse(pp1.equals(pp2));
116116
assertNotEquals(pp1.hashCode(), pp2.hashCode());
117-
118-
PathPatternParser alternateSeparatorParser = new PathPatternParser(':');
119-
pp1 = caseInsensitiveParser.parse("abc");
120-
pp2 = alternateSeparatorParser.parse("abc");
121-
assertFalse(pp1.equals(pp2));
122-
assertNotEquals(pp1.hashCode(), pp2.hashCode());
123117
}
124118

125119
@Test

spring-web/src/test/java/org/springframework/web/util/pattern/PathPatternTests.java

+1-68
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@
2424
import java.util.Map;
2525

2626
import org.hamcrest.Matchers;
27-
import org.junit.Ignore;
2827
import org.junit.Rule;
2928
import org.junit.Test;
3029
import org.junit.rules.ExpectedException;
@@ -51,9 +50,6 @@
5150
*/
5251
public class PathPatternTests {
5352

54-
private char separator = PathPatternParser.DEFAULT_SEPARATOR;
55-
56-
5753
@Test
5854
public void pathContainer() {
5955
assertEquals("[/][abc][/][def]",elementsToString(toPathContainer("/abc/def").elements()));
@@ -770,69 +766,6 @@ public void caseSensitivity() {
770766
assertMatches(p,"bAb");
771767
}
772768

773-
@Ignore
774-
@Test
775-
public void alternativeDelimiter() {
776-
try {
777-
this.separator = '.';
778-
779-
// test exact matching
780-
// checkMatches("test", "test");
781-
// checkMatches(".test", ".test");
782-
// checkNoMatch(".test/jpg", "test/jpg");
783-
// checkNoMatch("test", ".test");
784-
// checkNoMatch(".test", "test");
785-
786-
// test matching with ?'s
787-
checkMatches("t?st", "test");
788-
checkMatches("??st", "test");
789-
checkMatches("tes?", "test");
790-
checkMatches("te??", "test");
791-
checkMatches("?es?", "test");
792-
checkNoMatch("tes?", "tes");
793-
checkNoMatch("tes?", "testt");
794-
checkNoMatch("tes?", "tsst");
795-
796-
// test matching with *'s
797-
checkMatches("*", "test");
798-
checkMatches("test*", "test");
799-
checkMatches("test*", "testTest");
800-
checkMatches("*test*", "AnothertestTest");
801-
checkMatches("*test", "Anothertest");
802-
checkMatches("*/*", "test/");
803-
checkMatches("*/*", "test/test");
804-
checkMatches("*/*", "test/test/test");
805-
checkMatches("test*aaa", "testblaaaa");
806-
checkNoMatch("test*", "tst");
807-
checkNoMatch("test*", "tsttest");
808-
checkNoMatch("*test*", "tsttst");
809-
checkNoMatch("*test", "tsttst");
810-
checkNoMatch("*/*", "tsttst");
811-
checkNoMatch("test*aaa", "test");
812-
checkNoMatch("test*aaa", "testblaaab");
813-
814-
// test matching with ?'s and .'s
815-
checkMatches(".?", ".a");
816-
checkMatches(".?.a", ".a.a");
817-
checkMatches(".a.?", ".a.b");
818-
checkMatches(".??.a", ".aa.a");
819-
checkMatches(".a.??", ".a.bb");
820-
checkMatches(".?", ".a");
821-
822-
// test matching with **'s
823-
checkMatches(".**", ".testing.testing");
824-
checkMatches(".*.**", ".testing.testing");
825-
checkMatches(".bla*bla.test", ".blaXXXbla.test");
826-
checkMatches(".*bla.test", ".XXXbla.test");
827-
checkNoMatch(".bla*bla.test", ".blaXXXbl.test");
828-
checkNoMatch(".*bla.test", "XXXblab.test");
829-
checkNoMatch(".*bla.test", "XXXbl.test");
830-
}
831-
finally {
832-
this.separator = PathPatternParser.DEFAULT_SEPARATOR;
833-
}
834-
}
835-
836769
@Test
837770
public void extractPathWithinPattern_spr15259() {
838771
checkExtractPathWithinPattern("/**","//","/");
@@ -1319,7 +1252,7 @@ public static PathContainer toPathContainer(String path) {
13191252
}
13201253

13211254
private void checkMatches(String uriTemplate, String path) {
1322-
PathPatternParser parser = new PathPatternParser(this.separator);
1255+
PathPatternParser parser = new PathPatternParser();
13231256
parser.setMatchOptionalTrailingSlash(true);
13241257
PathPattern p = parser.parse(uriTemplate);
13251258
PathContainer pc = toPathContainer(path);

0 commit comments

Comments
 (0)