Move CS1 to REST API for reviewer suggestions

This allows us to remove a lot of code from the deprecated
SuggestServiceImpl, much of which has been duplicated inside of
SuggestReviewers for CS2 and the REST API.

Change-Id: I589b327be626bc4b9ec8c7e7770e8ee1cad4f7df

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

@@ -14,7 +14,6 @@
 
 package com.google.gerrit.common.data;
 
-import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.Project;
 import com.google.gwtjsonrpc.common.AsyncCallback;
 import com.google.gwtjsonrpc.common.RemoteJsonService;
@@ -27,15 +26,4 @@ import java.util.List;
 public interface SuggestService extends RemoteJsonService {
   void suggestAccountGroupForProject(Project.NameKey project, String query,
       int limit, AsyncCallback<List<GroupReference>> callback);
-
-  /**
-   * Suggests reviewers. A reviewer can be a user or a group. Inactive users,
-   * the system groups {@code SystemGroupBackend#ANONYMOUS_USERS} and
-   * {@code SystemGroupBackend#REGISTERED_USERS} and groups that have more than
-   * the configured {@code addReviewer.maxAllowed} members are not suggested as
-   * reviewers.
-   * @param changeId the change for which reviewers should be suggested
-   */
-  void suggestChangeReviewer(Change.Id changeId, String query, int limit,
-      AsyncCallback<List<ReviewerInfo>> callback);
 }

gerrit-gwtui/src/main/java/com/google/gerrit/client/change/ReviewerSuggestOracle.java

@@ -31,8 +31,7 @@ import java.util.ArrayList;
 import java.util.List;
 
 /** REST API based suggestion Oracle for reviewers. */
