opendev/gerrit
Code Issues Proposed changes

ChangeBundle: Handle leading whitespace in ReviewDb topics

Browse Source 
JGit's footer parser trims leading whitespace, so a topic with leading
whitespace in ReviewDb is trimmed in ReviewDb.

Change-Id: I9566c67227b1b46a202bd33b0cc924d3c0a798ca
This commit is contained in:
Dave Borowitz
2016-05-27 19:24:19 -04:00
parent 11be4eae7e
commit f85a569184
2 changed files with 58 additions and 2 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -489,7 +489,9 @@ public class ChangeBundle {
aSubj = s.trimLeadingFrom(aSubj);
excludeSubject = bSubj.startsWith(aSubj);
excludeOrigSubj = true;
excludeTopic = "".equals(a.getTopic()) && b.getTopic() == null;
String aTopic = trimLeadingOrNull(a.getTopic());
excludeTopic = Objects.equals(aTopic, b.getTopic())
|| "".equals(aTopic) && b.getTopic() == null;
aUpdated = bundleA.getLatestTimestamp();
} else if (bundleA.source == NOTE_DB && bundleB.source == REVIEW_DB) {
excludeCreatedOn = !timestampsDiffer(
@@ -497,7 +499,9 @@ public class ChangeBundle {
bSubj = s.trimLeadingFrom(bSubj);
excludeSubject = aSubj.startsWith(bSubj);
excludeOrigSubj = true;
excludeTopic = a.getTopic() == null && "".equals(b.getTopic());
String bTopic = trimLeadingOrNull(b.getTopic());
excludeTopic = Objects.equals(bTopic, a.getTopic())
|| a.getTopic() == null && "".equals(bTopic);
bUpdated = bundleB.getLatestTimestamp();
}
@@ -530,6 +534,10 @@ public class ChangeBundle {
}
}
private static String trimLeadingOrNull(String s) {
return s != null ? CharMatcher.whitespace().trimLeadingFrom(s) : null;
}
/**
* Set of fields that must always exactly match between ReviewDb and NoteDb.
* <p>

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

@@ -271,6 +271,54 @@ public class ChangeBundleTest {
assertDiffs(b1, b2,
"topic differs for Change.Id " + c1.getId() + ":"
+ " {topic} != {null}");
// Null is equal to a string that is all whitespace.
Change c4 = clone(c1);
c4.setTopic(" ");
b1 = new ChangeBundle(c4, messages(), patchSets(), approvals(), comments(),
reviewers(), REVIEW_DB);
b2 = new ChangeBundle(c2, messages(), patchSets(), approvals(), comments(),
reviewers(), NOTE_DB);
assertNoDiffs(b1, b2);
assertNoDiffs(b2, b1);
}
@Test
public void diffChangesIgnoresLeadingWhitespaceInReviewDbTopics()
throws Exception {
Change c1 = TestChanges.newChange(
new Project.NameKey("project"), new Account.Id(100));
c1.setTopic(" abc");
Change c2 = clone(c1);
c2.setTopic("abc");
// Both ReviewDb, exact match required.
ChangeBundle b1 = new ChangeBundle(c1, messages(), patchSets(), approvals(),
comments(), reviewers(), REVIEW_DB);
ChangeBundle b2 = new ChangeBundle(c2, messages(), patchSets(), approvals(),
comments(), reviewers(), REVIEW_DB);
assertDiffs(b1, b2,
"topic differs for Change.Id " + c1.getId() + ":"
+ " { abc} != {abc}");
// Leading whitespace in ReviewDb topic is ignored.
b1 = new ChangeBundle(c1, messages(), patchSets(), approvals(), comments(),
reviewers(), REVIEW_DB);
b2 = new ChangeBundle(c2, messages(), patchSets(), approvals(), comments(),
reviewers(), NOTE_DB);
assertNoDiffs(b1, b2);
assertNoDiffs(b2, b1);
// Must match except for the leading whitespace.
Change c3 = clone(c1);
c3.setTopic("cba");
b1 = new ChangeBundle(c1, messages(), patchSets(), approvals(), comments(),
reviewers(), REVIEW_DB);
b2 = new ChangeBundle(c3, messages(), patchSets(), approvals(), comments(),
reviewers(), NOTE_DB);
assertDiffs(b1, b2,
"topic differs for Change.Id " + c1.getId() + ":"
+ " { abc} != {cba}");
}
@Test