Merge changes I32dcce61,I17d7181d

* changes:
  Refactor to add reviewers/CCs through PostReview
  Add support for CC state to AddReviewerInput

Documentation/rest-api-changes.txt

@@ -2817,6 +2817,73 @@ describes the applied labels.
 A review cannot be set on a change edit. Trying to post a review for a
 change edit fails with `409 Conflict`.
 
+It is also possible to add one or more reviewers to a change simultaneously
+with a review.
+
+.Request
+----
+  POST /changes/myProject~master~I8473b95934b5732ac55d26311a706c9c2bde9940/revisions/674ac754f91e64a0efb8087e59a176484bd534d1/review HTTP/1.0
+  Content-Type: application/json; charset=UTF-8
+
+  {
+    "message": "Looks good to me, but Jane and John should also take a look.",
+    "labels": {
+      "Code-Review": 1
+    },
+    "reviewers": [
+      {
+        "reviewer": "jane.roe@example.com"
+      },
+      {
+        "reviewer": "john.doe@example.com"
+      }
+    ]
+  }
+----
+
+Each element of the `reviewers` list is an instance of
+link:#reviewer-input[ReviewerInput]. The corresponding result of
+adding each reviewer will be returned in a list of
+link:#add-reviewer-result[AddReviewerResult]. If there are any
+errors returned for reviewers, the entire review request will
+be rejected.
+
+.Response
+----
+  HTTP/1.1 200 OK
+  Content-Disposition: attachment
+  Content-Type: application/json; charset=UTF-8
+
+  )]}'
+  {
+    "labels": {
+      "Code-Review": 1
+    },
+    "reviewers": [
+      {
+        "input": "jane.roe@example.com",
+        "approvals": {
+          "Verified": " 0",
+          "Code-Review": " 0"
+        },
+        "_account_id": 1000097,
+        "name": "Jane Roe",
+        "email": "jane.roe@example.com"
+      },
+      {
+        "input": "john.doe@example.com",
+        "approvals": {
+          "Verified": " 0",
+          "Code-Review": " 0"
+        },
+        "_account_id": 1000096,
+        "name": "John Doe",
+        "email": "john.doe@example.com"
+      }
+    ]
+  }
+----
+
 [[rebase-revision]]
 === Rebase Revision
 --
@@ -4057,6 +4124,11 @@ set while adding the reviewer.
 |`reviewers`   |optional|
 The newly added reviewers as a list of link:#reviewer-info[
 ReviewerInfo] entities.
+|`ccs`         |optional|
+The newly CCed accounts as a list of link:#reviewer-info[
+ReviewerInfo] entities. This field will only appear if the requested
+`state` for the reviewer was `CC` *and* NoteDb is enabled on the
+server.
 |`error`       |optional|
 Error message explaining why the reviewer could not be added. +
 If a group was specified in the input and an error is returned, it
@@ -4989,6 +5061,9 @@ should be added as reviewer or the link:rest-api-groups.html#group-id[
 ID] of one group for which all members should be added as reviewers. +
 If an ID identifies both an account and a group, only the account is
 added as reviewer to the change.
+|`state`       |optional|
+Add reviewer in this state. Possible reviewer states are `REVIEWER`
+and `CC`. If not given, defaults to `REVIEWER`.
 |`confirmed`   |optional|
 Whether adding the reviewer is confirmed. +
 The Gerrit server may be configured to

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/ChangeReviewersIT.java

@@ -15,6 +15,8 @@
 package com.google.gerrit.acceptance.rest.change;
 
 import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.extensions.client.ReviewerState.CC;
+import static com.google.gerrit.extensions.client.ReviewerState.REVIEWER;
 
 import com.google.gerrit.acceptance.AbstractDaemonTest;
 import com.google.gerrit.acceptance.PushOneCommit;
@@ -22,12 +24,20 @@ import com.google.gerrit.acceptance.RestResponse;
 import com.google.gerrit.acceptance.TestAccount;
 import com.google.gerrit.extensions.api.changes.AddReviewerInput;
 import com.google.gerrit.extensions.api.changes.AddReviewerResult;
+import com.google.gerrit.extensions.api.changes.ReviewInput;
+import com.google.gerrit.extensions.api.changes.ReviewResult;
+import com.google.gerrit.extensions.client.ReviewerState;
+import com.google.gerrit.extensions.common.AccountInfo;
+import com.google.gerrit.extensions.common.ChangeInfo;
 import com.google.gerrit.server.change.PostReviewers;
+import com.google.gerrit.server.mail.Address;
+import com.google.gerrit.testutil.FakeEmailSender.Message;
 import com.google.gson.stream.JsonReader;
 
 import org.junit.Test;
 
 import java.util.ArrayList;
