Merge "Add new API to lookup accounts by email"

gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java

@@ -74,6 +74,7 @@ import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.OutputFormat;
 import com.google.gerrit.server.PatchSetUtil;
 import com.google.gerrit.server.account.AccountCache;
+import com.google.gerrit.server.account.Accounts;
 import com.google.gerrit.server.account.GroupCache;
 import com.google.gerrit.server.change.Abandon;
 import com.google.gerrit.server.change.ChangeResource;
@@ -200,6 +201,7 @@ public abstract class AbstractDaemonTest {
   @Inject protected AcceptanceTestRequestScope atrScope;
   @Inject protected AccountCache accountCache;
   @Inject protected AccountCreator accountCreator;
+  @Inject protected Accounts accounts;
   @Inject protected AllProjectsName allProjects;
   @Inject protected BatchUpdate.Factory batchUpdateFactory;
   @Inject protected ChangeData.Factory changeDataFactory;

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/accounts/AccountIT.java

@@ -38,6 +38,7 @@ import static org.junit.Assert.fail;
 import com.google.common.collect.FluentIterable;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.ImmutableSetMultimap;
 import com.google.common.collect.Iterables;
 import com.google.common.io.BaseEncoding;
 import com.google.common.util.concurrent.AtomicLongMap;
@@ -696,6 +697,33 @@ public class AccountIT extends AbstractDaemonTest {
     assertThat(byEmailCache.get("non-existing@example.com")).isEmpty();
   }
 
+  @Test
+  public void lookUpByEmail() throws Exception {
+    // exact match with scheme "mailto:"
+    assertEmail(accounts.byEmail(admin.email), admin);
+
+    // exact match with other scheme
+    String email = "foo.bar@example.com";
+    externalIdsUpdateFactory
+        .create()
+        .insert(ExternalId.createWithEmail(ExternalId.Key.parse("foo:bar"), admin.id, email));
+    assertEmail(accounts.byEmail(email), admin);
+
+    // wrong case doesn't match
+    assertThat(accounts.byEmail(admin.email.toUpperCase(Locale.US))).isEmpty();
+
+    // prefix doesn't match
+    assertThat(accounts.byEmail(admin.email.substring(0, admin.email.indexOf('@')))).isEmpty();
+
+    // non-existing doesn't match
+    assertThat(accounts.byEmail("non-existing@example.com")).isEmpty();
+
+    // lookup several accounts by email at once
+    ImmutableSetMultimap<String, Account.Id> byEmails = accounts.byEmails(admin.email, user.email);
+    assertEmail(byEmails.get(admin.email), admin);
+    assertEmail(byEmails.get(user.email), user);
+  }
+
   @Test
   public void putStatus() throws Exception {
     List<String> statuses = ImmutableList.of("OOO", "Busy");

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

@@ -14,13 +14,18 @@
 
 package com.google.gerrit.server.account;
 
+import static com.google.common.collect.ImmutableSet.toImmutableSet;
+import static com.google.common.collect.ImmutableSetMultimap.toImmutableSetMultimap;
 import static java.util.Comparator.comparing;
 import static java.util.stream.Collectors.toList;
 import static java.util.stream.Collectors.toSet;
 
+import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.ImmutableSetMultimap;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.RefNames;
 import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gerrit.server.account.externalids.ExternalIds;
 import com.google.gerrit.server.config.AllUsersName;
 import com.google.gerrit.server.config.GerritServerConfig;
 import com.google.gerrit.server.git.GitRepositoryManager;
@@ -32,6 +37,7 @@ import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
+import java.util.Map;
 import java.util.Objects;
 import java.util.Set;
 import java.util.stream.Stream;
@@ -50,17 +56,20 @@ public class Accounts {
   private final GitRepositoryManager repoManager;
   private final AllUsersName allUsersName;
   private final OutgoingEmailValidator emailValidator;
+  private final ExternalIds externalIds;
 
   @Inject
   Accounts(
       @GerritServerConfig Config cfg,
       GitRepositoryManager repoManager,
       AllUsersName allUsersName,
-      OutgoingEmailValidator emailValidator) {
+      OutgoingEmailValidator emailValidator,
+      ExternalIds externalIds) {
     this.readFromGit = cfg.getBoolean("user", null, "readAccountsFromGit", false);
     this.repoManager = repoManager;
     this.allUsersName = allUsersName;
     this.emailValidator = emailValidator;
+    this.externalIds = externalIds;
   }
 
   public Account get(ReviewDb db, Account.Id accountId)
@@ -89,6 +98,44 @@ public class Accounts {
     return db.accounts().get(accountIds).toList();
   }
 
+  /**
+   * Returns the accounts with the given email.
+   *
+   * <p>Each email should belong to a single account only. This means if more than one account is
+   * returned there is an inconsistency in the external IDs.
+   *
+   * <p>The accounts are retrieved via the external ID cache. Each access to the external ID cache
+   * requires reading the SHA1 of the refs/meta/external-ids branch. If accounts for multiple emails
+   * are needed it is more efficient to use {@link #byEmails(String...)} as this method reads the
+   * SHA1 of the refs/meta/external-ids branch only once (and not once per email).
+   *
+   * @see #byEmails(String...)
+   */
+  public ImmutableSet<Account.Id> byEmail(String email) throws IOException {
+    return externalIds.byEmail(email).stream().map(e -> e.accountId()).collect(toImmutableSet());
+  }
+
+  /**
+   * Returns the accounts for the given emails.
+   *
+   * <p>Each email should belong to a single account only. This means if more than one account for
+   * an email is returned there is an inconsistency in the external IDs.
+   *
+   * <p>The accounts are retrieved via the external ID cache. Each access to the external ID cache
+   * requires reading the SHA1 of the refs/meta/external-ids branch. If accounts for multiple emails
+   * are needed it is more efficient to use this method instead of {@link #byEmail(String)} as this
+   * method reads the SHA1 of the refs/meta/external-ids branch only once (and not once per email).
+   *
+   * @see #byEmail(String)
+   */
+  public ImmutableSetMultimap<String, Account.Id> byEmails(String... emails) throws IOException {
+    return externalIds
+        .byEmails(emails)
+        .entries()
+        .stream()
+        .collect(toImmutableSetMultimap(Map.Entry::getKey, e -> e.getValue().accountId()));
+  }
+
   /**
    * Returns all accounts.
    *

gerrit-server/src/main/java/com/google/gerrit/server/account/externalids/DisabledExternalIdCache.java

@@ -14,6 +14,7 @@
 
 package com.google.gerrit.server.account.externalids;
 
+import com.google.common.collect.ImmutableSetMultimap;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.inject.AbstractModule;
 import com.google.inject.Module;
@@ -89,7 +90,7 @@ public class DisabledExternalIdCache implements ExternalIdCache {
   }
 
   @Override
-  public Set<ExternalId> byEmail(String email) throws IOException {
+  public ImmutableSetMultimap<String, ExternalId> byEmails(String... emails) throws IOException {
     throw new UnsupportedOperationException();
   }
 }

gerrit-server/src/main/java/com/google/gerrit/server/account/externalids/ExternalIdCache.java

@@ -14,6 +14,8 @@
 
 package com.google.gerrit.server.account.externalids;
 
+import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.ImmutableSetMultimap;
 import com.google.gerrit.reviewdb.client.Account;
 import java.io.IOException;
 import java.util.Collection;
@@ -21,7 +23,12 @@ import java.util.Collections;
 import java.util.Set;
 import org.eclipse.jgit.lib.ObjectId;
 
-/** Caches external IDs of all accounts */
+/**
+ * Caches external IDs of all accounts.
+ *
+ * <p>On each cache access the SHA1 of the refs/meta/external-ids branch is read to verify that the
+ * cache is up to date.
+ */
 interface ExternalIdCache {
   void onCreate(ObjectId oldNotesRev, ObjectId newNotesRev, Collection<ExternalId> extId)
       throws IOException;
@@ -75,7 +82,11 @@ interface ExternalIdCache {
 
   Set<ExternalId> byAccount(Account.Id accountId) throws IOException;
 
-  Set<ExternalId> byEmail(String email) throws IOException;
+  ImmutableSetMultimap<String, ExternalId> byEmails(String... emails) throws IOException;
+
+  default ImmutableSet<ExternalId> byEmail(String email) throws IOException {
+    return byEmails(email).get(email);
+  }
 
   default void onCreate(ObjectId oldNotesRev, ObjectId newNotesRev, ExternalId extId)
       throws IOException {

gerrit-server/src/main/java/com/google/gerrit/server/account/externalids/ExternalIdCacheImpl.java

@@ -226,9 +226,14 @@ class ExternalIdCacheImpl implements ExternalIdCache {
   }
 
   @Override
-  public Set<ExternalId> byEmail(String email) throws IOException {
+  public ImmutableSetMultimap<String, ExternalId> byEmails(String... emails) throws IOException {
     try {
-      return extIdsByAccount.get(externalIdReader.readRevision()).byEmail().get(email);
+      AllExternalIds allExternalIds = extIdsByAccount.get(externalIdReader.readRevision());
+      ImmutableSetMultimap.Builder<String, ExternalId> byEmails = ImmutableSetMultimap.builder();
+      for (String email : emails) {
+        byEmails.putAll(email, allExternalIds.byEmail().get(email));
+      }
+      return byEmails.build();
     } catch (ExecutionException e) {
       throw new IOException("Cannot list external ids by email", e);
     }

gerrit-server/src/main/java/com/google/gerrit/server/account/externalids/ExternalIds.java

@@ -16,6 +16,7 @@ package com.google.gerrit.server.account.externalids;
 
 import static java.util.stream.Collectors.toSet;
 
+import com.google.common.collect.ImmutableSetMultimap;
 import com.google.gerrit.common.Nullable;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.inject.Inject;
@@ -74,7 +75,38 @@ public class ExternalIds {
     return byAccount(accountId).stream().filter(e -> e.key().isScheme(scheme)).collect(toSet());
   }
 
+  /**
+   * Returns the external ID with the given email.
+   *
+   * <p>Each email should belong to a single external ID only. This means if more than one external
+   * ID is returned there is an inconsistency in the external IDs.
+   *
+   * <p>The external IDs are retrieved from the external ID cache. Each access to the external ID
+   * cache requires reading the SHA1 of the refs/meta/external-ids branch. If external IDs for
+   * multiple emails are needed it is more efficient to use {@link #byEmails(String...)} as this
+   * method reads the SHA1 of the refs/meta/external-ids branch only once (and not once per email).
+   *
+   * @see #byEmails(String...)
+   */
   public Set<ExternalId> byEmail(String email) throws IOException {
     return externalIdCache.byEmail(email);
   }
+
+  /**
+   * Returns the external IDs for the given emails.
+   *
+   * <p>Each email should belong to a single external ID only. This means if more than one external
+   * ID for an email is returned there is an inconsistency in the external IDs.
+   *
+   * <p>The external IDs are retrieved from the external ID cache. Each access to the external ID
+   * cache requires reading the SHA1 of the refs/meta/external-ids branch. If external IDs for
+   * multiple emails are needed it is more efficient to use this method instead of {@link
+   * #byEmail(String)} as this method reads the SHA1 of the refs/meta/external-ids branch only once
+   * (and not once per email).
+   *
+   * @see #byEmail(String)
+   */
+  public ImmutableSetMultimap<String, ExternalId> byEmails(String... emails) throws IOException {
+    return externalIdCache.byEmails(emails);
+  }
 }