Merge changes from topic 'notedb-diff-fixes'
* changes: ChangeRebuilderImpl: Ensure first event creates change BatchUpdate: Prevent lastUpdatedOn from going backwards TestNotesMigration: Always set values in setFromEnv ChangeBundle: Handle empty topic on the ReviewDb side ChangeBundle: Ignore null originalSubject in ReviewDb changes NoteDb: Fix conversion of PatchSet.pushCertificate field
This commit is contained in:
@@ -231,6 +231,8 @@ public class CommentsIT extends AbstractDaemonTest {
|
||||
PushOneCommit.Result r = push.to("refs/for/master");
|
||||
String changeId = r.getChangeId();
|
||||
String revId = r.getCommit().getName();
|
||||
Timestamp origLastUpdated = r.getChange().change().getLastUpdatedOn();
|
||||
|
||||
ReviewInput input = new ReviewInput();
|
||||
CommentInput comment = newComment(file, Side.REVISION, line, "comment 1");
|
||||
comment.updated = timestamp;
|
||||
@@ -248,6 +250,10 @@ public class CommentsIT extends AbstractDaemonTest {
|
||||
assertCommentInfo(comment, actual);
|
||||
assertThat(actual.updated)
|
||||
.isEqualTo(gApi.changes().id(r.getChangeId()).info().created);
|
||||
|
||||
// Updating historic comments doesn't cause lastUpdatedOn to regress.
|
||||
assertThat(r.getChange().change().getLastUpdatedOn())
|
||||
.isEqualTo(origLastUpdated);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -36,7 +36,9 @@ import com.google.gerrit.reviewdb.client.RefNames;
|
||||
import com.google.gerrit.reviewdb.server.ReviewDb;
|
||||
import com.google.gerrit.server.ChangeUtil;
|
||||
import com.google.gerrit.server.PatchLineCommentsUtil;
|
||||
import com.google.gerrit.server.change.PostReview;
|
||||
import com.google.gerrit.server.change.Rebuild;
|
||||
import com.google.gerrit.server.change.RevisionResource;
|
||||
import com.google.gerrit.server.config.AllUsersName;
|
||||
import com.google.gerrit.server.notedb.ChangeBundle;
|
||||
import com.google.gerrit.server.notedb.ChangeNoteUtil;
|
||||
@@ -79,6 +81,9 @@ public class ChangeRebuilderIT extends AbstractDaemonTest {
|
||||
@Inject
|
||||
private PatchLineCommentsUtil plcUtil;
|
||||
|
||||
@Inject
|
||||
private Provider<PostReview> postReview;
|
||||
|
||||
@Before
|
||||
public void setUp() {
|
||||
assume().that(NoteDbMode.readWrite()).isFalse();
|
||||
@@ -358,6 +363,102 @@ public class ChangeRebuilderIT extends AbstractDaemonTest {
|
||||
assertDraftsUpToDate(true, id, user);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void pushCert() throws Exception {
|
||||
// We don't have the code in our test harness to do signed pushes, so just
|
||||
// use a hard-coded cert. This cert was actually generated by C git 2.2.0
|
||||
// (albeit not for sending to Gerrit).
|
||||
String cert = "certificate version 0.1\n"
|
||||
+ "pusher Dave Borowitz <dborowitz@google.com> 1433954361 -0700\n"
|
||||
+ "pushee git://localhost/repo.git\n"
|
||||
+ "nonce 1433954361-bde756572d665bba81d8\n"
|
||||
+ "\n"
|
||||
+ "0000000000000000000000000000000000000000"
|
||||
+ "b981a177396fb47345b7df3e4d3f854c6bea7"
|
||||
+ "s/heads/master\n"
|
||||
+ "-----BEGIN PGP SIGNATURE-----\n"
|
||||
+ "Version: GnuPG v1\n"
|
||||
+ "\n"
|
||||
+ "iQEcBAABAgAGBQJVeGg5AAoJEPfTicJkUdPkUggH/RKAeI9/i/LduuiqrL/SSdIa\n"
|
||||
+ "9tYaSqJKLbXz63M/AW4Sp+4u+dVCQvnAt/a35CVEnpZz6hN4Kn/tiswOWVJf4CO7\n"
|
||||
+ "htNubGs5ZMwvD6sLYqKAnrM3WxV/2TbbjzjZW6Jkidz3jz/WRT4SmjGYiEO7aA+V\n"
|
||||
+ "4ZdIS9f7sW5VsHHYlNThCA7vH8Uu48bUovFXyQlPTX0pToSgrWV3JnTxDNxfn3iG\n"
|
||||
+ "IL0zTY/qwVCdXgFownLcs6J050xrrBWIKqfcWr3u4D2aCLyR0v+S/KArr7ulZygY\n"
|
||||
+ "+SOklImn8TAZiNxhWtA6ens66IiammUkZYFv7SSzoPLFZT4dC84SmGPWgf94NoQ=\n"
|
||||
+ "=XFeC\n"
|
||||
+ "-----END PGP SIGNATURE-----\n";
|
||||
|
||||
PushOneCommit.Result r = createChange();
|
||||
PatchSet.Id psId = r.getPatchSetId();
|
||||
Change.Id id = psId.getParentKey();
|
||||
|
||||
PatchSet ps = db.patchSets().get(psId);
|
||||
ps.setPushCertificate(cert);
|
||||
db.patchSets().update(Collections.singleton(ps));
|
||||
indexer.index(db, project, id);
|
||||
|
||||
checker.rebuildAndCheckChanges(id);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void emptyTopic() throws Exception {
|
||||
PushOneCommit.Result r = createChange();
|
||||
Change.Id id = r.getPatchSetId().getParentKey();
|
||||
Change c = db.changes().get(id);
|
||||
assertThat(c.getTopic()).isNull();
|
||||
c.setTopic("");
|
||||
db.changes().update(Collections.singleton(c));
|
||||
|
||||
checker.rebuildAndCheckChanges(id);
|
||||
|
||||
notesMigration.setWriteChanges(true);
|
||||
notesMigration.setReadChanges(true);
|
||||
|
||||
// Rebuild and check was successful, but NoteDb doesn't support storing an
|
||||
// empty topic, so it comes out as null.
|
||||
ChangeNotes notes = notesFactory.create(db, project, id);
|
||||
assertThat(notes.getChange().getTopic()).isNull();
|
||||
}
|
||||
|
||||
@Test
|
||||
public void commentBeforeFirstPatchSet() throws Exception {
|
||||
PushOneCommit.Result r = createChange();
|
||||
PatchSet.Id psId = r.getPatchSetId();
|
||||
Change.Id id = psId.getParentKey();
|
||||
|
||||
Change c = db.changes().get(id);
|
||||
c.setCreatedOn(new Timestamp(c.getCreatedOn().getTime() - 5000));
|
||||
db.changes().update(Collections.singleton(c));
|
||||
indexer.index(db, project, id);
|
||||
|
||||
ReviewInput rin = new ReviewInput();
|
||||
rin.message = "comment";
|
||||
|
||||
Timestamp ts = new Timestamp(c.getCreatedOn().getTime() + 2000);
|
||||
RevisionResource revRsrc = parseCurrentRevisionResource(r.getChangeId());
|
||||
postReview.get().apply(revRsrc, rin, ts);
|
||||
|
||||
checker.rebuildAndCheckChanges(id);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void commentPredatingChangeBySomeoneOtherThanOwner() throws Exception {
|
||||
PushOneCommit.Result r = createChange();
|
||||
PatchSet.Id psId = r.getPatchSetId();
|
||||
Change.Id id = psId.getParentKey();
|
||||
Change c = db.changes().get(id);
|
||||
|
||||
ReviewInput rin = new ReviewInput();
|
||||
rin.message = "comment";
|
||||
|
||||
Timestamp ts = new Timestamp(c.getCreatedOn().getTime() - 10000);
|
||||
RevisionResource revRsrc = parseCurrentRevisionResource(r.getChangeId());
|
||||
setApiUser(user);
|
||||
postReview.get().apply(revRsrc, rin, ts);
|
||||
|
||||
checker.rebuildAndCheckChanges(id);
|
||||
}
|
||||
|
||||
private void setInvalidNoteDbState(Change.Id id) throws Exception {
|
||||
ReviewDb db = unwrapDb();
|
||||
Change c = db.changes().get(id);
|
||||
|
||||
@@ -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) {
|
||||
|
||||
@@ -49,7 +49,6 @@ import com.google.gerrit.extensions.restapi.RestApiException;
|
||||
import com.google.gerrit.extensions.restapi.RestModifyView;
|
||||
import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
|
||||
import com.google.gerrit.extensions.restapi.Url;
|
||||
import com.google.gerrit.reviewdb.client.Change;
|
||||
import com.google.gerrit.reviewdb.client.ChangeMessage;
|
||||
import com.google.gerrit.reviewdb.client.CommentRange;
|
||||
import com.google.gerrit.reviewdb.client.Patch;
|
||||
@@ -371,10 +370,6 @@ public class PostReview implements RestModifyView<RevisionResource, ReviewInput>
|
||||
dirty |= insertComments(ctx);
|
||||
dirty |= updateLabels(ctx);
|
||||
dirty |= insertMessage(ctx);
|
||||
Change c = notes.getChange();
|
||||
if (c.getLastUpdatedOn().before(ctx.getWhen())) {
|
||||
c.setLastUpdatedOn(ctx.getWhen());
|
||||
}
|
||||
if (dirty) {
|
||||
ctx.saveChange();
|
||||
}
|
||||
|
||||
@@ -620,7 +620,9 @@ public class BatchUpdate implements AutoCloseable {
|
||||
|
||||
private static Iterable<Change> bumpLastUpdatedOn(ChangeContext ctx) {
|
||||
Change c = ctx.getChange();
|
||||
c.setLastUpdatedOn(ctx.getWhen());
|
||||
if (c.getLastUpdatedOn().before(ctx.getWhen())) {
|
||||
c.setLastUpdatedOn(ctx.getWhen());
|
||||
}
|
||||
return Collections.singleton(c);
|
||||
}
|
||||
|
||||
|
||||
@@ -22,6 +22,7 @@ import static com.google.gerrit.reviewdb.server.ReviewDbUtil.intKeyOrdering;
|
||||
import static com.google.gerrit.server.notedb.ChangeBundle.Source.NOTE_DB;
|
||||
import static com.google.gerrit.server.notedb.ChangeBundle.Source.REVIEW_DB;
|
||||
|
||||
import com.google.common.base.CharMatcher;
|
||||
import com.google.common.base.Joiner;
|
||||
import com.google.common.collect.ComparisonChain;
|
||||
import com.google.common.collect.ImmutableCollection;
|
||||
@@ -311,8 +312,30 @@ 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;
|
||||
boolean excludeTopic = false;
|
||||
// Ignore null original subject on the ReviewDb side, as this field is
|
||||
// always set in NoteDb.
|
||||
//
|
||||
// Ignore empty topic on the ReviewDb side if it is null on the NoteDb side.
|
||||
if (bundleA.source == REVIEW_DB && bundleB.source == NOTE_DB) {
|
||||
excludeOrigSubj = a.getOriginalSubjectOrNull() == null;
|
||||
excludeTopic = "".equals(a.getTopic()) && b.getTopic() == null;
|
||||
} else if (bundleA.source == NOTE_DB && bundleB.source == REVIEW_DB) {
|
||||
excludeOrigSubj = b.getOriginalSubjectOrNull() == null;
|
||||
excludeTopic = a.getTopic() == null && "".equals(b.getTopic());
|
||||
}
|
||||
|
||||
List<String> exclude = Lists.newArrayList("rowVersion", "noteDbState");
|
||||
if (excludeOrigSubj) {
|
||||
exclude.add("originalSubject");
|
||||
}
|
||||
if (excludeTopic) {
|
||||
exclude.add("topic");
|
||||
}
|
||||
diffColumnsExcluding(diffs, Change.class, desc, bundleA, a, bundleB, b,
|
||||
"rowVersion", "noteDbState");
|
||||
exclude);
|
||||
}
|
||||
|
||||
private static void diffChangeMessages(List<String> diffs,
|
||||
@@ -379,10 +402,20 @@ public class ChangeBundle {
|
||||
PatchSet a = as.get(id);
|
||||
PatchSet b = bs.get(id);
|
||||
String desc = describe(id);
|
||||
diffColumns(diffs, PatchSet.class, desc, bundleA, a, bundleB, b);
|
||||
String pushCertField = "pushCertificate";
|
||||
diffColumnsExcluding(diffs, PatchSet.class, desc, bundleA, a, bundleB, b,
|
||||
pushCertField);
|
||||
diffValues(diffs, desc, trimPushCert(a), trimPushCert(b), pushCertField);
|
||||
}
|
||||
}
|
||||
|
||||
private static String trimPushCert(PatchSet ps) {
|
||||
if (ps.getPushCertificate() == null) {
|
||||
return null;
|
||||
}
|
||||
return CharMatcher.is('\n').trimTrailingFrom(ps.getPushCertificate());
|
||||
}
|
||||
|
||||
private static void diffPatchSetApprovals(List<String> diffs,
|
||||
ChangeBundle bundleA, ChangeBundle bundleB) {
|
||||
Map<PatchSetApproval.Key, PatchSetApproval> as = bundleA.patchSetApprovals;
|
||||
|
||||
@@ -240,16 +240,11 @@ public class ChangeRebuilderImpl extends ChangeRebuilder {
|
||||
// Delete ref only after hashtags have been read
|
||||
deleteRef(change, changeMetaRepo, manager.getChangeRepo().cmds);
|
||||
|
||||
Integer minPsNum = null;
|
||||
for (PatchSet ps : bundle.getPatchSets()) {
|
||||
int n = ps.getId().get();
|
||||
if (minPsNum == null || n < minPsNum) {
|
||||
minPsNum = n;
|
||||
}
|
||||
}
|
||||
Integer minPsNum = getMinPatchSetNum(bundle);
|
||||
|
||||
for (PatchSet ps : bundle.getPatchSets()) {
|
||||
events.add(new PatchSetEvent(
|
||||
change, ps, checkNotNull(minPsNum), manager.getChangeRepo().rw));
|
||||
change, ps, manager.getChangeRepo().rw));
|
||||
for (PatchLineComment c : getPatchLineComments(bundle, ps)) {
|
||||
PatchLineCommentEvent e =
|
||||
new PatchLineCommentEvent(c, change, ps, patchListCache);
|
||||
@@ -271,9 +266,7 @@ public class ChangeRebuilderImpl extends ChangeRebuilder {
|
||||
new ChangeMessageEvent(msg, noteDbChange, change.getCreatedOn()));
|
||||
}
|
||||
|
||||
sortEvents(change.getId(), events, minPsNum);
|
||||
|
||||
events.add(new FinalUpdatesEvent(change, noteDbChange));
|
||||
sortAndFillEvents(change, noteDbChange, events, minPsNum);
|
||||
|
||||
EventList<Event> el = new EventList<>();
|
||||
for (Event e : events) {
|
||||
@@ -299,6 +292,17 @@ public class ChangeRebuilderImpl extends ChangeRebuilder {
|
||||
}
|
||||
}
|
||||
|
||||
private static Integer getMinPatchSetNum(ChangeBundle bundle) {
|
||||
Integer minPsNum = null;
|
||||
for (PatchSet ps : bundle.getPatchSets()) {
|
||||
int n = ps.getId().get();
|
||||
if (minPsNum == null || n < minPsNum) {
|
||||
minPsNum = n;
|
||||
}
|
||||
}
|
||||
return minPsNum;
|
||||
}
|
||||
|
||||
private static List<PatchLineComment> getPatchLineComments(ChangeBundle bundle,
|
||||
final PatchSet ps) {
|
||||
return FluentIterable.from(bundle.getPatchLineComments())
|
||||
@@ -310,9 +314,19 @@ public class ChangeRebuilderImpl extends ChangeRebuilder {
|
||||
}).toSortedList(PatchLineCommentsUtil.PLC_ORDER);
|
||||
}
|
||||
|
||||
private void sortEvents(Change.Id changeId, List<Event> events,
|
||||
Integer minPsNum) {
|
||||
private void sortAndFillEvents(Change change, Change noteDbChange,
|
||||
List<Event> events, Integer minPsNum) {
|
||||
Collections.sort(events, EVENT_ORDER);
|
||||
events.add(new FinalUpdatesEvent(change, noteDbChange));
|
||||
|
||||
// Ensure the first event in the list creates the change, setting the author
|
||||
// and any required footers.
|
||||
Event first = events.get(0);
|
||||
if (first instanceof PatchSetEvent && change.getOwner().equals(first.who)) {
|
||||
((PatchSetEvent) first).createChange = true;
|
||||
} else {
|
||||
events.add(0, new CreateChangeEvent(change, minPsNum));
|
||||
}
|
||||
|
||||
// Fill in any missing patch set IDs using the latest patch set of the
|
||||
// change at the time of the event, because NoteDb can't represent actions
|
||||
@@ -327,7 +341,7 @@ public class ChangeRebuilderImpl extends ChangeRebuilder {
|
||||
int ps = firstNonNull(minPsNum, 1);
|
||||
for (Event e : events) {
|
||||
if (e.psId == null) {
|
||||
e.psId = new PatchSet.Id(changeId, ps);
|
||||
e.psId = new PatchSet.Id(change.getId(), ps);
|
||||
} else {
|
||||
ps = Math.max(ps, e.psId.get());
|
||||
}
|
||||
@@ -457,12 +471,17 @@ public class ChangeRebuilderImpl extends ChangeRebuilder {
|
||||
@Override
|
||||
public int compare(Event a, Event b) {
|
||||
return ComparisonChain.start()
|
||||
.compareTrueFirst(a.predatesChange, b.predatesChange)
|
||||
.compare(a.when, b.when)
|
||||
.compareTrueFirst(isPatchSet(a), isPatchSet(b))
|
||||
.compareTrueFirst(a.predatesChange, b.predatesChange)
|
||||
.compare(a.who, b.who, ReviewDbUtil.intKeyOrdering())
|
||||
.compare(a.psId, b.psId, ReviewDbUtil.intKeyOrdering().nullsLast())
|
||||
.result();
|
||||
}
|
||||
|
||||
private boolean isPatchSet(Event e) {
|
||||
return e instanceof PatchSetEvent;
|
||||
}
|
||||
};
|
||||
|
||||
private abstract static class Event {
|
||||
@@ -606,6 +625,46 @@ public class ChangeRebuilderImpl extends ChangeRebuilder {
|
||||
}
|
||||
}
|
||||
|
||||
private static void createChange(ChangeUpdate update, Change change) {
|
||||
update.setSubjectForCommit("Create change");
|
||||
update.setChangeId(change.getKey().get());
|
||||
update.setBranch(change.getDest().get());
|
||||
update.setSubject(change.getOriginalSubject());
|
||||
}
|
||||
|
||||
private static class CreateChangeEvent extends Event {
|
||||
private final Change change;
|
||||
|
||||
private static PatchSet.Id psId(Change change, Integer minPsNum) {
|
||||
int n;
|
||||
if (minPsNum == null) {
|
||||
// There were no patch sets for the change at all, so something is very
|
||||
// wrong. Bail and use 1 as the patch set.
|
||||
n = 1;
|
||||
} else {
|
||||
n = minPsNum;
|
||||
}
|
||||
return new PatchSet.Id(change.getId(), n);
|
||||
}
|
||||
|
||||
CreateChangeEvent(Change change, Integer minPsNum) {
|
||||
super(psId(change, minPsNum), change.getOwner(), change.getCreatedOn(),
|
||||
change.getCreatedOn(), null);
|
||||
this.change = change;
|
||||
}
|
||||
|
||||
@Override
|
||||
boolean uniquePerUpdate() {
|
||||
return true;
|
||||
}
|
||||
|
||||
@Override
|
||||
void apply(ChangeUpdate update) throws IOException, OrmException {
|
||||
checkUpdate(update);
|
||||
createChange(update, change);
|
||||
}
|
||||
}
|
||||
|
||||
private static class ApprovalEvent extends Event {
|
||||
private PatchSetApproval psa;
|
||||
|
||||
@@ -630,16 +689,15 @@ public class ChangeRebuilderImpl extends ChangeRebuilder {
|
||||
private static class PatchSetEvent extends Event {
|
||||
private final Change change;
|
||||
private final PatchSet ps;
|
||||
private final boolean createChange;
|
||||
private final RevWalk rw;
|
||||
private boolean createChange;
|
||||
|
||||
PatchSetEvent(Change change, PatchSet ps, int minPsNum, RevWalk rw) {
|
||||
PatchSetEvent(Change change, PatchSet ps, RevWalk rw) {
|
||||
super(ps.getId(), ps.getUploader(), ps.getCreatedOn(),
|
||||
change.getCreatedOn(), null);
|
||||
this.change = change;
|
||||
this.ps = ps;
|
||||
this.rw = rw;
|
||||
this.createChange = ps.getPatchSetId() == minPsNum;
|
||||
}
|
||||
|
||||
@Override
|
||||
@@ -650,12 +708,10 @@ public class ChangeRebuilderImpl extends ChangeRebuilder {
|
||||
@Override
|
||||
void apply(ChangeUpdate update) throws IOException, OrmException {
|
||||
checkUpdate(update);
|
||||
update.setSubject(change.getSubject());
|
||||
if (createChange) {
|
||||
update.setSubjectForCommit("Create change");
|
||||
update.setChangeId(change.getKey().get());
|
||||
update.setBranch(change.getDest().get());
|
||||
createChange(update, change);
|
||||
} else {
|
||||
update.setSubject(change.getSubject());
|
||||
update.setSubjectForCommit("Create patch set " + ps.getPatchSetId());
|
||||
}
|
||||
setRevision(update, ps);
|
||||
|
||||
@@ -37,7 +37,7 @@ class RevisionNote {
|
||||
"certificate version ".getBytes(UTF_8);
|
||||
// See org.eclipse.jgit.transport.PushCertificateParser.END_SIGNATURE
|
||||
private static final byte[] END_SIGNATURE =
|
||||
"-----END PGP SIGNATURE-----".getBytes(UTF_8);
|
||||
"-----END PGP SIGNATURE-----\n".getBytes(UTF_8);
|
||||
|
||||
private static void trimLeadingEmptyLines(byte[] bytes, MutableInteger p) {
|
||||
while (p.value < bytes.length && bytes[p.value] == '\n') {
|
||||
|
||||
@@ -181,6 +181,97 @@ 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 diffChangesConsidersEmptyReviewDbTopicEquivalentToNullInNoteDb()
|
||||
throws Exception {
|
||||
Change c1 = TestChanges.newChange(
|
||||
new Project.NameKey("project"), new Account.Id(100));
|
||||
c1.setTopic("");
|
||||
Change c2 = clone(c1);
|
||||
c2.setTopic(null);
|
||||
|
||||
// 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,
|
||||
"topic differs for Change.Id " + c1.getId() + ":"
|
||||
+ " {} != {null}");
|
||||
|
||||
// Topic ignored if ReviewDb is empty and NoteDb is null.
|
||||
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 empty 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,
|
||||
"topic differs for Change.Id " + c1.getId() + ":"
|
||||
+ " {} != {null}");
|
||||
|
||||
// Null is not equal to a non-empty string.
|
||||
Change c3 = clone(c1);
|
||||
c3.setTopic("topic");
|
||||
b1 = new ChangeBundle(c3, messages(), patchSets(), approvals(), comments(),
|
||||
REVIEW_DB);
|
||||
b2 = new ChangeBundle(c2, messages(), patchSets(), approvals(), comments(),
|
||||
NOTE_DB);
|
||||
assertDiffs(b1, b2,
|
||||
"topic differs for Change.Id " + c1.getId() + ":"
|
||||
+ " {topic} != {null}");
|
||||
}
|
||||
|
||||
@Test
|
||||
public void diffChangeMessageKeySets() throws Exception {
|
||||
Change c = TestChanges.newChange(project, accountId);
|
||||
@@ -482,6 +573,34 @@ public class ChangeBundleTest {
|
||||
assertDiffs(b3, b1, msg);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void diffPatchSetsIgnoresTrailingNewlinesInPushCertificate()
|
||||
throws Exception {
|
||||
subWindowResolution();
|
||||
Change c = TestChanges.newChange(project, accountId);
|
||||
PatchSet ps1 = new PatchSet(c.currentPatchSetId());
|
||||
ps1.setRevision(new RevId("deadbeefdeadbeefdeadbeefdeadbeefdeadbeef"));
|
||||
ps1.setUploader(accountId);
|
||||
ps1.setCreatedOn(roundToSecond(TimeUtil.nowTs()));
|
||||
ps1.setPushCertificate("some cert");
|
||||
PatchSet ps2 = clone(ps1);
|
||||
ps2.setPushCertificate(ps2.getPushCertificate() + "\n\n");
|
||||
|
||||
ChangeBundle b1 = new ChangeBundle(c, messages(), patchSets(ps1),
|
||||
approvals(), comments(), NOTE_DB);
|
||||
ChangeBundle b2 = new ChangeBundle(c, messages(), patchSets(ps2),
|
||||
approvals(), comments(), REVIEW_DB);
|
||||
assertNoDiffs(b1, b2);
|
||||
assertNoDiffs(b2, b1);
|
||||
|
||||
b1 = new ChangeBundle(c, messages(), patchSets(ps1), approvals(),
|
||||
comments(), REVIEW_DB);
|
||||
b2 = new ChangeBundle(c, messages(), patchSets(ps2), approvals(),
|
||||
comments(), NOTE_DB);
|
||||
assertNoDiffs(b1, b2);
|
||||
assertNoDiffs(b2, b1);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void diffPatchSetApprovalKeySets() throws Exception {
|
||||
Change c = TestChanges.newChange(project, accountId);
|
||||
|
||||
@@ -23,7 +23,6 @@ import static com.google.gerrit.testutil.TestChanges.incrementPatchSet;
|
||||
import static java.nio.charset.StandardCharsets.UTF_8;
|
||||
import static org.eclipse.jgit.lib.Constants.OBJ_BLOB;
|
||||
|
||||
import com.google.common.base.CharMatcher;
|
||||
import com.google.common.base.Function;
|
||||
import com.google.common.collect.ImmutableList;
|
||||
import com.google.common.collect.ImmutableListMultimap;
|
||||
@@ -900,7 +899,6 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
|
||||
+ "\n"
|
||||
+ "Nor is this a real signature.\n"
|
||||
+ "-----END PGP SIGNATURE-----\n";
|
||||
String trimmedCert = CharMatcher.is('\n').trimTrailingFrom(pushCert);
|
||||
|
||||
// ps2 with push cert
|
||||
Change c = newChange();
|
||||
@@ -917,8 +915,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
|
||||
assertThat(readNote(notes, commit)).isEqualTo(pushCert);
|
||||
Map<PatchSet.Id, PatchSet> patchSets = notes.getPatchSets();
|
||||
assertThat(patchSets.get(psId1).getPushCertificate()).isNull();
|
||||
assertThat(patchSets.get(psId2).getPushCertificate())
|
||||
.isEqualTo(trimmedCert);
|
||||
assertThat(patchSets.get(psId2).getPushCertificate()).isEqualTo(pushCert);
|
||||
assertThat(notes.getComments()).isEmpty();
|
||||
|
||||
// comment on ps2
|
||||
@@ -946,8 +943,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
|
||||
+ "\n");
|
||||
patchSets = notes.getPatchSets();
|
||||
assertThat(patchSets.get(psId1).getPushCertificate()).isNull();
|
||||
assertThat(patchSets.get(psId2).getPushCertificate())
|
||||
.isEqualTo(trimmedCert);
|
||||
assertThat(patchSets.get(psId2).getPushCertificate()).isEqualTo(pushCert);
|
||||
assertThat(notes.getComments()).isNotEmpty();
|
||||
}
|
||||
|
||||
|
||||
@@ -81,6 +81,8 @@ public class TestNotesMigration extends NotesMigration {
|
||||
case CHECK:
|
||||
case OFF:
|
||||
default:
|
||||
setWriteChanges(false);
|
||||
setReadChanges(false);
|
||||
break;
|
||||
}
|
||||
return this;
|
||||
|
||||
Reference in New Issue
Block a user