Merge "Support to set cherry-pick notification options"

2017-05-24 16:42:29 +00:00 · 2017-05-24 16:42:29 +00:00 · 41aa6b7085
commit 41aa6b7085
parent d54c6afa12 e04e846873
6 changed files with 105 additions and 48 deletions
--- a/Documentation/rest-api-changes.txt
+++ b/Documentation/rest-api-changes.txt
@ -5689,6 +5689,14 @@ The `CherryPickInput` entity contains information for cherry-picking a change to
 |`destination`      ||Destination branch
 |`parent`           |optional, defaults to 1|
 Number of the parent relative to which the cherry-pick should be considered.
+|`notify`           |optional|
+Notify handling that defines to whom email notifications should be sent
+after the cherry-pick. +
+Allowed values are `NONE`, `OWNER`, `OWNER_REVIEWERS` and `ALL`. +
+If not set, the default is `NONE`.
+|`notify_details`   |optional|
+Additional information about whom to notify about the update as a map
+of recipient type to link:#notify-info[NotifyInfo] entity.
 |===========================

 [[comment-info]]
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/revision/RevisionIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/revision/RevisionIT.java
@ -38,12 +38,16 @@ import com.google.common.collect.Iterators;
 import com.google.gerrit.acceptance.AbstractDaemonTest;
 import com.google.gerrit.acceptance.PushOneCommit;
 import com.google.gerrit.acceptance.RestResponse;
+import com.google.gerrit.acceptance.TestAccount;
 import com.google.gerrit.acceptance.TestProjectInput;
 import com.google.gerrit.common.data.Permission;
 import com.google.gerrit.extensions.api.changes.ChangeApi;
 import com.google.gerrit.extensions.api.changes.CherryPickInput;
 import com.google.gerrit.extensions.api.changes.DraftApi;
 import com.google.gerrit.extensions.api.changes.DraftInput;
+import com.google.gerrit.extensions.api.changes.NotifyHandling;
+import com.google.gerrit.extensions.api.changes.NotifyInfo;
+import com.google.gerrit.extensions.api.changes.RecipientType;
 import com.google.gerrit.extensions.api.changes.ReviewInput;
 import com.google.gerrit.extensions.api.changes.ReviewInput.CommentInput;
 import com.google.gerrit.extensions.api.changes.RevisionApi;
@ -70,6 +74,7 @@ import com.google.gerrit.extensions.restapi.ResourceConflictException;
 import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.Branch;
+import com.google.gerrit.reviewdb.client.Branch.NameKey;
 import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.PatchSetApproval;
 import com.google.gerrit.server.change.GetRevisionActions;
@ -622,6 +627,47 @@ public class RevisionIT extends AbstractDaemonTest {
    gApi.changes().id(mergeChangeResult.getChangeId()).current().cherryPick(cherryPickInput);
  }

