opendev/gerrit
Code Issues Proposed changes

Support arbitrary account queries via REST

Browse Source 
GET /accounts/?q=<query> supports arbitrary account queries now. So
far this REST endpoint was used for account suggestions, suggesting
accounts for a substring that the user has typed. This will continue
to work since this is handled by the default field for account
queries. However if the provided query contains query operators a
full-fledged account query is executed. This is undesired for account
suggestions. This is why the old behaviour for account suggestions is
preserved when the parameter 'suggest' is provided. The web UI and the
java extension API are adapted to set this new parameter for account
suggestions. For old clients the new REST endpoint provides enough
backward compatibility so that they are not breaking as it is rather
unlikely that a substring that was typed by a user contains a query
operator.

Using /accounts/?q=<query> for arbitrary account queries has the
advantage that this is consistent with the REST endpoint for querying
changes which is /changes/?q=<query>.

Rename SuggestAccounts to QueryAccounts to reflect the new
functionality and to have a consistent name with QueryChanges.

Since the account query predicates are now exposed to external users
this change also adds documentation for them.

Change-Id: Iabe3e52893a17a21d4bfdec6dab737e0cebaea23
Signed-off-by: Edwin Kempin <ekempin@google.com>
This commit is contained in:
Edwin Kempin
2016-06-30 11:33:47 +02:00
parent 4c8a1340a1
commit c236140227
6 changed files with 188 additions and 41 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
gerrit-server/src/main/java/com/google/gerrit/server/account/AccountsCollection.java
View File