+import java.util.Collection;
 import java.util.List;
 
 public class ChangeReviewersIT extends AbstractDaemonTest {
@@ -40,17 +50,14 @@ public class ChangeReviewersIT extends AbstractDaemonTest {
 
     int largeGroupSize = PostReviewers.DEFAULT_MAX_REVIEWERS + 1;
     int mediumGroupSize = PostReviewers.DEFAULT_MAX_REVIEWERS_WITHOUT_CHECK + 1;
-    List<TestAccount> users = new ArrayList<>(largeGroupSize);
+    List<TestAccount> users =
+        createAccounts(largeGroupSize, "addGroupAsReviewer");
     List<String> largeGroupUsernames = new ArrayList<>(mediumGroupSize);
-    List<String> mediumGroupUsernames = new ArrayList<>(mediumGroupSize);
+    for (TestAccount u : users) {
-    for (int i = 0; i < largeGroupSize; i++) {
+      largeGroupUsernames.add(u.username);
-      users.add(accounts.create(name("u" + i), name("u" + i + "@example.com"),
-          "Full Name " + i));
-      largeGroupUsernames.add(users.get(i).username);
-      if (i < mediumGroupSize) {
-        mediumGroupUsernames.add(users.get(i).username);
-      }
     }
+    List<String> mediumGroupUsernames =
+        largeGroupUsernames.subList(0, mediumGroupSize);
     gApi.groups().id(largeGroup).addMembers(
         largeGroupUsernames.toArray(new String[largeGroupSize]));
     gApi.groups().id(mediumGroup).addMembers(
@@ -84,6 +91,311 @@ public class ChangeReviewersIT extends AbstractDaemonTest {
     assertThat(result.confirm).isNull();
     assertThat(result.error).isNull();
     assertThat(result.reviewers).hasSize(mediumGroupSize);
+
+    // Verify that group members were added as reviewers.
+    ChangeInfo c = gApi.changes().id(r.getChangeId()).get();
+    assertReviewers(c, notesMigration.readChanges() ? REVIEWER : CC,
+        users.subList(0, mediumGroupSize));
+  }
+
+  @Test
+  public void addCcAccount() throws Exception {
+    PushOneCommit.Result r = createChange();
+    String changeId = r.getChangeId();
+    AddReviewerInput in = new AddReviewerInput();
+    in.reviewer = user.email;
+    in.state = CC;
+    AddReviewerResult result = addReviewer(changeId, in);
+
+    assertThat(result.input).isEqualTo(user.email);
+    assertThat(result.confirm).isNull();
+    assertThat(result.error).isNull();
+    ChangeInfo c = gApi.changes().id(r.getChangeId()).get();
+    if (notesMigration.readChanges()) {
+      assertThat(result.reviewers).isNull();
+      assertThat(result.ccs).hasSize(1);
+      AccountInfo ai = result.ccs.get(0);
+      assertThat(ai._accountId).isEqualTo(user.id.get());
+      assertReviewers(c, CC, user);
+    } else {
+      assertThat(result.ccs).isNull();
+      assertThat(result.reviewers).hasSize(1);
+      AccountInfo ai = result.reviewers.get(0);
+      assertThat(ai._accountId).isEqualTo(user.id.get());
+      assertReviewers(c, CC, user);
+    }
+
+    // Verify email was sent to CCed account.
+    List<Message> messages = sender.getMessages();
+    assertThat(messages).hasSize(1);
+    Message m = messages.get(0);
+    assertThat(m.rcpt()).containsExactly(user.emailAddress);
+    if (notesMigration.readChanges()) {
+      assertThat(m.body())
+          .contains(admin.fullName + " has uploaded a new change for review.");
+    } else {
+      assertThat(m.body()).contains("Hello " + user.fullName + ",\n");
+      assertThat(m.body()).contains("I'd like you to do a code review.");
+    }
+  }
+
+  @Test
+  public void addCcGroup() throws Exception {
+    List<TestAccount> users = createAccounts(6, "addCcGroup");
+    List<String> usernames = new ArrayList<>(6);
+    for (TestAccount u : users) {
+      usernames.add(u.username);
+    }
+
+    List<TestAccount> firstUsers = users.subList(0, 3);
+    List<String> firstUsernames = usernames.subList(0, 3);
+
+    PushOneCommit.Result r = createChange();
+    String changeId = r.getChangeId();
+    AddReviewerInput in = new AddReviewerInput();
+    in.reviewer = createGroup("cc1");
+    in.state = CC;
+    gApi.groups().id(in.reviewer)
+        .addMembers(firstUsernames.toArray(new String[firstUsernames.size()]));
+    AddReviewerResult result = addReviewer(changeId, in);
+
+    assertThat(result.input).isEqualTo(in.reviewer);
+    assertThat(result.confirm).isNull();
+    assertThat(result.error).isNull();
+    if (notesMigration.readChanges()) {
+      assertThat(result.reviewers).isNull();
+    } else {
+      assertThat(result.ccs).isNull();
+    }
+    ChangeInfo c = gApi.changes().id(r.getChangeId()).get();
+    assertReviewers(c, CC, firstUsers);
+
+    // Verify emails were sent to each of the group's accounts.
+    List<Message> messages = sender.getMessages();
+    assertThat(messages).hasSize(1);
+    Message m = messages.get(0);
+    List<Address> expectedAddresses = new ArrayList<>(firstUsers.size());
+    for (TestAccount u : firstUsers) {
+      expectedAddresses.add(u.emailAddress);
+    }
+    assertThat(m.rcpt()).containsExactlyElementsIn(expectedAddresses);
+
+    // CC a group that overlaps with some existing reviewers and CCed accounts.
+    TestAccount reviewer = accounts.create(name("reviewer"),
+        "addCcGroup-reviewer@example.com", "Reviewer");
+    result = addReviewer(changeId, reviewer.username);
+    assertThat(result.error).isNull();
+    sender.clear();
+    in.reviewer = createGroup("cc2");
+    gApi.groups().id(in.reviewer)
+        .addMembers(usernames.toArray(new String[usernames.size()]));
+    gApi.groups().id(in.reviewer).addMembers(reviewer.username);
+    result = addReviewer(changeId, in);
+    assertThat(result.input).isEqualTo(in.reviewer);
+    assertThat(result.confirm).isNull();
+    assertThat(result.error).isNull();
+    c = gApi.changes().id(r.getChangeId()).get();
+    if (notesMigration.readChanges()) {
+      assertThat(result.ccs).hasSize(3);
+      assertThat(result.reviewers).isNull();
+      assertReviewers(c, REVIEWER, reviewer);
+      assertReviewers(c, CC, users);
+    } else {
+      assertThat(result.ccs).isNull();
+      assertThat(result.reviewers).hasSize(3);
+      List<TestAccount> expectedUsers = new ArrayList<>(users.size() + 2);
+      expectedUsers.addAll(users);
+      expectedUsers.add(reviewer);
+      assertReviewers(c, CC, expectedUsers);
+    }
+
+    messages = sender.getMessages();
+    assertThat(messages).hasSize(1);
+    m = messages.get(0);
+    expectedAddresses = new ArrayList<>(4);
+    for (int i = 0; i < 3; i++) {
+      expectedAddresses.add(users.get(users.size() - i - 1).emailAddress);
+    }
+    if (notesMigration.readChanges()) {
+      expectedAddresses.add(reviewer.emailAddress);
+    }
+    assertThat(m.rcpt()).containsExactlyElementsIn(expectedAddresses);
+  }
+
+  @Test
+  public void transitionCcToReviewer() throws Exception {
+    PushOneCommit.Result r = createChange();
+    String changeId = r.getChangeId();
+    AddReviewerInput in = new AddReviewerInput();
+    in.reviewer = user.email;
+    in.state = CC;
+    addReviewer(changeId, in);
+    ChangeInfo c = gApi.changes().id(r.getChangeId()).get();
+    assertReviewers(c, REVIEWER);
+    assertReviewers(c, CC, user);
+
+    in.state = REVIEWER;
+    addReviewer(changeId, in);
+    c = gApi.changes().id(r.getChangeId()).get();
+    if (notesMigration.readChanges()) {
+      assertReviewers(c, REVIEWER, user);
+      assertReviewers(c, CC);
+    } else {
+      // If NoteDb not enabled, should have had no effect.
+      assertReviewers(c, REVIEWER);
+      assertReviewers(c, CC, user);
+    }
+  }
+
+  @Test
+  public void reviewAndAddReviewers() throws Exception {
+    TestAccount observer = accounts.user2();
+    PushOneCommit.Result r = createChange();
+    ReviewInput input = ReviewInput.approve()
+        .reviewer(user.email)
+        .reviewer(observer.email, CC, false);
+
+    ReviewResult result = review(r.getChangeId(), r.getCommit().name(), input);
+    assertThat(result.labels).isNotNull();
+    assertThat(result.reviewers).isNotNull();
+    assertThat(result.reviewers).hasSize(2);
+
+    // Verify reviewer and CC were added. If not in NoteDb read mode, both
+    // parties will be returned as CCed.
+    ChangeInfo c = gApi.changes()
+        .id(r.getChangeId())
+        .get();
+    if (notesMigration.readChanges()) {
+      assertReviewers(c, REVIEWER, admin, user);
+      assertReviewers(c, CC, observer);
+    } else {
+      // In legacy mode, change owner should be the only reviewer.
+      assertReviewers(c, REVIEWER, admin);
+      assertReviewers(c, CC, user, observer);
+   }
+
+    // Verify emails were sent to added reviewers.
+    List<Message> messages = sender.getMessages();
+    assertThat(messages).hasSize(3);
+    // First email to user.
+    Message m = messages.get(0);
+    assertThat(m.rcpt()).containsExactly(user.emailAddress);
+    assertThat(m.body()).contains("Hello " + user.fullName + ",\n");
+    assertThat(m.body()).contains("I'd like you to do a code review.");
+    assertThat(m.body()).contains("Change subject: " + PushOneCommit.SUBJECT + "\n");
+    // Second email to reviewer and observer.
+    m = messages.get(1);
+    if (notesMigration.readChanges()) {
+      assertThat(m.rcpt()).containsExactly(user.emailAddress, observer.emailAddress);
+      assertThat(m.body()).contains(admin.fullName + " has uploaded a new change for review.");
+    } else {
+      assertThat(m.rcpt()).containsExactly(observer.emailAddress);
+      assertThat(m.body()).contains("Hello " + observer.fullName + ",\n");
+      assertThat(m.body()).contains("I'd like you to do a code review.");
+    }
+
+    // Third email is review to user and observer.
+    m = messages.get(2);
+    assertThat(m.rcpt()).containsExactly(user.emailAddress, observer.emailAddress);
+    assertThat(m.body()).contains(admin.fullName + " has posted comments on this change.");
+    assertThat(m.body()).contains("Change subject: " + PushOneCommit.SUBJECT + "\n");
+    assertThat(m.body()).contains("Patch Set 1: Code-Review+2\n");
+  }
+
+  @Test
+  public void reviewAndAddGroupReviewers() throws Exception {
+    int largeGroupSize = PostReviewers.DEFAULT_MAX_REVIEWERS + 1;
+    int mediumGroupSize = PostReviewers.DEFAULT_MAX_REVIEWERS_WITHOUT_CHECK + 1;
+    List<TestAccount> users =
+        createAccounts(largeGroupSize, "reviewAndAddGroupReviewers");
+    List<String> usernames = new ArrayList<>(largeGroupSize);
+    for (TestAccount u : users) {
+      usernames.add(u.username);
+    }
+
+    String largeGroup = createGroup("largeGroup");
+    String mediumGroup = createGroup("mediumGroup");
+    gApi.groups().id(largeGroup).addMembers(
+        usernames.toArray(new String[largeGroupSize]));
+    gApi.groups().id(mediumGroup).addMembers(
+        usernames.subList(0, mediumGroupSize)
+            .toArray(new String[mediumGroupSize]));
+
+    TestAccount observer = accounts.user2();
+    PushOneCommit.Result r = createChange();
+
+    // Attempt to add overly large group as reviewers.
+    ReviewInput input = ReviewInput.approve()
+        .reviewer(user.email)
+        .reviewer(observer.email, CC, false)
+        .reviewer(largeGroup);
+    ReviewResult result = review(r.getChangeId(), r.getCommit().name(), input);
+    assertThat(result.labels).isNull();
+    assertThat(result.reviewers).isNotNull();
+    assertThat(result.reviewers).hasSize(3);
+    AddReviewerResult reviewerResult = result.reviewers.get(largeGroup);
+    assertThat(reviewerResult).isNotNull();
+    assertThat(reviewerResult.confirm).isNull();
+    assertThat(reviewerResult.error).isNotNull();
+    assertThat(reviewerResult.error).contains("has too many members to add them all as reviewers");
+
+    // No labels should have changed, and no reviewers/CCs should have been added.
+    ChangeInfo c = gApi.changes()
+        .id(r.getChangeId())
+        .get();
+    assertThat(c.messages).hasSize(1);
+    assertThat(c.reviewers.get(REVIEWER)).isNull();
+    assertThat(c.reviewers.get(CC)).isNull();
+
+    // Attempt to add group large enough to require confirmation, without
+    // confirmation, as reviewers.
+    input = ReviewInput.approve()
+        .reviewer(user.email)
+        .reviewer(observer.email, CC, false)
+        .reviewer(mediumGroup);
+    result = review(r.getChangeId(), r.getCommit().name(), input);
+    assertThat(result.labels).isNull();
+    assertThat(result.reviewers).isNotNull();
+    assertThat(result.reviewers).hasSize(3);
+    reviewerResult = result.reviewers.get(mediumGroup);
+    assertThat(reviewerResult).isNotNull();
+    assertThat(reviewerResult.confirm).isTrue();
+    assertThat(reviewerResult.error)
+        .contains("has " + mediumGroupSize + " members. Do you want to add them all"
+            + " as reviewers?");
+
+    // No labels should have changed, and no reviewers/CCs should have been added.
+    c = gApi.changes()
+        .id(r.getChangeId())
+        .get();
+    assertThat(c.messages).hasSize(1);
+    assertThat(c.reviewers.get(REVIEWER)).isNull();
+    assertThat(c.reviewers.get(CC)).isNull();
+
+    // Retrying with confirmation should successfully approve and add reviewers/CCs.
+    input = ReviewInput.approve()
+        .reviewer(user.email)
+        .reviewer(mediumGroup, CC, true);
+    result = review(r.getChangeId(), r.getCommit().name(), input);
+    assertThat(result.labels).isNotNull();
+    assertThat(result.reviewers).isNotNull();
+    assertThat(result.reviewers).hasSize(2);
+
+    c = gApi.changes()
+        .id(r.getChangeId())
+        .get();
+    assertThat(c.messages).hasSize(2);
+
+    if (notesMigration.readChanges()) {
+      assertReviewers(c, REVIEWER, admin, user);
+      assertReviewers(c, CC, users.subList(0, mediumGroupSize));
+    } else {
+      // If not in NoteDb mode, then user is returned with the CC group.
+      assertReviewers(c, REVIEWER, admin);
+      List<TestAccount> expectedCC = users.subList(0, mediumGroupSize);
+      expectedCC.add(user);
+      assertReviewers(c, CC, expectedCC);
+    }
   }
 
   private AddReviewerResult addReviewer(String changeId, String reviewer)
@@ -100,6 +412,12 @@ public class ChangeReviewersIT extends AbstractDaemonTest {
     return readContentFromJson(resp, AddReviewerResult.class);
   }
 
+  private ReviewResult review(String changeId, String revisionId, ReviewInput in) throws Exception {
+    RestResponse resp = adminRestSession.post(
+        "/changes/" + changeId + "/revisions/" + revisionId + "/review", in);
+    return readContentFromJson(resp, ReviewResult.class);
+  }
+
   private static <T> T readContentFromJson(RestResponse r, Class<T> clazz)
       throws Exception {
     r.assertOK();
@@ -107,4 +425,42 @@ public class ChangeReviewersIT extends AbstractDaemonTest {
     jsonReader.setLenient(true);
     return newGson().fromJson(jsonReader, clazz);
   }
+
+  private static void assertReviewers(ChangeInfo c, ReviewerState reviewerState,
+      TestAccount... accounts) throws Exception {
+    List<TestAccount> accountList = new ArrayList<>(accounts.length);
+    for (TestAccount a : accounts) {
+      accountList.add(a);
+    }
+    assertReviewers(c, reviewerState, accountList);
+  }
+
+  private static void assertReviewers(ChangeInfo c, ReviewerState reviewerState,
+      Iterable<TestAccount> accounts) throws Exception {
+    Collection<AccountInfo> actualAccounts = c.reviewers.get(reviewerState);
+    if (actualAccounts == null) {
+      assertThat(accounts.iterator().hasNext()).isFalse();
+      return;
+    }
+    assertThat(actualAccounts).isNotNull();
+    List<Integer> actualAccountIds = new ArrayList<>(actualAccounts.size());
+    for (AccountInfo account : actualAccounts) {
+      actualAccountIds.add(account._accountId);
+    }
+    List<Integer> expectedAccountIds = new ArrayList<>();
+    for (TestAccount account : accounts) {
+      expectedAccountIds.add(account.getId().get());
+    }
+    assertThat(actualAccountIds).containsExactlyElementsIn(expectedAccountIds);
+  }
+
+  private List<TestAccount> createAccounts(int n, String emailPrefix)
+      throws Exception {
+    List<TestAccount> result = new ArrayList<>(n);
+    for (int i = 0; i < n; i++) {
+      result.add(accounts.create(name("u" + i),
+          emailPrefix + "-" + i + "@example.com", "Full Name " + i));
+    }
+    return result;
+  }
 }

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

@@ -14,15 +14,22 @@
 
 package com.google.gerrit.extensions.api.changes;
 
+import static com.google.gerrit.extensions.client.ReviewerState.REVIEWER;
+
+import com.google.gerrit.extensions.client.ReviewerState;
 import com.google.gerrit.extensions.restapi.DefaultInput;
 
 public class AddReviewerInput {
   @DefaultInput
   public String reviewer;
   public Boolean confirmed;
+  public ReviewerState state;
 
   public boolean confirmed() {
     return (confirmed != null) ? confirmed : false;
   }
-}
 
+  public ReviewerState state() {
+    return (state != null) ? state : REVIEWER;
+  }
+}

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

@@ -15,6 +15,7 @@
 package com.google.gerrit.extensions.api.changes;
 
 import com.google.gerrit.common.Nullable;
+import com.google.gerrit.extensions.common.AccountInfo;
 
 import java.util.List;
 
@@ -48,6 +49,14 @@ public class AddReviewerResult {
   @Nullable
   public List<ReviewerInfo> reviewers;
 
+  /**
+   * List of accounts CCed on the change. The size of this list may be
+   * greater than one (e.g. when a group is CCed). Null if no accounts were CCed
+   * or if reviewers is non-null.
+   */
+  @Nullable
+  public List<AccountInfo> ccs;
+
   /**
    * Constructs a partially initialized result for the given reviewer.
    *

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

@@ -14,9 +14,13 @@
 
 package com.google.gerrit.extensions.api.changes;
 
+import static com.google.gerrit.extensions.client.ReviewerState.REVIEWER;
+
 import com.google.gerrit.extensions.client.Comment;
+import com.google.gerrit.extensions.client.ReviewerState;
 import com.google.gerrit.extensions.restapi.DefaultInput;
 
+import java.util.ArrayList;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
@@ -68,6 +72,11 @@ public class ReviewInput {
    */
   public String onBehalfOf;
 
+  /**
+   * Reviewers that should be added to this change.
+   */
+  public List<AddReviewerInput> reviewers;
+
   public enum DraftHandling {
     /** Delete pending drafts on this revision only. */
     DELETE,
@@ -112,6 +121,23 @@ public class ReviewInput {
     return label(name, (short) 1);
   }
 
+  public ReviewInput reviewer(String reviewer) {
+    return reviewer(reviewer, REVIEWER, false);
+  }
+
+  public ReviewInput reviewer(String reviewer, ReviewerState state,
+      boolean confirmed) {
+    AddReviewerInput input = new AddReviewerInput();
+    input.reviewer = reviewer;
+    input.state = state;
+    input.confirmed = confirmed;
+    if (reviewers == null) {
+      reviewers = new ArrayList<>();
+    }
+    reviewers.add(input);
+    return this;
+  }
+
   public static ReviewInput recommend() {
     return new ReviewInput().label("Code-Review", 1);
   }

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

@@ -0,0 +1,38 @@
+// 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;
+
+import com.google.gerrit.common.Nullable;
+
+import java.util.Map;
+
+/**
+ * Result object representing the outcome of a review request.
+ */
+public class ReviewResult {
+  /**
+   * Map of labels to values after the review was posted. Null if any
+   * reviewer additions were rejected.
+   */
+  @Nullable
+  public Map<String, Short> labels;
+
+  /**
+   * Map of account or group identifier to outcome of adding as a reviewer.
+   * Null if no reviewer additions were requested.
+   */
+  @Nullable
+  public Map<String, AddReviewerResult> reviewers;
+}

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

@@ -14,6 +14,7 @@
 
 package com.google.gerrit.server;
 
+import static com.google.gerrit.server.notedb.ReviewerStateInternal.CC;
 import static com.google.gerrit.server.notedb.ReviewerStateInternal.REVIEWER;
 
 import com.google.common.annotations.VisibleForTesting;
@@ -51,6 +52,7 @@ import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.Date;
+import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
@@ -122,7 +124,7 @@ public class ApprovalsUtil {
   }
 
   /**
-   * Get all reviewers for a change.
+   * Get all reviewers and CCed accounts for a change.
    *
    * @param allApprovals all approvals to consider; must all belong to the same
    *     change.
@@ -151,8 +153,16 @@ public class ApprovalsUtil {
       ChangeUpdate update, LabelTypes labelTypes, Change change,
       Iterable<Account.Id> wantReviewers) throws OrmException {
     PatchSet.Id psId = change.currentPatchSetId();
+    Collection<Account.Id> existingReviewers;
+    if (migration.readChanges()) {
+      // If using NoteDB, we only want reviewers in the REVIEWER state.
+      existingReviewers = notes.load().getReviewers().byState(REVIEWER);
+    } else {
+      // Prior to NoteDB, we gather all reviewers regardless of state.
+      existingReviewers = getReviewers(db, notes).all();
+    }
     return addReviewers(db, update, labelTypes, change, psId, false, null, null,
-        wantReviewers, getReviewers(db, notes).all());
+        wantReviewers, existingReviewers);
   }
 
   private List<PatchSetApproval> addReviewers(ReviewDb db, ChangeUpdate update,
@@ -191,6 +201,30 @@ public class ApprovalsUtil {
     return Collections.unmodifiableList(cells);
   }
 
+  /**
+   * Adds accounts to a change as reviewers in the CC state.
+   *
+   * @param notes change notes.
+   * @param update change update.
+   * @param wantCCs accounts to CC.
+   * @return whether a change was made.
+   * @throws OrmException
+   */
+  public Collection<Account.Id> addCcs(ChangeNotes notes, ChangeUpdate update,
+      Collection<Account.Id> wantCCs) throws OrmException {
+    return addCcs(update, wantCCs, notes.load().getReviewers());
+  }
+
+  private Collection<Account.Id> addCcs(ChangeUpdate update,
+      Collection<Account.Id> wantCCs, ReviewerSet existingReviewers) {
+    Set<Account.Id> need = new LinkedHashSet<>(wantCCs);
+    need.removeAll(existingReviewers.all());
+    for (Account.Id account : need) {
+      update.putReviewer(account, CC);
+    }
+    return need;
+  }
+
   public void addApprovals(ReviewDb db, ChangeUpdate update,
       LabelTypes labelTypes, PatchSet ps, ChangeControl changeCtl,
       Map<String, Short> approvals) throws OrmException {

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

@@ -166,7 +166,7 @@ class RevisionApiImpl implements RevisionApi {
   public void review(ReviewInput in) throws RestApiException {
     try {
       review.apply(revision, in);
-    } catch (OrmException | UpdateException e) {
+    } catch (OrmException | UpdateException | IOException e) {
       throw new RestApiException("Cannot post review", e);
     }
   }

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

@@ -24,6 +24,8 @@ import com.google.auto.value.AutoValue;
 import com.google.common.base.MoreObjects;
 import com.google.common.base.Strings;
 import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Lists;
+import com.google.common.collect.Maps;
 import com.google.common.collect.Ordering;
 import com.google.common.collect.Sets;
 import com.google.common.hash.HashCode;
@@ -34,10 +36,13 @@ import com.google.gerrit.common.data.LabelType;
 import com.google.gerrit.common.data.LabelTypes;
 import com.google.gerrit.common.data.Permission;
 import com.google.gerrit.common.data.PermissionRange;
+import com.google.gerrit.extensions.api.changes.AddReviewerInput;
+import com.google.gerrit.extensions.api.changes.AddReviewerResult;
 import com.google.gerrit.extensions.api.changes.NotifyHandling;
 import com.google.gerrit.extensions.api.changes.ReviewInput;
 import com.google.gerrit.extensions.api.changes.ReviewInput.CommentInput;
 import com.google.gerrit.extensions.api.changes.ReviewInput.DraftHandling;
+import com.google.gerrit.extensions.api.changes.ReviewResult;
 import com.google.gerrit.extensions.client.Side;
 import com.google.gerrit.extensions.restapi.AuthException;
 import com.google.gerrit.extensions.restapi.BadRequestException;
@@ -79,6 +84,7 @@ import com.google.inject.Singleton;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import java.io.IOException;
 import java.sql.Timestamp;
 import java.util.ArrayList;
 import java.util.Collection;
@@ -94,10 +100,6 @@ import java.util.Set;
 public class PostReview implements RestModifyView<RevisionResource, ReviewInput> {
   private static final Logger log = LoggerFactory.getLogger(PostReview.class);
 
-  static class Output {
-    Map<String, Short> labels;
-  }
-
   private final Provider<ReviewDb> db;
   private final BatchUpdate.Factory batchUpdateFactory;
   private final ChangesCollection changes;
@@ -110,6 +112,7 @@ public class PostReview implements RestModifyView<RevisionResource, ReviewInput>
   private final AccountsCollection accounts;
   private final EmailReviewComments.Factory email;
   private final CommentAdded commentAdded;
+  private final PostReviewers postReviewers;
 
   @Inject
   PostReview(Provider<ReviewDb> db,
@@ -123,7 +126,8 @@ public class PostReview implements RestModifyView<RevisionResource, ReviewInput>
       PatchListCache patchListCache,
       AccountsCollection accounts,
       EmailReviewComments.Factory email,
-      CommentAdded commentAdded) {
+      CommentAdded commentAdded,
+      PostReviewers postReviewers) {
     this.db = db;
     this.batchUpdateFactory = batchUpdateFactory;
     this.changes = changes;
@@ -136,16 +140,18 @@ public class PostReview implements RestModifyView<RevisionResource, ReviewInput>
     this.accounts = accounts;
     this.email = email;
     this.commentAdded = commentAdded;
+    this.postReviewers = postReviewers;
   }
 
   @Override
-  public Output apply(RevisionResource revision, ReviewInput input)
+  public ReviewResult apply(RevisionResource revision, ReviewInput input)
-      throws RestApiException, UpdateException, OrmException {
+      throws RestApiException, UpdateException, OrmException, IOException {
     return apply(revision, input, TimeUtil.nowTs());
   }
 
-  public Output apply(RevisionResource revision, ReviewInput input,
+  public ReviewResult apply(RevisionResource revision, ReviewInput input,
-      Timestamp ts) throws RestApiException, UpdateException, OrmException {
+      Timestamp ts)
+      throws RestApiException, UpdateException, OrmException, IOException {
     // Respect timestamp, but truncate at change created-on time.
     ts = Ordering.natural().max(ts, revision.getChange().getCreatedOn());
     if (revision.getEdit().isPresent()) {
@@ -165,15 +171,52 @@ public class PostReview implements RestModifyView<RevisionResource, ReviewInput>
       input.notify = NotifyHandling.NONE;
     }
 
+    Map<String, AddReviewerResult> reviewerJsonResults = null;
+    List<PostReviewers.Addition> reviewerResults = Lists.newArrayList();
+    boolean hasError = false;
+    boolean confirm = false;
+    if (input.reviewers != null) {
+      reviewerJsonResults = Maps.newHashMap();
+      for (AddReviewerInput reviewerInput : input.reviewers) {
+        PostReviewers.Addition result = postReviewers.prepareApplication(
+            revision.getChangeResource(), reviewerInput);
+        reviewerJsonResults.put(reviewerInput.reviewer, result.result);
+        if (result.result.error != null) {
+          hasError = true;
+          continue;
+        }
+        if (result.result.confirm != null) {
+          confirm = true;
+          continue;
+        }
+        reviewerResults.add(result);
+      }
+    }
+
+    ReviewResult output = new ReviewResult();
+    output.reviewers = reviewerJsonResults;
+    if (hasError || confirm) {
+      return output;
+    }
+    output.labels = input.labels;
+
     try (BatchUpdate bu = batchUpdateFactory.create(db.get(),
           revision.getChange().getProject(), revision.getUser(), ts)) {
+      // Apply reviewer changes first. Revision emails should be sent to the
+      // updated set of reviewers.
+      for (PostReviewers.Addition reviewerResult : reviewerResults) {
+        bu.addOp(revision.getChange().getId(), reviewerResult.op);
+      }
       bu.addOp(
           revision.getChange().getId(),
           new Op(revision.getPatchSet().getId(), input));
       bu.execute();
+
+      for (PostReviewers.Addition reviewerResult : reviewerResults) {
+        reviewerResult.gatherResults();
+      }
     }
-    Output output = new Output();
+
-    output.labels = input.labels;
     return output;
   }
 

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

@@ -14,6 +14,9 @@
 
 package com.google.gerrit.server.change;
 
+import static com.google.gerrit.extensions.client.ReviewerState.CC;
+import static com.google.gerrit.extensions.client.ReviewerState.REVIEWER;
+
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Lists;
@@ -23,6 +26,7 @@ import com.google.gerrit.common.errors.NoSuchGroupException;
 import com.google.gerrit.extensions.api.changes.AddReviewerInput;
 import com.google.gerrit.extensions.api.changes.AddReviewerResult;
 import com.google.gerrit.extensions.api.changes.ReviewerInfo;
+import com.google.gerrit.extensions.client.ReviewerState;
 import com.google.gerrit.extensions.restapi.BadRequestException;
 import com.google.gerrit.extensions.restapi.RestApiException;
 import com.google.gerrit.extensions.restapi.RestModifyView;
@@ -49,6 +53,7 @@ import com.google.gerrit.server.git.UpdateException;
 import com.google.gerrit.server.group.GroupsCollection;
 import com.google.gerrit.server.group.SystemGroupBackend;
 import com.google.gerrit.server.mail.AddReviewerSender;
+import com.google.gerrit.server.notedb.NotesMigration;
 import com.google.gerrit.server.project.ChangeControl;
 import com.google.gerrit.server.project.NoSuchProjectException;
 import com.google.gwtorm.server.OrmException;
@@ -62,15 +67,18 @@ import org.slf4j.LoggerFactory;
 
 import java.io.IOException;
 import java.text.MessageFormat;
+import java.util.ArrayList;
+import java.util.Collection;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
 
 @Singleton
-public class PostReviewers implements RestModifyView<ChangeResource, AddReviewerInput> {
+public class PostReviewers
-  private static final Logger log = LoggerFactory
+    implements RestModifyView<ChangeResource, AddReviewerInput> {
-      .getLogger(PostReviewers.class);
+  private static final Logger log =
+      LoggerFactory.getLogger(PostReviewers.class);
 
   public static final int DEFAULT_MAX_REVIEWERS_WITHOUT_CHECK = 10;
   public static final int DEFAULT_MAX_REVIEWERS = 20;
@@ -91,6 +99,7 @@ public class PostReviewers implements RestModifyView<ChangeResource, AddReviewer
   private final AccountCache accountCache;
   private final ReviewerJson json;
   private final ReviewerAdded reviewerAdded;
+  private final NotesMigration migration;
 
   @Inject
   PostReviewers(AccountsCollection accounts,
@@ -108,7 +117,8 @@ public class PostReviewers implements RestModifyView<ChangeResource, AddReviewer
       @GerritServerConfig Config cfg,
       AccountCache accountCache,
       ReviewerJson json,
-      ReviewerAdded reviewerAdded) {
+      ReviewerAdded reviewerAdded,
+      NotesMigration migration) {
     this.accounts = accounts;
     this.reviewerFactory = reviewerFactory;
     this.approvalsUtil = approvalsUtil;
@@ -125,49 +135,64 @@ public class PostReviewers implements RestModifyView<ChangeResource, AddReviewer
     this.accountCache = accountCache;
     this.json = json;
     this.reviewerAdded = reviewerAdded;
+    this.migration = migration;
   }
 
   @Override
   public AddReviewerResult apply(ChangeResource rsrc, AddReviewerInput input)
-      throws UpdateException, OrmException, RestApiException, IOException {
+      throws IOException, OrmException, RestApiException, UpdateException {
     if (input.reviewer == null) {
       throw new BadRequestException("missing reviewer field");
     }
 
+    Addition addition = prepareApplication(rsrc, input);
+    if (addition.op == null) {
+      return addition.result;
+    }
+    try (BatchUpdate bu = batchUpdateFactory.create(dbProvider.get(),
+        rsrc.getProject(), rsrc.getUser(), TimeUtil.nowTs())) {
+      Change.Id id = rsrc.getChange().getId();
+      bu.addOp(id, addition.op);
+      bu.execute();
+      addition.gatherResults();
+    }
+    return addition.result;
+  }
+
+  public Addition prepareApplication(ChangeResource rsrc, AddReviewerInput input)
+      throws OrmException, RestApiException, IOException {
     try {
       Account.Id accountId = accounts.parse(input.reviewer).getAccountId();
-      return putAccount(input.reviewer, reviewerFactory.create(rsrc, accountId));
+      return putAccount(input.reviewer, reviewerFactory.create(rsrc, accountId),
+          input.state());
     } catch (UnprocessableEntityException e) {
       try {
         return putGroup(rsrc, input);
       } catch (UnprocessableEntityException e2) {
-        throw new UnprocessableEntityException(MessageFormat.format(
+        throw new UnprocessableEntityException(MessageFormat
-            ChangeMessages.get().reviewerNotFound,
+            .format(ChangeMessages.get().reviewerNotFound, input.reviewer));
-            input.reviewer));
       }
     }
   }
 
-  private AddReviewerResult putAccount(String reviewer, ReviewerResource rsrc)
+  private Addition putAccount(String reviewer, ReviewerResource rsrc,
-      throws OrmException, UpdateException, RestApiException {
+      ReviewerState state) {
     Account member = rsrc.getReviewerUser().getAccount();
     ChangeControl control = rsrc.getReviewerControl();
-    AddReviewerResult result = new AddReviewerResult(reviewer);
     if (isValidReviewer(member, control)) {
-      addReviewers(rsrc.getChangeResource(), result,
+      return new Addition(reviewer, rsrc.getChangeResource(),
-          ImmutableMap.of(member.getId(), control));
+          ImmutableMap.of(member.getId(), control), state);
     }
-    return result;
+    return new Addition(reviewer);
   }
 
-  private AddReviewerResult putGroup(ChangeResource rsrc, AddReviewerInput input)
+  private Addition putGroup(ChangeResource rsrc, AddReviewerInput input)
-      throws UpdateException, RestApiException, OrmException, IOException {
+      throws RestApiException, OrmException, IOException {
-    GroupDescription.Basic group = groupsCollection.parseInternal(input.reviewer);
+    GroupDescription.Basic group =
-    AddReviewerResult result = new AddReviewerResult(input.reviewer);
+        groupsCollection.parseInternal(input.reviewer);
     if (!isLegalReviewerGroup(group.getGroupUUID())) {
-      result.error = MessageFormat.format(
+      return fail(input.reviewer, MessageFormat.format(ChangeMessages.get().groupIsNotAllowed,
-          ChangeMessages.get().groupIsNotAllowed, group.getName());
+          group.getName()));
-      return result;
     }
 
     Map<Account.Id, ChangeControl> reviewers = new HashMap<>();
@@ -187,22 +212,19 @@ public class PostReviewers implements RestModifyView<ChangeResource, AddReviewer
     int maxAllowed =
         cfg.getInt("addreviewer", "maxAllowed", DEFAULT_MAX_REVIEWERS);
     if (maxAllowed > 0 && members.size() > maxAllowed) {
-      result.error = MessageFormat.format(
+      return fail(input.reviewer, MessageFormat.format(
-          ChangeMessages.get().groupHasTooManyMembers, group.getName());
+          ChangeMessages.get().groupHasTooManyMembers, group.getName()));
-      return result;
     }
 
     // if maxWithoutCheck is set to 0, we never ask for confirmation
-    int maxWithoutConfirmation =
+    int maxWithoutConfirmation = cfg.getInt("addreviewer",
-        cfg.getInt("addreviewer", "maxWithoutConfirmation",
+        "maxWithoutConfirmation", DEFAULT_MAX_REVIEWERS_WITHOUT_CHECK);
-            DEFAULT_MAX_REVIEWERS_WITHOUT_CHECK);
     if (!input.confirmed() && maxWithoutConfirmation > 0
         && members.size() > maxWithoutConfirmation) {
-      result.confirm = true;
+      return fail(input.reviewer, true,
-      result.error = MessageFormat.format(
+          MessageFormat.format(
-          ChangeMessages.get().groupManyMembersConfirmation,
+              ChangeMessages.get().groupManyMembersConfirmation,
-          group.getName(), members.size());
+              group.getName(), members.size()));
-      return result;
     }
 
     for (Account member : members) {
@@ -211,8 +233,7 @@ public class PostReviewers implements RestModifyView<ChangeResource, AddReviewer
       }
     }
 
-    addReviewers(rsrc, result, reviewers);
+    return new Addition(input.reviewer, rsrc, reviewers, input.state());
-    return result;
   }
 
   private boolean isValidReviewer(Account member, ChangeControl control) {
@@ -225,80 +246,127 @@ public class PostReviewers implements RestModifyView<ChangeResource, AddReviewer
     return false;
   }
 
+  private Addition fail(String reviewer, String error) {
+    return fail(reviewer, false, error);
+  }
 
-  private void addReviewers(
+  private Addition fail(String reviewer, boolean confirm, String error) {
-      ChangeResource rsrc, AddReviewerResult result, Map<Account.Id, ChangeControl> reviewers)
+    Addition addition = new Addition(reviewer);
-      throws OrmException, RestApiException, UpdateException {
+    addition.result.confirm = confirm ? true : null;
-    try (BatchUpdate bu = batchUpdateFactory.create(
+    addition.result.error = error;
-            dbProvider.get(), rsrc.getProject(), rsrc.getUser(), TimeUtil.nowTs())) {
+    return addition;
-      Op op = new Op(rsrc, reviewers);
+  }
-      Change.Id id = rsrc.getChange().getId();
-      bu.addOp(id, op);
-      bu.execute();
 
-      result.reviewers = Lists.newArrayListWithCapacity(op.added.size());
+  class Addition {
-      for (PatchSetApproval psa : op.added) {
+    final AddReviewerResult result;
-        // New reviewers have value 0, don't bother normalizing.
+    final Op op;
-        result.reviewers.add(
+
-          json.format(new ReviewerInfo(psa.getAccountId().get()),
+    private final Map<Account.Id, ChangeControl> reviewers;
-              reviewers.get(psa.getAccountId()),
+
-              ImmutableList.of(psa)));
+    protected Addition(String reviewer) {
+      this(reviewer, null, null, REVIEWER);
+    }
+
+    protected Addition(String reviewer, ChangeResource rsrc,
+        Map<Account.Id, ChangeControl> reviewers, ReviewerState state) {
+      result = new AddReviewerResult(reviewer);
+      if (reviewers == null) {
+        this.reviewers = ImmutableMap.of();
+        op = null;
+        return;
       }
+      this.reviewers = reviewers;
+      op = new Op(rsrc, reviewers, state);
+    }
 
-      // We don't do this inside Op, since the accounts are in a different
+    void gatherResults() throws OrmException {
-      // table.
+      // Generate result details and fill AccountLoader. This occurs outside
-      accountLoaderFactory.create(true).fill(result.reviewers);
+      // the Op because the accounts are in a different table.
+      if (migration.readChanges() && op.state == CC) {
+        result.ccs = Lists.newArrayListWithCapacity(op.addedCCs.size());
+        for (Account.Id accountId : op.addedCCs) {
+          result.ccs.add(
+              json.format(new ReviewerInfo(accountId.get()), reviewers.get(accountId)));
+        }
+        accountLoaderFactory.create(true).fill(result.ccs);
+      } else {
+        result.reviewers = Lists.newArrayListWithCapacity(op.addedReviewers.size());
+        for (PatchSetApproval psa : op.addedReviewers) {
+          // New reviewers have value 0, don't bother normalizing.
+          result.reviewers.add(
+            json.format(new ReviewerInfo(psa.getAccountId().get()),
+                reviewers.get(psa.getAccountId()),
+                ImmutableList.of(psa)));
+        }
+        accountLoaderFactory.create(true).fill(result.reviewers);
+      }
     }
   }
 
-  private class Op extends BatchUpdate.Op {
+  class Op extends BatchUpdate.Op {
-    private final ChangeResource rsrc;
+    final Map<Account.Id, ChangeControl> reviewers;
-    private final Map<Account.Id, ChangeControl> reviewers;
+    final ReviewerState state;
+    List<PatchSetApproval> addedReviewers;
+    Collection<Account.Id> addedCCs;
 
-    private List<PatchSetApproval> added;
+    private final ChangeResource rsrc;
     private PatchSet patchSet;
 
-    Op(ChangeResource rsrc, Map<Account.Id, ChangeControl> reviewers) {
+    Op(ChangeResource rsrc, Map<Account.Id, ChangeControl> reviewers,
+        ReviewerState state) {
       this.rsrc = rsrc;
       this.reviewers = reviewers;
+      this.state = state;
     }
 
     @Override
     public boolean updateChange(ChangeContext ctx)
         throws RestApiException, OrmException, IOException {
-      added =
+      if (migration.readChanges() && state == CC) {
-          approvalsUtil.addReviewers(
+        addedCCs = approvalsUtil.addCcs(ctx.getNotes(),
-              ctx.getDb(),
+            ctx.getUpdate(ctx.getChange().currentPatchSetId()),
-              ctx.getNotes(),
+            reviewers.keySet());
-              ctx.getUpdate(ctx.getChange().currentPatchSetId()),
+        if (addedCCs.isEmpty()) {
-              rsrc.getControl().getLabelTypes(),
+          return false;
-              rsrc.getChange(),
+        }
-              reviewers.keySet());
+      } else {
-
+        addedReviewers = approvalsUtil.addReviewers(ctx.getDb(), ctx.getNotes(),
-      if (added.isEmpty()) {
+            ctx.getUpdate(ctx.getChange().currentPatchSetId()),
-        return false;
+            rsrc.getControl().getLabelTypes(), rsrc.getChange(),
+            reviewers.keySet());
+        if (addedReviewers.isEmpty()) {
+          return false;
+        }
       }
+
       patchSet = psUtil.current(dbProvider.get(), rsrc.getNotes());
       return true;
     }
 
     @Override
     public void postUpdate(Context ctx) throws Exception {
-      emailReviewers(rsrc.getChange(), added);
+      if (addedReviewers != null || addedCCs != null) {
-
+        if (addedReviewers == null) {
-      if (!added.isEmpty()) {
+          addedReviewers = new ArrayList<>();
-        for (PatchSetApproval psa : added) {
+        }
-          Account account = accountCache.get(psa.getAccountId()).getAccount();
+        if (addedCCs == null) {
-          reviewerAdded.fire(rsrc.getChange(), patchSet, account,
+          addedCCs = new ArrayList<>();
-              ctx.getAccount(),
+        }
-              ctx.getWhen());
+        emailReviewers(rsrc.getChange(), addedReviewers, addedCCs);
+        if (!addedReviewers.isEmpty()) {
+          for (PatchSetApproval psa : addedReviewers) {
+            Account account = accountCache.get(psa.getAccountId()).getAccount();
+            reviewerAdded.fire(rsrc.getChange(), patchSet, account,
+              ctx.getAccount(), ctx.getWhen());
+          }
         }
       }
     }
   }
 
-  private void emailReviewers(Change change, List<PatchSetApproval> added) {
+  private void emailReviewers(Change change, List<PatchSetApproval> added,
-    if (added.isEmpty()) {
+      Collection<Account.Id> copied) {
+    if (added.isEmpty() && copied.isEmpty()) {
       return;
     }
 
@@ -312,18 +380,27 @@ public class PostReviewers implements RestModifyView<ChangeResource, AddReviewer
         toMail.add(psa.getAccountId());
       }
     }
-    if (!toMail.isEmpty()) {
+    List<Account.Id> toCopy = Lists.newArrayListWithCapacity(copied.size());
-      try {
+    for (Account.Id id : copied) {
-        AddReviewerSender cm = addReviewerSenderFactory
+      if (!id.equals(userId)) {
-            .create(change.getProject(), change.getId());
+        toCopy.add(id);
-        cm.setFrom(userId);
-        cm.addReviewers(toMail);
-        cm.send();
-      } catch (Exception err) {
-        log.error("Cannot send email to new reviewers of change "
-            + change.getId(), err);
       }
     }
+    if (toMail.isEmpty() && toCopy.isEmpty()) {
+      return;
+    }
+
+    try {
+      AddReviewerSender cm = addReviewerSenderFactory
+          .create(change.getProject(), change.getId());
+      cm.setFrom(userId);
+      cm.addReviewers(toMail);
+      cm.addExtraCC(toCopy);
+      cm.send();
+    } catch (Exception err) {
+      log.error("Cannot send email to new reviewers of change "
+          + change.getId(), err);
+    }
   }
 
   public static boolean isLegalReviewerGroup(AccountGroup.UUID groupUUID) {

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

@@ -585,6 +585,7 @@ public class BatchUpdate implements AutoCloseable {
 
   public BatchUpdate addOp(Change.Id id, Op op) {
     checkArgument(!(op instanceof InsertChangeOp), "use insertChange");
+    checkNotNull(op);
     ops.put(id, op);
     return this;
   }