Remove Common.getAccountId and use Guice injection only

The server side code can use the Provider<CurrentUser> to obtain
the current user identity when it needs one.  The client side
only needs it in one place, Link, to compute the identity for
the "#mine" display.  This is easily obtained from the already
existing static account reference.  We might want to look at
using gwt-guice for the client side in the future, but that is
out of scope for now.

Signed-off-by: Shawn O. Pearce <sop@google.com>

src/main/java/com/google/gerrit/client/Gerrit.java

@@ -19,7 +19,6 @@ import com.google.gerrit.client.reviewdb.Account;
 import com.google.gerrit.client.reviewdb.AccountGeneralPreferences;
 import com.google.gerrit.client.rpc.Common;
 import com.google.gerrit.client.rpc.GerritCallback;
-import com.google.gerrit.client.rpc.Common.CurrentAccountImpl;
 import com.google.gerrit.client.ui.LinkMenuBar;
 import com.google.gerrit.client.ui.LinkMenuItem;
 import com.google.gerrit.client.ui.NeedsSignInKeyCommand;
@@ -203,13 +202,6 @@ public class Gerrit implements EntryPoint {
     };
     RootPanel.get("gerrit_body").add(body);
 
-    Common.setCurrentAccountImpl(new CurrentAccountImpl() {
-      public Account.Id getAccountId() {
-        final Account a = getUserAccount();
-        return a != null ? a.getId() : null;
-      }
-    });
-
     final RpcStatus rpcStatus = new RpcStatus(menuArea);
     JsonUtil.addRpcStartHandler(rpcStatus);
     JsonUtil.addRpcCompleteHandler(rpcStatus);

src/main/java/com/google/gerrit/client/Link.java

@@ -41,7 +41,6 @@ import com.google.gerrit.client.reviewdb.Change;
 import com.google.gerrit.client.reviewdb.Patch;
 import com.google.gerrit.client.reviewdb.PatchSet;
 import com.google.gerrit.client.reviewdb.Project;
-import com.google.gerrit.client.rpc.Common;
 import com.google.gerrit.client.ui.Screen;
 import com.google.gwt.core.client.GWT;
 import com.google.gwt.event.logical.shared.ValueChangeEvent;
