opendev/gerrit
Code Issues Proposed changes

ChangeBundle: Fix ChangeMessage comparison for extra patch sets

Browse Source 
We were only excluding messages for patch sets that don't exist in the
full, unfiltered set of PatchSet.Ids. But there might be extraneous
patch sets that actually do exist in the bundle, but are greater than
the current patch set ID on the change. We were already filtering
these out for other entity types in limitToValidPatchSets, just not
for ChangeBundle, which uses a slightly different implementation.

Change-Id: Id1a0a9462f4096deee7a1d1a6eb673bc37529958
This commit is contained in:
Dave Borowitz
2016-05-27 20:12:03 -04:00
parent f85a569184
commit cb4c481f77
2 changed files with 38 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeBundle.java
View File

@@ -403,12 +403,16 @@ public class ChangeBundle {
}
private Collection<ChangeMessage> filterChangeMessages() {
final Predicate<PatchSet.Id> upToCurrent = upToCurrentPredicate();
return Collections2.filter(changeMessages,
new Predicate<ChangeMessage>() {
@Override
public boolean apply(ChangeMessage in) {
PatchSet.Id psId = in.getPatchSetId();
return psId == null || patchSets.containsKey(psId);
if (psId == null) {
return true;
}
return upToCurrent.apply(psId) && patchSets.containsKey(psId);
}
});
}

33
gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeBundleTest.java
View File

@@ -703,6 +703,39 @@ public class ChangeBundleTest {
+ "Only in B:\n " + cm1);
}
@Test
public void diffChangeMessagesIgnoresMessagesOnPatchSetGreaterThanCurrent()
throws Exception {
Change c = TestChanges.newChange(project, accountId);
PatchSet ps1 = new PatchSet(new PatchSet.Id(c.getId(), 1));
ps1.setRevision(new RevId("deadbeefdeadbeefdeadbeefdeadbeefdeadbeef"));
ps1.setUploader(accountId);
ps1.setCreatedOn(TimeUtil.nowTs());
PatchSet ps2 = new PatchSet(new PatchSet.Id(c.getId(), 2));
ps2.setRevision(new RevId("badc0feebadc0feebadc0feebadc0feebadc0fee"));
ps2.setUploader(accountId);
ps2.setCreatedOn(TimeUtil.nowTs());
assertThat(c.currentPatchSetId()).isEqualTo(ps1.getId());
ChangeMessage cm1 = new ChangeMessage(
new ChangeMessage.Key(c.getId(), "uuid1"),
accountId, TimeUtil.nowTs(), ps1.getId());
cm1.setMessage("a message");
ChangeMessage cm2 = new ChangeMessage(
new ChangeMessage.Key(c.getId(), "uuid2"),
accountId, TimeUtil.nowTs(), ps2.getId());
cm2.setMessage("other message");
ChangeBundle b1 = new ChangeBundle(c, messages(cm1, cm2),
patchSets(ps1, ps2), approvals(), comments(), reviewers(), REVIEW_DB);
ChangeBundle b2 = new ChangeBundle(c, messages(cm1), patchSets(ps1),
approvals(), comments(), reviewers(), NOTE_DB);
assertNoDiffs(b1, b2);
assertNoDiffs(b2, b1);
}
@Test
public void diffPatchSetIdSets() throws Exception {
Change c = TestChanges.newChange(project, accountId);