Remove ParsingPathMatcher

Now that we also have RequestPath and PathContainer with the latter as
the required input, the ParsingPathMatcher adapter can be removed.

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

@@ -1,170 +0,0 @@
-/*
- * Copyright 2002-2017 the original author or authors.
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *      http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-package org.springframework.web.util.pattern;
-
-import java.util.Collections;
-import java.util.Comparator;
-import java.util.Map;
-import java.util.concurrent.ConcurrentHashMap;
-import java.util.concurrent.ConcurrentMap;
-import java.util.stream.Collectors;
-
-import org.springframework.http.server.PathContainer;
-import org.springframework.lang.Nullable;
-import org.springframework.util.PathMatcher;
-import org.springframework.web.util.pattern.PathPattern.PathMatchResult;
-
-/**
- * {@link PathMatcher} implementation for path patterns parsed
- * as {@link PathPatternParser} and compiled as {@link PathPattern}s.
- *
- * <p>Once parsed, {@link PathPattern}s are tailored for fast matching
- * and quick comparison.
- *
- * <p>Calls this {@link PathMatcher} implementation can lead to
- * {@link PatternParseException} if the provided patterns are
- * illegal.
- *
- * @author Andy Clement
- * @author Juergen Hoeller
- * @since 5.0
- * @see PathPattern
- */
-public class ParsingPathMatcher implements PathMatcher {
-
-	private final PathPatternParser parser = new PathPatternParser();
-
-	private final ConcurrentMap<String, PathPattern> cache = new ConcurrentHashMap<>(256);
-
-
-	@Override
-	public boolean isPattern(String path) {
-		// TODO crude, should be smarter, lookup pattern and ask it
-		return (path.indexOf('*') != -1 || path.indexOf('?') != -1);
-	}
-
-	@Override
-	public boolean match(String pattern, String path) {
-		PathPattern pathPattern = getPathPattern(pattern);
-		return pathPattern.matches(PathContainer.parsePath(path));
-	}
-
-	@Override
-	public boolean matchStart(String pattern, String path) {
-		PathPattern pathPattern = getPathPattern(pattern);
-		return pathPattern.matchStart(PathContainer.parsePath(path));
-	}
-
-	@Override
-	public String extractPathWithinPattern(String pattern, String path) {
-		PathPattern pathPattern = getPathPattern(pattern);
-		PathContainer pathContainer = PathContainer.parsePath(path);
-		return pathPattern.extractPathWithinPattern(pathContainer).value();
-	}
-
-	@Override
-	public Map<String, String> extractUriTemplateVariables(String pattern, String path) {
-		PathPattern pathPattern = getPathPattern(pattern);
-		PathContainer pathContainer = PathContainer.parsePath(path);
-		PathMatchResult results = pathPattern.matchAndExtract(pathContainer);
-		// Collapse PathMatchResults to simple value results
-		// TODO: (path parameters are lost in this translation)
-		if (results.getUriVariables().size() == 0) {
-			return Collections.emptyMap();
-		}
-		else {
-			return results.getUriVariables().entrySet().stream()
-					.collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));
-		}
-	}
-
-	@Override
-	public Comparator<String> getPatternComparator(String path) {
-		return new PathPatternStringComparatorConsideringPath(path);
-	}
-
-	@Override
-	public String combine(String pattern1, String pattern2) {
-		PathPattern pathPattern = getPathPattern(pattern1);
-		return pathPattern.combine(getPathPattern(pattern2)).getPatternString();
-	}
-
-	private PathPattern getPathPattern(String pattern) {
-		PathPattern pathPattern = this.cache.get(pattern);
-		if (pathPattern == null) {
-			pathPattern = this.parser.parse(pattern);
-			this.cache.put(pattern, pathPattern);
-		}
-		return pathPattern;
-	}
-
-
-	private class PathPatternStringComparatorConsideringPath implements Comparator<String> {
-
-		private final PatternComparatorConsideringPath ppcp;
-
-		public PathPatternStringComparatorConsideringPath(String path) {
-			this.ppcp = new PatternComparatorConsideringPath(path);
-		}
-
-		@Override
-		public int compare(@Nullable String o1, @Nullable String o2) {
-			if (o1 == null) {
-				return (o2 == null ? 0 : +1);
-			}
-			else if (o2 == null) {
-				return -1;
-			}
-			PathPattern p1 = getPathPattern(o1);
-			PathPattern p2 = getPathPattern(o2);
-			return this.ppcp.compare(p1, p2);
-		}
-	}
-
-
-	/**
-	 * {@link PathPattern} comparator that takes account of a specified
-	 * path and sorts anything that exactly matches it to be first.
-	 */
-	static class PatternComparatorConsideringPath implements Comparator<PathPattern> {
-
-		private final String path;
-
-		public PatternComparatorConsideringPath(String path) {
-			this.path = path;
-		}
-
-		@Override
-		public int compare(@Nullable PathPattern o1, @Nullable PathPattern o2) {
-			// Nulls get sorted to the end
-			if (o1 == null) {
-				return (o2 == null ? 0 : +1);
-			}
-			else if (o2 == null) {
-				return -1;
-			}
-			if (o1.getPatternString().equals(this.path)) {
-				return (o2.getPatternString().equals(this.path)) ? 0 : -1;
-			}
-			else if (o2.getPatternString().equals(this.path)) {
-				return +1;
-			}
-			return o1.compareTo(o2);
-		}
-	}
-
-}

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

