Description: Let sub-classes of GuiceFilter customize the servlet filter pipeline
Author: Stuart McCulloch <mcculls@gmail.com>
Bug-Google: http://code.google.com/p/google-guice/issues/detail?id=618
Last-Update: 2015-01-20

diff --git a/extensions/servlet/src/com/google/inject/servlet/AbstractFilterPipeline.java b/extensions/servlet/src/com/google/inject/servlet/AbstractFilterPipeline.java
new file mode 100644
index 0000000..ab8f746
--- /dev/null
+++ b/extensions/servlet/src/com/google/inject/servlet/AbstractFilterPipeline.java
@@ -0,0 +1,150 @@
+/**
+ * Copyright (C) 2008 Google Inc.
+ *
+ * Licensed 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 com.google.inject.servlet;
+
+import com.google.common.collect.Sets;
+import com.google.inject.Injector;
+import com.google.inject.Provider;
+
+import java.io.IOException;
+import java.util.Set;
+
+import javax.servlet.Filter;
+import javax.servlet.FilterChain;
+import javax.servlet.RequestDispatcher;
+import javax.servlet.ServletContext;
+import javax.servlet.ServletException;
+import javax.servlet.ServletRequest;
+import javax.servlet.ServletResponse;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletRequestWrapper;
+
+/**
+ * Skeleton implementation of a central routing/dispatch class which uses a sequence of
+ * {@link FilterDefinition}s to filter requests before delegating to the servlet pipeline.
+ *
+ * @author dhanji@gmail.com (Dhanji R. Prasanna)
+ */
+public abstract class AbstractFilterPipeline implements FilterPipeline {
+
+  private final AbstractServletPipeline servletPipeline;
+  private final Provider<ServletContext> servletContext;
+
+  //Unfortunately, we need the injector itself in order to create filters + servlets
+  private final Injector injector;
+
+  //Guards a DCL, so needs to be volatile
+  private volatile boolean initialized = false;
+
+  protected AbstractFilterPipeline(Injector injector, AbstractServletPipeline servletPipeline,
+      Provider<ServletContext> servletContext) {
+    this.injector = injector;
+    this.servletPipeline = servletPipeline;
+    this.servletContext = servletContext;
+  }
+
+  /**
+   * @return {@code true} if any filter mappings exist; otherwise {@code false}
+   */
+  protected abstract boolean hasFiltersMapped();
+
+  /**
+   * @return snapshot of the filter mappings currently defined for this pipeline
+   */
+  protected abstract FilterDefinition[] filterDefinitions();
+
+  public synchronized void initPipeline(ServletContext servletContext)
+      throws ServletException {
+
+    //double-checked lock, prevents duplicate initialization
+    if (initialized)
+      return;
+
+    // Used to prevent duplicate initialization.
+    Set<Filter> initializedSoFar = Sets.newIdentityHashSet();
+
+    for (FilterDefinition filterDefinition : filterDefinitions()) {
+      filterDefinition.init(servletContext, injector, initializedSoFar);
+    }
+
+    //next, initialize servlets...
+    servletPipeline.init(servletContext, injector);
+
+    //everything was ok...
+    initialized = true;
+  }
+
+  public void dispatch(ServletRequest request, ServletResponse response,
+      FilterChain proceedingFilterChain) throws IOException, ServletException {
+
+    //lazy init of filter pipeline (OK by the servlet specification). This is needed
+    //in order for us not to force users to create a GuiceServletContextListener subclass.
+    if (!initialized) {
+      initPipeline(servletContext.get());
+    }
+
+    //obtain the servlet pipeline to dispatch against
+    new FilterChainInvocation(filterDefinitions(), servletPipeline, proceedingFilterChain)
+        .doFilter(withDispatcher(request, servletPipeline), response);
+
+  }
+
+  /**
+   * Used to create an proxy that dispatches either to the guice-servlet pipeline or the regular
+   * pipeline based on uri-path match. This proxy also provides minimal forwarding support.
+   *
+   * We cannot forward from a web.xml Servlet/JSP to a guice-servlet (because the filter pipeline
+   * is not called again). However, we can wrap requests with our own dispatcher to forward the
+   * *other* way. web.xml Servlets/JSPs can forward to themselves as per normal.
+   *
+   * This is not a problem cuz we intend for people to migrate from web.xml to guice-servlet,
+   * incrementally, but not the other way around (which, we should actively discourage).
+   */
+  @SuppressWarnings({ "JavaDoc", "deprecation" })
+  private ServletRequest withDispatcher(ServletRequest servletRequest,
+      final AbstractServletPipeline servletPipeline) {
+
+    // don't wrap the request if there are no servlets mapped. This prevents us from inserting our
+    // wrapper unless it's actually going to be used. This is necessary for compatibility for apps
+    // that downcast their HttpServletRequests to a concrete implementation.
+    if (!servletPipeline.hasServletsMapped()) {
+      return servletRequest;
+    }
+
+    HttpServletRequest request = (HttpServletRequest) servletRequest;
+    //noinspection OverlyComplexAnonymousInnerClass
+    return new HttpServletRequestWrapper(request) {
+
+      @Override
+      public RequestDispatcher getRequestDispatcher(String path) {
+        final RequestDispatcher dispatcher = servletPipeline.getRequestDispatcher(path);
+
+        return (null != dispatcher) ? dispatcher : super.getRequestDispatcher(path);
+      }
+    };
+  }
+
+  public void destroyPipeline() {
+    //destroy servlets first
+    servletPipeline.destroy();
+
+    //go down chain and destroy all our filters
+    Set<Filter> destroyedSoFar = Sets.newIdentityHashSet();
+    for (FilterDefinition filterDefinition : filterDefinitions()) {
+      filterDefinition.destroy(destroyedSoFar);
+    }
+  }
+}
diff --git a/extensions/servlet/src/com/google/inject/servlet/AbstractServletPipeline.java b/extensions/servlet/src/com/google/inject/servlet/AbstractServletPipeline.java
new file mode 100644
index 0000000..811820a
--- /dev/null
+++ b/extensions/servlet/src/com/google/inject/servlet/AbstractServletPipeline.java
@@ -0,0 +1,187 @@
+/**
+ * Copyright (C) 2008 Google Inc.
+ *
+ * Licensed 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 com.google.inject.servlet;
+
+import com.google.common.base.Preconditions;
+import com.google.common.collect.Sets;
+import com.google.inject.Injector;
+
+import java.io.IOException;
+import java.util.Set;
+
+import javax.servlet.RequestDispatcher;
+import javax.servlet.ServletContext;
+import javax.servlet.ServletException;
+import javax.servlet.ServletRequest;
+import javax.servlet.ServletResponse;
+import javax.servlet.http.HttpServlet;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletRequestWrapper;
+
+/**
+ * Skeleton implementation of a wrapping dispatcher for servlets based on a sequence of
+ * {@link ServletDefinition}s, much like {@link AbstractFilterPipeline} is for filters.
+ *
+ * @author dhanji@gmail.com (Dhanji R. Prasanna)
+ */
+public abstract class AbstractServletPipeline {
+
+  /**
+   * @return {@code true} if any servlet mappings exist; otherwise {@code false}
+   */
+  protected abstract boolean hasServletsMapped();
+
+  /**
+   * @return snapshot of the servlet mappings currently defined for this pipeline
+   */
+  protected abstract ServletDefinition[] servletDefinitions();
+
+  void init(ServletContext servletContext, Injector injector) throws ServletException {
+    Set<HttpServlet> initializedSoFar = Sets.newIdentityHashSet();
+
+    for (ServletDefinition servletDefinition : servletDefinitions()) {
+      servletDefinition.init(servletContext, injector, initializedSoFar);
+    }
+  }
+
+  boolean service(ServletRequest request, ServletResponse response)
+      throws IOException, ServletException {
+
+    //stop at the first matching servlet and service
+    for (ServletDefinition servletDefinition : servletDefinitions()) {
+      if (servletDefinition.service(request, response)) {
+        return true;
+      }
+    }
+
+    //there was no match...
+    return false;
+  }
+
+  void destroy() {
+    Set<HttpServlet> destroyedSoFar = Sets.newIdentityHashSet();
+    for (ServletDefinition servletDefinition : servletDefinitions()) {
+      servletDefinition.destroy(destroyedSoFar);
+    }
+  }
+
+  /**
+   * @return Returns a request dispatcher wrapped with a servlet mapped to
+   * the given path or null if no mapping was found.
+   */
+  RequestDispatcher getRequestDispatcher(String path) {
+    final String newRequestUri = path;
+
+    // TODO(dhanji): check servlet spec to see if the following is legal or not.
+    // Need to strip query string if requested...
+
+    for (final ServletDefinition servletDefinition : servletDefinitions()) {
+      if (servletDefinition.shouldServe(path)) {
+        return new RequestDispatcher() {
+          public void forward(ServletRequest servletRequest, ServletResponse servletResponse)
+              throws ServletException, IOException {
+            Preconditions.checkState(!servletResponse.isCommitted(),
+                "Response has been committed--you can only call forward before"
+                + " committing the response (hint: don't flush buffers)");
+
+            // clear buffer before forwarding
+            servletResponse.resetBuffer();
+
+            ServletRequest requestToProcess;
+            if (servletRequest instanceof HttpServletRequest) {
+               requestToProcess = wrapRequest((HttpServletRequest)servletRequest, newRequestUri);
+            } else {
+              // This should never happen, but instead of throwing an exception
+              // we will allow a happy case pass thru for maximum tolerance to
+              // legacy (and internal) code.
+              requestToProcess = servletRequest;
+            }
+
+            // now dispatch to the servlet
+            doServiceImpl(servletDefinition, requestToProcess, servletResponse);
+          }
+
+          public void include(ServletRequest servletRequest, ServletResponse servletResponse)
+              throws ServletException, IOException {
+            // route to the target servlet
+            doServiceImpl(servletDefinition, servletRequest, servletResponse);
+          }
+
+          private void doServiceImpl(ServletDefinition servletDefinition, ServletRequest servletRequest,
+              ServletResponse servletResponse) throws ServletException, IOException {
+            servletRequest.setAttribute(REQUEST_DISPATCHER_REQUEST, Boolean.TRUE);
+
+            try {
+              servletDefinition.doService(servletRequest, servletResponse);
+            } finally {
+              servletRequest.removeAttribute(REQUEST_DISPATCHER_REQUEST);
+            }
+          }
+        };
+      }
+    }
+
+    //otherwise, can't process
+    return null;
+  }
+
+  // visible for testing
+  static HttpServletRequest wrapRequest(HttpServletRequest request, String newUri) {
+    return new RequestDispatcherRequestWrapper(request, newUri);
+  }
+
+  /**
+   * A Marker constant attribute that when present in the request indicates to Guice servlet that
+   * this request has been generated by a request dispatcher rather than the servlet pipeline.
+   * In accordance with section 8.4.2 of the Servlet 2.4 specification.
+   */
+  static final String REQUEST_DISPATCHER_REQUEST = "javax.servlet.forward.servlet_path";
+
+  private static class RequestDispatcherRequestWrapper extends HttpServletRequestWrapper {
+    private final String newRequestUri;
+
+    RequestDispatcherRequestWrapper(HttpServletRequest servletRequest, String newRequestUri) {
+      super(servletRequest);
+      this.newRequestUri = newRequestUri;
+    }
+
+    @Override
+    public String getRequestURI() {
+      return newRequestUri;
+    }
+
+    @Override
+    public StringBuffer getRequestURL() {
+      StringBuffer url = new StringBuffer();
+      String scheme = getScheme();
+      int port = getServerPort();
+
+      url.append(scheme);
+      url.append("://");
+      url.append(getServerName());
+      // port might be -1 in some cases (see java.net.URL.getPort)
+      if (port > 0 &&
+          (("http".equals(scheme) && (port != 80)) ||
+           ("https".equals(scheme) && (port != 443)))) {
+        url.append(':');
+        url.append(port);
+      }
+      url.append(getRequestURI());
+
+      return (url);
+    }
+  }
+}
diff --git a/extensions/servlet/src/com/google/inject/servlet/FilterChainInvocation.java b/extensions/servlet/src/com/google/inject/servlet/FilterChainInvocation.java
index b4112cf..bfe5a83 100644
--- a/extensions/servlet/src/com/google/inject/servlet/FilterChainInvocation.java
+++ b/extensions/servlet/src/com/google/inject/servlet/FilterChainInvocation.java
@@ -50,7 +50,7 @@ class FilterChainInvocation implements FilterChain {
   
   private final FilterDefinition[] filterDefinitions;
   private final FilterChain proceedingChain;
-  private final ManagedServletPipeline servletPipeline;
+  private final AbstractServletPipeline servletPipeline;
 
   //state variable tracks current link in filterchain
   private int index = -1;
@@ -58,7 +58,7 @@ class FilterChainInvocation implements FilterChain {
   private boolean cleanedStacks = false;
 
   public FilterChainInvocation(FilterDefinition[] filterDefinitions,
-      ManagedServletPipeline servletPipeline, FilterChain proceedingChain) {
+      AbstractServletPipeline servletPipeline, FilterChain proceedingChain) {
 
     this.filterDefinitions = filterDefinitions;
     this.servletPipeline = servletPipeline;
diff --git a/extensions/servlet/src/com/google/inject/servlet/FilterDefinition.java b/extensions/servlet/src/com/google/inject/servlet/FilterDefinition.java
index ff1e5b6..76ece31 100644
--- a/extensions/servlet/src/com/google/inject/servlet/FilterDefinition.java
+++ b/extensions/servlet/src/com/google/inject/servlet/FilterDefinition.java
@@ -37,11 +37,11 @@ import javax.servlet.ServletException;
 import javax.servlet.http.HttpServletRequest;
 
 /**
- * An internal representation of a filter definition against a particular URI pattern.
+ * Defines a filter mapped to a URI pattern and performs the request filtering for that filter.
  *
  * @author dhanji@gmail.com (Dhanji R. Prasanna)
  */
-class FilterDefinition implements ProviderWithExtensionVisitor<FilterDefinition> {
+public class FilterDefinition implements ProviderWithExtensionVisitor<FilterDefinition> {
   private final String pattern;
   private final Key<? extends Filter> filterKey;
   private final UriPatternMatcher patternMatcher;
@@ -90,7 +90,7 @@ class FilterDefinition implements ProviderWithExtensionVisitor<FilterDefinition>
     return uri != null && patternMatcher.matches(uri);
   }
 
-  public void init(final ServletContext servletContext, Injector injector,
+  void init(final ServletContext servletContext, Injector injector,
       Set<Filter> initializedSoFar) throws ServletException {
 
     // This absolutely must be a singleton, and so is only initialized once.
@@ -130,7 +130,7 @@ class FilterDefinition implements ProviderWithExtensionVisitor<FilterDefinition>
     initializedSoFar.add(filter);
   }
 
-  public void destroy(Set<Filter> destroyedSoFar) {
+  void destroy(Set<Filter> destroyedSoFar) {
     // filters are always singletons
     Filter reference = filter.get();
 
@@ -150,7 +150,7 @@ class FilterDefinition implements ProviderWithExtensionVisitor<FilterDefinition>
     }
   }
 
-  public Filter getFilterIfMatching(HttpServletRequest request) {
+  Filter getFilterIfMatching(HttpServletRequest request) {
 
     final String path = ServletUtils.getContextRelativePath(request);
     if (shouldFilter(path)) {
diff --git a/extensions/servlet/src/com/google/inject/servlet/FilterPipeline.java b/extensions/servlet/src/com/google/inject/servlet/FilterPipeline.java
index 985064b..c745d20 100644
--- a/extensions/servlet/src/com/google/inject/servlet/FilterPipeline.java
+++ b/extensions/servlet/src/com/google/inject/servlet/FilterPipeline.java
@@ -26,7 +26,7 @@ import javax.servlet.ServletRequest;
 import javax.servlet.ServletResponse;
 
 /**
- * An internal dispatcher for guice-servlet registered servlets and filters.
+ * A dispatcher abstraction for guice-servlet registered servlets and filters.
  * By default, we assume a Guice 1.0 style servlet module is in play. In other
  * words, we dispatch directly to the web.xml pipeline after setting up scopes.
  *
@@ -39,10 +39,27 @@ import javax.servlet.ServletResponse;
  * @author dhanji@gmail.com (Dhanji R. Prasanna)
  */
 @ImplementedBy(DefaultFilterPipeline.class)
-interface FilterPipeline {
+public interface FilterPipeline {
+
+  /**
+   * Initializes the pipeline, putting it into service.
+   * 
+   * @param context of the web application
+   */
   void initPipeline(ServletContext context) throws ServletException;
+
+  /**
+   * Destroys the pipeline, taking it out of service.
+   */
   void destroyPipeline();
 
+  /**
+   * Dispatches a request against the pipeline.
+   * 
+   * @param request to dispatch
+   * @param response to populate
+   * @param defaultFilterChain for last resort filtering
+   */
   void dispatch(ServletRequest request, ServletResponse response,
       FilterChain defaultFilterChain) throws IOException, ServletException;
 }
diff --git a/extensions/servlet/src/com/google/inject/servlet/GuiceFilter.java b/extensions/servlet/src/com/google/inject/servlet/GuiceFilter.java
index ba7a5af..0737b38 100644
--- a/extensions/servlet/src/com/google/inject/servlet/GuiceFilter.java
+++ b/extensions/servlet/src/com/google/inject/servlet/GuiceFilter.java
@@ -89,7 +89,7 @@ public class GuiceFilter implements Filter {
     this(null);
   }
 
-  @Inject GuiceFilter(FilterPipeline filterPipeline) {
+  @Inject protected GuiceFilter(FilterPipeline filterPipeline) {
     injectedPipeline = filterPipeline;
   }
 
diff --git a/extensions/servlet/src/com/google/inject/servlet/ManagedFilterPipeline.java b/extensions/servlet/src/com/google/inject/servlet/ManagedFilterPipeline.java
index 538e10a..76bc269 100644
--- a/extensions/servlet/src/com/google/inject/servlet/ManagedFilterPipeline.java
+++ b/extensions/servlet/src/com/google/inject/servlet/ManagedFilterPipeline.java
@@ -16,7 +16,6 @@
 package com.google.inject.servlet;
 
 import com.google.common.collect.Lists;
-import com.google.common.collect.Sets;
 import com.google.inject.Binding;
 import com.google.inject.Inject;
 import com.google.inject.Injector;
@@ -24,50 +23,41 @@ import com.google.inject.Provider;
 import com.google.inject.Singleton;
 import com.google.inject.TypeLiteral;
 
-import java.io.IOException;
 import java.util.List;
-import java.util.Set;
 
-import javax.servlet.Filter;
-import javax.servlet.FilterChain;
-import javax.servlet.RequestDispatcher;
 import javax.servlet.ServletContext;
-import javax.servlet.ServletException;
-import javax.servlet.ServletRequest;
-import javax.servlet.ServletResponse;
-import javax.servlet.http.HttpServletRequest;
-import javax.servlet.http.HttpServletRequestWrapper;
 
 /**
- * Central routing/dispatch class handles lifecycle of managed filters, and delegates to the servlet
- * pipeline.
+ * Managed implementation of a central routing/dispatch class which handles lifecycle of managed
+ * filters, and delegates to a managed servlet pipeline.
  *
  * @author dhanji@gmail.com (Dhanji R. Prasanna)
  */
 @Singleton
-class ManagedFilterPipeline implements FilterPipeline{
+class ManagedFilterPipeline extends AbstractFilterPipeline {
   private final FilterDefinition[] filterDefinitions;
-  private final ManagedServletPipeline servletPipeline;
-  private final Provider<ServletContext> servletContext;
 
-  //Unfortunately, we need the injector itself in order to create filters + servlets
-  private final Injector injector;
-
-  //Guards a DCL, so needs to be volatile
-  private volatile boolean initialized = false;
   private static final TypeLiteral<FilterDefinition> FILTER_DEFS =
       TypeLiteral.get(FilterDefinition.class);
 
   @Inject
   public ManagedFilterPipeline(Injector injector, ManagedServletPipeline servletPipeline,
       Provider<ServletContext> servletContext) {
-    this.injector = injector;
-    this.servletPipeline = servletPipeline;
-    this.servletContext = servletContext;
+    super(injector, servletPipeline, servletContext);
 
     this.filterDefinitions = collectFilterDefinitions(injector);
   }
 
+  @Override
+  protected boolean hasFiltersMapped() {
+    return filterDefinitions.length > 0;
+  }
+
+  @Override
+  protected FilterDefinition[] filterDefinitions() {
+    return filterDefinitions;
+  }
+
   /**
    * Introspects the injector and collects all instances of bound {@code List<FilterDefinition>}
    * into a master list.
@@ -84,86 +74,4 @@ class ManagedFilterPipeline implements FilterPipeline{
     // Copy to a fixed-size array for speed of iteration.
     return filterDefinitions.toArray(new FilterDefinition[filterDefinitions.size()]);
   }
-
-  public synchronized void initPipeline(ServletContext servletContext)
-      throws ServletException {
-
-    //double-checked lock, prevents duplicate initialization
-    if (initialized)
-      return;
-
-    // Used to prevent duplicate initialization.
-    Set<Filter> initializedSoFar = Sets.newIdentityHashSet();
-
-    for (FilterDefinition filterDefinition : filterDefinitions) {
-      filterDefinition.init(servletContext, injector, initializedSoFar);
-    }
-
-    //next, initialize servlets...
-    servletPipeline.init(servletContext, injector);
-
-    //everything was ok...
-    initialized = true;
-  }
-
-  public void dispatch(ServletRequest request, ServletResponse response,
-      FilterChain proceedingFilterChain) throws IOException, ServletException {
-
-    //lazy init of filter pipeline (OK by the servlet specification). This is needed
-    //in order for us not to force users to create a GuiceServletContextListener subclass.
-    if (!initialized) {
-      initPipeline(servletContext.get());
-    }
-
-    //obtain the servlet pipeline to dispatch against
-    new FilterChainInvocation(filterDefinitions, servletPipeline, proceedingFilterChain)
-        .doFilter(withDispatcher(request, servletPipeline), response);
-
-  }
-
-  /**
-   * Used to create an proxy that dispatches either to the guice-servlet pipeline or the regular
-   * pipeline based on uri-path match. This proxy also provides minimal forwarding support.
-   *
-   * We cannot forward from a web.xml Servlet/JSP to a guice-servlet (because the filter pipeline
-   * is not called again). However, we can wrap requests with our own dispatcher to forward the
-   * *other* way. web.xml Servlets/JSPs can forward to themselves as per normal.
-   *
-   * This is not a problem cuz we intend for people to migrate from web.xml to guice-servlet,
-   * incrementally, but not the other way around (which, we should actively discourage).
-   */
-  @SuppressWarnings({ "JavaDoc", "deprecation" })
-  private ServletRequest withDispatcher(ServletRequest servletRequest,
-      final ManagedServletPipeline servletPipeline) {
-
-    // don't wrap the request if there are no servlets mapped. This prevents us from inserting our
-    // wrapper unless it's actually going to be used. This is necessary for compatibility for apps
-    // that downcast their HttpServletRequests to a concrete implementation.
-    if (!servletPipeline.hasServletsMapped()) {
-      return servletRequest;
-    }
-
-    HttpServletRequest request = (HttpServletRequest) servletRequest;
-    //noinspection OverlyComplexAnonymousInnerClass
-    return new HttpServletRequestWrapper(request) {
-
-      @Override
-      public RequestDispatcher getRequestDispatcher(String path) {
-        final RequestDispatcher dispatcher = servletPipeline.getRequestDispatcher(path);
-
-        return (null != dispatcher) ? dispatcher : super.getRequestDispatcher(path);
-      }
-    };
-  }
-
-  public void destroyPipeline() {
-    //destroy servlets first
-    servletPipeline.destroy();
-
-    //go down chain and destroy all our filters
-    Set<Filter> destroyedSoFar = Sets.newIdentityHashSet();
-    for (FilterDefinition filterDefinition : filterDefinitions) {
-      filterDefinition.destroy(destroyedSoFar);
-    }
-  }
 }
diff --git a/extensions/servlet/src/com/google/inject/servlet/ManagedServletPipeline.java b/extensions/servlet/src/com/google/inject/servlet/ManagedServletPipeline.java
index 455551a..ab58a8e 100644
--- a/extensions/servlet/src/com/google/inject/servlet/ManagedServletPipeline.java
+++ b/extensions/servlet/src/com/google/inject/servlet/ManagedServletPipeline.java
@@ -15,27 +15,14 @@
  */
 package com.google.inject.servlet;
 
-import com.google.common.base.Preconditions;
 import com.google.common.collect.Lists;
-import com.google.common.collect.Sets;
 import com.google.inject.Binding;
 import com.google.inject.Inject;
 import com.google.inject.Injector;
 import com.google.inject.Singleton;
 import com.google.inject.TypeLiteral;
 
-import java.io.IOException;
 import java.util.List;
-import java.util.Set;
-
-import javax.servlet.RequestDispatcher;
-import javax.servlet.ServletContext;
-import javax.servlet.ServletException;
-import javax.servlet.ServletRequest;
-import javax.servlet.ServletResponse;
-import javax.servlet.http.HttpServlet;
-import javax.servlet.http.HttpServletRequest;
-import javax.servlet.http.HttpServletRequestWrapper;
 
 /**
  * A wrapping dispatcher for servlets, in much the same way as {@link ManagedFilterPipeline} is for
@@ -44,7 +31,7 @@ import javax.servlet.http.HttpServletRequestWrapper;
  * @author dhanji@gmail.com (Dhanji R. Prasanna)
  */
 @Singleton
-class ManagedServletPipeline {
+class ManagedServletPipeline extends AbstractServletPipeline {
   private final ServletDefinition[] servletDefinitions;
   private static final TypeLiteral<ServletDefinition> SERVLET_DEFS =
       TypeLiteral.get(ServletDefinition.class);
@@ -54,10 +41,16 @@ class ManagedServletPipeline {
     this.servletDefinitions = collectServletDefinitions(injector);
   }
 
-  boolean hasServletsMapped() {
+  @Override
+  protected boolean hasServletsMapped() {
     return servletDefinitions.length > 0;
   }
 
+  @Override
+  protected ServletDefinition[] servletDefinitions() {
+    return servletDefinitions;
+  }
+
   /**
    * Introspects the injector and collects all instances of bound {@code List<ServletDefinition>}
    * into a master list.
@@ -74,140 +67,4 @@ class ManagedServletPipeline {
     // Copy to a fixed size array for speed.
     return servletDefinitions.toArray(new ServletDefinition[servletDefinitions.size()]);
   }
-
-  public void init(ServletContext servletContext, Injector injector) throws ServletException {
-    Set<HttpServlet> initializedSoFar = Sets.newIdentityHashSet();
-
-    for (ServletDefinition servletDefinition : servletDefinitions) {
-      servletDefinition.init(servletContext, injector, initializedSoFar);
-    }
-  }
-
-  public boolean service(ServletRequest request, ServletResponse response)
-      throws IOException, ServletException {
-
-    //stop at the first matching servlet and service
-    for (ServletDefinition servletDefinition : servletDefinitions) {
-      if (servletDefinition.service(request, response)) {
-        return true;
-      }
-    }
-
-    //there was no match...
-    return false;
-  }
-
-  public void destroy() {
-    Set<HttpServlet> destroyedSoFar = Sets.newIdentityHashSet();
-    for (ServletDefinition servletDefinition : servletDefinitions) {
-      servletDefinition.destroy(destroyedSoFar);
-    }
-  }
-
-  /**
-   * @return Returns a request dispatcher wrapped with a servlet mapped to
-   * the given path or null if no mapping was found.
-   */
-  RequestDispatcher getRequestDispatcher(String path) {
-    final String newRequestUri = path;
-
-    // TODO(dhanji): check servlet spec to see if the following is legal or not.
-    // Need to strip query string if requested...
-
-    for (final ServletDefinition servletDefinition : servletDefinitions) {
-      if (servletDefinition.shouldServe(path)) {
-        return new RequestDispatcher() {
-          public void forward(ServletRequest servletRequest, ServletResponse servletResponse)
-              throws ServletException, IOException {
-            Preconditions.checkState(!servletResponse.isCommitted(),
-                "Response has been committed--you can only call forward before"
-                + " committing the response (hint: don't flush buffers)");
-
-            // clear buffer before forwarding
-            servletResponse.resetBuffer();
-
-            ServletRequest requestToProcess;
-            if (servletRequest instanceof HttpServletRequest) {
-               requestToProcess = wrapRequest((HttpServletRequest)servletRequest, newRequestUri);
-            } else {
-              // This should never happen, but instead of throwing an exception
-              // we will allow a happy case pass thru for maximum tolerance to
-              // legacy (and internal) code.
-              requestToProcess = servletRequest;
-            }
-
-            // now dispatch to the servlet
-            doServiceImpl(servletDefinition, requestToProcess, servletResponse);
-          }
-
-          public void include(ServletRequest servletRequest, ServletResponse servletResponse)
-              throws ServletException, IOException {
-            // route to the target servlet
-            doServiceImpl(servletDefinition, servletRequest, servletResponse);
-          }
-
-          private void doServiceImpl(ServletDefinition servletDefinition, ServletRequest servletRequest,
-              ServletResponse servletResponse) throws ServletException, IOException {
-            servletRequest.setAttribute(REQUEST_DISPATCHER_REQUEST, Boolean.TRUE);
-
-            try {
-              servletDefinition.doService(servletRequest, servletResponse);
-            } finally {
-              servletRequest.removeAttribute(REQUEST_DISPATCHER_REQUEST);
-            }
-          }
-        };
-      }
-    }
-
-    //otherwise, can't process
-    return null;
-  }
-
-  // visible for testing
-  static HttpServletRequest wrapRequest(HttpServletRequest request, String newUri) {
-    return new RequestDispatcherRequestWrapper(request, newUri);
-  }
-
-  /**
-   * A Marker constant attribute that when present in the request indicates to Guice servlet that
-   * this request has been generated by a request dispatcher rather than the servlet pipeline.
-   * In accordance with section 8.4.2 of the Servlet 2.4 specification.
-   */
-  public static final String REQUEST_DISPATCHER_REQUEST = "javax.servlet.forward.servlet_path";
-
-  private static class RequestDispatcherRequestWrapper extends HttpServletRequestWrapper {
-    private final String newRequestUri;
-
-    public RequestDispatcherRequestWrapper(HttpServletRequest servletRequest, String newRequestUri) {
-      super(servletRequest);
-      this.newRequestUri = newRequestUri;
-    }
-
-    @Override
-    public String getRequestURI() {
-      return newRequestUri;
-    }
-
-    @Override
-    public StringBuffer getRequestURL() {
-      StringBuffer url = new StringBuffer();
-      String scheme = getScheme();
-      int port = getServerPort();
-
-      url.append(scheme);
-      url.append("://");
-      url.append(getServerName());
-      // port might be -1 in some cases (see java.net.URL.getPort)
-      if (port > 0 &&
-          (("http".equals(scheme) && (port != 80)) ||
-           ("https".equals(scheme) && (port != 443)))) {
-        url.append(':');
-        url.append(port);
-      }
-      url.append(getRequestURI());
-
-      return (url);
-    }
-  }
 }
diff --git a/extensions/servlet/src/com/google/inject/servlet/ServletDefinition.java b/extensions/servlet/src/com/google/inject/servlet/ServletDefinition.java
index 11328ed..285ff31 100644
--- a/extensions/servlet/src/com/google/inject/servlet/ServletDefinition.java
+++ b/extensions/servlet/src/com/google/inject/servlet/ServletDefinition.java
@@ -46,12 +46,11 @@ import javax.servlet.http.HttpServletRequestWrapper;
 import javax.servlet.http.HttpServletResponse;
 
 /**
- * An internal representation of a servlet definition mapped to a particular URI pattern. Also
- * performs the request dispatch to that servlet. How nice and OO =)
+ * Defines a servlet mapped to a URI pattern and performs the request dispatch to that servlet.
  *
  * @author dhanji@gmail.com (Dhanji R. Prasanna)
  */
-class ServletDefinition implements ProviderWithExtensionVisitor<ServletDefinition> {
+public class ServletDefinition implements ProviderWithExtensionVisitor<ServletDefinition> {
   private final String pattern;
   private final Key<? extends HttpServlet> servletKey;
   private final UriPatternMatcher patternMatcher;
@@ -100,7 +99,7 @@ class ServletDefinition implements ProviderWithExtensionVisitor<ServletDefinitio
     return uri != null && patternMatcher.matches(uri);
   }
 
-  public void init(final ServletContext servletContext, Injector injector,
+  void init(final ServletContext servletContext, Injector injector,
       Set<HttpServlet> initializedSoFar) throws ServletException {
 
     // This absolutely must be a singleton, and so is only initialized once.
@@ -140,7 +139,7 @@ class ServletDefinition implements ProviderWithExtensionVisitor<ServletDefinitio
     initializedSoFar.add(httpServlet);
   }
 
-  public void destroy(Set<HttpServlet> destroyedSoFar) {
+  void destroy(Set<HttpServlet> destroyedSoFar) {
     HttpServlet reference = httpServlet.get();
 
     // Do nothing if this Servlet was invalid (usually due to not being scoped
@@ -169,7 +168,7 @@ class ServletDefinition implements ProviderWithExtensionVisitor<ServletDefinitio
    * @throws IOException If thrown by underlying servlet
    * @throws ServletException If thrown by underlying servlet
    */
-  public boolean service(ServletRequest servletRequest,
+  boolean service(ServletRequest servletRequest,
       ServletResponse servletResponse) throws IOException, ServletException {
 
     final HttpServletRequest request = (HttpServletRequest) servletRequest;
diff --git a/extensions/servlet/src/com/google/inject/servlet/UriPatternMatcher.java b/extensions/servlet/src/com/google/inject/servlet/UriPatternMatcher.java
index d8bac74..169dd89 100644
--- a/extensions/servlet/src/com/google/inject/servlet/UriPatternMatcher.java
+++ b/extensions/servlet/src/com/google/inject/servlet/UriPatternMatcher.java
@@ -22,7 +22,7 @@ package com.google.inject.servlet;
  *
  * @author dhanji@gmail.com (Dhanji R. Prasanna)
  */
-interface UriPatternMatcher {
+public interface UriPatternMatcher {
   /**
    * @param uri A "contextual" (i.e. relative) Request URI, *not* a complete one.
    * @return Returns true if the uri matches the pattern.
diff --git a/extensions/servlet/src/com/google/inject/servlet/UriPatternType.java b/extensions/servlet/src/com/google/inject/servlet/UriPatternType.java
index 80d6aea..c76199d 100644
--- a/extensions/servlet/src/com/google/inject/servlet/UriPatternType.java
+++ b/extensions/servlet/src/com/google/inject/servlet/UriPatternType.java
@@ -26,7 +26,14 @@ import java.util.regex.Pattern;
 public enum UriPatternType {
   SERVLET, REGEX;
 
-  static UriPatternMatcher get(UriPatternType type, String pattern) {
+  /**
+   * Returns the appropriate {@link UriPatternMatcher} for {@code pattern}.
+   * 
+   * @param type of pattern matching
+   * @param pattern for matching URI
+   * @return {@link UriPatternMatcher} that matches the given pattern
+   */
+  public static UriPatternMatcher get(UriPatternType type, String pattern) {
     switch (type) {
       case SERVLET:
         return new ServletStyleUriPatternMatcher(pattern);
commit c2a5f1b0d3dc946bf4c9c240bce600ccf65230fa
Author: Stuart McCulloch <mcculls@gmail.com>
Date:   Tue Jan 20 18:51:01 2015 +0000

    Enhance logging in filter/servlet definitions

diff --git a/extensions/servlet/src/com/google/inject/servlet/FilterDefinition.java b/extensions/servlet/src/com/google/inject/servlet/FilterDefinition.java
index 76ece31..6819665 100644
--- a/extensions/servlet/src/com/google/inject/servlet/FilterDefinition.java
+++ b/extensions/servlet/src/com/google/inject/servlet/FilterDefinition.java
@@ -15,6 +15,7 @@
  */
 package com.google.inject.servlet;
 
+import com.google.common.base.Strings;
 import com.google.common.collect.Iterators;
 import com.google.inject.Injector;
 import com.google.inject.Key;
@@ -29,6 +30,8 @@ import java.util.HashMap;
 import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.atomic.AtomicReference;
+import java.util.logging.Level;
+import java.util.logging.Logger;
 
 import javax.servlet.Filter;
 import javax.servlet.FilterConfig;
@@ -42,6 +45,8 @@ import javax.servlet.http.HttpServletRequest;
  * @author dhanji@gmail.com (Dhanji R. Prasanna)
  */
 public class FilterDefinition implements ProviderWithExtensionVisitor<FilterDefinition> {
+  private static final Logger logger = Logger.getLogger(FilterDefinition.class.getName());
+
   private final String pattern;
   private final Key<? extends Filter> filterKey;
   private final UriPatternMatcher patternMatcher;
@@ -154,7 +159,11 @@ public class FilterDefinition implements ProviderWithExtensionVisitor<FilterDefi
 
     final String path = ServletUtils.getContextRelativePath(request);
     if (shouldFilter(path)) {
-      return filter.get();
+      Filter reference = filter.get();
+      if (logger.isLoggable(Level.FINEST)) {
+        logger.finest("Filtering " + path + " with " + reference);
+      }
+      return reference;
     } else {
       return null;
     }
@@ -164,4 +173,10 @@ public class FilterDefinition implements ProviderWithExtensionVisitor<FilterDefi
   Filter getFilter() {
     return filter.get();
   }
+
+  public String toPaddedString(int padding) {
+    Filter reference = filter.get();
+    return Strings.padEnd(pattern, padding, ' ') + ' '
+        + (reference != null ? reference : filterKey);
+  }
 }
diff --git a/extensions/servlet/src/com/google/inject/servlet/GuiceFilter.java b/extensions/servlet/src/com/google/inject/servlet/GuiceFilter.java
index 0737b38..1a98bf4 100644
--- a/extensions/servlet/src/com/google/inject/servlet/GuiceFilter.java
+++ b/extensions/servlet/src/com/google/inject/servlet/GuiceFilter.java
@@ -99,9 +99,9 @@ public class GuiceFilter implements Filter {
 
     // This can happen if you create many injectors and they all have their own
     // servlet module. This is legal, caveat a small warning.
-    if (GuiceFilter.pipeline instanceof ManagedFilterPipeline) {
-      LOGGER.warning(MULTIPLE_INJECTORS_WARNING);
-    }
+    //if (GuiceFilter.pipeline instanceof ManagedFilterPipeline) {
+    //  LOGGER.warning(MULTIPLE_INJECTORS_WARNING);
+    //}
 
     // We overwrite the default pipeline
     GuiceFilter.pipeline = pipeline;
diff --git a/extensions/servlet/src/com/google/inject/servlet/ServletDefinition.java b/extensions/servlet/src/com/google/inject/servlet/ServletDefinition.java
index 285ff31..9e49ce5 100644
--- a/extensions/servlet/src/com/google/inject/servlet/ServletDefinition.java
+++ b/extensions/servlet/src/com/google/inject/servlet/ServletDefinition.java
@@ -17,6 +17,7 @@ package com.google.inject.servlet;
 
 import static com.google.inject.servlet.ManagedServletPipeline.REQUEST_DISPATCHER_REQUEST;
 
+import com.google.common.base.Strings;
 import com.google.common.collect.Iterators;
 import com.google.inject.Injector;
 import com.google.inject.Key;
@@ -34,6 +35,8 @@ import java.util.HashMap;
 import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.atomic.AtomicReference;
+import java.util.logging.Level;
+import java.util.logging.Logger;
 
 import javax.servlet.ServletConfig;
 import javax.servlet.ServletContext;
@@ -51,6 +54,8 @@ import javax.servlet.http.HttpServletResponse;
  * @author dhanji@gmail.com (Dhanji R. Prasanna)
  */
 public class ServletDefinition implements ProviderWithExtensionVisitor<ServletDefinition> {
+  private static final Logger logger = Logger.getLogger(ServletDefinition.class.getName());
+
   private final String pattern;
   private final Key<? extends HttpServlet> servletKey;
   private final UriPatternMatcher patternMatcher;
@@ -283,7 +288,14 @@ public class ServletDefinition implements ProviderWithExtensionVisitor<ServletDe
         = (previous != null) ? previous.getOriginalRequest() : request;
     GuiceFilter.localContext.set(new GuiceFilter.Context(originalRequest, request, response));
     try {
-      httpServlet.get().service(request, response);
+      HttpServlet reference = httpServlet.get();
+      if (logger.isLoggable(Level.FINEST)) {
+        String path = ServletUtils.getContextRelativePath(request);
+        logger.finest("Serving " + path + " with " + reference);
+      }
+      if (reference != null) {
+        reference.service(request, response);
+      }
     } finally {
       GuiceFilter.localContext.set(previous);
     }
@@ -296,4 +308,10 @@ public class ServletDefinition implements ProviderWithExtensionVisitor<ServletDe
   String getPattern() {
     return pattern;
   }
+
+  public String toPaddedString(int padding) {
+    HttpServlet reference = httpServlet.get();
+    return Strings.padEnd(pattern, padding, ' ') + ' '
+        + (reference != null ? reference : servletKey);
+  }
 }