Update the ThreadLocal based scopes to use RequestContext.

Bound the ThreadLocalRequestContext module, which makes the CurrentUser
available in the Global module. Removed any binding in the scopes that
provided the CurrentUser. Updated all of the scopes to propagate the
RequestContext. The PerThreadRequestScope.Propagator allows scoping
callables to enter a request context.

Change-Id: Idf682ed1d7485cf8c9cdd22cd89bfe1ad5296880

gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java

@@ -23,6 +23,7 @@ import com.google.gerrit.extensions.registration.DynamicSet;
 import com.google.gerrit.reviewdb.client.AuthType;
 import com.google.gerrit.rules.PrologModule;
 import com.google.gerrit.rules.RulesCache;
+import com.google.gerrit.server.AnonymousUser;
 import com.google.gerrit.server.FileTypeRegistry;
 import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.InternalUser;
@@ -64,8 +65,10 @@ import com.google.gerrit.server.project.ProjectState;
 import com.google.gerrit.server.project.SectionSortCache;
 import com.google.gerrit.server.tools.ToolsCatalog;
 import com.google.gerrit.server.util.IdGenerator;
+import com.google.gerrit.server.util.ThreadLocalRequestContext;
 import com.google.gerrit.server.workflow.FunctionState;
 import com.google.inject.Inject;
+import com.google.inject.servlet.RequestScoped;
 
 import org.apache.velocity.runtime.RuntimeInstance;
 import org.eclipse.jgit.lib.Config;
@@ -117,6 +120,7 @@ public class GerritGlobalModule extends FactoryModule {
     install(new AccessControlModule());
     install(new GitModule());
     install(new PrologModule());
+    install(ThreadLocalRequestContext.module());
 
     factory(AccountInfoCacheFactory.Factory.class);
     factory(CapabilityControl.Factory.class);
@@ -154,5 +158,7 @@ public class GerritGlobalModule extends FactoryModule {
     bind(GitReferenceUpdated.class);
     DynamicSet.setOf(binder(), GitReferenceUpdatedListener.class);
     DynamicSet.setOf(binder(), NewProjectCreatedListener.class);
+
+    bind(AnonymousUser.class);
   }
 }

gerrit-server/src/main/java/com/google/gerrit/server/config/GerritRequestModule.java

@@ -77,7 +77,6 @@ public class GerritRequestModule extends FactoryModule {
     bind(ListProjects.class);
     bind(ApprovalsUtil.class);
 
-    bind(AnonymousUser.class).in(RequestScoped.class);
     bind(PerRequestProjectControlCache.class).in(RequestScoped.class);
     bind(ChangeControl.Factory.class).in(SINGLETON);
     bind(GroupControl.Factory.class).in(SINGLETON);

gerrit-server/src/main/java/com/google/gerrit/server/git/ChangeMergeQueue.java

@@ -20,16 +20,17 @@ import static java.util.concurrent.TimeUnit.SECONDS;
 import com.google.gerrit.reviewdb.client.Branch;
 import com.google.gerrit.reviewdb.client.Project;
 import com.google.gerrit.server.CurrentUser;
-import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.RemotePeer;
 import com.google.gerrit.server.config.GerritRequestModule;
 import com.google.gerrit.server.ssh.SshInfo;
+import com.google.gerrit.server.util.RequestContext;
 import com.google.gerrit.server.util.RequestScopePropagator;
 import com.google.inject.AbstractModule;
 import com.google.inject.Inject;
 import com.google.inject.Injector;
 import com.google.inject.OutOfScopeException;
 import com.google.inject.Provider;
+import com.google.inject.Provides;
 import com.google.inject.Singleton;
 import com.google.inject.servlet.RequestScoped;
 
@@ -43,6 +44,7 @@ import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.concurrent.Callable;
 import java.util.concurrent.TimeUnit;
 
 @Singleton
@@ -57,6 +59,7 @@ public class ChangeMergeQueue implements MergeQueue {
 
   private final WorkQueue workQueue;
   private final Provider<MergeOp.Factory> bgFactory;
+  private final PerThreadRequestScope.Scoper threadScoper;
 
   @Inject
   ChangeMergeQueue(final WorkQueue wq, Injector parent) {
@@ -68,15 +71,9 @@ public class ChangeMergeQueue implements MergeQueue {
         bindScope(RequestScoped.class, PerThreadRequestScope.REQUEST);
         bind(RequestScopePropagator.class)
             .to(PerThreadRequestScope.Propagator.class);
+        bind(PerThreadRequestScope.Propagator.class);
         install(new GerritRequestModule());
 
-        bind(CurrentUser.class).to(IdentifiedUser.class);
-        bind(IdentifiedUser.class).toProvider(new Provider<IdentifiedUser>() {
-          @Override
-          public IdentifiedUser get() {
-            throw new OutOfScopeException("No user on merge thread");
-          }
-        });
         bind(SocketAddress.class).annotatedWith(RemotePeer.class).toProvider(
             new Provider<SocketAddress>() {
               @Override
@@ -91,8 +88,26 @@ public class ChangeMergeQueue implements MergeQueue {
           }
         });
       }
+
+      @Provides
+      public PerThreadRequestScope.Scoper provideScoper(
+          final PerThreadRequestScope.Propagator propagator) {
+        final RequestContext requestContext = new RequestContext() {
+          @Override
+          public CurrentUser getCurrentUser() {
+            throw new OutOfScopeException("No user on merge thread");
+          }
+        };
+        return new PerThreadRequestScope.Scoper() {
+          @Override
+          public <T> Callable<T> scope(Callable<T> callable) {
+            return propagator.scope(requestContext, callable);
+          }
+        };
+      }
     });
     bgFactory = child.getProvider(MergeOp.Factory.class);
+    threadScoper = child.getInstance(PerThreadRequestScope.Scoper.class);
   }
 
   @Override