@@ -464,19 +464,19 @@ public class PathPatternParserTests {
 	}
 
 	private void assertMatches(PathPattern pp, String path) {
-		assertTrue(pp.matches(PathPatternMatcherTests.toPathContainer(path)));
+		assertTrue(pp.matches(PathPatternTests.toPathContainer(path)));
 	}
 
 	private void assertNoMatch(PathPattern pp, String path) {
-		assertFalse(pp.matches(PathPatternMatcherTests.toPathContainer(path)));
+		assertFalse(pp.matches(PathPatternTests.toPathContainer(path)));
 	}
 	
 	private PathMatchResult matchAndExtract(PathPattern pp, String path) {
-		 return pp.matchAndExtract(PathPatternMatcherTests.toPathContainer(path));
+		 return pp.matchAndExtract(PathPatternTests.toPathContainer(path));
 	}
 
 	private PathContainer toPSC(String path) {
-		return PathPatternMatcherTests.toPathContainer(path);
+		return PathPatternTests.toPathContainer(path);
 	}
 
 }

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

@@ -49,7 +49,7 @@ import static org.junit.Assert.fail;
  *
  * @author Andy Clement
  */
-public class PathPatternMatcherTests {
+public class PathPatternTests {
 
 	private char separator = PathPatternParser.DEFAULT_SEPARATOR;
 
@@ -1072,11 +1072,10 @@ public class PathPatternMatcherTests {
 
 	@Test
 	public void patternComparator() {
-		Comparator<PathPattern> comparator = new ParsingPathMatcher.PatternComparatorConsideringPath("/hotels/new");
-
-		assertEquals(0, comparator.compare(null, null));
-		assertEquals(1, comparator.compare(null, parse("/hotels/new")));
-		assertEquals(-1, comparator.compare(parse("/hotels/new"), null));
+		Comparator<PathPattern> comparator = (p1, p2) -> {
+			int index = p1.compareTo(p2);
+			return (index != 0 ? index : p1.getPatternString().compareTo(p2.getPatternString()));
+		};
 
 		assertEquals(0, comparator.compare(parse("/hotels/new"), parse("/hotels/new")));
 
@@ -1110,8 +1109,9 @@ public class PathPatternMatcherTests {
 		assertEquals(-1, comparator.compare(parse("/hotels/*"), parse("/hotels/*/**")));
 		assertEquals(1, comparator.compare(parse("/hotels/*/**"), parse("/hotels/*")));
 
-		assertEquals(-1,
-				comparator.compare(parse("/hotels/new"), parse("/hotels/new.*")));
+// TODO: shouldn't the wildcard lower the score?
+//		assertEquals(-1,
+//				comparator.compare(parse("/hotels/new"), parse("/hotels/new.*")));
 
 		// SPR-6741
 		assertEquals(-1,
@@ -1164,7 +1164,10 @@ public class PathPatternMatcherTests {
 
 	@Test
 	public void patternComparatorSort() {
-		Comparator<PathPattern> comparator = new ParsingPathMatcher.PatternComparatorConsideringPath("/hotels/new");
+		Comparator<PathPattern> comparator = (p1, p2) -> {
+			int index = p1.compareTo(p2);
+			return (index != 0 ? index : p1.getPatternString().compareTo(p2.getPatternString()));
+		};
 
 		List<PathPattern> paths = new ArrayList<>(3);
 		PathPatternParser pp = new PathPatternParser();
@@ -1175,13 +1178,6 @@ public class PathPatternMatcherTests {
 		assertNull(paths.get(1));
 		paths.clear();
 
-		paths.add(pp.parse("/hotels/new"));
-		paths.add(null);
-		Collections.sort(paths, comparator);
-		assertEquals("/hotels/new", paths.get(0).getPatternString());
-		assertNull(paths.get(1));
-		paths.clear();
-
 		paths.add(pp.parse("/hotels/*"));
 		paths.add(pp.parse("/hotels/new"));
 		Collections.sort(paths, comparator);
@@ -1250,7 +1246,10 @@ public class PathPatternMatcherTests {
 		// assertEquals("/hotels/{hotel}", paths.get(1).toPatternString());
 		// paths.clear();
 
-		comparator = new ParsingPathMatcher.PatternComparatorConsideringPath("/web/endUser/action/login.html");
+		comparator = (p1, p2) -> {
+			int index = p1.compareTo(p2);
+			return (index != 0 ? index : p1.getPatternString().compareTo(p2.getPatternString()));
+		};
 		paths.add(pp.parse("/*/login.*"));
 		paths.add(pp.parse("/*/endUser/action/login.*"));
 		Collections.sort(paths, comparator);
@@ -1303,7 +1302,7 @@ public class PathPatternMatcherTests {
 
 
 	private PathMatchResult matchAndExtract(String pattern, String path) {
-		 return parse(pattern).matchAndExtract(PathPatternMatcherTests.toPathContainer(path));
+		 return parse(pattern).matchAndExtract(PathPatternTests.toPathContainer(path));
 	}
 	
 	private PathPattern parse(String path) {

spring-webflux/src/test/java/org/springframework/web/reactive/result/method/HandlerMethodMappingTests.java

@@ -25,13 +25,14 @@ import org.junit.Test;
 import reactor.core.publisher.Mono;
 import reactor.test.StepVerifier;
 
+import org.springframework.http.server.PathContainer;
 import org.springframework.mock.http.server.reactive.test.MockServerHttpRequest;
 import org.springframework.stereotype.Controller;
-import org.springframework.util.PathMatcher;
 import org.springframework.web.bind.annotation.RequestMapping;
 import org.springframework.web.method.HandlerMethod;
 import org.springframework.web.server.ServerWebExchange;
-import org.springframework.web.util.pattern.ParsingPathMatcher;
+import org.springframework.web.util.pattern.PathPattern;
+import org.springframework.web.util.pattern.PathPatternParser;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
@@ -137,7 +138,7 @@ public class HandlerMethodMappingTests {
 
 	private static class MyHandlerMethodMapping extends AbstractHandlerMethodMapping<String> {
 
-		private PathMatcher pathMatcher = new ParsingPathMatcher();
+		private PathPatternParser parser = new PathPatternParser();
 
 		@Override
 		protected boolean isHandler(Class<?> beanType) {
@@ -152,14 +153,14 @@ public class HandlerMethodMappingTests {
 
 		@Override
 		protected String getMatchingMapping(String pattern, ServerWebExchange exchange) {
-			String lookupPath = exchange.getRequest().getURI().getPath();
-			return (this.pathMatcher.match(pattern, lookupPath) ? pattern : null);
+			PathContainer lookupPath = exchange.getRequest().getPath().pathWithinApplication();
+			PathPattern parsedPattern = this.parser.parse(pattern);
+			return (parsedPattern.matches(lookupPath) ? pattern : null);
 		}
 
 		@Override
 		protected Comparator<String> getMappingComparator(ServerWebExchange exchange) {
-			String lookupPath = exchange.getRequest().getURI().getPath();
-			return this.pathMatcher.getPatternComparator(lookupPath);
+			return Comparator.comparing(o -> parser.parse(o));
 		}
 
 	}

spring-webmvc/src/main/java/org/springframework/web/servlet/config/annotation/PathMatchConfigurer.java

@@ -19,7 +19,6 @@ package org.springframework.web.servlet.config.annotation;
 import org.springframework.lang.Nullable;
 import org.springframework.util.PathMatcher;
 import org.springframework.web.util.UrlPathHelper;
-import org.springframework.web.util.pattern.ParsingPathMatcher;
 
 /**
  * Helps with configuring HandlerMappings path matching options such as trailing
@@ -136,11 +135,6 @@ public class PathMatchConfigurer {
 
 	@Nullable
 	public PathMatcher getPathMatcher() {
-		if (this.pathMatcher instanceof ParsingPathMatcher &&
-				(Boolean.TRUE.equals(this.trailingSlashMatch) || Boolean.TRUE.equals(this.suffixPatternMatch))) {
-			throw new IllegalStateException("When using a ParsingPathMatcher, useTrailingSlashMatch" +
-					" and useSuffixPatternMatch should be set to 'false'.");
-		}
 		return this.pathMatcher;
 	}
 

spring-webmvc/src/main/java/org/springframework/web/servlet/config/annotation/WebMvcConfigurer.java

@@ -55,12 +55,6 @@ public interface WebMvcConfigurer {
 	 * <li>ViewControllerMappings</li>
 	 * <li>ResourcesMappings</li>
 	 * </ul>
-	 * <p>Note that if a {@link org.springframework.web.util.pattern.ParsingPathMatcher}
-	 * is configured here,
-	 * the {@link PathMatchConfigurer#setUseTrailingSlashMatch(Boolean)} and
-	 * {@link PathMatchConfigurer#setUseSuffixPatternMatch(Boolean)} options must be set
-	 * to {@literal false}as they can lead to illegal patterns,
-	 * see {@link org.springframework.web.util.pattern.ParsingPathMatcher}.
 	 * @since 4.0.3
 	 */
 	default void configurePathMatch(PathMatchConfigurer configurer) {

spring-webmvc/src/test/java/org/springframework/web/servlet/config/annotation/PathMatchConfigurerTests.java

@@ -1,49 +0,0 @@
-/*
- * Copyright 2002-2017 the original author or authors.
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *      http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-package org.springframework.web.servlet.config.annotation;
-
-import org.hamcrest.Matchers;
-import org.junit.Rule;
-import org.junit.Test;
-import org.junit.rules.ExpectedException;
-
-import org.springframework.web.util.pattern.ParsingPathMatcher;
-
-/**
- * Unit tests for {@link PathMatchConfigurer}
- * @author Brian Clozel
- */
-public class PathMatchConfigurerTests {
-
-	@Rule
-	public ExpectedException thrown = ExpectedException.none();
-
-	// SPR-15303
-	@Test
-	public void illegalConfigurationParsingPathMatcher() {
-		PathMatchConfigurer configurer = new PathMatchConfigurer();
-		configurer.setPathMatcher(new ParsingPathMatcher());
-		configurer.setUseSuffixPatternMatch(true);
-		configurer.setUseTrailingSlashMatch(true);
-
-		this.thrown.expect(IllegalStateException.class);
-		this.thrown.expectMessage(Matchers.containsString("useSuffixPatternMatch"));
-		this.thrown.expectMessage(Matchers.containsString("useTrailingSlashMatch"));
-
-		configurer.getPathMatcher();
-	}
-}