Remove old Add/RemoveReviewer handlers

Convert remaining internal callers to use the new REST API.

Change-Id: Ifd706f5b0402fd04b2bb0d862bf75315baa34347

gerrit-common/src/main/java/com/google/gerrit/common/data/PatchDetailService.java

@@ -28,7 +28,6 @@ import com.google.gwtjsonrpc.common.RpcImpl;
 import com.google.gwtjsonrpc.common.RpcImpl.Version;
 import com.google.gwtjsonrpc.common.VoidResult;
 
-import java.util.List;
 import java.util.Set;
 
 @RpcImpl(version = Version.V2_0)
@@ -63,16 +62,6 @@ public interface PatchDetailService extends RemoteJsonService {
   @SignInRequired
   void deleteDraftPatchSet(PatchSet.Id psid, AsyncCallback<ChangeDetail> callback);
 
-  @Audit
-  @SignInRequired
-  void addReviewers(Change.Id id, List<String> reviewers, boolean confirmed,
-      AsyncCallback<ReviewerResult> callback);
-
-  @Audit
-  @SignInRequired
-  void removeReviewer(Change.Id id, Account.Id reviewerId,
-      AsyncCallback<ReviewerResult> callback);
-
   void userApprovals(Set<Change.Id> cids, Account.Id aid,
       AsyncCallback<ApprovalSummarySet> callback);
 

gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/SuggestServiceImpl.java

@@ -33,8 +33,8 @@ import com.google.gerrit.server.account.AccountControl;
 import com.google.gerrit.server.account.AccountVisibility;
 import com.google.gerrit.server.account.GroupBackend;
 import com.google.gerrit.server.account.GroupMembers;
+import com.google.gerrit.server.change.PostReviewers;
 import com.google.gerrit.server.config.GerritServerConfig;
-import com.google.gerrit.server.patch.AddReviewer;
 import com.google.gerrit.server.project.ChangeControl;
 import com.google.gerrit.server.project.NoSuchChangeException;
 import com.google.gerrit.server.project.NoSuchProjectException;
@@ -273,7 +273,7 @@ class SuggestServiceImpl extends BaseServiceImplementation implements
 
   private boolean suggestGroupAsReviewer(final Project.NameKey project,
       final GroupReference group) throws OrmException {
-    if (!AddReviewer.isLegalReviewerGroup(group.getUUID())) {
+    if (!PostReviewers.isLegalReviewerGroup(group.getUUID())) {
       return false;
     }
 
@@ -287,7 +287,7 @@ class SuggestServiceImpl extends BaseServiceImplementation implements
 
       final int maxAllowed =
           cfg.getInt("addreviewer", "maxAllowed",
-              AddReviewer.DEFAULT_MAX_REVIEWERS);
+              PostReviewers.DEFAULT_MAX_REVIEWERS);
       if (maxAllowed > 0 && members.size() > maxAllowed) {
         return false;
       }

gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/patch/AddReviewerHandler.java

@@ -1,61 +0,0 @@
-// Copyright (C) 2009 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.httpd.rpc.patch;
-
-import com.google.gerrit.common.data.ReviewerResult;
-import com.google.gerrit.httpd.rpc.Handler;
-import com.google.gerrit.httpd.rpc.changedetail.ChangeDetailFactory;
-import com.google.gerrit.reviewdb.client.Change;
-import com.google.gerrit.server.patch.AddReviewer;
-import com.google.inject.Inject;
-import com.google.inject.assistedinject.Assisted;
-
-import java.util.Collection;
-
-class AddReviewerHandler extends Handler<ReviewerResult> {
-  interface Factory {
-    AddReviewerHandler create(Change.Id changeId, Collection<String> reviewers,
-        boolean confirmed);
-  }
-
-  private final AddReviewer.Factory addReviewerFactory;
-  private final ChangeDetailFactory.Factory changeDetailFactory;
-
-  private final Change.Id changeId;
-  private final Collection<String> reviewers;
-  private final boolean confirmed;
-
-  @Inject
-  AddReviewerHandler(final AddReviewer.Factory addReviewerFactory,
-      final ChangeDetailFactory.Factory changeDetailFactory,
-      @Assisted final Change.Id changeId,
-      @Assisted final Collection<String> reviewers,
-      @Assisted final boolean confirmed) {
-
-    this.addReviewerFactory = addReviewerFactory;
-    this.changeDetailFactory = changeDetailFactory;
-
-    this.changeId = changeId;
-    this.reviewers = reviewers;
-    this.confirmed = confirmed;
-  }
-
-  @Override
-  public ReviewerResult call() throws Exception {
-    ReviewerResult result = addReviewerFactory.create(changeId, reviewers, confirmed).call();
-    result.setChange(changeDetailFactory.create(changeId).call());
-    return result;
-  }
-}

gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/patch/PatchDetailServiceImpl.java

@@ -21,7 +21,6 @@ import com.google.gerrit.common.data.ChangeDetail;
 import com.google.gerrit.common.data.PatchDetailService;
 import com.google.gerrit.common.data.PatchScript;
 import com.google.gerrit.common.data.ReviewResult;
-import com.google.gerrit.common.data.ReviewerResult;
 import com.google.gerrit.common.errors.NoSuchEntityException;
 import com.google.gerrit.httpd.rpc.BaseServiceImplementation;
 import com.google.gerrit.httpd.rpc.changedetail.ChangeDetailFactory;
@@ -52,7 +51,6 @@ import org.eclipse.jgit.errors.RepositoryNotFoundException;
 import java.io.IOException;
 import java.util.Collections;
 import java.util.HashMap;
-import java.util.List;
 import java.util.Map;
 import java.util.Set;
 
@@ -61,10 +59,8 @@ class PatchDetailServiceImpl extends BaseServiceImplementation implements
   private final ApprovalTypes approvalTypes;
 
   private final AccountInfoCacheFactory.Factory accountInfoCacheFactory;
-  private final AddReviewerHandler.Factory addReviewerHandlerFactory;
   private final ChangeControl.Factory changeControlFactory;
   private final DeleteDraftPatchSet.Factory deleteDraftPatchSetFactory;
-  private final RemoveReviewerHandler.Factory removeReviewerHandlerFactory;
   private final FunctionState.Factory functionStateFactory;
   private final PatchScriptFactory.Factory patchScriptFactoryFactory;
   private final SaveDraft.Factory saveDraftFactory;
@@ -75,8 +71,6 @@ class PatchDetailServiceImpl extends BaseServiceImplementation implements
       final Provider<CurrentUser> currentUser,
       final ApprovalTypes approvalTypes,
       final AccountInfoCacheFactory.Factory accountInfoCacheFactory,
-      final AddReviewerHandler.Factory addReviewerHandlerFactory,
-      final RemoveReviewerHandler.Factory removeReviewerHandlerFactory,
       final ChangeControl.Factory changeControlFactory,
       final DeleteDraftPatchSet.Factory deleteDraftPatchSetFactory,
       final FunctionState.Factory functionStateFactory,
@@ -87,8 +81,6 @@ class PatchDetailServiceImpl extends BaseServiceImplementation implements
     this.approvalTypes = approvalTypes;
 
     this.accountInfoCacheFactory = accountInfoCacheFactory;
-    this.addReviewerHandlerFactory = addReviewerHandlerFactory;
-    this.removeReviewerHandlerFactory = removeReviewerHandlerFactory;
     this.changeControlFactory = changeControlFactory;
     this.deleteDraftPatchSetFactory = deleteDraftPatchSetFactory;
     this.functionStateFactory = functionStateFactory;
@@ -170,16 +162,6 @@ class PatchDetailServiceImpl extends BaseServiceImplementation implements
     });
   }
 