@@ -186,19 +201,15 @@ public class ChangeMergeQueue implements MergeQueue {
     }
   }
 
-  private void mergeImpl(Branch.NameKey branch) {
+  private void mergeImpl(final Branch.NameKey branch) {
     try {
-      PerThreadRequestScope ctx = new PerThreadRequestScope();
-      PerThreadRequestScope old = PerThreadRequestScope.set(ctx);
-      try {
-        try {
+      threadScoper.scope(new Callable<Void>(){
+        @Override
+        public Void call() throws Exception {
           bgFactory.get().create(branch).merge();
-        } finally {
-          ctx.cleanup.run();
+          return null;
         }
-      } finally {
-        PerThreadRequestScope.set(old);
-      }
+      }).call();
     } catch (Throwable e) {
       log.error("Merge attempt for " + branch + " failed", e);
     } finally {

gerrit-server/src/main/java/com/google/gerrit/server/git/PerThreadRequestScope.java

@@ -14,50 +14,86 @@
 
 package com.google.gerrit.server.git;
 
-import com.google.gerrit.server.RequestCleanup;
+import com.google.common.collect.Maps;
+import com.google.gerrit.server.util.RequestContext;
+import com.google.gerrit.server.util.ThreadLocalRequestContext;
 import com.google.gerrit.server.util.ThreadLocalRequestScopePropagator;
+import com.google.inject.Inject;
 import com.google.inject.Key;
 import com.google.inject.OutOfScopeException;
 import com.google.inject.Provider;
 import com.google.inject.Scope;
 
-import java.util.HashMap;
 import java.util.Map;
+import java.util.concurrent.Callable;
 
 public class PerThreadRequestScope {
-  static class Propagator
-      extends ThreadLocalRequestScopePropagator<PerThreadRequestScope> {
-    Propagator() {
-      super(REQUEST, current);
+  public interface Scoper {
+    <T> Callable<T> scope(Callable<T> callable);
+  }
+
+  private static class Context {
+    private final Map<Key<?>, Object> map;
+
+    private Context() {
+      map = Maps.newHashMap();
     }
 
-    @Override
-    protected PerThreadRequestScope continuingContext(
-        PerThreadRequestScope ctx) {
-      return new PerThreadRequestScope();
+    private <T> T get(Key<T> key, Provider<T> creator) {
+      @SuppressWarnings("unchecked")
+      T t = (T) map.get(key);
+      if (t == null) {
+        t = creator.get();
+        map.put(key, t);
+      }
+      return t;
     }
   }
 
-  private static final ThreadLocal<PerThreadRequestScope> current =
-      new ThreadLocal<PerThreadRequestScope>();
+  public static class Propagator extends ThreadLocalRequestScopePropagator<Context> {
+    @Inject
+    Propagator(ThreadLocalRequestContext local) {
+      super(REQUEST, current, local);
+    }
 
-  private static PerThreadRequestScope requireContext() {
-    final PerThreadRequestScope ctx = current.get();
+    @Override
+    protected Context continuingContext(Context ctx) {
+      return new Context();
+    }
+
+    public <T> Callable<T> scope(RequestContext requestContext, Callable<T> callable) {
+      final Context ctx = new Context();
+      final Callable<T> wrapped = context(requestContext, cleanup(callable));
+      return new Callable<T>() {
+        @Override
+        public T call() throws Exception {
+          Context old = current.get();
+          current.set(ctx);
+          try {
+            return wrapped.call();
+          } finally {
+            current.set(old);
+          }
+        }
+      };
+    }
+  }
+
+  private static final ThreadLocal<Context> current = new ThreadLocal<Context>();
+
+  private static Context requireContext() {
+    final Context ctx = current.get();
     if (ctx == null) {
       throw new OutOfScopeException("Not in command/request");
     }
     return ctx;
   }
 
-  public static PerThreadRequestScope set(PerThreadRequestScope ctx) {
-    PerThreadRequestScope old = current.get();
-    current.set(ctx);
-    return old;
-  }
-
   public static final Scope REQUEST = new Scope() {
+    @Override
     public <T> Provider<T> scope(final Key<T> key, final Provider<T> creator) {
       return new Provider<T>() {
+        @Override
         public T get() {
           return requireContext().get(key, creator);
         }
@@ -74,26 +110,4 @@ public class PerThreadRequestScope {
       return "PerThreadRequestScope.REQUEST";
     }
   };
-
-  private static final Key<RequestCleanup> RC_KEY =
-      Key.get(RequestCleanup.class);
-
-  final RequestCleanup cleanup;
-  private final Map<Key<?>, Object> map;
-
-  public PerThreadRequestScope() {
-    cleanup = new RequestCleanup();
-    map = new HashMap<Key<?>, Object>();
-    map.put(RC_KEY, cleanup);
-  }
-
-  synchronized <T> T get(Key<T> key, Provider<T> creator) {
-    @SuppressWarnings("unchecked")
-    T t = (T) map.get(key);
-    if (t == null) {
-      t = creator.get();
-      map.put(key, t);
-    }
-    return t;
-  }
 }

gerrit-server/src/main/java/com/google/gerrit/server/util/GuiceRequestScopePropagator.java

@@ -38,17 +38,15 @@ public class GuiceRequestScopePropagator extends RequestScopePropagator {
 
   private final String url;
   private final SocketAddress peer;
-  private final CurrentUser user;
 
   @Inject
   GuiceRequestScopePropagator(
       @CanonicalWebUrl @Nullable Provider<String> urlProvider,
       @RemotePeer Provider<SocketAddress> remotePeerProvider,
-      Provider<CurrentUser> currentUserProvider) {
-    super(ServletScopes.REQUEST);
+      ThreadLocalRequestContext local) {
+    super(ServletScopes.REQUEST, local);
     this.url = urlProvider != null ? urlProvider.get() : null;
     this.peer = remotePeerProvider.get();
-    this.user = currentUserProvider.get();
   }
 
   /**
@@ -69,9 +67,6 @@ public class GuiceRequestScopePropagator extends RequestScopePropagator {
         Providers.of(peer));
     seedMap.put(Key.get(SocketAddress.class, RemotePeer.class), peer);
 
-    seedMap.put(Key.get(typeOfProvider(CurrentUser.class)), Providers.of(user));
-    seedMap.put(Key.get(CurrentUser.class), user);
-
     return ServletScopes.continueRequest(callable, seedMap);
   }
 

gerrit-server/src/main/java/com/google/gerrit/server/util/RequestScopePropagator.java

@@ -42,9 +42,12 @@ import java.util.concurrent.Executors;
 public abstract class RequestScopePropagator {
 
   private final Scope scope;
+  private final ThreadLocalRequestContext local;
 
-  protected RequestScopePropagator(Scope scope) {
+  protected RequestScopePropagator(Scope scope,
+      ThreadLocalRequestContext local) {
     this.scope = scope;
+    this.local = local;
   }
 
   /**
@@ -70,26 +73,8 @@ public abstract class RequestScopePropagator {
    * @return a new Callable which will execute in the current request scope.
    */
   public final <T> Callable<T> wrap(final Callable<T> callable) {
-    final Callable<T> wrapped = wrapImpl(new Callable<T>() {
-      @Override
-      public T call() throws Exception {
-        RequestCleanup cleanup = scope.scope(
-            Key.get(RequestCleanup.class),
-            new Provider<RequestCleanup>() {
-              @Override
-              public RequestCleanup get() {
-                return new RequestCleanup();
-              }
-            }).get();
-
-        try {
-          return callable.call();
-        } finally {
-          cleanup.run();
-        }
-      }
-    });
-
+    final Callable<T> wrapped =
+        wrapImpl(context(local.getContext(), cleanup(callable)));
     return new Callable<T>() {
       @Override
       public T call() throws Exception {
@@ -178,4 +163,41 @@ public abstract class RequestScopePropagator {
    * @see #wrap(Callable)
    */
   protected abstract <T> Callable<T> wrapImpl(final Callable<T> callable);
+
+  protected <T> Callable<T> context(final RequestContext context,
+      final Callable<T> callable) {
+    return new Callable<T>() {
+      @Override
+      public T call() throws Exception {
+        RequestContext old = local.setContext(context);
+        try {
+          return callable.call();
+        } finally {
+          local.setContext(old);
+        }
+      }
+    };
+  }
+
+  protected <T> Callable<T> cleanup(final Callable<T> callable) {
+    return new Callable<T>() {
+      @Override
+      public T call() throws Exception {
+        RequestCleanup cleanup = scope.scope(
+            Key.get(RequestCleanup.class),
+            new Provider<RequestCleanup>() {
+              @Override
+              public RequestCleanup get() {
+                return new RequestCleanup();
+              }
+            }).get();
+
+        try {
+          return callable.call();
+        } finally {
+          cleanup.run();
+        }
+      }
+    };
+  }
 }

gerrit-server/src/main/java/com/google/gerrit/server/util/ThreadLocalRequestScopePropagator.java

@@ -31,8 +31,8 @@ public abstract class ThreadLocalRequestScopePropagator<C>
   private final ThreadLocal<C> threadLocal;
 
   protected ThreadLocalRequestScopePropagator(Scope scope,
-      ThreadLocal<C> threadLocal) {
-    super(scope);
+      ThreadLocal<C> threadLocal, ThreadLocalRequestContext local) {
+    super(scope, local);
     this.threadLocal = threadLocal;
   }