Skip to content

Commit

Permalink
Optimize PathMappings
Browse files Browse the repository at this point in the history 
Improve exact matching
  • Loading branch information
@gregw
gregw committed Dec 16, 2022
1 parent 91da887 commit 943aa3e
Show file tree
Hide file tree
Showing 3 changed files with 91 additions and 44 deletions.
23 changes: 23 additions & 0 deletions jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/MappedResource.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

/**
Expand Down
105 changes: 61 additions & 44 deletions jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/PathMappings.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,10 @@
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;
Expand All @@ -43,12 +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));

private boolean _nonServletPathSpecs;
/**
* 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)
Expand Down Expand Up @@ -94,7 +97,7 @@ public void reset()
_optimizedExact = true;
_optimizedPrefix = true;
_optimizedSuffix = true;
_nonServletPathSpecs = false;
_orderIsSignificant = false;
_servletRoot = null;
_servletDefault = null;
}
Expand Down Expand Up @@ -132,11 +135,12 @@ public List<MatchedResource<E>> getMatchedList(String path)
*/
public List<MappedResource<E>> getMatches(String path)
{
boolean isRootPath = "/".equals(path);

if (_mappings.isEmpty())
return Collections.emptyList();

boolean isRootPath = "/".equals(path);

// Iterator over all the mapping, adding only those that match.
List<MappedResource<E>> matches = null;
for (MappedResource<E> mr : _mappings)
{
Expand Down Expand Up @@ -171,33 +175,44 @@ public List<MappedResource<E>> getMatches(String path)
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 (_nonServletPathSpecs)
return getMatchedMixed(path);
// 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 new MatchedResource<>(_servletRoot.getResource(), _servletRoot.getPathSpec(), _servletRoot.getPathSpec().matched(path));
return _servletRoot.getPreMatched();

MappedResource<E> candidate = _exactMap.getBest(path);
if (candidate != null)
{
MatchedPath matchedPath = candidate.getPathSpec().matched(path);
if (matchedPath != null)
return new MatchedResource<>(candidate.getResource(), candidate.getPathSpec(), matchedPath);
}
// try an exact match
MappedResource<E> exact = _exactMap.get(path);
if (exact != null)
return exact.getPreMatched();

candidate = _prefixMap.getBest(path);
if (candidate != null)
// Try a prefix match
MappedResource<E> prefix = _prefixMap.getBest(path);
if (prefix != null)
{
MatchedPath matchedPath = candidate.getPathSpec().matched(path);
MatchedPath matchedPath = prefix.getPathSpec().matched(path);
if (matchedPath != null)
return new MatchedResource<>(candidate.getResource(), candidate.getPathSpec(), matchedPath);
return new MatchedResource<>(prefix.getResource(), prefix.getPathSpec(), matchedPath);
}

// Try a suffix match
if (!_suffixMap.isEmpty())
{
int i = Math.max(0, path.lastIndexOf("/"));
Expand All @@ -208,13 +223,13 @@ public MatchedResource<E> getMatched(String path)
// Loop 3: "foo"
while ((i = path.indexOf('.', i + 1)) > 0)
{
candidate = _suffixMap.get(path, i + 1, path.length() - i - 1);
if (candidate == null)
prefix = _suffixMap.get(path, i + 1, path.length() - i - 1);
if (prefix == null)
continue;

MatchedPath matchedPath = candidate.getPathSpec().matched(path);
MatchedPath matchedPath = prefix.getPathSpec().matched(path);
if (matchedPath != null)
return new MatchedResource<>(candidate.getResource(), candidate.getPathSpec(), matchedPath);
return new MatchedResource<>(prefix.getResource(), prefix.getPathSpec(), matchedPath);
}
}

Expand All @@ -224,7 +239,13 @@ public MatchedResource<E> getMatched(String path)
return null;
}

private MatchedResource<E> getMatchedMixed(String path)
/**
* <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;
Expand Down Expand Up @@ -252,13 +273,9 @@ private MatchedResource<E> getMatchedMixed(String path)
{
if (_optimizedExact)
{
MappedResource<E> candidate = _exactMap.getBest(path);
if (candidate != null)
{
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;
}
Expand Down Expand Up @@ -387,7 +404,7 @@ public boolean put(PathSpec pathSpec, E resource)
// 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;
_nonServletPathSpecs = true;
_orderIsSignificant = true;
}
break;
case PREFIX_GLOB:
Expand All @@ -404,7 +421,7 @@ public boolean put(PathSpec pathSpec, E resource)
// 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;
_nonServletPathSpecs = true;
_orderIsSignificant = true;
}
break;
case SUFFIX_GLOB:
Expand All @@ -421,7 +438,7 @@ public boolean put(PathSpec pathSpec, E resource)
// 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;
_nonServletPathSpecs = true;
_orderIsSignificant = true;
}
break;
case ROOT:
Expand All @@ -432,7 +449,7 @@ public boolean put(PathSpec pathSpec, E resource)
}
else
{
_nonServletPathSpecs = true;
_orderIsSignificant = true;
}
break;
case DEFAULT:
Expand All @@ -443,7 +460,7 @@ public boolean put(PathSpec pathSpec, E resource)
}
else
{
_nonServletPathSpecs = true;
_orderIsSignificant = true;
}
break;
default:
Expand Down Expand Up @@ -482,7 +499,7 @@ public boolean remove(PathSpec pathSpec)
_exactMap.remove(exact);
// Recalculate _optimizeExact
_optimizedExact = canBeOptimized(PathSpecGroup.EXACT);
_nonServletPathSpecs = nonServletPathSpec();
_orderIsSignificant = nonServletPathSpec();
}
break;
case PREFIX_GLOB:
Expand All @@ -492,7 +509,7 @@ public boolean remove(PathSpec pathSpec)
_prefixMap.remove(prefix);
// Recalculate _optimizePrefix
_optimizedPrefix = canBeOptimized(PathSpecGroup.PREFIX_GLOB);
_nonServletPathSpecs = nonServletPathSpec();
_orderIsSignificant = nonServletPathSpec();
}
break;
case SUFFIX_GLOB:
Expand All @@ -502,22 +519,22 @@ public boolean remove(PathSpec pathSpec)
_suffixMap.remove(suffix);
// Recalculate _optimizeSuffix
_optimizedSuffix = canBeOptimized(PathSpecGroup.SUFFIX_GLOB);
_nonServletPathSpecs = nonServletPathSpec();
_orderIsSignificant = nonServletPathSpec();
}
break;
case ROOT:
_servletRoot = _mappings.stream()
.filter(mapping -> mapping.getPathSpec().getGroup() == PathSpecGroup.ROOT)
.filter(mapping -> mapping.getPathSpec() instanceof ServletPathSpec)
.findFirst().orElse(null);
_nonServletPathSpecs = nonServletPathSpec();
_orderIsSignificant = nonServletPathSpec();
break;
case DEFAULT:
_servletDefault = _mappings.stream()
.filter(mapping -> mapping.getPathSpec().getGroup() == PathSpecGroup.DEFAULT)
.filter(mapping -> mapping.getPathSpec() instanceof ServletPathSpec)
.findFirst().orElse(null);
_nonServletPathSpecs = nonServletPathSpec();
_orderIsSignificant = nonServletPathSpec();
break;
}
}
Expand Down
7 changes: 7 additions & 0 deletions jetty-http/src/test/java/org/eclipse/jetty/http/pathmap/PathMappingsTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,9 @@ public void testPathMap()
// @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");

Expand All @@ -210,6 +213,10 @@ public void testPathMap()
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 ''");
Expand Down

0 comments on commit 943aa3e

Please sign in to comment.