opendev/gerrit
Code Issues Proposed changes

Pass RevWalk from BatchUpdate Context to ApprovalCopier

Browse Source 
In NoteDb fused mode, the new patch set object might not be flushed into
the repo at the time when ApprovalCopier is called to copy patch sets.
ChangeKindCache supports passing in a RevWalk and Config for exactly
this use case, but those weren't previously plumbed through
ApprovalCopier. Do so now.

This doesn't affect other NoteDb migration states because in other cases
the object will have been flushed before calling updateChange.

Without this change, the rebase tests in ChangeIT would show a log entry
like:

WARN  com.google.gerrit.server.change.ChangeKindCacheImpl : Cannot check trivial rebase of new patch set 4bf6c821b45761f3cf62a97b9b61ce80171d653b in com.google.gerrit.acceptance.api.change.ChangeIT_rebaseViaRevisionApi_project
java.util.concurrent.ExecutionException: org.eclipse.jgit.errors.MissingObjectException: Missing unknown 4bf6c821b45761f3cf62a97b9b61ce80171d653b
	...
	at com.google.gerrit.server.change.ChangeKindCacheImpl.getChangeKind(ChangeKindCacheImpl.java:355)
	at com.google.gerrit.server.ApprovalCopier.getForPatchSet(ApprovalCopier.java:156)
	at com.google.gerrit.server.ApprovalCopier.copy(ApprovalCopier.java:102)
	at com.google.gerrit.server.ApprovalCopier.copy(ApprovalCopier.java:88)
	at com.google.gerrit.server.change.PatchSetInserter.updateChange(PatchSetInserter.java:279)
	at com.google.gerrit.server.change.RebaseChangeOp.updateChange(RebaseChangeOp.java:201)
	at com.google.gerrit.server.update.FusedNoteDbBatchUpdate.executeChangeOps(FusedNoteDbBatchUpdate.java:415)
        ...

This particular log entry turns out to be harmless, because the only
thing this trivial rebase check is used for is copying approvals to the
new patch set, which only happens in ReviewDb anyway. But since the
object flushing issue only happens when we've gone full-NoteDb, failing
to copy these approvals really does no harm, it just causes logspam.
Added some asserts to the existing rebase check anyway.

Despite the fact that there was no correctness issue here, it's still
nicer and could potentially reduce latency on a cache miss if we pass in
the RevWalk we already have.

Finally, rename the ApprovalCopier methods to indicate that copying is
really only a ReviewDb operation, and explicitly short-circuit the call
when NoteDb is the source of truth.

Change-Id: I188829a36081576eff79034cf7a822dbfc7934bc
This commit is contained in:
Dave Borowitz 2017-06-29 16:14:02 -04:00
parent 8a2cd86f68
commit 26aec0d705
17 changed files with 171 additions and 76 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

24
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java
View File

