Convert EqualsLabelPredicate to use PermissionBackend

Change-Id: I7bfba86c99ab5e914e4c0bc7a4d1b2132508625d

gerrit-server/src/main/java/com/google/gerrit/server/permissions/PermissionBackend.java

@@ -262,8 +262,8 @@ public abstract class PermissionBackend {
      * @throws PermissionBackendException backend cannot run test or check.
      */
     public short squashThenCheck(LabelType label, short val) throws PermissionBackendException {
-      short s = nearest(test(label), val);
+      short s = squashByTest(label, val);
-      if (s == 0) {
+      if (s == 0 || s == val) {
         return 0;
       }
       try {
@@ -274,11 +274,28 @@ public abstract class PermissionBackend {
       }
     }
 
+    /**
+     * Squash a label value to the nearest allowed value using only test methods.
+     *
+     * <p>Tests all possible values and selects the closet available to {@code val} while matching
+     * the sign of {@code val}. Unlike {@code #squashThenCheck(LabelType, short)} this method only
+     * uses {@code test} methods and should not be used in contexts like a review handler without
+     * checking the resulting score.
+     *
+     * @param label definition of the label to test values of.
+     * @param val previously denied value the user attempted.
+     * @return nearest likely allowed value, or {@code 0} if no value was identified.
+     * @throws PermissionBackendException backend cannot run test.
+     */
+    public short squashByTest(LabelType label, short val) throws PermissionBackendException {
+      return nearest(test(label), val);
+    }
+
     private static short nearest(Iterable<LabelPermission.WithValue> possible, short wanted) {
       short s = 0;
       for (LabelPermission.WithValue v : possible) {
-        if ((wanted < 0 && v.value() < 0 && wanted < v.value() && v.value() < s)
+        if ((wanted < 0 && v.value() < 0 && wanted <= v.value() && v.value() < s)
-            || (wanted > 0 && v.value() > 0 && wanted > v.value() && v.value() > s)) {
+            || (wanted > 0 && v.value() > 0 && wanted >= v.value() && v.value() > s)) {
           s = v.value();
         }
       }

gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java

@@ -66,6 +66,7 @@ import com.google.gerrit.server.notedb.ChangeNotes;
 import com.google.gerrit.server.notedb.NotesMigration;
 import com.google.gerrit.server.notedb.ReviewerStateInternal;
 import com.google.gerrit.server.patch.PatchListCache;
+import com.google.gerrit.server.permissions.PermissionBackend;
 import com.google.gerrit.server.project.ChangeControl;
 import com.google.gerrit.server.project.ListChildProjects;
 import com.google.gerrit.server.project.ProjectCache;
@@ -182,6 +183,7 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
     final AccountResolver accountResolver;
     final AllProjectsName allProjectsName;
     final AllUsersName allUsersName;
+    final PermissionBackend permissionBackend;
     final CapabilityControl.Factory capabilityControlFactory;
     final ChangeControl.GenericFactory changeControlGenericFactory;
     final ChangeData.Factory changeDataFactory;
@@ -221,6 +223,7 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
         DynamicMap<ChangeHasOperandFactory> hasOperands,
         IdentifiedUser.GenericFactory userFactory,
         Provider<CurrentUser> self,
+        PermissionBackend permissionBackend,
         CapabilityControl.Factory capabilityControlFactory,
         ChangeControl.GenericFactory changeControlGenericFactory,
         ChangeNotes.Factory notesFactory,
@@ -253,6 +256,7 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
           hasOperands,
           userFactory,
           self,
+          permissionBackend,
           capabilityControlFactory,
           changeControlGenericFactory,
           notesFactory,
@@ -287,6 +291,7 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
         DynamicMap<ChangeHasOperandFactory> hasOperands,
         IdentifiedUser.GenericFactory userFactory,
         Provider<CurrentUser> self,
+        PermissionBackend permissionBackend,
         CapabilityControl.Factory capabilityControlFactory,
         ChangeControl.GenericFactory changeControlGenericFactory,
         ChangeNotes.Factory notesFactory,
@@ -317,6 +322,7 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
       this.opFactories = opFactories;
       this.userFactory = userFactory;
       this.self = self;
+      this.permissionBackend = permissionBackend;
       this.capabilityControlFactory = capabilityControlFactory;
       this.notesFactory = notesFactory;
       this.changeControlGenericFactory = changeControlGenericFactory;
@@ -353,6 +359,7 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData> {
           hasOperands,
           userFactory,
           Providers.of(otherUser),
+          permissionBackend,
           capabilityControlFactory,
           changeControlGenericFactory,
           notesFactory,

gerrit-server/src/main/java/com/google/gerrit/server/query/change/EqualsLabelPredicate.java

@@ -16,7 +16,6 @@ package com.google.gerrit.server.query.change;
 
 import com.google.gerrit.common.data.LabelType;
 import com.google.gerrit.common.data.LabelTypes;
-import com.google.gerrit.common.data.Permission;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.AccountGroup;
 import com.google.gerrit.reviewdb.client.Change;
@@ -24,8 +23,9 @@ import com.google.gerrit.reviewdb.client.PatchSetApproval;
 import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.index.change.ChangeField;
-import com.google.gerrit.server.project.ChangeControl;
+import com.google.gerrit.server.permissions.ChangePermission;
-import com.google.gerrit.server.project.NoSuchChangeException;
+import com.google.gerrit.server.permissions.PermissionBackend;
+import com.google.gerrit.server.permissions.PermissionBackendException;
 import com.google.gerrit.server.project.ProjectCache;
 import com.google.gerrit.server.project.ProjectState;
 import com.google.gwtorm.server.OrmException;
@@ -33,7 +33,7 @@ import com.google.inject.Provider;
 
 class EqualsLabelPredicate extends ChangeIndexPredicate {
   private final ProjectCache projectCache;
-  private final ChangeControl.GenericFactory ccFactory;
+  private final PermissionBackend permissionBackend;
   private final IdentifiedUser.GenericFactory userFactory;
   private final Provider<ReviewDb> dbProvider;
   private final String label;
@@ -43,7 +43,7 @@ class EqualsLabelPredicate extends ChangeIndexPredicate {
 
   EqualsLabelPredicate(LabelPredicate.Args args, String label, int expVal, Account.Id account) {
     super(ChangeField.LABEL, ChangeField.formatLabel(label, expVal, account));
-    this.ccFactory = args.ccFactory;
+    this.permissionBackend = args.permissionBackend;
     this.projectCache = args.projectCache;
     this.userFactory = args.userFactory;
     this.dbProvider = args.dbProvider;
@@ -78,7 +78,7 @@ class EqualsLabelPredicate extends ChangeIndexPredicate {
     for (PatchSetApproval p : object.currentApprovals()) {
       if (labelType.matches(p)) {
         hasVote = true;
-        if (match(c, p.getValue(), p.getAccountId(), labelType)) {
+        if (match(object, p.getValue(), p.getAccountId(), labelType)) {
           return true;
         }
       }
@@ -104,40 +104,28 @@ class EqualsLabelPredicate extends ChangeIndexPredicate {
     return null;
   }
 
-  private boolean match(Change change, int value, Account.Id approver, LabelType type)
+  private boolean match(ChangeData cd, short value, Account.Id approver, LabelType type) {
-      throws OrmException {
+    if (value != expVal) {
-    int psVal = value;
+      return false;
-    if (psVal == expVal) {
+    }
-      // Double check the value is still permitted for the user.
+
-      //
+    if (account != null && !account.equals(approver)) {
-      IdentifiedUser reviewer = userFactory.create(approver);
+      return false;
-      try {
+    }
-        ChangeControl cc = ccFactory.controlFor(dbProvider.get(), change, reviewer);
+
-        if (!cc.isVisible(dbProvider.get())) {
+    IdentifiedUser reviewer = userFactory.create(approver);
-          // The user can't see the change anymore.
+    if (group != null && !reviewer.getEffectiveGroups().contains(group)) {
-          //
+      return false;
-          return false;
+    }
-        }
+
-        psVal = cc.getRange(Permission.forLabel(type.getName())).squash(psVal);
+    // Double check the value is still permitted for the user.
-      } catch (NoSuchChangeException e) {
+    try {
-        // The project has disappeared.
+      PermissionBackend.ForChange perm =
-        //
+          permissionBackend.user(reviewer).database(dbProvider).change(cd);
-        return false;
+      return perm.test(ChangePermission.READ) && expVal == perm.squashByTest(type, value);
-      }
+    } catch (PermissionBackendException e) {
-
+      return false;
-      if (account != null && !account.equals(approver)) {
-        return false;
-      }
-
-      if (group != null && !reviewer.getEffectiveGroups().contains(group)) {
-        return false;
-      }
-
-      if (psVal == expVal) {
-        return true;
-      }
     }
-    return false;
   }
 
   @Override

gerrit-server/src/main/java/com/google/gerrit/server/query/change/LabelPredicate.java

@@ -19,6 +19,7 @@ import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.AccountGroup;
 import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.server.permissions.PermissionBackend;
 import com.google.gerrit.server.project.ChangeControl;
 import com.google.gerrit.server.project.ProjectCache;
 import com.google.gerrit.server.query.OrPredicate;
@@ -36,6 +37,7 @@ public class LabelPredicate extends OrPredicate<ChangeData> {
 
   static class Args {
     final ProjectCache projectCache;
+    final PermissionBackend permissionBackend;
     final ChangeControl.GenericFactory ccFactory;
     final IdentifiedUser.GenericFactory userFactory;
     final Provider<ReviewDb> dbProvider;
@@ -45,6 +47,7 @@ public class LabelPredicate extends OrPredicate<ChangeData> {
 
     private Args(
         ProjectCache projectCache,
+        PermissionBackend permissionBackend,
         ChangeControl.GenericFactory ccFactory,
         IdentifiedUser.GenericFactory userFactory,
         Provider<ReviewDb> dbProvider,
@@ -52,6 +55,7 @@ public class LabelPredicate extends OrPredicate<ChangeData> {
         Set<Account.Id> accounts,
         AccountGroup.UUID group) {
       this.projectCache = projectCache;
+      this.permissionBackend = permissionBackend;
       this.ccFactory = ccFactory;
       this.userFactory = userFactory;
       this.dbProvider = dbProvider;
@@ -84,6 +88,7 @@ public class LabelPredicate extends OrPredicate<ChangeData> {
         predicates(
             new Args(
                 a.projectCache,
+                a.permissionBackend,
                 a.changeControlGenericFactory,
                 a.userFactory,
                 a.db,

gerrit-server/src/test/java/com/google/gerrit/server/index/change/FakeQueryBuilder.java

@@ -27,8 +27,8 @@ public class FakeQueryBuilder extends ChangeQueryBuilder {
         new FakeQueryBuilder.Definition<>(FakeQueryBuilder.class),
         new ChangeQueryBuilder.Arguments(
             null, null, null, null, null, null, null, null, null, null, null, null, null, null,
-            null, null, null, null, null, null, null, indexes, null, null, null, null, null, null,
+            null, null, null, null, null, null, null, null, indexes, null, null, null, null, null,
-            null, null, null));
+            null, null, null, null));
   }
 
   @Operator