From 326154270fa6338010a1c047044fc4a9f21e910b Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Wed, 30 Oct 2013 00:53:18 +0100 Subject: [PATCH] Refined caching of AntPathStringMatcher per pattern Introduced a "setCachePatterns(boolean)" method for explicit configuration, a default turnoff threshold at 65536 entries (at which point we're deciding that caching isn't worthwhile because patterns are unlikely to be reoccurring often enough), and an "AntPathStringMatcher getStringMatcher(String pattern)" template method. Issue: SPR-10803 --- .../springframework/util/AntPathMatcher.java | 280 +++++++++++------- .../util/AntPathMatcherTests.java | 40 ++- 2 files changed, 205 insertions(+), 115 deletions(-) diff --git a/spring-core/src/main/java/org/springframework/util/AntPathMatcher.java b/spring-core/src/main/java/org/springframework/util/AntPathMatcher.java index 2408a78bb8..3e2abc62a6 100644 --- a/spring-core/src/main/java/org/springframework/util/AntPathMatcher.java +++ b/spring-core/src/main/java/org/springframework/util/AntPathMatcher.java @@ -50,29 +50,56 @@ import java.util.regex.Pattern; */ public class AntPathMatcher implements PathMatcher { + private static final int CACHE_TURNOFF_THRESHOLD = 65536; + private static final Pattern VARIABLE_PATTERN = Pattern.compile("\\{[^/]+?\\}"); /** Default path separator: "/" */ public static final String DEFAULT_PATH_SEPARATOR = "/"; - private String pathSeparator = DEFAULT_PATH_SEPARATOR; - private final Map stringMatcherCache = - new ConcurrentHashMap(256); + private String pathSeparator = DEFAULT_PATH_SEPARATOR; private boolean trimTokens = true; + private volatile Boolean cachePatterns; + + final Map stringMatcherCache = + new ConcurrentHashMap(256); + - /** Set the path separator to use for pattern parsing. Default is "/", as in Ant. */ + /** + * Set the path separator to use for pattern parsing. + * Default is "/", as in Ant. + */ public void setPathSeparator(String pathSeparator) { this.pathSeparator = (pathSeparator != null ? pathSeparator : DEFAULT_PATH_SEPARATOR); } - /** Whether to trim tokenized paths and patterns. */ + /** + * Specify whether to trim tokenized paths and patterns. + * Default is {@code true}. + */ public void setTrimTokens(boolean trimTokens) { this.trimTokens = trimTokens; } + /** + * Specify whether to cache parsed pattern metadata for patterns passed + * into this matcher's {@link #match} method. A value of {@code true} + * activates an unlimited pattern cache; a value of {@code false} turns + * the pattern cache off completely. + *

Default is for the cache to be on, but with the variant to automatically + * turn it off when encountering too many patterns to cache at runtime + * (the threshold is 65536), assuming that arbitrary permutations of patterns + * are coming in, with little chance for encountering a reoccurring pattern. + * @see #getStringMatcher(String) + */ + public void setCachePatterns(boolean cachePatterns) { + this.cachePatterns = cachePatterns; + } + + @Override public boolean isPattern(String path) { return (path.indexOf('*') != -1 || path.indexOf('?') != -1); @@ -88,7 +115,6 @@ public class AntPathMatcher implements PathMatcher { return doMatch(pattern, path, false, null); } - /** * Actually match the given {@code path} against the given {@code pattern}. * @param pattern the pattern to match against @@ -97,9 +123,7 @@ public class AntPathMatcher implements PathMatcher { * as far as the given base path goes is sufficient) * @return {@code true} if the supplied {@code path} matched, {@code false} if it didn't */ - protected boolean doMatch(String pattern, String path, boolean fullMatch, - Map uriTemplateVariables) { - + protected boolean doMatch(String pattern, String path, boolean fullMatch, Map uriTemplateVariables) { if (path.startsWith(this.pathSeparator) != pattern.startsWith(this.pathSeparator)) { return false; } @@ -114,11 +138,11 @@ public class AntPathMatcher implements PathMatcher { // Match all elements up to the first ** while (pattIdxStart <= pattIdxEnd && pathIdxStart <= pathIdxEnd) { - String patDir = pattDirs[pattIdxStart]; - if ("**".equals(patDir)) { + String pattDir = pattDirs[pattIdxStart]; + if ("**".equals(pattDir)) { break; } - if (!matchStrings(patDir, pathDirs[pathIdxStart], uriTemplateVariables)) { + if (!matchStrings(pattDir, pathDirs[pathIdxStart], uriTemplateVariables)) { return false; } pattIdxStart++; @@ -155,11 +179,11 @@ public class AntPathMatcher implements PathMatcher { // up to last '**' while (pattIdxStart <= pattIdxEnd && pathIdxStart <= pathIdxEnd) { - String patDir = pattDirs[pattIdxEnd]; - if (patDir.equals("**")) { + String pattDir = pattDirs[pattIdxEnd]; + if (pattDir.equals("**")) { break; } - if (!matchStrings(patDir, pathDirs[pathIdxEnd], uriTemplateVariables)) { + if (!matchStrings(pattDir, pathDirs[pathIdxEnd], uriTemplateVariables)) { return false; } pattIdxEnd--; @@ -225,20 +249,49 @@ public class AntPathMatcher implements PathMatcher { } /** - * Tests whether or not a string matches against a pattern. The pattern may contain two special characters: - *
'*' means zero or more characters - *
'?' means one and only one character - * @param pattern pattern to match against. Must not be {@code null}. - * @param str string which must be matched against the pattern. Must not be {@code null}. - * @return {@code true} if the string matches against the pattern, or {@code false} otherwise. + * Tests whether or not a string matches against a pattern. + * @param pattern the pattern to match against (never {@code null}) + * @param str the String which must be matched against the pattern (never {@code null}) + * @return {@code true} if the string matches against the pattern, or {@code false} otherwise */ private boolean matchStrings(String pattern, String str, Map uriTemplateVariables) { - AntPathStringMatcher matcher = this.stringMatcherCache.get(pattern); + return getStringMatcher(pattern).matchStrings(str, uriTemplateVariables); + } + + /** + * Build or retrieve an {@link AntPathStringMatcher} for the given pattern. + *

The default implementation checks this AntPathMatcher's internal cache + * (see {@link #setCachePatterns}, creating a new AntPathStringMatcher instance + * through {@link AntPathStringMatcher#AntPathStringMatcher(String)} if none found. + * When encountering too many patterns to cache at runtime (the threshold is 65536), + * it turns the default cache off, assuming that arbitrary permutations of patterns + * are coming in, with little chance for encountering a reoccurring pattern. + *

This method may get overridden to implement a custom cache strategy. + * @param pattern the pattern to match against (never {@code null}) + * @return a corresponding AntPathStringMatcher (never {@code null}) + * @see #setCachePatterns + */ + protected AntPathStringMatcher getStringMatcher(String pattern) { + AntPathStringMatcher matcher = null; + Boolean cachePatterns = this.cachePatterns; + if (cachePatterns == null || cachePatterns.booleanValue()) { + matcher = this.stringMatcherCache.get(pattern); + } if (matcher == null) { matcher = new AntPathStringMatcher(pattern); - this.stringMatcherCache.put(pattern, matcher); + if (cachePatterns == null && this.stringMatcherCache.size() == CACHE_TURNOFF_THRESHOLD) { + // Try to adapt to the runtime situation that we're encountering: + // There are obviously too many different paths coming in here... + // So let's turn off the cache since the patterns are unlikely to be reoccurring. + this.cachePatterns = false; + this.stringMatcherCache.clear(); + return matcher; + } + if (cachePatterns == null || cachePatterns.booleanValue()) { + this.stringMatcherCache.put(pattern, matcher); + } } - return matcher.matchStrings(str, uriTemplateVariables); + return matcher; } /** @@ -381,11 +434,99 @@ public class AntPathMatcher implements PathMatcher { } - private static class AntPatternComparator implements Comparator { + /** + * Tests whether or not a string matches against a pattern via a {@link Pattern}. + *

The pattern may contain special characters: '*' means zero or more characters; '?' means one and + * only one character; '{' and '}' indicate a URI template pattern. For example /users/{user}. + */ + protected static class AntPathStringMatcher { + + private static final Pattern GLOB_PATTERN = Pattern.compile("\\?|\\*|\\{((?:\\{[^/]+?\\}|[^/{}]|\\\\[{}])+?)\\}"); + + private static final String DEFAULT_VARIABLE_PATTERN = "(.*)"; + + private final Pattern pattern; + + private final List variableNames = new LinkedList(); + + public AntPathStringMatcher(String pattern) { + StringBuilder patternBuilder = new StringBuilder(); + Matcher m = GLOB_PATTERN.matcher(pattern); + int end = 0; + while (m.find()) { + patternBuilder.append(quote(pattern, end, m.start())); + String match = m.group(); + if ("?".equals(match)) { + patternBuilder.append('.'); + } + else if ("*".equals(match)) { + patternBuilder.append(".*"); + } + else if (match.startsWith("{") && match.endsWith("}")) { + int colonIdx = match.indexOf(':'); + if (colonIdx == -1) { + patternBuilder.append(DEFAULT_VARIABLE_PATTERN); + this.variableNames.add(m.group(1)); + } + else { + String variablePattern = match.substring(colonIdx + 1, match.length() - 1); + patternBuilder.append('('); + patternBuilder.append(variablePattern); + patternBuilder.append(')'); + String variableName = match.substring(1, colonIdx); + this.variableNames.add(variableName); + } + } + end = m.end(); + } + patternBuilder.append(quote(pattern, end, pattern.length())); + this.pattern = Pattern.compile(patternBuilder.toString()); + } + + private String quote(String s, int start, int end) { + if (start == end) { + return ""; + } + return Pattern.quote(s.substring(start, end)); + } + + /** + * Main entry point. + * @return {@code true} if the string matches against the pattern, or {@code false} otherwise. + */ + public boolean matchStrings(String str, Map uriTemplateVariables) { + Matcher matcher = this.pattern.matcher(str); + if (matcher.matches()) { + if (uriTemplateVariables != null) { + // SPR-8455 + Assert.isTrue(this.variableNames.size() == matcher.groupCount(), + "The number of capturing groups in the pattern segment " + this.pattern + + " does not match the number of URI template variables it defines, which can occur if " + + " capturing groups are used in a URI template regex. Use non-capturing groups instead."); + for (int i = 1; i <= matcher.groupCount(); i++) { + String name = this.variableNames.get(i - 1); + String value = matcher.group(i); + uriTemplateVariables.put(name, value); + } + } + return true; + } + else { + return false; + } + } + } + + + /** + * The default {@link Comparator} implementation returned by + * {@link #getPatternComparator(String)}. + */ + protected static class AntPatternComparator implements Comparator { private final String path; - private AntPatternComparator(String path) { + public AntPatternComparator(String path) { this.path = path; } @@ -465,92 +606,7 @@ public class AntPathMatcher implements PathMatcher { * Returns the length of the given pattern, where template variables are considered to be 1 long. */ private int getPatternLength(String pattern) { - Matcher m = VARIABLE_PATTERN.matcher(pattern); - return m.replaceAll("#").length(); - } - } - - - /** - * Tests whether or not a string matches against a pattern via a {@link Pattern}. - *

The pattern may contain special characters: '*' means zero or more characters; '?' means one and - * only one character; '{' and '}' indicate a URI template pattern. For example /users/{user}. - */ - private static class AntPathStringMatcher { - - private static final Pattern GLOB_PATTERN = Pattern.compile("\\?|\\*|\\{((?:\\{[^/]+?\\}|[^/{}]|\\\\[{}])+?)\\}"); - - private static final String DEFAULT_VARIABLE_PATTERN = "(.*)"; - - private final Pattern pattern; - - private final List variableNames = new LinkedList(); - - public AntPathStringMatcher(String pattern) { - StringBuilder patternBuilder = new StringBuilder(); - Matcher m = GLOB_PATTERN.matcher(pattern); - int end = 0; - while (m.find()) { - patternBuilder.append(quote(pattern, end, m.start())); - String match = m.group(); - if ("?".equals(match)) { - patternBuilder.append('.'); - } - else if ("*".equals(match)) { - patternBuilder.append(".*"); - } - else if (match.startsWith("{") && match.endsWith("}")) { - int colonIdx = match.indexOf(':'); - if (colonIdx == -1) { - patternBuilder.append(DEFAULT_VARIABLE_PATTERN); - this.variableNames.add(m.group(1)); - } - else { - String variablePattern = match.substring(colonIdx + 1, match.length() - 1); - patternBuilder.append('('); - patternBuilder.append(variablePattern); - patternBuilder.append(')'); - String variableName = match.substring(1, colonIdx); - this.variableNames.add(variableName); - } - } - end = m.end(); - } - patternBuilder.append(quote(pattern, end, pattern.length())); - this.pattern = Pattern.compile(patternBuilder.toString()); - } - - private String quote(String s, int start, int end) { - if (start == end) { - return ""; - } - return Pattern.quote(s.substring(start, end)); - } - - /** - * Main entry point. - * @return {@code true} if the string matches against the pattern, or {@code false} otherwise. - */ - public boolean matchStrings(String str, Map uriTemplateVariables) { - Matcher matcher = this.pattern.matcher(str); - if (matcher.matches()) { - if (uriTemplateVariables != null) { - // SPR-8455 - Assert.isTrue(this.variableNames.size() == matcher.groupCount(), - "The number of capturing groups in the pattern segment " + this.pattern + - " does not match the number of URI template variables it defines, which can occur if " + - " capturing groups are used in a URI template regex. Use non-capturing groups instead."); - for (int i = 1; i <= matcher.groupCount(); i++) { - String name = this.variableNames.get(i - 1); - String value = matcher.group(i); - uriTemplateVariables.put(name, value); - } - } - return true; - } - else { - return false; - } + return VARIABLE_PATTERN.matcher(pattern).replaceAll("#").length(); } } diff --git a/spring-core/src/test/java/org/springframework/util/AntPathMatcherTests.java b/spring-core/src/test/java/org/springframework/util/AntPathMatcherTests.java index 6a5e5684c8..a1529b032b 100644 --- a/spring-core/src/test/java/org/springframework/util/AntPathMatcherTests.java +++ b/spring-core/src/test/java/org/springframework/util/AntPathMatcherTests.java @@ -23,10 +23,11 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; -import static org.junit.Assert.*; import org.junit.Before; import org.junit.Test; +import static org.junit.Assert.*; + /** * @author Alef Arendsen * @author Seth Ladd @@ -382,9 +383,10 @@ public class AntPathMatcherTests { try { pathMatcher.extractUriTemplateVariables("/web/{id:foo(bar)?}", "/web/foobar"); fail("Expected exception"); - } catch (IllegalArgumentException e) { + } + catch (IllegalArgumentException ex) { assertTrue("Expected helpful message on the use of capturing groups", - e.getMessage().contains("The number of capturing groups in the pattern")); + ex.getMessage().contains("The number of capturing groups in the pattern")); } } @@ -560,4 +562,36 @@ public class AntPathMatcherTests { assertTrue(pathMatcher.match("/group/{groupName}/members", "/group/ sales/members")); } + @Test + public void testDefaultCacheSetting() { + match(); + assertTrue(pathMatcher.stringMatcherCache.size() > 20); + + for (int i = 0; i < 65536; i++) { + pathMatcher.match("test" + i, "test"); + } + // Cache turned off because it went beyond the threshold + assertTrue(pathMatcher.stringMatcherCache.isEmpty()); + } + + @Test + public void testCacheSetToTrue() { + pathMatcher.setCachePatterns(true); + match(); + assertTrue(pathMatcher.stringMatcherCache.size() > 20); + + for (int i = 0; i < 65536; i++) { + pathMatcher.match("test" + i, "test"); + } + // Cache keeps being alive due to the explicit cache setting + assertTrue(pathMatcher.stringMatcherCache.size() > 65536); + } + + @Test + public void testCacheSetToFalse() { + pathMatcher.setCachePatterns(false); + match(); + assertTrue(pathMatcher.stringMatcherCache.isEmpty()); + } + }