opendev
/
gerrit
Code Issues Proposed changes

Use account index for reviewer suggestion 

Change-Id: Ia7777ac593723272e44705d064fae6d9446b40fb
Signed-off-by: Edwin Kempin <ekempin@google.com>
This commit is contained in:
Edwin Kempin 2016-07-01 14:41:35 +02:00
parent cdfd4d3f44
commit e2de9ccdc2
8 changed files with 134 additions and 40 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AccountCreator.java
View File

@ -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<ReviewDb> 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);

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

@ -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<SuggestedReviewerInfo> reviewers;

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

@ -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<SuggestReviewerInfo> result) {
List<RestReviewerSuggestion> 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;
}
}

43
gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/AccountSuggestOracle.java
View File

@ -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;
}
}
}

63
gerrit-server/src/main/java/com/google/gerrit/server/ReviewersUtil.java
View File

@ -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<ReviewDb> 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<ReviewDb> dbProvider,
GroupBackend groupBackend,
GroupMembers.Factory groupMembersFactory,
Provider<CurrentUser> currentUser) {
this.accountLoader = accountLoaderFactory.create(true);
Set<FillOptions> 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<AccountInfo> suggestedAccounts;
Collection<AccountInfo> suggestedAccounts;
if (useFullTextSearch) {
suggestedAccounts = suggestAccountFullTextSearch(suggestReviewers, visibilityControl);
suggestedAccounts =
suggestAccountFullTextSearch(suggestReviewers, visibilityControl);
} else {
suggestedAccounts = suggestAccount(suggestReviewers, visibilityControl);
suggestedAccounts = suggestAccounts(suggestReviewers, visibilityControl);
}
List<SuggestedReviewerInfo> reviewer = new ArrayList<>();
@ -173,9 +196,39 @@ public class ReviewersUtil {
return results;
}
private List<AccountInfo> suggestAccount(SuggestReviewers suggestReviewers,
private Collection<AccountInfo> suggestAccounts(SuggestReviewers suggestReviewers,
VisibilityControl visibilityControl)
throws OrmException {
AccountIndex searchIndex = indexes.getSearchIndex();
if (searchIndex != null) {
return suggestAccountsFromIndex(suggestReviewers);
}
return suggestAccountsFromDb(suggestReviewers, visibilityControl);
}
private Collection<AccountInfo> suggestAccountsFromIndex(
SuggestReviewers suggestReviewers) throws OrmException {
try {
Map<Account.Id, AccountInfo> matches = new LinkedHashMap<>();
QueryResult<AccountState> 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<AccountInfo> suggestAccountsFromDb(
SuggestReviewers suggestReviewers, VisibilityControl visibilityControl)
throws OrmException {
String query = suggestReviewers.getQuery();
int limit = suggestReviewers.getLimit();

2
gerrit-server/src/main/java/com/google/gerrit/server/account/QueryAccounts.java
View File

@ -197,7 +197,7 @@ public class QueryAccounts implements RestReadView<TopLevelResource> {
try {
Predicate<AccountState> queryPred;
if (suggest) {
queryPred = queryBuilder.defaultField(query);
queryPred = queryBuilder.defaultQuery(query);
queryProcessor.setLimit(suggestLimit);
} else {
queryPred = queryBuilder.parse(query);

15
gerrit-server/src/main/java/com/google/gerrit/server/query/account/AccountQueryBuilder.java
View File

@ -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<AccountState> {
return AccountPredicates.username(username);
}
public Predicate<AccountState> defaultQuery(String query) {
return Predicate.and(
Lists.transform(Splitter.on(' ').omitEmptyStrings().splitToList(query),
new Function<String, Predicate<AccountState>>() {
@Override
public Predicate<AccountState> apply(String s) {
return defaultField(s);
}
}));
}
@Override
public Predicate<AccountState> defaultField(String query) {
protected Predicate<AccountState> defaultField(String query) {
// Adapt the capacity of this list when adding more default predicates.
List<Predicate<AccountState>> preds = Lists.newArrayListWithCapacity(4);
if ("self".equalsIgnoreCase(query)) {

2
gerrit-server/src/test/java/com/google/gerrit/server/query/account/AbstractQueryAccountsTest.java
View File

@ -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);