Rename PatchSetApproval methods

Remove the "get" prefix on AutoValue getters. Also remove the "is" in
"isPostSubmit", which AutoValue treats the same as "get"; note how the
method on the Builder class was already required to be named postSubmit.

Change-Id: Ic2f6fe9b7195bcec9b021d65eb3ca6c9f74f0c45
This commit is contained in:
Dave Borowitz
2019-05-01 09:16:33 -07:00
parent bbbcc59d7e
commit 0e0c999911
46 changed files with 311 additions and 316 deletions

View File

@@ -98,18 +98,18 @@ public enum LabelFunction {
} }
for (PatchSetApproval a : approvals) { for (PatchSetApproval a : approvals) {
if (a.getValue() == 0) { if (a.value() == 0) {
continue; continue;
} }
if (isBlock && labelType.isMaxNegative(a)) { if (isBlock && labelType.isMaxNegative(a)) {
submitRecordLabel.appliedBy = a.getAccountId(); submitRecordLabel.appliedBy = a.accountId();
submitRecordLabel.status = SubmitRecord.Label.Status.REJECT; submitRecordLabel.status = SubmitRecord.Label.Status.REJECT;
return submitRecordLabel; return submitRecordLabel;
} }
if (labelType.isMaxPositive(a) || !requiresMaxValue) { if (labelType.isMaxPositive(a) || !requiresMaxValue) {
submitRecordLabel.appliedBy = a.getAccountId(); submitRecordLabel.appliedBy = a.accountId();
submitRecordLabel.status = SubmitRecord.Label.Status.MAY; submitRecordLabel.status = SubmitRecord.Label.Status.MAY;
if (isRequired) { if (isRequired) {

View File

@@ -155,7 +155,7 @@ public class LabelType {
} }
public boolean matches(PatchSetApproval psa) { public boolean matches(PatchSetApproval psa) {
return psa.getLabelId().get().equalsIgnoreCase(name); return psa.labelId().get().equalsIgnoreCase(name);
} }
public LabelFunction getFunction() { public LabelFunction getFunction() {
@@ -279,11 +279,11 @@ public class LabelType {
} }
public boolean isMaxNegative(PatchSetApproval ca) { public boolean isMaxNegative(PatchSetApproval ca) {
return maxNegative == ca.getValue(); return maxNegative == ca.value();
} }
public boolean isMaxPositive(PatchSetApproval ca) { public boolean isMaxPositive(PatchSetApproval ca) {
return maxPositive == ca.getValue(); return maxPositive == ca.value();
} }
public LabelValue getValue(short value) { public LabelValue getValue(short value) {
@@ -291,7 +291,7 @@ public class LabelType {
} }
public LabelValue getValue(PatchSetApproval ca) { public LabelValue getValue(PatchSetApproval ca) {
return byValue.get(ca.getValue()); return byValue.get(ca.value());
} }
public LabelId getLabelId() { public LabelId getLabelId() {

View File

@@ -48,7 +48,7 @@ public abstract class PatchSetApproval {
public abstract static class Builder { public abstract static class Builder {
public abstract Builder key(Key key); public abstract Builder key(Key key);
public abstract Key getKey(); public abstract Key key();
public abstract Builder value(short value); public abstract Builder value(short value);
@@ -68,21 +68,21 @@ public abstract class PatchSetApproval {
public abstract Builder realAccountId(Account.Id realAccountId); public abstract Builder realAccountId(Account.Id realAccountId);
abstract Optional<Account.Id> getRealAccountId(); abstract Optional<Account.Id> realAccountId();
public abstract Builder postSubmit(boolean isPostSubmit); public abstract Builder postSubmit(boolean isPostSubmit);
abstract PatchSetApproval autoBuild(); abstract PatchSetApproval autoBuild();
public PatchSetApproval build() { public PatchSetApproval build() {
if (!getRealAccountId().isPresent()) { if (!realAccountId().isPresent()) {
realAccountId(getKey().accountId()); realAccountId(key().accountId());
} }
return autoBuild(); return autoBuild();
} }
} }
public abstract Key getKey(); public abstract Key key();
/** /**
* Value assigned by the user. * Value assigned by the user.
@@ -100,40 +100,40 @@ public abstract class PatchSetApproval {
* and in the negative and positive direction a magnitude can be assumed.The further from 0 the * and in the negative and positive direction a magnitude can be assumed.The further from 0 the
* more assertive the approval. * more assertive the approval.
*/ */
public abstract short getValue(); public abstract short value();
public abstract Timestamp getGranted(); public abstract Timestamp granted();
public abstract Optional<String> getTag(); public abstract Optional<String> tag();
/** Real user that made this approval on behalf of the user recorded in {@link Key#accountId}. */ /** Real user that made this approval on behalf of the user recorded in {@link Key#accountId}. */
public abstract Account.Id getRealAccountId(); public abstract Account.Id realAccountId();
public abstract boolean isPostSubmit(); public abstract boolean postSubmit();
public abstract Builder toBuilder(); public abstract Builder toBuilder();
public PatchSetApproval copyWithPatchSet(PatchSet.Id psId) { public PatchSetApproval copyWithPatchSet(PatchSet.Id psId) {
return toBuilder().key(key(psId, getKey().accountId(), getKey().labelId())).build(); return toBuilder().key(key(psId, key().accountId(), key().labelId())).build();
} }
public PatchSet.Id getPatchSetId() { public PatchSet.Id patchSetId() {
return getKey().patchSetId(); return key().patchSetId();
} }
public Account.Id getAccountId() { public Account.Id accountId() {
return getKey().accountId(); return key().accountId();
} }
public LabelId getLabelId() { public LabelId labelId() {
return getKey().labelId(); return key().labelId();
} }
public String getLabel() { public String label() {
return getLabelId().get(); return labelId().get();
} }
public boolean isLegacySubmit() { public boolean isLegacySubmit() {
return getKey().isLegacySubmit(); return key().isLegacySubmit();
} }
} }

View File

@@ -34,18 +34,18 @@ public enum PatchSetApprovalProtoConverter
public Entities.PatchSetApproval toProto(PatchSetApproval patchSetApproval) { public Entities.PatchSetApproval toProto(PatchSetApproval patchSetApproval) {
Entities.PatchSetApproval.Builder builder = Entities.PatchSetApproval.Builder builder =
Entities.PatchSetApproval.newBuilder() Entities.PatchSetApproval.newBuilder()
.setKey(patchSetApprovalKeyProtoConverter.toProto(patchSetApproval.getKey())) .setKey(patchSetApprovalKeyProtoConverter.toProto(patchSetApproval.key()))
.setValue(patchSetApproval.getValue()) .setValue(patchSetApproval.value())
.setGranted(patchSetApproval.getGranted().getTime()) .setGranted(patchSetApproval.granted().getTime())
.setPostSubmit(patchSetApproval.isPostSubmit()); .setPostSubmit(patchSetApproval.postSubmit());
patchSetApproval.getTag().ifPresent(builder::setTag); patchSetApproval.tag().ifPresent(builder::setTag);
Account.Id realAccountId = patchSetApproval.getRealAccountId(); Account.Id realAccountId = patchSetApproval.realAccountId();
// PatchSetApproval#getRealAccountId automatically delegates to PatchSetApproval#getAccountId if // PatchSetApproval#getRealAccountId automatically delegates to PatchSetApproval#getAccountId if
// the real author is not set. However, the previous protobuf representation kept // the real author is not set. However, the previous protobuf representation kept
// 'realAccountId' empty if it wasn't set. To ensure binary compatibility, simulate the previous // 'realAccountId' empty if it wasn't set. To ensure binary compatibility, simulate the previous
// behavior. // behavior.
if (realAccountId != null && !Objects.equals(realAccountId, patchSetApproval.getAccountId())) { if (realAccountId != null && !Objects.equals(realAccountId, patchSetApproval.accountId())) {
builder.setRealAccountId(accountIdConverter.toProto(realAccountId)); builder.setRealAccountId(accountIdConverter.toProto(realAccountId));
} }

View File

@@ -104,13 +104,13 @@ public class ApprovalCopier {
Table<String, Account.Id, PatchSetApproval> wontCopy = HashBasedTable.create(); Table<String, Account.Id, PatchSetApproval> wontCopy = HashBasedTable.create();
for (PatchSetApproval psa : dontCopy) { for (PatchSetApproval psa : dontCopy) {
wontCopy.put(psa.getLabel(), psa.getAccountId(), psa); wontCopy.put(psa.label(), psa.accountId(), psa);
} }
Table<String, Account.Id, PatchSetApproval> byUser = HashBasedTable.create(); Table<String, Account.Id, PatchSetApproval> byUser = HashBasedTable.create();
for (PatchSetApproval psa : all.get(ps.id())) { for (PatchSetApproval psa : all.get(ps.id())) {
if (!wontCopy.contains(psa.getLabel(), psa.getAccountId())) { if (!wontCopy.contains(psa.label(), psa.accountId())) {
byUser.put(psa.getLabel(), psa.getAccountId(), psa); byUser.put(psa.label(), psa.accountId(), psa);
} }
} }
@@ -130,17 +130,17 @@ public class ApprovalCopier {
project.getNameKey(), rw, repoConfig, priorPs.commitId(), ps.commitId()); project.getNameKey(), rw, repoConfig, priorPs.commitId(), ps.commitId());
for (PatchSetApproval psa : priorApprovals) { for (PatchSetApproval psa : priorApprovals) {
if (wontCopy.contains(psa.getLabel(), psa.getAccountId())) { if (wontCopy.contains(psa.label(), psa.accountId())) {
continue; continue;
} }
if (byUser.contains(psa.getLabel(), psa.getAccountId())) { if (byUser.contains(psa.label(), psa.accountId())) {
continue; continue;
} }
if (!canCopy(project, psa, ps.id(), kind)) { if (!canCopy(project, psa, ps.id(), kind)) {
wontCopy.put(psa.getLabel(), psa.getAccountId(), psa); wontCopy.put(psa.label(), psa.accountId(), psa);
continue; continue;
} }
byUser.put(psa.getLabel(), psa.getAccountId(), psa.copyWithPatchSet(ps.id())); byUser.put(psa.label(), psa.accountId(), psa.copyWithPatchSet(ps.id()));
} }
} }
return labelNormalizer.normalize(notes, byUser.values()).getNormalized(); return labelNormalizer.normalize(notes, byUser.values()).getNormalized();
@@ -160,9 +160,9 @@ public class ApprovalCopier {
private static boolean canCopy( private static boolean canCopy(
ProjectState project, PatchSetApproval psa, PatchSet.Id psId, ChangeKind kind) { ProjectState project, PatchSetApproval psa, PatchSet.Id psId, ChangeKind kind) {
int n = psa.getKey().patchSetId().get(); int n = psa.key().patchSetId().get();
checkArgument(n != psId.get()); checkArgument(n != psId.get());
LabelType type = project.getLabelTypes().byLabel(psa.getLabelId()); LabelType type = project.getLabelTypes().byLabel(psa.labelId());
if (type == null) { if (type == null) {
return false; return false;
} else if ((type.isCopyMinScore() && type.isMaxNegative(psa)) } else if ((type.isCopyMinScore() && type.isMaxNegative(psa))

View File

@@ -88,7 +88,7 @@ public class ApprovalsUtil {
private static Iterable<PatchSetApproval> filterApprovals( private static Iterable<PatchSetApproval> filterApprovals(
Iterable<PatchSetApproval> psas, Account.Id accountId) { Iterable<PatchSetApproval> psas, Account.Id accountId) {
return Iterables.filter(psas, a -> Objects.equals(a.getAccountId(), accountId)); return Iterables.filter(psas, a -> Objects.equals(a.accountId(), accountId));
} }
private final ApprovalCopier copier; private final ApprovalCopier copier;
@@ -291,7 +291,7 @@ public class ApprovalsUtil {
cells.add(newApproval(ps.id(), user, lt.getLabelId(), vote.getValue(), ts).build()); cells.add(newApproval(ps.id(), user, lt.getLabelId(), vote.getValue(), ts).build());
} }
for (PatchSetApproval psa : cells) { for (PatchSetApproval psa : cells) {
update.putApproval(psa.getLabel(), psa.getValue()); update.putApproval(psa.label(), psa.value());
} }
return cells; return cells;
} }
@@ -359,8 +359,8 @@ public class ApprovalsUtil {
} }
PatchSetApproval submitter = null; PatchSetApproval submitter = null;
for (PatchSetApproval a : approvals) { for (PatchSetApproval a : approvals) {
if (a.getPatchSetId().equals(c) && a.getValue() > 0 && a.isLegacySubmit()) { if (a.patchSetId().equals(c) && a.value() > 0 && a.isLegacySubmit()) {
if (submitter == null || a.getGranted().compareTo(submitter.getGranted()) > 0) { if (submitter == null || a.granted().compareTo(submitter.granted()) > 0) {
submitter = a; submitter = a;
} }
} }
@@ -374,7 +374,7 @@ public class ApprovalsUtil {
if (!n.isEmpty()) { if (!n.isEmpty()) {
boolean first = true; boolean first = true;
for (Map.Entry<String, Short> e : n.entrySet()) { for (Map.Entry<String, Short> e : n.entrySet()) {
if (c.containsKey(e.getKey()) && c.get(e.getKey()).getValue() == e.getValue()) { if (c.containsKey(e.getKey()) && c.get(e.getKey()).value() == e.getValue()) {
continue; continue;
} }
if (first) { if (first) {

View File

@@ -152,10 +152,8 @@ public class PatchSetUtil {
ApprovalsUtil approvalsUtil = approvalsUtilProvider.get(); ApprovalsUtil approvalsUtil = approvalsUtilProvider.get();
for (PatchSetApproval ap : for (PatchSetApproval ap :
approvalsUtil.byPatchSet(notes, change.currentPatchSetId(), null, null)) { approvalsUtil.byPatchSet(notes, change.currentPatchSetId(), null, null)) {
LabelType type = projectState.getLabelTypes(notes).byLabel(ap.getLabel()); LabelType type = projectState.getLabelTypes(notes).byLabel(ap.label());
if (type != null if (type != null && ap.value() == 1 && type.getFunction() == LabelFunction.PATCH_SET_LOCK) {
&& ap.getValue() == 1
&& type.getFunction() == LabelFunction.PATCH_SET_LOCK) {
return true; return true;
} }
} }

View File

@@ -44,14 +44,14 @@ public class ReviewerSet {
first = psa; first = psa;
} else { } else {
checkArgument( checkArgument(
first.getKey().patchSetId().changeId().equals(psa.getKey().patchSetId().changeId()), first.key().patchSetId().changeId().equals(psa.key().patchSetId().changeId()),
"multiple change IDs: %s, %s", "multiple change IDs: %s, %s",
first.getKey(), first.key(),
psa.getKey()); psa.key());
} }
Account.Id id = psa.getAccountId(); Account.Id id = psa.accountId();
reviewers.put(REVIEWER, id, psa.getGranted()); reviewers.put(REVIEWER, id, psa.granted());
if (psa.getValue() != 0) { if (psa.value() != 0) {
reviewers.remove(CC, id); reviewers.remove(CC, id);
} }
} }

View File

@@ -593,14 +593,14 @@ class RevisionApiImpl implements RevisionApi {
EnumSet.of( EnumSet.of(
FillOptions.ID, FillOptions.NAME, FillOptions.EMAIL, FillOptions.USERNAME)); FillOptions.ID, FillOptions.NAME, FillOptions.EMAIL, FillOptions.USERNAME));
for (PatchSetApproval approval : approvals) { for (PatchSetApproval approval : approvals) {
String label = approval.getLabel(); String label = approval.label();
ApprovalInfo info = ApprovalInfo info =
new ApprovalInfo( new ApprovalInfo(
approval.getAccountId().get(), approval.accountId().get(),
Integer.valueOf(approval.getValue()), Integer.valueOf(approval.value()),
null, null,
approval.getTag().orElse(null), approval.tag().orElse(null),
approval.getGranted()); approval.granted());
accountLoader.put(info); accountLoader.put(info);
result.get(label).add(info); result.get(label).add(info);
} }

View File

@@ -240,7 +240,7 @@ public class AddReviewersOp implements BatchUpdateOp {
addReviewersEmail.emailReviewers( addReviewersEmail.emailReviewers(
ctx.getUser().asIdentifiedUser(), ctx.getUser().asIdentifiedUser(),
change, change,
Lists.transform(addedReviewers, PatchSetApproval::getAccountId), Lists.transform(addedReviewers, PatchSetApproval::accountId),
addedCCs, addedCCs,
addedReviewersByEmail, addedReviewersByEmail,
addedCCsByEmail, addedCCsByEmail,
@@ -249,7 +249,7 @@ public class AddReviewersOp implements BatchUpdateOp {
if (!addedReviewers.isEmpty()) { if (!addedReviewers.isEmpty()) {
List<AccountState> reviewers = List<AccountState> reviewers =
addedReviewers.stream() addedReviewers.stream()
.map(r -> accountCache.get(r.getAccountId())) .map(r -> accountCache.get(r.accountId()))
.flatMap(Streams::stream) .flatMap(Streams::stream)
.collect(toList()); .collect(toList());
reviewerAdded.fire(change, patchSet, reviewers, ctx.getAccount(), ctx.getWhen()); reviewerAdded.fire(change, patchSet, reviewers, ctx.getAccount(), ctx.getWhen());

View File

@@ -453,7 +453,7 @@ public class ChangeInserter implements InsertChangeOp {
cm.setNotify(notify); cm.setNotify(notify);
cm.addReviewers( cm.addReviewers(
reviewerAdditions.flattenResults(AddReviewersOp.Result::addedReviewers).stream() reviewerAdditions.flattenResults(AddReviewersOp.Result::addedReviewers).stream()
.map(PatchSetApproval::getAccountId) .map(PatchSetApproval::accountId)
.collect(toImmutableSet())); .collect(toImmutableSet()));
cm.addReviewersByEmail( cm.addReviewersByEmail(
reviewerAdditions.flattenResults(AddReviewersOp.Result::addedReviewersByEmail)); reviewerAdditions.flattenResults(AddReviewersOp.Result::addedReviewersByEmail));

View File

@@ -656,8 +656,8 @@ public class ChangeJson {
if (!s.isPresent()) { if (!s.isPresent()) {
return; return;
} }
out.submitted = s.get().getGranted(); out.submitted = s.get().granted();
out.submitter = accountLoader.get(s.get().getAccountId()); out.submitter = accountLoader.get(s.get().accountId());
} }
private Collection<ChangeMessageInfo> messages(ChangeData cd) { private Collection<ChangeMessageInfo> messages(ChangeData cd) {

View File

@@ -134,14 +134,14 @@ public class DeleteReviewerOp implements BatchUpdateOp {
// Check if removing this vote is OK // Check if removing this vote is OK
removeReviewerControl.checkRemoveReviewer(ctx.getNotes(), ctx.getUser(), a); removeReviewerControl.checkRemoveReviewer(ctx.getNotes(), ctx.getUser(), a);
del.add(a); del.add(a);
if (a.getPatchSetId().equals(currPs.id()) && a.getValue() != 0) { if (a.patchSetId().equals(currPs.id()) && a.value() != 0) {
oldApprovals.put(a.getLabel(), a.getValue()); oldApprovals.put(a.label(), a.value());
removedVotesMsg removedVotesMsg
.append("* ") .append("* ")
.append(a.getLabel()) .append(a.label())
.append(formatLabelValue(a.getValue())) .append(formatLabelValue(a.value()))
.append(" by ") .append(" by ")
.append(userFactory.create(a.getAccountId()).getNameEmail()) .append(userFactory.create(a.accountId()).getNameEmail())
.append("\n"); .append("\n");
votesRemoved = true; votesRemoved = true;
} }
@@ -195,7 +195,7 @@ public class DeleteReviewerOp implements BatchUpdateOp {
private Iterable<PatchSetApproval> approvals(ChangeContext ctx, Account.Id accountId) { private Iterable<PatchSetApproval> approvals(ChangeContext ctx, Account.Id accountId) {
Iterable<PatchSetApproval> approvals; Iterable<PatchSetApproval> approvals;
approvals = approvalsUtil.byChange(ctx.getNotes()).values(); approvals = approvalsUtil.byChange(ctx.getNotes()).values();
return Iterables.filter(approvals, psa -> accountId.equals(psa.getAccountId())); return Iterables.filter(approvals, psa -> accountId.equals(psa.accountId()));
} }
private String formatLabelValue(short value) { private String formatLabelValue(short value) {

View File

@@ -88,23 +88,23 @@ public class LabelNormalizer {
List<PatchSetApproval> deleted = Lists.newArrayListWithCapacity(approvals.size()); List<PatchSetApproval> deleted = Lists.newArrayListWithCapacity(approvals.size());
LabelTypes labelTypes = projectCache.checkedGet(notes.getProjectName()).getLabelTypes(notes); LabelTypes labelTypes = projectCache.checkedGet(notes.getProjectName()).getLabelTypes(notes);
for (PatchSetApproval psa : approvals) { for (PatchSetApproval psa : approvals) {
Change.Id changeId = psa.getKey().patchSetId().changeId(); Change.Id changeId = psa.key().patchSetId().changeId();
checkArgument( checkArgument(
changeId.equals(notes.getChangeId()), changeId.equals(notes.getChangeId()),
"Approval %s does not match change %s", "Approval %s does not match change %s",
psa.getKey(), psa.key(),
notes.getChange().getKey()); notes.getChange().getKey());
if (psa.isLegacySubmit()) { if (psa.isLegacySubmit()) {
unchanged.add(psa); unchanged.add(psa);
continue; continue;
} }
LabelType label = labelTypes.byLabel(psa.getLabelId()); LabelType label = labelTypes.byLabel(psa.labelId());
if (label == null) { if (label == null) {
deleted.add(psa); deleted.add(psa);
continue; continue;
} }
PatchSetApproval copy = applyTypeFloor(label, psa); PatchSetApproval copy = applyTypeFloor(label, psa);
if (copy.getValue() != psa.getValue()) { if (copy.value() != psa.value()) {
updated.add(copy); updated.add(copy);
} else { } else {
unchanged.add(psa); unchanged.add(psa);
@@ -116,11 +116,11 @@ public class LabelNormalizer {
private PatchSetApproval applyTypeFloor(LabelType lt, PatchSetApproval a) { private PatchSetApproval applyTypeFloor(LabelType lt, PatchSetApproval a) {
PatchSetApproval.Builder b = a.toBuilder(); PatchSetApproval.Builder b = a.toBuilder();
LabelValue atMin = lt.getMin(); LabelValue atMin = lt.getMin();
if (atMin != null && a.getValue() < atMin.getValue()) { if (atMin != null && a.value() < atMin.getValue()) {
b.value(atMin.getValue()); b.value(atMin.getValue());
} }
LabelValue atMax = lt.getMax(); LabelValue atMax = lt.getMax();
if (atMax != null && a.getValue() > atMax.getValue()) { if (atMax != null && a.value() > atMax.getValue()) {
b.value(atMax.getValue()); b.value(atMax.getValue());
} }
return b.build(); return b.build();

View File

@@ -206,8 +206,8 @@ public class LabelsJson {
if (standard) { if (standard) {
for (PatchSetApproval psa : cd.currentApprovals()) { for (PatchSetApproval psa : cd.currentApprovals()) {
if (type.matches(psa)) { if (type.matches(psa)) {
short val = psa.getValue(); short val = psa.value();
Account.Id accountId = psa.getAccountId(); Account.Id accountId = psa.accountId();
setLabelScores(accountLoader, type, e.getValue(), val, accountId); setLabelScores(accountLoader, type, e.getValue(), val, accountId);
} }
} }
@@ -260,7 +260,7 @@ public class LabelsJson {
accountId, accountId,
null, null,
null)) { null)) {
result.put(psa.getLabel(), psa.getValue()); result.put(psa.label(), psa.value());
} }
return result; return result;
} }
@@ -279,7 +279,7 @@ public class LabelsJson {
// we aren't including 0 votes for all users below, so we can just look at // we aren't including 0 votes for all users below, so we can just look at
// the latest patch set (in the next loop). // the latest patch set (in the next loop).
for (PatchSetApproval psa : cd.approvals().values()) { for (PatchSetApproval psa : cd.approvals().values()) {
allUsers.add(psa.getAccountId()); allUsers.add(psa.accountId());
} }
} }
@@ -287,13 +287,13 @@ public class LabelsJson {
SetMultimap<Account.Id, PatchSetApproval> current = SetMultimap<Account.Id, PatchSetApproval> current =
MultimapBuilder.hashKeys().hashSetValues().build(); MultimapBuilder.hashKeys().hashSetValues().build();
for (PatchSetApproval a : cd.currentApprovals()) { for (PatchSetApproval a : cd.currentApprovals()) {
allUsers.add(a.getAccountId()); allUsers.add(a.accountId());
LabelType type = labelTypes.byLabel(a.getLabelId()); LabelType type = labelTypes.byLabel(a.labelId());
if (type != null) { if (type != null) {
labelNames.add(type.getName()); labelNames.add(type.getName());
// Not worth the effort to distinguish between votable/non-votable for 0 // Not worth the effort to distinguish between votable/non-votable for 0
// values on closed changes, since they can't vote anyway. // values on closed changes, since they can't vote anyway.
current.put(a.getAccountId(), a); current.put(a.accountId(), a);
} }
} }
@@ -335,19 +335,19 @@ public class LabelsJson {
} }
} }
for (PatchSetApproval psa : current.get(accountId)) { for (PatchSetApproval psa : current.get(accountId)) {
LabelType type = labelTypes.byLabel(psa.getLabelId()); LabelType type = labelTypes.byLabel(psa.labelId());
if (type == null) { if (type == null) {
continue; continue;
} }
short val = psa.getValue(); short val = psa.value();
ApprovalInfo info = byLabel.get(type.getName()); ApprovalInfo info = byLabel.get(type.getName());
if (info != null) { if (info != null) {
info.value = Integer.valueOf(val); info.value = Integer.valueOf(val);
info.permittedVotingRange = pvr.getOrDefault(type.getName(), null); info.permittedVotingRange = pvr.getOrDefault(type.getName(), null);
info.date = psa.getGranted(); info.date = psa.granted();
info.tag = psa.getTag().orElse(null); info.tag = psa.tag().orElse(null);
if (psa.isPostSubmit()) { if (psa.postSubmit()) {
info.postSubmit = true; info.postSubmit = true;
} }
} }
@@ -441,13 +441,13 @@ public class LabelsJson {
Set<Account.Id> allUsers = new HashSet<>(); Set<Account.Id> allUsers = new HashSet<>();
allUsers.addAll(cd.reviewers().byState(ReviewerStateInternal.REVIEWER)); allUsers.addAll(cd.reviewers().byState(ReviewerStateInternal.REVIEWER));
for (PatchSetApproval psa : cd.approvals().values()) { for (PatchSetApproval psa : cd.approvals().values()) {
allUsers.add(psa.getAccountId()); allUsers.add(psa.accountId());
} }
Table<Account.Id, String, PatchSetApproval> current = Table<Account.Id, String, PatchSetApproval> current =
HashBasedTable.create(allUsers.size(), cd.getLabelTypes().getLabelTypes().size()); HashBasedTable.create(allUsers.size(), cd.getLabelTypes().getLabelTypes().size());
for (PatchSetApproval psa : cd.currentApprovals()) { for (PatchSetApproval psa : cd.currentApprovals()) {
current.put(psa.getAccountId(), psa.getLabel(), psa); current.put(psa.accountId(), psa.label(), psa);
} }
LabelTypes labelTypes = cd.getLabelTypes(); LabelTypes labelTypes = cd.getLabelTypes();
@@ -467,16 +467,16 @@ public class LabelsJson {
Timestamp date = null; Timestamp date = null;
PatchSetApproval psa = current.get(accountId, lt.getName()); PatchSetApproval psa = current.get(accountId, lt.getName());
if (psa != null) { if (psa != null) {
value = Integer.valueOf(psa.getValue()); value = Integer.valueOf(psa.value());
if (value == 0) { if (value == 0) {
// This may be a dummy approval that was inserted when the reviewer // This may be a dummy approval that was inserted when the reviewer
// was added. Explicitly check whether the user can vote on this // was added. Explicitly check whether the user can vote on this
// label. // label.
value = perm.test(new LabelPermission(lt)) ? 0 : null; value = perm.test(new LabelPermission(lt)) ? 0 : null;
} }
tag = psa.getTag().orElse(null); tag = psa.tag().orElse(null);
date = psa.getGranted(); date = psa.granted();
if (psa.isPostSubmit()) { if (psa.postSubmit()) {
logger.atWarning().log("unexpected post-submit approval on open change: %s", psa); logger.atWarning().log("unexpected post-submit approval on open change: %s", psa);
} }
} else { } else {

View File

@@ -469,8 +469,8 @@ public class ReviewerAdder {
// New reviewers have value 0, don't bother normalizing. // New reviewers have value 0, don't bother normalizing.
result.reviewers.add( result.reviewers.add(
json.format( json.format(
new ReviewerInfo(psa.getAccountId().get()), new ReviewerInfo(psa.accountId().get()),
psa.getAccountId(), psa.accountId(),
cd, cd,
ImmutableList.of(psa))); ImmutableList.of(psa)));
} }

View File

@@ -111,9 +111,9 @@ public class ReviewerJson {
out.approvals = new TreeMap<>(labelTypes.nameComparator()); out.approvals = new TreeMap<>(labelTypes.nameComparator());
for (PatchSetApproval ca : approvals) { for (PatchSetApproval ca : approvals) {
LabelType at = labelTypes.byLabel(ca.getLabelId()); LabelType at = labelTypes.byLabel(ca.labelId());
if (at != null) { if (at != null) {
out.approvals.put(at.getName(), formatValue(ca.getValue())); out.approvals.put(at.getName(), formatValue(ca.value()));
} }
} }

View File

@@ -540,7 +540,7 @@ public class EventFactory {
if (!list.isEmpty()) { if (!list.isEmpty()) {
p.approvals = new ArrayList<>(list.size()); p.approvals = new ArrayList<>(list.size());
for (PatchSetApproval a : list) { for (PatchSetApproval a : list) {
if (a.getValue() != 0) { if (a.value() != 0) {
p.approvals.add(asApprovalAttribute(a, labelTypes)); p.approvals.add(asApprovalAttribute(a, labelTypes));
} }
} }
@@ -599,13 +599,13 @@ public class EventFactory {
*/ */
public ApprovalAttribute asApprovalAttribute(PatchSetApproval approval, LabelTypes labelTypes) { public ApprovalAttribute asApprovalAttribute(PatchSetApproval approval, LabelTypes labelTypes) {
ApprovalAttribute a = new ApprovalAttribute(); ApprovalAttribute a = new ApprovalAttribute();
a.type = approval.getLabelId().get(); a.type = approval.labelId().get();
a.value = Short.toString(approval.getValue()); a.value = Short.toString(approval.value());
a.by = asAccountAttribute(approval.getAccountId()); a.by = asAccountAttribute(approval.accountId());
a.grantedOn = approval.getGranted().getTime() / 1000L; a.grantedOn = approval.granted().getTime() / 1000L;
a.oldValue = null; a.oldValue = null;
LabelType lt = labelTypes.byLabel(approval.getLabelId()); LabelType lt = labelTypes.byLabel(approval.labelId());
if (lt != null) { if (lt != null) {
a.description = lt.getName(); a.description = lt.getName();
} }

View File

@@ -522,7 +522,7 @@ public class MergeUtil {
PatchSetApproval submitAudit = null; PatchSetApproval submitAudit = null;
for (PatchSetApproval a : safeGetApprovals(notes, psId)) { for (PatchSetApproval a : safeGetApprovals(notes, psId)) {
if (a.getValue() <= 0) { if (a.value() <= 0) {
// Negative votes aren't counted. // Negative votes aren't counted.
continue; continue;
} }
@@ -530,13 +530,13 @@ public class MergeUtil {
if (a.isLegacySubmit()) { if (a.isLegacySubmit()) {
// Submit is treated specially, below (becomes committer) // Submit is treated specially, below (becomes committer)
// //
if (submitAudit == null || a.getGranted().compareTo(submitAudit.getGranted()) > 0) { if (submitAudit == null || a.granted().compareTo(submitAudit.granted()) > 0) {
submitAudit = a; submitAudit = a;
} }
continue; continue;
} }
final Account acc = identifiedUserFactory.create(a.getAccountId()).getAccount(); final Account acc = identifiedUserFactory.create(a.accountId()).getAccount();
final StringBuilder identbuf = new StringBuilder(); final StringBuilder identbuf = new StringBuilder();
if (acc.getFullName() != null && acc.getFullName().length() > 0) { if (acc.getFullName() != null && acc.getFullName().length() > 0) {
if (identbuf.length() > 0) { if (identbuf.length() > 0) {
@@ -561,12 +561,12 @@ public class MergeUtil {
} }
final String tag; final String tag;
if (isCodeReview(a.getLabelId())) { if (isCodeReview(a.labelId())) {
tag = "Reviewed-by"; tag = "Reviewed-by";
} else if (isVerified(a.getLabelId())) { } else if (isVerified(a.labelId())) {
tag = "Tested-by"; tag = "Tested-by";
} else { } else {
final LabelType lt = project.getLabelTypes().byLabel(a.getLabelId()); final LabelType lt = project.getLabelTypes().byLabel(a.labelId());
if (lt == null) { if (lt == null) {
continue; continue;
} }

View File

@@ -454,7 +454,7 @@ public class ReplaceOp implements BatchUpdateOp {
continue; continue;
} }
LabelType lt = projectState.getLabelTypes().byLabel(a.getLabelId()); LabelType lt = projectState.getLabelTypes().byLabel(a.labelId());
if (lt != null) { if (lt != null) {
current.put(lt.getName(), a); current.put(lt.getName(), a);
} }
@@ -551,7 +551,7 @@ public class ReplaceOp implements BatchUpdateOp {
Streams.concat( Streams.concat(
oldRecipients.getReviewers().stream(), oldRecipients.getReviewers().stream(),
reviewerAdditions.flattenResults(AddReviewersOp.Result::addedReviewers).stream() reviewerAdditions.flattenResults(AddReviewersOp.Result::addedReviewers).stream()
.map(PatchSetApproval::getAccountId)) .map(PatchSetApproval::accountId))
.collect(toImmutableSet())); .collect(toImmutableSet()));
cm.addExtraCC( cm.addExtraCC(
Streams.concat( Streams.concat(

View File

@@ -467,13 +467,12 @@ public class ChangeField {
Set<String> allApprovals = new HashSet<>(); Set<String> allApprovals = new HashSet<>();
Set<String> distinctApprovals = new HashSet<>(); Set<String> distinctApprovals = new HashSet<>();
for (PatchSetApproval a : cd.currentApprovals()) { for (PatchSetApproval a : cd.currentApprovals()) {
if (a.getValue() != 0 && !a.isLegacySubmit()) { if (a.value() != 0 && !a.isLegacySubmit()) {
allApprovals.add(formatLabel(a.getLabel(), a.getValue(), a.getAccountId())); allApprovals.add(formatLabel(a.label(), a.value(), a.accountId()));
if (owners && cd.change().getOwner().equals(a.getAccountId())) { if (owners && cd.change().getOwner().equals(a.accountId())) {
allApprovals.add( allApprovals.add(formatLabel(a.label(), a.value(), ChangeQueryBuilder.OWNER_ACCOUNT_ID));
formatLabel(a.getLabel(), a.getValue(), ChangeQueryBuilder.OWNER_ACCOUNT_ID));
} }
distinctApprovals.add(formatLabel(a.getLabel(), a.getValue())); distinctApprovals.add(formatLabel(a.label(), a.value()));
} }
} }
allApprovals.addAll(distinctApprovals); allApprovals.addAll(distinctApprovals);

View File

@@ -330,7 +330,7 @@ public class MailProcessor {
approvalsUtil approvalsUtil
.byPatchSetUser( .byPatchSetUser(
notes, psId, ctx.getAccountId(), ctx.getRevWalk(), ctx.getRepoView().getConfig()) notes, psId, ctx.getAccountId(), ctx.getRevWalk(), ctx.getRepoView().getConfig())
.forEach(a -> approvals.put(a.getLabel(), a.getValue())); .forEach(a -> approvals.put(a.label(), a.value()));
// Fire Gerrit event. Note that approvals can't be granted via email, so old and new approvals // Fire Gerrit event. Note that approvals can't be granted via email, so old and new approvals
// are always the same here. // are always the same here.
commentAdded.fire( commentAdded.fire(

View File

@@ -70,14 +70,14 @@ public class MergedSender extends ReplyToChangeSender {
Table<Account.Id, String, PatchSetApproval> neg = HashBasedTable.create(); Table<Account.Id, String, PatchSetApproval> neg = HashBasedTable.create();
for (PatchSetApproval ca : for (PatchSetApproval ca :
args.approvalsUtil.byPatchSet(changeData.notes(), patchSet.id(), null, null)) { args.approvalsUtil.byPatchSet(changeData.notes(), patchSet.id(), null, null)) {
LabelType lt = labelTypes.byLabel(ca.getLabelId()); LabelType lt = labelTypes.byLabel(ca.labelId());
if (lt == null) { if (lt == null) {
continue; continue;
} }
if (ca.getValue() > 0) { if (ca.value() > 0) {
pos.put(ca.getAccountId(), lt.getName(), ca); pos.put(ca.accountId(), lt.getName(), ca);
} else if (ca.getValue() < 0) { } else if (ca.value() < 0) {
neg.put(ca.getAccountId(), lt.getName(), ca); neg.put(ca.accountId(), lt.getName(), ca);
} }
} }
@@ -117,7 +117,7 @@ public class MergedSender extends ReplyToChangeSender {
} else { } else {
txt.append(lt.getName()); txt.append(lt.getName());
txt.append('='); txt.append('=');
txt.append(LabelValue.formatValue(ca.getValue())); txt.append(LabelValue.formatValue(ca.value()));
} }
} }
txt.append('\n'); txt.append('\n');

View File

@@ -76,7 +76,7 @@ public class ChangeNotes extends AbstractChangeNotes<ChangeNotes> {
private static final FluentLogger logger = FluentLogger.forEnclosingClass(); private static final FluentLogger logger = FluentLogger.forEnclosingClass();
static final Ordering<PatchSetApproval> PSA_BY_TIME = static final Ordering<PatchSetApproval> PSA_BY_TIME =
Ordering.from(comparing(PatchSetApproval::getGranted)); Ordering.from(comparing(PatchSetApproval::granted));
public static final Ordering<ChangeMessage> MESSAGE_BY_TIME = public static final Ordering<ChangeMessage> MESSAGE_BY_TIME =
Ordering.from(comparing(ChangeMessage::getWrittenOn)); Ordering.from(comparing(ChangeMessage::getWrittenOn));

View File

@@ -281,13 +281,13 @@ class ChangeNotesParser {
ListMultimap<PatchSet.Id, PatchSetApproval> result = ListMultimap<PatchSet.Id, PatchSetApproval> result =
MultimapBuilder.hashKeys().arrayListValues().build(); MultimapBuilder.hashKeys().arrayListValues().build();
for (PatchSetApproval.Builder a : approvals.values()) { for (PatchSetApproval.Builder a : approvals.values()) {
if (!patchSetCommitParsed(a.getKey().patchSetId())) { if (!patchSetCommitParsed(a.key().patchSetId())) {
continue; // Patch set deleted or missing. continue; // Patch set deleted or missing.
} else if (allPastReviewers.contains(a.getKey().accountId()) } else if (allPastReviewers.contains(a.key().accountId())
&& !reviewers.containsRow(a.getKey().accountId())) { && !reviewers.containsRow(a.key().accountId())) {
continue; // Reviewer was explicitly removed. continue; // Reviewer was explicitly removed.
} }
result.put(a.getKey().patchSetId(), a.build()); result.put(a.key().patchSetId(), a.build());
} }
result.keySet().forEach(k -> result.get(k).sort(ChangeNotes.PSA_BY_TIME)); result.keySet().forEach(k -> result.get(k).sort(ChangeNotes.PSA_BY_TIME));
return result; return result;
@@ -613,7 +613,7 @@ class ChangeNotesParser {
// up sorted after the submit during rebuilding. // up sorted after the submit during rebuilding.
if (status == Change.Status.MERGED) { if (status == Change.Status.MERGED) {
for (PatchSetApproval.Builder psa : bufferedApprovals) { for (PatchSetApproval.Builder psa : bufferedApprovals) {
if (!psa.getKey().isLegacySubmit()) { if (!psa.key().isLegacySubmit()) {
psa.postSubmit(true); psa.postSubmit(true);
} }
} }
@@ -798,7 +798,7 @@ class ChangeNotesParser {
if (!Objects.equals(realAccountId, committerId)) { if (!Objects.equals(realAccountId, committerId)) {
psa.realAccountId(realAccountId); psa.realAccountId(realAccountId);
} }
approvals.putIfAbsent(psa.getKey(), psa); approvals.putIfAbsent(psa.key(), psa);
return psa; return psa;
} }
@@ -837,7 +837,7 @@ class ChangeNotesParser {
if (!Objects.equals(realAccountId, committerId)) { if (!Objects.equals(realAccountId, committerId)) {
remove.realAccountId(realAccountId); remove.realAccountId(realAccountId);
} }
approvals.putIfAbsent(remove.getKey(), remove); approvals.putIfAbsent(remove.key(), remove);
return remove; return remove;
} }
@@ -1023,7 +1023,7 @@ class ChangeNotesParser {
comments.values(), c -> PatchSet.id(id, c.key.patchSetId), missing); comments.values(), c -> PatchSet.id(id, c.key.patchSetId), missing);
pruned += pruned +=
pruneEntitiesForMissingPatchSets( pruneEntitiesForMissingPatchSets(
approvals.values(), psa -> psa.getKey().patchSetId(), missing); approvals.values(), psa -> psa.key().patchSetId(), missing);
if (!missing.isEmpty()) { if (!missing.isEmpty()) {
logger.atWarning().log( logger.atWarning().log(

View File

@@ -560,7 +560,7 @@ public abstract class ChangeNotesState {
.approvals( .approvals(
proto.getApprovalList().stream() proto.getApprovalList().stream()
.map(bytes -> parseProtoFrom(PatchSetApprovalProtoConverter.INSTANCE, bytes)) .map(bytes -> parseProtoFrom(PatchSetApprovalProtoConverter.INSTANCE, bytes))
.map(a -> Maps.immutableEntry(a.getPatchSetId(), a)) .map(a -> Maps.immutableEntry(a.patchSetId(), a))
.collect(toImmutableList())) .collect(toImmutableList()))
.reviewers(toReviewerSet(proto.getReviewerList())) .reviewers(toReviewerSet(proto.getReviewerList()))
.reviewersByEmail(toReviewerByEmailSet(proto.getReviewerByEmailList())) .reviewersByEmail(toReviewerByEmailSet(proto.getReviewerByEmailList()))

View File

@@ -47,7 +47,7 @@ public class RemoveReviewerControl {
public void checkRemoveReviewer( public void checkRemoveReviewer(
ChangeNotes notes, CurrentUser currentUser, PatchSetApproval approval) ChangeNotes notes, CurrentUser currentUser, PatchSetApproval approval)
throws PermissionBackendException, AuthException { throws PermissionBackendException, AuthException {
checkRemoveReviewer(notes, currentUser, approval.getAccountId(), approval.getValue()); checkRemoveReviewer(notes, currentUser, approval.accountId(), approval.value());
} }
/** /**

View File

@@ -76,7 +76,7 @@ public class EqualsLabelPredicate extends ChangeIndexPredicate {
for (PatchSetApproval p : object.currentApprovals()) { for (PatchSetApproval p : object.currentApprovals()) {
if (labelType.matches(p)) { if (labelType.matches(p)) {
hasVote = true; hasVote = true;
if (match(object, p.getValue(), p.getAccountId())) { if (match(object, p.value(), p.accountId())) {
return true; return true;
} }
} }

View File

@@ -175,11 +175,11 @@ public class DeleteVote extends RetryingRestModifyView<VoteResource, DeleteVoteI
for (PatchSetApproval a : for (PatchSetApproval a :
approvalsUtil.byPatchSetUser( approvalsUtil.byPatchSetUser(
ctx.getNotes(), psId, accountId, ctx.getRevWalk(), ctx.getRepoView().getConfig())) { ctx.getNotes(), psId, accountId, ctx.getRevWalk(), ctx.getRepoView().getConfig())) {
if (labelTypes.byLabel(a.getLabelId()) == null) { if (labelTypes.byLabel(a.labelId()) == null) {
continue; // Ignore undefined labels. continue; // Ignore undefined labels.
} else if (!a.getLabel().equals(label)) { } else if (!a.label().equals(label)) {
// Populate map for non-matching labels, needed by VoteDeleted. // Populate map for non-matching labels, needed by VoteDeleted.
newApprovals.put(a.getLabel(), a.getValue()); newApprovals.put(a.label(), a.value());
continue; continue;
} else { } else {
try { try {
@@ -189,11 +189,11 @@ public class DeleteVote extends RetryingRestModifyView<VoteResource, DeleteVoteI
} }
} }
// Set the approval to 0 if vote is being removed. // Set the approval to 0 if vote is being removed.
newApprovals.put(a.getLabel(), (short) 0); newApprovals.put(a.label(), (short) 0);
found = true; found = true;
// Set old value, as required by VoteDeleted. // Set old value, as required by VoteDeleted.
oldApprovals.put(a.getLabel(), a.getValue()); oldApprovals.put(a.label(), a.value());
break; break;
} }
if (!found) { if (!found) {

View File

@@ -261,7 +261,7 @@ public class Move extends RetryingRestModifyView<ChangeResource, MoveInput, Chan
approvalsUtil.byPatchSet( approvalsUtil.byPatchSet(
ctx.getNotes(), psId, ctx.getRevWalk(), ctx.getRepoView().getConfig())) { ctx.getNotes(), psId, ctx.getRevWalk(), ctx.getRepoView().getConfig())) {
ProjectState projectState = projectCache.checkedGet(project); ProjectState projectState = projectCache.checkedGet(project);
LabelType type = projectState.getLabelTypes(ctx.getNotes()).byLabel(psa.getLabelId()); LabelType type = projectState.getLabelTypes(ctx.getNotes()).byLabel(psa.labelId());
// Only keep veto votes, defined as votes where: // Only keep veto votes, defined as votes where:
// 1- the label function allows minimum values to block submission. // 1- the label function allows minimum values to block submission.
// 2- the vote holds the minimum value. // 2- the vote holds the minimum value.
@@ -270,10 +270,10 @@ public class Move extends RetryingRestModifyView<ChangeResource, MoveInput, Chan
} }
// Remove votes from NoteDb. // Remove votes from NoteDb.
update.removeApprovalFor(psa.getAccountId(), psa.getLabel()); update.removeApprovalFor(psa.accountId(), psa.label());
approvals.add( approvals.add(
PatchSetApproval.builder() PatchSetApproval.builder()
.key(PatchSetApproval.key(psId, psa.getAccountId(), LabelId.create(psa.getLabel()))) .key(PatchSetApproval.key(psId, psa.accountId(), LabelId.create(psa.label())))
.value(0) .value(0)
.granted(ctx.getWhen()) .granted(ctx.getWhen())
.build()); .build());

View File

@@ -1062,7 +1062,7 @@ public class PostReview
private Map<String, Short> approvalsByKey(Collection<PatchSetApproval> patchsetApprovals) { private Map<String, Short> approvalsByKey(Collection<PatchSetApproval> patchsetApprovals) {
Map<String, Short> labels = new HashMap<>(); Map<String, Short> labels = new HashMap<>();
for (PatchSetApproval psa : patchsetApprovals) { for (PatchSetApproval psa : patchsetApprovals) {
labels.put(psa.getLabel(), psa.getValue()); labels.put(psa.label(), psa.value());
} }
return labels; return labels;
} }
@@ -1142,14 +1142,14 @@ public class PostReview
// User requested delete of this label. // User requested delete of this label.
oldApprovals.put(normName, null); oldApprovals.put(normName, null);
if (c != null) { if (c != null) {
if (c.getValue() != 0) { if (c.value() != 0) {
addLabelDelta(normName, (short) 0); addLabelDelta(normName, (short) 0);
oldApprovals.put(normName, previous.get(normName)); oldApprovals.put(normName, previous.get(normName));
} }
del.add(c); del.add(c);
update.putApproval(normName, (short) 0); update.putApproval(normName, (short) 0);
} }
} else if (c != null && c.getValue() != ent.getValue()) { } else if (c != null && c.value() != ent.getValue()) {
PatchSetApproval.Builder b = PatchSetApproval.Builder b =
c.toBuilder() c.toBuilder()
.value(ent.getValue()) .value(ent.getValue())
@@ -1158,14 +1158,14 @@ public class PostReview
ctx.getUser().updateRealAccountId(b::realAccountId); ctx.getUser().updateRealAccountId(b::realAccountId);
c = b.build(); c = b.build();
ups.add(c); ups.add(c);
addLabelDelta(normName, c.getValue()); addLabelDelta(normName, c.value());
oldApprovals.put(normName, previous.get(normName)); oldApprovals.put(normName, previous.get(normName));
approvals.put(normName, c.getValue()); approvals.put(normName, c.value());
update.putApproval(normName, ent.getValue()); update.putApproval(normName, ent.getValue());
} else if (c != null && c.getValue() == ent.getValue()) { } else if (c != null && c.value() == ent.getValue()) {
current.put(normName, c); current.put(normName, c);
oldApprovals.put(normName, null); oldApprovals.put(normName, null);
approvals.put(normName, c.getValue()); approvals.put(normName, c.value());
} else if (c == null) { } else if (c == null) {
c = c =
ApprovalsUtil.newApproval(psId, user, lt.getLabelId(), ent.getValue(), ctx.getWhen()) ApprovalsUtil.newApproval(psId, user, lt.getLabelId(), ent.getValue(), ctx.getWhen())
@@ -1173,9 +1173,9 @@ public class PostReview
.granted(ctx.getWhen()) .granted(ctx.getWhen())
.build(); .build();
ups.add(c); ups.add(c);
addLabelDelta(normName, c.getValue()); addLabelDelta(normName, c.value());
oldApprovals.put(normName, previous.get(normName)); oldApprovals.put(normName, previous.get(normName));
approvals.put(normName, c.getValue()); approvals.put(normName, c.value());
update.putReviewer(user.getAccountId(), REVIEWER); update.putReviewer(user.getAccountId(), REVIEWER);
update.putApproval(normName, ent.getValue()); update.putApproval(normName, ent.getValue());
} }
@@ -1217,7 +1217,7 @@ public class PostReview
List<String> disallowed = new ArrayList<>(labelTypes.getLabelTypes().size()); List<String> disallowed = new ArrayList<>(labelTypes.getLabelTypes().size());
for (PatchSetApproval psa : del) { for (PatchSetApproval psa : del) {
LabelType lt = requireNonNull(labelTypes.byLabel(psa.getLabel())); LabelType lt = requireNonNull(labelTypes.byLabel(psa.label()));
String normName = lt.getName(); String normName = lt.getName();
if (!lt.allowPostSubmit()) { if (!lt.allowPostSubmit()) {
disallowed.add(normName); disallowed.add(normName);
@@ -1229,7 +1229,7 @@ public class PostReview
} }
for (PatchSetApproval psa : ups) { for (PatchSetApproval psa : ups) {
LabelType lt = requireNonNull(labelTypes.byLabel(psa.getLabel())); LabelType lt = requireNonNull(labelTypes.byLabel(psa.label()));
String normName = lt.getName(); String normName = lt.getName();
if (!lt.allowPostSubmit()) { if (!lt.allowPostSubmit()) {
disallowed.add(normName); disallowed.add(normName);
@@ -1238,8 +1238,8 @@ public class PostReview
if (prev == null) { if (prev == null) {
continue; continue;
} }
checkState(prev != psa.getValue()); // Should be filtered out above. checkState(prev != psa.value()); // Should be filtered out above.
if (prev > psa.getValue()) { if (prev > psa.value()) {
reduced.add(psa); reduced.add(psa);
} }
// No need to set postSubmit bit, which is set automatically when parsing from NoteDb. // No need to set postSubmit bit, which is set automatically when parsing from NoteDb.
@@ -1254,7 +1254,7 @@ public class PostReview
throw new ResourceConflictException( throw new ResourceConflictException(
"Cannot reduce vote on labels for closed change: " "Cannot reduce vote on labels for closed change: "
+ reduced.stream() + reduced.stream()
.map(PatchSetApproval::getLabel) .map(PatchSetApproval::label)
.distinct() .distinct()
.sorted() .sorted()
.collect(joining(", "))); .collect(joining(", ")));
@@ -1313,7 +1313,7 @@ public class PostReview
continue; continue;
} }
LabelType lt = labelTypes.byLabel(a.getLabelId()); LabelType lt = labelTypes.byLabel(a.labelId());
if (lt != null) { if (lt != null) {
current.put(lt.getName(), a); current.put(lt.getName(), a);
} else { } else {

View File

@@ -209,7 +209,7 @@ public class ReviewerRecommender {
Map<Account.Id, MutableDouble> suggestions = new HashMap<>(); Map<Account.Id, MutableDouble> suggestions = new HashMap<>();
for (ChangeData cd : result) { for (ChangeData cd : result) {
for (PatchSetApproval approval : cd.currentApprovals()) { for (PatchSetApproval approval : cd.currentApprovals()) {
Account.Id id = approval.getAccountId(); Account.Id id = approval.accountId();
if (suggestions.containsKey(id)) { if (suggestions.containsKey(id)) {
suggestions.get(id).add(baseWeight); suggestions.get(id).add(baseWeight);
} else { } else {

View File

@@ -85,7 +85,7 @@ public class Votes implements ChildCollection<ReviewerResource, VoteResource> {
null, null,
null); null);
for (PatchSetApproval psa : byPatchSetUser) { for (PatchSetApproval psa : byPatchSetUser) {
votes.put(psa.getLabel(), psa.getValue()); votes.put(psa.label(), psa.value());
} }
return votes; return votes;
} }

View File

@@ -125,7 +125,7 @@ public final class DefaultSubmitRule implements SubmitRule {
private static List<PatchSetApproval> getApprovalsForLabel( private static List<PatchSetApproval> getApprovalsForLabel(
List<PatchSetApproval> approvals, LabelType t) { List<PatchSetApproval> approvals, LabelType t) {
return approvals.stream() return approvals.stream()
.filter(input -> input.getLabel().equals(t.getLabelId().get())) .filter(input -> input.label().equals(t.getLabelId().get()))
.collect(toImmutableList()); .collect(toImmutableList());
} }
} }

View File

@@ -155,7 +155,7 @@ public class IgnoreSelfApprovalRule implements SubmitRule {
static Collection<PatchSetApproval> filterOutPositiveApprovalsOfUser( static Collection<PatchSetApproval> filterOutPositiveApprovalsOfUser(
Collection<PatchSetApproval> approvals, Account.Id user) { Collection<PatchSetApproval> approvals, Account.Id user) {
return approvals.stream() return approvals.stream()
.filter(input -> input.getValue() < 0 || !input.getAccountId().equals(user)) .filter(input -> input.value() < 0 || !input.accountId().equals(user))
.collect(toImmutableList()); .collect(toImmutableList());
} }
@@ -163,7 +163,7 @@ public class IgnoreSelfApprovalRule implements SubmitRule {
static Collection<PatchSetApproval> filterApprovalsByLabel( static Collection<PatchSetApproval> filterApprovalsByLabel(
Collection<PatchSetApproval> approvals, LabelType t) { Collection<PatchSetApproval> approvals, LabelType t) {
return approvals.stream() return approvals.stream()
.filter(input -> input.getLabelId().get().equals(t.getLabelId().get())) .filter(input -> input.labelId().get().equals(t.getLabelId().get()))
.collect(toImmutableList()); .collect(toImmutableList());
} }
} }

View File

@@ -344,13 +344,13 @@ abstract class SubmitStrategyOp implements BatchUpdateOp {
for (PatchSetApproval psa : for (PatchSetApproval psa :
args.approvalsUtil.byPatchSet( args.approvalsUtil.byPatchSet(
ctx.getNotes(), psId, ctx.getRevWalk(), ctx.getRepoView().getConfig())) { ctx.getNotes(), psId, ctx.getRevWalk(), ctx.getRepoView().getConfig())) {
byKey.put(psa.getKey(), psa); byKey.put(psa.key(), psa);
} }
submitter = submitter =
ApprovalsUtil.newApproval(psId, ctx.getUser(), LabelId.legacySubmit(), 1, ctx.getWhen()) ApprovalsUtil.newApproval(psId, ctx.getUser(), LabelId.legacySubmit(), 1, ctx.getWhen())
.build(); .build();
byKey.put(submitter.getKey(), submitter); byKey.put(submitter.key(), submitter);
// Flatten out existing approvals for this patch set based upon the current // Flatten out existing approvals for this patch set based upon the current
// permissions. Once the change is closed the approvals are not updated at // permissions. Once the change is closed the approvals are not updated at
@@ -359,7 +359,7 @@ abstract class SubmitStrategyOp implements BatchUpdateOp {
// permissions get modified in the future, historical records stay accurate. // permissions get modified in the future, historical records stay accurate.
LabelNormalizer.Result normalized = LabelNormalizer.Result normalized =
args.labelNormalizer.normalize(ctx.getNotes(), byKey.values()); args.labelNormalizer.normalize(ctx.getNotes(), byKey.values());
update.putApproval(submitter.getLabel(), submitter.getValue()); update.putApproval(submitter.label(), submitter.value());
saveApprovals(normalized, update, false); saveApprovals(normalized, update, false);
return normalized; return normalized;
} }
@@ -367,10 +367,10 @@ abstract class SubmitStrategyOp implements BatchUpdateOp {
private void saveApprovals( private void saveApprovals(
LabelNormalizer.Result normalized, ChangeUpdate update, boolean includeUnchanged) { LabelNormalizer.Result normalized, ChangeUpdate update, boolean includeUnchanged) {
for (PatchSetApproval psa : normalized.updated()) { for (PatchSetApproval psa : normalized.updated()) {
update.putApprovalFor(psa.getAccountId(), psa.getLabel(), psa.getValue()); update.putApprovalFor(psa.accountId(), psa.label(), psa.value());
} }
for (PatchSetApproval psa : normalized.deleted()) { for (PatchSetApproval psa : normalized.deleted()) {
update.removeApprovalFor(psa.getAccountId(), psa.getLabel()); update.removeApprovalFor(psa.accountId(), psa.label());
} }
// TODO(dborowitz): Don't use a label in NoteDb; just check when status // TODO(dborowitz): Don't use a label in NoteDb; just check when status
@@ -378,7 +378,7 @@ abstract class SubmitStrategyOp implements BatchUpdateOp {
for (PatchSetApproval psa : normalized.unchanged()) { for (PatchSetApproval psa : normalized.unchanged()) {
if (includeUnchanged || psa.isLegacySubmit()) { if (includeUnchanged || psa.isLegacySubmit()) {
logger.atFine().log("Adding submit label %s", psa); logger.atFine().log("Adding submit label %s", psa);
update.putApprovalFor(psa.getAccountId(), psa.getLabel(), psa.getValue()); update.putApprovalFor(psa.accountId(), psa.label(), psa.value());
} }
} }
} }
@@ -386,7 +386,7 @@ abstract class SubmitStrategyOp implements BatchUpdateOp {
private String getByAccountName() { private String getByAccountName() {
requireNonNull(submitter, "getByAccountName called before submitter populated"); requireNonNull(submitter, "getByAccountName called before submitter populated");
Optional<Account> account = Optional<Account> account =
args.accountCache.get(submitter.getAccountId()).map(AccountState::getAccount); args.accountCache.get(submitter.accountId()).map(AccountState::getAccount);
if (account.isPresent() && account.get().getFullName() != null) { if (account.isPresent() && account.get().getFullName() != null) {
return " by " + account.get().getFullName(); return " by " + account.get().getFullName();
} }
@@ -489,7 +489,7 @@ abstract class SubmitStrategyOp implements BatchUpdateOp {
// have failed fast in one of the other steps. // have failed fast in one of the other steps.
try { try {
args.mergedSenderFactory args.mergedSenderFactory
.create(ctx.getProject(), getId(), submitter.getAccountId(), ctx.getNotify(getId())) .create(ctx.getProject(), getId(), submitter.accountId(), ctx.getNotify(getId()))
.sendAsync(); .sendAsync();
} catch (Exception e) { } catch (Exception e) {
logger.atSevere().withCause(e).log("Cannot email merged notification for %s", getId()); logger.atSevere().withCause(e).log("Cannot email merged notification for %s", getId());
@@ -498,7 +498,7 @@ abstract class SubmitStrategyOp implements BatchUpdateOp {
args.changeMerged.fire( args.changeMerged.fire(
updatedChange, updatedChange,
mergedPatchSet, mergedPatchSet,
args.accountCache.get(submitter.getAccountId()).orElse(null), args.accountCache.get(submitter.accountId()).orElse(null),
args.mergeTip.getCurrentTip().name(), args.mergeTip.getCurrentTip().name(),
ctx.getWhen()); ctx.getWhen());
} }

View File

@@ -38,16 +38,15 @@ class PRED__load_commit_labels_1 extends Predicate.P1 {
LabelTypes types = cd.getLabelTypes(); LabelTypes types = cd.getLabelTypes();
for (PatchSetApproval a : cd.currentApprovals()) { for (PatchSetApproval a : cd.currentApprovals()) {
LabelType t = types.byLabel(a.getLabelId()); LabelType t = types.byLabel(a.labelId());
if (t == null) { if (t == null) {
continue; continue;
} }
StructureTerm labelTerm = StructureTerm labelTerm =
new StructureTerm( new StructureTerm(sym_label, SymbolTerm.intern(t.getName()), new IntegerTerm(a.value()));
sym_label, SymbolTerm.intern(t.getName()), new IntegerTerm(a.getValue()));
StructureTerm userTerm = new StructureTerm(sym_user, new IntegerTerm(a.getAccountId().get())); StructureTerm userTerm = new StructureTerm(sym_user, new IntegerTerm(a.accountId().get()));
listHead = new ListTerm(new StructureTerm(sym_commit_label, labelTerm, userTerm), listHead); listHead = new ListTerm(new StructureTerm(sym_commit_label, labelTerm, userTerm), listHead);
} }

View File

@@ -281,10 +281,10 @@ public class RevisionIT extends AbstractDaemonTest {
PatchSetApproval psa = PatchSetApproval psa =
Iterators.getOnlyElement( Iterators.getOnlyElement(
cd.currentApprovals().stream().filter(a -> !a.isLegacySubmit()).iterator()); cd.currentApprovals().stream().filter(a -> !a.isLegacySubmit()).iterator());
assertThat(psa.getPatchSetId().get()).isEqualTo(2); assertThat(psa.patchSetId().get()).isEqualTo(2);
assertThat(psa.getLabel()).isEqualTo("Code-Review"); assertThat(psa.label()).isEqualTo("Code-Review");
assertThat(psa.getValue()).isEqualTo(2); assertThat(psa.value()).isEqualTo(2);
assertThat(psa.isPostSubmit()).isFalse(); assertThat(psa.postSubmit()).isFalse();
} }
@Test @Test

View File

@@ -323,8 +323,8 @@ public class SubmitOnPushIT extends AbstractDaemonTest {
private void assertSubmitApproval(PatchSet.Id patchSetId) throws Exception { private void assertSubmitApproval(PatchSet.Id patchSetId) throws Exception {
PatchSetApproval a = getSubmitter(patchSetId); PatchSetApproval a = getSubmitter(patchSetId);
assertThat(a.isLegacySubmit()).isTrue(); assertThat(a.isLegacySubmit()).isTrue();
assertThat(a.getValue()).isEqualTo((short) 1); assertThat(a.value()).isEqualTo((short) 1);
assertThat(a.getAccountId()).isEqualTo(admin.id()); assertThat(a.accountId()).isEqualTo(admin.id());
} }
private void assertCommit(Project.NameKey project, String branch) throws Exception { private void assertCommit(Project.NameKey project, String branch) throws Exception {

View File

@@ -107,11 +107,11 @@ public class ImpersonationIT extends AbstractDaemonTest {
revision.review(in); revision.review(in);
PatchSetApproval psa = Iterables.getOnlyElement(r.getChange().approvals().values()); PatchSetApproval psa = Iterables.getOnlyElement(r.getChange().approvals().values());
assertThat(psa.getPatchSetId().get()).isEqualTo(1); assertThat(psa.patchSetId().get()).isEqualTo(1);
assertThat(psa.getLabel()).isEqualTo("Code-Review"); assertThat(psa.label()).isEqualTo("Code-Review");
assertThat(psa.getAccountId()).isEqualTo(user.id()); assertThat(psa.accountId()).isEqualTo(user.id());
assertThat(psa.getValue()).isEqualTo(1); assertThat(psa.value()).isEqualTo(1);
assertThat(psa.getRealAccountId()).isEqualTo(admin.id()); assertThat(psa.realAccountId()).isEqualTo(admin.id());
ChangeData cd = r.getChange(); ChangeData cd = r.getChange();
ChangeMessage m = Iterables.getLast(cmUtil.byChange(cd.notes())); ChangeMessage m = Iterables.getLast(cmUtil.byChange(cd.notes()));
@@ -211,11 +211,11 @@ public class ImpersonationIT extends AbstractDaemonTest {
gApi.changes().id(r.getChangeId()).current().review(in); gApi.changes().id(r.getChangeId()).current().review(in);
PatchSetApproval psa = Iterables.getOnlyElement(r.getChange().approvals().values()); PatchSetApproval psa = Iterables.getOnlyElement(r.getChange().approvals().values());
assertThat(psa.getPatchSetId().get()).isEqualTo(1); assertThat(psa.patchSetId().get()).isEqualTo(1);
assertThat(psa.getLabel()).isEqualTo("Code-Review"); assertThat(psa.label()).isEqualTo("Code-Review");
assertThat(psa.getAccountId()).isEqualTo(user.id()); assertThat(psa.accountId()).isEqualTo(user.id());
assertThat(psa.getValue()).isEqualTo(1); assertThat(psa.value()).isEqualTo(1);
assertThat(psa.getRealAccountId()).isEqualTo(admin.id()); assertThat(psa.realAccountId()).isEqualTo(admin.id());
ChangeData cd = r.getChange(); ChangeData cd = r.getChange();
Comment c = Iterables.getOnlyElement(commentsUtil.publishedByChange(cd.notes())); Comment c = Iterables.getOnlyElement(commentsUtil.publishedByChange(cd.notes()));
@@ -345,8 +345,8 @@ public class ImpersonationIT extends AbstractDaemonTest {
assertThat(cd.change().isMerged()).isTrue(); assertThat(cd.change().isMerged()).isTrue();
PatchSetApproval submitter = PatchSetApproval submitter =
approvalsUtil.getSubmitter(cd.notes(), cd.change().currentPatchSetId()); approvalsUtil.getSubmitter(cd.notes(), cd.change().currentPatchSetId());
assertThat(submitter.getAccountId()).isEqualTo(admin2.id()); assertThat(submitter.accountId()).isEqualTo(admin2.id());
assertThat(submitter.getRealAccountId()).isEqualTo(admin.id()); assertThat(submitter.realAccountId()).isEqualTo(admin.id());
} }
@Test @Test
@@ -531,11 +531,11 @@ public class ImpersonationIT extends AbstractDaemonTest {
adminRestSession.postWithHeader(endpoint, runAsHeader(user2.id()), in).assertOK(); adminRestSession.postWithHeader(endpoint, runAsHeader(user2.id()), in).assertOK();
PatchSetApproval psa = Iterables.getOnlyElement(r.getChange().approvals().values()); PatchSetApproval psa = Iterables.getOnlyElement(r.getChange().approvals().values());
assertThat(psa.getPatchSetId().get()).isEqualTo(1); assertThat(psa.patchSetId().get()).isEqualTo(1);
assertThat(psa.getLabel()).isEqualTo("Code-Review"); assertThat(psa.label()).isEqualTo("Code-Review");
assertThat(psa.getAccountId()).isEqualTo(user.id()); assertThat(psa.accountId()).isEqualTo(user.id());
assertThat(psa.getValue()).isEqualTo(1); assertThat(psa.value()).isEqualTo(1);
assertThat(psa.getRealAccountId()).isEqualTo(admin.id()); // not user2 assertThat(psa.realAccountId()).isEqualTo(admin.id()); // not user2
ChangeData cd = r.getChange(); ChangeData cd = r.getChange();
ChangeMessage m = Iterables.getLast(cmUtil.byChange(cd.notes())); ChangeMessage m = Iterables.getLast(cmUtil.byChange(cd.notes()));

View File

@@ -1256,7 +1256,7 @@ public abstract class AbstractSubmit extends AbstractDaemonTest {
approvalsUtil.getSubmitter(cn, PatchSet.id(cn.getChangeId(), psId)); approvalsUtil.getSubmitter(cn, PatchSet.id(cn.getChangeId(), psId));
assertThat(submitter).isNotNull(); assertThat(submitter).isNotNull();
assertThat(submitter.isLegacySubmit()).isTrue(); assertThat(submitter.isLegacySubmit()).isTrue();
assertThat(submitter.getAccountId()).isEqualTo(user.id()); assertThat(submitter.accountId()).isEqualTo(user.id());
} }
protected void assertNoSubmitter(String changeId, int psId) throws Throwable { protected void assertNoSubmitter(String changeId, int psId) throws Throwable {

View File

@@ -81,7 +81,7 @@ public class LabelFunctionTest {
SubmitRecord.Label myLabel = LabelFunction.MAX_NO_BLOCK.check(VERIFIED_LABEL, approvals); SubmitRecord.Label myLabel = LabelFunction.MAX_NO_BLOCK.check(VERIFIED_LABEL, approvals);
assertThat(myLabel.status).isEqualTo(SubmitRecord.Label.Status.OK); assertThat(myLabel.status).isEqualTo(SubmitRecord.Label.Status.OK);
assertThat(myLabel.appliedBy).isEqualTo(APPROVAL_2.getAccountId()); assertThat(myLabel.appliedBy).isEqualTo(APPROVAL_2.accountId());
} }
private static LabelType makeLabel() { private static LabelType makeLabel() {
@@ -109,7 +109,7 @@ public class LabelFunctionTest {
SubmitRecord.Label myLabel = function.check(VERIFIED_LABEL, approvals); SubmitRecord.Label myLabel = function.check(VERIFIED_LABEL, approvals);
assertThat(myLabel.status).isEqualTo(SubmitRecord.Label.Status.REJECT); assertThat(myLabel.status).isEqualTo(SubmitRecord.Label.Status.REJECT);
assertThat(myLabel.appliedBy).isEqualTo(APPROVAL_M2.getAccountId()); assertThat(myLabel.appliedBy).isEqualTo(APPROVAL_M2.accountId());
} }
private static void checkNothingHappens(LabelFunction function) { private static void checkNothingHappens(LabelFunction function) {
@@ -140,6 +140,6 @@ public class LabelFunctionTest {
SubmitRecord.Label myLabel = function.check(VERIFIED_LABEL, approvals); SubmitRecord.Label myLabel = function.check(VERIFIED_LABEL, approvals);
assertThat(myLabel.status).isEqualTo(SubmitRecord.Label.Status.OK); assertThat(myLabel.status).isEqualTo(SubmitRecord.Label.Status.OK);
assertThat(myLabel.appliedBy).isEqualTo(APPROVAL_2.getAccountId()); assertThat(myLabel.appliedBy).isEqualTo(APPROVAL_2.accountId());
} }
} }

View File

@@ -156,13 +156,13 @@ public class PatchSetApprovalProtoConverterTest {
.build(); .build();
PatchSetApproval patchSetApproval = protoConverter.fromProto(proto); PatchSetApproval patchSetApproval = protoConverter.fromProto(proto);
assertThat(patchSetApproval.getPatchSetId()).isEqualTo(PatchSet.id(Change.id(42), 14)); assertThat(patchSetApproval.patchSetId()).isEqualTo(PatchSet.id(Change.id(42), 14));
assertThat(patchSetApproval.getAccountId()).isEqualTo(Account.id(100013)); assertThat(patchSetApproval.accountId()).isEqualTo(Account.id(100013));
assertThat(patchSetApproval.getLabelId()).isEqualTo(LabelId.create("label-8")); assertThat(patchSetApproval.labelId()).isEqualTo(LabelId.create("label-8"));
// Default values for unset protobuf fields which can't be unset in the entity object. // Default values for unset protobuf fields which can't be unset in the entity object.
assertThat(patchSetApproval.getValue()).isEqualTo(0); assertThat(patchSetApproval.value()).isEqualTo(0);
assertThat(patchSetApproval.getGranted()).isEqualTo(new Timestamp(0)); assertThat(patchSetApproval.granted()).isEqualTo(new Timestamp(0));
assertThat(patchSetApproval.isPostSubmit()).isEqualTo(false); assertThat(patchSetApproval.postSubmit()).isEqualTo(false);
} }
@Test @Test
@@ -194,12 +194,12 @@ public class PatchSetApprovalProtoConverterTest {
assertThatSerializedClass(PatchSetApproval.class) assertThatSerializedClass(PatchSetApproval.class)
.hasAutoValueMethods( .hasAutoValueMethods(
ImmutableMap.<String, Type>builder() ImmutableMap.<String, Type>builder()
.put("getKey", PatchSetApproval.Key.class) .put("key", PatchSetApproval.Key.class)
.put("getValue", short.class) .put("value", short.class)
.put("getGranted", Timestamp.class) .put("granted", Timestamp.class)
.put("getTag", new TypeLiteral<Optional<String>>() {}.getType()) .put("tag", new TypeLiteral<Optional<String>>() {}.getType())
.put("getRealAccountId", Account.Id.class) .put("realAccountId", Account.Id.class)
.put("isPostSubmit", boolean.class) .put("postSubmit", boolean.class)
.put("toBuilder", PatchSetApproval.Builder.class) .put("toBuilder", PatchSetApproval.Builder.class)
.build()); .build());
} }

View File

@@ -392,8 +392,7 @@ public class ChangeNotesStateTest {
assertRoundTrip( assertRoundTrip(
newBuilder() newBuilder()
.approvals( .approvals(ImmutableListMultimap.of(a2.patchSetId(), a2, a1.patchSetId(), a1).entries())
ImmutableListMultimap.of(a2.getPatchSetId(), a2, a1.getPatchSetId(), a1).entries())
.build(), .build(),
ChangeNotesStateProto.newBuilder() ChangeNotesStateProto.newBuilder()
.setMetaId(SHA_BYTES) .setMetaId(SHA_BYTES)
@@ -797,12 +796,12 @@ public class ChangeNotesStateTest {
assertThatSerializedClass(PatchSetApproval.class) assertThatSerializedClass(PatchSetApproval.class)
.hasAutoValueMethods( .hasAutoValueMethods(
ImmutableMap.<String, Type>builder() ImmutableMap.<String, Type>builder()
.put("getKey", PatchSetApproval.Key.class) .put("key", PatchSetApproval.Key.class)
.put("getValue", short.class) .put("value", short.class)
.put("getGranted", Timestamp.class) .put("granted", Timestamp.class)
.put("getTag", new TypeLiteral<Optional<String>>() {}.getType()) .put("tag", new TypeLiteral<Optional<String>>() {}.getType())
.put("getRealAccountId", Account.Id.class) .put("realAccountId", Account.Id.class)
.put("isPostSubmit", boolean.class) .put("postSubmit", boolean.class)
.put("toBuilder", PatchSetApproval.Builder.class) .put("toBuilder", PatchSetApproval.Builder.class)
.build()); .build());
} }

View File

@@ -163,7 +163,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
ImmutableListMultimap<PatchSet.Id, PatchSetApproval> approvals = notes.getApprovals(); ImmutableListMultimap<PatchSet.Id, PatchSetApproval> approvals = notes.getApprovals();
assertThat(approvals).hasSize(1); assertThat(approvals).hasSize(1);
assertThat(approvals.entries().asList().get(0).getValue().getTag()).hasValue(tag2); assertThat(approvals.entries().asList().get(0).getValue().tag()).hasValue(tag2);
} }
@Test @Test
@@ -210,8 +210,8 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
ImmutableListMultimap<PatchSet.Id, PatchSetApproval> approvals = notes.getApprovals(); ImmutableListMultimap<PatchSet.Id, PatchSetApproval> approvals = notes.getApprovals();
assertThat(approvals).hasSize(1); assertThat(approvals).hasSize(1);
PatchSetApproval approval = approvals.entries().asList().get(0).getValue(); PatchSetApproval approval = approvals.entries().asList().get(0).getValue();
assertThat(approval.getTag()).hasValue(integrationTag); assertThat(approval.tag()).hasValue(integrationTag);
assertThat(approval.getValue()).isEqualTo(-1); assertThat(approval.value()).isEqualTo(-1);
ImmutableListMultimap<ObjectId, Comment> comments = notes.getComments(); ImmutableListMultimap<ObjectId, Comment> comments = notes.getComments();
assertThat(comments).hasSize(1); assertThat(comments).hasSize(1);
@@ -237,17 +237,17 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
List<PatchSetApproval> psas = notes.getApprovals().get(c.currentPatchSetId()); List<PatchSetApproval> psas = notes.getApprovals().get(c.currentPatchSetId());
assertThat(psas).hasSize(2); assertThat(psas).hasSize(2);
assertThat(psas.get(0).getPatchSetId()).isEqualTo(c.currentPatchSetId()); assertThat(psas.get(0).patchSetId()).isEqualTo(c.currentPatchSetId());
assertThat(psas.get(0).getAccountId().get()).isEqualTo(1); assertThat(psas.get(0).accountId().get()).isEqualTo(1);
assertThat(psas.get(0).getLabel()).isEqualTo("Code-Review"); assertThat(psas.get(0).label()).isEqualTo("Code-Review");
assertThat(psas.get(0).getValue()).isEqualTo((short) -1); assertThat(psas.get(0).value()).isEqualTo((short) -1);
assertThat(psas.get(0).getGranted()).isEqualTo(truncate(after(c, 2000))); assertThat(psas.get(0).granted()).isEqualTo(truncate(after(c, 2000)));
assertThat(psas.get(1).getPatchSetId()).isEqualTo(c.currentPatchSetId()); assertThat(psas.get(1).patchSetId()).isEqualTo(c.currentPatchSetId());
assertThat(psas.get(1).getAccountId().get()).isEqualTo(1); assertThat(psas.get(1).accountId().get()).isEqualTo(1);
assertThat(psas.get(1).getLabel()).isEqualTo("Verified"); assertThat(psas.get(1).label()).isEqualTo("Verified");
assertThat(psas.get(1).getValue()).isEqualTo((short) 1); assertThat(psas.get(1).value()).isEqualTo((short) 1);
assertThat(psas.get(1).getGranted()).isEqualTo(psas.get(0).getGranted()); assertThat(psas.get(1).granted()).isEqualTo(psas.get(0).granted());
} }
@Test @Test
@@ -269,18 +269,18 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
assertThat(psas).hasSize(2); assertThat(psas).hasSize(2);
PatchSetApproval psa1 = Iterables.getOnlyElement(psas.get(ps1)); PatchSetApproval psa1 = Iterables.getOnlyElement(psas.get(ps1));
assertThat(psa1.getPatchSetId()).isEqualTo(ps1); assertThat(psa1.patchSetId()).isEqualTo(ps1);
assertThat(psa1.getAccountId().get()).isEqualTo(1); assertThat(psa1.accountId().get()).isEqualTo(1);
assertThat(psa1.getLabel()).isEqualTo("Code-Review"); assertThat(psa1.label()).isEqualTo("Code-Review");
assertThat(psa1.getValue()).isEqualTo((short) -1); assertThat(psa1.value()).isEqualTo((short) -1);
assertThat(psa1.getGranted()).isEqualTo(truncate(after(c, 2000))); assertThat(psa1.granted()).isEqualTo(truncate(after(c, 2000)));
PatchSetApproval psa2 = Iterables.getOnlyElement(psas.get(ps2)); PatchSetApproval psa2 = Iterables.getOnlyElement(psas.get(ps2));
assertThat(psa2.getPatchSetId()).isEqualTo(ps2); assertThat(psa2.patchSetId()).isEqualTo(ps2);
assertThat(psa2.getAccountId().get()).isEqualTo(1); assertThat(psa2.accountId().get()).isEqualTo(1);
assertThat(psa2.getLabel()).isEqualTo("Code-Review"); assertThat(psa2.label()).isEqualTo("Code-Review");
assertThat(psa2.getValue()).isEqualTo((short) +1); assertThat(psa2.value()).isEqualTo((short) +1);
assertThat(psa2.getGranted()).isEqualTo(truncate(after(c, 4000))); assertThat(psa2.granted()).isEqualTo(truncate(after(c, 4000)));
} }
@Test @Test
@@ -293,8 +293,8 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
ChangeNotes notes = newNotes(c); ChangeNotes notes = newNotes(c);
PatchSetApproval psa = PatchSetApproval psa =
Iterables.getOnlyElement(notes.getApprovals().get(c.currentPatchSetId())); Iterables.getOnlyElement(notes.getApprovals().get(c.currentPatchSetId()));
assertThat(psa.getLabel()).isEqualTo("Code-Review"); assertThat(psa.label()).isEqualTo("Code-Review");
assertThat(psa.getValue()).isEqualTo((short) -1); assertThat(psa.value()).isEqualTo((short) -1);
update = newUpdate(c, changeOwner); update = newUpdate(c, changeOwner);
update.putApproval("Code-Review", (short) 1); update.putApproval("Code-Review", (short) 1);
@@ -302,8 +302,8 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
notes = newNotes(c); notes = newNotes(c);
psa = Iterables.getOnlyElement(notes.getApprovals().get(c.currentPatchSetId())); psa = Iterables.getOnlyElement(notes.getApprovals().get(c.currentPatchSetId()));
assertThat(psa.getLabel()).isEqualTo("Code-Review"); assertThat(psa.label()).isEqualTo("Code-Review");
assertThat(psa.getValue()).isEqualTo((short) 1); assertThat(psa.value()).isEqualTo((short) 1);
} }
@Test @Test
@@ -322,17 +322,17 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
List<PatchSetApproval> psas = notes.getApprovals().get(c.currentPatchSetId()); List<PatchSetApproval> psas = notes.getApprovals().get(c.currentPatchSetId());
assertThat(psas).hasSize(2); assertThat(psas).hasSize(2);
assertThat(psas.get(0).getPatchSetId()).isEqualTo(c.currentPatchSetId()); assertThat(psas.get(0).patchSetId()).isEqualTo(c.currentPatchSetId());
assertThat(psas.get(0).getAccountId().get()).isEqualTo(1); assertThat(psas.get(0).accountId().get()).isEqualTo(1);
assertThat(psas.get(0).getLabel()).isEqualTo("Code-Review"); assertThat(psas.get(0).label()).isEqualTo("Code-Review");
assertThat(psas.get(0).getValue()).isEqualTo((short) -1); assertThat(psas.get(0).value()).isEqualTo((short) -1);
assertThat(psas.get(0).getGranted()).isEqualTo(truncate(after(c, 2000))); assertThat(psas.get(0).granted()).isEqualTo(truncate(after(c, 2000)));
assertThat(psas.get(1).getPatchSetId()).isEqualTo(c.currentPatchSetId()); assertThat(psas.get(1).patchSetId()).isEqualTo(c.currentPatchSetId());
assertThat(psas.get(1).getAccountId().get()).isEqualTo(2); assertThat(psas.get(1).accountId().get()).isEqualTo(2);
assertThat(psas.get(1).getLabel()).isEqualTo("Code-Review"); assertThat(psas.get(1).label()).isEqualTo("Code-Review");
assertThat(psas.get(1).getValue()).isEqualTo((short) 1); assertThat(psas.get(1).value()).isEqualTo((short) 1);
assertThat(psas.get(1).getGranted()).isEqualTo(truncate(after(c, 3000))); assertThat(psas.get(1).granted()).isEqualTo(truncate(after(c, 3000)));
} }
@Test @Test
@@ -345,9 +345,9 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
ChangeNotes notes = newNotes(c); ChangeNotes notes = newNotes(c);
PatchSetApproval psa = PatchSetApproval psa =
Iterables.getOnlyElement(notes.getApprovals().get(c.currentPatchSetId())); Iterables.getOnlyElement(notes.getApprovals().get(c.currentPatchSetId()));
assertThat(psa.getAccountId().get()).isEqualTo(1); assertThat(psa.accountId().get()).isEqualTo(1);
assertThat(psa.getLabel()).isEqualTo("Not-For-Long"); assertThat(psa.label()).isEqualTo("Not-For-Long");
assertThat(psa.getValue()).isEqualTo((short) 1); assertThat(psa.value()).isEqualTo((short) 1);
update = newUpdate(c, changeOwner); update = newUpdate(c, changeOwner);
update.removeApproval("Not-For-Long"); update.removeApproval("Not-For-Long");
@@ -357,9 +357,9 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
assertThat(notes.getApprovals()) assertThat(notes.getApprovals())
.containsExactlyEntriesIn( .containsExactlyEntriesIn(
ImmutableListMultimap.of( ImmutableListMultimap.of(
psa.getPatchSetId(), psa.patchSetId(),
PatchSetApproval.builder() PatchSetApproval.builder()
.key(psa.getKey()) .key(psa.key())
.value(0) .value(0)
.granted(update.getWhen()) .granted(update.getWhen())
.build())); .build()));
@@ -375,9 +375,9 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
ChangeNotes notes = newNotes(c); ChangeNotes notes = newNotes(c);
PatchSetApproval psa = PatchSetApproval psa =
Iterables.getOnlyElement(notes.getApprovals().get(c.currentPatchSetId())); Iterables.getOnlyElement(notes.getApprovals().get(c.currentPatchSetId()));
assertThat(psa.getAccountId()).isEqualTo(otherUserId); assertThat(psa.accountId()).isEqualTo(otherUserId);
assertThat(psa.getLabel()).isEqualTo("Not-For-Long"); assertThat(psa.label()).isEqualTo("Not-For-Long");
assertThat(psa.getValue()).isEqualTo((short) 1); assertThat(psa.value()).isEqualTo((short) 1);
update = newUpdate(c, changeOwner); update = newUpdate(c, changeOwner);
update.removeApprovalFor(otherUserId, "Not-For-Long"); update.removeApprovalFor(otherUserId, "Not-For-Long");
@@ -387,9 +387,9 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
assertThat(notes.getApprovals()) assertThat(notes.getApprovals())
.containsExactlyEntriesIn( .containsExactlyEntriesIn(
ImmutableListMultimap.of( ImmutableListMultimap.of(
psa.getPatchSetId(), psa.patchSetId(),
PatchSetApproval.builder() PatchSetApproval.builder()
.key(psa.getKey()) .key(psa.key())
.value(0) .value(0)
.granted(update.getWhen()) .granted(update.getWhen())
.build())); .build()));
@@ -401,9 +401,9 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
notes = newNotes(c); notes = newNotes(c);
psa = Iterables.getOnlyElement(notes.getApprovals().get(c.currentPatchSetId())); psa = Iterables.getOnlyElement(notes.getApprovals().get(c.currentPatchSetId()));
assertThat(psa.getAccountId()).isEqualTo(otherUserId); assertThat(psa.accountId()).isEqualTo(otherUserId);
assertThat(psa.getLabel()).isEqualTo("Not-For-Long"); assertThat(psa.label()).isEqualTo("Not-For-Long");
assertThat(psa.getValue()).isEqualTo((short) 2); assertThat(psa.value()).isEqualTo((short) 2);
} }
@Test @Test
@@ -417,17 +417,17 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
ChangeNotes notes = newNotes(c); ChangeNotes notes = newNotes(c);
ImmutableList<PatchSetApproval> approvals = ImmutableList<PatchSetApproval> approvals =
notes.getApprovals().get(c.currentPatchSetId()).stream() notes.getApprovals().get(c.currentPatchSetId()).stream()
.sorted(comparing(a -> a.getAccountId().get())) .sorted(comparing(a -> a.accountId().get()))
.collect(toImmutableList()); .collect(toImmutableList());
assertThat(approvals).hasSize(2); assertThat(approvals).hasSize(2);
assertThat(approvals.get(0).getAccountId()).isEqualTo(changeOwner.getAccountId()); assertThat(approvals.get(0).accountId()).isEqualTo(changeOwner.getAccountId());
assertThat(approvals.get(0).getLabel()).isEqualTo("Code-Review"); assertThat(approvals.get(0).label()).isEqualTo("Code-Review");
assertThat(approvals.get(0).getValue()).isEqualTo((short) 1); assertThat(approvals.get(0).value()).isEqualTo((short) 1);
assertThat(approvals.get(1).getAccountId()).isEqualTo(otherUser.getAccountId()); assertThat(approvals.get(1).accountId()).isEqualTo(otherUser.getAccountId());
assertThat(approvals.get(1).getLabel()).isEqualTo("Code-Review"); assertThat(approvals.get(1).label()).isEqualTo("Code-Review");
assertThat(approvals.get(1).getValue()).isEqualTo((short) -1); assertThat(approvals.get(1).value()).isEqualTo((short) -1);
} }
@Test @Test
@@ -457,12 +457,12 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
ChangeNotes notes = newNotes(c); ChangeNotes notes = newNotes(c);
List<PatchSetApproval> approvals = Lists.newArrayList(notes.getApprovals().values()); List<PatchSetApproval> approvals = Lists.newArrayList(notes.getApprovals().values());
assertThat(approvals).hasSize(2); assertThat(approvals).hasSize(2);
assertThat(approvals.get(0).getLabel()).isEqualTo("Verified"); assertThat(approvals.get(0).label()).isEqualTo("Verified");
assertThat(approvals.get(0).getValue()).isEqualTo((short) 1); assertThat(approvals.get(0).value()).isEqualTo((short) 1);
assertThat(approvals.get(0).isPostSubmit()).isFalse(); assertThat(approvals.get(0).postSubmit()).isFalse();
assertThat(approvals.get(1).getLabel()).isEqualTo("Code-Review"); assertThat(approvals.get(1).label()).isEqualTo("Code-Review");
assertThat(approvals.get(1).getValue()).isEqualTo((short) 2); assertThat(approvals.get(1).value()).isEqualTo((short) 2);
assertThat(approvals.get(1).isPostSubmit()).isTrue(); assertThat(approvals.get(1).postSubmit()).isTrue();
} }
@Test @Test
@@ -497,18 +497,18 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
List<PatchSetApproval> approvals = Lists.newArrayList(notes.getApprovals().values()); List<PatchSetApproval> approvals = Lists.newArrayList(notes.getApprovals().values());
assertThat(approvals).hasSize(3); assertThat(approvals).hasSize(3);
assertThat(approvals.get(0).getAccountId()).isEqualTo(ownerId); assertThat(approvals.get(0).accountId()).isEqualTo(ownerId);
assertThat(approvals.get(0).getLabel()).isEqualTo("Verified"); assertThat(approvals.get(0).label()).isEqualTo("Verified");
assertThat(approvals.get(0).getValue()).isEqualTo(1); assertThat(approvals.get(0).value()).isEqualTo(1);
assertThat(approvals.get(0).isPostSubmit()).isFalse(); assertThat(approvals.get(0).postSubmit()).isFalse();
assertThat(approvals.get(1).getAccountId()).isEqualTo(ownerId); assertThat(approvals.get(1).accountId()).isEqualTo(ownerId);
assertThat(approvals.get(1).getLabel()).isEqualTo("Code-Review"); assertThat(approvals.get(1).label()).isEqualTo("Code-Review");
assertThat(approvals.get(1).getValue()).isEqualTo(2); assertThat(approvals.get(1).value()).isEqualTo(2);
assertThat(approvals.get(1).isPostSubmit()).isFalse(); // During submit. assertThat(approvals.get(1).postSubmit()).isFalse(); // During submit.
assertThat(approvals.get(2).getAccountId()).isEqualTo(otherId); assertThat(approvals.get(2).accountId()).isEqualTo(otherId);
assertThat(approvals.get(2).getLabel()).isEqualTo("Other-Label"); assertThat(approvals.get(2).label()).isEqualTo("Other-Label");
assertThat(approvals.get(2).getValue()).isEqualTo(2); assertThat(approvals.get(2).value()).isEqualTo(2);
assertThat(approvals.get(2).isPostSubmit()).isTrue(); assertThat(approvals.get(2).postSubmit()).isTrue();
} }
@Test @Test
@@ -589,8 +589,8 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
ChangeNotes notes = newNotes(c); ChangeNotes notes = newNotes(c);
List<PatchSetApproval> psas = notes.getApprovals().get(c.currentPatchSetId()); List<PatchSetApproval> psas = notes.getApprovals().get(c.currentPatchSetId());
assertThat(psas).hasSize(2); assertThat(psas).hasSize(2);
assertThat(psas.get(0).getAccountId()).isEqualTo(changeOwner.getAccount().getId()); assertThat(psas.get(0).accountId()).isEqualTo(changeOwner.getAccount().getId());
assertThat(psas.get(1).getAccountId()).isEqualTo(otherUser.getAccount().getId()); assertThat(psas.get(1).accountId()).isEqualTo(otherUser.getAccount().getId());
update = newUpdate(c, changeOwner); update = newUpdate(c, changeOwner);
update.removeReviewer(otherUser.getAccount().getId()); update.removeReviewer(otherUser.getAccount().getId());
@@ -599,7 +599,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
notes = newNotes(c); notes = newNotes(c);
psas = notes.getApprovals().get(c.currentPatchSetId()); psas = notes.getApprovals().get(c.currentPatchSetId());
assertThat(psas).hasSize(1); assertThat(psas).hasSize(1);
assertThat(psas.get(0).getAccountId()).isEqualTo(changeOwner.getAccount().getId()); assertThat(psas.get(0).accountId()).isEqualTo(changeOwner.getAccount().getId());
} }
@Test @Test
@@ -1211,13 +1211,13 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
List<PatchSetApproval> psas = notes.getApprovals().get(c.currentPatchSetId()); List<PatchSetApproval> psas = notes.getApprovals().get(c.currentPatchSetId());
assertThat(psas).hasSize(2); assertThat(psas).hasSize(2);
assertThat(psas.get(0).getAccountId()).isEqualTo(changeOwner.getAccount().getId()); assertThat(psas.get(0).accountId()).isEqualTo(changeOwner.getAccount().getId());
assertThat(psas.get(0).getLabel()).isEqualTo("Verified"); assertThat(psas.get(0).label()).isEqualTo("Verified");
assertThat(psas.get(0).getValue()).isEqualTo((short) 1); assertThat(psas.get(0).value()).isEqualTo((short) 1);
assertThat(psas.get(1).getAccountId()).isEqualTo(otherUser.getAccount().getId()); assertThat(psas.get(1).accountId()).isEqualTo(otherUser.getAccount().getId());
assertThat(psas.get(1).getLabel()).isEqualTo("Code-Review"); assertThat(psas.get(1).label()).isEqualTo("Code-Review");
assertThat(psas.get(1).getValue()).isEqualTo((short) 2); assertThat(psas.get(1).value()).isEqualTo((short) 2);
} }
@Test @Test
@@ -1325,11 +1325,11 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
PatchSetApproval approval1 = PatchSetApproval approval1 =
newNotes(c1).getApprovals().get(c1.currentPatchSetId()).iterator().next(); newNotes(c1).getApprovals().get(c1.currentPatchSetId()).iterator().next();
assertThat(approval1.getLabel()).isEqualTo("Verified"); assertThat(approval1.label()).isEqualTo("Verified");
PatchSetApproval approval2 = PatchSetApproval approval2 =
newNotes(c2).getApprovals().get(c2.currentPatchSetId()).iterator().next(); newNotes(c2).getApprovals().get(c2.currentPatchSetId()).iterator().next();
assertThat(approval2.getLabel()).isEqualTo("Code-Review"); assertThat(approval2.label()).isEqualTo("Code-Review");
} }
@Test @Test