Add #absentUser() to ForProject, ForRef and ForChange

We used to think adding this method to the top level is enough
and could avoid misuses. But it turns out we still need them in
other permission backend classes so that we only need to rescope
a user when necessary, which could provide better performance.
See I67b72b59 as an example.

Change-Id: I9ef30fb38315250fa63fb8b3d27e19e19d5e3e22

java/com/google/gerrit/server/permissions/ChangeControl.java

@@ -29,6 +29,7 @@ import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.Project;
 import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.CurrentUser;
+import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.notedb.ChangeNotes;
 import com.google.gerrit.server.permissions.PermissionBackend.ForChange;
 import com.google.gerrit.server.query.change.ChangeData;
@@ -47,11 +48,16 @@ class ChangeControl {
   static class Factory {
     private final ChangeData.Factory changeDataFactory;
     private final ChangeNotes.Factory notesFactory;
+    private final IdentifiedUser.GenericFactory identifiedUserFactory;
 
     @Inject
-    Factory(ChangeData.Factory changeDataFactory, ChangeNotes.Factory notesFactory) {
+    Factory(
+        ChangeData.Factory changeDataFactory,
+        ChangeNotes.Factory notesFactory,
+        IdentifiedUser.GenericFactory identifiedUserFactory) {
       this.changeDataFactory = changeDataFactory;
       this.notesFactory = notesFactory;
+      this.identifiedUserFactory = identifiedUserFactory;
     }
 
     ChangeControl create(
@@ -61,17 +67,22 @@ class ChangeControl {
     }
 
     ChangeControl create(RefControl refControl, ChangeNotes notes) {
-      return new ChangeControl(changeDataFactory, refControl, notes);
+      return new ChangeControl(changeDataFactory, identifiedUserFactory, refControl, notes);
     }
   }
 
   private final ChangeData.Factory changeDataFactory;
+  private final IdentifiedUser.GenericFactory identifiedUserFactory;
   private final RefControl refControl;
   private final ChangeNotes notes;
 
   private ChangeControl(
-      ChangeData.Factory changeDataFactory, RefControl refControl, ChangeNotes notes) {
+      ChangeData.Factory changeDataFactory,
+      IdentifiedUser.GenericFactory identifiedUserFactory,
+      RefControl refControl,
+      ChangeNotes notes) {
     this.changeDataFactory = changeDataFactory;
+    this.identifiedUserFactory = identifiedUserFactory;
     this.refControl = refControl;
     this.notes = notes;
   }
@@ -84,7 +95,8 @@ class ChangeControl {
     if (getUser().equals(who)) {
       return this;
     }
-    return new ChangeControl(changeDataFactory, refControl.forUser(who), notes);
+    return new ChangeControl(
+        changeDataFactory, identifiedUserFactory, refControl.forUser(who), notes);
   }
 
   private CurrentUser getUser() {
@@ -260,6 +272,11 @@ class ChangeControl {
       return user().equals(user) ? this : forUser(user).asForChange(cd, db);
     }
 
+    @Override
+    public ForChange absentUser(Account.Id id) {
+      return user(identifiedUserFactory.create(id));
+    }
+
     @Override
     public String resourcePath() {
       if (resourcePath == null) {

java/com/google/gerrit/server/permissions/DefaultPermissionBackend.java

@@ -79,8 +79,8 @@ public class DefaultPermissionBackend extends PermissionBackend {
   }
 
   @Override
-  public WithUser absentUser(Account.Id user) {
-    IdentifiedUser identifiedUser = identifiedUserFactory.create(checkNotNull(user, "user"));
+  public WithUser absentUser(Account.Id id) {
+    IdentifiedUser identifiedUser = identifiedUserFactory.create(checkNotNull(id, "user"));
     return new WithUserImpl(identifiedUser);
   }
 

java/com/google/gerrit/server/permissions/FailedPermissionBackend.java

@@ -15,6 +15,7 @@
 package com.google.gerrit.server.permissions;
 
 import com.google.gerrit.extensions.api.access.GlobalOrPluginPermission;
+import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.Project;
 import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.CurrentUser;
@@ -128,6 +129,11 @@ public class FailedPermissionBackend {
       return this;
     }
 
+    @Override
+    public ForProject absentUser(Account.Id id) {
+      return this;
+    }
+
     @Override
     public String resourcePath() {
       throw new UnsupportedOperationException(
@@ -181,6 +187,11 @@ public class FailedPermissionBackend {
       return this;
     }
 
+    @Override
+    public ForRef absentUser(Account.Id id) {
+      return this;
+    }
+
     @Override
     public String resourcePath() {
       throw new UnsupportedOperationException(
@@ -233,6 +244,11 @@ public class FailedPermissionBackend {
       return this;
     }
 
+    @Override
+    public ForChange absentUser(Account.Id id) {
+      return this;
+    }
+
     @Override
     public String resourcePath() {
       throw new UnsupportedOperationException(

java/com/google/gerrit/server/permissions/PermissionBackend.java

@@ -112,7 +112,7 @@ public abstract class PermissionBackend {
    *
    * <p>Usage should be very limited as this can expose a group-oracle.
    */
-  public abstract WithUser absentUser(Account.Id user);
+  public abstract WithUser absentUser(Account.Id id);
 
   /**
    * Check whether this {@code PermissionBackend} respects the same global capabilities as the
@@ -305,6 +305,9 @@ public abstract class PermissionBackend {
     /** Returns a new instance rescoped to same project, but different {@code user}. */
     public abstract ForProject user(CurrentUser user);
 
+    /** @see PermissionBackend#absentUser(Account.Id) */
+    public abstract ForProject absentUser(Account.Id id);
+
     /** Returns an instance scoped for {@code ref} in this project. */
     public abstract ForRef ref(String ref);
 
@@ -413,6 +416,9 @@ public abstract class PermissionBackend {
     /** Returns a new instance rescoped to same reference, but different {@code user}. */
     public abstract ForRef user(CurrentUser user);
 
+    /** @see PermissionBackend#absentUser(Account.Id) */
+    public abstract ForRef absentUser(Account.Id id);
+
     /** Returns an instance scoped to change. */
     public abstract ForChange change(ChangeData cd);
 
@@ -471,6 +477,9 @@ public abstract class PermissionBackend {
     /** Returns a new instance rescoped to same change, but different {@code user}. */
     public abstract ForChange user(CurrentUser user);
 
+    /** @see PermissionBackend#absentUser(Account.Id) */
+    public abstract ForChange absentUser(Account.Id id);
+
     /** Verify scoped user can {@code perm}, throwing if denied. */
     public abstract void check(ChangePermissionOrLabel perm)
         throws AuthException, PermissionBackendException;

java/com/google/gerrit/server/permissions/ProjectControl.java

@@ -20,6 +20,7 @@ import com.google.gerrit.common.data.AccessSection;
 import com.google.gerrit.common.data.Permission;
 import com.google.gerrit.common.data.PermissionRule;
 import com.google.gerrit.extensions.restapi.AuthException;
+import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.AccountGroup;
 import com.google.gerrit.reviewdb.client.Branch;
 import com.google.gerrit.reviewdb.client.Change;
@@ -27,6 +28,7 @@ import com.google.gerrit.reviewdb.client.Project;
 import com.google.gerrit.reviewdb.client.RefNames;
 import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.CurrentUser;
+import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.account.GroupMembership;
 import com.google.gerrit.server.config.GitReceivePackGroups;
 import com.google.gerrit.server.config.GitUploadPackGroups;
@@ -67,6 +69,7 @@ class ProjectControl {
   private final ChangeControl.Factory changeControlFactory;
   private final PermissionCollection.Factory permissionFilter;
   private final DefaultRefFilter.Factory refFilterFactory;
+  private final IdentifiedUser.GenericFactory identifiedUserFactory;
 
   private List<SectionMatcher> allSections;
   private Map<String, RefControl> refControls;
@@ -80,6 +83,7 @@ class ProjectControl {
       ChangeControl.Factory changeControlFactory,
       PermissionBackend permissionBackend,
       DefaultRefFilter.Factory refFilterFactory,
+      IdentifiedUser.GenericFactory identifiedUserFactory,
       @Assisted CurrentUser who,
       @Assisted ProjectState ps) {
     this.changeControlFactory = changeControlFactory;
@@ -88,6 +92,7 @@ class ProjectControl {
     this.permissionFilter = permissionFilter;
     this.permissionBackend = permissionBackend;
     this.refFilterFactory = refFilterFactory;
+    this.identifiedUserFactory = identifiedUserFactory;
     user = who;
     state = ps;
   }
@@ -101,6 +106,7 @@ class ProjectControl {
             changeControlFactory,
             permissionBackend,
             refFilterFactory,
+            identifiedUserFactory,
             who,
             state);
     // Not per-user, and reusing saves lookup time.
@@ -132,7 +138,7 @@ class ProjectControl {
     RefControl ctl = refControls.get(refName);
     if (ctl == null) {
       PermissionCollection relevant = permissionFilter.filter(access(), refName, user);
-      ctl = new RefControl(this, refName, relevant);
+      ctl = new RefControl(identifiedUserFactory, this, refName, relevant);
       refControls.put(refName, ctl);
     }
     return ctl;
@@ -326,6 +332,11 @@ class ProjectControl {
       return forUser(user).asForProject().database(db);
     }
 
+    @Override
+    public ForProject absentUser(Account.Id id) {
+      return user(identifiedUserFactory.create(id));
+    }
+
     @Override
     public String resourcePath() {
       if (resourcePath == null) {

java/com/google/gerrit/server/permissions/RefControl.java

@@ -21,10 +21,12 @@ import com.google.gerrit.common.data.PermissionRange;
 import com.google.gerrit.common.data.PermissionRule;
 import com.google.gerrit.common.data.PermissionRule.Action;
 import com.google.gerrit.extensions.restapi.AuthException;
+import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.Project;
 import com.google.gerrit.reviewdb.client.RefNames;
 import com.google.gerrit.server.CurrentUser;
+import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.notedb.ChangeNotes;
 import com.google.gerrit.server.permissions.PermissionBackend.ForChange;
 import com.google.gerrit.server.permissions.PermissionBackend.ForRef;
@@ -39,6 +41,7 @@ import java.util.Set;
 
 /** Manages access control for Git references (aka branches, tags). */
 class RefControl {
+  private final IdentifiedUser.GenericFactory identifiedUserFactory;
   private final ProjectControl projectControl;
   private final String refName;
 
@@ -52,7 +55,12 @@ class RefControl {
   private Boolean canForgeCommitter;
   private Boolean isVisible;
 
-  RefControl(ProjectControl projectControl, String ref, PermissionCollection relevant) {
+  RefControl(
+      IdentifiedUser.GenericFactory identifiedUserFactory,
+      ProjectControl projectControl,
+      String ref,
+      PermissionCollection relevant) {
+    this.identifiedUserFactory = identifiedUserFactory;
     this.projectControl = projectControl;
     this.refName = ref;
     this.relevant = relevant;
@@ -71,7 +79,7 @@ class RefControl {
     if (relevant.isUserSpecific()) {
       return newCtl.controlForRef(refName);
     }
-    return new RefControl(newCtl, refName, relevant);
+    return new RefControl(identifiedUserFactory, newCtl, refName, relevant);
   }
 
   /** Is this user a ref owner? */
@@ -403,6 +411,11 @@ class RefControl {
       return forUser(user).asForRef().database(db);
     }
 
+    @Override
+    public ForRef absentUser(Account.Id id) {
+      return user(identifiedUserFactory.create(id));
+    }
+
     @Override
     public String resourcePath() {
       if (resourcePath == null) {

javatests/com/google/gerrit/server/config/ListCapabilitiesTest.java

@@ -87,7 +87,7 @@ public class ListCapabilitiesTest {
     }
 
     @Override
-    public WithUser absentUser(Id user) {
+    public WithUser absentUser(Id id) {
       throw new UnsupportedOperationException();
     }
 

javatests/com/google/gerrit/server/extensions/webui/UiActionsTest.java

@@ -77,6 +77,11 @@ public class UiActionsTest {
       throw new UnsupportedOperationException("not implemented");
     }
 
+    @Override
+    public ForProject absentUser(Account.Id id) {
+      throw new UnsupportedOperationException("not implemented");
+    }
+
     @Override
     public ForRef ref(String ref) {
       throw new UnsupportedOperationException("not implemented");

javatests/com/google/gerrit/server/permissions/RefControlTest.java

@@ -47,6 +47,7 @@ import com.google.gerrit.reviewdb.client.AccountGroup;
 import com.google.gerrit.reviewdb.client.Project;
 import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.CurrentUser;
+import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.account.CapabilityCollection;
 import com.google.gerrit.server.account.GroupMembership;
 import com.google.gerrit.server.account.ListGroupMembership;
@@ -202,6 +203,7 @@ public class RefControlTest {
   @Inject private InMemoryDatabase schemaFactory;
   @Inject private ThreadLocalRequestContext requestContext;
   @Inject private DefaultRefFilter.Factory refFilterFactory;
+  @Inject private IdentifiedUser.GenericFactory identifiedUserFactory;
 
   @Before
   public void setUp() throws Exception {
@@ -986,6 +988,7 @@ public class RefControlTest {
         changeControlFactory,
         permissionBackend,
         refFilterFactory,
+        identifiedUserFactory,
         new MockUser(name, memberOf),
         newProjectState(local));
   }