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
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 df2b5cc..0b18315 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 @@
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 @@
//
// 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 @@
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 977ac32..decda283 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
@@ -302,6 +302,42 @@
}
@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);
int id = c.getId().get();
