Full text search in memory for review suggestions

This change also makes it possible to configure maximum
displayed reviewers.

On some Gerrit instances the full name is formatted like:
<given name> <surname>
and email like:
<given name>.<surname>@...
This would make it impossible to get reviewer suggestions from
surnames.
Since gwtorm doesn't support sql LIKE there is no straight forward
way of filtering on substring in the DB. Hence this in memory approach.

For performance reasons this implementation differs from the default
implementation in that it does not look at the email_address in
account_external_ids but only at the preferred_email of accounts.
The default implementation does only look for 10 matches and afterwards
filters out the acconts that are not allowed to view the change.

Configuration:
suggest.maxSuggestedReviewers
Maximum number of suggested reviewers (default 10).

suggest.fullTextSearch
Enable full text search (default "false").

suggest.fullTextSearchMaxMatches
Maximum number of matches to be checked for accessability when using
full text search (default 100).

Change-Id: Ia4c3a15263783bc144e66a05854c3915392095b5

Documentation/config-gerrit.txt

@@ -3110,6 +3110,16 @@ otherwise, this value applies to both `suggest.accounts` and
 New configurations should prefer the boolean value for this field
 and an enum value for `accounts.visibility`.
 
+[[suggest.maxSuggestedReviewers]]suggest.maxSuggestedReviewers::
++
+The maximum numbers of reviewers suggested.
++
+By default 10.
+
+[[suggest.fullTextSearch]]suggest.fullTextSearch::
++
+If 'true' the reviewer completion suggestions will be based on a full text search.
+
 [[suggest.from]]suggest.from::
 +
 The number of characters that a user must have typed before suggestions
@@ -3117,6 +3127,15 @@ are provided. If set to 0, suggestions are always provided.
 +
 By default 0.
 
+[[suggest.fullTextSearchMaxMatches]]suggest.fullTextSearchMaxMatches::
++
+The maximum number of matches evaluated for change access when using full text search.
++
+Making this number too high could have a negative impact on performance.
++
+By default 100.
+
+
 [[theme]]
 === Section theme
 

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

@@ -161,6 +161,36 @@ public class SuggestReviewersIT extends AbstractDaemonTest {
     assertEquals("User2", Iterables.getOnlyElement(reviewers).account.name);
   }
 
