Merge changes Idf06a875,I6b10e93f

* changes:
  Make ChangeControl#getRange private and migrate caller
  Migrate ApprovalsUtil#checkApprovals() to PermissionBackend

gerrit-server/src/main/java/com/google/gerrit/server/ApprovalCopier.java

@@ -30,6 +30,7 @@ import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.change.ChangeKindCache;
 import com.google.gerrit.server.git.LabelNormalizer;
 import com.google.gerrit.server.notedb.NoteDbChangeState.PrimaryStorage;
+import com.google.gerrit.server.permissions.PermissionBackendException;
 import com.google.gerrit.server.project.ChangeControl;
 import com.google.gerrit.server.project.ProjectCache;
 import com.google.gerrit.server.project.ProjectState;
@@ -204,7 +205,7 @@ public class ApprovalCopier {
         }
       }
       return labelNormalizer.normalize(ctl, byUser.values()).getNormalized();
-    } catch (IOException e) {
+    } catch (IOException | PermissionBackendException e) {
       throw new OrmException(e);
     }
   }

gerrit-server/src/main/java/com/google/gerrit/server/ApprovalsUtil.java

@@ -31,8 +31,6 @@ import com.google.common.primitives.Shorts;
 import com.google.gerrit.common.Nullable;
 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.common.data.PermissionRange;
 import com.google.gerrit.extensions.restapi.AuthException;
 import com.google.gerrit.extensions.restapi.BadRequestException;
 import com.google.gerrit.extensions.restapi.RestApiException;
@@ -48,6 +46,7 @@ import com.google.gerrit.server.notedb.ChangeUpdate;
 import com.google.gerrit.server.notedb.NotesMigration;
 import com.google.gerrit.server.notedb.ReviewerStateInternal;
 import com.google.gerrit.server.permissions.ChangePermission;
+import com.google.gerrit.server.permissions.LabelPermission;
 import com.google.gerrit.server.permissions.PermissionBackend;
 import com.google.gerrit.server.permissions.PermissionBackendException;
 import com.google.gerrit.server.project.ChangeControl;
