PathPattern caching with HandlerMappingIntrospector

This commit evolves the solution from M1 by parsing and caching patterns with the
target HandlerMapping's PathPatternParser. This makes it unnecessary for callers to
be aware of pattern parsing.

Closes gh-25312

Author: rstoyanchev

spring-webmvc/src/main/java/org/springframework/web/servlet/handler/HandlerMappingIntrospector.java

@@ -22,6 +22,8 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Properties;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.stream.Collectors;
 
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletRequestWrapper;
@@ -76,6 +78,10 @@ public class HandlerMappingIntrospector
 	@Nullable
 	private List<HandlerMapping> handlerMappings;
 
+	@Nullable
+	private Map<HandlerMapping, MatchableHandlerMapping> pathPatternMatchableHandlerMappings =
+			new ConcurrentHashMap<>();
+
 
 	/**
 	 * Constructor for use with {@link ApplicationContextAware}.
@@ -113,6 +119,7 @@ public void afterPropertiesSet() {
 		if (this.handlerMappings == null) {
 			Assert.notNull(this.applicationContext, "No ApplicationContext");
 			this.handlerMappings = initHandlerMappings(this.applicationContext);
+			this.pathPatternMatchableHandlerMappings = initPathPatternMatchableHandlerMappings(this.handlerMappings);
 		}
 	}
 
@@ -130,14 +137,16 @@ public void afterPropertiesSet() {
 	@Nullable
 	public MatchableHandlerMapping getMatchableHandlerMapping(HttpServletRequest request) throws Exception {
 		Assert.notNull(this.handlerMappings, "Handler mappings not initialized");
+		Assert.notNull(this.pathPatternMatchableHandlerMappings, "Handler mappings with PathPatterns not initialized");
 		HttpServletRequest wrapper = new RequestAttributeChangeIgnoringWrapper(request);
 		for (HandlerMapping handlerMapping : this.handlerMappings) {
 			Object handler = handlerMapping.getHandler(wrapper);
 			if (handler == null) {
 				continue;
 			}
 			if (handlerMapping instanceof MatchableHandlerMapping) {
-				return ((MatchableHandlerMapping) handlerMapping);
+				return this.pathPatternMatchableHandlerMappings.getOrDefault(
+						handlerMapping, (MatchableHandlerMapping) handlerMapping);
 			}
 			throw new IllegalStateException("HandlerMapping is not a MatchableHandlerMapping");
 		}
@@ -212,6 +221,16 @@ private static List<HandlerMapping> initFallback(ApplicationContext applicationC
 		return result;
 	}
 
+	private static Map<HandlerMapping, MatchableHandlerMapping> initPathPatternMatchableHandlerMappings(
+			List<HandlerMapping> mappings) {
+
+		return mappings.stream()
+				.filter(mapping -> mapping instanceof MatchableHandlerMapping)
+				.map(mapping -> (MatchableHandlerMapping) mapping)
+				.filter(mapping -> mapping.getPatternParser() != null)
+				.collect(Collectors.toMap(mapping -> mapping, PathPatternMatchableHandlerMapping::new));
+	}
+
 
 	/**
 	 * Request wrapper that ignores request attribute changes.

spring-webmvc/src/main/java/org/springframework/web/servlet/handler/MatchableHandlerMapping.java

@@ -18,11 +18,8 @@
 
 import javax.servlet.http.HttpServletRequest;
 
-import org.springframework.http.server.PathContainer;
 import org.springframework.lang.Nullable;
 import org.springframework.web.servlet.HandlerMapping;
-import org.springframework.web.util.ServletRequestPathUtils;
-import org.springframework.web.util.pattern.PathPattern;
 import org.springframework.web.util.pattern.PathPatternParser;
 
 /**
@@ -46,21 +43,6 @@ default PathPatternParser getPatternParser() {
 		return null;
 	}
 
-	/**
-	 * Determine whether the request matches the given pattern. Use this method
-	 * when {@link #getPatternParser()} is not {@code null} which means that the
-	 * {@code HandlerMapping} has pre-parsed patterns enabled.
-	 * @param request the current request
-	 * @param pattern the pattern to match
-	 * @return the result from request matching, or {@code null} if none
-	 * @since 5.3
-	 */
-	@Nullable
-	default RequestMatchResult match(HttpServletRequest request, PathPattern pattern) {
-		PathContainer path = ServletRequestPathUtils.getParsedRequestPath(request).pathWithinApplication();
-		return (pattern.matches(path) ? new RequestMatchResult(pattern, path) : null);
-	}
-
 	/**
 	 * Determine whether the request matches the given pattern. Use this method
 	 * when {@link #getPatternParser()} returns {@code null} which means that the

spring-webmvc/src/main/java/org/springframework/web/servlet/handler/PathPatternMatchableHandlerMapping.java

@@ -0,0 +1,73 @@
+/*
+ * Copyright 2002-2020 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
+ *
+ *      https://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.handler;
+
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+
+import javax.servlet.http.HttpServletRequest;
+
+import org.springframework.http.server.PathContainer;
+import org.springframework.lang.Nullable;
+import org.springframework.util.Assert;
+import org.springframework.web.servlet.HandlerExecutionChain;
+import org.springframework.web.util.ServletRequestPathUtils;
+import org.springframework.web.util.pattern.PathPattern;
+import org.springframework.web.util.pattern.PathPatternParser;
+
+/**
+ * Wraps {@link MatchableHandlerMapping}s configured with a {@link PathPatternParser}
+ * in order to parse patterns lazily and cache them for re-ues.
+ *
+ * @author Rossen Stoyanchev
+ * @since 5.3
+ */
+class PathPatternMatchableHandlerMapping implements MatchableHandlerMapping {
+
+	private static final int MAX_PATTERNS = 1024;
+
+
+	private final MatchableHandlerMapping delegate;
+
+	private final PathPatternParser parser;
+
+	private final Map<String, PathPattern> pathPatternCache = new ConcurrentHashMap<>();
+
+
+	public PathPatternMatchableHandlerMapping(MatchableHandlerMapping delegate) {
+		Assert.notNull(delegate, "Delegate MatchableHandlerMapping is required.");
+		Assert.notNull(delegate.getPatternParser(), "PatternParser is required.");
+		this.delegate = delegate;
+		this.parser = delegate.getPatternParser();
+	}
+
+	@Nullable
+	@Override
+	public RequestMatchResult match(HttpServletRequest request, String pattern) {
+		PathPattern pathPattern = this.pathPatternCache.computeIfAbsent(pattern, value -> {
+			Assert.isTrue(this.pathPatternCache.size() < MAX_PATTERNS, "Max size for pattern cache exceeded.");
+			return this.parser.parse(pattern);
+		});
+		PathContainer path = ServletRequestPathUtils.getParsedRequestPath(request).pathWithinApplication();
+		return (pathPattern.matches(path) ? new RequestMatchResult(pathPattern, path) : null);
+	}
+
+	@Nullable
+	@Override
+	public HandlerExecutionChain getHandler(HttpServletRequest request) throws Exception {
+		return this.delegate.getHandler(request);
+	}
+}

