Allow setting a comment message on a push

Gerrit has a feature that allow setting labels, reviewers, and topic
on push[1] of a new change or patchset.  This adds an additional
'message' parameter to allow setting a comment message.

[1] https://gerrit-review.googlesource.com/Documentation/user-upload.html#push_create

Change-Id: I9e242e4de2892723a0eb03c8a3b7fe3c574058ac
Feature: Issue 4015

Documentation/user-upload.txt

@@ -178,6 +178,20 @@ updates:
   git push ssh://john.doe@git.example.com:29418/kernel/common HEAD:refs/for/experimental%topic=driver/i42
 ====
 
+[[message]]
+A comment message can be applied to the change by using the `message` (or `m`)
+option:
+
+====
+  git push ssh://john.doe@git.example.com:29418/kernel/common HEAD:refs/for/experimental%m=This_is_a_rebase_on_master
+====
+
+.Note
+****
+git push refs parameter does not allow spaces.  Use the '_' character instead,
+it will then be applied as "This is a rebase on master".
+****
+
 [[review_labels]]
 Review labels can be applied to the change by using the `label` (or `l`)
 option in the reference:

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/AbstractPushForReview.java

@@ -34,6 +34,7 @@ import com.google.gerrit.extensions.api.changes.ReviewInput;
 import com.google.gerrit.extensions.api.changes.ReviewInput.NotifyHandling;
 import com.google.gerrit.extensions.client.InheritableBoolean;
 import com.google.gerrit.extensions.common.ChangeInfo;
+import com.google.gerrit.extensions.common.ChangeMessageInfo;
 import com.google.gerrit.extensions.common.EditInfo;
 import com.google.gerrit.extensions.common.LabelInfo;
 import com.google.gerrit.reviewdb.client.AccountGroup;
@@ -51,6 +52,7 @@ import org.junit.Before;
 import org.junit.BeforeClass;
 import org.junit.Test;
 
+import java.util.Collection;
 import java.util.Set;
 
 public abstract class AbstractPushForReview extends AbstractDaemonTest {
@@ -236,6 +238,20 @@ public abstract class AbstractPushForReview extends AbstractDaemonTest {
     assertThat(edit).isNotNull();
   }
 
+  @Test
+  public void testPushForMasterWithMessage() throws Exception {
+    PushOneCommit.Result r = pushTo("refs/for/master/%m=my_test_message");
+    r.assertOkStatus();
+    r.assertChange(Change.Status.NEW, null);
+    ChangeInfo ci = get(r.getChangeId());
+    Collection<ChangeMessageInfo> changeMessages = ci.messages;
+    assertThat(changeMessages).hasSize(1);
+    for (ChangeMessageInfo cm : changeMessages) {
+      assertThat(cm.message).isEqualTo(
+          "Uploaded patch set 1.\nmy test message");
+    }
+  }
+
   @Test
   public void testPushForMasterWithApprovals() throws Exception {
     PushOneCommit.Result r = pushTo("refs/for/master/%l=Code-Review");

gerrit-server/src/main/java/com/google/gerrit/server/ApprovalsUtil.java

@@ -47,6 +47,7 @@ import com.google.gerrit.server.notedb.ChangeUpdate;
 import com.google.gerrit.server.notedb.NotesMigration;
 import com.google.gerrit.server.notedb.ReviewerStateInternal;
 import com.google.gerrit.server.project.ChangeControl;
+import com.google.gerrit.server.util.LabelVote;
 import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
 import com.google.inject.Singleton;
@@ -330,4 +331,25 @@ public class ApprovalsUtil {
     }
     return submitter;
   }
+
+  public static String renderMessageWithApprovals(int patchSetId,
+      Map<String, Short> n, Map<String, PatchSetApproval> c) {
+    StringBuilder msgs = new StringBuilder("Uploaded patch set " + patchSetId);
+    if (!n.isEmpty()) {
+      boolean first = true;
+      for (Map.Entry<String, Short> e : n.entrySet()) {
+        if (c.containsKey(e.getKey())
+            && c.get(e.getKey()).getValue() == e.getValue()) {
+          continue;
+        }
+        if (first) {
+          msgs.append(":");
+          first = false;
+        }
+        msgs.append(" ")
+            .append(LabelVote.create(e.getKey(), e.getValue()).format());
+      }
+    }
+    return msgs.append('.').toString();
+  }
 }

gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java