@ -624,12 +624,18 @@ public class ChangeIT extends AbstractDaemonTest {
revision.review(ReviewInput.approve());
revision.submit();
// Add an approval whose score should be copied on trivial rebase
gApi.changes().id(r2.getChangeId()).current().review(ReviewInput.recommend());
String changeId = r2.getChangeId();
// Rebase the second change
rebase.call(changeId);
// Second change should have 2 patch sets
ChangeInfo c2 = gApi.changes().id(changeId).get();
// Second change should have 2 patch sets and an approval
ChangeInfo c2 =
gApi.changes()
.id(changeId)
.get(EnumSet.of(ListChangesOption.CURRENT_REVISION, ListChangesOption.DETAILED_LABELS));
assertThat(c2.revisions.get(c2.currentRevision)._number).isEqualTo(2);
// ...and the committer and description should be correct
@ -643,6 +649,20 @@ public class ChangeIT extends AbstractDaemonTest {
String description = info.revisions.get(info.currentRevision).description;
assertThat(description).isEqualTo("Rebase");
// ...and the approval was copied
LabelInfo cr = c2.labels.get("Code-Review");
assertThat(cr).isNotNull();
assertThat(cr.all).hasSize(1);
assertThat(cr.all.get(0).value).isEqualTo(1);
if (notesMigration.changePrimaryStorage() == PrimaryStorage.REVIEW_DB) {
// Ensure record was actually copied under ReviewDb
List<PatchSetApproval> psas =
db.patchSetApprovals().byPatchSet(new PatchSet.Id(new Change.Id(c2._number), 2)).toList();
assertThat(psas).hasSize(1);
assertThat(psas.get(0).getValue()).isEqualTo((short) 1);
}
// Rebasing the second change again should fail
exception.expect(ResourceConflictException.class);
exception.expectMessage("Change is already up to date");

122
gerrit-server/src/main/java/com/google/gerrit/server/ApprovalCopier.java
View File

@ -20,6 +20,7 @@ import static com.google.common.base.Preconditions.checkNotNull;
import com.google.common.collect.HashBasedTable;
import com.google.common.collect.ListMultimap;
import com.google.common.collect.Table;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.data.LabelType;
import com.google.gerrit.extensions.client.ChangeKind;
import com.google.gerrit.reviewdb.client.Account;
@ -27,8 +28,8 @@ import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.PatchSetApproval;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.change.ChangeKindCache;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.LabelNormalizer;
import com.google.gerrit.server.notedb.NoteDbChangeState.PrimaryStorage;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.ProjectState;
@ -41,8 +42,8 @@ import java.util.Collection;
import java.util.Collections;
import java.util.List;
import java.util.TreeMap;
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.revwalk.RevWalk;
/**
@ -53,7 +54,6 @@ import org.eclipse.jgit.revwalk.RevWalk;
*/
@Singleton
public class ApprovalCopier {
private final GitRepositoryManager repoManager;
private final ProjectCache projectCache;
private final ChangeKindCache changeKindCache;
private final LabelNormalizer labelNormalizer;
@ -62,13 +62,11 @@ public class ApprovalCopier {
@Inject
ApprovalCopier(
GitRepositoryManager repoManager,
ProjectCache projectCache,
ChangeKindCache changeKindCache,
LabelNormalizer labelNormalizer,
ChangeData.Factory changeDataFactory,
PatchSetUtil psUtil) {
this.repoManager = repoManager;
this.projectCache = projectCache;
this.changeKindCache = changeKindCache;
this.labelNormalizer = labelNormalizer;
@ -82,10 +80,18 @@ public class ApprovalCopier {
* @param db review database.
* @param ctl change control for user uploading PatchSet
* @param ps new PatchSet
* @param rw open walk that can read the patch set commit; null to open the repo on demand.
* @param repoConfig repo config used for change kind detection; null to read from repo on demand.
* @throws OrmException
*/
public void copy(ReviewDb db, ChangeControl ctl, PatchSet ps) throws OrmException {
copy(db, ctl, ps, Collections.<PatchSetApproval>emptyList());
public void copyInReviewDb(
ReviewDb db,
ChangeControl ctl,
PatchSet ps,
@Nullable RevWalk rw,
@Nullable Config repoConfig)
throws OrmException {
copyInReviewDb(db, ctl, ps, rw, repoConfig, Collections.emptyList());
}
/**
@ -94,31 +100,56 @@ public class ApprovalCopier {
* @param db review database.
* @param ctl change control for user uploading PatchSet
* @param ps new PatchSet
* @param rw open walk that can read the patch set commit; null to open the repo on demand.
* @param repoConfig repo config used for change kind detection; null to read from repo on demand.
* @param dontCopy PatchSetApprovals indicating which (account, label) pairs should not be copied
* @throws OrmException
*/
public void copy(ReviewDb db, ChangeControl ctl, PatchSet ps, Iterable<PatchSetApproval> dontCopy)
public void copyInReviewDb(
ReviewDb db,
ChangeControl ctl,
PatchSet ps,
@Nullable RevWalk rw,
@Nullable Config repoConfig,
Iterable<PatchSetApproval> dontCopy)
throws OrmException {
db.patchSetApprovals().insert(getForPatchSet(db, ctl, ps, dontCopy));
}
Iterable<PatchSetApproval> getForPatchSet(ReviewDb db, ChangeControl ctl, PatchSet.Id psId)
throws OrmException {
return getForPatchSet(db, ctl, psId, Collections.<PatchSetApproval>emptyList());
if (PrimaryStorage.of(ctl.getChange()) == PrimaryStorage.REVIEW_DB) {
db.patchSetApprovals().insert(getForPatchSet(db, ctl, ps, rw, repoConfig, dontCopy));
}
}
Iterable<PatchSetApproval> getForPatchSet(
ReviewDb db, ChangeControl ctl, PatchSet.Id psId, Iterable<PatchSetApproval> dontCopy)
ReviewDb db,
ChangeControl ctl,
PatchSet.Id psId,
@Nullable RevWalk rw,
@Nullable Config repoConfig)
throws OrmException {
return getForPatchSet(db, ctl, psId, rw, repoConfig, Collections.<PatchSetApproval>emptyList());
}
Iterable<PatchSetApproval> getForPatchSet(
ReviewDb db,
ChangeControl ctl,
PatchSet.Id psId,
@Nullable RevWalk rw,
@Nullable Config repoConfig,
Iterable<PatchSetApproval> dontCopy)
throws OrmException {
PatchSet ps = psUtil.get(db, ctl.getNotes(), psId);
if (ps == null) {
return Collections.emptyList();
}
return getForPatchSet(db, ctl, ps, dontCopy);
return getForPatchSet(db, ctl, ps, rw, repoConfig, dontCopy);
}
private Iterable<PatchSetApproval> getForPatchSet(
ReviewDb db, ChangeControl ctl, PatchSet ps, Iterable<PatchSetApproval> dontCopy)
ReviewDb db,
ChangeControl ctl,
PatchSet ps,
@Nullable RevWalk rw,
@Nullable Config repoConfig,
Iterable<PatchSetApproval> dontCopy)
throws OrmException {
checkNotNull(ps, "ps should not be null");
ChangeData cd = changeDataFactory.create(db, ctl);
@ -141,41 +172,38 @@ public class ApprovalCopier {
TreeMap<Integer, PatchSet> patchSets = getPatchSets(cd);
try (Repository repo = repoManager.openRepository(project.getProject().getNameKey());
RevWalk rw = new RevWalk(repo)) {
// Walk patch sets strictly less than current in descending order.
Collection<PatchSet> allPrior =
patchSets.descendingMap().tailMap(ps.getId().get(), false).values();
for (PatchSet priorPs : allPrior) {
List<PatchSetApproval> priorApprovals = all.get(priorPs.getId());
if (priorApprovals.isEmpty()) {
// Walk patch sets strictly less than current in descending order.
Collection<PatchSet> allPrior =
patchSets.descendingMap().tailMap(ps.getId().get(), false).values();
for (PatchSet priorPs : allPrior) {
List<PatchSetApproval> priorApprovals = all.get(priorPs.getId());
if (priorApprovals.isEmpty()) {
continue;
}
ChangeKind kind =
changeKindCache.getChangeKind(
project.getProject().getNameKey(),
rw,
repoConfig,
ObjectId.fromString(priorPs.getRevision().get()),
ObjectId.fromString(ps.getRevision().get()));
for (PatchSetApproval psa : priorApprovals) {
if (wontCopy.contains(psa.getLabel(), psa.getAccountId())) {
continue;
}
ChangeKind kind =
changeKindCache.getChangeKind(
project.getProject().getNameKey(),
rw,
repo.getConfig(),
ObjectId.fromString(priorPs.getRevision().get()),
ObjectId.fromString(ps.getRevision().get()));
for (PatchSetApproval psa : priorApprovals) {
if (wontCopy.contains(psa.getLabel(), psa.getAccountId())) {
continue;
}
if (byUser.contains(psa.getLabel(), psa.getAccountId())) {
continue;
}
if (!canCopy(project, psa, ps.getId(), kind)) {
wontCopy.put(psa.getLabel(), psa.getAccountId(), psa);
continue;
}
byUser.put(psa.getLabel(), psa.getAccountId(), copy(psa, ps.getId()));
if (byUser.contains(psa.getLabel(), psa.getAccountId())) {
continue;
}
if (!canCopy(project, psa, ps.getId(), kind)) {
wontCopy.put(psa.getLabel(), psa.getAccountId(), psa);
continue;
}
byUser.put(psa.getLabel(), psa.getAccountId(), copy(psa, ps.getId()));
}
return labelNormalizer.normalize(ctl, byUser.values()).getNormalized();
}
return labelNormalizer.normalize(ctl, byUser.values()).getNormalized();
} catch (IOException e) {
throw new OrmException(e);
}

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

@ -28,6 +28,7 @@ import com.google.common.collect.Lists;
import com.google.common.collect.Ordering;
import com.google.common.collect.Sets;
import com.google.common.primitives.Shorts;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.data.LabelType;
import com.google.gerrit.common.data.LabelTypes;
import com.google.gerrit.common.data.Permission;
@ -60,6 +61,8 @@ import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.revwalk.RevWalk;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@ -376,20 +379,31 @@ public class ApprovalsUtil {
return notes.load().getApprovals();
}
public Iterable<PatchSetApproval> byPatchSet(ReviewDb db, ChangeControl ctl, PatchSet.Id psId)
public Iterable<PatchSetApproval> byPatchSet(
ReviewDb db,
ChangeControl ctl,
PatchSet.Id psId,
@Nullable RevWalk rw,
@Nullable Config repoConfig)
throws OrmException {
if (!migration.readChanges()) {
return sortApprovals(db.patchSetApprovals().byPatchSet(psId));
}
return copier.getForPatchSet(db, ctl, psId);
return copier.getForPatchSet(db, ctl, psId, rw, repoConfig);
}
public Iterable<PatchSetApproval> byPatchSetUser(
ReviewDb db, ChangeControl ctl, PatchSet.Id psId, Account.Id accountId) throws OrmException {
ReviewDb db,
ChangeControl ctl,
PatchSet.Id psId,
Account.Id accountId,
@Nullable RevWalk rw,
@Nullable Config repoConfig)
throws OrmException {
if (!migration.readChanges()) {
return sortApprovals(db.patchSetApprovals().byPatchSetUser(psId, accountId));
}
return filterApprovals(byPatchSet(db, ctl, psId), accountId);
return filterApprovals(byPatchSet(db, ctl, psId, rw, repoConfig), accountId);
}
public PatchSetApproval getSubmitter(ReviewDb db, ChangeNotes notes, PatchSet.Id c) {

2
gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java
View File

@ -1068,7 +1068,7 @@ public class ChangeJson {
Map<String, Short> result = new HashMap<>();
for (PatchSetApproval psa :
approvalsUtil.byPatchSetUser(
db.get(), ctl, cd.change().currentPatchSetId(), user.getAccountId())) {
db.get(), ctl, cd.change().currentPatchSetId(), user.getAccountId(), null, null)) {
result.put(psa.getLabel(), psa.getValue());
}
return result;

7
gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteVote.java
View File

@ -53,6 +53,7 @@ import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
import java.io.IOException;
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
@ -142,7 +143,7 @@ public class DeleteVote extends RetryingRestModifyView<VoteResource, DeleteVoteI
@Override
public boolean updateChange(ChangeContext ctx)
throws OrmException, AuthException, ResourceNotFoundException {
throws OrmException, AuthException, ResourceNotFoundException, IOException {
ChangeControl ctl = ctx.getControl();
change = ctl.getChange();
PatchSet.Id psId = change.currentPatchSetId();
@ -151,7 +152,9 @@ public class DeleteVote extends RetryingRestModifyView<VoteResource, DeleteVoteI
boolean found = false;
LabelTypes labelTypes = ctx.getControl().getLabelTypes();
for (PatchSetApproval a : approvalsUtil.byPatchSetUser(ctx.getDb(), ctl, psId, accountId)) {
for (PatchSetApproval a :
approvalsUtil.byPatchSetUser(
ctx.getDb(), ctl, psId, accountId, ctx.getRevWalk(), ctx.getRepoView().getConfig())) {
if (labelTypes.byLabel(a.getLabelId()) == null) {
continue; // Ignore undefined labels.
} else if (!a.getLabel().equals(label)) {

3
gerrit-server/src/main/java/com/google/gerrit/server/change/PatchSetInserter.java
View File

@ -276,7 +276,8 @@ public class PatchSetInserter implements BatchUpdateOp {
}
change.setCurrentPatchSet(patchSetInfo);
if (copyApprovals) {
approvalCopier.copy(db, ctl, patchSet);
approvalCopier.copyInReviewDb(
db, ctl, patchSet, ctx.getRevWalk(), ctx.getRepoView().getConfig());
}
if (changeMessage != null) {
cmUtil.addChangeMessage(db, update, changeMessage);

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

@ -789,7 +789,7 @@ public class PostReview
@Override
public boolean updateChange(ChangeContext ctx)
throws OrmException, ResourceConflictException, UnprocessableEntityException {
throws OrmException, ResourceConflictException, UnprocessableEntityException, IOException {
user = ctx.getIdentifiedUser();
notes = ctx.getNotes();
ps = psUtil.get(ctx.getDb(), ctx.getNotes(), psId);
@ -1062,7 +1062,8 @@ public class PostReview
return false;
}
private boolean updateLabels(ChangeContext ctx) throws OrmException, ResourceConflictException {
private boolean updateLabels(ChangeContext ctx)
throws OrmException, ResourceConflictException, IOException {
Map<String, Short> inLabels =
MoreObjects.firstNonNull(in.labels, Collections.<String, Short>emptyMap());
@ -1242,12 +1243,18 @@ public class PostReview
}
private Map<String, PatchSetApproval> scanLabels(ChangeContext ctx, List<PatchSetApproval> del)
throws OrmException {
throws OrmException, IOException {
LabelTypes labelTypes = ctx.getControl().getLabelTypes();
Map<String, PatchSetApproval> current = new HashMap<>();
for (PatchSetApproval a :
approvalsUtil.byPatchSetUser(ctx.getDb(), ctx.getControl(), psId, user.getAccountId())) {
approvalsUtil.byPatchSetUser(
ctx.getDb(),
ctx.getControl(),
psId,
user.getAccountId(),
ctx.getRevWalk(),
ctx.getRepoView().getConfig())) {
if (a.isLegacySubmit()) {
continue;
}

3
gerrit-server/src/main/java/com/google/gerrit/server/change/ReviewerJson.java
View File

@ -103,7 +103,8 @@ public class ReviewerJson {
out,
perm,
cd,
approvalsUtil.byPatchSetUser(db.get(), ctl, psId, new Account.Id(out._accountId)));
approvalsUtil.byPatchSetUser(
db.get(), ctl, psId, new Account.Id(out._accountId), null, null));
}
public ReviewerInfo format(

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

@ -86,7 +86,9 @@ public class Votes implements ChildCollection<ReviewerResource, VoteResource> {
db.get(),
rsrc.getControl(),
rsrc.getChange().currentPatchSetId(),
rsrc.getReviewerUser().getAccountId());
rsrc.getReviewerUser().getAccountId(),
null,
null);
for (PatchSetApproval psa : byPatchSetUser) {
votes.put(psa.getLabel(), psa.getValue());
}

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

@ -450,7 +450,7 @@ public class MergeUtil {
private Iterable<PatchSetApproval> safeGetApprovals(ChangeControl ctl, PatchSet.Id psId) {
try {
return approvalsUtil.byPatchSet(db.get(), ctl, psId);
return approvalsUtil.byPatchSet(db.get(), ctl, psId, null, null);
} catch (OrmException e) {
log.error("Can't read approval records for " + psId, e);
return Collections.emptyList();

19
gerrit-server/src/main/java/com/google/gerrit/server/git/ReplaceOp.java
View File

@ -303,7 +303,13 @@ public class ReplaceOp implements BatchUpdateOp {
newPatchSet,
ctx.getControl(),
approvals);
approvalCopier.copy(ctx.getDb(), ctx.getControl(), newPatchSet, newApprovals);
approvalCopier.copyInReviewDb(
ctx.getDb(),
ctx.getControl(),
newPatchSet,
ctx.getRevWalk(),
ctx.getRepoView().getConfig(),
newApprovals);
approvalsUtil.addReviewers(
ctx.getDb(),
update,
@ -336,7 +342,7 @@ public class ReplaceOp implements BatchUpdateOp {
}
private ChangeMessage createChangeMessage(ChangeContext ctx, String reviewMessage)
throws OrmException {
throws OrmException, IOException {
String approvalMessage =
ApprovalsUtil.renderMessageWithApprovals(
patchSetId.get(), approvals, scanLabels(ctx, approvals));
@ -379,13 +385,18 @@ public class ReplaceOp implements BatchUpdateOp {
}
private Map<String, PatchSetApproval> scanLabels(ChangeContext ctx, Map<String, Short> approvals)
throws OrmException {
throws OrmException, IOException {
Map<String, PatchSetApproval> current = new HashMap<>();
// We optimize here and only retrieve current when approvals provided
if (!approvals.isEmpty()) {
for (PatchSetApproval a :
approvalsUtil.byPatchSetUser(
ctx.getDb(), ctx.getControl(), priorPatchSetId, ctx.getAccountId())) {
ctx.getDb(),
ctx.getControl(),
priorPatchSetId,
ctx.getAccountId(),
ctx.getRevWalk(),
ctx.getRepoView().getConfig())) {
if (a.isLegacySubmit()) {
continue;
}

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

@ -329,7 +329,8 @@ abstract class SubmitStrategyOp implements BatchUpdateOp {
null);
}
private void setApproval(ChangeContext ctx, IdentifiedUser user) throws OrmException {
private void setApproval(ChangeContext ctx, IdentifiedUser user)
throws OrmException, IOException {
Change.Id id = ctx.getChange().getId();
List<SubmitRecord> records = args.commitStatus.getSubmitRecords(id);
PatchSet.Id oldPsId = toMerge.getPatchsetId();
@ -351,11 +352,12 @@ abstract class SubmitStrategyOp implements BatchUpdateOp {
}
private LabelNormalizer.Result approve(ChangeContext ctx, ChangeUpdate update)
throws OrmException {
throws OrmException, IOException {
PatchSet.Id psId = update.getPatchSetId();
Map<PatchSetApproval.Key, PatchSetApproval> byKey = new HashMap<>();
for (PatchSetApproval psa :
args.approvalsUtil.byPatchSet(ctx.getDb(), ctx.getControl(), psId)) {
args.approvalsUtil.byPatchSet(
ctx.getDb(), ctx.getControl(), psId, ctx.getRevWalk(), ctx.getRepoView().getConfig())) {
byKey.put(psa.getKey(), psa);
}

8
gerrit-server/src/main/java/com/google/gerrit/server/mail/receive/MailProcessor.java
View File

@ -325,7 +325,13 @@ public class MailProcessor {
// Get previous approvals from this user
Map<String, Short> approvals = new HashMap<>();
approvalsUtil
.byPatchSetUser(ctx.getDb(), changeControl, psId, ctx.getAccountId())
.byPatchSetUser(
ctx.getDb(),
changeControl,
psId,
ctx.getAccountId(),
ctx.getRevWalk(),
ctx.getRepoView().getConfig())
.forEach(a -> approvals.put(a.getLabel(), a.getValue()));
// Fire Gerrit event. Note that approvals can't be granted via email, so old and new approvals
// are always the same here.

2
gerrit-server/src/main/java/com/google/gerrit/server/mail/send/MergedSender.java
View File

@ -70,7 +70,7 @@ public class MergedSender extends ReplyToChangeSender {
Table<Account.Id, String, PatchSetApproval> neg = HashBasedTable.create();
for (PatchSetApproval ca :
args.approvalsUtil.byPatchSet(
args.db.get(), changeData.changeControl(), patchSet.getId())) {
args.db.get(), changeData.changeControl(), patchSet.getId(), null, null)) {
LabelType lt = labelTypes.byLabel(ca.getLabelId());
if (lt == null) {
continue;

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

@ -342,7 +342,7 @@ public class ChangeControl {
}
for (PatchSetApproval ap :
approvalsUtil.byPatchSet(db, this, getChange().currentPatchSetId())) {
approvalsUtil.byPatchSet(db, this, getChange().currentPatchSetId(), null, null)) {
LabelType type = getLabelTypes().byLabel(ap.getLabel());
if (type != null
&& ap.getValue() == 1

2
gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java
View File

@ -814,7 +814,7 @@ public class ChangeData {
try {
currentApprovals =
ImmutableList.copyOf(
approvalsUtil.byPatchSet(db, changeControl(), c.currentPatchSetId()));
approvalsUtil.byPatchSet(db, changeControl(), c.currentPatchSetId(), null, null));
} catch (OrmException e) {
if (e.getCause() instanceof NoSuchChangeException) {
currentApprovals = Collections.emptyList();

2
plugins/reviewnotes

@ -1 +1 @@
Subproject commit f0930e9dfc56644105cb21e7246096097eeaa385
Subproject commit be803eb40fdcd5bfa11d9a808863585f86228e06