Account queries: Add option to include secondary emails

If an account is suggested due to a match on a secondary email address
the UI needs to have the secondary email address available to show it
in the suggestion (see Ife5e3 for more details).

Change-Id: I23c72630f54250e7ac0a7ff26d4b28e8a6af0df3
Signed-off-by: Edwin Kempin <ekempin@google.com>

Documentation/rest-api-accounts.txt

@@ -61,13 +61,18 @@ generally disabled by default. Optional fields are:
 for each account.
 --
 
+[[all-emails]]
+--
+* `ALL_EMAILS`: Includes all registered emails.
+--
+
 [[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
+For account suggestions link:#details[account details] and
-returned.
+link:#all-emails[all emails] are always returned.
 
 .Request
 ----
@@ -1950,29 +1955,34 @@ registered.
 The `AccountInfo` entity contains information about an account.
 
 [options="header",cols="1,^1,5"]
-|=============================
+|===============================
-|Field Name      ||Description
+|Field Name        ||Description
-|`_account_id`   ||The numeric ID of the account.
+|`_account_id`     ||The numeric ID of the account.
-|`name`          |optional|The full name of the user. +
+|`name`            |optional|The full name of the user. +
 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:#details[DETAILS] for account queries.
-|`email`         |optional|
+|`email`           |optional|
 The email address the user prefers to be contacted through. +
 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:#details[DETAILS] for account queries.
+and options link:#details[DETAILS] and link:#all-emails[
-|`username`      |optional|The username of the user. +
+ALL_EMAILS] for account queries.
+|`secondary_emails`|optional|
+A list of the secondary email addresses of the user. +
+Only set for account queries when the link:#all-emails[ALL_EMAILS]
+option is set.
+|`username`        |optional|The username of the user. +
 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:#details[DETAILS] for account queries.
-|`_more_accounts`|optional, not set if `false`|
+|`_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.
-|=============================
+|===============================
 
 [[account-input]]
 === AccountInput

gerrit-extension-api/src/main/java/com/google/gerrit/extensions/client/ListAccountsOption.java

@@ -20,7 +20,10 @@ import java.util.Set;
 /** Output options available for retrieval of account details. */
 public enum ListAccountsOption {
   /** Return detailed account properties. */
-  DETAILS(0);
+  DETAILS(0),
+
+  /** Return all secondary emails. */
+  ALL_EMAILS(1);
 
   private final int value;
 

gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/AccountInfo.java

@@ -20,6 +20,7 @@ public class AccountInfo {
   public Integer _accountId;
   public String name;
   public String email;
+  public List<String> secondaryEmails;
   public String username;
   public List<AvatarInfo> avatars;
   public Boolean _moreAccounts;

gerrit-server/src/main/java/com/google/gerrit/server/account/AccountDirectory.java

@@ -32,6 +32,9 @@ public abstract class AccountDirectory {
     /** Preferred email address to contact the user at. */
     EMAIL,
 
+    /** All secondary email addresses of the user. */
+    SECONDARY_EMAILS,
+
     /** User profile images. */
     AVATARS,
 

gerrit-server/src/main/java/com/google/gerrit/server/account/AccountLoader.java

@@ -23,8 +23,8 @@ import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.server.account.AccountDirectory.DirectoryException;
 import com.google.gerrit.server.account.AccountDirectory.FillOptions;
 import com.google.gwtorm.server.OrmException;
-import com.google.inject.Inject;
 import com.google.inject.assistedinject.Assisted;
+import com.google.inject.assistedinject.AssistedInject;
 
 import java.util.ArrayList;
 import java.util.Collection;
@@ -36,7 +36,7 @@ import java.util.Map;
 import java.util.Set;
 
 public class AccountLoader {
-  private static final Set<FillOptions> DETAILED_OPTIONS =
+  public static final Set<FillOptions> DETAILED_OPTIONS =
       Collections.unmodifiableSet(EnumSet.of(
           FillOptions.ID,
           FillOptions.NAME,
@@ -46,6 +46,7 @@ public class AccountLoader {
 
   public interface Factory {
     AccountLoader create(boolean detailed);
+    AccountLoader create(Set<FillOptions> options);
   }
 
   private final InternalAccountDirectory directory;
@@ -53,10 +54,20 @@ public class AccountLoader {
   private final Map<Account.Id, AccountInfo> created;
   private final List<AccountInfo> provided;
 
-  @Inject
+  @AssistedInject
-  AccountLoader(InternalAccountDirectory directory, @Assisted boolean detailed) {
+  AccountLoader(InternalAccountDirectory directory,
+      @Assisted boolean detailed) {
+    this(directory,
+        detailed
+            ? DETAILED_OPTIONS
+            : InternalAccountDirectory.ID_ONLY);
+  }
+
+  @AssistedInject
+  AccountLoader(InternalAccountDirectory directory,
+      @Assisted Set<FillOptions> options) {
     this.directory = directory;
-    options = detailed ? DETAILED_OPTIONS : InternalAccountDirectory.ID_ONLY;
+    this.options = options;
     created = new HashMap<>();
     provided = new ArrayList<>();
   }

gerrit-server/src/main/java/com/google/gerrit/server/account/AccountState.java

@@ -14,6 +14,7 @@
 
 package com.google.gerrit.server.account;
 
+import static com.google.gerrit.reviewdb.client.AccountExternalId.SCHEME_MAILTO;
 import static com.google.gerrit.reviewdb.client.AccountExternalId.SCHEME_USERNAME;
 
 import com.google.common.cache.Cache;
@@ -26,6 +27,7 @@ import com.google.gerrit.server.CurrentUser.PropertyKey;
 import com.google.gerrit.server.IdentifiedUser;
 
 import java.util.Collection;
+import java.util.HashSet;
 import java.util.Set;
 
 public class AccountState {
@@ -88,6 +90,16 @@ public class AccountState {
     return null;
   }
 
+  public static Set<String> getEmails(Collection<AccountExternalId> ids) {
+    Set<String> emails = new HashSet<>();
+    for (AccountExternalId id : ids) {
+      if (id.isScheme(SCHEME_MAILTO)) {
+        emails.add(id.getSchemeRest());
+      }
+    }
+    return emails;
+  }
+
   /**
    * Lookup a previously stored property.
    * <p>

gerrit-server/src/main/java/com/google/gerrit/server/account/InternalAccountDirectory.java

@@ -17,10 +17,12 @@ package com.google.gerrit.server.account;
 import com.google.common.base.Strings;
 import com.google.common.collect.ArrayListMultimap;
 import com.google.common.collect.Multimap;
+import com.google.gerrit.common.Nullable;
 import com.google.gerrit.extensions.common.AccountInfo;
 import com.google.gerrit.extensions.common.AvatarInfo;
 import com.google.gerrit.extensions.registration.DynamicItem;
 import com.google.gerrit.reviewdb.client.Account;
+import com.google.gerrit.reviewdb.client.AccountExternalId;
 import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.avatar.AvatarProvider;
@@ -31,8 +33,10 @@ import com.google.inject.Provider;
 import com.google.inject.Singleton;
 
 import java.util.ArrayList;
+import java.util.Collection;
 import java.util.Collections;
 import java.util.EnumSet;
+import java.util.List;
 import java.util.Set;
 
 @Singleton
@@ -76,7 +80,7 @@ public class InternalAccountDirectory extends AccountDirectory {
       Account.Id id = new Account.Id(info._accountId);
       AccountState state = accountCache.getIfPresent(id);
       if (state != null) {
-        fill(info, state.getAccount(), options);
+        fill(info, state.getAccount(), state.getExternalIds(), options);
       } else {
         missing.put(id, info);
       }
@@ -84,12 +88,14 @@ public class InternalAccountDirectory extends AccountDirectory {
     if (!missing.isEmpty()) {
       try {
         for (Account account : db.get().accounts().get(missing.keySet())) {
-          if (options.contains(FillOptions.USERNAME)) {
+          Collection<AccountExternalId> externalIds = null;
-            account.setUserName(AccountState.getUserName(
+          if (options.contains(FillOptions.USERNAME)
-                db.get().accountExternalIds().byAccount(account.getId()).toList()));
+              || options.contains(FillOptions.SECONDARY_EMAILS)) {
+            externalIds = db.get().accountExternalIds()
+                .byAccount(account.getId()).toList();
           }
           for (AccountInfo info : missing.get(account.getId())) {
-            fill(info, account, options);
+            fill(info, account, externalIds, options);
           }
         }
       } catch (OrmException e) {
@@ -100,6 +106,7 @@ public class InternalAccountDirectory extends AccountDirectory {
 
   private void fill(AccountInfo info,
       Account account,
+      @Nullable Collection<AccountExternalId> externalIds,
       Set<FillOptions> options) {
     if (options.contains(FillOptions.ID)) {
       info._accountId = account.getId().get();
@@ -116,8 +123,15 @@ public class InternalAccountDirectory extends AccountDirectory {
     if (options.contains(FillOptions.EMAIL)) {
       info.email = account.getPreferredEmail();
     }
+    if (options.contains(FillOptions.SECONDARY_EMAILS)) {
+      info.secondaryEmails = externalIds != null
+          ? getSecondaryEmails(account, externalIds)
+          : null;
+    }
     if (options.contains(FillOptions.USERNAME)) {
-      info.username = account.getUserName();
+      info.username = externalIds != null
+          ? AccountState.getUserName(externalIds)
+          : null;
     }
     if (options.contains(FillOptions.AVATARS)) {
       AvatarProvider ap = avatar.get();
@@ -141,6 +155,16 @@ public class InternalAccountDirectory extends AccountDirectory {
     }
   }
 
+  public List<String> getSecondaryEmails(Account account,
+      Collection<AccountExternalId> externalIds) {
+    List<String> emails = new ArrayList<>(AccountState.getEmails(externalIds));
+    if (account.getPreferredEmail() != null) {
+      emails.remove(account.getPreferredEmail());
+    }
+    Collections.sort(emails);
+    return emails;
+  }
+
   private static void addAvatar(
       AvatarProvider provider,
       AccountInfo account,

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

@@ -25,6 +25,7 @@ import com.google.gerrit.extensions.restapi.TopLevelResource;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.AccountExternalId;
 import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gerrit.server.account.AccountDirectory.FillOptions;
 import com.google.gerrit.server.api.accounts.AccountInfoComparator;
 import com.google.gerrit.server.config.GerritServerConfig;
 import com.google.gerrit.server.index.account.AccountIndex;
@@ -46,6 +47,7 @@ import java.util.HashMap;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 
 public class QueryAccounts implements RestReadView<TopLevelResource> {
   private static final int MAX_SUGGEST_RESULTS = 100;
@@ -152,8 +154,20 @@ public class QueryAccounts implements RestReadView<TopLevelResource> {
       return Collections.emptyList();
     }
 
-    accountLoader = accountLoaderFactory
+    Set<FillOptions> fillOptions = EnumSet.of(FillOptions.ID);
-        .create(suggest || options.contains(ListAccountsOption.DETAILS));
+    if (options.contains(ListAccountsOption.DETAILS)) {
+      fillOptions.addAll(AccountLoader.DETAILED_OPTIONS);
+    }
+    if (options.contains(ListAccountsOption.ALL_EMAILS)) {
+      fillOptions.add(FillOptions.EMAIL);
+      fillOptions.add(FillOptions.SECONDARY_EMAILS);
+    }
+    if (suggest) {
+      fillOptions.addAll(AccountLoader.DETAILED_OPTIONS);
+      fillOptions.add(FillOptions.EMAIL);
+      fillOptions.add(FillOptions.SECONDARY_EMAILS);
+    }
+    accountLoader = accountLoaderFactory.create(fillOptions);
 
     AccountIndex searchIndex = indexes.getSearchIndex();
     if (searchIndex != null) {

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

@@ -294,6 +294,34 @@ public abstract class AbstractQueryAccountsTest extends GerritServerTests {
     assertThat(ai.avatars).isNull();
   }
 
+  @Test
+  public void withSecondaryEmails() throws Exception {
+    AccountInfo user1 =
+        newAccount("myuser", "My User", "my.user@example.com", true);
+    String[] secondaryEmails =
+        new String[] {"bar@example.com", "foo@example.com"};
+    addEmails(user1, secondaryEmails);
+
+
+    List<AccountInfo> result = assertQuery(user1.username, user1);
+    assertThat(result.get(0).secondaryEmails).isNull();
+
+    result = assertQuery(
+        newQuery(user1.username).withOption(ListAccountsOption.DETAILS), user1);
+    assertThat(result.get(0).secondaryEmails).isNull();
+
+    result = assertQuery(
+        newQuery(user1.username).withOption(ListAccountsOption.ALL_EMAILS),
+        user1);
+    assertThat(result.get(0).secondaryEmails)
+        .containsExactlyElementsIn(Arrays.asList(secondaryEmails)).inOrder();
+
+    result = assertQuery(newQuery(user1.username).withOptions(
+        ListAccountsOption.DETAILS, ListAccountsOption.ALL_EMAILS), user1);
+    assertThat(result.get(0).secondaryEmails)
+        .containsExactlyElementsIn(Arrays.asList(secondaryEmails)).inOrder();
+  }
+
   protected AccountInfo newAccount(String username) throws Exception {
     return newAccountWithEmail(username, null);
   }
@@ -359,6 +387,15 @@ public abstract class AbstractQueryAccountsTest extends GerritServerTests {
     return id;
   }
 
+  private void addEmails(AccountInfo account, String... emails)
+      throws Exception {
+    Account.Id id = new Account.Id(account._accountId);
+    for (String email : emails) {
+      accountManager.link(id, AuthRequest.forEmail(email));
+    }
+    accountCache.evict(id);
+  }
+
   protected QueryRequest newQuery(Object query) throws RestApiException {
     return gApi.accounts().query(query.toString());
   }