@@ -1127,6 +1127,7 @@ public class ReceiveCommits {
     Set<Account.Id> reviewer = Sets.newLinkedHashSet();
     Set<Account.Id> cc = Sets.newLinkedHashSet();
     Map<String, Short> labels = new HashMap<>();
+    String message;
     List<RevCommit> baseCommit;
     LabelTypes labelTypes;
     CmdLineParser clp;
@@ -1183,6 +1184,13 @@ public class ReceiveCommits {
       labels.put(v.label(), v.value());
     }
 
+    @Option(name = "--message", aliases = {"-m"}, metaVar = "MESSAGE",
+        usage = "Comment message to apply to the review")
+    void addMessage(final String token) {
+      // git push does not allow spaces in refs.
+      message = token.replace("_", " ");
+    }
+
     @Option(name = "--hashtag", aliases = {"-t"}, metaVar = "HASHTAG",
         usage = "add hashtag to changes")
     void addHashtag(String token) throws CmdLineException {
@@ -1750,8 +1758,12 @@ public class ReceiveCommits {
       recipients.add(getRecipientsFromFooters(
           accountResolver, magicBranch.draft, footerLines));
       recipients.remove(me);
-      String msg = renderMessageWithApprovals(psId.get(), null,
+      StringBuilder msg =
-          approvals, Collections.<String, PatchSetApproval> emptyMap());
+          new StringBuilder(ApprovalsUtil.renderMessageWithApprovals(psId.get(),
+              approvals, Collections.<String, PatchSetApproval> emptyMap()));
+      if (!Strings.isNullOrEmpty(magicBranch.message)) {
+        msg.append("\n").append(magicBranch.message);
+      }
       try (BatchUpdate bu = batchUpdateFactory.create(state.db,
            magicBranch.dest.getParentKey(), user, TimeUtil.nowTs())) {
         bu.setRepository(state.repo, state.rw, state.ins);
@@ -1759,7 +1771,7 @@ public class ReceiveCommits {
             .setReviewers(recipients.getReviewers())
             .setExtraCC(recipients.getCcOnly())
             .setApprovals(approvals)
-            .setMessage(msg)
+            .setMessage(msg.toString())
             .setNotify(magicBranch.notify)
             .setRequestScopePropagator(requestScopePropagator)
             .setSendMail(true)
@@ -1890,32 +1902,6 @@ public class ReceiveCommits {
     }
   }
 
-  private String renderMessageWithApprovals(int patchSetId, String suffix,
-      Map<String, Short> n, Map<String, PatchSetApproval> c) {
-    StringBuilder msgs = new StringBuilder("Uploaded patch set " + patchSetId);
-    if (!n.isEmpty()) {
-      boolean first = true;
-      for (Map.Entry<String, Short> e : n.entrySet()) {
-        if (c.containsKey(e.getKey())
-            && c.get(e.getKey()).getValue() == e.getValue()) {
-          continue;
-        }
-        if (first) {
-          msgs.append(":");
-          first = false;
-        }
-        msgs.append(" ")
-            .append(LabelVote.create(e.getKey(), e.getValue()).format());
-      }
-    }
-
-    if (!Strings.isNullOrEmpty(suffix)) {
-      msgs.append(suffix);
-    }
-
-    return msgs.append('.').toString();
-  }
-
   private class ReplaceRequest {
     final Change.Id ontoChange;
     final ObjectId newCommitId;

gerrit-server/src/main/java/com/google/gerrit/server/git/ReplaceOp.java

@@ -49,7 +49,6 @@ import com.google.gerrit.server.project.ChangeControl;
 import com.google.gerrit.server.project.NoSuchChangeException;
 import com.google.gerrit.server.project.ProjectControl;
 import com.google.gerrit.server.query.change.ChangeData;
-import com.google.gerrit.server.util.LabelVote;
 import com.google.gerrit.server.util.RequestScopePropagator;
 import com.google.gwtorm.server.OrmException;
 import com.google.inject.assistedinject.Assisted;
@@ -201,8 +200,10 @@ public class ReplaceOp extends BatchUpdate.Op {
     ChangeUpdate update = ctx.getUpdate(patchSetId);
     update.setSubjectForCommit("Create patch set " + patchSetId.get());
 
+    String reviewMessage = null;
     if (magicBranch != null) {
       recipients.add(magicBranch.getMailRecipients());
+      reviewMessage = magicBranch.message;
       approvals.putAll(magicBranch.labels);
       Set<String> hashtags = magicBranch.hashtags;
       if (hashtags != null && !hashtags.isEmpty()) {
@@ -242,12 +243,21 @@ public class ReplaceOp extends BatchUpdate.Op {
         approvals);
     recipients.add(oldRecipients);
 
+    String approvalMessage = ApprovalsUtil.renderMessageWithApprovals(
+        patchSetId.get(), approvals, scanLabels(ctx, approvals));
+    StringBuilder message = new StringBuilder(approvalMessage);
+    String kindMessage = changeKindMessage(changeKind);
+    if (!Strings.isNullOrEmpty(kindMessage)) {
+      message.append(kindMessage);
+    }
+    if (!Strings.isNullOrEmpty(reviewMessage)) {
+      message.append("\n").append(reviewMessage);
+    }
     msg = new ChangeMessage(
         new ChangeMessage.Key(change.getId(),
             ChangeUtil.messageUUID(ctx.getDb())),
         ctx.getUser().getAccountId(), ctx.getWhen(), patchSetId);
-    msg.setMessage(renderMessageWithApprovals(patchSetId.get(),
+    msg.setMessage(message.toString());
-        changeKindMessage(changeKind), approvals, scanLabels(ctx, approvals)));
     cmUtil.addChangeMessage(ctx.getDb(), update, msg);
 
     if (mergedIntoRef == null) {
@@ -272,32 +282,6 @@ public class ReplaceOp extends BatchUpdate.Op {
     }
   }
 
-  private static String renderMessageWithApprovals(int patchSetId,
-      String suffix, Map<String, Short> n, Map<String, PatchSetApproval> c) {
-    StringBuilder msgs = new StringBuilder("Uploaded patch set " + patchSetId);
-    if (!n.isEmpty()) {
-      boolean first = true;
-      for (Map.Entry<String, Short> e : n.entrySet()) {
-        if (c.containsKey(e.getKey())
-            && c.get(e.getKey()).getValue() == e.getValue()) {
-          continue;
-        }
-        if (first) {
-          msgs.append(":");
-          first = false;
-        }
-        msgs.append(" ")
-            .append(LabelVote.create(e.getKey(), e.getValue()).format());
-      }
-    }
-
-    if (!Strings.isNullOrEmpty(suffix)) {
-      msgs.append(suffix);
-    }
-
-    return msgs.append('.').toString();
-  }
-
   private Map<String, PatchSetApproval> scanLabels(ChangeContext ctx,
       Map<String, Short> approvals) throws OrmException {
     Map<String, PatchSetApproval> current = new HashMap<>();