From 9a5fc785edc2ebd270f0db28728469a9f4c30936 Mon Sep 17 00:00:00 2001
From: Dave Borowitz <dborowitz@google.com>
Date: Tue, 3 May 2016 19:47:50 -0400
Subject: [PATCH] ChangeBundle: Allow ReviewDb subject to be prefix of NoteDb

Older versions of Gerrit had a maximum length for the subject column,
so some subjects may have been truncated. NoteDb on the other hand
always takes the full subject line as reported by JGit. So, consider
subjects equivalent if the ReviewDb subject is a prefix of the NoteDb
subject.

Change-Id: Id309607a7023c12c2146a53cce9bffd8bfd627fe
---
 .../gerrit/server/notedb/ChangeBundle.java    | 12 +++++++
 .../server/notedb/ChangeBundleTest.java       | 36 +++++++++++++++++++
 2 files changed, 48 insertions(+)

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 df2b5ccd82..0b183158ab 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
@@ -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");
     }
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 977ac325f2..decda283b3 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
@@ -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);