Add reviewers by email to REST API and Email sender

This change adds support for adding reviewers by email in PostReviewer
and ChangeInfo. It also adds support for deleting reviewers using the
ReviewerResource by adding Address to it.

It further adds support for sending emails to reviewers that were
added by email. This support includes emails for adding and removing a
reviewer as well as for general change notifications.

This change also adds tests and updates the documentation.

Bug: Issue 4134
Change-Id: I901d711956ee69c25a05b455e068ec109821d79d

Documentation/rest-api-changes.txt

@@ -2805,6 +2805,41 @@ To confirm the addition of the reviewers, resend the request with the
   }
 ----
 
+If link:config-project-config.html#reviewer.enableByEmail[reviewer.enableByEmail] is set
+for the project, reviewers and CCs are not required to have a Gerrit account. If you POST
+an email address of a reviewer or CC then, they will be added to the change even if they
+don't have a Gerrit account.
+
+If this option is disabled, the request would fail with `400 Bad Request` if the email
+address can't be resolved to an active Gerrit account.
+
+Note that the name is optional so both "un.registered@reviewer.com" and
+"John Doe <un.registered@reviewer.com>" are valid inputs.
+
+Reviewers without Gerrit accounts can only be added on changes visible to anonymous users.
+
+.Request
+----
+  POST /changes/myProject~master~I8473b95934b5732ac55d26311a706c9c2bde9940/reviewers HTTP/1.0
+  Content-Type: application/json; charset=UTF-8
+
+  {
+    "reviewer": "John Doe <un.registered@reviewer.com>"
+  }
+----
+
+.Response
+----
+  HTTP/1.1 200 OK
+  Content-Disposition: attachment
+  Content-Type: application/json; charset=UTF-8
+
+  )]}'
+  {
+    "input": "John Doe <un.registered@reviewer.com>"
+  }
+----
+
 [[delete-reviewer]]
 === Delete Reviewer
 --
@@ -6183,6 +6218,10 @@ In addition `ReviewerInfo` has the following fields:
 |`approvals`   |
 The approvals of the reviewer as a map that maps the label names to the
 approval values ("`-2`", "`-1`", "`0`", "`+1`", "`+2`").
+|`_account_id`   |
+This field is inherited from `AccountInfo` but is optional here if an
+unregistered reviewer was added by email. See
+link:rest-api-changes.html#add-reviewer[add-reviewer] for details.
 |==========================
 
 [[reviewer-input]]

gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java

@@ -88,6 +88,7 @@ import com.google.gerrit.server.group.SystemGroupBackend;
 import com.google.gerrit.server.index.change.ChangeIndex;
 import com.google.gerrit.server.index.change.ChangeIndexCollection;
 import com.google.gerrit.server.index.change.ChangeIndexer;
+import com.google.gerrit.server.mail.Address;
 import com.google.gerrit.server.mail.send.EmailHeader;
 import com.google.gerrit.server.notedb.ChangeNoteUtil;
 import com.google.gerrit.server.notedb.ChangeNotes;
