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 <ekempin@google.com>

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.

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<ListAccountsOption> 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<ListAccountsOption> 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<ListAccountsOption> getOptions() {
+      return options;
+    }
   }
 
   /**

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<ListAccountsOption> fromBits(int v) {
+    EnumSet<ListAccountsOption> 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<ListAccountsOption> set) {
+    int r = 0;
+    for (ListAccountsOption o : set) {
+      r |= 1 << o.value;
+    }
+    return r;
+  }
+}

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<TopLevelResource> {
   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<TopLevelResource> {
   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<ListAccountsOption> options;
 
   @Option(name = "--suggest", metaVar = "SUGGEST", usage = "suggest users")
   public void setSuggest(boolean suggest) {
@@ -82,6 +86,16 @@ public class QueryAccounts implements RestReadView<TopLevelResource> {
     }
   }
 
+  @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<TopLevelResource> {
       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<TopLevelResource> {
       return Collections.emptyList();
     }
 
+    accountLoader = accountLoaderFactory
+        .create(suggest || options.contains(ListAccountsOption.DETAILS));
+
     AccountIndex searchIndex = indexes.getSearchIndex();
     if (searchIndex != null) {
       return queryFromIndex();

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

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<AccountInfo> 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);
   }