Separate request context filter from cleanup

Preparation work to allow the allocation of the HTTP
request context for use in the ProjectQoSFilter.

Having separate filters for setting the current context
and cleaning it up, allows other filters, like ProjectQoSFilter,
to access the current user without having necessarily to
have its context cleaned up.

The first attempt to have ProjectQoSFilter with a context
has failed because the cleanup was done on the upstream
filter, causing a double invocation and thus an exception.

By having the context set *before* the ProjectQoSFilter and
then cleaned up *after* the filter, allows to have the current
user injected and still doing the cleanup only once the filter
has correctly finished its processing.

Bug: Issue 12070
Change-Id: I7fbda3d787f54ef6af6d6ae75c28d4d73ba197fb

4 files changed, 72 insertions, 11 deletions

diff --git a/java/com/google/gerrit/httpd/RequestCleanupFilter.java b/java/com/google/gerrit/httpd/RequestCleanupFilter.java
new file mode 100644
index 0000000000..30c795e185
--- /dev/null
+++ b/java/com/google/gerrit/httpd/RequestCleanupFilter.java
@@ -0,0 +1,65 @@
+// Copyright (C) 2019 The Android Open Source Project
+//
+// 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.gerrit.httpd;
+
+import com.google.gerrit.server.RequestCleanup;
+import com.google.inject.Inject;
+import com.google.inject.Module;
+import com.google.inject.Provider;
+import com.google.inject.Singleton;
+import com.google.inject.servlet.ServletModule;
+import java.io.IOException;
+import javax.servlet.Filter;
+import javax.servlet.FilterChain;
+import javax.servlet.FilterConfig;
+import javax.servlet.ServletException;
+import javax.servlet.ServletRequest;
+import javax.servlet.ServletResponse;
+
+/** Executes any pending {@link RequestCleanup} at the end of a request. */
+@Singleton
+public class RequestCleanupFilter implements Filter {
+  public static Module module() {
+    return new ServletModule() {
+      @Override
+      protected void configureServlets() {
+        filter("/*").through(RequestCleanupFilter.class);
+      }
+    };
+  }
+
+  private final Provider<RequestCleanup> cleanup;
+
+  @Inject
+  RequestCleanupFilter(Provider<RequestCleanup> r) {
+    cleanup = r;
+  }
+
+  @Override
+  public void init(FilterConfig filterConfig) {}
+
+  @Override
+  public void destroy() {}
+
+  @Override
+  public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain)
+      throws IOException, ServletException {
+    try {
+      chain.doFilter(request, response);
+    } finally {
+      cleanup.get().run();
+    }
+  }
+}
diff --git a/java/com/google/gerrit/httpd/RequestContextFilter.java b/java/com/google/gerrit/httpd/RequestContextFilter.java
index 6e027969fa..effbac0ee0 100644
--- a/java/com/google/gerrit/httpd/RequestContextFilter.java
+++ b/java/com/google/gerrit/httpd/RequestContextFilter.java
@@ -14,7 +14,6 @@
 
 package com.google.gerrit.httpd;
 
-import com.google.gerrit.server.RequestCleanup;
 import com.google.gerrit.server.util.RequestContext;
 import com.google.gerrit.server.util.ThreadLocalRequestContext;
 import com.google.inject.Inject;
@@ -30,7 +29,7 @@ import javax.servlet.ServletException;
 import javax.servlet.ServletRequest;
 import javax.servlet.ServletResponse;
 
-/** Executes any pending {@link RequestCleanup} at the end of a request. */
+/** Set the request context for the downstream filters and invocation. */
 @Singleton
 public class RequestContextFilter implements Filter {
   public static Module module() {
@@ -42,14 +41,11 @@ public class RequestContextFilter implements Filter {
     };
   }
 
-  private final Provider<RequestCleanup> cleanup;
   private final Provider<HttpRequestContext> requestContext;
   private final ThreadLocalRequestContext local;
 
   @Inject
-  RequestContextFilter(
-      Provider<RequestCleanup> r, Provider<HttpRequestContext> c, ThreadLocalRequestContext l) {
-    cleanup = r;
+  RequestContextFilter(Provider<HttpRequestContext> c, ThreadLocalRequestContext l) {
     requestContext = c;
     local = l;
   }
@@ -65,11 +61,7 @@ public class RequestContextFilter implements Filter {
       throws IOException, ServletException {
     RequestContext old = local.setContext(requestContext.get());
     try {
-      try {
-        chain.doFilter(request, response);
-      } finally {
-        cleanup.get().run();
-      }
+      chain.doFilter(request, response);
     } finally {
       local.setContext(old);
     }
diff --git a/java/com/google/gerrit/httpd/init/WebAppInitializer.java b/java/com/google/gerrit/httpd/init/WebAppInitializer.java
index aadfeba4c5..c9cf2f4e85 100644
--- a/java/com/google/gerrit/httpd/init/WebAppInitializer.java
+++ b/java/com/google/gerrit/httpd/init/WebAppInitializer.java
@@ -28,6 +28,7 @@ import com.google.gerrit.httpd.GetUserFilter;
 import com.google.gerrit.httpd.GitOverHttpModule;
 import com.google.gerrit.httpd.H2CacheBasedWebSession;
 import com.google.gerrit.httpd.HttpCanonicalWebUrlProvider;
+import com.google.gerrit.httpd.RequestCleanupFilter;
 import com.google.gerrit.httpd.RequestContextFilter;
 import com.google.gerrit.httpd.RequestMetricsFilter;
 import com.google.gerrit.httpd.RequireSslFilter;
@@ -416,6 +417,7 @@ public class WebAppInitializer extends GuiceServletContextListener implements Fi
     modules.add(RequestMetricsFilter.module());
     modules.add(sysInjector.getInstance(GerritAuthModule.class));
     modules.add(sysInjector.getInstance(GitOverHttpModule.class));
+    modules.add(RequestCleanupFilter.module());
     modules.add(AllRequestFilter.module());
     modules.add(sysInjector.getInstance(WebModule.class));
     modules.add(sysInjector.getInstance(RequireSslFilter.Module.class));
diff --git a/java/com/google/gerrit/pgm/Daemon.java b/java/com/google/gerrit/pgm/Daemon.java
index b08842ed57..8be8456c8d 100644
--- a/java/com/google/gerrit/pgm/Daemon.java
+++ b/java/com/google/gerrit/pgm/Daemon.java
@@ -33,6 +33,7 @@ import com.google.gerrit.httpd.GetUserFilter;
 import com.google.gerrit.httpd.GitOverHttpModule;
 import com.google.gerrit.httpd.H2CacheBasedWebSession;
 import com.google.gerrit.httpd.HttpCanonicalWebUrlProvider;
+import com.google.gerrit.httpd.RequestCleanupFilter;
 import com.google.gerrit.httpd.RequestContextFilter;
 import com.google.gerrit.httpd.RequestMetricsFilter;
 import com.google.gerrit.httpd.RequireSslFilter;
@@ -587,6 +588,7 @@ public class Daemon extends SiteProgram {
     modules.add(H2CacheBasedWebSession.module());
     modules.add(sysInjector.getInstance(GerritAuthModule.class));
     modules.add(sysInjector.getInstance(GitOverHttpModule.class));
+    modules.add(RequestCleanupFilter.module());
     modules.add(AllRequestFilter.module());
     modules.add(sysInjector.getInstance(WebModule.class));
     modules.add(sysInjector.getInstance(RequireSslFilter.Module.class));