Port AddReviewer to new REST API

Reviewers can now be added by a POST to /changes/*/reviewers with body
containing the reviewer string to search for.

The new code closely parallels the old but is simplified where
possible. The new handler only supports putting a single reviewer,
which is all the old handler was ever used for. Error handling is also
simplified, using server-supplied error text.

The response body includes a list "reviewers" of reviewers that were
newly added (which may be empty if), or an "error" string and optional
"confirm" boolean. If "confirm" is true, the error text describes why
confirmation is required, and redoing the POST with "confirmed": true in
the JSON body indicates confirmation.

Change-Id: If3d72bc1d3be7f6b96bfae7819d8b4b575104eaa

gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeMessages.properties

@@ -1,3 +1,5 @@
+# Changes to this file should also be made in
+# gerrit-server/src/main/resources/com/google/gerrit/server/change/ChangeMessages.properties
 accountDashboardTitle = Code Review Dashboard for {0}
 changesOpenInProject = Open Changes In {0}
 changesMergedInProject = Merged Changes In {0}

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

@@ -28,6 +28,7 @@ import com.google.inject.Inject;
 import com.google.inject.Provider;
 import com.google.inject.assistedinject.Assisted;
 
+import java.util.Collection;
 import java.util.List;
 import java.util.Map;
 
@@ -89,6 +90,14 @@ public class AccountInfo {
       }
     }
 
+    public void fill(Collection<? extends AccountInfo> infos)
+        throws OrmException {
+      for (AccountInfo info : infos) {
+        put(info);
+      }
+      fill();
+    }
+
     private void fill(AccountInfo info, Account account) {
       info.name = account.getFullName();
       if (detailed) {
@@ -98,7 +107,7 @@ public class AccountInfo {
     }
   }
 
-  private transient Account.Id _id;
+  transient Account.Id _id;
 
   protected AccountInfo(Account.Id id) {
     _id = id;

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

@@ -0,0 +1,28 @@
+// Copyright (C) 2013 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 org.eclipse.jgit.nls.NLS;
+import org.eclipse.jgit.nls.TranslationBundle;
+
+public class ChangeMessages extends TranslationBundle {
+  public static ChangeMessages get() {
+    return NLS.getBundleFor(ChangeMessages.class);
+  }
+
+  public String groupIsNotAllowed;
+  public String groupHasTooManyMembers;
+  public String groupManyMembersConfirmation;
+}

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

@@ -21,7 +21,6 @@ import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.PatchSetApproval;
 import com.google.gerrit.reviewdb.server.ReviewDb;
-import com.google.gerrit.server.account.AccountCache;
 import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
@@ -29,17 +28,14 @@ import com.google.inject.Provider;
 import java.util.Map;
 
 class ListReviewers implements RestReadView<ChangeResource> {
-  private final AccountCache accountCache;
   private final Provider<ReviewDb> dbProvider;
   private final ReviewerJson json;
   private final ReviewerResource.Factory resourceFactory;
 
   @Inject
-  ListReviewers(AccountCache accountCache,
+  ListReviewers(Provider<ReviewDb> dbProvider,
-      Provider<ReviewDb> dbProvider,
       ReviewerResource.Factory resourceFactory,
       ReviewerJson json) {
-    this.accountCache = accountCache;
     this.dbProvider = dbProvider;
     this.resourceFactory = resourceFactory;
     this.json = json;
@@ -55,8 +51,7 @@ class ListReviewers implements RestReadView<ChangeResource> {
          : db.patchSetApprovals().byChange(changeId)) {
       Account.Id accountId = patchSetApproval.getAccountId();
       if (!reviewers.containsKey(accountId)) {
-        Account account = accountCache.get(accountId).getAccount();
+        reviewers.put(accountId, resourceFactory.create(rsrc, accountId));
-        reviewers.put(accountId, resourceFactory.create(rsrc, account));
       }
     }
     return json.format(reviewers.values());

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

@@ -46,10 +46,11 @@ public class Module extends RestApiModule {
     delete(CHANGE_KIND, "topic").to(PutTopic.class);
     post(CHANGE_KIND, "abandon").to(Abandon.class);
     post(CHANGE_KIND, "restore").to(Restore.class);
-    child(CHANGE_KIND, "reviewers").to(Reviewers.class);
     post(CHANGE_KIND, "revert").to(Revert.class);
     post(CHANGE_KIND, "submit").to(Submit.CurrentRevision.class);
 
+    post(CHANGE_KIND, "reviewers").to(PostReviewers.class);
+    child(CHANGE_KIND, "reviewers").to(Reviewers.class);
     get(REVIEWER_KIND).to(GetReviewer.class);
     delete(REVIEWER_KIND).to(DeleteReviewer.class);
 

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

@@ -0,0 +1,288 @@
+// Copyright (C) 2013 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.base.Objects;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Iterables;
+import com.google.common.collect.Lists;
+import com.google.common.collect.Sets;
+import com.google.gerrit.common.ChangeHooks;
+import com.google.gerrit.common.data.ApprovalTypes;
+import com.google.gerrit.common.errors.EmailException;
+import com.google.gerrit.common.errors.NoSuchGroupException;
+import com.google.gerrit.extensions.restapi.AuthException;
+import com.google.gerrit.extensions.restapi.DefaultInput;
+import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
+import com.google.gerrit.extensions.restapi.RestModifyView;
+import com.google.gerrit.reviewdb.client.Account;
+import com.google.gerrit.reviewdb.client.AccountGroup;
+import com.google.gerrit.reviewdb.client.ApprovalCategory;
+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.IdentifiedUser;
+import com.google.gerrit.server.account.AccountCache;
+import com.google.gerrit.server.account.GroupCache;
+import com.google.gerrit.server.account.GroupMembers;
+import com.google.gerrit.server.change.PostReviewers.Input;
+import com.google.gerrit.server.change.ReviewerJson.PostResult;
+import com.google.gerrit.server.change.ReviewerJson.ReviewerInfo;
+import com.google.gerrit.server.config.GerritServerConfig;
+import com.google.gerrit.server.mail.AddReviewerSender;
+import com.google.gerrit.server.project.ChangeControl;
+import com.google.gerrit.server.project.NoSuchChangeException;
+import com.google.gerrit.server.project.NoSuchProjectException;
+import com.google.gwtorm.server.OrmException;
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+
+import org.eclipse.jgit.lib.Config;
+
+import java.text.MessageFormat;
+import java.util.List;
+import java.util.Set;
+
+class PostReviewers implements RestModifyView<ChangeResource, Input> {
+  public final static int DEFAULT_MAX_REVIEWERS_WITHOUT_CHECK = 10;
+  public final static int DEFAULT_MAX_REVIEWERS = 20;
+
+  static class Input {
+    @DefaultInput
+    String reviewer;
+    Boolean confirmed;
+
+    boolean confirmed() {
+      return Objects.firstNonNull(confirmed, false);
+    }
+  }
+
+  private final Reviewers.Parser parser;
+  private final ReviewerResource.Factory reviewerFactory;
+  private final AddReviewerSender.Factory addReviewerSenderFactory;
+  private final GroupCache groupCache;
+  private final GroupMembers.Factory groupMembersFactory;
+  private final AccountInfo.Loader.Factory accountLoaderFactory;
+  private final Provider<ReviewDb> db;
+  private final IdentifiedUser currentUser;
+  private final IdentifiedUser.GenericFactory identifiedUserFactory;
+  private final ApprovalCategory.Id addReviewerCategoryId;
+  private final Config cfg;
+  private final ChangeHooks hooks;
+  private final AccountCache accountCache;
+  private final ReviewerJson json;
+
+  @Inject
+  PostReviewers(Reviewers.Parser parser,
+      ReviewerResource.Factory reviewerFactory,
+      AddReviewerSender.Factory addReviewerSenderFactory,
+      GroupCache groupCache,
+      GroupMembers.Factory groupMembersFactory,
+      AccountInfo.Loader.Factory accountLoaderFactory,
+      Provider<ReviewDb> db,
+      IdentifiedUser currentUser,
+      IdentifiedUser.GenericFactory identifiedUserFactory,
+      ApprovalTypes approvalTypes,
+      @GerritServerConfig Config cfg,
+      ChangeHooks hooks,
+      AccountCache accountCache,
+      ReviewerJson json) {
+    this.parser = parser;
+    this.reviewerFactory = reviewerFactory;
+    this.addReviewerSenderFactory = addReviewerSenderFactory;
+    this.groupCache = groupCache;
+    this.groupMembersFactory = groupMembersFactory;
+    this.accountLoaderFactory = accountLoaderFactory;
+    this.db = db;
+    this.currentUser = currentUser;
+    this.identifiedUserFactory = identifiedUserFactory;
+    this.cfg = cfg;
+    this.hooks = hooks;
+    this.accountCache = accountCache;
+    this.json = json;
+
+    this.addReviewerCategoryId = Iterables.getLast(
+        approvalTypes.getApprovalTypes()).getCategory().getId();
+  }
+
+  @Override
+  public PostResult apply(ChangeResource rsrc, Input input)
+      throws ResourceNotFoundException, AuthException, OrmException,
+      EmailException {
+    Account.Id accountId = parser.parse(rsrc, input.reviewer);
+    try {
+      if (accountId != null) {
+        return putAccount(reviewerFactory.create(rsrc, accountId));
+      } else {
+        return putGroup(rsrc, input);
+      }
+    } catch (NoSuchChangeException e) {
+      throw new ResourceNotFoundException(e.getMessage());
+    } catch (NoSuchGroupException e) {
+      throw new ResourceNotFoundException(e.getMessage());
+    } catch (NoSuchProjectException e) {
+      throw new ResourceNotFoundException(e.getMessage());
+    }
+  }
+
+  private PostResult putAccount(ReviewerResource rsrc) throws OrmException,
+      EmailException, NoSuchChangeException {
+    PostResult result = new PostResult();
+    addReviewers(rsrc, result, ImmutableSet.of(rsrc.getUser()));
+    return result;
+  }
+
+  private PostResult putGroup(ChangeResource rsrc, Input input)
+      throws ResourceNotFoundException, AuthException, NoSuchGroupException,
+      NoSuchProjectException, OrmException, NoSuchChangeException,
+      EmailException {
+    AccountGroup group =
+        groupCache.get(new AccountGroup.NameKey(input.reviewer));
+    if (group == null) {
+      throw new ResourceNotFoundException(input.reviewer);
+    }
+    PostResult result = new PostResult();
+    if (!isLegalReviewerGroup(group.getGroupUUID())) {
+      result.error = MessageFormat.format(
+          ChangeMessages.get().groupIsNotAllowed, group.getName());
+      return result;
+    }
+
+    Set<IdentifiedUser> reviewers = Sets.newLinkedHashSet();
+    ChangeControl control = rsrc.getControl();
+    Set<Account> members = groupMembersFactory.create(control.getCurrentUser())
+        .listAccounts(group.getGroupUUID(), control.getProject().getNameKey());
+
+    // if maxAllowed is set to 0, it is allowed to add any number of
+    // reviewers
+    int maxAllowed =
+        cfg.getInt("addreviewer", "maxAllowed", DEFAULT_MAX_REVIEWERS);
+    if (maxAllowed > 0 && members.size() > maxAllowed) {
+      result.error = MessageFormat.format(
+          ChangeMessages.get().groupHasTooManyMembers, group.getName());
+      return result;
+    }
+
+    // if maxWithoutCheck is set to 0, we never ask for confirmation
+    int maxWithoutConfirmation =
+        cfg.getInt("addreviewer", "maxWithoutConfirmation",
+            DEFAULT_MAX_REVIEWERS_WITHOUT_CHECK);
+    if (!input.confirmed() && maxWithoutConfirmation > 0
+        && members.size() > maxWithoutConfirmation) {
+      result.confirm = true;
+      result.error = MessageFormat.format(
+          ChangeMessages.get().groupManyMembersConfirmation,
+          group.getName(), members.size());
+      return result;
+    }
+
+    for (Account member : members) {
+      if (member.isActive()) {
+        IdentifiedUser user = identifiedUserFactory.create(member.getId());
+        // Does not account for draft status as a user might want to let a
+        // reviewer see a draft.
+        if (control.forUser(user).isRefVisible()) {
+          reviewers.add(user);
+        }
+      }
+    }
+
+    addReviewers(rsrc, result, reviewers);
+    return result;
+  }
+
+  private void addReviewers(ChangeResource rsrc, PostResult result,
+      Set<IdentifiedUser> reviewers) throws OrmException, EmailException,
+      NoSuchChangeException {
+    if (reviewers.isEmpty()) {
+      result.reviewers = ImmutableList.of();
+      return;
+    }
+
+    PatchSet.Id psid = rsrc.getChange().currentPatchSetId();
+    Set<Account.Id> existing = Sets.newHashSet();
+    for (PatchSetApproval psa : db.get().patchSetApprovals().byPatchSet(psid)) {
+      existing.add(psa.getAccountId());
+    }
+
+    result.reviewers = Lists.newArrayListWithCapacity(reviewers.size());
+    List<PatchSetApproval> toInsert =
+        Lists.newArrayListWithCapacity(reviewers.size());
+    for (IdentifiedUser user : reviewers) {
+      Account.Id id = user.getAccountId();
+      if (existing.contains(id)) {
+        continue;
+      }
+      ChangeControl control = rsrc.getControl().forUser(user);
+      PatchSetApproval psa = dummyApproval(control.getChange(), psid, id);
+      result.reviewers.add(json.format(
+          new ReviewerInfo(id), control, ImmutableList.of(psa)));
+      toInsert.add(psa);
+    }
+    db.get().patchSetApprovals().insert(toInsert);
+    accountLoaderFactory.create(true).fill(result.reviewers);
+    postAdd(rsrc.getChange(), result);
+  }
+
+  private void postAdd(Change change, PostResult result)
+      throws OrmException, EmailException {
+    if (result.reviewers.isEmpty()) {
+      return;
+    }
+
+    // Execute hook for added reviewers
+    //
+    PatchSet patchSet = db.get().patchSets().get(change.currentPatchSetId());
+    for (AccountInfo info : result.reviewers) {
+      Account account = accountCache.get(info._id).getAccount();
+      hooks.doReviewerAddedHook(change, account, patchSet, db.get());
+    }
+
+    // Email the reviewers
+    //
+    // The user knows they added themselves, don't bother emailing them.
+    List<Account.Id> added =
+        Lists.newArrayListWithCapacity(result.reviewers.size());
+    for (AccountInfo info : result.reviewers) {
+      if (!info._id.equals(currentUser.getAccountId())) {
+        added.add(info._id);
+      }
+    }
+    if (!added.isEmpty()) {
+      AddReviewerSender cm;
+
+      cm = addReviewerSenderFactory.create(change);
+      cm.setFrom(currentUser.getAccountId());
+      cm.addReviewers(added);
+      cm.send();
+    }
+  }
+
+  public static boolean isLegalReviewerGroup(AccountGroup.UUID groupUUID) {
+    return !(AccountGroup.ANONYMOUS_USERS.equals(groupUUID)
+             || AccountGroup.REGISTERED_USERS.equals(groupUUID));
+  }
+
+  private PatchSetApproval dummyApproval(Change change, PatchSet.Id patchSetId,
+      Account.Id reviewerId) {
+    PatchSetApproval dummyApproval =
+        new PatchSetApproval(new PatchSetApproval.Key(patchSetId, reviewerId,
+            addReviewerCategoryId), (short) 0);
+    dummyApproval.cache(change);
+    return dummyApproval;
+  }
+}

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

@@ -22,7 +22,6 @@ import com.google.gerrit.common.data.ApprovalTypes;
 import com.google.gerrit.common.data.PermissionRange;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.ApprovalCategoryValue;
-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;
@@ -58,7 +57,7 @@ public class ReviewerJson {
     List<ReviewerInfo> infos = Lists.newArrayListWithCapacity(rsrcs.size());
     AccountInfo.Loader loader = accountLoaderFactory.create(true);
     for (ReviewerResource rsrc : rsrcs) {
-      ReviewerInfo info = formatOne(rsrc);
+      ReviewerInfo info = format(rsrc, null);
       loader.put(info);
       infos.add(info);
     }
@@ -70,17 +69,15 @@ public class ReviewerJson {
     return format(ImmutableList.<ReviewerResource> of(rsrc));
   }
 
-  private ReviewerInfo formatOne(ReviewerResource rsrc) throws OrmException {
+  public ReviewerInfo format(ReviewerInfo out, ChangeControl control,
-    Account.Id id = rsrc.getUser().getAccountId();
+      List<PatchSetApproval> approvals) throws OrmException {
-    ReviewerInfo out = new ReviewerInfo(id);
+    PatchSet.Id psId = control.getChange().currentPatchSetId();
 
-    Change change = rsrc.getChange();
+    if (approvals == null) {
-    PatchSet.Id psId = change.currentPatchSetId();
+      approvals = db.get().patchSetApprovals()
+          .byPatchSetUser(psId, out._id).toList();
+    }
 
-    List<PatchSetApproval> approvals = db.get().patchSetApprovals()
-        .byPatchSetUser(psId, id).toList();
-
-    ChangeControl control = rsrc.getControl().forUser(rsrc.getUser());
     FunctionState fs = functionState.create(control, psId, approvals);
     for (ApprovalType at : approvalTypes.getApprovalTypes()) {
       CategoryFunction.forCategory(at.getCategory()).run(at, fs);
@@ -106,6 +103,12 @@ public class ReviewerJson {
     return out;
   }
 
+  private ReviewerInfo format(ReviewerResource rsrc,
+      List<PatchSetApproval> approvals) throws OrmException {
+    return format(new ReviewerInfo(rsrc.getUser().getAccountId()),
+        rsrc.getUserControl(), approvals);
+  }
+
   public static class ReviewerInfo extends AccountInfo {
     final String kind = "gerritcodereview#reviewer";
     Map<String, String> approvals;
@@ -114,4 +117,10 @@ public class ReviewerJson {
       super(id);
     }
   }
+
+  public static class PostResult {
+    List<ReviewerInfo> reviewers;
+    Boolean confirm;
+    String error;
+  }
 }

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

@@ -17,6 +17,7 @@ package com.google.gerrit.server.change;
 import com.google.gerrit.extensions.restapi.RestView;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.server.project.ChangeControl;
 import com.google.inject.TypeLiteral;
 import com.google.inject.assistedinject.Assisted;
 import com.google.inject.assistedinject.AssistedInject;
@@ -27,7 +28,7 @@ public class ReviewerResource extends ChangeResource {
 
   static interface Factory {
     ReviewerResource create(ChangeResource rsrc, IdentifiedUser user);
-    ReviewerResource create(ChangeResource rsrc, Account account);
+    ReviewerResource create(ChangeResource rsrc, Account.Id id);
   }
 
   private final IdentifiedUser user;
@@ -42,11 +43,19 @@ public class ReviewerResource extends ChangeResource {
   @AssistedInject
   ReviewerResource(IdentifiedUser.GenericFactory userFactory,
       @Assisted ChangeResource rsrc,
-      @Assisted Account account) {
+      @Assisted Account.Id id) {
-    this(rsrc, userFactory.create(account.getId()));
+    this(rsrc, userFactory.create(id));
   }
 
   public IdentifiedUser getUser() {
     return user;
   }
+
+  /**
+   * @return the control for the reviewer's user (as opposed to the caller's
+   *     user as returned by {@link #getControl()}).
+   */
+  public ChangeControl getUserControl() {
+    return getControl().forUser(user);
+  }
 }

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

