diff --git a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/Change.java b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/Change.java index 26bab58bdd..863ed0870f 100644 --- a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/Change.java +++ b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/Change.java @@ -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) { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeBundle.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeBundle.java index e6332ea0a4..8654834868 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeBundle.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeBundle.java @@ -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 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 diffs, diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeBundleTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeBundleTest.java index ea12ecaffe..f3a6698046 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeBundleTest.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeBundleTest.java @@ -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);