+  @Test
+  public void cherryPickNotify() throws Exception {
+    createBranch(new NameKey(project, "branch-1"));
+    createBranch(new NameKey(project, "branch-2"));
+    createBranch(new NameKey(project, "branch-3"));
+
+    // Creates a change for 'admin'.
+    PushOneCommit.Result result = createChange();
+    String changeId = project.get() + "~master~" + result.getChangeId();
+
+    // 'user' cherry-picks the change to a new branch, the source change's author/committer('admin')
+    // will be added as a reviewer of the newly created change.
+    setApiUser(user);
+    CherryPickInput input = new CherryPickInput();
+    input.message = "it goes to a new branch";
+
+    // Enable the notification. 'admin' as a reviewer should be notified.
+    input.destination = "branch-1";
+    input.notify = NotifyHandling.ALL;
+    sender.clear();
+    gApi.changes().id(changeId).current().cherryPick(input);
+    assertNotifyCc(admin);
+
+    // Disable the notification. 'admin' as a reviewer should not be notified any more.
+    input.destination = "branch-2";
+    input.notify = NotifyHandling.NONE;
+    sender.clear();
+    gApi.changes().id(changeId).current().cherryPick(input);
+    assertThat(sender.getMessages()).hasSize(0);
+
+    // Disable the notification. The user provided in the 'notifyDetails' should still be notified.
+    TestAccount userToNotify = accounts.user2();
+    input.destination = "branch-3";
+    input.notify = NotifyHandling.NONE;
+    input.notifyDetails =
+        ImmutableMap.of(RecipientType.TO, new NotifyInfo(ImmutableList.of(userToNotify.email)));
+    sender.clear();
+    gApi.changes().id(changeId).current().cherryPick(input);
+    assertNotifyTo(userToNotify);
+  }
+
  @Test
  public void canRebase() throws Exception {
    PushOneCommit push = pushFactory.create(db, admin.getIdent(), testRepo);
--- a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/CherryPickInput.java
+++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/CherryPickInput.java
@ -14,8 +14,13 @@

 package com.google.gerrit.extensions.api.changes;

+import java.util.Map;
+
 public class CherryPickInput {
  public String message;
  public String destination;
  public Integer parent;
+
+  public NotifyHandling notify = NotifyHandling.NONE;
+  public Map<RecipientType, NotifyInfo> notifyDetails;
 }
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/CherryPick.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/CherryPick.java
@ -66,7 +66,7 @@ public class CherryPick
      BatchUpdate.Factory updateFactory, RevisionResource revision, CherryPickInput input)
      throws OrmException, IOException, UpdateException, RestApiException {
    final ChangeControl control = revision.getControl();
-    int parent = input.parent == null ? 1 : input.parent;
+    input.parent = input.parent == null ? 1 : input.parent;

    if (input.message == null || input.message.trim().isEmpty()) {
      throw new BadRequestException("message must be non-empty");
@ -100,10 +100,9 @@ public class CherryPick
              updateFactory,
              revision.getChange(),
              revision.getPatchSet(),
-              input.message,
+              input,
              refName,
-              refControl,
-              parent);
+              refControl);
      return json.noOptions().format(revision.getProject(), cherryPickedChangeId);
    } catch (InvalidChangeOperationException e) {
      throw new BadRequestException(e.getMessage());
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/CherryPickChange.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/CherryPickChange.java
@ -18,7 +18,8 @@ import com.google.common.base.Strings;
 import com.google.gerrit.common.FooterConstants;
 import com.google.gerrit.common.Nullable;
 import com.google.gerrit.common.TimeUtil;
-import com.google.gerrit.extensions.api.changes.NotifyHandling;
+import com.google.gerrit.extensions.api.changes.CherryPickInput;
+import com.google.gerrit.extensions.restapi.BadRequestException;
 import com.google.gerrit.extensions.restapi.MergeConflictException;
 import com.google.gerrit.extensions.restapi.RestApiException;
 import com.google.gerrit.reviewdb.client.Branch;
@ -80,6 +81,7 @@ public class CherryPickChange {
  private final MergeUtil.Factory mergeUtilFactory;
  private final ChangeMessagesUtil changeMessagesUtil;
  private final PatchSetUtil psUtil;
+  private final NotifyUtil notifyUtil;

  @Inject
  CherryPickChange(
@ -93,7 +95,8 @@ public class CherryPickChange {
      PatchSetInserter.Factory patchSetInserterFactory,
      MergeUtil.Factory mergeUtilFactory,
      ChangeMessagesUtil changeMessagesUtil,
-      PatchSetUtil psUtil) {
+      PatchSetUtil psUtil,
+      NotifyUtil notifyUtil) {
    this.db = db;
    this.seq = seq;
    this.queryProvider = queryProvider;
@ -105,16 +108,16 @@ public class CherryPickChange {
    this.mergeUtilFactory = mergeUtilFactory;
    this.changeMessagesUtil = changeMessagesUtil;
    this.psUtil = psUtil;
+    this.notifyUtil = notifyUtil;
  }

  public Change.Id cherryPick(
      BatchUpdate.Factory batchUpdateFactory,
      Change change,
      PatchSet patch,
-      String message,
+      CherryPickInput input,
      String ref,
-      RefControl refControl,
-      int parent)
+      RefControl refControl)
      throws OrmException, IOException, InvalidChangeOperationException, IntegrationException,
          UpdateException, RestApiException {
    return cherryPick(
@ -125,10 +128,9 @@ public class CherryPickChange {
        change.getTopic(),
        change.getProject(),
        ObjectId.fromString(patch.getRevision().get()),
-        message,
+        input,
        ref,
-        refControl,
-        parent);
+        refControl);
  }

  public Change.Id cherryPick(
@ -139,10 +141,9 @@ public class CherryPickChange {
      @Nullable String sourceChangeTopic,
      Project.NameKey project,
      ObjectId sourceCommit,
-      String message,
+      CherryPickInput input,
      String targetRef,
-      RefControl targetRefControl,
-      int parent)
+      RefControl targetRefControl)
      throws OrmException, IOException, InvalidChangeOperationException, IntegrationException,
          UpdateException, RestApiException {

@ -170,12 +171,12 @@ public class CherryPickChange {

      CodeReviewCommit commitToCherryPick = revWalk.parseCommit(sourceCommit);

-      if (parent <= 0 || parent > commitToCherryPick.getParentCount()) {
+      if (input.parent <= 0 || input.parent > commitToCherryPick.getParentCount()) {
        throw new InvalidChangeOperationException(
            String.format(
                "Cherry Pick: Parent %s does not exist. Please specify a parent in"
                    + " range [1, %s].",
-                parent, commitToCherryPick.getParentCount()));
+                input.parent, commitToCherryPick.getParentCount()));
      }

      Timestamp now = TimeUtil.nowTs();
@ -187,8 +188,8 @@ public class CherryPickChange {
              mergeTip,
              commitToCherryPick.getAuthorIdent(),
              committerIdent,
-              message);
-      String commitMessage = ChangeIdUtil.insertId(message, computedChangeId).trim() + '\n';
+              input.message);
+      String commitMessage = ChangeIdUtil.insertId(input.message, computedChangeId).trim() + '\n';

      CodeReviewCommit cherryPickCommit;
      try {
@ -204,7 +205,7 @@ public class CherryPickChange {
                    committerIdent,
                    commitMessage,
                    revWalk,
-                    parent - 1,
+                    input.parent - 1,
                    false);

        Change.Key changeKey;
@ -234,7 +235,7 @@ public class CherryPickChange {
            // will be added as a new patch set.
            ChangeControl destCtl =
                targetRefControl.getProjectControl().controlFor(destChanges.get(0).notes());
-            result = insertPatchSet(bu, git, destCtl, cherryPickCommit);
+            result = insertPatchSet(bu, git, destCtl, cherryPickCommit, input);
          } else {
            // Change key not found on destination branch. We can create a new
            // change.
@ -249,7 +250,8 @@ public class CherryPickChange {
                    targetRefControl.getRefName(),
                    newTopic,
                    sourceBranch,
-                    sourceCommit);
+                    sourceCommit,
+                    input);

            if (sourceChangeId != null && sourcePatchId != null) {
              bu.addOp(
@ -268,20 +270,23 @@ public class CherryPickChange {
  }

  private Change.Id insertPatchSet(
-      BatchUpdate bu, Repository git, ChangeControl destCtl, CodeReviewCommit cherryPickCommit)
-      throws IOException, OrmException {
+      BatchUpdate bu,
+      Repository git,
+      ChangeControl destCtl,
+      CodeReviewCommit cherryPickCommit,
+      CherryPickInput input)
+      throws IOException, OrmException, BadRequestException {
    Change destChange = destCtl.getChange();
    PatchSet.Id psId = ChangeUtil.nextPatchSetId(git, destChange.currentPatchSetId());
-    PatchSetInserter inserter = patchSetInserterFactory.create(destCtl, psId, cherryPickCommit);
-    PatchSet.Id newPatchSetId = inserter.getPatchSetId();
    PatchSet current = psUtil.current(db.get(), destCtl.getNotes());

-    bu.addOp(
-        destChange.getId(),
-        inserter
-            .setMessage("Uploaded patch set " + newPatchSetId.get() + ".")
-            .setDraft(current.isDraft())
-            .setNotify(NotifyHandling.NONE));
+    PatchSetInserter inserter = patchSetInserterFactory.create(destCtl, psId, cherryPickCommit);
+    inserter
+        .setMessage("Uploaded patch set " + inserter.getPatchSetId().get() + ".")
+        .setDraft(current.isDraft())
+        .setNotify(input.notify)
+        .setAccountsToNotify(notifyUtil.resolveAccounts(input.notifyDetails));
+    bu.addOp(destChange.getId(), inserter);
    return destChange.getId();
  }

@ -291,12 +296,15 @@ public class CherryPickChange {
      String refName,
      String topic,
      Branch.NameKey sourceBranch,
-      ObjectId sourceCommit)
-      throws OrmException, IOException {
+      ObjectId sourceCommit,
+      CherryPickInput input)
+      throws OrmException, IOException, BadRequestException {
    Change.Id changeId = new Change.Id(seq.nextChangeId());
    ChangeInserter ins =
        changeInserterFactory.create(changeId, cherryPickCommit, refName).setTopic(topic);
-    ins.setMessage(messageForDestinationChange(ins.getPatchSetId(), sourceBranch, sourceCommit));
+    ins.setMessage(messageForDestinationChange(ins.getPatchSetId(), sourceBranch, sourceCommit))
+        .setNotify(input.notify)
+        .setAccountsToNotify(notifyUtil.resolveAccounts(input.notifyDetails));
    bu.insertChange(ins);
    return changeId;
  }
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/CherryPickCommit.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/CherryPickCommit.java
@ -59,9 +59,11 @@ public class CherryPickCommit
  public ChangeInfo applyImpl(
      BatchUpdate.Factory updateFactory, CommitResource rsrc, CherryPickInput input)
      throws OrmException, IOException, UpdateException, RestApiException {
+    RevCommit commit = rsrc.getCommit();
    String message = Strings.nullToEmpty(input.message).trim();
+    input.message = message.isEmpty() ? commit.getFullMessage() : message;
    String destination = Strings.nullToEmpty(input.destination).trim();
-    int parent = input.parent == null ? 1 : input.parent;
+    input.parent = input.parent == null ? 1 : input.parent;

    if (destination.isEmpty()) {
      throw new BadRequestException("destination must be non-empty");
@ -73,7 +75,6 @@ public class CherryPickCommit
      throw new AuthException(capable.getMessage());
    }

-    RevCommit commit = rsrc.getCommit();
    String refName = RefNames.fullName(destination);
    RefControl refControl = projectControl.controlForRef(refName);
    if (!refControl.canUpload()) {
@ -84,17 +85,7 @@ public class CherryPickCommit
    try {
      Change.Id cherryPickedChangeId =
          cherryPickChange.cherryPick(
-              updateFactory,
-              null,
-              null,
-              null,
-              null,
-              project,
-              commit,
-              message.isEmpty() ? commit.getFullMessage() : message,
-              refName,
-              refControl,
-              parent);
+              updateFactory, null, null, null, null, project, commit, input, refName, refControl);
      return json.noOptions().format(project, cherryPickedChangeId);
    } catch (InvalidChangeOperationException e) {
      throw new BadRequestException(e.getMessage());