@@ -28,7 +28,6 @@ import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.AnonymousUser;
 import com.google.gerrit.server.CurrentUser;
 import com.google.gerrit.server.IdentifiedUser;
-import com.google.gerrit.server.account.AccountCache;
 import com.google.gerrit.server.account.AccountResolver;
 import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
@@ -40,23 +39,50 @@ public class Reviewers implements
     ChildCollection<ChangeResource, ReviewerResource> {
   private final DynamicMap<RestView<ReviewerResource>> views;
   private final Provider<ReviewDb> dbProvider;
-  private final AccountResolver resolver;
+  private final Parser parser;
   private final ReviewerResource.Factory resourceFactory;
-  private final AccountCache accountCache;
   private final Provider<ListReviewers> list;
 
+  static class Parser {
+    private final AccountResolver resolver;
+
+    @Inject
+    Parser(AccountResolver resolver) {
+      this.resolver = resolver;
+    }
+
+    Account.Id parse(ChangeResource rsrc, String id)
+        throws OrmException, AuthException {
+      if (id.equals("self")) {
+        CurrentUser user = rsrc.getControl().getCurrentUser();
+        if (user instanceof IdentifiedUser) {
+          return ((IdentifiedUser) user).getAccountId();
+        } else if (user instanceof AnonymousUser) {
+          throw new AuthException("Authentication required");
+        } else {
+          return null;
+        }
+      } else {
+        Set<Account.Id> matches = resolver.findAll(id);
+        if (matches.size() != 1) {
+          return null;
+        }
+        return Iterables.getOnlyElement(matches);
+      }
+    }
+  }
+
+
   @Inject
   Reviewers(Provider<ReviewDb> dbProvider,
-      AccountResolver resolver,
+      Parser parser,
       ReviewerResource.Factory resourceFactory,
       DynamicMap<RestView<ReviewerResource>> views,
-      AccountCache accountCache,
       Provider<ListReviewers> list) {
     this.dbProvider = dbProvider;
-    this.resolver = resolver;
+    this.parser = parser;
     this.resourceFactory = resourceFactory;
     this.views = views;
-    this.accountCache = accountCache;
     this.list = list;
   }
 
@@ -73,28 +99,10 @@ public class Reviewers implements
   @Override
   public ReviewerResource parse(ChangeResource rsrc, IdString id)
       throws OrmException, ResourceNotFoundException, AuthException {
-    Account.Id accountId;
+    Account.Id accountId = parser.parse(rsrc, id.get());
-    if (id.equals("self")) {
-      CurrentUser user = rsrc.getControl().getCurrentUser();
-      if (user instanceof IdentifiedUser) {
-        accountId = ((IdentifiedUser) user).getAccountId();
-      } else if (user instanceof AnonymousUser) {
-        throw new AuthException("Authentication required");
-      } else {
-        throw new ResourceNotFoundException(id);
-      }
-    } else {
-      Set<Account.Id> matches = resolver.findAll(id.get());
-      if (matches.size() != 1) {
-        throw new ResourceNotFoundException(id);
-      }
-      accountId = Iterables.getOnlyElement(matches);
-    }
-
     // See if the id exists as a reviewer for this change
-    if (fetchAccountIds(rsrc).contains(accountId)) {
+    if (accountId != null && fetchAccountIds(rsrc).contains(accountId)) {
-      Account account = accountCache.get(accountId).getAccount();
+      return resourceFactory.create(rsrc, accountId);
-      return resourceFactory.create(rsrc, account);
     }
     throw new ResourceNotFoundException(id);
   }

gerrit-server/src/main/resources/com/google/gerrit/server/change/ChangeMessages.properties

@@ -0,0 +1,5 @@
+# Changes to this file should also be made in
+# gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeMessages.properties
+groupIsNotAllowed =  The group {0} cannot be added as reviewer.
+groupHasTooManyMembers = The group {0} has too many members to add them all as reviewers.
+groupManyMembersConfirmation = The group {0} has {1} members. Do you want to add them all as reviewers?