Remove unused label details from old RPC handlers

Change-Id: Ibe2ea9dc416e0e824cfacc5e5e26855b051734a2

gerrit-common/src/main/java/com/google/gerrit/common/data/ApprovalDetail.java

@@ -15,12 +15,9 @@
 package com.google.gerrit.common.data;
 
 import com.google.gerrit.reviewdb.client.Account;
-import com.google.gerrit.reviewdb.client.ApprovalCategory;
 import com.google.gerrit.reviewdb.client.PatchSetApproval;
 
-import java.sql.Timestamp;
 import java.util.ArrayList;
-import java.util.Comparator;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
@@ -28,19 +25,6 @@ import java.util.Map;
 import java.util.Set;
 
 public class ApprovalDetail {
-  public static final Comparator<ApprovalDetail> SORT =
-      new Comparator<ApprovalDetail>() {
-        public int compare(final ApprovalDetail o1, final ApprovalDetail o2) {
-          int cmp;
-          cmp = o2.hasNonZero - o1.hasNonZero;
-          if (cmp != 0) return cmp;
-          return o1.sortOrder.compareTo(o2.sortOrder);
-        }
-      };
-
-  static final Timestamp EG_0 = new Timestamp(0);
-  static final Timestamp EG_D = new Timestamp(Long.MAX_VALUE);
-
   protected Account.Id account;
   protected List<PatchSetApproval> approvals;
   protected boolean canRemove;
@@ -49,8 +33,6 @@ public class ApprovalDetail {
   private transient Set<String> approved;
   private transient Set<String> rejected;
   private transient Map<String, Integer> values;
-  private transient int hasNonZero;
-  private transient Timestamp sortOrder = EG_D;
 
   protected ApprovalDetail() {
   }
@@ -72,41 +54,6 @@ public class ApprovalDetail {
     canRemove = removeable;
   }
 
-  @Deprecated
-  public List<PatchSetApproval> getPatchSetApprovals() {
-    return approvals;
-  }
-
-  @Deprecated
-  public PatchSetApproval getPatchSetApproval(ApprovalCategory.Id category) {
-    for (PatchSetApproval psa : approvals) {
-      if (psa.getCategoryId().equals(category)) {
-        return psa;
-      }
-    }
-    return null;
-  }
-
-  public void sortFirst() {
-    hasNonZero = 1;
-    sortOrder = ApprovalDetail.EG_0;
-  }
-
-  @Deprecated
-  public void add(final PatchSetApproval ca) {
-    approvals.add(ca);
-
-    final Timestamp g = ca.getGranted();
-    if (g != null && g.compareTo(sortOrder) < 0) {
-      sortOrder = g;
-      // Value is not set, but code calling this deprecated method does not
-      // call getValue.
-    }
-    if (ca.getValue() != 0) {
-      hasNonZero = 1;
-    }
-  }
-
   public void approved(String label) {
     if (approved == null) {
       approved = new HashSet<String>();

gerrit-common/src/main/java/com/google/gerrit/common/data/ChangeDetail.java

@@ -19,9 +19,6 @@ import com.google.gerrit.reviewdb.client.ChangeMessage;
 import com.google.gerrit.reviewdb.client.PatchSet;
 import com.google.gerrit.reviewdb.client.Project;
 
-import java.util.ArrayList;
-import java.util.Collection;
-import java.util.Collections;
 import java.util.List;
 
 /** Detail necessary to display a change. */
@@ -40,7 +37,6 @@ public class ChangeDetail {
   protected List<ChangeInfo> dependsOn;
   protected List<ChangeInfo> neededBy;
   protected List<PatchSet> patchSets;
-  protected List<ApprovalDetail> approvals;
   protected List<SubmitRecord> submitRecords;
   protected Project.SubmitType submitType;
   protected SubmitTypeRecord submitTypeRecord;
@@ -191,15 +187,6 @@ public class ChangeDetail {
     patchSets = s;
   }
 
-  public List<ApprovalDetail> getApprovals() {
-    return approvals;
-  }
-
-  public void setApprovals(Collection<ApprovalDetail> list) {
-    approvals = new ArrayList<ApprovalDetail>(list);
-    Collections.sort(approvals, ApprovalDetail.SORT);
-  }
-
   public void setSubmitRecords(List<SubmitRecord> all) {
     submitRecords = all;
   }

gerrit-common/src/main/java/com/google/gerrit/common/data/PatchSetPublishDetail.java

@@ -16,12 +16,8 @@ package com.google.gerrit.common.data;
 
 import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.PatchLineComment;
-import com.google.gerrit.reviewdb.client.PatchSetApproval;
 import com.google.gerrit.reviewdb.client.PatchSetInfo;
 
-import java.util.ArrayList;
-import java.util.Collection;
-import java.util.Collections;
 import java.util.List;
 
 public class PatchSetPublishDetail {
@@ -29,38 +25,10 @@ public class PatchSetPublishDetail {
   protected PatchSetInfo patchSetInfo;
   protected Change change;
   protected List<PatchLineComment> drafts;
-  protected List<PermissionRange> labels;
-  protected List<ApprovalDetail> approvals;
   protected List<SubmitRecord> submitRecords;
   protected SubmitTypeRecord submitTypeRecord;
-  protected List<PatchSetApproval> given;
   protected boolean canSubmit;
 
-  public List<PermissionRange> getLabels() {
-    return labels;
-  }
-
-  public void setLabels(List<PermissionRange> labels) {
-    this.labels = labels;
-  }
-
-  public List<ApprovalDetail> getApprovals() {
-    return approvals;
-  }
-
-  public void setApprovals(Collection<ApprovalDetail> list) {
-    approvals = new ArrayList<ApprovalDetail>(list);
-    Collections.sort(approvals, ApprovalDetail.SORT);
-  }
-
-  public void setSubmitRecords(List<SubmitRecord> all) {
-    submitRecords = all;
-  }
-
-  public List<SubmitRecord> getSubmitRecords() {
-    return submitRecords;
-  }
-
   public void setSubmitTypeRecord(SubmitTypeRecord submitTypeRecord) {
     this.submitTypeRecord = submitTypeRecord;
   }
@@ -69,14 +37,6 @@ public class PatchSetPublishDetail {
     return submitTypeRecord;
   }
 
-  public List<PatchSetApproval> getGiven() {
-    return given;
-  }
-
-  public void setGiven(List<PatchSetApproval> given) {
-    this.given = given;
-  }
-
   public void setAccounts(AccountInfoCache accounts) {
     this.accounts = accounts;
   }
@@ -113,24 +73,6 @@ public class PatchSetPublishDetail {
     return drafts;
   }
 
-  public PermissionRange getRange(final String permissionName) {
-    for (PermissionRange s : labels) {
-      if (s.getName().equals(permissionName)) {
-        return s;
-      }
-    }
-    return null;
-  }
-
-  public PatchSetApproval getChangeApproval(String label) {
-    for (PatchSetApproval a : given) {
-      if (a.getLabel() != null && a.getLabel().equals(label)) {
-        return a;
-      }
-    }
-    return null;
-  }
-
   public boolean canSubmit() {
     return canSubmit;
   }

gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/ChangeDetailFactory.java

@@ -14,12 +14,8 @@
 
 package com.google.gerrit.httpd.rpc.changedetail;
 
-import com.google.gerrit.common.data.ApprovalDetail;
-import com.google.gerrit.common.data.ApprovalType;
-import com.google.gerrit.common.data.ApprovalTypes;
 import com.google.gerrit.common.data.ChangeDetail;
 import com.google.gerrit.common.data.ChangeInfo;
-import com.google.gerrit.common.data.PermissionRange;
 import com.google.gerrit.common.data.SubmitRecord;
 import com.google.gerrit.common.errors.NoSuchEntityException;
 import com.google.gerrit.httpd.rpc.Handler;
@@ -28,7 +24,6 @@ import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.ChangeMessage;
 import com.google.gerrit.reviewdb.client.PatchSet;
 import com.google.gerrit.reviewdb.client.PatchSetAncestor;
-import com.google.gerrit.reviewdb.client.PatchSetApproval;
 import com.google.gerrit.reviewdb.client.RevId;
 import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.AnonymousUser;
@@ -45,8 +40,6 @@ import com.google.gerrit.server.patch.PatchSetInfoNotAvailableException;
 import com.google.gerrit.server.project.ChangeControl;
 import com.google.gerrit.server.project.NoSuchChangeException;
 import com.google.gerrit.server.query.change.ChangeData;
-import com.google.gerrit.server.workflow.CategoryFunction;
-import com.google.gerrit.server.workflow.FunctionState;
 import com.google.gwtorm.server.OrmException;
 import com.google.gwtorm.server.ResultSet;
 import com.google.inject.Inject;
@@ -71,11 +64,7 @@ public class ChangeDetailFactory extends Handler<ChangeDetail> {
     ChangeDetailFactory create(Change.Id id);
   }
 
-  private final ApprovalTypes approvalTypes;
   private final ChangeControl.Factory changeControlFactory;
-  private final ChangeControl.GenericFactory changeControlGenericFactory;
-  private final IdentifiedUser.GenericFactory identifiedUserFactory;
-  private final FunctionState.Factory functionState;
   private final PatchSetDetailFactory.Factory patchSetDetail;
   private final AccountInfoCacheFactory aic;
   private final AnonymousUser anonymousUser;
@@ -96,26 +85,19 @@ public class ChangeDetailFactory extends Handler<ChangeDetail> {
   private List<Change> currentDepChanges;
 
   @Inject
-  ChangeDetailFactory(final ApprovalTypes approvalTypes,
-      final FunctionState.Factory functionState,
+  ChangeDetailFactory(
       final PatchSetDetailFactory.Factory patchSetDetail, final ReviewDb db,
       final GitRepositoryManager repoManager,
       final ChangeControl.Factory changeControlFactory,
-      final ChangeControl.GenericFactory changeControlGenericFactory,
-      final IdentifiedUser.GenericFactory identifiedUserFactory,
       final AccountInfoCacheFactory.Factory accountInfoCacheFactory,
       final AnonymousUser anonymousUser,
       final MergeOp.Factory opFactory,
       @GerritServerConfig final Config cfg,
       @Assisted final Change.Id id) {
-    this.approvalTypes = approvalTypes;
-    this.functionState = functionState;
     this.patchSetDetail = patchSetDetail;
     this.db = db;
     this.repoManager = repoManager;
     this.changeControlFactory = changeControlFactory;
-    this.changeControlGenericFactory = changeControlGenericFactory;
-    this.identifiedUserFactory = identifiedUserFactory;
     this.anonymousUser = anonymousUser;
     this.aic = accountInfoCacheFactory.create();
 
@@ -230,55 +212,6 @@ public class ChangeDetailFactory extends Handler<ChangeDetail> {
         testMerge) {
       ChangeUtil.testMerge(opFactory, detail.getChange());
     }
-
-    final PatchSet.Id psId = detail.getChange().currentPatchSetId();
-    final List<PatchSetApproval> allApprovals =
-        db.patchSetApprovals().byChange(changeId).toList();
-
-    if (detail.getChange().getStatus().isOpen()) {
-      final FunctionState fs = functionState.create(control, psId, allApprovals);
-
-      for (final ApprovalType at : approvalTypes.getApprovalTypes()) {
-        CategoryFunction.forCategory(at.getCategory()).run(at, fs);
-      }
-    }
-
-    final boolean canRemoveReviewers = detail.getChange().getStatus().isOpen() //
-        && control.getCurrentUser() instanceof IdentifiedUser;
-    final HashMap<Account.Id, ApprovalDetail> ad =
-        new HashMap<Account.Id, ApprovalDetail>();
-    for (PatchSetApproval ca : allApprovals) {
-      ApprovalDetail d = ad.get(ca.getAccountId());
-      if (d == null) {
-        d = new ApprovalDetail(ca.getAccountId());
-        d.setCanRemove(canRemoveReviewers);
-        ad.put(d.getAccount(), d);
-      }
-      if (d.canRemove()) {
-        d.setCanRemove(control.canRemoveReviewer(ca));
-      }
-      if (ca.getPatchSetId().equals(psId)) {
-        d.add(ca);
-      }
-      final ChangeControl chgCtrl =
-          changeControlGenericFactory.controlFor(detail.getChange(),
-              identifiedUserFactory.create(ca.getAccountId()));
-      for (PermissionRange pr : chgCtrl.getLabelRanges()) {
-        if (pr.getMin() != 0 || pr.getMax() != 0) {
-          d.votable(pr.getLabel());
-        }
-      }
-    }
-
-    final Account.Id owner = detail.getChange().getOwner();
-    if (ad.containsKey(owner)) {
-      // Ensure the owner always sorts to the top of the table
-      //
-      ad.get(owner).sortFirst();
-    }
-
-    aic.want(ad.keySet());
-    detail.setApprovals(ad.values());
   }
 
   private boolean isReviewer(Change change) {

gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/PatchSetPublishDetailFactory.java

@@ -14,18 +14,15 @@
 
 package com.google.gerrit.httpd.rpc.changedetail;
 
-import com.google.gerrit.common.data.ApprovalDetail;
 import com.google.gerrit.common.data.ApprovalType;
 import com.google.gerrit.common.data.ApprovalTypes;
 import com.google.gerrit.common.data.PatchSetPublishDetail;
 import com.google.gerrit.common.data.PermissionRange;
 import com.google.gerrit.common.data.SubmitRecord;
 import com.google.gerrit.httpd.rpc.Handler;
-import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.PatchLineComment;
 import com.google.gerrit.reviewdb.client.PatchSet;
-import com.google.gerrit.reviewdb.client.PatchSetApproval;
 import com.google.gerrit.reviewdb.client.PatchSetInfo;
 import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.IdentifiedUser;
@@ -34,14 +31,10 @@ import com.google.gerrit.server.patch.PatchSetInfoFactory;
 import com.google.gerrit.server.patch.PatchSetInfoNotAvailableException;
 import com.google.gerrit.server.project.ChangeControl;
 import com.google.gerrit.server.project.NoSuchChangeException;
-import com.google.gerrit.server.workflow.CategoryFunction;
-import com.google.gerrit.server.workflow.FunctionState;
 import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
 import com.google.inject.assistedinject.Assisted;
 
-import java.util.ArrayList;
-import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -53,10 +46,7 @@ final class PatchSetPublishDetailFactory extends Handler<PatchSetPublishDetail>
 
   private final PatchSetInfoFactory infoFactory;
   private final ReviewDb db;
-  private final FunctionState.Factory functionState;
   private final ChangeControl.Factory changeControlFactory;
-  private final ChangeControl.GenericFactory changeControlGenericFactory;
-  private final IdentifiedUser.GenericFactory identifiedUserFactory;
   private final ApprovalTypes approvalTypes;
   private final AccountInfoCacheFactory aic;
   private final IdentifiedUser user;
@@ -71,18 +61,12 @@ final class PatchSetPublishDetailFactory extends Handler<PatchSetPublishDetail>
   PatchSetPublishDetailFactory(final PatchSetInfoFactory infoFactory,
       final ReviewDb db,
       final AccountInfoCacheFactory.Factory accountInfoCacheFactory,
-      final FunctionState.Factory functionState,
       final ChangeControl.Factory changeControlFactory,
-      final ChangeControl.GenericFactory changeControlGenericFactory,
-      final IdentifiedUser.GenericFactory identifiedUserFactory,
       final ApprovalTypes approvalTypes,
       final IdentifiedUser user, @Assisted final PatchSet.Id patchSetId) {
     this.infoFactory = infoFactory;
     this.db = db;
-    this.functionState = functionState;
     this.changeControlFactory = changeControlFactory;
-    this.changeControlGenericFactory = changeControlGenericFactory;
-    this.identifiedUserFactory = identifiedUserFactory;
     this.approvalTypes = approvalTypes;
     this.aic = accountInfoCacheFactory.create();
     this.user = user;
@@ -107,9 +91,6 @@ final class PatchSetPublishDetailFactory extends Handler<PatchSetPublishDetail>
     detail.setChange(change);
     detail.setDrafts(drafts);
 
-    List<PermissionRange> allowed = Collections.emptyList();
-    List<PatchSetApproval> given = Collections.emptyList();
-
     if (change.getStatus().isOpen()
         && patchSetId.equals(change.currentPatchSetId())) {
       // TODO Push this selection of labels down into the Prolog interpreter.
@@ -126,11 +107,6 @@ final class PatchSetPublishDetailFactory extends Handler<PatchSetPublishDetail>
           rangeByName.put(r.getLabel(), r);
         }
       }
-      allowed = new ArrayList<PermissionRange>();
-
-      given = db.patchSetApprovals() //
-          .byPatchSetUser(patchSetId, user.getAccountId()) //
-          .toList();
 
       boolean couldSubmit = false;
       List<SubmitRecord> submitRecords = control.canSubmit(db, patchSet);
@@ -148,10 +124,6 @@ final class PatchSetPublishDetailFactory extends Handler<PatchSetPublishDetail>
             boolean canMakeOk = false;
             PermissionRange range = rangeByName.get(lbl.label);
             if (range != null) {
-              if (!allowed.contains(range)) {
-                allowed.add(range);
-              }
-
               ApprovalType at = approvalTypes.byLabel(lbl.label);
               if (at == null || at.getMax().getValue() == range.getMax()) {
                 canMakeOk = true;
@@ -186,70 +158,11 @@ final class PatchSetPublishDetailFactory extends Handler<PatchSetPublishDetail>
       if (couldSubmit && control.getRefControl().canSubmit()) {
         detail.setCanSubmit(true);
       }
-
-      detail.setSubmitRecords(submitRecords);
     }
 
     detail.setSubmitTypeRecord(control.getSubmitTypeRecord(db, patchSet));
-
-    detail.setLabels(allowed);
-    detail.setGiven(given);
-    loadApprovals(detail, control);
-
     detail.setAccounts(aic.create());
 
     return detail;
   }
-
-  private void loadApprovals(final PatchSetPublishDetail detail,
-      final ChangeControl control) throws OrmException, NoSuchChangeException {
-    final PatchSet.Id psId = detail.getChange().currentPatchSetId();
-    final Change.Id changeId = patchSetId.getParentKey();
-    final List<PatchSetApproval> allApprovals =
-        db.patchSetApprovals().byChange(changeId).toList();
-
-    if (detail.getChange().getStatus().isOpen()) {
-      final FunctionState fs = functionState.create(control, psId, allApprovals);
-
-      for (final ApprovalType at : approvalTypes.getApprovalTypes()) {
-        CategoryFunction.forCategory(at.getCategory()).run(at, fs);
-      }
-    }
-
-    final boolean canRemoveReviewers = detail.getChange().getStatus().isOpen() //
-        && control.getCurrentUser() instanceof IdentifiedUser;
-    final HashMap<Account.Id, ApprovalDetail> ad =
-        new HashMap<Account.Id, ApprovalDetail>();
-    for (PatchSetApproval ca : allApprovals) {
-      ApprovalDetail d = ad.get(ca.getAccountId());
-      if (d == null) {
-        d = new ApprovalDetail(ca.getAccountId());
-        d.setCanRemove(canRemoveReviewers);
-        ad.put(d.getAccount(), d);
-      }
-      if (d.canRemove()) {
-        d.setCanRemove(control.canRemoveReviewer(ca));
-      }
-      if (ca.getPatchSetId().equals(psId)) {
-        d.add(ca);
-      }
-      final ChangeControl chgCtrl =
-          changeControlGenericFactory.controlFor(detail.getChange(),
-              identifiedUserFactory.create(ca.getAccountId()));
-      for (PermissionRange pr : chgCtrl.getLabelRanges()) {
-        if (pr.getMin() != 0 || pr.getMax() != 0) {
-          d.votable(pr.getLabel());
-        }
-      }
-    }
-
-    final Account.Id owner = detail.getChange().getOwner();
-    if (ad.containsKey(owner)) {
-      // Ensure the owner always sorts to the top of the table
-      ad.get(owner).sortFirst();
-    }
-
-    aic.want(ad.keySet());
-    detail.setApprovals(ad.values());
-  }
 }