notedb: Support updating comments on multiple revisions
All other metadata updates must still be associated with a single
patch set, but comments (including draft comments) may now be modified
across patch set boundaries.
Change-Id: I47080059b2d7cce619729eeb68fde07961a19d01
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeDraftUpdate.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeDraftUpdate.java
index 0df9b90..47c1731 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeDraftUpdate.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeDraftUpdate.java
@@ -16,7 +16,6 @@
import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Preconditions.checkState;
-import static com.google.gerrit.server.PatchLineCommentsUtil.getCommentPsId;
import static com.google.gerrit.server.notedb.CommentsInNotesUtil.addCommentToMap;
import com.google.common.collect.ListMultimap;
@@ -170,11 +169,6 @@
}
private void verifyComment(PatchLineComment comment) {
- checkState(psId != null,
- "setPatchSetId must be called first");
- checkArgument(getCommentPsId(comment).equals(psId),
- "Comment on %s does not match configured patch set %s",
- getCommentPsId(comment), psId);
if (migration.writeChanges()) {
checkArgument(comment.getRevId() != null);
}
@@ -290,7 +284,7 @@
}
commit.setAuthor(newIdent(getUser().getAccount(), when));
commit.setCommitter(new PersonIdent(serverIdent, when));
- commit.setMessage(String.format("Comment on patch set %d", psId.get()));
+ commit.setMessage("Update draft comments");
return true;
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeUpdate.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeUpdate.java
index 41202bd..43c232b 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeUpdate.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeUpdate.java
@@ -15,7 +15,6 @@
package com.google.gerrit.server.notedb;
import static com.google.common.base.Preconditions.checkArgument;
-import static com.google.gerrit.server.PatchLineCommentsUtil.getCommentPsId;
import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_HASHTAGS;
import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_LABEL;
import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_PATCH_SET;
@@ -241,7 +240,7 @@
}
private void insertDraftComment(PatchLineComment c) throws OrmException {
- createDraftUpdateIfNull(c);
+ createDraftUpdateIfNull();
draftUpdate.insertComment(c);
}
@@ -262,7 +261,7 @@
}
private void upsertDraftComment(PatchLineComment c) {
- createDraftUpdateIfNull(c);
+ createDraftUpdateIfNull();
draftUpdate.upsertComment(c);
}
@@ -281,38 +280,28 @@
}
private void updateDraftComment(PatchLineComment c) throws OrmException {
- createDraftUpdateIfNull(c);
+ createDraftUpdateIfNull();
draftUpdate.updateComment(c);
}
private void deleteDraftComment(PatchLineComment c) throws OrmException {
- createDraftUpdateIfNull(c);
+ createDraftUpdateIfNull();
draftUpdate.deleteComment(c);
}
private void deleteDraftCommentIfPresent(PatchLineComment c)
throws OrmException {
- createDraftUpdateIfNull(c);
+ createDraftUpdateIfNull();
draftUpdate.deleteCommentIfPresent(c);
}
- private void createDraftUpdateIfNull(PatchLineComment c) {
+ private void createDraftUpdateIfNull() {
if (draftUpdate == null) {
draftUpdate = draftUpdateFactory.create(ctl, when);
- if (psId != null) {
- draftUpdate.setPatchSetId(psId);
- } else {
- draftUpdate.setPatchSetId(getCommentPsId(c));
- }
}
}
private void verifyComment(PatchLineComment c) {
- checkArgument(psId != null,
- "setPatchSetId must be called first");
- checkArgument(getCommentPsId(c).equals(psId),
- "Comment on %s doesn't match previous patch set %s",
- getCommentPsId(c), psId);
checkArgument(c.getRevId() != null);
checkArgument(c.getStatus() == Status.PUBLISHED,
"Cannot add a draft comment to a ChangeUpdate. Use a ChangeDraftUpdate"
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesTest.java
index 9379280..6067442 100644
--- a/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesTest.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesTest.java
@@ -1211,4 +1211,48 @@
assertThat(newNotes(c).getComments()).containsExactly(
ImmutableMultimap.of(new RevId(rev), comment));
}
+
+ @Test
+ public void updateCommentsForMultipleRevisions() throws Exception {
+ Change c = newChange();
+ String uuid = "uuid";
+ String rev1 = "abcd1234abcd1234abcd1234abcd1234abcd1234";
+ String rev2 = "abcd4567abcd4567abcd4567abcd4567abcd4567";
+ CommentRange range = new CommentRange(1, 1, 2, 1);
+ PatchSet.Id ps1 = c.currentPatchSetId();
+ String filename = "filename1";
+ short side = (short) 1;
+
+ incrementPatchSet(c);
+ PatchSet.Id ps2 = c.currentPatchSetId();
+
+ ChangeUpdate update = newUpdate(c, otherUser);
+ update.setPatchSetId(ps2);
+ Timestamp now = TimeUtil.nowTs();
+ PatchLineComment comment1 = newComment(ps1, filename,
+ uuid, range, range.getEndLine(), otherUser, null, now, "comment on ps1",
+ side, rev1, Status.DRAFT);
+ PatchLineComment comment2 = newComment(ps2, filename,
+ uuid, range, range.getEndLine(), otherUser, null, now, "comment on ps2",
+ side, rev2, Status.DRAFT);
+ update.upsertComment(comment1);
+ update.upsertComment(comment2);
+ update.commit();
+
+ ChangeNotes notes = newNotes(c);
+ assertThat(notes.getDraftComments(otherUserId)).hasSize(2);
+ assertThat(notes.getComments()).isEmpty();
+
+ update = newUpdate(c, otherUser);
+ update.setPatchSetId(ps2);
+ comment1.setStatus(Status.PUBLISHED);
+ comment2.setStatus(Status.PUBLISHED);
+ update.upsertComment(comment1);
+ update.upsertComment(comment2);
+ update.commit();
+
+ notes = newNotes(c);
+ assertThat(notes.getDraftComments(otherUserId)).isEmpty();
+ assertThat(notes.getComments()).hasSize(2);
+ }
}
