From 2e8e9dc8cebdb7e378e5640b01b7a53e018ef58a Mon Sep 17 00:00:00 2001 From: Edwin Kempin Date: Fri, 1 Jul 2016 11:03:57 +0200 Subject: [PATCH] Account query: Add option to control if details should be returned By default account queries do not return details, only for account suggestions details are always returned since the details are needed to display the suggestions. Change-Id: I49a9aa115b227ff80017e037e05eaeb051669c2e Signed-off-by: Edwin Kempin --- Documentation/rest-api-accounts.txt | 37 ++++++++---- .../extensions/api/accounts/Accounts.java | 24 ++++++++ .../extensions/client/ListAccountsOption.java | 59 +++++++++++++++++++ .../gerrit/server/account/QueryAccounts.java | 24 +++++++- .../server/api/accounts/AccountsImpl.java | 4 ++ .../account/AbstractQueryAccountsTest.java | 24 ++++++++ 6 files changed, 157 insertions(+), 15 deletions(-) create mode 100644 gerrit-extension-api/src/main/java/com/google/gerrit/extensions/client/ListAccountsOption.java diff --git a/Documentation/rest-api-accounts.txt b/Documentation/rest-api-accounts.txt index 8715773e0a..3698a97bbe 100644 --- a/Documentation/rest-api-accounts.txt +++ b/Documentation/rest-api-accounts.txt @@ -36,15 +36,9 @@ returned. [ { "_account_id": 1000096, - "name": "John Doe", - "email": "john.doe@example.com", - "username": "john" }, { "_account_id": 1001439, - "name": "John Smith", - "email": "john.smith@example.com", - "username": "jsmith", "_more_accounts": true } ] @@ -57,11 +51,24 @@ object has a `_more_accounts: true` JSON field set. The `S` or `start` query parameter can be supplied to skip a number of accounts from the list. +Additional fields can be obtained by adding `o` parameters, each +option slows down the query response time to the client so they are +generally disabled by default. Optional fields are: + +[[details]] +-- +* `DETAILS`: Includes full name, preferred email, username and avatars +for each account. +-- + [[suggest-account]] To get account suggestions set the parameter `suggest` and provide the typed substring as query `q`. If a result limit `n` is not specified, then the default 10 is used. +For account suggestions link:#details[account details] are always +returned. + .Request ---- GET /accounts/?suggest&q=John HTTP/1.0 @@ -1947,15 +1954,21 @@ The `AccountInfo` entity contains information about an account. |Field Name ||Description |`_account_id` ||The numeric ID of the account. |`name` |optional|The full name of the user. + -Only set if link:rest-api-changes.html#detailed-accounts[detailed -account information] is requested. +Only set if detailed account information is requested. + +See option link:rest-api-changes.html#detailed-accounts[ +DETAILED_ACCOUNTS] for change queries + +and option link:#detaileds[DETAILS] for account queries. |`email` |optional| The email address the user prefers to be contacted through. + -Only set if link:rest-api-changes.html#detailed-accounts[detailed -account information] is requested. +Only set if detailed account information is requested. + +See option link:rest-api-changes.html#detailed-accounts[ +DETAILED_ACCOUNTS] for change queries + +and option link:#detaileds[DETAILS] for account queries. |`username` |optional|The username of the user. + -Only set if link:rest-api-changes.html#detailed-accounts[detailed -account information] is requested. +Only set if detailed account information is requested. + +See option link:rest-api-changes.html#detailed-accounts[ +DETAILED_ACCOUNTS] for change queries + +and option link:#detaileds[DETAILS] for account queries. |`_more_accounts`|optional, not set if `false`| Whether the query would deliver more results if not limited. + Only set on the last account that is returned. diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/accounts/Accounts.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/accounts/Accounts.java index dc2bfd8dd2..fe8a1d89af 100644 --- a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/accounts/Accounts.java +++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/accounts/Accounts.java @@ -14,10 +14,13 @@ package com.google.gerrit.extensions.api.accounts; +import com.google.gerrit.extensions.client.ListAccountsOption; import com.google.gerrit.extensions.common.AccountInfo; import com.google.gerrit.extensions.restapi.NotImplementedException; import com.google.gerrit.extensions.restapi.RestApiException; +import java.util.Arrays; +import java.util.EnumSet; import java.util.List; public interface Accounts { @@ -141,6 +144,8 @@ public interface Accounts { private String query; private int limit; private int start; + private EnumSet options = + EnumSet.noneOf(ListAccountsOption.class); /** * Executes query and returns a list of accounts. @@ -175,6 +180,21 @@ public interface Accounts { return this; } + public QueryRequest withOption(ListAccountsOption options) { + this.options.add(options); + return this; + } + + public QueryRequest withOptions(ListAccountsOption... options) { + this.options.addAll(Arrays.asList(options)); + return this; + } + + public QueryRequest withOptions(EnumSet options) { + this.options = options; + return this; + } + public String getQuery() { return query; } @@ -186,6 +206,10 @@ public interface Accounts { public int getStart() { return start; } + + public EnumSet getOptions() { + return options; + } } /** diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/client/ListAccountsOption.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/client/ListAccountsOption.java new file mode 100644 index 0000000000..bc3679cb4f --- /dev/null +++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/client/ListAccountsOption.java @@ -0,0 +1,59 @@ +// Copyright (C) 2016 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.extensions.client; + +import java.util.EnumSet; +import java.util.Set; + +/** Output options available for retrieval of account details. */ +public enum ListAccountsOption { + /** Return detailed account properties. */ + DETAILS(0); + + private final int value; + + ListAccountsOption(int v) { + this.value = v; + } + + public int getValue() { + return value; + } + + public static EnumSet fromBits(int v) { + EnumSet r = EnumSet.noneOf(ListAccountsOption.class); + for (ListAccountsOption o : ListAccountsOption.values()) { + if ((v & (1 << o.value)) != 0) { + r.add(o); + v &= ~(1 << o.value); + } + if (v == 0) { + return r; + } + } + if (v != 0) { + throw new IllegalArgumentException("unknown " + Integer.toHexString(v)); + } + return r; + } + + public static int toBits(Set set) { + int r = 0; + for (ListAccountsOption o : set) { + r |= 1 << o.value; + } + return r; + } +} 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 436cbdcb04..7c3ef1eecf 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 @@ -16,6 +16,7 @@ package com.google.gerrit.server.account; import com.google.common.base.Strings; import com.google.common.collect.ImmutableList; +import com.google.gerrit.extensions.client.ListAccountsOption; import com.google.gerrit.extensions.common.AccountInfo; import com.google.gerrit.extensions.restapi.BadRequestException; import com.google.gerrit.extensions.restapi.MethodNotAllowedException; @@ -40,6 +41,7 @@ import org.eclipse.jgit.lib.Config; import org.kohsuke.args4j.Option; import java.util.Collections; +import java.util.EnumSet; import java.util.HashMap; import java.util.LinkedHashMap; import java.util.List; @@ -50,7 +52,7 @@ public class QueryAccounts implements RestReadView { private static final String MAX_SUFFIX = "\u9fa5"; private final AccountControl accountControl; - private final AccountLoader accountLoader; + private final AccountLoader.Factory accountLoaderFactory; private final AccountCache accountCache; private final AccountIndexCollection indexes; private final AccountQueryBuilder queryBuilder; @@ -59,10 +61,12 @@ public class QueryAccounts implements RestReadView { private final boolean suggestConfig; private final int suggestFrom; + private AccountLoader accountLoader; private boolean suggest; private int suggestLimit = 10; private String query; private Integer start; + private EnumSet options; @Option(name = "--suggest", metaVar = "SUGGEST", usage = "suggest users") public void setSuggest(boolean suggest) { @@ -82,6 +86,16 @@ public class QueryAccounts implements RestReadView { } } + @Option(name = "-o", usage = "Output options per account") + public void addOption(ListAccountsOption o) { + options.add(o); + } + + @Option(name = "-O", usage = "Output option flags, in hex") + void setOptionFlagsHex(String hex) { + options.addAll(ListAccountsOption.fromBits(Integer.parseInt(hex, 16))); + } + @Option(name = "--query", aliases = {"-q"}, metaVar = "QUERY", usage = "match users") public void setQuery(String query) { this.query = query; @@ -102,14 +116,15 @@ public class QueryAccounts implements RestReadView { AccountQueryProcessor queryProcessor, ReviewDb db, @GerritServerConfig Config cfg) { - accountControl = accountControlFactory.get(); - accountLoader = accountLoaderFactory.create(true); + this.accountControl = accountControlFactory.get(); + this.accountLoaderFactory = accountLoaderFactory; this.accountCache = accountCache; this.indexes = indexes; this.queryBuilder = queryBuilder; this.queryProcessor = queryProcessor; this.db = db; this.suggestFrom = cfg.getInt("suggest", null, "from", 0); + this.options = EnumSet.noneOf(ListAccountsOption.class); if ("off".equalsIgnoreCase(cfg.getString("suggest", null, "accounts"))) { suggestConfig = false; @@ -137,6 +152,9 @@ public class QueryAccounts implements RestReadView { return Collections.emptyList(); } + accountLoader = accountLoaderFactory + .create(suggest || options.contains(ListAccountsOption.DETAILS)); + AccountIndex searchIndex = indexes.getSearchIndex(); if (searchIndex != null) { return queryFromIndex(); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/api/accounts/AccountsImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/api/accounts/AccountsImpl.java index d92b8f7035..39e8b518db 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/api/accounts/AccountsImpl.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/api/accounts/AccountsImpl.java @@ -16,6 +16,7 @@ package com.google.gerrit.server.api.accounts; import com.google.gerrit.extensions.api.accounts.AccountApi; import com.google.gerrit.extensions.api.accounts.Accounts; +import com.google.gerrit.extensions.client.ListAccountsOption; import com.google.gerrit.extensions.common.AccountInfo; import com.google.gerrit.extensions.restapi.AuthException; import com.google.gerrit.extensions.restapi.IdString; @@ -124,6 +125,9 @@ public class AccountsImpl implements Accounts { myQueryAccounts.setQuery(r.getQuery()); myQueryAccounts.setLimit(r.getLimit()); myQueryAccounts.setStart(r.getStart()); + for (ListAccountsOption option : r.getOptions()) { + myQueryAccounts.addOption(option); + } return myQueryAccounts.apply(TopLevelResource.INSTANCE); } catch (OrmException e) { throw new RestApiException("Cannot retrieve suggested accounts", e); 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 1a9b312043..355edaf122 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 @@ -22,6 +22,7 @@ import com.google.common.collect.FluentIterable; import com.google.common.collect.ImmutableList; import com.google.gerrit.extensions.api.GerritApi; import com.google.gerrit.extensions.api.accounts.Accounts.QueryRequest; +import com.google.gerrit.extensions.client.ListAccountsOption; import com.google.gerrit.extensions.common.AccountInfo; import com.google.gerrit.extensions.restapi.ResourceNotFoundException; import com.google.gerrit.extensions.restapi.RestApiException; @@ -270,6 +271,29 @@ public abstract class AbstractQueryAccountsTest extends GerritServerTests { assertQuery(newQuery(domain).withStart(1), user2, user3); } + @Test + public void withDetails() throws Exception { + AccountInfo user1 = + newAccount("myuser", "My User", "my.user@example.com", true); + + List result = assertQuery(user1.username, user1); + AccountInfo ai = result.get(0); + assertThat(ai._accountId).isEqualTo(user1._accountId); + assertThat(ai.name).isNull(); + assertThat(ai.username).isNull(); + assertThat(ai.email).isNull(); + assertThat(ai.avatars).isNull(); + + result = assertQuery( + newQuery(user1.username).withOption(ListAccountsOption.DETAILS), user1); + ai = result.get(0); + assertThat(ai._accountId).isEqualTo(user1._accountId); + assertThat(ai.name).isEqualTo(user1.name); + assertThat(ai.username).isEqualTo(user1.username); + assertThat(ai.email).isEqualTo(user1.email); + assertThat(ai.avatars).isNull(); + } + protected AccountInfo newAccount(String username) throws Exception { return newAccountWithEmail(username, null); }