From 5e9744faa1346e37d27cef348b940ba16ba27ad0 Mon Sep 17 00:00:00 2001 From: Dave Borowitz Date: Thu, 14 Feb 2013 12:22:26 -0800 Subject: [PATCH] Remove unused label details from old RPC handlers Change-Id: Ibe2ea9dc416e0e824cfacc5e5e26855b051734a2 --- .../gerrit/common/data/ApprovalDetail.java | 53 ----------- .../gerrit/common/data/ChangeDetail.java | 13 --- .../common/data/PatchSetPublishDetail.java | 58 ------------- .../rpc/changedetail/ChangeDetailFactory.java | 69 +-------------- .../PatchSetPublishDetailFactory.java | 87 ------------------- 5 files changed, 1 insertion(+), 279 deletions(-) diff --git a/gerrit-common/src/main/java/com/google/gerrit/common/data/ApprovalDetail.java b/gerrit-common/src/main/java/com/google/gerrit/common/data/ApprovalDetail.java index 57e773da60..86d70d4cad 100644 --- a/gerrit-common/src/main/java/com/google/gerrit/common/data/ApprovalDetail.java +++ b/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 SORT = - new Comparator() { - 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 approvals; protected boolean canRemove; @@ -49,8 +33,6 @@ public class ApprovalDetail { private transient Set approved; private transient Set rejected; private transient Map 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 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(); diff --git a/gerrit-common/src/main/java/com/google/gerrit/common/data/ChangeDetail.java b/gerrit-common/src/main/java/com/google/gerrit/common/data/ChangeDetail.java index 23969a0d9d..7ac21db51c 100644 --- a/gerrit-common/src/main/java/com/google/gerrit/common/data/ChangeDetail.java +++ b/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 dependsOn; protected List neededBy; protected List patchSets; - protected List approvals; protected List submitRecords; protected Project.SubmitType submitType; protected SubmitTypeRecord submitTypeRecord; @@ -191,15 +187,6 @@ public class ChangeDetail { patchSets = s; } - public List getApprovals() { - return approvals; - } - - public void setApprovals(Collection list) { - approvals = new ArrayList(list); - Collections.sort(approvals, ApprovalDetail.SORT); - } - public void setSubmitRecords(List all) { submitRecords = all; } diff --git a/gerrit-common/src/main/java/com/google/gerrit/common/data/PatchSetPublishDetail.java b/gerrit-common/src/main/java/com/google/gerrit/common/data/PatchSetPublishDetail.java index 7637f7571c..a9b6335071 100644 --- a/gerrit-common/src/main/java/com/google/gerrit/common/data/PatchSetPublishDetail.java +++ b/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 drafts; - protected List labels; - protected List approvals; protected List submitRecords; protected SubmitTypeRecord submitTypeRecord; - protected List given; protected boolean canSubmit; - public List getLabels() { - return labels; - } - - public void setLabels(List labels) { - this.labels = labels; - } - - public List getApprovals() { - return approvals; - } - - public void setApprovals(Collection list) { - approvals = new ArrayList(list); - Collections.sort(approvals, ApprovalDetail.SORT); - } - - public void setSubmitRecords(List all) { - submitRecords = all; - } - - public List getSubmitRecords() { - return submitRecords; - } - public void setSubmitTypeRecord(SubmitTypeRecord submitTypeRecord) { this.submitTypeRecord = submitTypeRecord; } @@ -69,14 +37,6 @@ public class PatchSetPublishDetail { return submitTypeRecord; } - public List getGiven() { - return given; - } - - public void setGiven(List 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; } diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/ChangeDetailFactory.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/ChangeDetailFactory.java index 5418be360a..4ace9f48d9 100644 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/ChangeDetailFactory.java +++ b/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 { 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 { private List 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 { testMerge) { ChangeUtil.testMerge(opFactory, detail.getChange()); } - - final PatchSet.Id psId = detail.getChange().currentPatchSetId(); - final List 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 ad = - new HashMap(); - 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) { diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/PatchSetPublishDetailFactory.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/PatchSetPublishDetailFactory.java index b38f6cf967..fa594cdb89 100644 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/PatchSetPublishDetailFactory.java +++ b/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 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 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 detail.setChange(change); detail.setDrafts(drafts); - List allowed = Collections.emptyList(); - List 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 rangeByName.put(r.getLabel(), r); } } - allowed = new ArrayList(); - - given = db.patchSetApprovals() // - .byPatchSetUser(patchSetId, user.getAccountId()) // - .toList(); boolean couldSubmit = false; List submitRecords = control.canSubmit(db, patchSet); @@ -148,10 +124,6 @@ final class PatchSetPublishDetailFactory extends Handler 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 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 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 ad = - new HashMap(); - 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()); - } }