Support notify option for delete reviewer REST endpoint

Some automated tools may want to suppress email notifications when
removing reviewers.

Change-Id: I26f66272d4f871aae9d36f3524faea9ffa86ea35
Signed-off-by: Edwin Kempin <ekempin@google.com>

Documentation/rest-api-changes.txt

@@ -2408,14 +2408,33 @@ To confirm the addition of the reviewers, resend the request with the
 [[delete-reviewer]]
 === Delete Reviewer
 --
-'DELETE /changes/link:#change-id[\{change-id\}]/reviewers/link:rest-api-accounts.html#account-id[\{account-id\}]'
+'DELETE /changes/link:#change-id[\{change-id\}]/reviewers/link:rest-api-accounts.html#account-id[\{account-id\}]' +
+'POST /changes/link:#change-id[\{change-id\}]/reviewers/link:rest-api-accounts.html#account-id[\{account-id\}]/delete'
 --
 
 Deletes a reviewer from a change.
 
+Options can be provided in the request body as a
+link:#delete-reviewer-input[DeleteReviewerInput] entity.
+
 .Request
 ----
   DELETE /changes/myProject~master~I8473b95934b5732ac55d26311a706c9c2bde9940/reviewers/John%20Doe HTTP/1.0
+  POST /changes/myProject~master~I8473b95934b5732ac55d26311a706c9c2bde9940/reviewers/John%20Doe/delete HTTP/1.0
+----
+
+Please note that some proxies prohibit request bodies for DELETE
+requests. In this case, if you want to specify options, use a POST
+request:
+
+.Request
+----
+  POST /changes/myProject~master~I8473b95934b5732ac55d26311a706c9c2bde9940/reviewers/John%20Doe/delete HTTP/1.0
+  Content-Type: application/json; charset=UTF-8
+
+  {
+    "notify": "NONE"
+  }
 ----
 
 .Response
@@ -4588,6 +4607,21 @@ Links to the commit in external sites as a list of
 link:#web-link-info[WebLinkInfo] entities.
 |===========================
 
+[[delete-reviewer-input]]
+=== DeleteReviewerInput
+The `DeleteReviewerInput` entity contains options for the deletion of a
+reviewer.
+
+[options="header",cols="1,^1,5"]
+|=======================
+|Field Name||Description
+|`notify`  |optional|
+Notify handling that defines to whom email notifications should be sent
+after the reviewer is deleted. +
+Allowed values are `NONE`, `OWNER`, `OWNER_REVIEWERS` and `ALL`. +
+If not set, the default is `ALL`.
+|=======================
+
 [[delete-vote-input]]
 === DeleteVoteInput
 The `DeleteVoteInput` entity contains options for the deletion of a

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java

@@ -46,6 +46,7 @@ import com.google.gerrit.common.FooterConstants;
 import com.google.gerrit.common.data.LabelType;
 import com.google.gerrit.common.data.Permission;
 import com.google.gerrit.extensions.api.changes.AddReviewerInput;
+import com.google.gerrit.extensions.api.changes.DeleteReviewerInput;
 import com.google.gerrit.extensions.api.changes.DeleteVoteInput;
 import com.google.gerrit.extensions.api.changes.NotifyHandling;
 import com.google.gerrit.extensions.api.changes.RebaseInput;