+  @Test
+  @GerritConfig(name = "suggest.maxSuggestedReviewers", value = "2")
+  public void suggestReviewersMaxNbrSuggestions() throws Exception {
+    String changeId = createChange().getChangeId();
+    List<SuggestedReviewerInfo> reviewers =
+        suggestReviewers(changeId, "user", 5);
+    assertEquals(2, reviewers.size());
+  }
+
+  @Test
+  @GerritConfig(name = "suggest.fullTextSearch", value = "true")
+  public void suggestReviewersFullTextSearch() throws Exception {
+    String changeId = createChange().getChangeId();
+    List<SuggestedReviewerInfo> reviewers =
+        suggestReviewers(changeId, "ser", 5);
+    assertEquals(4, reviewers.size());
+  }
+
+  @Test
+  @GerritConfigs(
+      {@GerritConfig(name = "suggest.fulltextsearch", value = "true"),
+       @GerritConfig(name = "suggest.fullTextSearchMaxMatches", value = "2")
+  })
+  public void suggestReviewersFullTextSearchLimitMaxMatches() throws Exception {
+    String changeId = createChange().getChangeId();
+    List<SuggestedReviewerInfo> reviewers =
+        suggestReviewers(changeId, "ser", 3);
+    assertEquals(2, reviewers.size());
+  }
+
   private List<SuggestedReviewerInfo> suggestReviewers(RestSession session,
       String changeId, String query, int n) throws IOException {
     return newGson().fromJson(

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

@@ -0,0 +1,69 @@
+// Copyright (C) 2014 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.cache.CacheBuilder;
+import com.google.common.cache.CacheLoader;
+import com.google.common.cache.LoadingCache;
+import com.google.common.collect.ImmutableList;
+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.reviewdb.server.ReviewDb;
+import com.google.gwtorm.server.SchemaFactory;
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+import com.google.inject.Singleton;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.Collections;
+import java.util.List;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.TimeUnit;
+/**
+ * The suggest oracle may be called many times in rapid succession during the course of one operation.
+ * It would be easy to have a simple Cache<Boolean, List<Account>> with a short expiration time of 30s.
+ * Cache only has a single key we're just using Cache for the expiration behavior.
+ */
+@Singleton
+public class ReviewerSuggestionCache {
+  private static final Logger log = LoggerFactory
+      .getLogger(ReviewerSuggestionCache.class);
+  private final LoadingCache<Boolean, List<Account>> cache;
+
+  @Inject
+  ReviewerSuggestionCache(final Provider<ReviewDb> dbProvider) {
+    this.cache =
+        CacheBuilder.newBuilder().maximumSize(1)
+            .expireAfterWrite(30, TimeUnit.SECONDS)
+            .build(new CacheLoader<Boolean, List<Account>>() {
+              @Override
+              public List<Account> load(Boolean key) throws Exception {
+                return ImmutableList.copyOf(dbProvider.get().accounts().all());
+              }
+            });
+  }
+
+  List<Account> get() {
+    try {
+      return cache.get(true);
+    } catch (ExecutionException e) {
+      log.warn("Cannot fetch reviewers from cache", e);
+      return Collections.emptyList();
+    }
+  }
+}

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

@@ -57,7 +57,8 @@ import java.util.Set;
 public class SuggestReviewers implements RestReadView<ChangeResource> {
 
   private static final String MAX_SUFFIX = "\u9fa5";
-  private static final int MAX = 10;
+  private static final int DEFAULT_MAX_SUGGESTED = 10;
+  private static final int DEFAULT_MAX_MATCHES = 100;
 
   private final AccountInfo.Loader.Factory accountLoaderFactory;
   private final AccountControl.Factory accountControlFactory;
@@ -72,11 +73,17 @@ public class SuggestReviewers implements RestReadView<ChangeResource> {
   private final int maxAllowed;
   private int limit;
   private String query;
+  private boolean useFullTextSearch;
+  private final int fullTextMaxMatches;
+  private final int maxSuggestedReviewers;
+  private final ReviewerSuggestionCache reviewerSuggestionCache;
 
   @Option(name = "--limit", aliases = {"-n"}, metaVar = "CNT",
       usage = "maximum number of reviewers to list")
   public void setLimit(int l) {
-    this.limit = l <= 0 ? MAX : Math.min(l, MAX);
+    this.limit =
+        l <= 0 ? maxSuggestedReviewers : Math.min(l,
+            maxSuggestedReviewers);
   }
 
   @Option(name = "--query", aliases = {"-q"}, metaVar = "QUERY",
@@ -95,7 +102,8 @@ public class SuggestReviewers implements RestReadView<ChangeResource> {
       Provider<CurrentUser> currentUser,
       Provider<ReviewDb> dbProvider,
       @GerritServerConfig Config cfg,
-      GroupBackend groupBackend) {
+      GroupBackend groupBackend,
+      ReviewerSuggestionCache reviewerSuggestionCache) {
     this.accountLoaderFactory = accountLoaderFactory;
     this.accountControlFactory = accountControlFactory;
     this.accountCache = accountCache;
@@ -104,12 +112,18 @@ public class SuggestReviewers implements RestReadView<ChangeResource> {
     this.identifiedUserFactory = identifiedUserFactory;
     this.currentUser = currentUser;
     this.groupBackend = groupBackend;
-
+    this.reviewerSuggestionCache = reviewerSuggestionCache;
+    this.maxSuggestedReviewers =
+        cfg.getInt("suggest", "maxSuggestedReviewers", DEFAULT_MAX_SUGGESTED);
+    this.fullTextMaxMatches =
+        cfg.getInt("suggest", "fullTextSearchMaxMatches",
+            DEFAULT_MAX_MATCHES);
     String suggest = cfg.getString("suggest", null, "accounts");
     if ("OFF".equalsIgnoreCase(suggest)
         || "false".equalsIgnoreCase(suggest)) {
       this.suggestAccounts = false;
     } else {
+      this.useFullTextSearch = cfg.getBoolean("suggest", "fullTextSearch", false);
       this.suggestAccounts = (av != AccountVisibility.NONE);
     }
 
@@ -134,7 +148,12 @@ public class SuggestReviewers implements RestReadView<ChangeResource> {
     }
 
     VisibilityControl visibilityControl = getVisibility(rsrc);
-    List<AccountInfo> suggestedAccounts = suggestAccount(visibilityControl);
+    List<AccountInfo> suggestedAccounts;
+    if (useFullTextSearch) {
+      suggestedAccounts = suggestAccountFullTextSearch(visibilityControl);
+    } else {
+      suggestedAccounts = suggestAccount(visibilityControl);
+    }
     accountLoaderFactory.create(true).fill(suggestedAccounts);
 
     List<SuggestedReviewerInfo> reviewer = Lists.newArrayList();
@@ -220,6 +239,42 @@ public class SuggestReviewers implements RestReadView<ChangeResource> {
     return Lists.newArrayList(r.values());
   }
 
+  private List<AccountInfo> suggestAccountFullTextSearch(
+      VisibilityControl visibilityControl) throws OrmException {
+    String str = query.toLowerCase();
+    LinkedHashMap<Account.Id, AccountInfo> accountMap = Maps.newLinkedHashMap();
+    List<Account> fullNameMatches = Lists.newArrayListWithCapacity(fullTextMaxMatches);
+    List<Account> emailMatches = Lists.newArrayListWithCapacity(fullTextMaxMatches);
+    for (Account a : reviewerSuggestionCache.get()) {
+      if (a.getFullName() != null
+          && a.getFullName().toLowerCase().contains(str)) {
+        fullNameMatches.add(a);
+      } else if (a.getPreferredEmail() != null
+          && emailMatches.size() < fullTextMaxMatches
+          && a.getPreferredEmail().toLowerCase().contains(str)) {
+        emailMatches.add(a);
+      }
+      if (fullNameMatches.size() >= fullTextMaxMatches) {
+        break;
+      }
+    }
+    for (Account a : fullNameMatches) {
+      addSuggestion(accountMap, a, new AccountInfo(a.getId()), visibilityControl);
+      if (accountMap.size() >= limit) {
+        break;
+      }
+    }
+    if (accountMap.size() < limit) {
+      for (Account a : emailMatches) {
+        addSuggestion(accountMap, a, new AccountInfo(a.getId()), visibilityControl);
+        if (accountMap.size() >= limit) {
+          break;
+        }
+      }
+    }
+    return Lists.newArrayList(accountMap.values());
+  }
+
   private void addSuggestion(Map<Account.Id, AccountInfo> map, Account account,
       AccountInfo info, VisibilityControl visibilityControl)
       throws OrmException {