diff --git a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AccountCreator.java b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AccountCreator.java index 36325021a5..bce0b5a430 100644 --- a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AccountCreator.java +++ b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AccountCreator.java @@ -27,6 +27,7 @@ import com.google.gerrit.server.account.AccountByEmailCache; import com.google.gerrit.server.account.AccountCache; import com.google.gerrit.server.account.GroupCache; import com.google.gerrit.server.account.VersionedAuthorizedKeys; +import com.google.gerrit.server.index.account.AccountIndexer; import com.google.gerrit.server.ssh.SshKeyCache; import com.google.gwtorm.server.SchemaFactory; import com.google.inject.Inject; @@ -52,6 +53,7 @@ public class AccountCreator { private final SshKeyCache sshKeyCache; private final AccountCache accountCache; private final AccountByEmailCache byEmailCache; + private final AccountIndexer indexer; @Inject AccountCreator(SchemaFactory schema, @@ -59,7 +61,8 @@ public class AccountCreator { GroupCache groupCache, SshKeyCache sshKeyCache, AccountCache accountCache, - AccountByEmailCache byEmailCache) { + AccountByEmailCache byEmailCache, + AccountIndexer indexer) { accounts = new HashMap<>(); reviewDbProvider = schema; this.authorizedKeys = authorizedKeys; @@ -67,6 +70,7 @@ public class AccountCreator { this.sshKeyCache = sshKeyCache; this.accountCache = accountCache; this.byEmailCache = byEmailCache; + this.indexer = indexer; } public synchronized TestAccount create(String username, String email, @@ -113,6 +117,8 @@ public class AccountCreator { accountCache.evictByUsername(username); byEmailCache.evict(email); + indexer.index(id); + account = new TestAccount(id, username, email, fullName, sshKey, httpPass); accounts.put(username, account); diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/SuggestReviewersIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/SuggestReviewersIT.java index 3eb46cffc7..29886194cf 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/SuggestReviewersIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/SuggestReviewersIT.java @@ -170,6 +170,15 @@ public class SuggestReviewersIT extends AbstractDaemonTest { @Test @GerritConfig(name = "suggest.fullTextSearch", value = "true") public void suggestReviewersFullTextSearch() throws Exception { + testSuggestReviewersFullTextSearch(); + } + + @Test + public void suggestReviewersFullTextSearchWithAccountIndex() throws Exception { + testSuggestReviewersFullTextSearch(); + } + + private void testSuggestReviewersFullTextSearch() throws Exception { String changeId = createChange().getChangeId(); List reviewers; diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/ReviewerSuggestOracle.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/ReviewerSuggestOracle.java index 1c0486df77..a852fa0b75 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/ReviewerSuggestOracle.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/ReviewerSuggestOracle.java @@ -14,13 +14,13 @@ package com.google.gerrit.client.change; -import com.google.gerrit.client.FormatUtil; import com.google.gerrit.client.admin.Util; import com.google.gerrit.client.changes.ChangeApi; import com.google.gerrit.client.groups.GroupBaseInfo; import com.google.gerrit.client.info.AccountInfo; import com.google.gerrit.client.rpc.GerritCallback; import com.google.gerrit.client.rpc.Natives; +import com.google.gerrit.client.ui.AccountSuggestOracle; import com.google.gerrit.client.ui.SuggestAfterTypingNCharsOracle; import com.google.gerrit.reviewdb.client.Change; import com.google.gwt.core.client.JavaScriptObject; @@ -42,7 +42,7 @@ public class ReviewerSuggestOracle extends SuggestAfterTypingNCharsOracle { public void onSuccess(JsArray result) { List r = new ArrayList<>(result.length()); for (SuggestReviewerInfo reviewer : Natives.asList(result)) { - r.add(new RestReviewerSuggestion(reviewer)); + r.add(new RestReviewerSuggestion(reviewer, req.getQuery())); } cb.onSuggestionsReady(req, new Response(r)); } @@ -60,29 +60,29 @@ public class ReviewerSuggestOracle extends SuggestAfterTypingNCharsOracle { } private static class RestReviewerSuggestion implements Suggestion { - private final SuggestReviewerInfo reviewer; + private final String displayString; + private final String replacementString; - RestReviewerSuggestion(final SuggestReviewerInfo reviewer) { - this.reviewer = reviewer; + RestReviewerSuggestion(SuggestReviewerInfo reviewer, String query) { + if (reviewer.account() != null) { + this.replacementString = AccountSuggestOracle.AccountSuggestion + .format(reviewer.account(), query); + this.displayString = replacementString; + } else { + this.replacementString = reviewer.group().name(); + this.displayString = + replacementString + " (" + Util.C.suggestedGroupLabel() + ")"; + } } @Override public String getDisplayString() { - if (reviewer.account() != null) { - return FormatUtil.nameEmail(reviewer.account()); - } - return reviewer.group().name() - + " (" - + Util.C.suggestedGroupLabel() - + ")"; + return displayString; } @Override public String getReplacementString() { - if (reviewer.account() != null) { - return FormatUtil.nameEmail(reviewer.account()); - } - return reviewer.group().name(); + return replacementString; } } diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/AccountSuggestOracle.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/AccountSuggestOracle.java index af8572b007..3702e68658 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/AccountSuggestOracle.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/AccountSuggestOracle.java @@ -42,25 +42,11 @@ public class AccountSuggestOracle extends SuggestAfterTypingNCharsOracle { }); } - private static class AccountSuggestion implements SuggestOracle.Suggestion { + public static class AccountSuggestion implements SuggestOracle.Suggestion { private final String suggestion; AccountSuggestion(AccountInfo info, String query) { - String s = FormatUtil.nameEmail(info); - if (!s.toLowerCase().contains(query.toLowerCase()) - && info.secondaryEmails() != null) { - for (String email : Natives.asList(info.secondaryEmails())) { - AccountInfo info2 = AccountInfo.create(info._accountId(), info.name(), - email, info.username()); - String s2 = FormatUtil.nameEmail(info2); - if (s2.toLowerCase().contains(query.toLowerCase())) { - s = s2; - break; - } - } - } - - this.suggestion = s; + this.suggestion = format(info, query); } @Override @@ -72,5 +58,30 @@ public class AccountSuggestOracle extends SuggestAfterTypingNCharsOracle { public String getReplacementString() { return suggestion; } + + public static String format(AccountInfo info, String query) { + String s = FormatUtil.nameEmail(info); + if (!containsQuery(s, query) && info.secondaryEmails() != null) { + for (String email : Natives.asList(info.secondaryEmails())) { + AccountInfo info2 = AccountInfo.create(info._accountId(), info.name(), + email, info.username()); + String s2 = FormatUtil.nameEmail(info2); + if (containsQuery(s2, query)) { + s = s2; + break; + } + } + } + return s; + } + + private static boolean containsQuery(String s, String query) { + for (String qterm : query.split("\\s+")) { + if (!s.toLowerCase().contains(qterm.toLowerCase())) { + return false; + } + } + return true; + } } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/ReviewersUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/ReviewersUtil.java index 2f98a0c8cd..7b1e963e1e 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/ReviewersUtil.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/ReviewersUtil.java @@ -17,6 +17,7 @@ package com.google.gerrit.server; import com.google.common.base.Function; import com.google.common.base.MoreObjects; import com.google.common.base.Strings; +import com.google.common.collect.ImmutableList; import com.google.common.collect.Iterables; import com.google.common.collect.Lists; import com.google.common.collect.Ordering; @@ -35,20 +36,30 @@ import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.server.account.AccountCache; import com.google.gerrit.server.account.AccountControl; import com.google.gerrit.server.account.AccountLoader; +import com.google.gerrit.server.account.AccountState; import com.google.gerrit.server.account.GroupBackend; import com.google.gerrit.server.account.GroupMembers; +import com.google.gerrit.server.account.AccountDirectory.FillOptions; import com.google.gerrit.server.change.PostReviewers; import com.google.gerrit.server.change.ReviewerSuggestionCache; import com.google.gerrit.server.change.SuggestReviewers; +import com.google.gerrit.server.index.account.AccountIndex; +import com.google.gerrit.server.index.account.AccountIndexCollection; import com.google.gerrit.server.project.NoSuchProjectException; import com.google.gerrit.server.project.ProjectControl; +import com.google.gerrit.server.query.QueryParseException; +import com.google.gerrit.server.query.QueryResult; +import com.google.gerrit.server.query.account.AccountQueryBuilder; +import com.google.gerrit.server.query.account.AccountQueryProcessor; import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; import com.google.inject.Provider; import java.io.IOException; import java.util.ArrayList; +import java.util.Collection; import java.util.Collections; +import java.util.EnumSet; import java.util.HashMap; import java.util.Iterator; import java.util.LinkedHashMap; @@ -74,6 +85,9 @@ public class ReviewersUtil { }); private final AccountLoader accountLoader; private final AccountCache accountCache; + private final AccountIndexCollection indexes; + private final AccountQueryBuilder queryBuilder; + private final AccountQueryProcessor queryProcessor; private final ReviewerSuggestionCache reviewerSuggestionCache; private final AccountControl accountControl; private final Provider dbProvider; @@ -84,14 +98,22 @@ public class ReviewersUtil { @Inject ReviewersUtil(AccountLoader.Factory accountLoaderFactory, AccountCache accountCache, + AccountIndexCollection indexes, + AccountQueryBuilder queryBuilder, + AccountQueryProcessor queryProcessor, ReviewerSuggestionCache reviewerSuggestionCache, AccountControl.Factory accountControlFactory, Provider dbProvider, GroupBackend groupBackend, GroupMembers.Factory groupMembersFactory, Provider currentUser) { - this.accountLoader = accountLoaderFactory.create(true); + Set fillOptions = EnumSet.of(FillOptions.SECONDARY_EMAILS); + fillOptions.addAll(AccountLoader.DETAILED_OPTIONS); + this.accountLoader = accountLoaderFactory.create(fillOptions); this.accountCache = accountCache; + this.indexes = indexes; + this.queryBuilder = queryBuilder; + this.queryProcessor = queryProcessor; this.reviewerSuggestionCache = reviewerSuggestionCache; this.accountControl = accountControlFactory.get(); this.dbProvider = dbProvider; @@ -122,11 +144,12 @@ public class ReviewersUtil { return Collections.emptyList(); } - List suggestedAccounts; + Collection suggestedAccounts; if (useFullTextSearch) { - suggestedAccounts = suggestAccountFullTextSearch(suggestReviewers, visibilityControl); + suggestedAccounts = + suggestAccountFullTextSearch(suggestReviewers, visibilityControl); } else { - suggestedAccounts = suggestAccount(suggestReviewers, visibilityControl); + suggestedAccounts = suggestAccounts(suggestReviewers, visibilityControl); } List reviewer = new ArrayList<>(); @@ -173,9 +196,39 @@ public class ReviewersUtil { return results; } - private List suggestAccount(SuggestReviewers suggestReviewers, + private Collection suggestAccounts(SuggestReviewers suggestReviewers, VisibilityControl visibilityControl) throws OrmException { + AccountIndex searchIndex = indexes.getSearchIndex(); + if (searchIndex != null) { + return suggestAccountsFromIndex(suggestReviewers); + } + return suggestAccountsFromDb(suggestReviewers, visibilityControl); + } + + private Collection suggestAccountsFromIndex( + SuggestReviewers suggestReviewers) throws OrmException { + try { + Map matches = new LinkedHashMap<>(); + QueryResult result = queryProcessor + .setLimit(suggestReviewers.getLimit()) + .query(queryBuilder.defaultQuery(suggestReviewers.getQuery())); + for (AccountState accountState : result.entities()) { + Account.Id id = accountState.getAccount().getId(); + matches.put(id, accountLoader.get(id)); + } + + accountLoader.fill(); + + return matches.values(); + } catch (QueryParseException e) { + return ImmutableList.of(); + } + } + + private Collection suggestAccountsFromDb( + SuggestReviewers suggestReviewers, VisibilityControl visibilityControl) + throws OrmException { String query = suggestReviewers.getQuery(); int limit = suggestReviewers.getLimit(); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/QueryAccounts.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/QueryAccounts.java index 9487c1f6bb..000637afd0 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/account/QueryAccounts.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/QueryAccounts.java @@ -197,7 +197,7 @@ public class QueryAccounts implements RestReadView { try { Predicate queryPred; if (suggest) { - queryPred = queryBuilder.defaultField(query); + queryPred = queryBuilder.defaultQuery(query); queryProcessor.setLimit(suggestLimit); } else { queryPred = queryBuilder.parse(query); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/account/AccountQueryBuilder.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/account/AccountQueryBuilder.java index d620fb85bf..e41b971ea1 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/query/account/AccountQueryBuilder.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/account/AccountQueryBuilder.java @@ -14,6 +14,8 @@ package com.google.gerrit.server.query.account; +import com.google.common.base.Function; +import com.google.common.base.Splitter; import com.google.common.collect.Lists; import com.google.common.primitives.Ints; import com.google.gerrit.common.errors.NotSignedInException; @@ -122,8 +124,19 @@ public class AccountQueryBuilder extends QueryBuilder { return AccountPredicates.username(username); } + public Predicate defaultQuery(String query) { + return Predicate.and( + Lists.transform(Splitter.on(' ').omitEmptyStrings().splitToList(query), + new Function>() { + @Override + public Predicate apply(String s) { + return defaultField(s); + } + })); + } + @Override - public Predicate defaultField(String query) { + protected Predicate defaultField(String query) { // Adapt the capacity of this list when adding more default predicates. List> preds = Lists.newArrayListWithCapacity(4); if ("self".equalsIgnoreCase(query)) { diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/query/account/AbstractQueryAccountsTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/query/account/AbstractQueryAccountsTest.java index 9f2971f399..f7b3b11d6a 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/query/account/AbstractQueryAccountsTest.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/query/account/AbstractQueryAccountsTest.java @@ -255,6 +255,8 @@ public abstract class AbstractQueryAccountsTest extends GerritServerTests { assertQuery("Doe", user1); assertQuery("doe", user1); assertQuery("DOE", user1); + assertQuery("Jo Do", user1); + assertQuery("jo do", user1); assertQuery("self", currentUserInfo, user3); assertQuery("name:John", user1); assertQuery("name:john", user1);