@@ -996,6 +997,15 @@ public class ChangeIT extends AbstractDaemonTest {
 
   @Test
   public void removeReviewer() throws Exception {
+    testRemoveReviewer(true);
+  }
+
+  @Test
+  public void removeNoNotify() throws Exception {
+    testRemoveReviewer(false);
+  }
+
+  private void testRemoveReviewer(boolean notify) throws Exception {
     PushOneCommit.Result r = createChange();
     String changeId = r.getChangeId();
     gApi.changes()
@@ -1023,16 +1033,24 @@ public class ChangeIT extends AbstractDaemonTest {
 
     sender.clear();
     setApiUser(admin);
+    DeleteReviewerInput input = new DeleteReviewerInput();
+    if (!notify) {
+      input.notify = NotifyHandling.NONE;
+    }
     gApi.changes()
         .id(changeId)
         .reviewer(user.getId().toString())
-        .remove();
+        .remove(input);
 
-    assertThat(sender.getMessages()).hasSize(1);
-    Message message = sender.getMessages().get(0);
-    assertThat(message.body()).contains(
-        "Removed reviewer " + user.fullName + " with the following votes");
-    assertThat(message.body()).contains("* Code-Review+1 by " + user.fullName);
+    if (notify) {
+      assertThat(sender.getMessages()).hasSize(1);
+      Message message = sender.getMessages().get(0);
+      assertThat(message.body()).contains(
+          "Removed reviewer " + user.fullName + " with the following votes");
+      assertThat(message.body()).contains("* Code-Review+1 by " + user.fullName);
+    } else {
+      assertThat(sender.getMessages()).hasSize(0);
+    }
 
     reviewers = gApi.changes()
         .id(changeId)

gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/DeleteReviewerInput.java

@@ -0,0 +1,21 @@
+// Copyright (C) 2016 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.extensions.api.changes;
+
+/** Input passed to {@code DELETE /changes/[id]/reviewers/[id]}. */
+public class DeleteReviewerInput {
+  /** Who to send email notifications to after the reviewer is deleted. */
+  public NotifyHandling notify = NotifyHandling.ALL;
+}

gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/ReviewerApi.java

@@ -25,6 +25,7 @@ public interface ReviewerApi {
   void deleteVote(String label) throws RestApiException;
   void deleteVote(DeleteVoteInput input) throws RestApiException;
   void remove() throws RestApiException;
+  void remove(DeleteReviewerInput input) throws RestApiException;
 
   /**
    * A default implementation which allows source compatibility
@@ -50,5 +51,10 @@ public interface ReviewerApi {
     public void remove() throws RestApiException {
       throw new NotImplementedException();
     }
+
+    @Override
+    public void remove(DeleteReviewerInput input) throws RestApiException {
+      throw new NotImplementedException();
+    }
   }
 }

gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ReviewerApiImpl.java

@@ -14,6 +14,7 @@
 
 package com.google.gerrit.server.api.changes;
 
+import com.google.gerrit.extensions.api.changes.DeleteReviewerInput;
 import com.google.gerrit.extensions.api.changes.DeleteVoteInput;
 import com.google.gerrit.extensions.api.changes.ReviewerApi;
 import com.google.gerrit.extensions.restapi.RestApiException;
@@ -79,8 +80,13 @@ public class ReviewerApiImpl implements ReviewerApi {
 
   @Override
   public void remove() throws RestApiException {
+    remove(new DeleteReviewerInput());
+  }
+
+  @Override
+  public void remove(DeleteReviewerInput input) throws RestApiException {
     try {
-      deleteReviewer.apply(reviewer, null);
+      deleteReviewer.apply(reviewer, input);
     } catch (UpdateException e) {
       throw new RestApiException("Cannot remove reviewer", e);
     }

gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteReviewer.java

@@ -20,6 +20,8 @@ import com.google.common.collect.Lists;
 import com.google.gerrit.common.TimeUtil;
 import com.google.gerrit.common.data.LabelType;
 import com.google.gerrit.common.data.LabelTypes;
+import com.google.gerrit.extensions.api.changes.DeleteReviewerInput;
+import com.google.gerrit.extensions.api.changes.NotifyHandling;
 import com.google.gerrit.extensions.restapi.AuthException;
 import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
 import com.google.gerrit.extensions.restapi.Response;
@@ -38,7 +40,6 @@ import com.google.gerrit.server.ChangeMessagesUtil;
 import com.google.gerrit.server.ChangeUtil;
 import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.PatchSetUtil;
-import com.google.gerrit.server.change.DeleteReviewer.Input;
 import com.google.gerrit.server.extensions.events.ReviewerDeleted;
 import com.google.gerrit.server.git.BatchUpdate;
 import com.google.gerrit.server.git.BatchUpdate.ChangeContext;
@@ -62,13 +63,11 @@ import java.util.List;
 import java.util.Map;
 
 @Singleton
-public class DeleteReviewer implements RestModifyView<ReviewerResource, Input> {
+public class DeleteReviewer
+    implements RestModifyView<ReviewerResource, DeleteReviewerInput> {
   private static final Logger log = LoggerFactory
       .getLogger(DeleteReviewer.class);
 
-  public static class Input {
-  }
-
   private final Provider<ReviewDb> dbProvider;
   private final ApprovalsUtil approvalsUtil;
   private final PatchSetUtil psUtil;
@@ -104,12 +103,19 @@ public class DeleteReviewer implements RestModifyView<ReviewerResource, Input> {
   }
 
   @Override
-  public Response<?> apply(ReviewerResource rsrc, Input input)
+  public Response<?> apply(ReviewerResource rsrc, DeleteReviewerInput input)
       throws RestApiException, UpdateException {
+    if (input == null) {
+      input = new DeleteReviewerInput();
+    }
+    if (input.notify == null) {
+      input.notify = NotifyHandling.ALL;
+    }
+
     try (BatchUpdate bu = batchUpdateFactory.create(dbProvider.get(),
         rsrc.getChangeResource().getProject(),
         rsrc.getChangeResource().getUser(), TimeUtil.nowTs())) {
-      Op op = new Op(rsrc.getReviewerUser().getAccount());
+      Op op = new Op(rsrc.getReviewerUser().getAccount(), input);
       bu.addOp(rsrc.getChange().getId(), op);
       bu.execute();
     }
@@ -119,6 +125,7 @@ public class DeleteReviewer implements RestModifyView<ReviewerResource, Input> {
 
   private class Op extends BatchUpdate.Op {
     private final Account reviewer;
+    private final DeleteReviewerInput input;
     ChangeMessage changeMessage;
     Change currChange;
     PatchSet currPs;
@@ -126,8 +133,9 @@ public class DeleteReviewer implements RestModifyView<ReviewerResource, Input> {
     Map<String, Short> newApprovals = new HashMap<>();
     Map<String, Short> oldApprovals = new HashMap<>();
 
-    Op(Account reviewerAccount) {
+    Op(Account reviewerAccount, DeleteReviewerInput input) {
       this.reviewer = reviewerAccount;
+      this.input = input;
     }
 
     @Override
@@ -190,11 +198,14 @@ public class DeleteReviewer implements RestModifyView<ReviewerResource, Input> {
 
     @Override
     public void postUpdate(Context ctx) {
-      emailReviewers(ctx.getProject(), currChange, del, changeMessage);
+      if (input.notify.compareTo(NotifyHandling.NONE) > 0) {
+        emailReviewers(ctx.getProject(), currChange, del, changeMessage);
+      }
       reviewerDeleted.fire(currChange, currPs, reviewer,
           ctx.getAccount(),
           changeMessage.getMessage(),
           newApprovals, oldApprovals,
+          input.notify,
           ctx.getWhen());
     }
 
@@ -234,30 +245,31 @@ public class DeleteReviewer implements RestModifyView<ReviewerResource, Input> {
       }
       return Short.toString(value);
     }
-  }
 
-  private void emailReviewers(Project.NameKey projectName, Change change,
-      List<PatchSetApproval> dels, ChangeMessage changeMessage) {
+    private void emailReviewers(Project.NameKey projectName, Change change,
+        List<PatchSetApproval> dels, ChangeMessage changeMessage) {
 
-    // The user knows they removed themselves, don't bother emailing them.
-    List<Account.Id> toMail = Lists.newArrayListWithCapacity(dels.size());
-    Account.Id userId = user.get().getAccountId();
-    for (PatchSetApproval psa : dels) {
-      if (!psa.getAccountId().equals(userId)) {
-        toMail.add(psa.getAccountId());
+      // The user knows they removed themselves, don't bother emailing them.
+      List<Account.Id> toMail = Lists.newArrayListWithCapacity(dels.size());
+      Account.Id userId = user.get().getAccountId();
+      for (PatchSetApproval psa : dels) {
+        if (!psa.getAccountId().equals(userId)) {
+          toMail.add(psa.getAccountId());
+        }
       }
-    }
-    if (!toMail.isEmpty()) {
-      try {
-        DeleteReviewerSender cm =
-            deleteReviewerSenderFactory.create(projectName, change.getId());
-        cm.setFrom(userId);
-        cm.addReviewers(toMail);
-        cm.setChangeMessage(changeMessage.getMessage(),
-            changeMessage.getWrittenOn());
-        cm.send();
-      } catch (Exception err) {
-        log.error("Cannot email update for change " + change.getId(), err);
+      if (!toMail.isEmpty()) {
+        try {
+          DeleteReviewerSender cm =
+              deleteReviewerSenderFactory.create(projectName, change.getId());
+          cm.setFrom(userId);
+          cm.addReviewers(toMail);
+          cm.setChangeMessage(changeMessage.getMessage(),
+              changeMessage.getWrittenOn());
+          cm.setNotify(input.notify);
+          cm.send();
+        } catch (Exception err) {
+          log.error("Cannot email update for change " + change.getId(), err);
+        }
       }
     }
   }

gerrit-server/src/main/java/com/google/gerrit/server/change/Module.java

@@ -78,6 +78,7 @@ public class Module extends RestApiModule {
     child(CHANGE_KIND, "reviewers").to(Reviewers.class);
     get(REVIEWER_KIND).to(GetReviewer.class);
     delete(REVIEWER_KIND).to(DeleteReviewer.class);
+    post(REVIEWER_KIND, "delete").to(DeleteReviewer.class);
     child(REVIEWER_KIND, "votes").to(Votes.class);
     delete(VOTE_KIND).to(DeleteVote.class);
     post(VOTE_KIND, "delete").to(DeleteVote.class);

gerrit-server/src/main/java/com/google/gerrit/server/extensions/events/ReviewerDeleted.java

@@ -53,12 +53,13 @@ public class ReviewerDeleted {
   public void fire(ChangeInfo change, RevisionInfo revision,
       AccountInfo reviewer, AccountInfo remover, String message,
       Map<String, ApprovalInfo> newApprovals,
-      Map<String, ApprovalInfo> oldApprovals, Timestamp when) {
+      Map<String, ApprovalInfo> oldApprovals, NotifyHandling notify,
+      Timestamp when) {
     if (!listeners.iterator().hasNext()) {
       return;
     }
     Event event = new Event(change, revision, reviewer, remover, message,
-        newApprovals, oldApprovals, when);
+        newApprovals, oldApprovals, notify, when);
     for (ReviewerDeletedListener listener : listeners) {
       try {
         listener.onReviewerDeleted(event);
@@ -69,9 +70,8 @@ public class ReviewerDeleted {
   }
 
   public void fire(Change change, PatchSet patchSet, Account reviewer,
-      Account remover, String message,
-      Map<String, Short> newApprovals,
-      Map<String, Short> oldApprovals, Timestamp when) {
+      Account remover, String message, Map<String, Short> newApprovals,
+      Map<String, Short> oldApprovals, NotifyHandling notify, Timestamp when) {
     if (!listeners.iterator().hasNext()) {
       return;
     }
@@ -83,6 +83,7 @@ public class ReviewerDeleted {
           message,
           util.approvals(reviewer, newApprovals, when),
           util.approvals(reviewer, oldApprovals, when),
+          notify,
           when);
     } catch (PatchListNotAvailableException | GpgException | IOException
         | OrmException e) {
@@ -101,8 +102,9 @@ public class ReviewerDeleted {
     Event(ChangeInfo change, RevisionInfo revision, AccountInfo reviewer,
         AccountInfo remover, String comment,
         Map<String, ApprovalInfo> newApprovals,
-        Map<String, ApprovalInfo> oldApprovals, Timestamp when) {
-      super(change, revision, remover, when, NotifyHandling.ALL);
+        Map<String, ApprovalInfo> oldApprovals, NotifyHandling notify,
+        Timestamp when) {
+      super(change, revision, remover, when, notify);
       this.reviewer = reviewer;
       this.comment = comment;
       this.newApprovals = newApprovals;

gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/SetReviewersCommand.java

@@ -15,6 +15,7 @@
 package com.google.gerrit.sshd.commands;
 
 import com.google.gerrit.extensions.api.changes.AddReviewerInput;
+import com.google.gerrit.extensions.api.changes.DeleteReviewerInput;
 import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.Change;
@@ -111,7 +112,7 @@ public class SetReviewersCommand extends SshCommand {
       ReviewerResource rsrc = reviewerFactory.create(changeRsrc, reviewer);
       String error = null;
       try {
-        deleteReviewer.apply(rsrc, new DeleteReviewer.Input());
+        deleteReviewer.apply(rsrc, new DeleteReviewerInput());
       } catch (ResourceNotFoundException e) {
         error = String.format("could not remove %s: not found", reviewer);
       } catch (Exception e) {