PathMappings optimizations (#9055)
* Avoid iterations if only ServletPathSpec instances * Avoid tests for empty mappings. * Better reset implementation * More test coverage
This commit is contained in:
parent
99b1b1e240
commit
4916377686
|
@ -21,11 +21,34 @@ public class MappedResource<E> implements Comparable<MappedResource<E>>
|
|||
{
|
||||
private final PathSpec pathSpec;
|
||||
private final E resource;
|
||||
private final MatchedResource<E> preMatched;
|
||||
|
||||
public MappedResource(PathSpec pathSpec, E resource)
|
||||
{
|
||||
this.pathSpec = pathSpec;
|
||||
this.resource = resource;
|
||||
|
||||
MatchedResource<E> matched;
|
||||
switch (pathSpec.getGroup())
|
||||
{
|
||||
case ROOT:
|
||||
matched = new MatchedResource<>(resource, pathSpec, pathSpec.matched("/"));
|
||||
break;
|
||||
case EXACT:
|
||||
matched = new MatchedResource<>(resource, pathSpec, pathSpec.matched(pathSpec.getDeclaration()));
|
||||
break;
|
||||
default:
|
||||
matched = null;
|
||||
}
|
||||
this.preMatched = matched;
|
||||
}
|
||||
|
||||
/**
|
||||
* @return A pre match {@link MatchedResource} for ROOT and EXACT matches, else null;
|
||||
*/
|
||||
public MatchedResource<E> getPreMatched()
|
||||
{
|
||||
return preMatched;
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
@ -15,9 +15,12 @@ package org.eclipse.jetty.http.pathmap;
|
|||
|
||||
import java.io.IOException;
|
||||
import java.util.ArrayList;
|
||||
import java.util.Collections;
|
||||
import java.util.Comparator;
|
||||
import java.util.HashMap;
|
||||
import java.util.Iterator;
|
||||
import java.util.List;
|
||||
import java.util.Map;
|
||||
import java.util.Set;
|
||||
import java.util.TreeSet;
|
||||
import java.util.function.Predicate;
|
||||
|
@ -42,11 +45,13 @@ public class PathMappings<E> implements Iterable<MappedResource<E>>, Dumpable
|
|||
private static final Logger LOG = LoggerFactory.getLogger(PathMappings.class);
|
||||
private final Set<MappedResource<E>> _mappings = new TreeSet<>(Comparator.comparing(MappedResource::getPathSpec));
|
||||
|
||||
/**
|
||||
* When _orderIsSignificant is true, the order of the MappedResources is significant and a match needs to be iteratively
|
||||
* tried against each mapping (ordered by group then add order) to find the first that matches.
|
||||
*/
|
||||
private boolean _orderIsSignificant;
|
||||
private boolean _optimizedExact = true;
|
||||
private final Index.Mutable<MappedResource<E>> _exactMap = new Index.Builder<MappedResource<E>>()
|
||||
.caseSensitive(true)
|
||||
.mutable()
|
||||
.build();
|
||||
private final Map<String, MappedResource<E>> _exactMap = new HashMap<>();
|
||||
private boolean _optimizedPrefix = true;
|
||||
private final Index.Mutable<MappedResource<E>> _prefixMap = new Index.Builder<MappedResource<E>>()
|
||||
.caseSensitive(true)
|
||||
|
@ -58,6 +63,9 @@ public class PathMappings<E> implements Iterable<MappedResource<E>>, Dumpable
|
|||
.mutable()
|
||||
.build();
|
||||
|
||||
private MappedResource<E> _servletRoot;
|
||||
private MappedResource<E> _servletDefault;
|
||||
|
||||
@Override
|
||||
public String dump()
|
||||
{
|
||||
|
@ -86,6 +94,12 @@ public class PathMappings<E> implements Iterable<MappedResource<E>>, Dumpable
|
|||
_mappings.clear();
|
||||
_prefixMap.clear();
|
||||
_suffixMap.clear();
|
||||
_optimizedExact = true;
|
||||
_optimizedPrefix = true;
|
||||
_optimizedSuffix = true;
|
||||
_orderIsSignificant = false;
|
||||
_servletRoot = null;
|
||||
_servletDefault = null;
|
||||
}
|
||||
|
||||
public void removeIf(Predicate<MappedResource<E>> predicate)
|
||||
|
@ -121,31 +135,117 @@ public class PathMappings<E> implements Iterable<MappedResource<E>>, Dumpable
|
|||
*/
|
||||
public List<MappedResource<E>> getMatches(String path)
|
||||
{
|
||||
if (_mappings.isEmpty())
|
||||
return Collections.emptyList();
|
||||
|
||||
boolean isRootPath = "/".equals(path);
|
||||
|
||||
List<MappedResource<E>> ret = new ArrayList<>();
|
||||
// Iterator over all the mapping, adding only those that match.
|
||||
List<MappedResource<E>> matches = null;
|
||||
for (MappedResource<E> mr : _mappings)
|
||||
{
|
||||
switch (mr.getPathSpec().getGroup())
|
||||
{
|
||||
case ROOT:
|
||||
if (isRootPath)
|
||||
ret.add(mr);
|
||||
{
|
||||
if (matches == null)
|
||||
matches = new ArrayList<>();
|
||||
matches.add(mr);
|
||||
}
|
||||
break;
|
||||
case DEFAULT:
|
||||
if (isRootPath || mr.getPathSpec().matched(path) != null)
|
||||
ret.add(mr);
|
||||
{
|
||||
if (matches == null)
|
||||
matches = new ArrayList<>();
|
||||
matches.add(mr);
|
||||
}
|
||||
break;
|
||||
default:
|
||||
if (mr.getPathSpec().matched(path) != null)
|
||||
ret.add(mr);
|
||||
{
|
||||
if (matches == null)
|
||||
matches = new ArrayList<>();
|
||||
matches.add(mr);
|
||||
}
|
||||
break;
|
||||
}
|
||||
}
|
||||
return ret;
|
||||
return matches == null ? Collections.emptyList() : matches;
|
||||
}
|
||||
|
||||
/**
|
||||
* <p>Find the best single match for a path.</p>
|
||||
* <p>The match may be found by optimized direct lookups when possible, otherwise all mappings
|
||||
* are iterated over and the first match returned</p>
|
||||
* @param path The path to match
|
||||
* @return A {@link MatchedResource} instance or null if no mappings matched.
|
||||
* @see #getMatchedIteratively(String)
|
||||
*/
|
||||
public MatchedResource<E> getMatched(String path)
|
||||
{
|
||||
if (_mappings.isEmpty())
|
||||
return null;
|
||||
|
||||
// If order is significant, then we need to match by iterating over all mappings.
|
||||
if (_orderIsSignificant)
|
||||
return getMatchedIteratively(path);
|
||||
|
||||
// Otherwise, we can try optimized matches against each group
|
||||
|
||||
// Try a root match
|
||||
if (_servletRoot != null && "/".equals(path))
|
||||
return _servletRoot.getPreMatched();
|
||||
|
||||
// try an exact match
|
||||
MappedResource<E> exact = _exactMap.get(path);
|
||||
if (exact != null)
|
||||
return exact.getPreMatched();
|
||||
|
||||
// Try a prefix match
|
||||
MappedResource<E> prefix = _prefixMap.getBest(path);
|
||||
if (prefix != null)
|
||||
{
|
||||
MatchedPath matchedPath = prefix.getPathSpec().matched(path);
|
||||
if (matchedPath != null)
|
||||
return new MatchedResource<>(prefix.getResource(), prefix.getPathSpec(), matchedPath);
|
||||
}
|
||||
|
||||
// Try a suffix match
|
||||
if (!_suffixMap.isEmpty())
|
||||
{
|
||||
int i = Math.max(0, path.lastIndexOf("/"));
|
||||
// Loop through each suffix mark
|
||||
// Input is "/a.b.c.foo"
|
||||
// Loop 1: "b.c.foo"
|
||||
// Loop 2: "c.foo"
|
||||
// Loop 3: "foo"
|
||||
while ((i = path.indexOf('.', i + 1)) > 0)
|
||||
{
|
||||
prefix = _suffixMap.get(path, i + 1, path.length() - i - 1);
|
||||
if (prefix == null)
|
||||
continue;
|
||||
|
||||
MatchedPath matchedPath = prefix.getPathSpec().matched(path);
|
||||
if (matchedPath != null)
|
||||
return new MatchedResource<>(prefix.getResource(), prefix.getPathSpec(), matchedPath);
|
||||
}
|
||||
}
|
||||
|
||||
if (_servletDefault != null)
|
||||
return new MatchedResource<>(_servletDefault.getResource(), _servletDefault.getPathSpec(), _servletDefault.getPathSpec().matched(path));
|
||||
|
||||
return null;
|
||||
}
|
||||
|
||||
/**
|
||||
* <p>Iterate over all mappings, returning the first that matches.</p>
|
||||
* @param path The path to match.
|
||||
* @return A {@link MatchedResource} instance or null if no mappings matched.
|
||||
* @see #getMatched(String)
|
||||
*/
|
||||
private MatchedResource<E> getMatchedIteratively(String path)
|
||||
{
|
||||
MatchedPath matchedPath;
|
||||
PathSpecGroup lastGroup = null;
|
||||
|
@ -173,19 +273,9 @@ public class PathMappings<E> implements Iterable<MappedResource<E>>, Dumpable
|
|||
{
|
||||
if (_optimizedExact)
|
||||
{
|
||||
int i = path.length();
|
||||
while (i >= 0)
|
||||
{
|
||||
MappedResource<E> candidate = _exactMap.getBest(path, 0, i--);
|
||||
if (candidate == null)
|
||||
continue;
|
||||
|
||||
matchedPath = candidate.getPathSpec().matched(path);
|
||||
if (matchedPath != null)
|
||||
{
|
||||
return new MatchedResource<>(candidate.getResource(), candidate.getPathSpec(), matchedPath);
|
||||
}
|
||||
}
|
||||
MappedResource<E> exact = _exactMap.get(path);
|
||||
if (exact != null)
|
||||
return exact.getPreMatched();
|
||||
// If we reached here, there's NO optimized EXACT Match possible, skip simple match below
|
||||
skipRestOfGroup = true;
|
||||
}
|
||||
|
@ -196,17 +286,14 @@ public class PathMappings<E> implements Iterable<MappedResource<E>>, Dumpable
|
|||
{
|
||||
if (_optimizedPrefix)
|
||||
{
|
||||
int i = path.length();
|
||||
while (i >= 0)
|
||||
MappedResource<E> candidate = _prefixMap.getBest(path);
|
||||
if (candidate != null)
|
||||
{
|
||||
MappedResource<E> candidate = _prefixMap.getBest(path, 0, i--);
|
||||
if (candidate == null)
|
||||
continue;
|
||||
|
||||
matchedPath = candidate.getPathSpec().matched(path);
|
||||
if (matchedPath != null)
|
||||
return new MatchedResource<>(candidate.getResource(), candidate.getPathSpec(), matchedPath);
|
||||
}
|
||||
|
||||
// If we reached here, there's NO optimized PREFIX Match possible, skip simple match below
|
||||
skipRestOfGroup = true;
|
||||
}
|
||||
|
@ -317,6 +404,7 @@ public class PathMappings<E> implements Iterable<MappedResource<E>>, Dumpable
|
|||
// Note: Example exact in Regex that can cause problems `^/a\Q/b\E/` (which is only ever matching `/a/b/`)
|
||||
// Note: UriTemplate can handle exact easily enough
|
||||
_optimizedExact = false;
|
||||
_orderIsSignificant = true;
|
||||
}
|
||||
break;
|
||||
case PREFIX_GLOB:
|
||||
|
@ -333,6 +421,7 @@ public class PathMappings<E> implements Iterable<MappedResource<E>>, Dumpable
|
|||
// Note: Example Prefix in Regex that can cause problems `^/a/b+` or `^/a/bb*` ('b' one or more times)
|
||||
// Note: Example Prefix in UriTemplate that might cause problems `/a/{b}/{c}`
|
||||
_optimizedPrefix = false;
|
||||
_orderIsSignificant = true;
|
||||
}
|
||||
break;
|
||||
case SUFFIX_GLOB:
|
||||
|
@ -349,6 +438,29 @@ public class PathMappings<E> implements Iterable<MappedResource<E>>, Dumpable
|
|||
// Note: Example suffix in Regex that can cause problems `^.*/path/name.ext` or `^/a/.*(ending)`
|
||||
// Note: Example suffix in UriTemplate that can cause problems `/{a}/name.ext`
|
||||
_optimizedSuffix = false;
|
||||
_orderIsSignificant = true;
|
||||
}
|
||||
break;
|
||||
case ROOT:
|
||||
if (pathSpec instanceof ServletPathSpec)
|
||||
{
|
||||
if (_servletRoot == null)
|
||||
_servletRoot = entry;
|
||||
}
|
||||
else
|
||||
{
|
||||
_orderIsSignificant = true;
|
||||
}
|
||||
break;
|
||||
case DEFAULT:
|
||||
if (pathSpec instanceof ServletPathSpec)
|
||||
{
|
||||
if (_servletDefault == null)
|
||||
_servletDefault = entry;
|
||||
}
|
||||
else
|
||||
{
|
||||
_orderIsSignificant = true;
|
||||
}
|
||||
break;
|
||||
default:
|
||||
|
@ -387,6 +499,7 @@ public class PathMappings<E> implements Iterable<MappedResource<E>>, Dumpable
|
|||
_exactMap.remove(exact);
|
||||
// Recalculate _optimizeExact
|
||||
_optimizedExact = canBeOptimized(PathSpecGroup.EXACT);
|
||||
_orderIsSignificant = nonServletPathSpec();
|
||||
}
|
||||
break;
|
||||
case PREFIX_GLOB:
|
||||
|
@ -396,6 +509,7 @@ public class PathMappings<E> implements Iterable<MappedResource<E>>, Dumpable
|
|||
_prefixMap.remove(prefix);
|
||||
// Recalculate _optimizePrefix
|
||||
_optimizedPrefix = canBeOptimized(PathSpecGroup.PREFIX_GLOB);
|
||||
_orderIsSignificant = nonServletPathSpec();
|
||||
}
|
||||
break;
|
||||
case SUFFIX_GLOB:
|
||||
|
@ -405,8 +519,23 @@ public class PathMappings<E> implements Iterable<MappedResource<E>>, Dumpable
|
|||
_suffixMap.remove(suffix);
|
||||
// Recalculate _optimizeSuffix
|
||||
_optimizedSuffix = canBeOptimized(PathSpecGroup.SUFFIX_GLOB);
|
||||
_orderIsSignificant = nonServletPathSpec();
|
||||
}
|
||||
break;
|
||||
case ROOT:
|
||||
_servletRoot = _mappings.stream()
|
||||
.filter(mapping -> mapping.getPathSpec().getGroup() == PathSpecGroup.ROOT)
|
||||
.filter(mapping -> mapping.getPathSpec() instanceof ServletPathSpec)
|
||||
.findFirst().orElse(null);
|
||||
_orderIsSignificant = nonServletPathSpec();
|
||||
break;
|
||||
case DEFAULT:
|
||||
_servletDefault = _mappings.stream()
|
||||
.filter(mapping -> mapping.getPathSpec().getGroup() == PathSpecGroup.DEFAULT)
|
||||
.filter(mapping -> mapping.getPathSpec() instanceof ServletPathSpec)
|
||||
.findFirst().orElse(null);
|
||||
_orderIsSignificant = nonServletPathSpec();
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -420,6 +549,12 @@ public class PathMappings<E> implements Iterable<MappedResource<E>>, Dumpable
|
|||
.allMatch((mapping) -> mapping.getPathSpec() instanceof ServletPathSpec);
|
||||
}
|
||||
|
||||
private boolean nonServletPathSpec()
|
||||
{
|
||||
return _mappings.stream()
|
||||
.allMatch((mapping) -> mapping.getPathSpec() instanceof ServletPathSpec);
|
||||
}
|
||||
|
||||
@Override
|
||||
public String toString()
|
||||
{
|
||||
|
|
|
@ -13,7 +13,12 @@
|
|||
|
||||
package org.eclipse.jetty.http.pathmap;
|
||||
|
||||
import java.util.stream.Stream;
|
||||
|
||||
import org.junit.jupiter.api.Test;
|
||||
import org.junit.jupiter.params.ParameterizedTest;
|
||||
import org.junit.jupiter.params.provider.Arguments;
|
||||
import org.junit.jupiter.params.provider.MethodSource;
|
||||
|
||||
import static org.hamcrest.CoreMatchers.equalTo;
|
||||
import static org.hamcrest.CoreMatchers.is;
|
||||
|
@ -184,6 +189,9 @@ public class PathMappingsTest
|
|||
// @checkstyle-disable-check : AvoidEscapedUnicodeCharactersCheck
|
||||
p.put(new ServletPathSpec("/\u20ACuro/*"), "11");
|
||||
// @checkstyle-enable-check : AvoidEscapedUnicodeCharactersCheck
|
||||
p.put(new ServletPathSpec("/prefix"), "12");
|
||||
p.put(new ServletPathSpec("/prefix/"), "13");
|
||||
p.put(new ServletPathSpec("/prefix/*"), "14");
|
||||
|
||||
p.put(new ServletPathSpec("/*"), "0");
|
||||
|
||||
|
@ -205,6 +213,10 @@ public class PathMappingsTest
|
|||
assertEquals("0", p.getMatched("/suffix/path.gz").getResource(), "Match longest suffix");
|
||||
assertEquals("5", p.getMatched("/animal/path.gz").getResource(), "prefix rather than suffix");
|
||||
|
||||
assertEquals("12", p.getMatched("/prefix").getResource());
|
||||
assertEquals("13", p.getMatched("/prefix/").getResource());
|
||||
assertEquals("14", p.getMatched("/prefix/info").getResource());
|
||||
|
||||
assertEquals("0", p.getMatched("/Other/path").getResource(), "default");
|
||||
|
||||
assertEquals("10", p.getMatched("/").getResource(), "match / with ''");
|
||||
|
@ -319,30 +331,40 @@ public class PathMappingsTest
|
|||
assertThat(p.get(new RegexPathSpec("/a/b/c")), nullValue());
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testServletMultipleSuffixMappings()
|
||||
public static Stream<Arguments> suffixMatches()
|
||||
{
|
||||
return Stream.of(
|
||||
Arguments.of("/a.b.c.foo", "resourceFoo"),
|
||||
Arguments.of("/a.b.c.bar", "resourceBar"),
|
||||
Arguments.of("/a.b.c.foo.bar", "resourceFooBar"),
|
||||
Arguments.of("/a.b/c.d.foo", "resourceFoo"),
|
||||
Arguments.of("/a.b/c.d.bar", "resourceBar"),
|
||||
Arguments.of("/a.b/c.d.foo.bar", "resourceFooBar"),
|
||||
Arguments.of("/a.b.c.pop", null),
|
||||
Arguments.of("/a.foo.c.pop", null),
|
||||
Arguments.of("/a.foop", null),
|
||||
Arguments.of("/a%2Efoo", null)
|
||||
);
|
||||
}
|
||||
|
||||
@ParameterizedTest
|
||||
@MethodSource("suffixMatches")
|
||||
public void testServletMultipleSuffixMappings(String path, String matched)
|
||||
{
|
||||
PathMappings<String> p = new PathMappings<>();
|
||||
p.put(new ServletPathSpec("*.foo"), "resourceFoo");
|
||||
p.put(new ServletPathSpec("*.bar"), "resourceBar");
|
||||
p.put(new ServletPathSpec("*.foo.bar"), "resourceFooBar");
|
||||
p.put(new ServletPathSpec("*.zed"), "resourceZed");
|
||||
|
||||
MatchedResource<String> matched;
|
||||
|
||||
matched = p.getMatched("/a.b.c.foo");
|
||||
assertThat(matched.getResource(), is("resourceFoo"));
|
||||
|
||||
matched = p.getMatched("/a.b.c.bar");
|
||||
assertThat(matched.getResource(), is("resourceBar"));
|
||||
|
||||
matched = p.getMatched("/a.b.c.pop");
|
||||
assertNull(matched);
|
||||
|
||||
matched = p.getMatched("/a.foo.c.pop");
|
||||
assertNull(matched);
|
||||
|
||||
matched = p.getMatched("/a%2Efoo");
|
||||
assertNull(matched);
|
||||
MatchedResource<String> match = p.getMatched(path);
|
||||
if (matched == null)
|
||||
assertThat(match, nullValue());
|
||||
else
|
||||
{
|
||||
assertThat(match, notNullValue());
|
||||
assertThat(p.getMatched(path).getResource(), equalTo(matched));
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
|
@ -449,4 +471,45 @@ public class PathMappingsTest
|
|||
assertThat(PathSpec.from("^.*"), instanceOf(RegexPathSpec.class));
|
||||
assertThat(PathSpec.from("^/"), instanceOf(RegexPathSpec.class));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testOptimalExactIterative()
|
||||
{
|
||||
PathMappings<String> p = new PathMappings<>();
|
||||
p.put(new ServletPathSpec(""), "resourceR");
|
||||
p.put(new ServletPathSpec("/"), "resourceD");
|
||||
p.put(new ServletPathSpec("/exact"), "resourceE");
|
||||
p.put(new ServletPathSpec("/a/*"), "resourceP");
|
||||
p.put(new ServletPathSpec("*.do"), "resourceS");
|
||||
|
||||
// Add an non-servlet Exact to avoid non iterative
|
||||
RegexPathSpec regexPathSpec = new RegexPathSpec("^/some/exact$");
|
||||
p.put(regexPathSpec, "someExact");
|
||||
|
||||
assertThat(p.getMatched("/").getResource(), equalTo("resourceR"));
|
||||
assertThat(p.getMatched("/something").getResource(), equalTo("resourceD"));
|
||||
assertThat(p.getMatched("/exact").getResource(), equalTo("resourceE"));
|
||||
assertThat(p.getMatched("/a").getResource(), equalTo("resourceP"));
|
||||
assertThat(p.getMatched("/a/").getResource(), equalTo("resourceP"));
|
||||
assertThat(p.getMatched("/a/info").getResource(), equalTo("resourceP"));
|
||||
assertThat(p.getMatched("/foo.do").getResource(), equalTo("resourceS"));
|
||||
assertThat(p.getMatched("/a/foo.do").getResource(), equalTo("resourceP"));
|
||||
assertThat(p.getMatched("/b/foo.do").getResource(), equalTo("resourceS"));
|
||||
|
||||
// Make regex a prefix match to test iterative exact match code
|
||||
p.remove(regexPathSpec);
|
||||
regexPathSpec = new RegexPathSpec("/prefix/.*");
|
||||
p.put(regexPathSpec, "somePrefix");
|
||||
|
||||
assertThat(p.getMatched("/").getResource(), equalTo("resourceR"));
|
||||
assertThat(p.getMatched("/something").getResource(), equalTo("resourceD"));
|
||||
assertThat(p.getMatched("/exact").getResource(), equalTo("resourceE"));
|
||||
assertThat(p.getMatched("/a").getResource(), equalTo("resourceP"));
|
||||
assertThat(p.getMatched("/a/").getResource(), equalTo("resourceP"));
|
||||
assertThat(p.getMatched("/a/info").getResource(), equalTo("resourceP"));
|
||||
assertThat(p.getMatched("/foo.do").getResource(), equalTo("resourceS"));
|
||||
assertThat(p.getMatched("/a/foo.do").getResource(), equalTo("resourceP"));
|
||||
assertThat(p.getMatched("/b/foo.do").getResource(), equalTo("resourceS"));
|
||||
|
||||
}
|
||||
}
|
||||
|
|
Loading…
Reference in New Issue