opendev/gerrit
Code Issues Proposed changes

Remove most remaining references to ApprovalCategory(Value)

Browse Source 
These classes should only be used in code directly interfacing with
the ReviewDb layer. For things like map keys, use label names or IDs
as appropriate (and document it to avoid confusion).

Change-Id: I1471725220eda63b4668d5c623ff7b984fd0a46b
This commit is contained in:
Dave Borowitz
2013-02-18 16:14:52 -08:00
parent 95ea146fc9
commit b323e616bd
12 changed files with 80 additions and 92 deletions
Download Patch File Download Diff File Expand all files Collapse all files

25
gerrit-server/src/main/java/com/google/gerrit/common/ChangeHookRunner.java
View File

@@ -14,15 +14,13 @@
package com.google.gerrit.common;
import com.google.gerrit.common.data.ContributorAgreement;
import com.google.gerrit.common.data.LabelType;
import com.google.gerrit.common.data.LabelTypes;
import com.google.gerrit.common.data.ContributorAgreement;
import com.google.gerrit.extensions.events.LifecycleListener;
import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.gerrit.lifecycle.LifecycleModule;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.ApprovalCategory;
import com.google.gerrit.reviewdb.client.ApprovalCategoryValue;
import com.google.gerrit.reviewdb.client.Branch;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchSet;
@@ -385,8 +383,8 @@ public class ChangeHookRunner implements ChangeHooks, LifecycleListener {
}
public void doCommentAddedHook(final Change change, final Account account,
final PatchSet patchSet, final String comment, final Map<ApprovalCategory.Id,
ApprovalCategoryValue.Id> approvals, final ReviewDb db) throws OrmException {
final PatchSet patchSet, final String comment, final Map<String, Short> approvals,
final ReviewDb db) throws OrmException {
final CommentAddedEvent event = new CommentAddedEvent();
event.change = eventFactory.asChangeAttribute(change);
@@ -398,7 +396,7 @@ public class ChangeHookRunner implements ChangeHooks, LifecycleListener {
if (approvals.size() > 0) {
event.approvals = new ApprovalAttribute[approvals.size()];
int i = 0;
for (Map.Entry<ApprovalCategory.Id, ApprovalCategoryValue.Id> approval : approvals.entrySet()) {
for (Map.Entry<String, Short> approval : approvals.entrySet()) {
event.approvals[i++] = getApprovalAttribute(labelTypes, approval);
}
}
@@ -415,8 +413,11 @@ public class ChangeHookRunner implements ChangeHooks, LifecycleListener {
addArg(args, "--author", getDisplayName(account));
addArg(args, "--commit", event.patchSet.revision);
addArg(args, "--comment", comment == null ? "" : comment);
for (Map.Entry<ApprovalCategory.Id, ApprovalCategoryValue.Id> approval : approvals.entrySet()) {
addArg(args, "--" + approval.getKey().get(), Short.toString(approval.getValue().get()));
for (Map.Entry<String, Short> approval : approvals.entrySet()) {
LabelType lt = labelTypes.byLabel(approval.getKey());
if (lt != null) {
addArg(args, "--" + lt.getId(), Short.toString(approval.getValue()));
}
}
runHook(change.getProject(), commentAddedHook, args);
@@ -614,14 +615,14 @@ public class ChangeHookRunner implements ChangeHooks, LifecycleListener {
* @return object suitable for serialization to JSON
*/
private ApprovalAttribute getApprovalAttribute(LabelTypes labelTypes,
Entry<ApprovalCategory.Id, ApprovalCategoryValue.Id> approval) {
Entry<String, Short> approval) {
ApprovalAttribute a = new ApprovalAttribute();
a.type = approval.getKey().get();
LabelType lt = labelTypes.byId(approval.getKey().get());
a.type = approval.getKey();
LabelType lt = labelTypes.byLabel(approval.getKey());
if (lt != null) {
a.description = lt.getName();
}
a.value = Short.toString(approval.getValue().get());
a.value = Short.toString(approval.getValue());
return a;
}

8
gerrit-server/src/main/java/com/google/gerrit/common/ChangeHooks.java
View File

@@ -17,13 +17,11 @@ package com.google.gerrit.common;
import com.google.gerrit.common.ChangeHookRunner.HookResult;
import com.google.gerrit.common.data.ContributorAgreement;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.ApprovalCategory;
import com.google.gerrit.reviewdb.client.ApprovalCategoryValue;
import com.google.gerrit.reviewdb.client.Branch;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gwtorm.server.OrmException;
@@ -65,12 +63,12 @@ public interface ChangeHooks {
* @param patchSet The patchset this comment is related to.
* @param account The gerrit user who added the comment.
* @param comment The comment given.
* @param approvals Map of Approval Categories and Scores
* @param approvals Map of label IDs to scores
* @throws OrmException
*/
public void doCommentAddedHook(Change change, Account account,
PatchSet patchSet, String comment,
Map<ApprovalCategory.Id, ApprovalCategoryValue.Id> approvals, ReviewDb db)
Map<String, Short> approvals, ReviewDb db)
throws OrmException;
/**

6
gerrit-server/src/main/java/com/google/gerrit/common/DisabledChangeHooks.java
View File

@@ -17,11 +17,9 @@ package com.google.gerrit.common;
import com.google.gerrit.common.ChangeHookRunner.HookResult;
import com.google.gerrit.common.data.ContributorAgreement;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.ApprovalCategory;
import com.google.gerrit.reviewdb.client.ApprovalCategoryValue;
import com.google.gerrit.reviewdb.client.Branch.NameKey;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.Branch.NameKey;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.IdentifiedUser;
@@ -64,7 +62,7 @@ public final class DisabledChangeHooks implements ChangeHooks {
@Override
public void doCommentAddedHook(Change change, Account account,
PatchSet patchSet, String comment,
Map<ApprovalCategory.Id, ApprovalCategoryValue.Id> approvals, ReviewDb db) {
Map<String, Short> approvals, ReviewDb db) {
}
@Override

15
gerrit-server/src/main/java/com/google/gerrit/server/ApprovalsUtil.java
View File

@@ -86,15 +86,12 @@ public class ApprovalsUtil {
List<PatchSetApproval> patchSetApprovals =
db.patchSetApprovals().byChange(dest.getParentKey()).toList();
for (PatchSetApproval a : patchSetApprovals) {
// ApprovalCategory.SUBMIT is still in db but not relevant in git-store
if (!ApprovalCategory.SUBMIT.equals(a.getCategoryId())) {
final LabelType type = labelTypes.byId(a.getCategoryId().get());
if (a.getPatchSetId().equals(source) &&
type.isCopyMinScore() &&
type.isMaxNegative(a)) {
db.patchSetApprovals().insert(
Collections.singleton(new PatchSetApproval(dest, a)));
}
LabelType type = labelTypes.byId(a.getCategoryId().get());
if (type != null && a.getPatchSetId().equals(source) &&
type.isCopyMinScore() &&
type.isMaxNegative(a)) {
db.patchSetApprovals().insert(
Collections.singleton(new PatchSetApproval(dest, a)));
}
}
return patchSetApprovals;

14
gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java
View File

@@ -29,7 +29,6 @@ import com.google.gerrit.extensions.restapi.DefaultInput;
import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.extensions.restapi.Url;
import com.google.gerrit.reviewdb.client.ApprovalCategory;
import com.google.gerrit.reviewdb.client.ApprovalCategoryValue;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.ChangeMessage;
import com.google.gerrit.reviewdb.client.Patch;
@@ -110,8 +109,7 @@ public class PostReview implements RestModifyView<RevisionResource, Input> {
private Timestamp timestamp;
private List<PatchLineComment> comments = Lists.newArrayList();
private List<String> labelDelta = Lists.newArrayList();
@Deprecated private Map<ApprovalCategory.Id, ApprovalCategoryValue.Id> categories
= Maps.newHashMap();
private Map<String, Short> categories = Maps.newHashMap();
@Inject
PostReview(ReviewDb db,
@@ -366,9 +364,7 @@ public class PostReview implements RestModifyView<RevisionResource, Input> {
c.cache(change);
upd.add(c);
labelDelta.add(format(name, c.getValue()));
categories.put(
lt.getApprovalCategoryId(),
lt.getApprovalCategoryValueId(c.getValue()));
categories.put(name, c.getValue());
} else if (c != null && c.getValue() == ent.getValue()) {
current.put(name, c);
} else if (c == null) {
@@ -381,9 +377,7 @@ public class PostReview implements RestModifyView<RevisionResource, Input> {
c.cache(change);
ins.add(c);
labelDelta.add(format(name, c.getValue()));
categories.put(
lt.getApprovalCategoryId(),
lt.getApprovalCategoryValueId(c.getValue()));
categories.put(name, c.getValue());
}
}
@@ -430,7 +424,7 @@ public class PostReview implements RestModifyView<RevisionResource, Input> {
Map<String, PatchSetApproval> current = Maps.newHashMap();
for (PatchSetApproval a : db.patchSetApprovals().byPatchSetUser(
rsrc.getPatchSet().getId(), rsrc.getAccountId())) {
if (ApprovalCategory.SUBMIT.equals(a.getCategoryId())) {
if (ApprovalCategory.SUBMIT_ID.equals(a.getCategoryId().get())) {
continue;
}

4
gerrit-server/src/main/java/com/google/gerrit/server/change/Submit.java
View File

@@ -187,7 +187,7 @@ public class Submit implements RestModifyView<RevisionResource, Input> {
new Predicate<PatchSetApproval>() {
@Override
public boolean apply(PatchSetApproval input) {
return ApprovalCategory.SUBMIT.equals(input.getCategoryId());
return ApprovalCategory.SUBMIT_ID.equals(input.getCategoryId().get());
}
}), null);
if (submit == null) {
@@ -195,7 +195,7 @@ public class Submit implements RestModifyView<RevisionResource, Input> {
new PatchSetApproval.Key(
rev.getId(),
caller.getAccountId(),
ApprovalCategory.SUBMIT),
new ApprovalCategory.Id(ApprovalCategory.SUBMIT_ID)),
(short) 1);
}
submit.setValue((short) 1);

2
gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java
View File

@@ -939,7 +939,7 @@ public class MergeOp {
}
for (PatchSetApproval a : approvals) {
if (a.getValue() > 0
&& ApprovalCategory.SUBMIT.equals(a.getCategoryId())
&& ApprovalCategory.SUBMIT_ID.equals(a.getCategoryId().get())
&& a.getPatchSetId().equals(merged)) {
if (submitter == null
|| a.getGranted().compareTo(submitter.getGranted()) > 0) {

15
gerrit-server/src/main/java/com/google/gerrit/server/git/MergeUtil.java
View File

@@ -82,10 +82,8 @@ public class MergeUtil {
private static final String R_HEADS_MASTER =
Constants.R_HEADS + Constants.MASTER;
private static final ApprovalCategory.Id CRVW = //
new ApprovalCategory.Id("CRVW");
private static final ApprovalCategory.Id VRIF = //
new ApprovalCategory.Id("VRIF");
private static final String CRVW = "CRVW";
private static final String VRIF = "VRIF";
private static final FooterKey REVIEWED_ON = new FooterKey("Reviewed-on");
private static final FooterKey CHANGE_ID = new FooterKey("Change-Id");
@@ -173,8 +171,7 @@ public class MergeUtil {
final List<PatchSetApproval> approvals =
reviewDb.patchSetApprovals().byPatchSet(c).toList();
for (PatchSetApproval a : approvals) {
if (a.getValue() > 0
&& ApprovalCategory.SUBMIT.equals(a.getCategoryId())) {
if (a.getValue() > 0 && ApprovalCategory.isSubmit(a)) {
if (submitter == null
|| a.getGranted().compareTo(submitter.getGranted()) > 0) {
submitter = a;
@@ -260,7 +257,7 @@ public class MergeUtil {
continue;
}
if (ApprovalCategory.SUBMIT.equals(a.getCategoryId())) {
if (ApprovalCategory.isSubmit(a)) {
// Submit is treated specially, below (becomes committer)
//
if (submitAudit == null
@@ -297,9 +294,9 @@ public class MergeUtil {
}
final String tag;
if (CRVW.equals(a.getCategoryId())) {
if (CRVW.equals(a.getCategoryId().get())) {
tag = "Reviewed-by";
} else if (VRIF.equals(a.getCategoryId())) {
} else if (VRIF.equals(a.getCategoryId().get())) {
tag = "Tested-by";
} else {
final LabelType lt =