-  public void addReviewers(final Change.Id id, final List<String> reviewers,
-      final boolean confirmed, final AsyncCallback<ReviewerResult> callback) {
-    addReviewerHandlerFactory.create(id, reviewers, confirmed).to(callback);
-  }
-
-  public void removeReviewer(final Change.Id id, final Account.Id reviewerId,
-      final AsyncCallback<ReviewerResult> callback) {
-    removeReviewerHandlerFactory.create(id, reviewerId).to(callback);
-  }
-
   public void userApprovals(final Set<Change.Id> cids, final Account.Id aid,
       final AsyncCallback<ApprovalSummarySet> callback) {
     run(callback, new Action<ApprovalSummarySet>() {

gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/patch/PatchModule.java

@@ -28,8 +28,6 @@ public class PatchModule extends RpcServletModule {
     install(new FactoryModule() {
       @Override
       protected void configure() {
-        factory(AddReviewerHandler.Factory.class);
-        factory(RemoveReviewerHandler.Factory.class);
         factory(PatchScriptFactory.Factory.class);
         factory(SaveDraft.Factory.class);
       }

gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/patch/RemoveReviewerHandler.java

@@ -1,59 +0,0 @@
-// Copyright (C) 2010 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.httpd.rpc.patch;
-
-import com.google.gerrit.common.data.ReviewerResult;
-import com.google.gerrit.httpd.rpc.Handler;
-import com.google.gerrit.httpd.rpc.changedetail.ChangeDetailFactory;
-import com.google.gerrit.reviewdb.client.Account;
-import com.google.gerrit.reviewdb.client.Change;
-import com.google.gerrit.server.patch.RemoveReviewer;
-import com.google.inject.Inject;
-import com.google.inject.assistedinject.Assisted;
-
-import java.util.Collections;
-
-/**
- * Implement the remote logic that removes a reviewer from a change.
- */
-class RemoveReviewerHandler extends Handler<ReviewerResult> {
-  interface Factory {
-    RemoveReviewerHandler create(Change.Id changeId, Account.Id reviewerId);
-  }
-
-  private final RemoveReviewer.Factory removeReviewerFactory;
-  private final Account.Id reviewerId;
-  private final Change.Id changeId;
-  private final ChangeDetailFactory.Factory changeDetailFactory;
-
-  @Inject
-  RemoveReviewerHandler(final RemoveReviewer.Factory removeReviewerFactory,
-      final ChangeDetailFactory.Factory changeDetailFactory,
-      @Assisted Change.Id changeId, @Assisted Account.Id reviewerId) {
-    this.removeReviewerFactory = removeReviewerFactory;
-    this.changeId = changeId;
-    this.reviewerId = reviewerId;
-    this.changeDetailFactory = changeDetailFactory;
-  }
-
-  @Override
-  public ReviewerResult call() throws Exception {
-    ReviewerResult result = removeReviewerFactory.create(
-        changeId, Collections.singleton(reviewerId)).call();
-    result.setChange(changeDetailFactory.create(changeId).call());
-    return result;
-  }
-
-}

gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/project/ReviewProjectAccess.java

@@ -27,14 +27,17 @@ import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.ChangeUtil;
 import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.account.GroupBackend;
+import com.google.gerrit.server.change.ChangeResource;
+import com.google.gerrit.server.change.PostReviewers;
 import com.google.gerrit.server.git.GitRepositoryManager;
 import com.google.gerrit.server.git.MetaDataUpdate;
 import com.google.gerrit.server.git.ProjectConfig;
-import com.google.gerrit.server.patch.AddReviewer;
 import com.google.gerrit.server.patch.PatchSetInfoFactory;
+import com.google.gerrit.server.project.ChangeControl;
 import com.google.gerrit.server.project.ProjectControl;
 import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
+import com.google.inject.Provider;
 import com.google.inject.assistedinject.Assisted;
 
 import org.eclipse.jgit.lib.ObjectId;
@@ -58,14 +61,16 @@ public class ReviewProjectAccess extends ProjectAccessHandler<Change.Id> {
   private final ReviewDb db;
   private final IdentifiedUser user;
   private final PatchSetInfoFactory patchSetInfoFactory;
-  private final AddReviewer.Factory addReviewerFactory;
+  private final Provider<PostReviewers> reviewersProvider;
+  private final ChangeControl.GenericFactory changeFactory;
 
   @Inject
   ReviewProjectAccess(final ProjectControl.Factory projectControlFactory,
       final GroupBackend groupBackend,
       final MetaDataUpdate.User metaDataUpdateFactory, final ReviewDb db,
       final IdentifiedUser user, final PatchSetInfoFactory patchSetInfoFactory,
-      final AddReviewer.Factory addReviewerFactory,
+      final Provider<PostReviewers> reviewersProvider,
+      final ChangeControl.GenericFactory changeFactory,
 
       @Assisted final Project.NameKey projectName,
       @Nullable @Assisted final ObjectId base,
@@ -76,7 +81,8 @@ public class ReviewProjectAccess extends ProjectAccessHandler<Change.Id> {
     this.db = db;
     this.user = user;
     this.patchSetInfoFactory = patchSetInfoFactory;
-    this.addReviewerFactory = addReviewerFactory;
+    this.reviewersProvider = reviewersProvider;
+    this.changeFactory = changeFactory;
   }
 
   @Override
@@ -111,7 +117,7 @@ public class ReviewProjectAccess extends ProjectAccessHandler<Change.Id> {
       insertAncestors(ps.getId(), commit);
       db.patchSets().insert(Collections.singleton(ps));
       db.changes().insert(Collections.singleton(change));
-      addProjectOwnersAsReviewers(changeId);
+      addProjectOwnersAsReviewers(change);
       db.commit();
     } finally {
       db.rollback();
@@ -133,12 +139,15 @@ public class ReviewProjectAccess extends ProjectAccessHandler<Change.Id> {
     db.patchSetAncestors().insert(toInsert);
   }
 
-  private void addProjectOwnersAsReviewers(final Change.Id changeId) {
+  private void addProjectOwnersAsReviewers(final Change change) {
     final String projectOwners =
         groupBackend.get(AccountGroup.PROJECT_OWNERS).getName();
     try {
-      addReviewerFactory.create(changeId, Collections.singleton(projectOwners),
+      ChangeResource rsrc =
-          false).call();
+          new ChangeResource(changeFactory.controlFor(change, user));
+      PostReviewers.Input input = new PostReviewers.Input();
+      input.reviewer = projectOwners;
+      reviewersProvider.get().apply(rsrc, input);
     } catch (Exception e) {
       // one of the owner groups is not visible to the user and this it why it
       // can't be added as reviewer

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

@@ -32,8 +32,8 @@ import com.google.inject.Provider;
 
 import java.util.List;
 
-class DeleteReviewer implements RestModifyView<ReviewerResource, Input> {
+public class DeleteReviewer implements RestModifyView<ReviewerResource, Input> {
-  static class Input {
+  public static class Input {
   }
 
   private final Provider<ReviewDb> dbProvider;

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

@@ -57,13 +57,13 @@ import java.text.MessageFormat;
 import java.util.List;
 import java.util.Set;
 
-class PostReviewers implements RestModifyView<ChangeResource, Input> {
+public 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 {
+  public static class Input {
     @DefaultInput
-    String reviewer;
+    public String reviewer;
     Boolean confirmed;
 
     boolean confirmed() {

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

@@ -144,8 +144,8 @@ public class ReviewerJson {
   }
 
   public static class PostResult {
-    List<ReviewerInfo> reviewers;
+    public List<ReviewerInfo> reviewers;
+    public String error;
     Boolean confirm;
-    String error;
   }
 }

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

@@ -26,7 +26,7 @@ public class ReviewerResource extends ChangeResource {
   public static final TypeLiteral<RestView<ReviewerResource>> REVIEWER_KIND =
       new TypeLiteral<RestView<ReviewerResource>>() {};
 
-  static interface Factory {
+  public static interface Factory {
     ReviewerResource create(ChangeResource rsrc, IdentifiedUser user);
     ReviewerResource create(ChangeResource rsrc, Account.Id id);
   }

gerrit-server/src/main/java/com/google/gerrit/server/config/GerritRequestModule.java

@@ -26,7 +26,6 @@ import com.google.gerrit.server.git.BanCommit;
 import com.google.gerrit.server.git.MergeOp;
 import com.google.gerrit.server.git.MetaDataUpdate;
 import com.google.gerrit.server.git.SubmoduleOp;
-import com.google.gerrit.server.patch.AddReviewer;
 import com.google.gerrit.server.patch.RemoveReviewer;
 import com.google.gerrit.server.project.ChangeControl;
 import com.google.gerrit.server.project.CreateProject;
@@ -56,7 +55,6 @@ public class GerritRequestModule extends FactoryModule {
     // Not really per-request, but dammit, I don't know where else to
     // easily park this stuff.
     //
-    factory(AddReviewer.Factory.class);
     factory(DeleteDraftPatchSet.Factory.class);
     factory(PublishDraft.Factory.class);
     factory(RemoveReviewer.Factory.class);

gerrit-server/src/main/java/com/google/gerrit/server/patch/AddReviewer.java

@@ -1,273 +0,0 @@
-// Copyright (C) 2009 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.patch;
-
-import com.google.gerrit.common.ChangeHooks;
-import com.google.gerrit.common.data.ApprovalType;
-import com.google.gerrit.common.data.ApprovalTypes;
-import com.google.gerrit.common.data.ReviewerResult;
-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.AccountResolver;
-import com.google.gerrit.server.account.GroupCache;
-import com.google.gerrit.server.account.GroupMembers;
-import com.google.gerrit.server.config.GerritServerConfig;
-import com.google.gerrit.server.mail.AddReviewerSender;
-import com.google.gerrit.server.project.ChangeControl;
-import com.google.gwtorm.server.OrmException;
-import com.google.inject.Inject;
-import com.google.inject.assistedinject.Assisted;
-
-import org.eclipse.jgit.lib.Config;
-
-import java.util.ArrayList;
-import java.util.Collection;
-import java.util.HashSet;
-import java.util.List;
-import java.util.Set;
-import java.util.concurrent.Callable;
-
-public class AddReviewer implements Callable<ReviewerResult> {
-  public final static int DEFAULT_MAX_REVIEWERS_WITHOUT_CHECK = 10;
-  public final static int DEFAULT_MAX_REVIEWERS = 20;
-
-  public interface Factory {
-    AddReviewer create(Change.Id changeId,
-        Collection<String> userNameOrEmailOrGroupNames, boolean confirmed);
-  }
-
-  private final AddReviewerSender.Factory addReviewerSenderFactory;
-  private final AccountResolver accountResolver;
-  private final GroupCache groupCache;
-  private final GroupMembers.Factory groupMembersFactory;
-  private final ChangeControl.Factory changeControlFactory;
-  private final 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 Change.Id changeId;
-  private final Collection<String> reviewers;
-  private final boolean confirmed;
-
-  @Inject
-  AddReviewer(final AddReviewerSender.Factory addReviewerSenderFactory,
-      final AccountResolver accountResolver, final GroupCache groupCache,
-      final GroupMembers.Factory groupMembersFactory,
-      final ChangeControl.Factory changeControlFactory, final ReviewDb db,
-      final IdentifiedUser.GenericFactory identifiedUserFactory,
-      final IdentifiedUser currentUser, final ApprovalTypes approvalTypes,
-      final @GerritServerConfig Config cfg, final ChangeHooks hooks,
-      final AccountCache accountCache,
-      @Assisted final Change.Id changeId,
-      @Assisted final Collection<String> reviewers,
-      @Assisted final boolean confirmed) {
-    this.addReviewerSenderFactory = addReviewerSenderFactory;
-    this.accountResolver = accountResolver;
-    this.groupCache = groupCache;
-    this.groupMembersFactory = groupMembersFactory;
-    this.db = db;
-    this.changeControlFactory = changeControlFactory;
-    this.identifiedUserFactory = identifiedUserFactory;
-    this.currentUser = currentUser;
-    this.cfg = cfg;
-    this.hooks = hooks;
-    this.accountCache = accountCache;
-
-    final List<ApprovalType> allTypes = approvalTypes.getApprovalTypes();
-    addReviewerCategoryId =
-        allTypes.get(allTypes.size() - 1).getCategory().getId();
-
-    this.changeId = changeId;
-    this.reviewers = reviewers;
-    this.confirmed = confirmed;
-  }
-
-  @Override
-  public ReviewerResult call() throws Exception {
-    final Set<Account.Id> reviewerIds = new HashSet<Account.Id>();
-    final ChangeControl control = changeControlFactory.validateFor(changeId);
-
-    final ReviewerResult result = new ReviewerResult();
-    for (final String reviewer : reviewers) {
-      final Account account = accountResolver.find(reviewer);
-      if (account == null) {
-        AccountGroup group = groupCache.get(new AccountGroup.NameKey(reviewer));
-
-        if (group == null) {
-          result.addError(new ReviewerResult.Error(
-              ReviewerResult.Error.Type.REVIEWER_NOT_FOUND, reviewer));
-          continue;
-        }
-
-        if (!isLegalReviewerGroup(group.getGroupUUID())) {
-          result.addError(new ReviewerResult.Error(
-              ReviewerResult.Error.Type.GROUP_NOT_ALLOWED, reviewer));
-          continue;
-        }
-
-        final Set<Account> members = groupMembersFactory.create(currentUser)
-            .listAccounts(group.getGroupUUID(),
-                control.getProject().getNameKey());
-        if (members == null || members.size() == 0) {
-          result.addError(new ReviewerResult.Error(
-              ReviewerResult.Error.Type.GROUP_EMPTY, reviewer));
-          continue;
-        }
-
-        // if maxAllowed is set to 0, it is allowed to add any number of
-        // reviewers
-        final int maxAllowed =
-            cfg.getInt("addreviewer", "maxAllowed", DEFAULT_MAX_REVIEWERS);
-        if (maxAllowed > 0 && members.size() > maxAllowed) {
-          result.setMemberCount(members.size());
-          result.setAskForConfirmation(false);
-          result.addError(new ReviewerResult.Error(
-              ReviewerResult.Error.Type.GROUP_HAS_TOO_MANY_MEMBERS, reviewer));
-          continue;
-        }
-
-        // if maxWithoutCheck is set to 0, we never ask for confirmation
-        final int maxWithoutConfirmation =
-            cfg.getInt("addreviewer", "maxWithoutConfirmation",
-                DEFAULT_MAX_REVIEWERS_WITHOUT_CHECK);
-        if (!confirmed && maxWithoutConfirmation > 0
-            && members.size() > maxWithoutConfirmation) {
-          result.setMemberCount(members.size());
-          result.setAskForConfirmation(true);
-          result.addError(new ReviewerResult.Error(
-              ReviewerResult.Error.Type.GROUP_HAS_TOO_MANY_MEMBERS, reviewer));
-          continue;
-        }
-
-        for (final Account member : members) {
-          if (member.isActive()) {
-            final 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()) {
-              reviewerIds.add(member.getId());
-            }
-          }
-        }
-        continue;
-      }
-
-      if (!account.isActive()) {
-        result.addError(new ReviewerResult.Error(
-            ReviewerResult.Error.Type.ACCOUNT_INACTIVE,
-            formatUser(account, reviewer)));
-        continue;
-      }
-
-      final IdentifiedUser user = identifiedUserFactory.create(account.getId());
-      // Does not account for draft status as a user might want to let a
-      // reviewer see a draft.
-      if (!control.forUser(user).isRefVisible()) {
-        result.addError(new ReviewerResult.Error(
-            ReviewerResult.Error.Type.CHANGE_NOT_VISIBLE,
-            formatUser(account, reviewer)));
-        continue;
-      }
-
-      reviewerIds.add(account.getId());
-    }
-
-    if (reviewerIds.isEmpty()) {
-      return result;
-    }
-
-    // Add the reviewers to the database
-    //
-    final Set<Account.Id> added = new HashSet<Account.Id>();
-    final List<PatchSetApproval> toInsert = new ArrayList<PatchSetApproval>();
-    final Change change = control.getChange();
-    final PatchSet.Id psid = change.currentPatchSetId();
-    for (final Account.Id reviewer : reviewerIds) {
-      if (!exists(psid, reviewer)) {
-        // This reviewer has not entered an approval for this change yet.
-        //
-        final PatchSetApproval myca =
-            dummyApproval(control.getChange(), psid, reviewer);
-        toInsert.add(myca);
-        added.add(reviewer);
-      }
-    }
-    db.patchSetApprovals().insert(toInsert);
-
-    // Execute hook for added reviewers
-    //
-    final PatchSet patchSet = db.patchSets().get(psid);
-    for (final Account.Id id : added) {
-      final Account account = accountCache.get(id).getAccount();
-      hooks.doReviewerAddedHook(change, account, patchSet, db);
-    }
-
-    // Email the reviewers
-    //
-    // The user knows they added themselves, don't bother emailing them.
-    added.remove(currentUser.getAccountId());
-    if (!added.isEmpty()) {
-      final AddReviewerSender cm;
-
-      cm = addReviewerSenderFactory.create(control.getChange());
-      cm.setFrom(currentUser.getAccountId());
-      cm.addReviewers(added);
-      cm.send();
-    }
-
-    return result;
-  }
-
-  private String formatUser(Account account, String nameOrEmail) {
-    if (nameOrEmail.matches("^[1-9][0-9]*$")) {
-      return RemoveReviewer.formatUser(account, nameOrEmail);
-    } else {
-      return nameOrEmail;
-    }
-  }
-
-  private boolean exists(final PatchSet.Id patchSetId,
-      final Account.Id reviewerId) throws OrmException {
-    return db.patchSetApprovals().byPatchSetUser(patchSetId, reviewerId)
-        .iterator().hasNext();
-  }
-
-  private PatchSetApproval dummyApproval(final Change change,
-      final PatchSet.Id patchSetId, final Account.Id reviewerId) {
-    final PatchSetApproval dummyApproval =
-        new PatchSetApproval(new PatchSetApproval.Key(patchSetId, reviewerId,
-            addReviewerCategoryId), (short) 0);
-    dummyApproval.cache(change);
-    return dummyApproval;
-  }
-
-  public static boolean isLegalReviewerGroup(final AccountGroup.UUID groupUUID) {
-    return !(AccountGroup.ANONYMOUS_USERS.equals(groupUUID)
-             || AccountGroup.REGISTERED_USERS.equals(groupUUID));
-  }
-}

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

@@ -14,14 +14,16 @@
 
 package com.google.gerrit.sshd.commands;
 
-import com.google.gerrit.common.data.ReviewerResult;
+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.PatchSet;
 import com.google.gerrit.reviewdb.client.RevId;
 import com.google.gerrit.reviewdb.server.ReviewDb;
-import com.google.gerrit.server.patch.AddReviewer;
+import com.google.gerrit.server.change.ChangeResource;
-import com.google.gerrit.server.patch.RemoveReviewer;
+import com.google.gerrit.server.change.DeleteReviewer;
+import com.google.gerrit.server.change.PostReviewers;
+import com.google.gerrit.server.change.ReviewerResource;
 import com.google.gerrit.server.project.ChangeControl;
 import com.google.gerrit.server.project.NoSuchChangeException;
 import com.google.gerrit.server.project.ProjectControl;
@@ -30,6 +32,7 @@ import com.google.gerrit.sshd.SshCommand;
 import com.google.gwtorm.server.OrmException;
 import com.google.gwtorm.server.ResultSet;
 import com.google.inject.Inject;
+import com.google.inject.Provider;
 
 import org.kohsuke.args4j.Argument;
 import org.kohsuke.args4j.Option;
@@ -37,7 +40,6 @@ import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import java.io.IOException;
-import java.text.MessageFormat;
 import java.util.ArrayList;
 import java.util.HashSet;
 import java.util.List;
@@ -74,10 +76,13 @@ public class SetReviewersCommand extends SshCommand {
   private ReviewDb db;
 
   @Inject
-  private AddReviewer.Factory addReviewerFactory;
+  private ReviewerResource.Factory reviewerFactory;
 
   @Inject
-  private RemoveReviewer.Factory removeReviewerFactory;
+  private Provider<PostReviewers> postReviewersProvider;
+
+  @Inject
+  private Provider<DeleteReviewer> deleteReviewerProvider;
 
   @Inject
   private ChangeControl.Factory changeControlFactory;
@@ -104,62 +109,48 @@ public class SetReviewersCommand extends SshCommand {
   }
 
   private boolean modifyOne(Change.Id changeId) throws Exception {
-    changeControlFactory.validateFor(changeId);
+    ChangeResource changeRsrc =
-
+        new ChangeResource(changeControlFactory.validateFor(changeId));
-    ReviewerResult result;
     boolean ok = true;
 
     // Remove reviewers
     //
-    result = removeReviewerFactory.create(changeId, toRemove).call();
+    DeleteReviewer delete = deleteReviewerProvider.get();
-    ok &= result.getErrors().isEmpty();
+    for (Account.Id reviewer : toRemove) {
-    for (ReviewerResult.Error resultError : result.getErrors()) {
+      ReviewerResource rsrc = reviewerFactory.create(changeRsrc, reviewer);
-      String message;
+      String error = null;;
-      switch (resultError.getType()) {
+      try {
-        case REMOVE_NOT_PERMITTED:
+        delete.apply(rsrc, new DeleteReviewer.Input());
-          message = "not permitted to remove {0} from {1}";
+      } catch (ResourceNotFoundException e) {
-          break;
+        error = String.format("could not remove %s: not found", reviewer);
-        case COULD_NOT_REMOVE:
+      } catch (Exception e) {
-          message = "could not remove {0} from {1}";
+        error = String.format("could not remove %s: %s",
-          break;
+            reviewer, e.getMessage());
-        default:
+      }
-          message = "could not remove {0}: {2}";
+      if (error != null) {
+        ok = false;
+        writeError("error", error);
       }
-      writeError("error", MessageFormat.format(message,
-          resultError.getName(), changeId, resultError.getType()));
     }
 
     // Add reviewers
     //
-    result =
+    PostReviewers post = postReviewersProvider.get();
-        addReviewerFactory.create(changeId, toAdd, true).call();
+    for (String reviewer : toAdd) {
-    ok &= result.getErrors().isEmpty();
+      PostReviewers.Input input = new PostReviewers.Input();
-    for (ReviewerResult.Error resultError : result.getErrors()) {
+      input.reviewer = reviewer;
-      String message;
+      String error;
-      switch (resultError.getType()) {
+      try {
-        case REVIEWER_NOT_FOUND:
+        error = post.apply(changeRsrc, input).error;
-          message = "account or group {0} not found";
+      } catch (ResourceNotFoundException e) {
-          break;
+        error = String.format("could not add %s: not found", reviewer);
-        case ACCOUNT_INACTIVE:
+      } catch (Exception e) {
-          message = "account {0} inactive";
+        error = String.format("could not add %s: %s", reviewer, e.getMessage());
-          break;
+      }
-        case CHANGE_NOT_VISIBLE:
+      if (error != null) {
-          message = "change {1} not visible to {0}";
+        ok = false;
-          break;
+        writeError("error", error);
-        case GROUP_EMPTY:
-          message = "group {0} is empty";
-          break;
-        case GROUP_HAS_TOO_MANY_MEMBERS:
-          message = "group {0} has too many members";
-          break;
-        case GROUP_NOT_ALLOWED:
-          message = "group {0} is not allowed as reviewer";
-          break;
-        default:
-          message = "could not add {0}: {2}";
       }
-      writeError("error", MessageFormat.format(message,
-          resultError.getName(), changeId, resultError.getType()));
     }
 
     return ok;