opendev/gerrit
Code Issues Proposed changes

ChangeBundle: Ignore null originalSubject in ReviewDb changes

Browse Source 
Original subject is a nullable field added relatively recently, so
there are many changes that don't have it set. In NoteDb we
reconstruct this by looking at the patch set commit, so it is always
present. Don't consider this a difference between ReviewDb and NoteDb.

Change-Id: I143430fed2da531806f2b8dcc17465f827df80d7
This commit is contained in:
Dave Borowitz
2016-04-28 13:25:36 -04:00
parent d72ef55b0d
commit 713d2b2c36
3 changed files with 66 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/Change.java
View File

@@ -572,6 +572,10 @@ public final class Change {
return originalSubject != null ? originalSubject : subject;
}
public String getOriginalSubjectOrNull() {
return originalSubject;
}
/** Get the id of the most current {@link PatchSet} in this change. */
public PatchSet.Id currentPatchSetId() {
if (currentPatchSetId > 0) {

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

@@ -312,8 +312,24 @@ public class ChangeBundle {
Change a = bundleA.change;
Change b = bundleB.change;
String desc = a.getId().equals(b.getId()) ? describe(a.getId()) : "Changes";
boolean excludeOrigSubj = false;
// Ignore null original subject on the ReviewDb side, as this field is
// always set in NoteDb.
if (bundleA.source == REVIEW_DB && bundleB.source == NOTE_DB
&& a.getOriginalSubjectOrNull() == null) {
excludeOrigSubj = true;
} else if (bundleA.source == NOTE_DB && bundleB.source == REVIEW_DB
&& b.getOriginalSubjectOrNull() == null) {
excludeOrigSubj = true;
}
List<String> exclude = Lists.newArrayList("rowVersion", "noteDbState");
if (excludeOrigSubj) {
exclude.add("originalSubject");
}
diffColumnsExcluding(diffs, Change.class, desc, bundleA, a, bundleB, b,
"rowVersion", "noteDbState");
exclude);
}
private static void diffChangeMessages(List<String> diffs,

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

@@ -181,6 +181,51 @@ public class ChangeBundleTest {
assertDiffs(b3, b1, msg);
}
@Test
public void diffChangesIgnoresNullOriginalSubjectInReviewDb()
throws Exception {
Change c1 = TestChanges.newChange(
new Project.NameKey("project"), new Account.Id(100));
c1.setCurrentPatchSet(c1.currentPatchSetId(), "Subject", null);
Change c2 = clone(c1);
c2.setCurrentPatchSet(
c2.currentPatchSetId(), c1.getSubject(), "Original subject");
// Both ReviewDb, exact match required.
ChangeBundle b1 = new ChangeBundle(c1, messages(), patchSets(), approvals(),
comments(), REVIEW_DB);
ChangeBundle b2 = new ChangeBundle(c2, messages(), patchSets(), approvals(),
comments(), REVIEW_DB);
assertDiffs(b1, b2,
"originalSubject differs for Change.Id " + c1.getId() + ":"
+ " {null} != {Original subject}");
// Both NoteDb, exact match required.
b1 = new ChangeBundle(c1, messages(), patchSets(), approvals(), comments(),
NOTE_DB);
b2 = new ChangeBundle(c2, messages(), patchSets(), approvals(), comments(),
NOTE_DB);
assertDiffs(b1, b2,
"originalSubject differs for Change.Id " + c1.getId() + ":"
+ " {null} != {Original subject}");
// Original subject ignored if ReviewDb has null value.
b1 = new ChangeBundle(c1, messages(), patchSets(), approvals(), comments(),
REVIEW_DB);
b2 = new ChangeBundle(c2, messages(), patchSets(), approvals(), comments(),
NOTE_DB);
assertNoDiffs(b1, b2);
// Exact match still required if NoteDb has null value (not realistic).
b1 = new ChangeBundle(c1, messages(), patchSets(), approvals(), comments(),
NOTE_DB);
b2 = new ChangeBundle(c2, messages(), patchSets(), approvals(), comments(),
REVIEW_DB);
assertDiffs(b1, b2,
"originalSubject differs for Change.Id " + c1.getId() + ":"
+ " {null} != {Original subject}");
}
@Test
public void diffChangeMessageKeySets() throws Exception {
Change c = TestChanges.newChange(project, accountId);