@@ -152,7 +151,13 @@ public class Link implements ValueChangeHandler<String> {
     }
 
     if (MINE.equals(token)) {
-      return new AccountDashboardScreen(Common.getAccountId());
+      if (Gerrit.isSignedIn()) {
+        return new AccountDashboardScreen(Gerrit.getUserAccount().getId());
+      } else {
+        final Screen r = new AccountDashboardScreen(null);
+        r.setRequiresSignIn(true);
+        return r;
+      }
     }
     if (token.startsWith("mine,")) {
       if (MINE_STARRED.equals(token)) {

src/main/java/com/google/gerrit/client/changes/AccountDashboardScreen.java

@@ -19,7 +19,6 @@ import com.google.gerrit.client.Link;
 import com.google.gerrit.client.data.AccountDashboardInfo;
 import com.google.gerrit.client.data.AccountInfo;
 import com.google.gerrit.client.reviewdb.Account;
-import com.google.gerrit.client.rpc.Common;
 import com.google.gerrit.client.rpc.ScreenLoadCallback;
 import com.google.gerrit.client.ui.Screen;
 
@@ -32,8 +31,7 @@ public class AccountDashboardScreen extends Screen {
   private ChangeTable.Section closed;
 
   public AccountDashboardScreen(final Account.Id id) {
-    ownerId = id != null ? id : Common.getAccountId();
+    ownerId = id;
-    setRequiresSignIn(ownerId == null);
   }
 
   @Override

src/main/java/com/google/gerrit/client/rpc/Common.java

@@ -16,7 +16,6 @@ package com.google.gerrit.client.rpc;
 
 import com.google.gerrit.client.data.AccountCache;
 import com.google.gerrit.client.data.GerritConfig;
-import com.google.gerrit.client.reviewdb.Account;
 import com.google.gerrit.client.reviewdb.ReviewDb;
 import com.google.gwtorm.client.SchemaFactory;
 
@@ -24,7 +23,6 @@ public class Common {
   private static GerritConfig config;
   private static SchemaFactory<ReviewDb> schema;
   private static AccountCache accountCache;
-  private static CurrentAccountImpl caImpl;
 
   /** Get the public configuration data used by this Gerrit instance. */
   public static GerritConfig getGerritConfig() {
@@ -60,18 +58,4 @@ public class Common {
   public static void setSchemaFactory(final SchemaFactory<ReviewDb> imp) {
     schema = imp;
   }
-
-  /** Get the unique id for this account; null if there is no account. */
-  public static Account.Id getAccountId() {
-    return caImpl.getAccountId();
-  }
-
-  public static void setCurrentAccountImpl(final CurrentAccountImpl i) {
-    caImpl = i;
-  }
-
-  public interface CurrentAccountImpl {
-    /** Get the unique id for this account; null if there is no account. */
-    public Account.Id getAccountId();
-  }
 }

src/main/java/com/google/gerrit/server/BaseServiceImplementation.java

@@ -14,6 +14,7 @@
 
 package com.google.gerrit.server;
 
+import com.google.gerrit.client.reviewdb.Account;
 import com.google.gerrit.client.reviewdb.ReviewDb;
 import com.google.gerrit.client.rpc.CorruptEntityException;
 import com.google.gerrit.client.rpc.NoSuchEntityException;
@@ -27,9 +28,21 @@ import com.google.inject.Provider;
 /** Support for services which require a {@link ReviewDb} instance. */
 public class BaseServiceImplementation {
   private final Provider<ReviewDb> schema;
+  private final Provider<? extends CurrentUser> currentUser;
 
-  protected BaseServiceImplementation(final Provider<ReviewDb> sf) {
+  protected BaseServiceImplementation(final Provider<ReviewDb> schema,
-    schema = sf;
+      final Provider<? extends CurrentUser> currentUser) {
+    this.schema = schema;
+    this.currentUser = currentUser;
+  }
+
+  @Deprecated
+  protected Account.Id getAccountId() {
+    CurrentUser u = currentUser.get();
+    if (u instanceof IdentifiedUser) {
+      return ((IdentifiedUser) u).getAccountId();
+    }
+    return null;
   }
 
   /**

src/main/java/com/google/gerrit/server/http/GerritServletConfig.java

@@ -16,15 +16,9 @@ package com.google.gerrit.server.http;
 
 import static com.google.inject.Stage.PRODUCTION;
 
-import com.google.gerrit.client.reviewdb.Account;
-import com.google.gerrit.client.rpc.Common;
-import com.google.gerrit.client.rpc.Common.CurrentAccountImpl;
 import com.google.gerrit.git.PushAllProjectsOp;
 import com.google.gerrit.git.ReloadSubmitQueueOp;
 import com.google.gerrit.git.WorkQueue;
-import com.google.gerrit.server.AnonymousUser;
-import com.google.gerrit.server.CurrentUser;
-import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.config.AuthConfig;
 import com.google.gerrit.server.config.CanonicalWebUrlProvider;
 import com.google.gerrit.server.config.DatabaseModule;
@@ -38,7 +32,6 @@ import com.google.inject.CreationException;
 import com.google.inject.Guice;
 import com.google.inject.Injector;
 import com.google.inject.Module;
-import com.google.inject.OutOfScopeException;
 import com.google.inject.Provider;
 import com.google.inject.ProvisionException;
 import com.google.inject.servlet.GuiceServletContextListener;
@@ -152,35 +145,6 @@ public class GerritServletConfig extends GuiceServletContextListener {
     super.contextInitialized(event);
     init();
 
-    // Temporary hack to make Common.getAccountId() honor the Guice
-    // managed request state in either SSH or HTTP environments.
-    //
-    Common.setCurrentAccountImpl(new CurrentAccountImpl() {
-      private final Provider<IdentifiedUser> sshUser =
-          sshInjector.getProvider(IdentifiedUser.class);
-      private final Provider<CurrentUser> webUser =
-          webInjector.getProvider(CurrentUser.class);
-
-      public Account.Id getAccountId() {
-        try {
-          return sshUser.get().getAccountId();
-        } catch (ProvisionException notSsh) {
-          if (!(notSsh.getCause() instanceof OutOfScopeException)) {
-            throw notSsh;
-          }
-        }
-
-        CurrentUser u = webUser.get();
-        if (u instanceof IdentifiedUser) {
-          return ((IdentifiedUser) u).getAccountId();
-        } else if (u instanceof AnonymousUser) {
-          return null;
-        } else {
-          throw new OutOfScopeException("Cannot determine current user");
-        }
-      }
-    });
-
     try {
       sysInjector.getInstance(PushAllProjectsOp.Factory.class).create(null)
           .start(30, TimeUnit.SECONDS);

src/main/java/com/google/gerrit/server/rpc/ChangeListServiceImpl.java

@@ -90,10 +90,10 @@ public class ChangeListServiceImpl extends BaseServiceImplementation implements
   private final ChangeControl.Factory changeControlFactory;
 
   @Inject
-  ChangeListServiceImpl(final Provider<ReviewDb> sf,
+  ChangeListServiceImpl(final Provider<ReviewDb> schema,
       final Provider<CurrentUser> currentUser,
       final ChangeControl.Factory changeControlFactory) {
-    super(sf);
+    super(schema, currentUser);
     this.currentUser = currentUser;
     this.changeControlFactory = changeControlFactory;
   }
@@ -268,7 +268,7 @@ public class ChangeListServiceImpl extends BaseServiceImplementation implements
 
   public void forAccount(final Account.Id id,
       final AsyncCallback<AccountDashboardInfo> callback) {
-    final Account.Id me = Common.getAccountId();
+    final Account.Id me = getAccountId();
     final Account.Id target = id != null ? id : me;
     if (target == null) {
       callback.onFailure(new NoSuchEntityException());
@@ -325,7 +325,7 @@ public class ChangeListServiceImpl extends BaseServiceImplementation implements
       final AsyncCallback<SingleListChangeInfo> callback) {
     run(callback, new Action<SingleListChangeInfo>() {
       public SingleListChangeInfo run(final ReviewDb db) throws OrmException {
-        final Account.Id me = Common.getAccountId();
+        final Account.Id me = getAccountId();
         final AccountInfoCacheFactory ac = new AccountInfoCacheFactory(db);
         final SingleListChangeInfo d = new SingleListChangeInfo();
         final Set<Change.Id> starred = currentUser.get().getStarredChanges();
@@ -344,7 +344,7 @@ public class ChangeListServiceImpl extends BaseServiceImplementation implements
   public void myDraftChanges(final AsyncCallback<SingleListChangeInfo> callback) {
     run(callback, new Action<SingleListChangeInfo>() {
       public SingleListChangeInfo run(final ReviewDb db) throws OrmException {
-        final Account.Id me = Common.getAccountId();
+        final Account.Id me = getAccountId();
         final AccountInfoCacheFactory ac = new AccountInfoCacheFactory(db);
         final SingleListChangeInfo d = new SingleListChangeInfo();
         final Set<Change.Id> starred = currentUser.get().getStarredChanges();
@@ -365,7 +365,7 @@ public class ChangeListServiceImpl extends BaseServiceImplementation implements
       final AsyncCallback<VoidResult> callback) {
     run(callback, new Action<VoidResult>() {
       public VoidResult run(final ReviewDb db) throws OrmException {
-        final Account.Id me = Common.getAccountId();
+        final Account.Id me = getAccountId();
         final Set<Change.Id> existing = currentUser.get().getStarredChanges();
         final ArrayList<StarredChange> add = new ArrayList<StarredChange>();
         final ArrayList<StarredChange> remove = new ArrayList<StarredChange>();
@@ -503,7 +503,7 @@ public class ChangeListServiceImpl extends BaseServiceImplementation implements
     }
 
     public SingleListChangeInfo run(final ReviewDb db) throws OrmException {
-      final Account.Id me = Common.getAccountId();
+      final Account.Id me = getAccountId();
       final AccountInfoCacheFactory ac = new AccountInfoCacheFactory(db);
       final SingleListChangeInfo d = new SingleListChangeInfo();
       final Set<Change.Id> starred = currentUser.get().getStarredChanges();

src/main/java/com/google/gerrit/server/rpc/GroupAdminServiceImpl.java

@@ -54,12 +54,12 @@ class GroupAdminServiceImpl extends BaseServiceImplementation implements
   private final GroupControl.Factory groupControlFactory;
 
   @Inject
-  GroupAdminServiceImpl(final Provider<ReviewDb> sf,
+  GroupAdminServiceImpl(final Provider<ReviewDb> schema,
-      final Provider<IdentifiedUser> iu, final AccountCache2 accountCache,
+      final Provider<IdentifiedUser> currentUser,
-      final GroupCache groupCache,
+      final AccountCache2 accountCache, final GroupCache groupCache,
       final GroupControl.Factory groupControlFactory) {
-    super(sf);
+    super(schema, currentUser);
-    this.identifiedUser = iu;
+    this.identifiedUser = currentUser;
     this.accountCache = accountCache;
     this.groupCache = groupCache;
     this.groupControlFactory = groupControlFactory;
@@ -118,7 +118,7 @@ class GroupAdminServiceImpl extends BaseServiceImplementation implements
         group.setNameKey(nameKey);
         group.setDescription("");
 
-        final Account.Id me = Common.getAccountId();
+        final Account.Id me = getAccountId();
         final AccountGroupMember m =
             new AccountGroupMember(
                 new AccountGroupMember.Key(me, group.getId()));
@@ -229,8 +229,8 @@ class GroupAdminServiceImpl extends BaseServiceImplementation implements
           final Transaction txn = db.beginTransaction();
           db.accountGroupMembers().insert(Collections.singleton(m), txn);
           db.accountGroupMembersAudit().insert(
-              Collections.singleton(new AccountGroupMemberAudit(m, Common
+              Collections.singleton(new AccountGroupMemberAudit(m,
-                  .getAccountId())), txn);
+                  getAccountId())), txn);
           txn.commit();
           accountCache.evict(m.getAccountId());
         }
@@ -259,7 +259,7 @@ class GroupAdminServiceImpl extends BaseServiceImplementation implements
           }
         }
 
-        final Account.Id me = Common.getAccountId();
+        final Account.Id me = getAccountId();
         for (final AccountGroupMember.Key k : keys) {
           final AccountGroupMember m = db.accountGroupMembers().get(k);
           if (m != null) {

src/main/java/com/google/gerrit/server/rpc/SuggestServiceImpl.java

@@ -44,11 +44,11 @@ class SuggestServiceImpl extends BaseServiceImplementation implements
   private final ProjectCache projectCache;
 
   @Inject
-  SuggestServiceImpl(final Provider<ReviewDb> sf, final ProjectCache pc,
+  SuggestServiceImpl(final Provider<ReviewDb> schema, final ProjectCache pc,
-      final Provider<CurrentUser> cu) {
+      final Provider<CurrentUser> currentUser) {
-    super(sf);
+    super(schema, currentUser);
-    projectCache = pc;
+    this.projectCache = pc;
-    currentUser = cu;
+    this.currentUser = currentUser;
   }
 
   public void suggestProjectNameKey(final String query, final int limit,

src/main/java/com/google/gerrit/server/rpc/account/AccountSecurityImpl.java

@@ -28,6 +28,7 @@ import com.google.gerrit.client.rpc.InvalidSshKeyException;
 import com.google.gerrit.client.rpc.NoSuchEntityException;
 import com.google.gerrit.server.BaseServiceImplementation;
 import com.google.gerrit.server.ContactStore;
+import com.google.gerrit.server.CurrentUser;
 import com.google.gerrit.server.account.AccountByEmailCache;
 import com.google.gerrit.server.account.AccountCache2;
 import com.google.gerrit.server.config.AuthConfig;
@@ -74,12 +75,13 @@ class AccountSecurityImpl extends BaseServiceImplementation implements
   private final ExternalIdDetailFactory.Factory externalIdDetailFactory;
 
   @Inject
-  AccountSecurityImpl(final Provider<ReviewDb> sf, final ContactStore cs,
+  AccountSecurityImpl(final Provider<ReviewDb> schema,
+      final Provider<CurrentUser> currentUser, final ContactStore cs,
       final AuthConfig ac, final RegisterNewEmailSender.Factory esf,
       final SshKeyCache skc, final AccountByEmailCache abec,
       final AccountCache2 uac,
       final ExternalIdDetailFactory.Factory externalIdDetailFactory) {
-    super(sf);
+    super(schema, currentUser);
     contactStore = cs;
     authConfig = ac;
     registerNewEmailFactory = esf;
@@ -95,7 +97,7 @@ class AccountSecurityImpl extends BaseServiceImplementation implements
   public void mySshKeys(final AsyncCallback<List<AccountSshKey>> callback) {
     run(callback, new Action<List<AccountSshKey>>() {
       public List<AccountSshKey> run(ReviewDb db) throws OrmException {
-        return db.accountSshKeys().byAccount(Common.getAccountId()).toList();
+        return db.accountSshKeys().byAccount(getAccountId()).toList();
       }
     });
   }
@@ -105,7 +107,7 @@ class AccountSecurityImpl extends BaseServiceImplementation implements
     run(callback, new Action<AccountSshKey>() {
       public AccountSshKey run(final ReviewDb db) throws OrmException, Failure {
         int max = 0;
-        final Account.Id me = Common.getAccountId();
+        final Account.Id me = getAccountId();
         for (final AccountSshKey k : db.accountSshKeys().byAccount(me)) {
           max = Math.max(max, k.getKey().get());
         }
@@ -138,7 +140,7 @@ class AccountSecurityImpl extends BaseServiceImplementation implements
       final AsyncCallback<VoidResult> callback) {
     run(callback, new Action<VoidResult>() {
       public VoidResult run(final ReviewDb db) throws OrmException, Failure {
-        final Account.Id me = Common.getAccountId();
+        final Account.Id me = getAccountId();
         for (final AccountSshKey.Id keyId : ids) {
           if (!me.equals(keyId.getParentKey()))
             throw new Failure(new NoSuchEntityException());
@@ -181,7 +183,7 @@ class AccountSecurityImpl extends BaseServiceImplementation implements
           throws OrmException, Failure {
         // Don't permit deletes unless they are for our own account
         //
-        final Account.Id me = Common.getAccountId();
+        final Account.Id me = getAccountId();
         for (final AccountExternalId.Key keyId : keys) {
           if (!me.equals(keyId.getParentKey()))
             throw new Failure(new NoSuchEntityException());
@@ -236,7 +238,7 @@ class AccountSecurityImpl extends BaseServiceImplementation implements
       final ContactInformation info, final AsyncCallback<Account> callback) {
     run(callback, new Action<Account>() {
       public Account run(ReviewDb db) throws OrmException, Failure {
-        final Account me = db.accounts().get(Common.getAccountId());
+        final Account me = db.accounts().get(getAccountId());
         final String oldUser = me.getSshUserName();
         final String oldEmail = me.getPreferredEmail();
         me.setFullName(name != null && !name.isEmpty() ? name : null);
@@ -288,7 +290,7 @@ class AccountSecurityImpl extends BaseServiceImplementation implements
 
         final AccountAgreement a =
             new AccountAgreement(new AccountAgreement.Key(
-                Common.getAccountId(), id));
+                getAccountId(), id));
         if (cla.isAutoVerify()) {
           a.review(AccountAgreement.Status.VERIFIED, null);
         }
@@ -339,7 +341,7 @@ class AccountSecurityImpl extends BaseServiceImplementation implements
 
     run(callback, new Action<VoidResult>() {
       public VoidResult run(ReviewDb db) throws OrmException {
-        final Account.Id me = Common.getAccountId();
+        final Account.Id me = getAccountId();
         final List<AccountExternalId> exists =
             db.accountExternalIds().byAccountEmail(me, newEmail).toList();
         if (!exists.isEmpty()) {

src/main/java/com/google/gerrit/server/rpc/account/AccountServiceImpl.java

@@ -50,13 +50,13 @@ class AccountServiceImpl extends BaseServiceImplementation implements
   private final AgreementInfoFactory.Factory agreementInfoFactory;
 
   @Inject
-  AccountServiceImpl(final Provider<ReviewDb> sf,
+  AccountServiceImpl(final Provider<ReviewDb> schema,
-      final Provider<IdentifiedUser> currentUser,
+      final Provider<IdentifiedUser> identifiedUser,
       final AccountCache2 accountCache,
       final ProjectControl.Factory projectControlFactory,
       final AgreementInfoFactory.Factory agreementInfoFactory) {
-    super(sf);
+    super(schema, identifiedUser);
-    this.currentUser = currentUser;
+    this.currentUser = identifiedUser;
     this.accountCache = accountCache;
     this.projectControlFactory = projectControlFactory;
     this.agreementInfoFactory = agreementInfoFactory;
@@ -70,7 +70,7 @@ class AccountServiceImpl extends BaseServiceImplementation implements
       final AsyncCallback<VoidResult> callback) {
     run(callback, new Action<VoidResult>() {
       public VoidResult run(final ReviewDb db) throws OrmException, Failure {
-        final Account a = db.accounts().get(Common.getAccountId());
+        final Account a = db.accounts().get(getAccountId());
         if (a == null) {
           throw new Failure(new NoSuchEntityException());
         }
@@ -91,7 +91,7 @@ class AccountServiceImpl extends BaseServiceImplementation implements
             new ArrayList<AccountProjectWatchInfo>();
 
         for (final AccountProjectWatch w : db.accountProjectWatches()
-            .byAccount(Common.getAccountId()).toList()) {
+            .byAccount(getAccountId()).toList()) {
           final ProjectControl ctl;
           try {
             ctl = projectControlFactory.validateFor(w.getProjectNameKey());
@@ -132,7 +132,7 @@ class AccountServiceImpl extends BaseServiceImplementation implements
 
   public void updateProjectWatch(final AccountProjectWatch watch,
       final AsyncCallback<VoidResult> callback) {
-    if (!Common.getAccountId().equals(watch.getAccountId())) {
+    if (!getAccountId().equals(watch.getAccountId())) {
       callback.onFailure(new NoSuchEntityException());
       return;
     }
@@ -149,7 +149,7 @@ class AccountServiceImpl extends BaseServiceImplementation implements
       final AsyncCallback<VoidResult> callback) {
     run(callback, new Action<VoidResult>() {
       public VoidResult run(final ReviewDb db) throws OrmException, Failure {
-        final Account.Id me = Common.getAccountId();
+        final Account.Id me = getAccountId();
         for (final AccountProjectWatch.Key keyId : keys) {
           if (!me.equals(keyId.getParentKey()))
             throw new Failure(new NoSuchEntityException());

src/main/java/com/google/gerrit/server/rpc/changedetail/ChangeDetailFactory.java

@@ -30,7 +30,6 @@ import com.google.gerrit.client.reviewdb.PatchSetAncestor;
 import com.google.gerrit.client.reviewdb.Project;
 import com.google.gerrit.client.reviewdb.RevId;
 import com.google.gerrit.client.reviewdb.ReviewDb;
-import com.google.gerrit.client.rpc.Common;
 import com.google.gerrit.client.rpc.NoSuchEntityException;
 import com.google.gerrit.server.patch.PatchSetInfoNotAvailableException;
 import com.google.gerrit.server.project.ChangeControl;
@@ -67,6 +66,7 @@ class ChangeDetailFactory extends Handler<ChangeDetail> {
 
   private AccountInfoCacheFactory acc;
   private ChangeDetail detail;
+  private ChangeControl control;
 
   @Inject
   ChangeDetailFactory(final GerritConfig gerritConfig,
@@ -86,7 +86,7 @@ class ChangeDetailFactory extends Handler<ChangeDetail> {
   @Override
   public ChangeDetail call() throws OrmException, NoSuchEntityException,
       PatchSetInfoNotAvailableException, NoSuchChangeException {
-    final ChangeControl control = changeControlFactory.validateFor(changeId);
+    control = changeControlFactory.validateFor(changeId);
     final Change change = control.getChange();
     final Project proj = control.getProject();
     final PatchSet patch = db.patchSets().get(change.currentPatchSetId());
@@ -129,7 +129,6 @@ class ChangeDetailFactory extends Handler<ChangeDetail> {
         db.changeApprovals().byChange(changeId).toList();
 
     if (detail.getChange().getStatus().isOpen()) {
-      final Account.Id me = Common.getAccountId();
       final FunctionState fs =
           functionState.create(detail.getChange(), allApprovals);
 
@@ -146,7 +145,8 @@ class ChangeDetailFactory extends Handler<ChangeDetail> {
         }
       }
       for (final ApprovalType at : gerritConfig.getActionTypes()) {
-        if (CategoryFunction.forCategory(at.getCategory()).isValid(me, at, fs)) {
+        if (CategoryFunction.forCategory(at.getCategory()).isValid(
+            control.getCurrentUser(), at, fs)) {
           currentActions.add(at.getCategory().getId());
         }
       }
@@ -182,6 +182,7 @@ class ChangeDetailFactory extends Handler<ChangeDetail> {
     final PatchSet.Id psId = detail.getChange().currentPatchSetId();
     final PatchSetDetailFactory loader = patchSetDetail.create(psId);
     loader.patchSet = detail.getCurrentPatchSet();
+    loader.control = control;
     detail.setCurrentPatchSetDetail(loader.call());
 
     final HashSet<Change.Id> changesToGet = new HashSet<Change.Id>();

src/main/java/com/google/gerrit/server/rpc/changedetail/PatchSetDetailFactory.java

@@ -21,8 +21,8 @@ import com.google.gerrit.client.reviewdb.Patch;
 import com.google.gerrit.client.reviewdb.PatchLineComment;
 import com.google.gerrit.client.reviewdb.PatchSet;
 import com.google.gerrit.client.reviewdb.ReviewDb;
-import com.google.gerrit.client.rpc.Common;
 import com.google.gerrit.client.rpc.NoSuchEntityException;
+import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.patch.PatchSetInfoFactory;
 import com.google.gerrit.server.patch.PatchSetInfoNotAvailableException;
 import com.google.gerrit.server.project.ChangeControl;
@@ -50,6 +50,7 @@ class PatchSetDetailFactory extends Handler<PatchSetDetail> {
   private final PatchSet.Id psId;
 
   private PatchSetDetail detail;
+  ChangeControl control;
   PatchSet patchSet;
 
   @Inject
@@ -66,8 +67,8 @@ class PatchSetDetailFactory extends Handler<PatchSetDetail> {
   @Override
   public PatchSetDetail call() throws OrmException, NoSuchEntityException,
       PatchSetInfoNotAvailableException, NoSuchChangeException {
-    if (patchSet == null) {
+    if (control == null || patchSet == null) {
-      changeControlFactory.validateFor(psId.getParentKey());
+      control = changeControlFactory.validateFor(psId.getParentKey());
       patchSet = db.patchSets().get(psId);
       if (patchSet == null) {
         throw new NoSuchEntityException();
@@ -80,12 +81,13 @@ class PatchSetDetailFactory extends Handler<PatchSetDetail> {
     detail.setInfo(infoFactory.get(psId));
     detail.setPatches(db.patches().byPatchSet(psId).toList());
 
-    final Account.Id me = Common.getAccountId();
+    if (control.getCurrentUser() instanceof IdentifiedUser) {
-    if (me != null) {
       // If we are signed in, compute the number of draft comments by the
       // current user on each of these patch files. This way they can more
       // quickly locate where they have pending drafts, and review them.
       //
+      final Account.Id me =
+          ((IdentifiedUser) control.getCurrentUser()).getAccountId();
       final List<PatchLineComment> comments =
           db.patchComments().draft(psId, me).toList();
       if (!comments.isEmpty()) {

src/main/java/com/google/gerrit/server/rpc/changedetail/SubmitAction.java

@@ -27,6 +27,7 @@ import com.google.gerrit.client.rpc.Common;
 import com.google.gerrit.client.rpc.NoSuchEntityException;
 import com.google.gerrit.git.MergeQueue;
 import com.google.gerrit.server.ChangeUtil;
+import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.rpc.Handler;
 import com.google.gerrit.server.workflow.CategoryFunction;
 import com.google.gerrit.server.workflow.FunctionState;
@@ -49,15 +50,20 @@ class SubmitAction extends Handler<VoidResult> {
   private final MergeQueue merger;
   private final GerritConfig gerritConfig;
   private final FunctionState.Factory functionState;
+  private final IdentifiedUser user;
+
   private final PatchSet.Id patchSetId;
 
   @Inject
   SubmitAction(final ReviewDb db, final MergeQueue mq, final GerritConfig gc,
-      final FunctionState.Factory fs, @Assisted final PatchSet.Id patchSetId) {
+      final FunctionState.Factory fs, final IdentifiedUser user,
+      @Assisted final PatchSet.Id patchSetId) {
     this.db = db;
     this.merger = mq;
     this.gerritConfig = gc;
     this.functionState = fs;
+    this.user = user;
+
     this.patchSetId = patchSetId;
   }
 
@@ -81,9 +87,8 @@ class SubmitAction extends Handler<VoidResult> {
         new ArrayList<ChangeApproval>(db.changeApprovals().byChange(
             change.getId()).toList());
 
-    final Account.Id me = Common.getAccountId();
     final ChangeApproval.Key ak =
-        new ChangeApproval.Key(change.getId(), me, SUBMIT);
+        new ChangeApproval.Key(change.getId(), user.getAccountId(), SUBMIT);
     ChangeApproval myAction = null;
     boolean isnew = true;
     for (final ChangeApproval ca : allApprovals) {
@@ -111,7 +116,7 @@ class SubmitAction extends Handler<VoidResult> {
     for (ApprovalType c : gerritConfig.getApprovalTypes()) {
       CategoryFunction.forCategory(c.getCategory()).run(c, fs);
     }
-    if (!CategoryFunction.forCategory(actionType.getCategory()).isValid(me,
+    if (!CategoryFunction.forCategory(actionType.getCategory()).isValid(user,
         actionType, fs)) {
       throw new IllegalStateException(actionType.getCategory().getName()
           + " not permitted");

src/main/java/com/google/gerrit/server/rpc/patch/PatchDetailServiceImpl.java

@@ -38,6 +38,7 @@ import com.google.gerrit.client.rpc.NoSuchAccountException;
 import com.google.gerrit.client.rpc.NoSuchEntityException;
 import com.google.gerrit.server.BaseServiceImplementation;
 import com.google.gerrit.server.ChangeUtil;
+import com.google.gerrit.server.CurrentUser;
 import com.google.gerrit.server.mail.AddReviewerSender;
 import com.google.gerrit.server.mail.CommentSender;
 import com.google.gerrit.server.mail.EmailException;
@@ -76,14 +77,15 @@ class PatchDetailServiceImpl extends BaseServiceImplementation implements
   private final ApprovalCategory.Id addReviewerCategoryId;
 
   @Inject
-  PatchDetailServiceImpl(final Provider<ReviewDb> sf,
+  PatchDetailServiceImpl(final Provider<ReviewDb> schema,
+      final Provider<CurrentUser> currentUser,
       final AddReviewerSender.Factory arsf, final CommentSender.Factory csf,
       final PatchSetInfoFactory psif, final GerritConfig gc,
       final AbandonChange.Factory abandonChangeFactory,
       final CommentDetailFactory.Factory commentDetailFactory,
       final PatchScriptFactory.Factory patchScriptFactoryFactory,
       final SaveDraft.Factory saveDraftFactory) {
-    super(sf);
+    super(schema, currentUser);
     patchSetInfoFactory = psif;
     addReviewerSenderFactory = arsf;
     commentSenderFactory = csf;
@@ -131,7 +133,7 @@ class PatchDetailServiceImpl extends BaseServiceImplementation implements
         if (comment == null) {
           throw new Failure(new NoSuchEntityException());
         }
-        if (!Common.getAccountId().equals(comment.getAuthor())) {
+        if (!getAccountId().equals(comment.getAuthor())) {
           throw new Failure(new NoSuchEntityException());
         }
         if (comment.getStatus() != PatchLineComment.Status.DRAFT) {
@@ -160,7 +162,7 @@ class PatchDetailServiceImpl extends BaseServiceImplementation implements
         try {
           final CommentSender cm;
           cm = commentSenderFactory.create(r.change);
-          cm.setFrom(Common.getAccountId());
+          cm.setFrom(getAccountId());
           cm.setPatchSet(r.patchSet, patchSetInfoFactory.get(psid));
           cm.setChangeMessage(r.message);
           cm.setPatchLineComments(r.comments);
@@ -185,7 +187,7 @@ class PatchDetailServiceImpl extends BaseServiceImplementation implements
       final boolean reviewed, AsyncCallback<VoidResult> callback) {
     run(callback, new Action<VoidResult>() {
       public VoidResult run(ReviewDb db) throws OrmException {
-        Account.Id account = Common.getAccountId();
+        Account.Id account = getAccountId();
         AccountPatchReview.Key key =
             new AccountPatchReview.Key(patchKey, account);
         AccountPatchReview apr = db.accountPatchReviews().get(key);
@@ -212,7 +214,7 @@ class PatchDetailServiceImpl extends BaseServiceImplementation implements
       final String messageText, final Set<ApprovalCategoryValue.Id> approvals,
       final ReviewDb db, final Transaction txn) throws OrmException {
     final PublishResult r = new PublishResult();
-    final Account.Id me = Common.getAccountId();
+    final Account.Id me = getAccountId();
     r.change = db.changes().get(psid.getParentKey());
     r.patchSet = db.patchSets().get(psid);
     if (r.change == null || r.patchSet == null) {
@@ -344,7 +346,7 @@ class PatchDetailServiceImpl extends BaseServiceImplementation implements
         try {
           final AddReviewerSender cm;
           cm = addReviewerSenderFactory.create(change);
-          cm.setFrom(Common.getAccountId());
+          cm.setFrom(getAccountId());
           cm.setReviewDb(db);
           cm.addReviewers(reviewerIds);
           cm.send();

src/main/java/com/google/gerrit/server/rpc/patch/PatchScriptFactory.java

@@ -18,7 +18,6 @@ import com.google.gerrit.client.data.PatchScript;
 import com.google.gerrit.client.data.PatchScriptSettings;
 import com.google.gerrit.client.data.PatchScriptSettings.Whitespace;
 import com.google.gerrit.client.patches.CommentDetail;
-import com.google.gerrit.client.reviewdb.Account;
 import com.google.gerrit.client.reviewdb.AccountGeneralPreferences;
 import com.google.gerrit.client.reviewdb.Change;
 import com.google.gerrit.client.reviewdb.Patch;
@@ -26,10 +25,10 @@ import com.google.gerrit.client.reviewdb.PatchLineComment;
 import com.google.gerrit.client.reviewdb.PatchSet;
 import com.google.gerrit.client.reviewdb.Project;
 import com.google.gerrit.client.reviewdb.ReviewDb;
-import com.google.gerrit.client.rpc.Common;
 import com.google.gerrit.client.rpc.CorruptEntityException;
 import com.google.gerrit.server.FileTypeRegistry;
 import com.google.gerrit.server.GerritServer;
+import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.config.Nullable;
 import com.google.gerrit.server.patch.DiffCache;
 import com.google.gerrit.server.patch.DiffCacheContent;
@@ -87,6 +86,8 @@ class PatchScriptFactory extends Handler<PatchScript> {
   private Project.NameKey projectKey;
   private Repository git;
 
+  private ChangeControl control;
+
   @Inject
   PatchScriptFactory(final GerritServer gs, final FileTypeRegistry ftr,
       final DiffCache dc, final ReviewDb db,
@@ -115,7 +116,7 @@ class PatchScriptFactory extends Handler<PatchScript> {
     validatePatchSetId(psa);
     validatePatchSetId(psb);
 
-    final ChangeControl control = changeControlFactory.validateFor(changeId);
+    control = changeControlFactory.validateFor(changeId);
     change = control.getChange();
     patch = db.patches().get(patchKey);
 
@@ -262,9 +263,9 @@ class PatchScriptFactory extends Handler<PatchScript> {
       r.include(p);
     }
 
-    final Account.Id me = Common.getAccountId();
+    if (control.getCurrentUser() instanceof IdentifiedUser) {
-    if (me != null) {
+      for (PatchLineComment p : db.patchComments().draft(changeId, pn,
-      for (PatchLineComment p : db.patchComments().draft(changeId, pn, me)) {
+          ((IdentifiedUser) control.getCurrentUser()).getAccountId())) {
         r.include(p);
       }
     }

src/main/java/com/google/gerrit/server/rpc/project/ProjectAdminServiceImpl.java

@@ -80,18 +80,18 @@ class ProjectAdminServiceImpl extends BaseServiceImplementation implements
   private final ProjectDetailFactory.Factory projectDetailFactory;
 
   @Inject
-  ProjectAdminServiceImpl(final Provider<ReviewDb> sf, final GerritServer gs,
+  ProjectAdminServiceImpl(final Provider<ReviewDb> schema,
-      final ProjectCache pc, final ReplicationQueue rq,
+      final GerritServer gs, final ProjectCache pc, final ReplicationQueue rq,
-      final Provider<IdentifiedUser> iu,
+      final Provider<IdentifiedUser> currentUser,
       @WildProjectName final Project.NameKey wp,
       final ProjectControl.Factory projectControlFactory,
       final ProjectDetailFactory.Factory projectDetailFactory) {
-    super(sf);
+    super(schema, currentUser);
     this.server = gs;
     this.projectCache = pc;
     this.replication = rq;
     this.wildProject = wp;
-    this.identifiedUser = iu;
+    this.identifiedUser = currentUser;
     this.projectControlFactory = projectControlFactory;
     this.projectDetailFactory = projectDetailFactory;
   }
@@ -395,7 +395,7 @@ class ProjectAdminServiceImpl extends BaseServiceImplementation implements
           throw new Failure(new InvalidNameException());
         }
 
-        final Account me = Common.getAccountCache().get(Common.getAccountId());
+        final Account me = Common.getAccountCache().get(getAccountId());
         if (me == null) {
           throw new Failure(new NoSuchEntityException());
         }

src/main/java/com/google/gerrit/server/workflow/CategoryFunction.java

@@ -15,10 +15,10 @@
 package com.google.gerrit.server.workflow;
 
 import com.google.gerrit.client.data.ApprovalType;
-import com.google.gerrit.client.reviewdb.Account;
 import com.google.gerrit.client.reviewdb.ApprovalCategory;
 import com.google.gerrit.client.reviewdb.ChangeApproval;
 import com.google.gerrit.client.reviewdb.ProjectRight;
+import com.google.gerrit.server.CurrentUser;
 
 import java.util.HashMap;
 import java.util.Map;
@@ -35,7 +35,7 @@ public abstract class CategoryFunction {
 
   /**
    * Locate a function by category.
-   * 
+   *
    * @param category the category the function is for.
    * @return the function implementation; {@link NoOpFunction} if the function
    *         is not known to Gerrit and thus cannot be executed.
@@ -47,7 +47,7 @@ public abstract class CategoryFunction {
 
   /**
    * Locate a function by name.
-   * 
+   *
    * @param functionName the function's unique name.
    * @return the function implementation; null if the function is not known to
    *         Gerrit and thus cannot be executed.
@@ -60,7 +60,7 @@ public abstract class CategoryFunction {
    * Normalize ChangeApprovals and set the valid flag for this category.
    * <p>
    * Implementors should invoke:
-   * 
+   *
    * <pre>
    * state.valid(at, true);
    * </pre>
@@ -70,7 +70,7 @@ public abstract class CategoryFunction {
    * <p>
    * An example implementation which requires at least one positive and no
    * negatives might be:
-   * 
+   *
    * <pre>
    * boolean neg = false, pos = false;
    * for (final ChangeApproval ca : state.getApprovals(at)) {
@@ -80,17 +80,17 @@ public abstract class CategoryFunction {
    * }
    * state.valid(at, !neg &amp;&amp; pos);
    * </pre>
-   * 
+   *
    * @param at the cached category description to process.
    * @param state state to read approvals and project rights from, and to update
    *        the valid status into.
    */
   public abstract void run(ApprovalType at, FunctionState state);
 
-  public boolean isValid(final Account.Id accountId, final ApprovalType at,
+  public boolean isValid(final CurrentUser user, final ApprovalType at,
       final FunctionState state) {
     for (final ProjectRight pr : state.getAllRights(at)) {
-      if (state.isMember(accountId, pr.getAccountGroupId())
+      if (user.getEffectiveGroups().contains(pr.getAccountGroupId())
           && (pr.getMinValue() < 0 || pr.getMaxValue() > 0)) {
         return true;
       }

src/main/java/com/google/gerrit/server/workflow/FunctionState.java

@@ -48,8 +48,6 @@ public class FunctionState {
   private final AccountCache2 accountCache;
   private final ProjectCache projectCache;
 
-  private final Map<Account.Id, Set<AccountGroup.Id>> groupCache =
-      new HashMap<Account.Id, Set<AccountGroup.Id>>();
   private final Map<ApprovalCategory.Id, Collection<ChangeApproval>> approvals =
       new HashMap<ApprovalCategory.Id, Collection<ChangeApproval>>();
   private final Map<ApprovalCategory.Id, Boolean> valid =
@@ -182,24 +180,6 @@ public class FunctionState {
     return r;
   }
 
-  public boolean isMember(final ChangeApproval ca, final ProjectRight r) {
-    return isMember(ca.getAccountId(), r.getAccountGroupId());
-  }
-
-  public boolean isMember(final Account.Id accountId,
-      final AccountGroup.Id groupId) {
-    return getGroups(accountId).contains(groupId);
-  }
-
-  public Set<AccountGroup.Id> getGroups(final Account.Id id) {
-    Set<AccountGroup.Id> g = groupCache.get(id);
-    if (g == null) {
-      g = accountCache.get(id).getEffectiveGroups();
-      groupCache.put(id, g);
-    }
-    return g;
-  }
-
   /**
    * Normalize the approval record down to the range permitted by the type, in
    * case the type was modified since the approval was originally granted.
@@ -236,7 +216,9 @@ public class FunctionState {
     //
     short minAllowed = 0, maxAllowed = 0;
     for (final ProjectRight r : getAllRights(a.getCategoryId())) {
-      if (isMember(a, r)) {
+      final Account.Id who = a.getAccountId();
+      final AccountGroup.Id grp = r.getAccountGroupId();
+      if (accountCache.get(who).getEffectiveGroups().contains(grp)) {
         minAllowed = (short) Math.min(minAllowed, r.getMinValue());
         maxAllowed = (short) Math.max(maxAllowed, r.getMaxValue());
       }

src/main/java/com/google/gerrit/server/workflow/NoOpFunction.java

@@ -15,7 +15,7 @@
 package com.google.gerrit.server.workflow;
 
 import com.google.gerrit.client.data.ApprovalType;
-import com.google.gerrit.client.reviewdb.Account;
+import com.google.gerrit.server.CurrentUser;
 
 /** A function that does nothing. */
 public class NoOpFunction extends CategoryFunction {
@@ -26,7 +26,7 @@ public class NoOpFunction extends CategoryFunction {
   }
 
   @Override
-  public boolean isValid(final Account.Id accountId, final ApprovalType at,
+  public boolean isValid(final CurrentUser user, final ApprovalType at,
       final FunctionState state) {
     return false;
   }

src/main/java/com/google/gerrit/server/workflow/SubmitFunction.java

@@ -15,11 +15,11 @@
 package com.google.gerrit.server.workflow;
 
 import com.google.gerrit.client.data.ApprovalType;
-import com.google.gerrit.client.reviewdb.Account;
 import com.google.gerrit.client.reviewdb.ApprovalCategory;
 import com.google.gerrit.client.reviewdb.Change;
 import com.google.gerrit.client.reviewdb.ProjectRight;
 import com.google.gerrit.client.rpc.Common;
+import com.google.gerrit.server.CurrentUser;
 
 /**
  * Computes if the submit function can be used.
@@ -40,11 +40,11 @@ public class SubmitFunction extends CategoryFunction {
   }
 
   @Override
-  public boolean isValid(final Account.Id accountId, final ApprovalType at,
+  public boolean isValid(final CurrentUser user, final ApprovalType at,
       final FunctionState state) {
     if (valid(at, state)) {
       for (final ProjectRight pr : state.getAllRights(at)) {
-        if (state.isMember(accountId, pr.getAccountGroupId())
+        if (user.getEffectiveGroups().contains(pr.getAccountGroupId())
             && pr.getMaxValue() > 0) {
           return true;
         }