@@ -1200,21 +1201,29 @@ public abstract class AbstractDaemonTest {
   }
 
   protected void assertNotifyTo(TestAccount expected) {
+    assertNotifyTo(expected.emailAddress);
+  }
+
+  protected void assertNotifyTo(Address expected) {
     assertThat(sender.getMessages()).hasSize(1);
     Message m = sender.getMessages().get(0);
-    assertThat(m.rcpt()).containsExactly(expected.emailAddress);
+    assertThat(m.rcpt()).containsExactly(expected);
     assertThat(((EmailHeader.AddressList) m.headers().get("To")).getAddressList())
-        .containsExactly(expected.emailAddress);
+        .containsExactly(expected);
     assertThat(m.headers().get("CC").isEmpty()).isTrue();
   }
 
   protected void assertNotifyCc(TestAccount expected) {
+    assertNotifyCc(expected.emailAddress);
+  }
+
+  protected void assertNotifyCc(Address expected) {
     assertThat(sender.getMessages()).hasSize(1);
     Message m = sender.getMessages().get(0);
-    assertThat(m.rcpt()).containsExactly(expected.emailAddress);
+    assertThat(m.rcpt()).containsExactly(expected);
     assertThat(m.headers().get("To").isEmpty()).isTrue();
     assertThat(((EmailHeader.AddressList) m.headers().get("CC")).getAddressList())
-        .containsExactly(expected.emailAddress);
+        .containsExactly(expected);
   }
 
   protected void assertNotifyBcc(TestAccount expected) {

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

@@ -0,0 +1,245 @@
+// Copyright (C) 2017 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.acceptance.rest.change;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.common.truth.TruthJUnit.assume;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
+import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.acceptance.NoHttpd;
+import com.google.gerrit.acceptance.PushOneCommit;
+import com.google.gerrit.extensions.api.changes.AddReviewerInput;
+import com.google.gerrit.extensions.api.changes.ReviewInput;
+import com.google.gerrit.extensions.client.ListChangesOption;
+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.extensions.restapi.BadRequestException;
+import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
+import com.google.gerrit.server.git.ProjectConfig;
+import com.google.gerrit.server.mail.Address;
+import com.google.gerrit.testutil.FakeEmailSender.Message;
+import java.util.EnumSet;
+import java.util.List;
+import org.junit.Before;
+import org.junit.Test;
+
+@NoHttpd
+public class ChangeReviewersByEmailIT extends AbstractDaemonTest {
+
+  @Before
+  public void setUp() throws Exception {
+    ProjectConfig cfg = projectCache.checkedGet(project).getConfig();
+    cfg.setEnableReviewerByEmail(true);
+    saveProjectConfig(project, cfg);
+  }
+
+  @Test
+  public void addByEmail() throws Exception {
+    assume().that(notesMigration.readChanges()).isTrue();
+    AccountInfo acc = new AccountInfo("Foo Bar", "foo.bar@gerritcodereview.com");
+
+    for (ReviewerState state : ImmutableList.of(ReviewerState.CC, ReviewerState.REVIEWER)) {
+      PushOneCommit.Result r = createChange();
+
+      AddReviewerInput input = new AddReviewerInput();
+      input.reviewer = toRfcAddressString(acc);
+      input.state = state;
+      gApi.changes().id(r.getChangeId()).addReviewer(input);
+
+      ChangeInfo info =
+          gApi.changes().id(r.getChangeId()).get(EnumSet.of(ListChangesOption.DETAILED_LABELS));
+      assertThat(info.reviewers).isEqualTo(ImmutableMap.of(state, ImmutableList.of(acc)));
+      // All reviewers added by email should be removable
+      assertThat(info.removableReviewers).isEqualTo(ImmutableList.of(acc));
+    }
+  }
+
+  @Test
+  public void removeByEmail() throws Exception {
+    assume().that(notesMigration.readChanges()).isTrue();
+    AccountInfo acc = new AccountInfo("Foo Bar", "foo.bar@gerritcodereview.com");
+
+    for (ReviewerState state : ImmutableList.of(ReviewerState.CC, ReviewerState.REVIEWER)) {
+      PushOneCommit.Result r = createChange();
+
+      AddReviewerInput addInput = new AddReviewerInput();
+      addInput.reviewer = toRfcAddressString(acc);
+      addInput.state = state;
+      gApi.changes().id(r.getChangeId()).addReviewer(addInput);
+
+      gApi.changes().id(r.getChangeId()).reviewer(acc.email).remove();
+
+      ChangeInfo info =
+          gApi.changes().id(r.getChangeId()).get(EnumSet.of(ListChangesOption.DETAILED_LABELS));
+      assertThat(info.reviewers).isEmpty();
+    }
+  }
+
+  @Test
+  public void convertFromCCToReviewer() throws Exception {
+    assume().that(notesMigration.readChanges()).isTrue();
+    AccountInfo acc = new AccountInfo("Foo Bar", "foo.bar@gerritcodereview.com");
+
+    PushOneCommit.Result r = createChange();
+
+    AddReviewerInput addInput = new AddReviewerInput();
+    addInput.reviewer = toRfcAddressString(acc);
+    addInput.state = ReviewerState.CC;
+    gApi.changes().id(r.getChangeId()).addReviewer(addInput);
+
+    AddReviewerInput modifyInput = new AddReviewerInput();
+    modifyInput.reviewer = addInput.reviewer;
+    modifyInput.state = ReviewerState.REVIEWER;
+    gApi.changes().id(r.getChangeId()).addReviewer(modifyInput);
+
+    ChangeInfo info =
+        gApi.changes().id(r.getChangeId()).get(EnumSet.of(ListChangesOption.DETAILED_LABELS));
+    assertThat(info.reviewers)
+        .isEqualTo(ImmutableMap.of(ReviewerState.REVIEWER, ImmutableList.of(acc)));
+  }
+
+  @Test
+  public void addedReviewersGetNotified() throws Exception {
+    assume().that(notesMigration.readChanges()).isTrue();
+    AccountInfo acc = new AccountInfo("Foo Bar", "foo.bar@gerritcodereview.com");
+
+    for (ReviewerState state : ImmutableList.of(ReviewerState.CC, ReviewerState.REVIEWER)) {
+      PushOneCommit.Result r = createChange();
+
+      AddReviewerInput input = new AddReviewerInput();
+      input.reviewer = toRfcAddressString(acc);
+      input.state = state;
+      gApi.changes().id(r.getChangeId()).addReviewer(input);
+
+      List<Message> messages = sender.getMessages();
+      assertThat(messages).hasSize(1);
+      assertThat(messages.get(0).rcpt()).containsExactly(Address.parse(input.reviewer));
+      sender.clear();
+    }
+  }
+
+  @Test
+  public void removingReviewerTriggersNotification() throws Exception {
+    assume().that(notesMigration.readChanges()).isTrue();
+    AccountInfo acc = new AccountInfo("Foo Bar", "foo.bar@gerritcodereview.com");
+
+    for (ReviewerState state : ImmutableList.of(ReviewerState.CC, ReviewerState.REVIEWER)) {
+      PushOneCommit.Result r = createChange();
+
+      AddReviewerInput addInput = new AddReviewerInput();
+      addInput.reviewer = toRfcAddressString(acc);
+      addInput.state = state;
+      gApi.changes().id(r.getChangeId()).addReviewer(addInput);
+
+      // Review change as user
+      ReviewInput reviewInput = new ReviewInput();
+      reviewInput.message = "I have a comment";
+      setApiUser(user);
+      revision(r).review(reviewInput);
+      setApiUser(admin);
+
+      sender.clear();
+
+      // Delete as admin
+      gApi.changes().id(r.getChangeId()).reviewer(addInput.reviewer).remove();
+
+      List<Message> messages = sender.getMessages();
+      assertThat(messages).hasSize(1);
+      assertThat(messages.get(0).rcpt())
+          .containsExactly(Address.parse(addInput.reviewer), user.emailAddress);
+      sender.clear();
+    }
+  }
+
+  @Test
+  public void reviewerAndCCReceiveRegularNotification() throws Exception {
+    assume().that(notesMigration.readChanges()).isTrue();
+    AccountInfo acc = new AccountInfo("Foo Bar", "foo.bar@gerritcodereview.com");
+
+    for (ReviewerState state : ImmutableList.of(ReviewerState.CC, ReviewerState.REVIEWER)) {
+      PushOneCommit.Result r = createChange();
+
+      AddReviewerInput input = new AddReviewerInput();
+      input.reviewer = toRfcAddressString(acc);
+      input.state = state;
+      gApi.changes().id(r.getChangeId()).addReviewer(input);
+      sender.clear();
+
+      gApi.changes()
+          .id(r.getChangeId())
+          .revision(r.getCommit().name())
+          .review(ReviewInput.approve());
+
+      if (state == ReviewerState.CC) {
+        assertNotifyCc(Address.parse(input.reviewer));
+      } else {
+        assertNotifyTo(Address.parse(input.reviewer));
+      }
+    }
+  }
+
+  @Test
+  public void rejectMissingEmail() throws Exception {
+    assume().that(notesMigration.readChanges()).isTrue();
+    PushOneCommit.Result r = createChange();
+
+    exception.expect(UnprocessableEntityException.class);
+    exception.expectMessage("email invalid");
+    gApi.changes().id(r.getChangeId()).addReviewer("");
+  }
+
+  @Test
+  public void rejectMalformedEmail() throws Exception {
+    assume().that(notesMigration.readChanges()).isTrue();
+    PushOneCommit.Result r = createChange();
+
+    exception.expect(UnprocessableEntityException.class);
+    exception.expectMessage("email invalid");
+    gApi.changes().id(r.getChangeId()).addReviewer("Foo Bar <foo.bar@");
+  }
+
+  @Test
+  public void rejectOnNonPublicChange() throws Exception {
+    assume().that(notesMigration.readChanges()).isTrue();
+    PushOneCommit.Result r = createDraftChange();
+
+    exception.expect(BadRequestException.class);
+    exception.expectMessage("change is not publicly visible");
+    gApi.changes().id(r.getChangeId()).addReviewer("Foo Bar <foo.bar@gerritcodereview.com>");
+  }
+
+  @Test
+  public void rejectWhenFeatureIsDisabled() throws Exception {
+    assume().that(notesMigration.readChanges()).isTrue();
+
+    ProjectConfig cfg = projectCache.checkedGet(project).getConfig();
+    cfg.setEnableReviewerByEmail(false);
+    saveProjectConfig(project, cfg);
+
+    PushOneCommit.Result r = createChange();
+
+    exception.expect(UnprocessableEntityException.class);
+    exception.expectMessage(
+        "Foo Bar <foo.bar@gerritcodereview.com> does not identify a registered user or group");
+    gApi.changes().id(r.getChangeId()).addReviewer("Foo Bar <foo.bar@gerritcodereview.com>");
+  }
+
+  private static String toRfcAddressString(AccountInfo info) {
+    return (new Address(info.name, info.email)).toString();
+  }
+}

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

@@ -25,6 +25,13 @@ public class ReviewerInfo extends AccountInfo {
    */
   @Nullable public Map<String, String> approvals;
 
+  public static ReviewerInfo byEmail(@Nullable String name, String email) {
+    ReviewerInfo info = new ReviewerInfo();
+    info.name = name;
+    info.email = email;
+    return info;
+  }
+
   public ReviewerInfo(Integer id) {
     super(id);
   }
@@ -33,4 +40,6 @@ public class ReviewerInfo extends AccountInfo {
   public String toString() {
     return username;
   }
+
+  private ReviewerInfo() {}
 }

gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/AccountInfo.java

@@ -15,6 +15,7 @@
 package com.google.gerrit.extensions.common;
 
 import java.util.List;
+import java.util.Objects;
 
 public class AccountInfo {
   public Integer _accountId;
@@ -29,4 +30,34 @@ public class AccountInfo {
   public AccountInfo(Integer id) {
     this._accountId = id;
   }
+
+  /** To be used ONLY in connection with unregistered reviewers and CCs. */
+  public AccountInfo(String name, String email) {
+    this.name = name;
+    this.email = email;
+  }
+
+  @Override
+  public boolean equals(Object o) {
+    if (o instanceof AccountInfo) {
+      AccountInfo accountInfo = (AccountInfo) o;
+      return Objects.equals(_accountId, accountInfo._accountId)
+          && Objects.equals(name, accountInfo.name)
+          && Objects.equals(email, accountInfo.email)
+          && Objects.equals(secondaryEmails, accountInfo.secondaryEmails)
+          && Objects.equals(username, accountInfo.username)
+          && Objects.equals(avatars, accountInfo.avatars)
+          && Objects.equals(_moreAccounts, accountInfo._moreAccounts)
+          && Objects.equals(status, accountInfo.status);
+    }
+    return false;
+  }
+
+  @Override
+  public int hashCode() {
+    return Objects.hash(
+        _accountId, name, email, secondaryEmails, username, avatars, _moreAccounts, status);
+  }
+
+  protected AccountInfo() {}
 }

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

@@ -108,6 +108,7 @@ import com.google.gerrit.server.git.LabelNormalizer;
 import com.google.gerrit.server.git.MergeUtil;
 import com.google.gerrit.server.index.change.ChangeField;
 import com.google.gerrit.server.index.change.ChangeIndexCollection;
+import com.google.gerrit.server.mail.Address;
 import com.google.gerrit.server.notedb.ChangeNotes;
 import com.google.gerrit.server.notedb.ReviewerStateInternal;
 import com.google.gerrit.server.patch.PatchListNotAvailableException;
@@ -531,6 +532,12 @@ public class ChangeJson {
           cd.reviewers().asTable().rowMap().entrySet()) {
         out.reviewers.put(e.getKey().asReviewerState(), toAccountInfo(e.getValue().keySet()));
       }
+      // TODO(hiesel) Load from ChangeData instead after the data was added there
+      for (Map.Entry<ReviewerStateInternal, Map<Address, Timestamp>> e :
+          cd.notes().getReviewersByEmail().asTable().rowMap().entrySet()) {
+        out.reviewers.put(
+            e.getKey().asReviewerState(), toAccountInfoByEmail(e.getValue().keySet()));
+      }
 
       out.removableReviewers = removableReviewers(ctl, out);
     }
@@ -1075,9 +1082,11 @@ public class ChangeJson {
     Collection<AccountInfo> ccs = out.reviewers.get(ReviewerState.CC);
     if (ccs != null) {
       for (AccountInfo ai : ccs) {
-        Account.Id id = new Account.Id(ai._accountId);
+        if (ai._accountId != null) {
-        if (ctl.canRemoveReviewer(id, 0)) {
+          Account.Id id = new Account.Id(ai._accountId);
-          removable.add(id);
+          if (ctl.canRemoveReviewer(id, 0)) {
+            removable.add(id);
+          }
         }
       }
     }
@@ -1091,6 +1100,14 @@ public class ChangeJson {
     for (Account.Id id : removable) {
       result.add(accountLoader.get(id));
     }
+    // Reviewers added by email are always removable
+    for (Collection<AccountInfo> infos : out.reviewers.values()) {
+      for (AccountInfo info : infos) {
+        if (info._accountId == null) {
+          result.add(info);
+        }
+      }
+    }
     return result;
   }
 
@@ -1102,6 +1119,14 @@ public class ChangeJson {
         .collect(toList());
   }
 
+  private Collection<AccountInfo> toAccountInfoByEmail(Collection<Address> addresses) {
+    return addresses
+        .stream()
+        .map(a -> new AccountInfo(a.getName(), a.getEmail()))
+        .sorted(AccountInfoComparator.ORDER_NULLS_FIRST)
+        .collect(toList());
+  }
+
   @Nullable
   private Repository openRepoIfNecessary(ChangeControl ctl) throws IOException {
     if (has(ALL_COMMITS) || has(CURRENT_COMMIT) || has(COMMIT_FOOTERS)) {

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

@@ -14,92 +14,38 @@
 
 package com.google.gerrit.server.change;
 
-import com.google.common.collect.Iterables;
 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;
 import com.google.gerrit.extensions.restapi.RestApiException;
 import com.google.gerrit.extensions.restapi.RestModifyView;
-import com.google.gerrit.reviewdb.client.Account;
-import com.google.gerrit.reviewdb.client.Change;
-import com.google.gerrit.reviewdb.client.ChangeMessage;
-import com.google.gerrit.reviewdb.client.PatchSet;
-import com.google.gerrit.reviewdb.client.PatchSetApproval;
-import com.google.gerrit.reviewdb.client.Project;
 import com.google.gerrit.reviewdb.server.ReviewDb;
-import com.google.gerrit.reviewdb.server.ReviewDbUtil;
-import com.google.gerrit.server.ApprovalsUtil;
-import com.google.gerrit.server.ChangeMessagesUtil;
-import com.google.gerrit.server.IdentifiedUser;
-import com.google.gerrit.server.PatchSetUtil;
-import com.google.gerrit.server.extensions.events.ReviewerDeleted;
-import com.google.gerrit.server.mail.send.DeleteReviewerSender;
-import com.google.gerrit.server.notedb.ChangeUpdate;
-import com.google.gerrit.server.notedb.NoteDbChangeState.PrimaryStorage;
-import com.google.gerrit.server.notedb.NotesMigration;
 import com.google.gerrit.server.update.BatchUpdate;
 import com.google.gerrit.server.update.BatchUpdateOp;
-import com.google.gerrit.server.update.BatchUpdateReviewDb;
-import com.google.gerrit.server.update.ChangeContext;
-import com.google.gerrit.server.update.Context;
 import com.google.gerrit.server.update.UpdateException;
-import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
 import com.google.inject.Singleton;
-import java.util.ArrayList;
-import java.util.Collections;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 
 @Singleton
 public class DeleteReviewer implements RestModifyView<ReviewerResource, DeleteReviewerInput> {
-  private static final Logger log = LoggerFactory.getLogger(DeleteReviewer.class);
 
   private final Provider<ReviewDb> dbProvider;
-  private final ApprovalsUtil approvalsUtil;
-  private final PatchSetUtil psUtil;
-  private final ChangeMessagesUtil cmUtil;
   private final BatchUpdate.Factory batchUpdateFactory;
-  private final IdentifiedUser.GenericFactory userFactory;
+  private final DeleteReviewerOp.Factory deleteReviewerOpFactory;
-  private final ReviewerDeleted reviewerDeleted;
+  private final DeleteReviewerByEmailOp.Factory deleteReviewerByEmailOpFactory;
-  private final Provider<IdentifiedUser> user;
-  private final DeleteReviewerSender.Factory deleteReviewerSenderFactory;
-  private final NotesMigration migration;
-  private final NotifyUtil notifyUtil;
 
   @Inject
   DeleteReviewer(
       Provider<ReviewDb> dbProvider,
-      ApprovalsUtil approvalsUtil,
-      PatchSetUtil psUtil,
-      ChangeMessagesUtil cmUtil,
       BatchUpdate.Factory batchUpdateFactory,
-      IdentifiedUser.GenericFactory userFactory,
+      DeleteReviewerOp.Factory deleteReviewerOpFactory,
-      ReviewerDeleted reviewerDeleted,
+      DeleteReviewerByEmailOp.Factory deleteReviewerByEmailOpFactory) {
-      Provider<IdentifiedUser> user,
-      DeleteReviewerSender.Factory deleteReviewerSenderFactory,
-      NotesMigration migration,
-      NotifyUtil notifyUtil) {
     this.dbProvider = dbProvider;
-    this.approvalsUtil = approvalsUtil;
-    this.psUtil = psUtil;
-    this.cmUtil = cmUtil;
     this.batchUpdateFactory = batchUpdateFactory;
-    this.userFactory = userFactory;
+    this.deleteReviewerOpFactory = deleteReviewerOpFactory;
-    this.reviewerDeleted = reviewerDeleted;
+    this.deleteReviewerByEmailOpFactory = deleteReviewerByEmailOpFactory;
-    this.user = user;
-    this.deleteReviewerSenderFactory = deleteReviewerSenderFactory;
-    this.migration = migration;
-    this.notifyUtil = notifyUtil;
   }
 
   @Override
@@ -118,151 +64,15 @@ public class DeleteReviewer implements RestModifyView<ReviewerResource, DeleteRe
             rsrc.getChangeResource().getProject(),
             rsrc.getChangeResource().getUser(),
             TimeUtil.nowTs())) {
-      Op op = new Op(rsrc.getReviewerUser().getAccount(), input);
+      BatchUpdateOp op;
+      if (rsrc.isByEmail()) {
+        op = deleteReviewerByEmailOpFactory.create(rsrc.getReviewerByEmail(), input);
+      } else {
+        op = deleteReviewerOpFactory.create(rsrc.getReviewerUser().getAccount(), input);
+      }
       bu.addOp(rsrc.getChange().getId(), op);
       bu.execute();
     }
-
     return Response.none();
   }
-
-  private class Op implements BatchUpdateOp {
-    private final Account reviewer;
-    private final DeleteReviewerInput input;
-    ChangeMessage changeMessage;
-    Change currChange;
-    PatchSet currPs;
-    Map<String, Short> newApprovals = new HashMap<>();
-    Map<String, Short> oldApprovals = new HashMap<>();
-
-    Op(Account reviewerAccount, DeleteReviewerInput input) {
-      this.reviewer = reviewerAccount;
-      this.input = input;
-    }
-
-    @Override
-    public boolean updateChange(ChangeContext ctx)
-        throws AuthException, ResourceNotFoundException, OrmException {
-      Account.Id reviewerId = reviewer.getId();
-      if (!approvalsUtil.getReviewers(ctx.getDb(), ctx.getNotes()).all().contains(reviewerId)) {
-        throw new ResourceNotFoundException();
-      }
-      currChange = ctx.getChange();
-      currPs = psUtil.current(ctx.getDb(), ctx.getNotes());
-
-      LabelTypes labelTypes = ctx.getControl().getLabelTypes();
-      // removing a reviewer will remove all her votes
-      for (LabelType lt : labelTypes.getLabelTypes()) {
-        newApprovals.put(lt.getName(), (short) 0);
-      }
-
-      StringBuilder msg = new StringBuilder();
-      msg.append("Removed reviewer " + reviewer.getFullName());
-      StringBuilder removedVotesMsg = new StringBuilder();
-      removedVotesMsg.append(" with the following votes:\n\n");
-      List<PatchSetApproval> del = new ArrayList<>();
-      boolean votesRemoved = false;
-      for (PatchSetApproval a : approvals(ctx, reviewerId)) {
-        if (ctx.getControl().canRemoveReviewer(a)) {
-          del.add(a);
-          if (a.getPatchSetId().equals(currPs.getId()) && a.getValue() != 0) {
-            oldApprovals.put(a.getLabel(), a.getValue());
-            removedVotesMsg
-                .append("* ")
-                .append(a.getLabel())
-                .append(formatLabelValue(a.getValue()))
-                .append(" by ")
-                .append(userFactory.create(a.getAccountId()).getNameEmail())
-                .append("\n");
-            votesRemoved = true;
-          }
-        } else {
-          throw new AuthException("delete reviewer not permitted");
-        }
-      }
-
-      if (votesRemoved) {
-        msg.append(removedVotesMsg);
-      } else {
-        msg.append(".");
-      }
-      ctx.getDb().patchSetApprovals().delete(del);
-      ChangeUpdate update = ctx.getUpdate(currPs.getId());
-      update.removeReviewer(reviewerId);
-
-      changeMessage =
-          ChangeMessagesUtil.newMessage(
-              ctx, msg.toString(), ChangeMessagesUtil.TAG_DELETE_REVIEWER);
-      cmUtil.addChangeMessage(ctx.getDb(), update, changeMessage);
-
-      return true;
-    }
-
-    @Override
-    public void postUpdate(Context ctx) {
-      if (NotifyUtil.shouldNotify(input.notify, input.notifyDetails)) {
-        emailReviewers(ctx.getProject(), currChange, changeMessage);
-      }
-      reviewerDeleted.fire(
-          currChange,
-          currPs,
-          reviewer,
-          ctx.getAccount(),
-          changeMessage.getMessage(),
-          newApprovals,
-          oldApprovals,
-          input.notify,
-          ctx.getWhen());
-    }
-
-    private Iterable<PatchSetApproval> approvals(ChangeContext ctx, Account.Id accountId)
-        throws OrmException {
-      Change.Id changeId = ctx.getNotes().getChangeId();
-      Iterable<PatchSetApproval> approvals;
-      PrimaryStorage r = PrimaryStorage.of(ctx.getChange());
-
-      if (migration.readChanges() && r == PrimaryStorage.REVIEW_DB) {
-        // Because NoteDb and ReviewDb have different semantics for zero-value
-        // approvals, we must fall back to ReviewDb as the source of truth here.
-        ReviewDb db = ctx.getDb();
-
-        if (db instanceof BatchUpdateReviewDb) {
-          db = ((BatchUpdateReviewDb) db).unsafeGetDelegate();
-        }
-        db = ReviewDbUtil.unwrapDb(db);
-        approvals = db.patchSetApprovals().byChange(changeId);
-      } else {
-        approvals = approvalsUtil.byChange(ctx.getDb(), ctx.getNotes()).values();
-      }
-
-      return Iterables.filter(approvals, psa -> accountId.equals(psa.getAccountId()));
-    }
-
-    private String formatLabelValue(short value) {
-      if (value > 0) {
-        return "+" + value;
-      }
-      return Short.toString(value);
-    }
-
-    private void emailReviewers(
-        Project.NameKey projectName, Change change, ChangeMessage changeMessage) {
-      Account.Id userId = user.get().getAccountId();
-      if (userId.equals(reviewer.getId())) {
-        // The user knows they removed themselves, don't bother emailing them.
-        return;
-      }
-      try {
-        DeleteReviewerSender cm = deleteReviewerSenderFactory.create(projectName, change.getId());
-        cm.setFrom(userId);
-        cm.addReviewers(Collections.singleton(reviewer.getId()));
-        cm.setChangeMessage(changeMessage.getMessage(), changeMessage.getWrittenOn());
-        cm.setNotify(input.notify);
-        cm.setAccountsToNotify(notifyUtil.resolveAccounts(input.notifyDetails));
-        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/DeleteReviewerByEmailOp.java

@@ -0,0 +1,96 @@
+// Copyright (C) 2017 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.server.change;
+
+import com.google.gerrit.extensions.api.changes.DeleteReviewerInput;
+import com.google.gerrit.reviewdb.client.Change;
+import com.google.gerrit.reviewdb.client.ChangeMessage;
+import com.google.gerrit.reviewdb.client.PatchSet;
+import com.google.gerrit.server.ChangeUtil;
+import com.google.gerrit.server.mail.Address;
+import com.google.gerrit.server.mail.send.DeleteReviewerSender;
+import com.google.gerrit.server.update.BatchUpdateOp;
+import com.google.gerrit.server.update.ChangeContext;
+import com.google.gerrit.server.update.Context;
+import com.google.gwtorm.server.OrmException;
+import com.google.inject.Inject;
+import com.google.inject.assistedinject.Assisted;
+import java.util.Collections;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class DeleteReviewerByEmailOp implements BatchUpdateOp {
+  private static final Logger log = LoggerFactory.getLogger(DeleteReviewer.class);
+
+  public interface Factory {
+    DeleteReviewerByEmailOp create(Address reviewer, DeleteReviewerInput input);
+  }
+
+  private final DeleteReviewerSender.Factory deleteReviewerSenderFactory;
+  private final NotifyUtil notifyUtil;
+  private final Address reviewer;
+  private final DeleteReviewerInput input;
+
+  private ChangeMessage changeMessage;
+  private Change.Id changeId;
+
+  @Inject
+  DeleteReviewerByEmailOp(
+      DeleteReviewerSender.Factory deleteReviewerSenderFactory,
+      NotifyUtil notifyUtil,
+      @Assisted Address reviewer,
+      @Assisted DeleteReviewerInput input) {
+    this.deleteReviewerSenderFactory = deleteReviewerSenderFactory;
+    this.notifyUtil = notifyUtil;
+    this.reviewer = reviewer;
+    this.input = input;
+  }
+
+  @Override
+  public boolean updateChange(ChangeContext ctx) throws OrmException {
+    changeId = ctx.getChange().getId();
+    PatchSet.Id psId = ctx.getChange().currentPatchSetId();
+    String msg = "Removed reviewer " + reviewer;
+    changeMessage =
+        new ChangeMessage(
+            new ChangeMessage.Key(changeId, ChangeUtil.messageUuid()),
+            ctx.getAccountId(),
+            ctx.getWhen(),
+            psId);
+    changeMessage.setMessage(msg);
+
+    ctx.getUpdate(psId).setChangeMessage(msg);
+    ctx.getUpdate(psId).removeReviewerByEmail(reviewer);
+    return true;
+  }
+
+  @Override
+  public void postUpdate(Context ctx) {
+    if (!NotifyUtil.shouldNotify(input.notify, input.notifyDetails)) {
+      return;
+    }
+    try {
+      DeleteReviewerSender cm = deleteReviewerSenderFactory.create(ctx.getProject(), changeId);
+      cm.setFrom(ctx.getAccountId());
+      cm.addReviewersByEmail(Collections.singleton(reviewer));
+      cm.setChangeMessage(changeMessage.getMessage(), changeMessage.getWrittenOn());
+      cm.setNotify(input.notify);
+      cm.setAccountsToNotify(notifyUtil.resolveAccounts(input.notifyDetails));
+      cm.send();
+    } catch (Exception err) {
+      log.error("Cannot email update for change " + changeId, err);
+    }
+  }
+}

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

@@ -0,0 +1,232 @@
+// Copyright (C) 2017 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.server.change;
+
+import com.google.common.collect.Iterables;
+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.restapi.AuthException;
+import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
+import com.google.gerrit.reviewdb.client.Account;
+import com.google.gerrit.reviewdb.client.Change;
+import com.google.gerrit.reviewdb.client.ChangeMessage;
+import com.google.gerrit.reviewdb.client.PatchSet;
+import com.google.gerrit.reviewdb.client.PatchSetApproval;
+import com.google.gerrit.reviewdb.client.Project;
+import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gerrit.reviewdb.server.ReviewDbUtil;
+import com.google.gerrit.server.ApprovalsUtil;
+import com.google.gerrit.server.ChangeMessagesUtil;
+import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.server.PatchSetUtil;
+import com.google.gerrit.server.extensions.events.ReviewerDeleted;
+import com.google.gerrit.server.mail.send.DeleteReviewerSender;
+import com.google.gerrit.server.notedb.ChangeUpdate;
+import com.google.gerrit.server.notedb.NoteDbChangeState.PrimaryStorage;
+import com.google.gerrit.server.notedb.NotesMigration;
+import com.google.gerrit.server.update.BatchUpdateOp;
+import com.google.gerrit.server.update.BatchUpdateReviewDb;
+import com.google.gerrit.server.update.ChangeContext;
+import com.google.gerrit.server.update.Context;
+import com.google.gwtorm.server.OrmException;
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+import com.google.inject.assistedinject.Assisted;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class DeleteReviewerOp implements BatchUpdateOp {
+  private static final Logger log = LoggerFactory.getLogger(DeleteReviewer.class);
+
+  public interface Factory {
+    DeleteReviewerOp create(Account reviewerAccount, DeleteReviewerInput input);
+  }
+
+  private final ApprovalsUtil approvalsUtil;
+  private final PatchSetUtil psUtil;
+  private final ChangeMessagesUtil cmUtil;
+  private final IdentifiedUser.GenericFactory userFactory;
+  private final ReviewerDeleted reviewerDeleted;
+  private final Provider<IdentifiedUser> user;
+  private final DeleteReviewerSender.Factory deleteReviewerSenderFactory;
+  private final NotesMigration migration;
+  private final NotifyUtil notifyUtil;
+
+  private final Account reviewer;
+  private final DeleteReviewerInput input;
+
+  ChangeMessage changeMessage;
+  Change currChange;
+  PatchSet currPs;
+  Map<String, Short> newApprovals = new HashMap<>();
+  Map<String, Short> oldApprovals = new HashMap<>();
+
+  @Inject
+  DeleteReviewerOp(
+      ApprovalsUtil approvalsUtil,
+      PatchSetUtil psUtil,
+      ChangeMessagesUtil cmUtil,
+      IdentifiedUser.GenericFactory userFactory,
+      ReviewerDeleted reviewerDeleted,
+      Provider<IdentifiedUser> user,
+      DeleteReviewerSender.Factory deleteReviewerSenderFactory,
+      NotesMigration migration,
+      NotifyUtil notifyUtil,
+      @Assisted Account reviewerAccount,
+      @Assisted DeleteReviewerInput input) {
+    this.approvalsUtil = approvalsUtil;
+    this.psUtil = psUtil;
+    this.cmUtil = cmUtil;
+    this.userFactory = userFactory;
+    this.reviewerDeleted = reviewerDeleted;
+    this.user = user;
+    this.deleteReviewerSenderFactory = deleteReviewerSenderFactory;
+    this.migration = migration;
+    this.notifyUtil = notifyUtil;
+
+    this.reviewer = reviewerAccount;
+    this.input = input;
+  }
+
+  @Override
+  public boolean updateChange(ChangeContext ctx)
+      throws AuthException, ResourceNotFoundException, OrmException {
+    Account.Id reviewerId = reviewer.getId();
+    if (!approvalsUtil.getReviewers(ctx.getDb(), ctx.getNotes()).all().contains(reviewerId)) {
+      throw new ResourceNotFoundException();
+    }
+    currChange = ctx.getChange();
+    currPs = psUtil.current(ctx.getDb(), ctx.getNotes());
+
+    LabelTypes labelTypes = ctx.getControl().getLabelTypes();
+    // removing a reviewer will remove all her votes
+    for (LabelType lt : labelTypes.getLabelTypes()) {
+      newApprovals.put(lt.getName(), (short) 0);
+    }
+
+    StringBuilder msg = new StringBuilder();
+    msg.append("Removed reviewer " + reviewer.getFullName());
+    StringBuilder removedVotesMsg = new StringBuilder();
+    removedVotesMsg.append(" with the following votes:\n\n");
+    List<PatchSetApproval> del = new ArrayList<>();
+    boolean votesRemoved = false;
+    for (PatchSetApproval a : approvals(ctx, reviewerId)) {
+      if (ctx.getControl().canRemoveReviewer(a)) {
+        del.add(a);
+        if (a.getPatchSetId().equals(currPs.getId()) && a.getValue() != 0) {
+          oldApprovals.put(a.getLabel(), a.getValue());
+          removedVotesMsg
+              .append("* ")
+              .append(a.getLabel())
+              .append(formatLabelValue(a.getValue()))
+              .append(" by ")
+              .append(userFactory.create(a.getAccountId()).getNameEmail())
+              .append("\n");
+          votesRemoved = true;
+        }
+      } else {
+        throw new AuthException("delete reviewer not permitted");
+      }
+    }
+
+    if (votesRemoved) {
+      msg.append(removedVotesMsg);
+    } else {
+      msg.append(".");
+    }
+    ctx.getDb().patchSetApprovals().delete(del);
+    ChangeUpdate update = ctx.getUpdate(currPs.getId());
+    update.removeReviewer(reviewerId);
+
+    changeMessage =
+        ChangeMessagesUtil.newMessage(ctx, msg.toString(), ChangeMessagesUtil.TAG_DELETE_REVIEWER);
+    cmUtil.addChangeMessage(ctx.getDb(), update, changeMessage);
+
+    return true;
+  }
+
+  @Override
+  public void postUpdate(Context ctx) {
+    if (NotifyUtil.shouldNotify(input.notify, input.notifyDetails)) {
+      emailReviewers(ctx.getProject(), currChange, changeMessage);
+    }
+    reviewerDeleted.fire(
+        currChange,
+        currPs,
+        reviewer,
+        ctx.getAccount(),
+        changeMessage.getMessage(),
+        newApprovals,
+        oldApprovals,
+        input.notify,
+        ctx.getWhen());
+  }
+
+  private Iterable<PatchSetApproval> approvals(ChangeContext ctx, Account.Id accountId)
+      throws OrmException {
+    Change.Id changeId = ctx.getNotes().getChangeId();
+    Iterable<PatchSetApproval> approvals;
+    PrimaryStorage r = PrimaryStorage.of(ctx.getChange());
+
+    if (migration.readChanges() && r == PrimaryStorage.REVIEW_DB) {
+      // Because NoteDb and ReviewDb have different semantics for zero-value
+      // approvals, we must fall back to ReviewDb as the source of truth here.
+      ReviewDb db = ctx.getDb();
+
+      if (db instanceof BatchUpdateReviewDb) {
+        db = ((BatchUpdateReviewDb) db).unsafeGetDelegate();
+      }
+      db = ReviewDbUtil.unwrapDb(db);
+      approvals = db.patchSetApprovals().byChange(changeId);
+    } else {
+      approvals = approvalsUtil.byChange(ctx.getDb(), ctx.getNotes()).values();
+    }
+
+    return Iterables.filter(approvals, psa -> accountId.equals(psa.getAccountId()));
+  }
+
+  private String formatLabelValue(short value) {
+    if (value > 0) {
+      return "+" + value;
+    }
+    return Short.toString(value);
+  }
+
+  private void emailReviewers(
+      Project.NameKey projectName, Change change, ChangeMessage changeMessage) {
+    Account.Id userId = user.get().getAccountId();
+    if (userId.equals(reviewer.getId())) {
+      // The user knows they removed themselves, don't bother emailing them.
+      return;
+    }
+    try {
+      DeleteReviewerSender cm = deleteReviewerSenderFactory.create(projectName, change.getId());
+      cm.setFrom(userId);
+      cm.addReviewers(Collections.singleton(reviewer.getId()));
+      cm.setChangeMessage(changeMessage.getMessage(), changeMessage.getWrittenOn());
+      cm.setNotify(input.notify);
+      cm.setAccountsToNotify(notifyUtil.resolveAccounts(input.notifyDetails));
+      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/ListReviewers.java

@@ -19,6 +19,7 @@ import com.google.gerrit.extensions.restapi.RestReadView;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.ApprovalsUtil;
+import com.google.gerrit.server.mail.Address;
 import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
@@ -48,11 +49,16 @@ class ListReviewers implements RestReadView<ChangeResource> {
 
   @Override
   public List<ReviewerInfo> apply(ChangeResource rsrc) throws OrmException {
-    Map<Account.Id, ReviewerResource> reviewers = new LinkedHashMap<>();
+    Map<String, ReviewerResource> reviewers = new LinkedHashMap<>();
     ReviewDb db = dbProvider.get();
     for (Account.Id accountId : approvalsUtil.getReviewers(db, rsrc.getNotes()).all()) {
-      if (!reviewers.containsKey(accountId)) {
+      if (!reviewers.containsKey(accountId.toString())) {
-        reviewers.put(accountId, resourceFactory.create(rsrc, accountId));
+        reviewers.put(accountId.toString(), resourceFactory.create(rsrc, accountId));
+      }
+    }
+    for (Address adr : rsrc.getNotes().getReviewersByEmail().all()) {
+      if (!reviewers.containsKey(adr.toString())) {
+        reviewers.put(adr.toString(), new ReviewerResource(rsrc, adr));
       }
     }
     return json.format(reviewers.values());

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

@@ -20,6 +20,7 @@ import com.google.gerrit.extensions.restapi.RestReadView;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.ApprovalsUtil;
+import com.google.gerrit.server.mail.Address;
 import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
@@ -54,11 +55,16 @@ class ListRevisionReviewers implements RestReadView<RevisionResource> {
       throw new MethodNotAllowedException("Cannot list reviewers on non-current patch set");
     }
 
-    Map<Account.Id, ReviewerResource> reviewers = new LinkedHashMap<>();
+    Map<String, ReviewerResource> reviewers = new LinkedHashMap<>();
     ReviewDb db = dbProvider.get();
     for (Account.Id accountId : approvalsUtil.getReviewers(db, rsrc.getNotes()).all()) {
-      if (!reviewers.containsKey(accountId)) {
+      if (!reviewers.containsKey(accountId.toString())) {
-        reviewers.put(accountId, resourceFactory.create(rsrc, accountId));
+        reviewers.put(accountId.toString(), resourceFactory.create(rsrc, accountId));
+      }
+    }
+    for (Address address : rsrc.getNotes().getReviewersByEmail().all()) {
+      if (!reviewers.containsKey(address.toString())) {
+        reviewers.put(address.toString(), new ReviewerResource(rsrc, address));
       }
     }
     return json.format(reviewers.values());

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

@@ -161,5 +161,7 @@ public class Module extends RestApiModule {
     factory(SetAssigneeOp.Factory.class);
     factory(SetHashtagsOp.Factory.class);
     factory(ChangeResource.Factory.class);
+    factory(DeleteReviewerOp.Factory.class);
+    factory(DeleteReviewerByEmailOp.Factory.class);
   }
 }

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

@@ -86,6 +86,7 @@ import com.google.gerrit.server.PatchSetUtil;
 import com.google.gerrit.server.ReviewerSet;
 import com.google.gerrit.server.account.AccountsCollection;
 import com.google.gerrit.server.extensions.events.CommentAdded;
+import com.google.gerrit.server.mail.Address;
 import com.google.gerrit.server.notedb.ChangeNotes;
 import com.google.gerrit.server.notedb.ChangeUpdate;
 import com.google.gerrit.server.notedb.NotesMigration;
@@ -313,14 +314,18 @@ public class PostReview implements RestModifyView<RevisionResource, ReviewInput>
       ListMultimap<RecipientType, Account.Id> accountsToNotify) {
     List<Account.Id> to = new ArrayList<>();
     List<Account.Id> cc = new ArrayList<>();
+    List<Address> toByEmail = new ArrayList<>();
+    List<Address> ccByEmail = new ArrayList<>();
     for (PostReviewers.Addition addition : reviewerAdditions) {
       if (addition.op.state == ReviewerState.REVIEWER) {
         to.addAll(addition.op.reviewers.keySet());
+        toByEmail.addAll(addition.op.reviewersByEmail);
       } else if (addition.op.state == ReviewerState.CC) {
         cc.addAll(addition.op.reviewers.keySet());
+        ccByEmail.addAll(addition.op.reviewersByEmail);
       }
     }
-    postReviewers.emailReviewers(change, to, cc, notify, accountsToNotify);
+    postReviewers.emailReviewers(change, to, cc, toByEmail, ccByEmail, notify, accountsToNotify);
   }
 
   private RevisionResource onBehalfOf(RevisionResource rev, ReviewInput in)

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

@@ -23,6 +23,7 @@ import com.google.common.collect.ImmutableListMultimap;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ListMultimap;
 import com.google.common.collect.Lists;
+import com.google.gerrit.common.Nullable;
 import com.google.gerrit.common.TimeUtil;
 import com.google.gerrit.common.data.GroupDescription;
 import com.google.gerrit.common.errors.NoSuchGroupException;
@@ -32,6 +33,7 @@ import com.google.gerrit.extensions.api.changes.NotifyHandling;
 import com.google.gerrit.extensions.api.changes.RecipientType;
 import com.google.gerrit.extensions.api.changes.ReviewerInfo;
 import com.google.gerrit.extensions.client.ReviewerState;
+import com.google.gerrit.extensions.common.AccountInfo;
 import com.google.gerrit.extensions.restapi.BadRequestException;
 import com.google.gerrit.extensions.restapi.RestApiException;
 import com.google.gerrit.extensions.restapi.RestModifyView;
@@ -42,6 +44,7 @@ import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.PatchSet;
 import com.google.gerrit.reviewdb.client.PatchSetApproval;
 import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gerrit.server.AnonymousUser;
 import com.google.gerrit.server.ApprovalsUtil;
 import com.google.gerrit.server.CurrentUser;
 import com.google.gerrit.server.IdentifiedUser;
@@ -52,12 +55,17 @@ import com.google.gerrit.server.account.AccountsCollection;
 import com.google.gerrit.server.account.GroupMembers;
 import com.google.gerrit.server.config.GerritServerConfig;
 import com.google.gerrit.server.extensions.events.ReviewerAdded;
+import com.google.gerrit.server.git.ProjectConfig;
 import com.google.gerrit.server.group.GroupsCollection;
 import com.google.gerrit.server.group.SystemGroupBackend;
+import com.google.gerrit.server.mail.Address;
 import com.google.gerrit.server.mail.send.AddReviewerSender;
+import com.google.gerrit.server.mail.send.OutgoingEmailValidator;
 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.project.NoSuchProjectException;
+import com.google.gerrit.server.project.ProjectCache;
 import com.google.gerrit.server.update.BatchUpdate;
 import com.google.gerrit.server.update.BatchUpdateOp;
 import com.google.gerrit.server.update.ChangeContext;
@@ -104,6 +112,8 @@ public class PostReviewers implements RestModifyView<ChangeResource, AddReviewer
   private final NotesMigration migration;
   private final AccountCache accountCache;
   private final NotifyUtil notifyUtil;
+  private final ProjectCache projectCache;
+  private final Provider<AnonymousUser> anonymousProvider;
 
   @Inject
   PostReviewers(
@@ -124,7 +134,9 @@ public class PostReviewers implements RestModifyView<ChangeResource, AddReviewer
       ReviewerAdded reviewerAdded,
       NotesMigration migration,
       AccountCache accountCache,
-      NotifyUtil notifyUtil) {
+      NotifyUtil notifyUtil,
+      ProjectCache projectCache,
+      Provider<AnonymousUser> anonymousProvider) {
     this.accounts = accounts;
     this.reviewerFactory = reviewerFactory;
     this.approvalsUtil = approvalsUtil;
@@ -143,6 +155,8 @@ public class PostReviewers implements RestModifyView<ChangeResource, AddReviewer
     this.migration = migration;
     this.accountCache = accountCache;
     this.notifyUtil = notifyUtil;
+    this.projectCache = projectCache;
+    this.anonymousProvider = anonymousProvider;
   }
 
   @Override
@@ -167,6 +181,7 @@ public class PostReviewers implements RestModifyView<ChangeResource, AddReviewer
     return addition.result;
   }
 
+  // TODO(hiesel) Refactor this as it starts to become unreadable
   public Addition prepareApplication(
       ChangeResource rsrc, AddReviewerInput input, boolean allowGroup)
       throws OrmException, RestApiException, IOException {
@@ -174,17 +189,28 @@ public class PostReviewers implements RestModifyView<ChangeResource, AddReviewer
     try {
       accountId = accounts.parse(input.reviewer).getAccountId();
     } catch (UnprocessableEntityException e) {
+      ProjectConfig projectConfig = projectCache.checkedGet(rsrc.getProject()).getConfig();
       if (allowGroup) {
         try {
           return putGroup(rsrc, input);
         } catch (UnprocessableEntityException e2) {
-          throw new UnprocessableEntityException(
+          if (!projectConfig.getEnableReviewerByEmail()) {
-              MessageFormat.format(
+            throw new UnprocessableEntityException(
-                  ChangeMessages.get().reviewerNotFoundUserOrGroup, input.reviewer));
+                MessageFormat.format(
+                    ChangeMessages.get().reviewerNotFoundUserOrGroup, input.reviewer));
+          }
         }
       }
-      throw new UnprocessableEntityException(
+      if (!projectConfig.getEnableReviewerByEmail()) {
-          MessageFormat.format(ChangeMessages.get().reviewerNotFoundUser, input.reviewer));
+        throw new UnprocessableEntityException(
+            MessageFormat.format(ChangeMessages.get().reviewerNotFoundUser, input.reviewer));
+      }
+      return putAccountByEmail(
+          input.reviewer,
+          rsrc,
+          input.state(),
+          input.notify,
+          notifyUtil.resolveAccounts(input.notifyDetails));
     }
     return putAccount(
         input.reviewer,
@@ -199,6 +225,7 @@ public class PostReviewers implements RestModifyView<ChangeResource, AddReviewer
         user.getUserName(),
         revision.getChangeResource(),
         ImmutableMap.of(user.getAccountId(), revision.getControl()),
+        null,
         CC,
         NotifyHandling.NONE,
         ImmutableListMultimap.of());
@@ -218,6 +245,7 @@ public class PostReviewers implements RestModifyView<ChangeResource, AddReviewer
           reviewer,
           rsrc.getChangeResource(),
           ImmutableMap.of(member.getId(), control),
+          null,
           state,
           notify,
           accountsToNotify);
@@ -228,6 +256,32 @@ public class PostReviewers implements RestModifyView<ChangeResource, AddReviewer
     throw new UnprocessableEntityException(String.format("Account of %s is inactive.", reviewer));
   }
 
+  private Addition putAccountByEmail(
+      String reviewer,
+      ChangeResource rsrc,
+      ReviewerState state,
+      NotifyHandling notify,
+      ListMultimap<RecipientType, Account.Id> accountsToNotify)
+      throws UnprocessableEntityException, OrmException, BadRequestException {
+    if (!rsrc.getControl().forUser(anonymousProvider.get()).isVisible(dbProvider.get())) {
+      throw new BadRequestException("change is not publicly visible");
+    }
+    if (!migration.readChanges()) {
+      throw new BadRequestException("feature only supported in NoteDb");
+    }
+    Address adr;
+    try {
+      adr = Address.parse(reviewer);
+    } catch (IllegalArgumentException e) {
+      throw new UnprocessableEntityException(String.format("email invalid %s", reviewer));
+    }
+    if (!OutgoingEmailValidator.isValid(adr.getEmail())) {
+      throw new UnprocessableEntityException(String.format("email invalid %s", reviewer));
+    }
+    return new Addition(
+        reviewer, rsrc, null, ImmutableList.of(adr), state, notify, accountsToNotify);
+  }
+
   private Addition putGroup(ChangeResource rsrc, AddReviewerInput input)
       throws RestApiException, OrmException, IOException {
     GroupDescription.Basic group = groupsCollection.parseInternal(input.reviewer);
@@ -283,6 +337,7 @@ public class PostReviewers implements RestModifyView<ChangeResource, AddReviewer
         input.reviewer,
         rsrc,
         reviewers,
+        null,
         input.state(),
         input.notify,
         notifyUtil.resolveAccounts(input.notifyDetails));
@@ -314,26 +369,28 @@ public class PostReviewers implements RestModifyView<ChangeResource, AddReviewer
     final Op op;
 
     private final Map<Account.Id, ChangeControl> reviewers;
+    private final Collection<Address> reviewersByEmail;
 
     protected Addition(String reviewer) {
-      this(reviewer, null, null, REVIEWER, null, ImmutableListMultimap.of());
+      this(reviewer, null, null, null, REVIEWER, null, ImmutableListMultimap.of());
     }
 
     protected Addition(
         String reviewer,
         ChangeResource rsrc,
-        Map<Account.Id, ChangeControl> reviewers,
+        @Nullable Map<Account.Id, ChangeControl> reviewers,
+        @Nullable Collection<Address> reviewersByEmail,
         ReviewerState state,
         NotifyHandling notify,
         ListMultimap<RecipientType, Account.Id> accountsToNotify) {
       result = new AddReviewerResult(reviewer);
-      if (reviewers == null) {
+      this.reviewers = reviewers == null ? ImmutableMap.of() : reviewers;
-        this.reviewers = ImmutableMap.of();
+      this.reviewersByEmail = reviewersByEmail == null ? ImmutableList.of() : reviewersByEmail;
+      if (reviewers == null && reviewersByEmail == null) {
         op = null;
         return;
       }
-      this.reviewers = reviewers;
+      op = new Op(rsrc, this.reviewers, this.reviewersByEmail, state, notify, accountsToNotify);
-      op = new Op(rsrc, reviewers, state, notify, accountsToNotify);
     }
 
     void gatherResults() throws OrmException {
@@ -345,6 +402,9 @@ public class PostReviewers implements RestModifyView<ChangeResource, AddReviewer
           result.ccs.add(json.format(new ReviewerInfo(accountId.get()), reviewers.get(accountId)));
         }
         accountLoaderFactory.create(true).fill(result.ccs);
+        for (Address a : reviewersByEmail) {
+          result.ccs.add(new AccountInfo(a.getName(), a.getEmail()));
+        }
       } else {
         result.reviewers = Lists.newArrayListWithCapacity(op.addedReviewers.size());
         for (PatchSetApproval psa : op.addedReviewers) {
@@ -356,17 +416,22 @@ public class PostReviewers implements RestModifyView<ChangeResource, AddReviewer
                   ImmutableList.of(psa)));
         }
         accountLoaderFactory.create(true).fill(result.reviewers);
+        for (Address a : reviewersByEmail) {
+          result.reviewers.add(ReviewerInfo.byEmail(a.getName(), a.getEmail()));
+        }
       }
     }
   }
 
   public class Op implements BatchUpdateOp {
     final Map<Account.Id, ChangeControl> reviewers;
+    final Collection<Address> reviewersByEmail;
     final ReviewerState state;
     final NotifyHandling notify;
     final ListMultimap<RecipientType, Account.Id> accountsToNotify;
-    List<PatchSetApproval> addedReviewers;
+    List<PatchSetApproval> addedReviewers = new ArrayList<>();
-    Collection<Account.Id> addedCCs;
+    Collection<Account.Id> addedCCs = new ArrayList<>();
+    Collection<Address> addedCCsByEmail = new ArrayList<>();
 
     private final ChangeResource rsrc;
     private PatchSet patchSet;
@@ -374,11 +439,13 @@ public class PostReviewers implements RestModifyView<ChangeResource, AddReviewer
     Op(
         ChangeResource rsrc,
         Map<Account.Id, ChangeControl> reviewers,
+        Collection<Address> reviewersByEmail,
         ReviewerState state,
         NotifyHandling notify,
         ListMultimap<RecipientType, Account.Id> accountsToNotify) {
       this.rsrc = rsrc;
       this.reviewers = reviewers;
+      this.reviewersByEmail = reviewersByEmail;
       this.state = state;
       this.notify = notify;
       this.accountsToNotify = checkNotNull(accountsToNotify);
@@ -387,55 +454,55 @@ public class PostReviewers implements RestModifyView<ChangeResource, AddReviewer
     @Override
     public boolean updateChange(ChangeContext ctx)
         throws RestApiException, OrmException, IOException {
-      if (migration.readChanges() && state == CC) {
+      if (!reviewers.isEmpty()) {
-        addedCCs =
+        if (migration.readChanges() && state == CC) {
-            approvalsUtil.addCcs(
+          addedCCs =
-                ctx.getNotes(),
+              approvalsUtil.addCcs(
-                ctx.getUpdate(ctx.getChange().currentPatchSetId()),
+                  ctx.getNotes(),
-                reviewers.keySet());
+                  ctx.getUpdate(ctx.getChange().currentPatchSetId()),
-        if (addedCCs.isEmpty()) {
+                  reviewers.keySet());
-          return false;
+          if (addedCCs.isEmpty()) {
-        }
+            return false;
-      } else {
+          }
-        addedReviewers =
+        } else {
-            approvalsUtil.addReviewers(
+          addedReviewers =
-                ctx.getDb(),
+              approvalsUtil.addReviewers(
-                ctx.getNotes(),
+                  ctx.getDb(),
-                ctx.getUpdate(ctx.getChange().currentPatchSetId()),
+                  ctx.getNotes(),
-                rsrc.getControl().getLabelTypes(),
+                  ctx.getUpdate(ctx.getChange().currentPatchSetId()),
-                rsrc.getChange(),
+                  rsrc.getControl().getLabelTypes(),
-                reviewers.keySet());
+                  rsrc.getChange(),
-        if (addedReviewers.isEmpty()) {
+                  reviewers.keySet());
-          return false;
+          if (addedReviewers.isEmpty()) {
+            return false;
+          }
         }
       }
 
+      for (Address a : reviewersByEmail) {
+        ctx.getUpdate(ctx.getChange().currentPatchSetId())
+            .putReviewerByEmail(a, ReviewerStateInternal.fromReviewerState(state));
+      }
+
       patchSet = psUtil.current(dbProvider.get(), rsrc.getNotes());
       return true;
     }
 
     @Override
     public void postUpdate(Context ctx) throws Exception {
-      if (addedReviewers != null || addedCCs != null) {
+      emailReviewers(
-        if (addedReviewers == null) {
+          rsrc.getChange(),
-          addedReviewers = new ArrayList<>();
+          Lists.transform(addedReviewers, r -> r.getAccountId()),
-        }
+          addedCCs == null ? ImmutableList.of() : addedCCs,
-        if (addedCCs == null) {
+          reviewersByEmail,
-          addedCCs = new ArrayList<>();
+          addedCCsByEmail,
-        }
+          notify,
-        emailReviewers(
+          accountsToNotify);
-            rsrc.getChange(),
+      if (!addedReviewers.isEmpty()) {
-            Lists.transform(addedReviewers, r -> r.getAccountId()),
+        List<Account> reviewers =
-            addedCCs,
+            Lists.transform(
-            notify,
+                addedReviewers, psa -> accountCache.get(psa.getAccountId()).getAccount());
-            accountsToNotify);
+        reviewerAdded.fire(rsrc.getChange(), patchSet, reviewers, ctx.getAccount(), ctx.getWhen());
-        if (!addedReviewers.isEmpty()) {
-          List<Account> reviewers =
-              Lists.transform(
-                  addedReviewers, psa -> accountCache.get(psa.getAccountId()).getAccount());
-          reviewerAdded.fire(
-              rsrc.getChange(), patchSet, reviewers, ctx.getAccount(), ctx.getWhen());
-        }
       }
     }
   }
@@ -444,9 +511,11 @@ public class PostReviewers implements RestModifyView<ChangeResource, AddReviewer
       Change change,
       Collection<Account.Id> added,
       Collection<Account.Id> copied,
+      Collection<Address> addedByEmail,
+      Collection<Address> copiedByEmail,
       NotifyHandling notify,
       ListMultimap<RecipientType, Account.Id> accountsToNotify) {
-    if (added.isEmpty() && copied.isEmpty()) {
+    if (added.isEmpty() && copied.isEmpty() && addedByEmail.isEmpty() && copiedByEmail.isEmpty()) {
       return;
     }
 
@@ -466,7 +535,7 @@ public class PostReviewers implements RestModifyView<ChangeResource, AddReviewer
         toCopy.add(id);
       }
     }
-    if (toMail.isEmpty() && toCopy.isEmpty()) {
+    if (toMail.isEmpty() && toCopy.isEmpty() && addedByEmail.isEmpty() && copiedByEmail.isEmpty()) {
       return;
     }
 
@@ -478,7 +547,9 @@ public class PostReviewers implements RestModifyView<ChangeResource, AddReviewer
       cm.setAccountsToNotify(accountsToNotify);
       cm.setFrom(userId);
       cm.addReviewers(toMail);
+      cm.addReviewersByEmail(addedByEmail);
       cm.addExtraCC(toCopy);
+      cm.addExtraCCByEmail(copiedByEmail);
       cm.send();
     } catch (Exception err) {
       log.error("Cannot send email to new reviewers of change " + change.getId(), err);

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

@@ -14,11 +14,15 @@
 
 package com.google.gerrit.server.change;
 
+import static com.google.common.base.Preconditions.checkArgument;
+
+import com.google.gerrit.common.Nullable;
 import com.google.gerrit.extensions.restapi.RestResource;
 import com.google.gerrit.extensions.restapi.RestView;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.server.mail.Address;
 import com.google.gerrit.server.project.ChangeControl;
 import com.google.inject.TypeLiteral;
 import com.google.inject.assistedinject.Assisted;
@@ -36,7 +40,8 @@ public class ReviewerResource implements RestResource {
 
   private final ChangeResource change;
   private final RevisionResource revision;
-  private final IdentifiedUser user;
+  @Nullable private final IdentifiedUser user;
+  @Nullable private final Address address;
 
   @AssistedInject
   ReviewerResource(
@@ -44,8 +49,9 @@ public class ReviewerResource implements RestResource {
       @Assisted ChangeResource change,
       @Assisted Account.Id id) {
     this.change = change;
-    this.revision = null;
     this.user = userFactory.create(id);
+    this.revision = null;
+    this.address = null;
   }
 
   @AssistedInject
@@ -56,6 +62,21 @@ public class ReviewerResource implements RestResource {
     this.revision = revision;
     this.change = revision.getChangeResource();
     this.user = userFactory.create(id);
+    this.address = null;
+  }
+
+  ReviewerResource(ChangeResource change, Address address) {
+    this.change = change;
+    this.address = address;
+    this.revision = null;
+    this.user = null;
+  }
+
+  ReviewerResource(RevisionResource revision, Address address) {
+    this.revision = revision;
+    this.change = revision.getChangeResource();
+    this.address = address;
+    this.user = null;
   }
 
   public ChangeResource getChangeResource() {
@@ -75,10 +96,28 @@ public class ReviewerResource implements RestResource {
   }
 
   public IdentifiedUser getReviewerUser() {
+    checkArgument(user != null, "no user provided");
     return user;
   }
 
+  public Address getReviewerByEmail() {
+    checkArgument(address != null, "no address provided");
+    return address;
+  }
+
   /**
+   * Check if this resource was constructed by email or by {@code Account.Id}.
+   *
+   * @return true if the resource was constructed by providing an {@code Address}; false if the
+   *     resource was constructed by providing an {@code Account.Id}.
+   */
+  public boolean isByEmail() {
+    return user == null;
+  }
+
+  /**
+   * Get the control for the caller's user.
+   *
    * @return the control for the caller's user (as opposed to the reviewer's user as returned by
    *     {@link #getReviewerControl()}).
    */
@@ -87,10 +126,13 @@ public class ReviewerResource implements RestResource {
   }
 
   /**
+   * Get the control for the reviewer's user.
+   *
    * @return the control for the reviewer's user (as opposed to the caller's user as returned by
    *     {@link #getControl()}).
    */
   public ChangeControl getReviewerControl() {
+    checkArgument(user != null, "no user provided");
     return change.getControl().forUser(user);
   }
 }

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

@@ -25,6 +25,7 @@ import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.ApprovalsUtil;
 import com.google.gerrit.server.account.AccountsCollection;
+import com.google.gerrit.server.mail.Address;
 import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
@@ -69,12 +70,26 @@ public class Reviewers implements ChildCollection<ChangeResource, ReviewerResour
   @Override
   public ReviewerResource parse(ChangeResource rsrc, IdString id)
       throws OrmException, ResourceNotFoundException, AuthException {
-    Account.Id accountId = accounts.parse(TopLevelResource.INSTANCE, id).getUser().getAccountId();
+    Address address = Address.tryParse(id.get());
 
+    Account.Id accountId = null;
+    try {
+      accountId = accounts.parse(TopLevelResource.INSTANCE, id).getUser().getAccountId();
+    } catch (ResourceNotFoundException e) {
+      if (address == null) {
+        throw e;
+      }
+    }
     // See if the id exists as a reviewer for this change
-    if (fetchAccountIds(rsrc).contains(accountId)) {
+    if (accountId != null && fetchAccountIds(rsrc).contains(accountId)) {
       return resourceFactory.create(rsrc, accountId);
     }
+
+    // See if the address exists as a reviewer on the change
+    if (address != null && rsrc.getNotes().getReviewersByEmail().all().contains(address)) {
+      return new ReviewerResource(rsrc, address);
+    }
+
     throw new ResourceNotFoundException(id);
   }
 

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

@@ -26,6 +26,7 @@ import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.ApprovalsUtil;
 import com.google.gerrit.server.account.AccountsCollection;
+import com.google.gerrit.server.mail.Address;
 import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
@@ -73,14 +74,28 @@ public class RevisionReviewers implements ChildCollection<RevisionResource, Revi
     if (!rsrc.isCurrent()) {
       throw new MethodNotAllowedException("Cannot access on non-current patch set");
     }
+    Address address = Address.tryParse(id.get());
 
-    Account.Id accountId = accounts.parse(TopLevelResource.INSTANCE, id).getUser().getAccountId();
+    Account.Id accountId = null;
-
+    try {
+      accountId = accounts.parse(TopLevelResource.INSTANCE, id).getUser().getAccountId();
+    } catch (ResourceNotFoundException e) {
+      if (address == null) {
+        throw e;
+      }
+    }
     Collection<Account.Id> reviewers =
         approvalsUtil.getReviewers(dbProvider.get(), rsrc.getNotes()).all();
+    // See if the id exists as a reviewer for this change
     if (reviewers.contains(accountId)) {
       return resourceFactory.create(rsrc, accountId);
     }
+
+    // See if the address exists as a reviewer on the change
+    if (address != null && rsrc.getNotes().getReviewersByEmail().all().contains(address)) {
+      return new ReviewerResource(rsrc, address);
+    }
+
     throw new ResourceNotFoundException(id);
   }
 }

gerrit-server/src/main/java/com/google/gerrit/server/mail/Address.java

@@ -42,6 +42,14 @@ public class Address {
     throw new IllegalArgumentException("Invalid email address: " + in);
   }
 
+  public static Address tryParse(String in) {
+    try {
+      return parse(in);
+    } catch (IllegalArgumentException e) {
+      return null;
+    }
+  }
+
   final String name;
   final String email;
 

gerrit-server/src/main/java/com/google/gerrit/server/mail/send/ChangeEmail.java

@@ -180,6 +180,20 @@ public abstract class ChangeEmail extends NotificationEmail {
     setHeader("X-Gerrit-Change-Number", "" + change.getChangeId());
     setChangeUrlHeader();
     setCommitIdHeader();
+
+    if (notify.ordinal() >= NotifyHandling.OWNER_REVIEWERS.ordinal()) {
+      try {
+        // TODO(hiesel) Load from index instead
+        addByEmail(
+            RecipientType.CC,
+            changeData.notes().getReviewersByEmail().byState(ReviewerStateInternal.CC));
+        addByEmail(
+            RecipientType.TO,
+            changeData.notes().getReviewersByEmail().byState(ReviewerStateInternal.REVIEWER));
+      } catch (OrmException e) {
+        throw new EmailException("Failed to add unregistered CCs " + change.getChangeId(), e);
+      }
+    }
   }
 
   private void setChangeUrlHeader() {

gerrit-server/src/main/java/com/google/gerrit/server/mail/send/DeleteReviewerSender.java

@@ -20,6 +20,7 @@ import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.Project;
 import com.google.gerrit.server.account.WatchConfig.NotifyType;
+import com.google.gerrit.server.mail.Address;
 import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
 import com.google.inject.assistedinject.Assisted;
@@ -32,6 +33,7 @@ import java.util.Set;
 /** Let users know that a reviewer and possibly her review have been removed. */
 public class DeleteReviewerSender extends ReplyToChangeSender {
   private final Set<Account.Id> reviewers = new HashSet<>();
+  private final Set<Address> reviewersByEmail = new HashSet<>();
 
   public interface Factory extends ReplyToChangeSender.Factory<DeleteReviewerSender> {
     @Override
@@ -49,6 +51,10 @@ public class DeleteReviewerSender extends ReplyToChangeSender {
     reviewers.addAll(cc);
   }
 
+  public void addReviewersByEmail(Collection<Address> cc) {
+    reviewersByEmail.addAll(cc);
+  }
+
   @Override
   protected void init() throws EmailException {
     super.init();
@@ -58,6 +64,7 @@ public class DeleteReviewerSender extends ReplyToChangeSender {
     ccExistingReviewers();
     includeWatchers(NotifyType.ALL_COMMENTS);
     add(RecipientType.TO, reviewers);
+    addByEmail(RecipientType.TO, reviewersByEmail);
     removeUsersThatIgnoredTheChange();
   }
 
@@ -70,13 +77,16 @@ public class DeleteReviewerSender extends ReplyToChangeSender {
   }
 
   public List<String> getReviewerNames() {
-    if (reviewers.isEmpty()) {
+    if (reviewers.isEmpty() && reviewersByEmail.isEmpty()) {
       return null;
     }
     List<String> names = new ArrayList<>();
     for (Account.Id id : reviewers) {
       names.add(getNameFor(id));
     }
+    for (Address a : reviewersByEmail) {
+      names.add(a.toString());
+    }
     return names;
   }
 

gerrit-server/src/main/java/com/google/gerrit/server/mail/send/NewChangeSender.java

@@ -17,6 +17,7 @@ package com.google.gerrit.server.mail.send;
 import com.google.gerrit.common.errors.EmailException;
 import com.google.gerrit.extensions.api.changes.RecipientType;
 import com.google.gerrit.reviewdb.client.Account;
+import com.google.gerrit.server.mail.Address;
 import com.google.gerrit.server.query.change.ChangeData;
 import com.google.gwtorm.server.OrmException;
 import java.util.ArrayList;
@@ -28,7 +29,9 @@ import java.util.Set;
 /** Sends an email alerting a user to a new change for them to review. */
 public abstract class NewChangeSender extends ChangeEmail {
   private final Set<Account.Id> reviewers = new HashSet<>();
+  private final Set<Address> reviewersByEmail = new HashSet<>();
   private final Set<Account.Id> extraCC = new HashSet<>();
+  private final Set<Address> extraCCByEmail = new HashSet<>();
 
   protected NewChangeSender(EmailArguments ea, ChangeData cd) throws OrmException {
     super(ea, "newchange", cd);
@@ -38,10 +41,18 @@ public abstract class NewChangeSender extends ChangeEmail {
     reviewers.addAll(cc);
   }
 
+  public void addReviewersByEmail(final Collection<Address> cc) {
+    reviewersByEmail.addAll(cc);
+  }
+
   public void addExtraCC(final Collection<Account.Id> cc) {
     extraCC.addAll(cc);
   }
 
+  public void addExtraCCByEmail(final Collection<Address> cc) {
+    extraCCByEmail.addAll(cc);
+  }
+
   @Override
   protected void init() throws EmailException {
     super.init();
@@ -55,9 +66,11 @@ public abstract class NewChangeSender extends ChangeEmail {
       case ALL:
       default:
         add(RecipientType.CC, extraCC);
+        extraCCByEmail.stream().forEach(cc -> add(RecipientType.CC, cc));
         //$FALL-THROUGH$
       case OWNER_REVIEWERS:
         add(RecipientType.TO, reviewers);
+        addByEmail(RecipientType.TO, reviewersByEmail);
         break;
     }
 

gerrit-server/src/main/java/com/google/gerrit/server/mail/send/OutgoingEmail.java

@@ -441,6 +441,13 @@ public abstract class OutgoingEmail {
     }
   }
 
+  /** Schedule this message for delivery to the listed address. */
+  protected void addByEmail(final RecipientType rt, final Collection<Address> list) {
+    for (final Address id : list) {
+      add(rt, id);
+    }
+  }
+
   protected void add(final RecipientType rt, final UserIdentity who) {
     if (who != null && who.getAccount() != null) {
       add(rt, who.getAccount());

gerrit-server/src/main/java/com/google/gerrit/server/notedb/ReviewerStateInternal.java

@@ -29,13 +29,17 @@ public enum ReviewerStateInternal {
   /** The user was previously a reviewer on the change, but was removed. */
   REMOVED(new FooterKey("Removed"), ReviewerState.REMOVED);
 
+  public static ReviewerStateInternal fromReviewerState(ReviewerState state) {
+    return ReviewerStateInternal.values()[state.ordinal()];
+  }
+
   static {
     boolean ok = true;
     if (ReviewerStateInternal.values().length != ReviewerState.values().length) {
       ok = false;
     }
-    for (ReviewerStateInternal s : ReviewerStateInternal.values()) {
+    for (int i = 0; i < ReviewerStateInternal.values().length; i++) {
-      ok &= s.name().equals(s.state.name());
+      ok &= ReviewerState.values()[i].equals(ReviewerStateInternal.values()[i].state);
     }
     if (!ok) {
       throw new IllegalStateException(