@@ -308,7 +307,7 @@ public class ApprovalsUtil {
    * @param update change update.
    * @param labelTypes label types for the containing project.
    * @param ps patch set being approved.
-   * @param changeCtl change control for user adding approvals.
+   * @param user user adding approvals.
    * @param approvals approvals to add.
    * @throws RestApiException
    * @throws OrmException
@@ -318,10 +317,10 @@ public class ApprovalsUtil {
       ChangeUpdate update,
       LabelTypes labelTypes,
       PatchSet ps,
-      ChangeControl changeCtl,
+      CurrentUser user,
       Map<String, Short> approvals)
-      throws RestApiException, OrmException {
+      throws RestApiException, OrmException, PermissionBackendException {
-    Account.Id accountId = changeCtl.getUser().getAccountId();
+    Account.Id accountId = user.getAccountId();
     checkArgument(
         accountId.equals(ps.getUploader()),
         "expected user %s to match patch set uploader %s",
@@ -330,12 +329,12 @@ public class ApprovalsUtil {
     if (approvals.isEmpty()) {
       return ImmutableList.of();
     }
-    checkApprovals(approvals, changeCtl);
+    checkApprovals(approvals, permissionBackend.user(user).database(db).change(update.getNotes()));
     List<PatchSetApproval> cells = new ArrayList<>(approvals.size());
     Date ts = update.getWhen();
     for (Map.Entry<String, Short> vote : approvals.entrySet()) {
       LabelType lt = labelTypes.byLabel(vote.getKey());
-      cells.add(newApproval(ps.getId(), changeCtl.getUser(), lt.getLabelId(), vote.getValue(), ts));
+      cells.add(newApproval(ps.getId(), user, lt.getLabelId(), vote.getValue(), ts));
     }
     for (PatchSetApproval psa : cells) {
       update.putApproval(psa.getLabel(), psa.getValue());
@@ -356,13 +355,15 @@ public class ApprovalsUtil {
     }
   }
 
-  private static void checkApprovals(Map<String, Short> approvals, ChangeControl changeCtl)
+  private static void checkApprovals(
-      throws AuthException {
+      Map<String, Short> approvals, PermissionBackend.ForChange forChange)
+      throws AuthException, PermissionBackendException {
     for (Map.Entry<String, Short> vote : approvals.entrySet()) {
       String name = vote.getKey();
       Short value = vote.getValue();
-      PermissionRange range = changeCtl.getRange(Permission.forLabel(name));
+      try {
-      if (range == null || !range.contains(value)) {
+        forChange.check(new LabelPermission.WithValue(name, value));
+      } catch (AuthException e) {
         throw new AuthException(
             String.format("applying label \"%s\": %d is restricted", name, value));
       }

gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java

@@ -378,7 +378,7 @@ public class ChangeInserter implements InsertChangeOp {
 
   @Override
   public boolean updateChange(ChangeContext ctx)
-      throws RestApiException, OrmException, IOException {
+      throws RestApiException, OrmException, IOException, PermissionBackendException {
     change = ctx.getChange(); // Use defensive copy created by ChangeControl.
     ReviewDb db = ctx.getDb();
     ChangeControl ctl = ctx.getControl();
@@ -444,7 +444,7 @@ public class ChangeInserter implements InsertChangeOp {
         filterOnChangeVisibility(db, ctx.getNotes(), reviewersToAdd),
         Collections.<Account.Id>emptySet());
     approvalsUtil.addApprovalsForNewPatchSet(
-        db, update, labelTypes, patchSet, ctx.getControl(), approvals);
+        db, update, labelTypes, patchSet, ctx.getUser(), approvals);
     // Check if approvals are changing in with this update. If so, add current user to reviewers.
     // Note that this is done separately as addReviewers is filtering out the change owner as
     // reviewer which is needed in several other code paths.

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

@@ -24,13 +24,15 @@ import com.google.common.collect.Lists;
 import com.google.gerrit.common.data.LabelType;
 import com.google.gerrit.common.data.LabelTypes;
 import com.google.gerrit.common.data.LabelValue;
-import com.google.gerrit.common.data.Permission;
+import com.google.gerrit.extensions.restapi.AuthException;
-import com.google.gerrit.common.data.PermissionRange;
-import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.Change;
 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.notedb.ChangeNotes;
+import com.google.gerrit.server.permissions.LabelPermission;
+import com.google.gerrit.server.permissions.PermissionBackend;
+import com.google.gerrit.server.permissions.PermissionBackendException;
 import com.google.gerrit.server.project.ChangeControl;
 import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
@@ -76,15 +78,18 @@ public class LabelNormalizer {
   private final Provider<ReviewDb> db;
   private final ChangeControl.GenericFactory changeFactory;
   private final IdentifiedUser.GenericFactory userFactory;
+  private final PermissionBackend permissionBackend;
 
   @Inject
   LabelNormalizer(
       Provider<ReviewDb> db,
       ChangeControl.GenericFactory changeFactory,
-      IdentifiedUser.GenericFactory userFactory) {
+      IdentifiedUser.GenericFactory userFactory,
+      PermissionBackend permissionBackend) {
     this.db = db;
     this.changeFactory = changeFactory;
     this.userFactory = userFactory;
+    this.permissionBackend = permissionBackend;
   }
 
   /**
@@ -96,7 +101,7 @@ public class LabelNormalizer {
    * @throws OrmException
    */
   public Result normalize(Change change, Collection<PatchSetApproval> approvals)
-      throws OrmException {
+      throws OrmException, PermissionBackendException {
     IdentifiedUser user = userFactory.create(change.getOwner());
     return normalize(changeFactory.controlFor(db.get(), change, user), approvals);
   }
@@ -108,7 +113,8 @@ public class LabelNormalizer {
    *     for the user. Approvals for unknown labels are not included in the output, nor are
    *     approvals where the user has no permissions for that label.
    */
-  public Result normalize(ChangeControl ctl, Collection<PatchSetApproval> approvals) {
+  public Result normalize(ChangeControl ctl, Collection<PatchSetApproval> approvals)
+      throws PermissionBackendException {
     List<PatchSetApproval> unchanged = Lists.newArrayListWithCapacity(approvals.size());
     List<PatchSetApproval> updated = Lists.newArrayListWithCapacity(approvals.size());
     List<PatchSetApproval> deleted = Lists.newArrayListWithCapacity(approvals.size());
@@ -132,7 +138,7 @@ public class LabelNormalizer {
       }
       PatchSetApproval copy = copy(psa);
       applyTypeFloor(label, copy);
-      if (!applyRightFloor(ctl, label, copy)) {
+      if (!applyRightFloor(ctl.getNotes(), label, copy)) {
         deleted.add(psa);
       } else if (copy.getValue() != psa.getValue()) {
         updated.add(copy);
@@ -147,19 +153,24 @@ public class LabelNormalizer {
     return new PatchSetApproval(src.getPatchSetId(), src);
   }
 
-  private PermissionRange getRange(ChangeControl ctl, LabelType lt, Account.Id id) {
+  private boolean applyRightFloor(ChangeNotes notes, LabelType lt, PatchSetApproval a)
-    String permission = Permission.forLabel(lt.getName());
+      throws PermissionBackendException {
-    IdentifiedUser user = userFactory.create(id);
+    PermissionBackend.ForChange forChange =
-    return ctl.forUser(user).getRange(permission);
+        permissionBackend.user(userFactory.create(a.getAccountId())).database(db).change(notes);
-  }
+    // Check if the user is allowed to vote on the label at all
-
+    try {
-  private boolean applyRightFloor(ChangeControl ctl, LabelType lt, PatchSetApproval a) {
+      forChange.check(new LabelPermission(lt.getName()));
-    PermissionRange range = getRange(ctl, lt, a.getAccountId());
+    } catch (AuthException e) {
-    if (range.isEmpty()) {
       return false;
     }
-    a.setValue((short) range.squash(a.getValue()));
+    // Squash vote to nearest allowed value
-    return true;
+    try {
+      forChange.check(new LabelPermission.WithValue(lt.getName(), a.getValue()));
+      return true;
+    } catch (AuthException e) {
+      a.setValue(forChange.squashThenCheck(lt, a.getValue()));
+      return true;
+    }
   }
 
   private void applyTypeFloor(LabelType lt, PatchSetApproval a) {

gerrit-server/src/main/java/com/google/gerrit/server/git/receive/ReplaceOp.java

@@ -52,6 +52,7 @@ import com.google.gerrit.server.mail.MailUtil.MailRecipients;
 import com.google.gerrit.server.mail.send.ReplacePatchSetSender;
 import com.google.gerrit.server.notedb.ChangeNotes;
 import com.google.gerrit.server.notedb.ChangeUpdate;
+import com.google.gerrit.server.permissions.PermissionBackendException;
 import com.google.gerrit.server.project.ProjectCache;
 import com.google.gerrit.server.project.ProjectControl;
 import com.google.gerrit.server.query.change.ChangeData;
@@ -224,7 +225,7 @@ public class ReplaceOp implements BatchUpdateOp {
 
   @Override
   public boolean updateChange(ChangeContext ctx)
-      throws RestApiException, OrmException, IOException {
+      throws RestApiException, OrmException, IOException, PermissionBackendException {
     notes = ctx.getNotes();
     Change change = notes.getChange();
     if (change == null || change.getStatus().isClosed()) {
@@ -306,7 +307,7 @@ public class ReplaceOp implements BatchUpdateOp {
             update,
             projectControl.getProjectState().getLabelTypes(),
             newPatchSet,
-            ctx.getControl(),
+            ctx.getUser(),
             approvals);
     approvalCopier.copyInReviewDb(
         ctx.getDb(),

gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/SubmitStrategyOp.java

@@ -45,6 +45,7 @@ import com.google.gerrit.server.git.MergeUtil;
 import com.google.gerrit.server.git.ProjectConfig;
 import com.google.gerrit.server.git.SubmoduleException;
 import com.google.gerrit.server.notedb.ChangeUpdate;
+import com.google.gerrit.server.permissions.PermissionBackendException;
 import com.google.gerrit.server.project.ProjectState;
 import com.google.gerrit.server.update.BatchUpdateOp;
 import com.google.gerrit.server.update.ChangeContext;
@@ -330,7 +331,7 @@ abstract class SubmitStrategyOp implements BatchUpdateOp {
   }
 
   private void setApproval(ChangeContext ctx, IdentifiedUser user)
-      throws OrmException, IOException {
+      throws OrmException, IOException, PermissionBackendException {
     Change.Id id = ctx.getChange().getId();
     List<SubmitRecord> records = args.commitStatus.getSubmitRecords(id);
     PatchSet.Id oldPsId = toMerge.getPatchsetId();
@@ -352,7 +353,7 @@ abstract class SubmitStrategyOp implements BatchUpdateOp {
   }
 
   private LabelNormalizer.Result approve(ChangeContext ctx, ChangeUpdate update)
-      throws OrmException, IOException {
+      throws OrmException, IOException, PermissionBackendException {
     PatchSet.Id psId = update.getPatchSetId();
     Map<PatchSetApproval.Key, PatchSetApproval> byKey = new HashMap<>();
     for (PatchSetApproval psa :

gerrit-server/src/main/java/com/google/gerrit/server/project/ChangeControl.java

@@ -269,7 +269,7 @@ public class ChangeControl {
   }
 
   /** The range of permitted values associated with a label permission. */
-  public PermissionRange getRange(String permission) {
+  private PermissionRange getRange(String permission) {
     return getRefControl().getRange(permission, isOwner());
   }