@@ -40,7 +40,7 @@ public class AccountsCollection implements
private final AccountResolver resolver;
private final AccountControl.Factory accountControlFactory;
private final IdentifiedUser.GenericFactory userFactory;
private final Provider<SuggestAccounts> list;
private final Provider<QueryAccounts> list;
private final DynamicMap<RestView<AccountResource>> views;
private final CreateAccount.Factory createAccountFactory;
@@ -49,7 +49,7 @@ public class AccountsCollection implements
AccountResolver resolver,
AccountControl.Factory accountControlFactory,
IdentifiedUser.GenericFactory userFactory,
Provider<SuggestAccounts> list,
Provider<QueryAccounts> list,
DynamicMap<RestView<AccountResource>> views,
CreateAccount.Factory createAccountFactory) {
this.self = self;

75
gerrit-server/src/main/java/com/google/gerrit/server/account/SuggestAccounts.java → gerrit-server/src/main/java/com/google/gerrit/server/account/QueryAccounts.java
View File

@@ -26,7 +26,9 @@ import com.google.gerrit.reviewdb.client.AccountExternalId;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.api.accounts.AccountInfoComparator;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.index.account.AccountIndex;
import com.google.gerrit.server.index.account.AccountIndexCollection;
import com.google.gerrit.server.query.Predicate;
import com.google.gerrit.server.query.QueryParseException;
import com.google.gerrit.server.query.QueryResult;
import com.google.gerrit.server.query.account.AccountQueryBuilder;
@@ -44,8 +46,8 @@ import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
public class SuggestAccounts implements RestReadView<TopLevelResource> {
private static final int MAX_RESULTS = 100;
public class QueryAccounts implements RestReadView<TopLevelResource> {
private static final int MAX_SUGGEST_RESULTS = 100;
private static final String MAX_SUFFIX = "\u9fa5";
private final AccountControl accountControl;
@@ -55,22 +57,29 @@ public class SuggestAccounts implements RestReadView<TopLevelResource> {
private final AccountQueryBuilder queryBuilder;
private final AccountQueryProcessor queryProcessor;
private final ReviewDb db;
private final boolean suggest;
private final boolean suggestConfig;
private final int suggestFrom;
private int limit = 10;
private boolean suggest;
private int suggestLimit = 10;
private String query;
@Option(name = "--suggest", metaVar = "SUGGEST", usage = "suggest users")
public void setSuggest(boolean suggest) {
this.suggest = suggest;
}
@Option(name = "--limit", aliases = {"-n"}, metaVar = "CNT", usage = "maximum number of users to return")
public void setLimit(int n) {
queryProcessor.setLimit(n);
if (n < 0) {
limit = 10;
suggestLimit = 10;
} else if (n == 0) {
limit = MAX_RESULTS;
suggestLimit = MAX_SUGGEST_RESULTS;
} else {
limit = Math.min(n, MAX_RESULTS);
suggestLimit = Math.min(n, MAX_SUGGEST_RESULTS);
}
queryProcessor.setLimit(limit);
}
@Option(name = "--query", aliases = {"-q"}, metaVar = "QUERY", usage = "match users")
@@ -79,7 +88,7 @@ public class SuggestAccounts implements RestReadView<TopLevelResource> {
}
@Inject
SuggestAccounts(AccountControl.Factory accountControlFactory,
QueryAccounts(AccountControl.Factory accountControlFactory,
AccountLoader.Factory accountLoaderFactory,
AccountCache accountCache,
AccountIndexCollection indexes,
@@ -97,7 +106,7 @@ public class SuggestAccounts implements RestReadView<TopLevelResource> {
this.suggestFrom = cfg.getInt("suggest", null, "from", 0);
if ("off".equalsIgnoreCase(cfg.getString("suggest", null, "accounts"))) {
suggest = false;
suggestConfig = false;
} else {
boolean suggest;
try {
@@ -107,7 +116,7 @@ public class SuggestAccounts implements RestReadView<TopLevelResource> {
} catch (IllegalArgumentException err) {
suggest = cfg.getBoolean("suggest", null, "accounts", true);
}
this.suggest = suggest;
this.suggestConfig = suggest;
}
}
@@ -118,33 +127,49 @@ public class SuggestAccounts implements RestReadView<TopLevelResource> {
throw new BadRequestException("missing query field");
}
if (!suggest || query.length() < suggestFrom) {
if (suggest && (!suggestConfig || query.length() < suggestFrom)) {
return Collections.emptyList();
}
Collection<AccountInfo> matches =
indexes.getSearchIndex() != null
? queryFromIndex()
: queryFromDb();
AccountIndex searchIndex = indexes.getSearchIndex();
Collection<AccountInfo> matches;
if (searchIndex != null) {
matches = queryFromIndex();
} else {
if (!suggest) {
throw new MethodNotAllowedException();
}
matches = queryFromDb();
}
return AccountInfoComparator.ORDER_NULLS_LAST.sortedCopy(matches);
}
public Collection<AccountInfo> queryFromIndex()
throws MethodNotAllowedException, OrmException {
throws BadRequestException, MethodNotAllowedException, OrmException {
if (queryProcessor.isDisabled()) {
throw new MethodNotAllowedException("query disabled");
}
Map<Account.Id, AccountInfo> matches = new LinkedHashMap<>();
try {
QueryResult<AccountState> result =
queryProcessor.query(queryBuilder.defaultField(query));
Predicate<AccountState> queryPred;
if (suggest) {
queryPred = queryBuilder.defaultField(query);
queryProcessor.setLimit(suggestLimit);
} else {
queryPred = queryBuilder.parse(query);
}
QueryResult<AccountState> result = queryProcessor.query(queryPred);
for (AccountState accountState : result.entities()) {
Account.Id id = accountState.getAccount().getId();
matches.put(id, accountLoader.get(id));
}
} catch (QueryParseException e) {
return ImmutableSet.of();
if (suggest) {
return ImmutableSet.of();
}
throw new BadRequestException(e.getMessage());
}
accountLoader.fill();
@@ -158,18 +183,18 @@ public class SuggestAccounts implements RestReadView<TopLevelResource> {
Map<Account.Id, AccountInfo> matches = new LinkedHashMap<>();
Map<Account.Id, String> queryEmail = new HashMap<>();
for (Account p : db.accounts().suggestByFullName(a, b, limit)) {
for (Account p : db.accounts().suggestByFullName(a, b, suggestLimit)) {
addSuggestion(matches, p);
}
if (matches.size() < limit) {
if (matches.size() < suggestLimit) {
for (Account p : db.accounts()
.suggestByPreferredEmail(a, b, limit - matches.size())) {
.suggestByPreferredEmail(a, b, suggestLimit - matches.size())) {
addSuggestion(matches, p);
}
}
if (matches.size() < limit) {
if (matches.size() < suggestLimit) {
for (AccountExternalId e : db.accountExternalIds()
.suggestByEmailAddress(a, b, limit - matches.size())) {
.suggestByEmailAddress(a, b, suggestLimit - matches.size())) {
if (addSuggestion(matches, e.getAccountId())) {
queryEmail.put(e.getAccountId(), e.getEmailAddress());
}

17
gerrit-server/src/main/java/com/google/gerrit/server/api/accounts/AccountsImpl.java
View File

@@ -24,7 +24,7 @@ import com.google.gerrit.extensions.restapi.TopLevelResource;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.account.AccountResource;
import com.google.gerrit.server.account.AccountsCollection;
import com.google.gerrit.server.account.SuggestAccounts;
import com.google.gerrit.server.account.QueryAccounts;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
@@ -37,17 +37,17 @@ public class AccountsImpl implements Accounts {
private final AccountsCollection accounts;
private final AccountApiImpl.Factory api;
private final Provider<CurrentUser> self;
private final Provider<SuggestAccounts> suggestAccountsProvider;
private final Provider<QueryAccounts> queryAccountsProvider;
@Inject
AccountsImpl(AccountsCollection accounts,
AccountApiImpl.Factory api,
Provider<CurrentUser> self,
Provider<SuggestAccounts> suggestAccountsProvider) {
Provider<QueryAccounts> queryAccountsProvider) {
this.accounts = accounts;
this.api = api;
this.self = self;
this.suggestAccountsProvider = suggestAccountsProvider;
this.queryAccountsProvider = queryAccountsProvider;
}
@Override
@@ -92,10 +92,11 @@ public class AccountsImpl implements Accounts {
private List<AccountInfo> suggestAccounts(SuggestAccountsRequest r)
throws RestApiException {
try {
SuggestAccounts mySuggestAccounts = suggestAccountsProvider.get();
mySuggestAccounts.setQuery(r.getQuery());
mySuggestAccounts.setLimit(r.getLimit());
return mySuggestAccounts.apply(TopLevelResource.INSTANCE);
QueryAccounts myQueryAccounts = queryAccountsProvider.get();
myQueryAccounts.setSuggest(true);
myQueryAccounts.setQuery(r.getQuery());
myQueryAccounts.setLimit(r.getLimit());
return myQueryAccounts.apply(TopLevelResource.INSTANCE);
} catch (OrmException e) {
throw new RestApiException("Cannot retrieve suggested accounts", e);
}