FELIX-4888 : ServletHandler's are not sorted by longest matching path. Pattern matching (WiP)
git-svn-id: https://svn.apache.org/repos/asf/felix/trunk@1680465 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/http/base/src/main/java/org/apache/felix/http/base/internal/registry/HandlerRegistry.java b/http/base/src/main/java/org/apache/felix/http/base/internal/registry/HandlerRegistry.java
index 54b83c4..1cc83f2 100644
--- a/http/base/src/main/java/org/apache/felix/http/base/internal/registry/HandlerRegistry.java
+++ b/http/base/src/main/java/org/apache/felix/http/base/internal/registry/HandlerRegistry.java
@@ -231,7 +231,7 @@
}
- public PathResolution resolveServlet(final String requestURI)
+ public PathResolution resolveServlet(@Nonnull final String requestURI)
{
final List<PerContextHandlerRegistry> regs = this.registrations;
for(final PerContextHandlerRegistry r : regs)
diff --git a/http/base/src/main/java/org/apache/felix/http/base/internal/registry/PathResolution.java b/http/base/src/main/java/org/apache/felix/http/base/internal/registry/PathResolution.java
index fc6b2cf..8287f33 100644
--- a/http/base/src/main/java/org/apache/felix/http/base/internal/registry/PathResolution.java
+++ b/http/base/src/main/java/org/apache/felix/http/base/internal/registry/PathResolution.java
@@ -24,4 +24,6 @@
public String pathInfo;
public String requestURI;
+
+ public String[] patterns;
}
diff --git a/http/base/src/main/java/org/apache/felix/http/base/internal/registry/PathResolver.java b/http/base/src/main/java/org/apache/felix/http/base/internal/registry/PathResolver.java
new file mode 100644
index 0000000..9de1f39
--- /dev/null
+++ b/http/base/src/main/java/org/apache/felix/http/base/internal/registry/PathResolver.java
@@ -0,0 +1,27 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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.apache.felix.http.base.internal.registry;
+
+
+public interface PathResolver extends Comparable<PathResolver> {
+
+ PathResolution match(String uri);
+
+ int getRanking();
+
+ int getOrdering();
+}
diff --git a/http/base/src/main/java/org/apache/felix/http/base/internal/registry/PathResolverFactory.java b/http/base/src/main/java/org/apache/felix/http/base/internal/registry/PathResolverFactory.java
new file mode 100644
index 0000000..57535c2
--- /dev/null
+++ b/http/base/src/main/java/org/apache/felix/http/base/internal/registry/PathResolverFactory.java
@@ -0,0 +1,228 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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.apache.felix.http.base.internal.registry;
+
+/**
+ * The path resolver factory creates a path resolver for a pattern.
+ * The servlet spec supports different patterns
+ * - path mapping, a pattern starting with / and ending with /*
+ * - extension mapping, a pattern starting with *.
+ * - default mapping, the pattern /
+ * - root mapping, the pattern is the empty string
+ * - exact match
+ *
+ * Exact match is tried first, followed by longest match and finally
+ * extension match.
+ */
+public abstract class PathResolverFactory {
+
+ public static PathResolver create(final String pattern)
+ {
+ if ( pattern.length() == 0 )
+ {
+ return new RootMatcher();
+ }
+ else if ( pattern.equals("/") )
+ {
+ return new DefaultMatcher();
+ }
+ else if ( pattern.startsWith("*.") )
+ {
+ return new ExtensionMatcher(pattern.substring(1));
+ }
+ else if ( pattern.endsWith("/*") )
+ {
+ return new PathMatcher(pattern);
+ }
+ return new ExactAndPathMatcher(pattern);
+ }
+
+ public static abstract class AbstractMatcher implements PathResolver
+ {
+ private final int ranking;
+
+ public AbstractMatcher(final int ranking)
+ {
+ this.ranking = ranking;
+ }
+
+ @Override
+ public int compareTo(final PathResolver o) {
+ int result = o.getRanking() - this.ranking;
+ if ( result == 0 )
+ {
+ result = o.getOrdering() - this.getOrdering();
+ }
+ return result;
+ }
+
+ @Override
+ public int getRanking() {
+ return this.ranking;
+ }
+
+ @Override
+ public int getOrdering() {
+ return 0;
+ }
+ }
+
+ public static final class RootMatcher extends AbstractMatcher
+ {
+ public RootMatcher()
+ {
+ super(2);
+ }
+
+ @Override
+ public PathResolution match(final String uri) {
+ if ( uri.length() == 0 )
+ {
+ final PathResolution pr = new PathResolution();
+ pr.pathInfo = "/";
+ pr.servletPath = "";
+ pr.requestURI = "";
+
+ return pr;
+ }
+ return null;
+ }
+ }
+
+ public static final class DefaultMatcher extends AbstractMatcher
+ {
+ public DefaultMatcher()
+ {
+ super(1);
+ }
+
+ @Override
+ public PathResolution match(final String uri) {
+ final PathResolution pr = new PathResolution();
+ pr.pathInfo = null;
+ pr.servletPath = uri;
+ pr.requestURI = uri;
+
+ return pr;
+ }
+ }
+
+ public static final class ExactAndPathMatcher extends AbstractMatcher
+ {
+ private final String path;
+
+ private final String prefix;
+
+ public ExactAndPathMatcher(final String pattern)
+ {
+ super(4);
+ this.path = pattern;
+ this.prefix = pattern.concat("/");
+ }
+
+ @Override
+ public PathResolution match(final String uri) {
+ if ( uri.equals(this.path) )
+ {
+ final PathResolution pr = new PathResolution();
+ pr.pathInfo = null;
+ pr.servletPath = uri;
+ pr.requestURI = uri;
+
+ return pr;
+ }
+ else if ( uri.startsWith(prefix) )
+ {
+ final PathResolution pr = new PathResolution();
+ pr.servletPath = this.prefix.substring(0, this.prefix.length() - 1);
+ pr.pathInfo = uri.substring(pr.servletPath.length());
+ pr.requestURI = uri;
+
+ return pr;
+ }
+ return null;
+ }
+
+ @Override
+ public int getOrdering()
+ {
+ return this.path.length();
+ }
+ }
+
+ public static final class PathMatcher extends AbstractMatcher
+ {
+ private final String prefix;
+
+ public PathMatcher(final String pattern)
+ {
+ super(4);
+ this.prefix = pattern.substring(0, pattern.length() - 1);
+ }
+
+ @Override
+ public PathResolution match(final String uri) {
+ if ( uri.startsWith(this.prefix) )
+ {
+ final PathResolution pr = new PathResolution();
+ pr.servletPath = this.prefix.substring(0, this.prefix.length() - 1);
+ pr.pathInfo = uri.substring(pr.servletPath.length());
+ pr.requestURI = uri;
+
+ return pr;
+ }
+ return null;
+ }
+
+ @Override
+ public int getOrdering()
+ {
+ return this.prefix.length() + 1;
+ }
+ }
+
+ public static final class ExtensionMatcher extends AbstractMatcher
+ {
+ private final String extension;
+
+ public ExtensionMatcher(final String extension)
+ {
+ super(3);
+ this.extension = extension;
+ }
+
+ @Override
+ public PathResolution match(final String uri) {
+ if ( uri.endsWith(this.extension) )
+ {
+ final PathResolution pr = new PathResolution();
+ pr.pathInfo = null;
+ pr.servletPath = uri;
+ pr.requestURI = uri;
+
+ return pr;
+ }
+ return null;
+ }
+
+ @Override
+ public int getOrdering()
+ {
+ return this.extension.length();
+ }
+ }
+}
diff --git a/http/base/src/main/java/org/apache/felix/http/base/internal/registry/PerContextHandlerRegistry.java b/http/base/src/main/java/org/apache/felix/http/base/internal/registry/PerContextHandlerRegistry.java
index c403083..a5888c7 100644
--- a/http/base/src/main/java/org/apache/felix/http/base/internal/registry/PerContextHandlerRegistry.java
+++ b/http/base/src/main/java/org/apache/felix/http/base/internal/registry/PerContextHandlerRegistry.java
@@ -116,7 +116,7 @@
return result;
}
- public String isMatching(final String requestURI)
+ public String isMatching(@Nonnull final String requestURI)
{
if (requestURI.equals(this.path))
{
@@ -133,7 +133,7 @@
return null;
}
- public PathResolution resolve(final String relativeRequestURI)
+ public PathResolution resolve(@Nonnull final String relativeRequestURI)
{
return this.servletRegistry.resolve(relativeRequestURI);
}
diff --git a/http/base/src/main/java/org/apache/felix/http/base/internal/registry/ServletRegistration.java b/http/base/src/main/java/org/apache/felix/http/base/internal/registry/ServletRegistration.java
index cec8959..af9be43 100644
--- a/http/base/src/main/java/org/apache/felix/http/base/internal/registry/ServletRegistration.java
+++ b/http/base/src/main/java/org/apache/felix/http/base/internal/registry/ServletRegistration.java
@@ -16,26 +16,22 @@
*/
package org.apache.felix.http.base.internal.registry;
-import java.util.regex.Matcher;
-import java.util.regex.Pattern;
-
import javax.annotation.Nonnull;
import org.apache.felix.http.base.internal.handler.ServletHandler;
-import org.apache.felix.http.base.internal.util.UriUtils;
/**
- * Servlet is registered with a pattern and a servlet handler
+ * A servlet is registered with a resolver and a servlet handler
*/
public class ServletRegistration
{
private final ServletHandler handler;
- private final Pattern pattern;
+ private final PathResolver resolver;
- public ServletRegistration(@Nonnull final ServletHandler handler, @Nonnull final Pattern pattern)
+ public ServletRegistration(@Nonnull final ServletHandler handler, @Nonnull final PathResolver resolver)
{
this.handler = handler;
- this.pattern = pattern;
+ this.resolver = resolver;
}
public ServletHandler getServletHandler()
@@ -43,19 +39,18 @@
return this.handler;
}
+ public PathResolver getPathResolver()
+ {
+ return this.resolver;
+ }
+
public PathResolution resolve(@Nonnull final String requestURI)
{
- final Matcher matcher = pattern.matcher(requestURI);
- if (matcher.find(0))
+ final PathResolution pr = this.resolver.match(requestURI);
+ if ( pr != null )
{
- final PathResolution pr = new PathResolution();
- pr.servletPath = matcher.groupCount() > 0 ? matcher.group(1) : matcher.group();
- pr.pathInfo = UriUtils.compactPath(UriUtils.relativePath(pr.servletPath, requestURI));
pr.handler = this.handler;
-
- return pr;
}
-
- return null;
+ return pr;
}
}
diff --git a/http/base/src/main/java/org/apache/felix/http/base/internal/registry/ServletRegistry.java b/http/base/src/main/java/org/apache/felix/http/base/internal/registry/ServletRegistry.java
index 9f1771d..b267d8b 100644
--- a/http/base/src/main/java/org/apache/felix/http/base/internal/registry/ServletRegistry.java
+++ b/http/base/src/main/java/org/apache/felix/http/base/internal/registry/ServletRegistry.java
@@ -24,7 +24,6 @@
import java.util.List;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
-import java.util.regex.Pattern;
import javax.annotation.Nonnull;
@@ -32,7 +31,6 @@
import org.apache.felix.http.base.internal.runtime.ServletInfo;
import org.apache.felix.http.base.internal.runtime.dto.ResourceDTOBuilder;
import org.apache.felix.http.base.internal.runtime.dto.ServletDTOBuilder;
-import org.apache.felix.http.base.internal.util.PatternUtil;
import org.osgi.service.http.runtime.dto.DTOConstants;
import org.osgi.service.http.runtime.dto.FailedResourceDTO;
import org.osgi.service.http.runtime.dto.FailedServletDTO;
@@ -44,11 +42,8 @@
* The servlet registry keeps the mappings for all servlets (by using their pattern)
* for a single servlet context.
*
- * TODO - servlet name handling
- *
* TODO - sort active servlet mappings by pattern length, longest first (avoids looping over all)
*
- * TODO - replace patterns with own matchers
*/
public final class ServletRegistry
{
@@ -66,17 +61,25 @@
public ServletHandler handler;
}
- public PathResolution resolve(final String relativeRequestURI)
+ /**
+ * Resolve a request uri
+ *
+ * @param relativeRequestURI The request uri
+ * @return A path resolution if a servlet matched, {@code null} otherwise
+ */
+ public PathResolution resolve(@Nonnull final String relativeRequestURI)
{
- int len = -1;
+ PathResolver resolver = null;
PathResolution candidate = null;
for(final Map.Entry<String, ServletRegistration> entry : this.activeServletMappings.entrySet())
{
final PathResolution pr = entry.getValue().resolve(relativeRequestURI);
- if ( pr != null && entry.getKey().length() > len )
+ if ( pr != null && (resolver == null || entry.getValue().getPathResolver().compareTo(resolver) < 0) )
{
+ // TODO - we should have all patterns under which this servlet is actively registered
+ pr.patterns = new String[] {entry.getKey()};
candidate = pr;
- len = entry.getKey().length();
+ resolver = entry.getValue().getPathResolver();
}
}
return candidate;
@@ -287,8 +290,7 @@
final int result = handler.init();
if ( result == -1 )
{
- final Pattern p = Pattern.compile(PatternUtil.convertToRegEx(pattern));
- final ServletRegistration reg = new ServletRegistration(handler, p);
+ final ServletRegistration reg = new ServletRegistration(handler, PathResolverFactory.create(pattern));
this.activeServletMappings.put(pattern, reg);
// add ok
diff --git a/http/base/src/main/java/org/apache/felix/http/base/internal/runtime/ServletInfo.java b/http/base/src/main/java/org/apache/felix/http/base/internal/runtime/ServletInfo.java
index b2dc85e..0bb70cf 100644
--- a/http/base/src/main/java/org/apache/felix/http/base/internal/runtime/ServletInfo.java
+++ b/http/base/src/main/java/org/apache/felix/http/base/internal/runtime/ServletInfo.java
@@ -121,14 +121,7 @@
{
super(serviceRanking);
this.name = name;
- if ( pattern.equals("/") )
- {
- this.patterns = new String[] {"/", "/*"};
- }
- else
- {
- this.patterns = new String[] {pattern, pattern + "/*"};
- }
+ this.patterns = new String[] {pattern};
this.initParams = Collections.unmodifiableMap(initParams);
this.asyncSupported = true;
this.errorPage = null;
diff --git a/http/base/src/main/java/org/apache/felix/http/base/internal/runtime/dto/RequestInfoDTOBuilder.java b/http/base/src/main/java/org/apache/felix/http/base/internal/runtime/dto/RequestInfoDTOBuilder.java
index dc173ae..542f584 100644
--- a/http/base/src/main/java/org/apache/felix/http/base/internal/runtime/dto/RequestInfoDTOBuilder.java
+++ b/http/base/src/main/java/org/apache/felix/http/base/internal/runtime/dto/RequestInfoDTOBuilder.java
@@ -51,12 +51,12 @@
if (pr.handler.getServletInfo().isResource())
{
requestInfoDTO.resourceDTO = ResourceDTOBuilder.build(pr.handler, -1);
- requestInfoDTO.resourceDTO.patterns = pr.handler.getServletInfo().getPatterns();
+ requestInfoDTO.resourceDTO.patterns = pr.patterns;
}
else
{
requestInfoDTO.servletDTO = ServletDTOBuilder.build(pr.handler, -1);
- requestInfoDTO.servletDTO.patterns = pr.handler.getServletInfo().getPatterns();
+ requestInfoDTO.servletDTO.patterns = pr.patterns;
}
final FilterHandler[] filterHandlers = registry.getFilters(pr, null, path);
diff --git a/http/base/src/test/java/org/apache/felix/http/base/internal/registry/ServletRegistryTest.java b/http/base/src/test/java/org/apache/felix/http/base/internal/registry/ServletRegistryTest.java
index 55c1571..f4ffa15 100644
--- a/http/base/src/test/java/org/apache/felix/http/base/internal/registry/ServletRegistryTest.java
+++ b/http/base/src/test/java/org/apache/felix/http/base/internal/registry/ServletRegistryTest.java
@@ -209,6 +209,57 @@
assertEmpty(dto, holder);
}
+ @Test public void testServletOrdering() throws InvalidSyntaxException
+ {
+ final ServletHandler h1 = createServletHandler(1L, 0, "/foo");
+ reg.addServlet(h1);
+ final ServletHandler h2 = createServletHandler(2L, 0, "/foo/*");
+ reg.addServlet(h2);
+ final ServletHandler h3 = createServletHandler(3L, 0, "/foo/rsrc");
+ reg.addServlet(h3);
+ final ServletHandler h4 = createServletHandler(4L, 0, "/foo/rsrc/*");
+ reg.addServlet(h4);
+ final ServletHandler h5 = createServletHandler(5L, 0, "/other");
+ reg.addServlet(h5);
+
+ PathResolution pr = null;
+
+ pr = reg.resolve("/foo");
+ assertNotNull(pr);
+ assertEquals("/foo", pr.patterns[0]);
+ assertEquals(h1, pr.handler);
+
+ pr = reg.resolve("/fool");
+ assertNull(pr);
+
+ pr = reg.resolve("/foo/bar");
+ assertEquals("/foo/*", pr.patterns[0]);
+ assertEquals(h2, pr.handler);
+
+ pr = reg.resolve("/foo/rsrc");
+ assertEquals("/foo/rsrc", pr.patterns[0]);
+ assertEquals(h3, pr.handler);
+
+ pr = reg.resolve("/foo/rsrc/some");
+ assertEquals("/foo/rsrc/*", pr.patterns[0]);
+ assertEquals(h4, pr.handler);
+
+ pr = reg.resolve("/other");
+ assertEquals("/other", pr.patterns[0]);
+ assertEquals(h5, pr.handler);
+
+ pr = reg.resolve("/other/bla");
+ assertEquals("/other", pr.patterns[0]);
+ assertEquals(h5, pr.handler);
+
+ // cleanup
+ reg.removeServlet(h1.getServletInfo(), true);
+ reg.removeServlet(h2.getServletInfo(), true);
+ reg.removeServlet(h3.getServletInfo(), true);
+ reg.removeServlet(h4.getServletInfo(), true);
+ reg.removeServlet(h5.getServletInfo(), true);
+ }
+
private static ServletInfo createServletInfo(final long id, final int ranking, final String... paths) throws InvalidSyntaxException
{
final BundleContext bCtx = mock(BundleContext.class);