Merge "ChangeBundle: Allow ReviewDb subject to be prefix of NoteDb"

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

@@ -334,11 +334,18 @@ public class ChangeBundle {
     Change b = bundleB.change;
     String desc = a.getId().equals(b.getId()) ? describe(a.getId()) : "Changes";
 
+    boolean excludeSubject = false;
     boolean excludeOrigSubj = false;
     boolean excludeTopic = false;
     Timestamp aUpdated = a.getLastUpdatedOn();
     Timestamp bUpdated = b.getLastUpdatedOn();
 
+
+    // Ignore subject if the NoteDb subject starts with the ReviewDb subject.
+    // The NoteDb subject is read directly from the commit, whereas the ReviewDb
+    // subject historically may have been truncated to fit in a SQL varchar
+    // column.
+    //
     // Ignore null original subject on the ReviewDb side, as this field is
     // always set in NoteDb.
     //
@@ -346,10 +353,12 @@ public class ChangeBundle {
     //
     // Use max timestamp of all ReviewDb entities when comparing with NoteDb.
     if (bundleA.source == REVIEW_DB && bundleB.source == NOTE_DB) {
+      excludeSubject = b.getSubject().startsWith(a.getSubject());
       excludeOrigSubj = a.getOriginalSubjectOrNull() == null;
       excludeTopic = "".equals(a.getTopic()) && b.getTopic() == null;
       aUpdated = bundleA.getLatestTimestamp();
     } else if (bundleA.source == NOTE_DB && bundleB.source == REVIEW_DB) {
+      excludeSubject = a.getSubject().startsWith(b.getSubject());
       excludeOrigSubj = b.getOriginalSubjectOrNull() == null;
       excludeTopic = a.getTopic() == null && "".equals(b.getTopic());
       bUpdated = bundleB.getLatestTimestamp();
@@ -358,6 +367,9 @@ public class ChangeBundle {
     String updatedField = "lastUpdatedOn";
     List<String> exclude =
         Lists.newArrayList(updatedField, "noteDbState", "rowVersion");
+    if (excludeSubject) {
+      exclude.add("subject");
+    }
     if (excludeOrigSubj) {
       exclude.add("originalSubject");
     }

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

@@ -301,6 +301,42 @@ public class ChangeBundleTest {
     assertNoDiffs(b1, b2);
   }
 
+  @Test
+  public void diffChangesAllowsReviewDbSubjectToBePrefixOfNoteDbSubject()
+      throws Exception {
+    Change c1 = TestChanges.newChange(
+        new Project.NameKey("project"), new Account.Id(100));
+    Change c2 = clone(c1);
+    c2.setCurrentPatchSet(c1.currentPatchSetId(),
+        c1.getSubject().substring(0, 10), c1.getOriginalSubject());
+    assertThat(c2.getSubject()).isNotEqualTo(c1.getSubject());
+
+    // 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,
+        "subject differs for Change.Id " + c1.getId() + ":"
+            + " {Change subject} != {Change sub}");
+
+    // ReviewDb has shorter subject, allowed.
+    b1 = new ChangeBundle(c1, messages(), patchSets(), approvals(),
+        comments(), NOTE_DB);
+    b2 = new ChangeBundle(c2, messages(), patchSets(), approvals(),
+        comments(), REVIEW_DB);
+    assertNoDiffs(b1, b2);
+
+    // NoteDb has shorter subject, not allowed.
+    b1 = new ChangeBundle(c1, messages(), patchSets(), approvals(),
+        comments(), REVIEW_DB);
+    b2 = new ChangeBundle(c2, messages(), patchSets(), approvals(),
+        comments(), NOTE_DB);
+    assertDiffs(b1, b2,
+        "subject differs for Change.Id " + c1.getId() + ":"
+            + " {Change subject} != {Change sub}");
+  }
+
   @Test
   public void diffChangeMessageKeySets() throws Exception {
     Change c = TestChanges.newChange(project, accountId);