-public class RestReviewerSuggestOracle extends SuggestAfterTypingNCharsOracle {
-
+public class ReviewerSuggestOracle extends SuggestAfterTypingNCharsOracle {
   private Change.Id changeId;
 
   @Override

gerrit-gwtui/src/main/java/com/google/gerrit/client/change/Reviewers.java

@@ -74,13 +74,13 @@ public class Reviewers extends Composite {
   private ChangeScreen2.Style style;
   private Element ccText;
 
-  private RestReviewerSuggestOracle reviewerSuggestOracle;
+  private ReviewerSuggestOracle reviewerSuggestOracle;
   private HintTextBox nameTxtBox;
   private Change.Id changeId;
   private boolean submitOnSelection;
 
   Reviewers() {
-    reviewerSuggestOracle = new RestReviewerSuggestOracle();
+    reviewerSuggestOracle = new ReviewerSuggestOracle();
     nameTxtBox = new HintTextBox();
     suggestBox = new SuggestBox(reviewerSuggestOracle, nameTxtBox);
     initWidget(uiBinder.createAndBindUi(this));

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

@@ -21,6 +21,7 @@ import com.google.gerrit.client.ConfirmationDialog;
 import com.google.gerrit.client.ErrorDialog;
 import com.google.gerrit.client.Gerrit;
 import com.google.gerrit.client.account.AccountInfo;
+import com.google.gerrit.client.change.ReviewerSuggestOracle;
 import com.google.gerrit.client.change.Reviewers.PostInput;
 import com.google.gerrit.client.change.Reviewers.PostResult;
 import com.google.gerrit.client.changes.ChangeInfo.ApprovalInfo;
@@ -29,7 +30,6 @@ import com.google.gerrit.client.rpc.GerritCallback;
 import com.google.gerrit.client.rpc.Natives;
 import com.google.gerrit.client.ui.AccountLinkPanel;
 import com.google.gerrit.client.ui.AddMemberBox;
-import com.google.gerrit.client.ui.ReviewerSuggestOracle;
 import com.google.gerrit.common.data.ApprovalDetail;
 import com.google.gerrit.common.data.SubmitRecord;
 import com.google.gerrit.reviewdb.client.Account;

gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/ReviewerSuggestOracle.java

@@ -1,85 +0,0 @@
-// Copyright (C) 2011 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.client.ui;
-
-import com.google.gerrit.client.FormatUtil;
-import com.google.gerrit.client.RpcStatus;
-import com.google.gerrit.client.admin.Util;
-import com.google.gerrit.client.rpc.GerritCallback;
-import com.google.gerrit.common.data.AccountInfo;
-import com.google.gerrit.common.data.ReviewerInfo;
-import com.google.gerrit.reviewdb.client.Change;
-import com.google.gwt.user.client.ui.SuggestOracle;
-
-import java.util.ArrayList;
-import java.util.List;
-
-/** Suggestion Oracle for reviewers. */
-public class ReviewerSuggestOracle extends SuggestAfterTypingNCharsOracle {
-
-  private Change.Id changeId;
-
-  @Override
-  protected void _onRequestSuggestions(final Request req, final Callback callback) {
-    RpcStatus.hide(new Runnable() {
-      @Override
-      public void run() {
-        SuggestUtil.SVC.suggestChangeReviewer(changeId, req.getQuery(),
-            req.getLimit(), new GerritCallback<List<ReviewerInfo>>() {
-              @Override
-              public void onSuccess(final List<ReviewerInfo> result) {
-                final List<ReviewerSuggestion> r =
-                    new ArrayList<>(result.size());
-                for (final ReviewerInfo reviewer : result) {
-                  r.add(new ReviewerSuggestion(reviewer));
-                }
-                callback.onSuggestionsReady(req, new Response(r));
-              }
-            });
-      }
-    });
-  }
-
-  public void setChange(Change.Id changeId) {
-    this.changeId = changeId;
-  }
-
-  private static class ReviewerSuggestion implements SuggestOracle.Suggestion {
-    private final ReviewerInfo reviewerInfo;
-
-    ReviewerSuggestion(final ReviewerInfo reviewerInfo) {
-      this.reviewerInfo = reviewerInfo;
-    }
-
-    @Override
-    public String getDisplayString() {
-      final AccountInfo accountInfo = reviewerInfo.getAccountInfo();
-      if (accountInfo != null) {
-        return FormatUtil.nameEmail(FormatUtil.asInfo(accountInfo));
-      }
-      return reviewerInfo.getGroup().getName() + " ("
-          + Util.C.suggestedGroupLabel() + ")";
-    }
-
-    @Override
-    public String getReplacementString() {
-      final AccountInfo accountInfo = reviewerInfo.getAccountInfo();
-      if (accountInfo != null) {
-        return FormatUtil.nameEmail(FormatUtil.asInfo(accountInfo));
-      }
-      return reviewerInfo.getGroup().getName();
-    }
-  }
-}

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

@@ -17,144 +17,34 @@ package com.google.gerrit.httpd.rpc;
 import com.google.common.collect.Iterables;
 import com.google.common.collect.Lists;
 import com.google.gerrit.common.Nullable;
-import com.google.gerrit.common.data.AccountInfo;
 import com.google.gerrit.common.data.GroupReference;
-import com.google.gerrit.common.data.ReviewerInfo;
 import com.google.gerrit.common.data.SuggestService;
-import com.google.gerrit.common.errors.NoSuchGroupException;
-import com.google.gerrit.reviewdb.client.Account;
-import com.google.gerrit.reviewdb.client.AccountExternalId;
-import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.Project;
 import com.google.gerrit.reviewdb.server.ReviewDb;
 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.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.project.ChangeControl;
-import com.google.gerrit.server.project.NoSuchChangeException;
 import com.google.gerrit.server.project.NoSuchProjectException;
 import com.google.gerrit.server.project.ProjectControl;
 import com.google.gwtjsonrpc.common.AsyncCallback;
-import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
 
-import org.eclipse.jgit.lib.Config;
-
-import java.io.IOException;
-import java.util.ArrayList;
 import java.util.Collections;
-import java.util.LinkedHashMap;
 import java.util.List;
-import java.util.Map;
-import java.util.Set;
 
 class SuggestServiceImpl extends BaseServiceImplementation implements
     SuggestService {
-  private static final String MAX_SUFFIX = "\u9fa5";
-
-  private final Provider<ReviewDb> reviewDbProvider;
-  private final AccountCache accountCache;
-  private final GroupMembers.Factory groupMembersFactory;
-  private final IdentifiedUser.GenericFactory identifiedUserFactory;
-  private final ChangeControl.Factory changeControlFactory;
   private final ProjectControl.Factory projectControlFactory;
-  private final Config cfg;
   private final GroupBackend groupBackend;
-  private final boolean suggestAccounts;
 
   @Inject
   SuggestServiceImpl(final Provider<ReviewDb> schema,
-      final AccountCache accountCache,
-      final GroupMembers.Factory groupMembersFactory,
       final Provider<CurrentUser> currentUser,
-      final IdentifiedUser.GenericFactory identifiedUserFactory,
-      final ChangeControl.Factory changeControlFactory,
       final ProjectControl.Factory projectControlFactory,
-      @GerritServerConfig final Config cfg, final GroupBackend groupBackend) {
+      final GroupBackend groupBackend) {
     super(schema, currentUser);
-    this.reviewDbProvider = schema;
-    this.accountCache = accountCache;
-    this.groupMembersFactory = groupMembersFactory;
-    this.identifiedUserFactory = identifiedUserFactory;
-    this.changeControlFactory = changeControlFactory;
     this.projectControlFactory = projectControlFactory;
-    this.cfg = cfg;
     this.groupBackend = groupBackend;
-
-    if ("OFF".equals(cfg.getString("suggest", null, "accounts"))) {
-      this.suggestAccounts = false;
-    } else {
-      boolean suggestAccounts;
-      try {
-        AccountVisibility av =
-            cfg.getEnum("suggest", null, "accounts", AccountVisibility.ALL);
-        suggestAccounts = (av != AccountVisibility.NONE);
-      } catch (IllegalArgumentException err) {
-        suggestAccounts = cfg.getBoolean("suggest", null, "accounts", true);
-      }
-      this.suggestAccounts = suggestAccounts;
-    }
-  }
-
-  private interface VisibilityControl {
-    boolean isVisible(Account account) throws OrmException;
-  }
-
-  private List<AccountInfo> suggestAccount(final ReviewDb db,
-      final String query, final Boolean active, final int limit,
-      VisibilityControl visibilityControl)
-      throws OrmException {
-    if (!suggestAccounts) {
-      return Collections.emptyList();
-    }
-
-    final String a = query;
-    final String b = a + MAX_SUFFIX;
-    final int max = 10;
-    final int n = limit <= 0 ? max : Math.min(limit, max);
-
-    LinkedHashMap<Account.Id, AccountInfo> r = new LinkedHashMap<>();
-    for (final Account p : db.accounts().suggestByFullName(a, b, n)) {
-      addSuggestion(r, p, new AccountInfo(p), active, visibilityControl);
-    }
-    if (r.size() < n) {
-      for (final Account p : db.accounts().suggestByPreferredEmail(a, b,
-          n - r.size())) {
-        addSuggestion(r, p, new AccountInfo(p), active, visibilityControl);
-      }
-    }
-    if (r.size() < n) {
-      for (final AccountExternalId e : db.accountExternalIds()
-          .suggestByEmailAddress(a, b, n - r.size())) {
-        if (!r.containsKey(e.getAccountId())) {
-          final Account p = accountCache.get(e.getAccountId()).getAccount();
-          final AccountInfo info = new AccountInfo(p);
-          info.setPreferredEmail(e.getEmailAddress());
-          addSuggestion(r, p, info, active, visibilityControl);
-        }
-      }
-    }
-    return new ArrayList<>(r.values());
-  }
-
-  private void addSuggestion(Map<Account.Id, AccountInfo> map, Account account,
-      AccountInfo info, Boolean active, VisibilityControl visibilityControl)
-      throws OrmException {
-    if (map.containsKey(account.getId())) {
-      return;
-    }
-    if (active != null && active != account.isActive()) {
-      return;
-    }
-    if (visibilityControl.isVisible(account)) {
-      map.put(account.getId(), info);
-    }
   }
 
   @Override
@@ -183,96 +73,4 @@ class SuggestServiceImpl extends BaseServiceImplementation implements
         groupBackend.suggest(query, projectControl),
         limit <= 0 ? 10 : Math.min(limit, 10)));
   }
-
-  @Override
-  public void suggestChangeReviewer(final Change.Id change,
-      final String query, final int limit,
-      final AsyncCallback<List<ReviewerInfo>> callback) {
-    run(callback, new Action<List<ReviewerInfo>>() {
-      @Override
-      public List<ReviewerInfo> run(final ReviewDb db)
-          throws OrmException, Failure {
-        final ChangeControl changeControl;
-        try {
-          changeControl = changeControlFactory.controlFor(change);
-        } catch (NoSuchChangeException e) {
-          return Collections.emptyList();
-        }
-
-        VisibilityControl visibilityControl;
-        if (changeControl.getRefControl().isVisibleByRegisteredUsers()) {
-          visibilityControl = new VisibilityControl() {
-            @Override
-            public boolean isVisible(Account account) throws OrmException {
-              return true;
-            }
-          };
-        } else {
-          visibilityControl = new VisibilityControl() {
-            @Override
-            public boolean isVisible(Account account) throws OrmException {
-              IdentifiedUser who =
-                  identifiedUserFactory.create(reviewDbProvider, account.getId());
-              // we can't use changeControl directly as it won't suggest reviewers
-              // to drafts
-              return changeControl.forUser(who).isRefVisible();
-            }
-          };
-        }
-
-        final List<AccountInfo> suggestedAccounts =
-            suggestAccount(db, query, Boolean.TRUE, limit, visibilityControl);
-        final List<ReviewerInfo> reviewer =
-            new ArrayList<>(suggestedAccounts.size());
-        for (final AccountInfo a : suggestedAccounts) {
-          reviewer.add(new ReviewerInfo(a));
-        }
-        final List<GroupReference> suggestedAccountGroups =
-            suggestAccountGroup(changeControl.getProjectControl(), query, limit);
-        for (final GroupReference g : suggestedAccountGroups) {
-          if (suggestGroupAsReviewer(changeControl.getProject().getNameKey(), g)) {
-            reviewer.add(new ReviewerInfo(g));
-          }
-        }
-
-        Collections.sort(reviewer);
-        if (reviewer.size() <= limit) {
-          return reviewer;
-        } else {
-          return reviewer.subList(0, limit);
-        }
-      }
-    });
-  }
-
-  private boolean suggestGroupAsReviewer(final Project.NameKey project,
-      final GroupReference group) throws OrmException, Failure {
-    if (!PostReviewers.isLegalReviewerGroup(group.getUUID())) {
-      return false;
-    }
-
-    try {
-      final Set<Account> members = groupMembersFactory.create(getCurrentUser())
-          .listAccounts(group.getUUID(), project);
-
-      if (members.isEmpty()) {
-        return false;
-      }
-
-      final int maxAllowed =
-          cfg.getInt("addreviewer", "maxAllowed",
-              PostReviewers.DEFAULT_MAX_REVIEWERS);
-      if (maxAllowed > 0 && members.size() > maxAllowed) {
-        return false;
-      }
-    } catch (NoSuchGroupException e) {
-      return false;
-    } catch (NoSuchProjectException e) {
-      return false;
-    } catch (IOException e) {
-      throw new Failure(e);
-    }
-
-    return true;
-  }
 }