You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@felix.apache.org by cz...@apache.org on 2015/05/25 12:26:44 UTC
svn commit: r1681578 - in
/felix/trunk/http/base/src/main/java/org/apache/felix/http/base/internal/registry:
PathResolver.java PathResolverFactory.java ServletRegistry.java
Author: cziegeler
Date: Mon May 25 10:26:44 2015
New Revision: 1681578
URL: http://svn.apache.org/r1681578
Log:
FELIX-4860 : Revisit HandlerRegistry implementation. Improve servlet registry by sorting patterns, highest matching first
Modified:
felix/trunk/http/base/src/main/java/org/apache/felix/http/base/internal/registry/PathResolver.java
felix/trunk/http/base/src/main/java/org/apache/felix/http/base/internal/registry/PathResolverFactory.java
felix/trunk/http/base/src/main/java/org/apache/felix/http/base/internal/registry/ServletRegistry.java
Modified: felix/trunk/http/base/src/main/java/org/apache/felix/http/base/internal/registry/PathResolver.java
URL: http://svn.apache.org/viewvc/felix/trunk/http/base/src/main/java/org/apache/felix/http/base/internal/registry/PathResolver.java?rev=1681578&r1=1681577&r2=1681578&view=diff
==============================================================================
--- felix/trunk/http/base/src/main/java/org/apache/felix/http/base/internal/registry/PathResolver.java (original)
+++ felix/trunk/http/base/src/main/java/org/apache/felix/http/base/internal/registry/PathResolver.java Mon May 25 10:26:44 2015
@@ -28,4 +28,6 @@ public interface PathResolver extends Co
int getRanking();
int getOrdering();
+
+ String getPattern();
}
Modified: felix/trunk/http/base/src/main/java/org/apache/felix/http/base/internal/registry/PathResolverFactory.java
URL: http://svn.apache.org/viewvc/felix/trunk/http/base/src/main/java/org/apache/felix/http/base/internal/registry/PathResolverFactory.java?rev=1681578&r1=1681577&r2=1681578&view=diff
==============================================================================
--- felix/trunk/http/base/src/main/java/org/apache/felix/http/base/internal/registry/PathResolverFactory.java (original)
+++ felix/trunk/http/base/src/main/java/org/apache/felix/http/base/internal/registry/PathResolverFactory.java Mon May 25 10:26:44 2015
@@ -49,7 +49,7 @@ public abstract class PathResolverFactor
}
else if ( pattern.startsWith("*.") )
{
- return new ExtensionMatcher(handler, pattern.substring(1));
+ return new ExtensionMatcher(handler, pattern);
}
else if ( pattern.endsWith("/*") )
{
@@ -67,16 +67,20 @@ public abstract class PathResolverFactor
{
private final int ranking;
+ private final String pattern;
+
private final ServletHandler handler;
- public AbstractMatcher(final ServletHandler handler, final int ranking)
+ public AbstractMatcher(final ServletHandler handler, final String pattern, final int ranking)
{
this.handler = handler;
this.ranking = ranking;
+ this.pattern = pattern;
}
@Override
- public int compareTo(final PathResolver o) {
+ public int compareTo(final PathResolver o)
+ {
int result = o.getRanking() - this.ranking;
if ( result == 0 )
{
@@ -86,26 +90,55 @@ public abstract class PathResolverFactor
}
@Override
- public ServletHandler getServletHandler() {
+ public ServletHandler getServletHandler()
+ {
return this.handler;
}
@Override
- public int getRanking() {
+ public int getRanking()
+ {
return this.ranking;
}
@Override
- public int getOrdering() {
+ public int getOrdering()
+ {
return 0;
}
+
+ @Override
+ public String getPattern()
+ {
+ return this.pattern;
+ }
+
+ @Override
+ public int hashCode()
+ {
+ return pattern.hashCode();
+ }
+
+ @Override
+ public boolean equals(final Object obj) {
+ if (this == obj)
+ {
+ return true;
+ }
+ if (obj == null || getClass() != obj.getClass())
+ {
+ return false;
+ }
+ final AbstractMatcher other = (AbstractMatcher) obj;
+ return this.pattern.equals(other.pattern);
+ }
}
public static final class RootMatcher extends AbstractMatcher
{
public RootMatcher(final ServletHandler handler)
{
- super(handler, 2);
+ super(handler, "", 2);
}
@Override
@@ -128,7 +161,7 @@ public abstract class PathResolverFactor
{
public DefaultMatcher(final ServletHandler handler)
{
- super(handler, 1);
+ super(handler, "/", 1);
}
@Override
@@ -151,7 +184,7 @@ public abstract class PathResolverFactor
public ExactAndPathMatcher(final ServletHandler handler, final String pattern)
{
- super(handler, 4);
+ super(handler, pattern, 4);
this.path = pattern;
this.prefix = pattern.concat("/");
}
@@ -194,7 +227,7 @@ public abstract class PathResolverFactor
public PathMatcher(final ServletHandler handler, final String pattern)
{
- super(handler, 4);
+ super(handler, pattern, 4);
this.prefix = pattern.substring(0, pattern.length() - 1);
}
@@ -224,10 +257,10 @@ public abstract class PathResolverFactor
{
private final String extension;
- public ExtensionMatcher(final ServletHandler handler, final String extension)
+ public ExtensionMatcher(final ServletHandler handler, final String pattern)
{
- super(handler, 3);
- this.extension = extension;
+ super(handler, pattern, 3);
+ this.extension = pattern.substring(1);
}
@Override
@@ -258,7 +291,7 @@ public abstract class PathResolverFactor
public RegexMatcher(final String regex)
{
- super(null, 0);
+ super(null, regex, 0);
this.pattern = Pattern.compile(regex);
}
Modified: felix/trunk/http/base/src/main/java/org/apache/felix/http/base/internal/registry/ServletRegistry.java
URL: http://svn.apache.org/viewvc/felix/trunk/http/base/src/main/java/org/apache/felix/http/base/internal/registry/ServletRegistry.java?rev=1681578&r1=1681577&r2=1681578&view=diff
==============================================================================
--- felix/trunk/http/base/src/main/java/org/apache/felix/http/base/internal/registry/ServletRegistry.java (original)
+++ felix/trunk/http/base/src/main/java/org/apache/felix/http/base/internal/registry/ServletRegistry.java Mon May 25 10:26:44 2015
@@ -43,13 +43,10 @@ import org.osgi.service.http.runtime.dto
/**
* The servlet registry keeps the mappings for all servlets (by using their pattern)
* for a single servlet context.
- *
- * TODO - sort active servlet mappings by pattern length, longest first (avoids looping over all)
- * TODO - we could collapse all three maps into a single map (activeServletMappings, inactiveServletMappings and statusMapping)
*/
public final class ServletRegistry
{
- private final Map<String, PathResolver> activeServletMappings = new ConcurrentHashMap<String, PathResolver>();
+ private volatile List<PathResolver> activeResolvers = Collections.emptyList();
private final Map<String, List<ServletHandler>> inactiveServletMappings = new HashMap<String, List<ServletHandler>>();
@@ -71,20 +68,30 @@ public final class ServletRegistry
*/
public PathResolution resolve(@Nonnull final String relativeRequestURI)
{
- PathResolver resolver = null;
- PathResolution candidate = null;
- for(final Map.Entry<String, PathResolver> entry : this.activeServletMappings.entrySet())
+ final List<PathResolver> resolvers = this.activeResolvers;
+ for(final PathResolver entry : resolvers)
{
- final PathResolution pr = entry.getValue().resolve(relativeRequestURI);
- if ( pr != null && (resolver == null || entry.getValue().compareTo(resolver) < 0) )
+ final PathResolution pr = entry.resolve(relativeRequestURI);
+ if ( pr != null )
{
// TODO - we should have all patterns under which this servlet is actively registered
- pr.patterns = new String[] {entry.getKey()};
- candidate = pr;
- resolver = entry.getValue();
+ pr.patterns = new String[] {entry.getPattern()};
+ return pr;
}
}
- return candidate;
+ return null;
+ }
+
+ private PathResolver findResolver(final List<PathResolver> resolvers, final String pattern)
+ {
+ for(final PathResolver pr : resolvers)
+ {
+ if ( pr.getPattern().equals(pattern) )
+ {
+ return pr;
+ }
+ }
+ return null;
}
/**
@@ -98,6 +105,8 @@ public final class ServletRegistry
// Can be null in case of error-handling servlets...
if ( handler.getServletInfo().getPatterns() != null )
{
+ final List<PathResolver> resolvers = new ArrayList<PathResolver>(this.activeResolvers);
+
final ServletRegistrationStatus status = new ServletRegistrationStatus();
status.handler = handler;
@@ -111,13 +120,13 @@ public final class ServletRegistry
continue;
}
patterns.add(pattern);
- final PathResolver regHandler = this.activeServletMappings.get(pattern);
+ final PathResolver regHandler = findResolver(resolvers, pattern);
if ( regHandler != null )
{
if ( regHandler.getServletHandler().getServletInfo().compareTo(handler.getServletInfo()) > 0 )
{
// replace if no error with new servlet
- if ( this.tryToActivate(pattern, handler, status) )
+ if ( this.tryToActivate(resolvers, pattern, handler, status, regHandler) )
{
isActive = true;
final String oldName = regHandler.getServletHandler().getName();
@@ -140,7 +149,7 @@ public final class ServletRegistry
else
{
// add to active
- if ( this.tryToActivate(pattern, handler, status) )
+ if ( this.tryToActivate(resolvers, pattern, handler, status, null) )
{
isActive = true;
}
@@ -151,6 +160,8 @@ public final class ServletRegistry
{
addToNameMapping(handler);
}
+ Collections.sort(resolvers);
+ this.activeResolvers = resolvers;
}
}
@@ -213,6 +224,8 @@ public final class ServletRegistry
{
if ( info.getPatterns() != null )
{
+ final List<PathResolver> resolvers = new ArrayList<PathResolver>(this.activeResolvers);
+
this.statusMapping.remove(info);
ServletHandler cleanupHandler = null;
@@ -225,7 +238,7 @@ public final class ServletRegistry
continue;
}
patterns.add(pattern);
- final PathResolver regHandler = this.activeServletMappings.get(pattern);
+ final PathResolver regHandler = this.findResolver(resolvers, pattern);
if ( regHandler != null && regHandler.getServletHandler().getServletInfo().equals(info) )
{
cleanupHandler = regHandler.getServletHandler();
@@ -234,7 +247,7 @@ public final class ServletRegistry
final List<ServletHandler> inactiveList = this.inactiveServletMappings.get(pattern);
if ( inactiveList == null )
{
- this.activeServletMappings.remove(pattern);
+ resolvers.remove(regHandler);
}
else
{
@@ -243,7 +256,7 @@ public final class ServletRegistry
{
final ServletHandler h = inactiveList.remove(0);
boolean activate = h.getServlet() == null;
- done = this.tryToActivate(pattern, h, this.statusMapping.get(h.getServletInfo()));
+ done = this.tryToActivate(resolvers, pattern, h, this.statusMapping.get(h.getServletInfo()), regHandler);
if ( !done )
{
done = inactiveList.isEmpty();
@@ -286,6 +299,9 @@ public final class ServletRegistry
}
}
+ Collections.sort(resolvers);
+ this.activeResolvers = resolvers;
+
if ( cleanupHandler != null )
{
cleanupHandler.dispose();
@@ -295,7 +311,7 @@ public final class ServletRegistry
public synchronized void cleanup()
{
- this.activeServletMappings.clear();
+ this.activeResolvers = Collections.emptyList();
this.inactiveServletMappings.clear();
this.servletsByName.clear();
this.statusMapping.clear();
@@ -314,14 +330,22 @@ public final class ServletRegistry
status.pathToStatus.put(pattern, DTOConstants.FAILURE_REASON_SHADOWED_BY_OTHER_SERVICE);
}
- private boolean tryToActivate(final String pattern, final ServletHandler handler, final ServletRegistrationStatus status)
+ private boolean tryToActivate(final List<PathResolver> resolvers,
+ final String pattern,
+ final ServletHandler handler,
+ final ServletRegistrationStatus status,
+ final PathResolver oldResolver)
{
// add to active
final int result = handler.init();
if ( result == -1 )
{
- final PathResolver reg = PathResolverFactory.createPatternMatcher(handler, pattern);
- this.activeServletMappings.put(pattern, reg);
+ if ( oldResolver != null )
+ {
+ resolvers.remove(oldResolver);
+ }
+ final PathResolver resolver = PathResolverFactory.createPatternMatcher(handler, pattern);
+ resolvers.add(resolver);
// add ok
status.pathToStatus.put(pattern, result);