spring-webmvc/src/test/java/org/springframework/web/servlet/handler/HandlerMappingIntrospectorTests.java

@@ -16,6 +16,7 @@
 
 package org.springframework.web.servlet.handler;
 
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
@@ -44,6 +45,7 @@
 import org.springframework.web.util.ServletRequestPathUtils;
 import org.springframework.web.util.pattern.PathPattern;
 import org.springframework.web.util.pattern.PathPatternParser;
+import org.springframework.web.util.pattern.PatternParseException;
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatIllegalStateException;
@@ -111,7 +113,7 @@ void defaultHandlerMappings() {
 	@ValueSource(booleans = {true, false})
 	void getMatchable(boolean usePathPatterns) throws Exception {
 
-		PathPatternParser parser = new PathPatternParser();
+		TestPathPatternParser parser = new TestPathPatternParser();
 
 		GenericWebApplicationContext context = new GenericWebApplicationContext();
 		context.registerBean("mapping", SimpleUrlHandlerMapping.class, () -> {
@@ -134,16 +136,14 @@ void getMatchable(boolean usePathPatterns) throws Exception {
 		MatchableHandlerMapping mapping = initIntrospector(context).getMatchableHandlerMapping(request);
 
 		assertThat(mapping).isNotNull();
-		assertThat(mapping).isEqualTo(context.getBean("mapping"));
 		assertThat(request.getAttribute(BEST_MATCHING_PATTERN_ATTRIBUTE)).as("Attribute changes not ignored").isNull();
 
-		String pattern = "/p*/*";
-		PathPattern pathPattern = parser.parse(pattern);
-		assertThat(usePathPatterns ? mapping.match(request, pathPattern) : mapping.match(request, pattern)).isNotNull();
+		assertThat(mapping.match(request, "/p*/*")).isNotNull();
+		assertThat(mapping.match(request, "/b*/*")).isNull();
 
-		pattern = "/b*/*";
-		pathPattern = parser.parse(pattern);
-		assertThat(usePathPatterns ? mapping.match(request, pathPattern) : mapping.match(request, pattern)).isNull();
+		if (usePathPatterns) {
+			assertThat(parser.getParsedPatterns()).containsExactly("/path/*", "/p*/*", "/b*/*");
+		}
 	}
 
 	@Test
@@ -230,4 +230,20 @@ void handle() {
 		}
 	}
 
+	private static class TestPathPatternParser extends PathPatternParser {
+
+		private final List<String> parsedPatterns = new ArrayList<>();
+
+
+		public List<String> getParsedPatterns() {
+			return this.parsedPatterns;
+		}
+
+		@Override
+		public PathPattern parse(String pathPattern) throws PatternParseException {
+			this.parsedPatterns.add(pathPattern);
+			return super.parse(pathPattern);
+		}
+	}
+
 }