Fix deletion of draft comment refs

In NoteDb, we store draft comments in the All-Users repository. This
improves performance in the googlesource.com deployment, as
high-traffic repositories can be configured to bypass Gerrit ACL
checks for the ref advertisement.

This makes publishing comments non-atomic: after publishing comments
successfully, the update of the draft branch may fail. For this
reason, any NoteDb update that touches the draft ref in All-Users will
schedule a deletion of all published comments for the change.

If there were multiple patchsets, scheduling deletions of items that
were not in the draft ref triggered a logic error in
ChangeDraftUpdate, which caused the branch to become empty rather than
deleted. This had the following consequences:

* unused draft refs persisted in the All-Users repository, polluting
  the namespace.

* published draft comments as well as deleted draft comments were kept
  in the history of the draft ref, keeping them alive for GC, and
  causing a steady increase of repository size.

In other words, for this to trigger, a change would have to publish
draft comments on a change with multiple patchsets, and a previously
published comment. The problem was not visible to users of Gerrit.

This changes fixes the problem by simply looking at the notemap to be
serialized, and deleting the draft ref if the new notemap is empty.

Existing refs must still be cleaned up. We could introduce code,
perhaps in the ProjectConsistencyChecker, to remove the stale refs
from All-Users.

Change-Id: I6f7b65828bb047bbff041b4d78f6b40190e76219

java/com/google/gerrit/server/notedb/ChangeDraftUpdate.java

@@ -198,10 +198,9 @@ public class ChangeDraftUpdate extends AbstractChangeUpdate {
       return NO_OP_UPDATE;
     }
 
-    // If we touched every revision and there are no comments left, tell the
+    // If there are no comments left, tell the
     // caller to delete the entire ref.
-    boolean touchedAllRevs = updatedRevs.equals(rnm.revisionNotes.keySet());
+    if (!rnm.noteMap.iterator().hasNext()) {
-    if (touchedAllRevs && !hasComments) {
       return null;
     }
 

javatests/com/google/gerrit/acceptance/server/change/CommentsIT.java

@@ -47,6 +47,7 @@ import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
 import com.google.gerrit.extensions.restapi.TopLevelResource;
 import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.Patch;
+import com.google.gerrit.reviewdb.client.RefNames;
 import com.google.gerrit.server.change.ChangeResource;
 import com.google.gerrit.server.change.RevisionResource;
 import com.google.gerrit.server.notedb.ChangeNoteUtil;
@@ -69,6 +70,7 @@ import java.util.function.Supplier;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 import org.eclipse.jgit.lib.ObjectReader;
+import org.eclipse.jgit.lib.Ref;
 import org.eclipse.jgit.lib.Repository;
 import org.eclipse.jgit.notes.NoteMap;
 import org.eclipse.jgit.revwalk.RevCommit;
@@ -287,6 +289,47 @@ public class CommentsIT extends AbstractDaemonTest {
     revision(r).review(input);
   }
 
+  @Test
+  public void postCommentsUnreachableData() throws Exception {
+    setApiUser(admin);
+
+    String file = "file";
+    PushOneCommit push =
+        pushFactory.create(db, admin.getIdent(), testRepo, "first subject", file, "l1\nl2\n");
+
+    String dest = "refs/for/master";
+    PushOneCommit.Result r1 = push.to(dest);
+    r1.assertOkStatus();
+    String changeId = r1.getChangeId();
+    String revId = r1.getCommit().getName();
+
+    PushOneCommit.Result r2 = amendChange(r1.getChangeId());
+    r2.assertOkStatus();
+
+    String draftRefName = RefNames.refsDraftComments(r1.getChange().getId(), admin.getId());
+
+    DraftInput draft = newDraft(file, Side.REVISION, 1, "comment");
+    addDraft(changeId, "1", draft);
+    ReviewInput reviewInput = new ReviewInput();
+    reviewInput.drafts = DraftHandling.PUBLISH;
+    reviewInput.message = "foo";
+    gApi.changes().id(r1.getChangeId()).revision(1).review(reviewInput);
+
+    addDraft(changeId, "2", newDraft(file, Side.REVISION, 2, "comment2"));
+    reviewInput = new ReviewInput();
+    reviewInput.drafts = DraftHandling.PUBLISH_ALL_REVISIONS;
+    reviewInput.message = "bar";
+    gApi.changes().id(r1.getChangeId()).revision(2).review(reviewInput);
+
+    Map<String, List<CommentInfo>> drafts = getDraftComments(changeId, revId);
+    assertThat(drafts.isEmpty()).isTrue();
+
+    try (Repository repo = repoManager.openRepository(allUsers)) {
+      Ref ref = repo.exactRef(draftRefName);
+      assertThat(ref).isNull();
+    }
+  }
+
   @Test
   public void listComments() throws Exception {
     String file = "file";