Remove usage of AccountByEmailCache

With change I1c24da1378 there is a new Emails class that allows looking
up accounts by email. To find accounts by email it gets external IDs by
email from the ExternalIdCache and extracts the account IDs from the
external IDs. This is exactly what AccountByEmailCacheImpl.Loader was
doing. In addition the Emails class does an index lookup to also find
accounts by preferred email (see commit message of change I1c24da1378
for an explanation of why this is needed).

Adapt all code to use the new Emails class instead of the
AccountByEmailCache.

Looking up accounts by email via the ExternalIdCache means that the SHA1
of the refs/meta/external-ids branch is read on each lookup (to verify
that the cache is up to date). To avoid reading the SHA1 of the
refs/meta/external-ids branch multiple times when looking up accounts
by email in a loop the Emails class offers a method that can lookup
accounts for several emails at once. This method is currently not used
by Gerrit core, but plugins may need it (e.g. the find-owners plugin).

When emails are changed the ExternalIdCache is automatically evicted
since it detects when the refs/meta/external-ids branch was updated,
hence manual cache eviction for this cache is not needed.

Accounts are also reindexed if the preferred email is changed so that
looking up accounts by preferred email via the account index always
returns up-to-date results.

The AccountByEmailCache is only removed in the follow-up change. This
allows Google to adapt internal code to use the new API before the
AccountByEmailCache is dropped.

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

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

@@ -78,7 +78,6 @@ import com.google.gerrit.gpg.testutil.TestKey;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.RefNames;
 import com.google.gerrit.server.Sequences;
-import com.google.gerrit.server.account.AccountByEmailCache;
 import com.google.gerrit.server.account.AccountConfig;
 import com.google.gerrit.server.account.AccountsUpdate;
 import com.google.gerrit.server.account.Emails;
@@ -145,8 +144,6 @@ public class AccountIT extends AbstractDaemonTest {
 
   @Inject private AccountsUpdate.Server accountsUpdate;
 
-  @Inject private AccountByEmailCache byEmailCache;
-
   @Inject private ExternalIds externalIds;
 
   @Inject private ExternalIdsUpdate.User externalIdsUpdateFactory;
@@ -679,28 +676,6 @@ public class AccountIT extends AbstractDaemonTest {
     gApi.accounts().id(admin.id.get()).deleteEmail(admin.email);
   }
 
-  @Test
-  public void lookUpFromCacheByEmail() throws Exception {
-    // exact match with scheme "mailto:"
-    assertEmail(byEmailCache.get(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(byEmailCache.get(email), admin);
-
-    // wrong case doesn't match
-    assertThat(byEmailCache.get(admin.email.toUpperCase(Locale.US))).isEmpty();
-
-    // prefix doesn't match
-    assertThat(byEmailCache.get(admin.email.substring(0, admin.email.indexOf('@')))).isEmpty();
-
-    // non-existing doesn't match
-    assertThat(byEmailCache.get("non-existing@example.com")).isEmpty();
-  }
-
   @Test
   public void lookUpByEmail() throws Exception {
     // exact match with scheme "mailto:"

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

@@ -36,22 +36,22 @@ import org.eclipse.jgit.errors.ConfigInvalidException;
 public class AccountResolver {
   private final Realm realm;
   private final Accounts accounts;
-  private final AccountByEmailCache byEmail;
   private final AccountCache byId;
   private final Provider<InternalAccountQuery> accountQueryProvider;
+  private final Emails emails;
 
   @Inject
   AccountResolver(
       Realm realm,
       Accounts accounts,
-      AccountByEmailCache byEmail,
       AccountCache byId,
-      Provider<InternalAccountQuery> accountQueryProvider) {
+      Provider<InternalAccountQuery> accountQueryProvider,
+      Emails emails) {
     this.realm = realm;
     this.accounts = accounts;
-    this.byEmail = byEmail;
     this.byId = byId;
     this.accountQueryProvider = accountQueryProvider;
+    this.emails = emails;
   }
 
   /**
@@ -136,7 +136,8 @@ public class AccountResolver {
    * @return the single account that matches; null if no account matches or there are multiple
    *     candidates.
    */
-  public Account findByNameOrEmail(ReviewDb db, String nameOrEmail) throws OrmException {
+  public Account findByNameOrEmail(ReviewDb db, String nameOrEmail)
+      throws OrmException, IOException {
     Set<Account.Id> r = findAllByNameOrEmail(db, nameOrEmail);
     return r.size() == 1 ? byId.get(r.iterator().next()).getAccount() : null;
   }
@@ -149,11 +150,12 @@ public class AccountResolver {
    *     address ("email@example"), a full name ("Full Name").
    * @return the accounts that match, empty collection if none. Never null.
    */
-  public Set<Account.Id> findAllByNameOrEmail(ReviewDb db, String nameOrEmail) throws OrmException {
+  public Set<Account.Id> findAllByNameOrEmail(ReviewDb db, String nameOrEmail)
+      throws OrmException, IOException {
     int lt = nameOrEmail.indexOf('<');
     int gt = nameOrEmail.indexOf('>');
     if (lt >= 0 && gt > lt && nameOrEmail.contains("@")) {
-      Set<Account.Id> ids = byEmail.get(nameOrEmail.substring(lt + 1, gt));
+      Set<Account.Id> ids = emails.getAccountFor(nameOrEmail.substring(lt + 1, gt));
       if (ids.isEmpty() || ids.size() == 1) {
         return ids;
       }
@@ -171,7 +173,7 @@ public class AccountResolver {
     }
 
     if (nameOrEmail.contains("@")) {
-      return byEmail.get(nameOrEmail);
+      return emails.getAccountFor(nameOrEmail);
     }
 
     Account.Id id = realm.lookup(nameOrEmail);

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

@@ -19,20 +19,23 @@ import com.google.gerrit.extensions.client.AccountFieldName;
 import com.google.gerrit.extensions.client.AuthType;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.server.config.AuthConfig;
+import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
+import com.google.inject.Provider;
 import com.google.inject.Singleton;
+import java.io.IOException;
 import java.util.Set;
 
 @Singleton
 public class DefaultRealm extends AbstractRealm {
   private final EmailExpander emailExpander;
-  private final AccountByEmailCache byEmail;
+  private final Provider<Emails> emails;
   private final AuthConfig authConfig;
 
   @Inject
-  DefaultRealm(EmailExpander emailExpander, AccountByEmailCache byEmail, AuthConfig authConfig) {
+  DefaultRealm(EmailExpander emailExpander, Provider<Emails> emails, AuthConfig authConfig) {
     this.emailExpander = emailExpander;
-    this.byEmail = byEmail;
+    this.emails = emails;
     this.authConfig = authConfig;
   }
 
@@ -75,11 +78,15 @@ public class DefaultRealm extends AbstractRealm {
   public void onCreateAccount(AuthRequest who, Account account) {}
 
   @Override
-  public Account.Id lookup(String accountName) {
+  public Account.Id lookup(String accountName) throws IOException {
     if (emailExpander.canExpand(accountName)) {
-      final Set<Account.Id> c = byEmail.get(emailExpander.expand(accountName));
-      if (1 == c.size()) {
-        return c.iterator().next();
+      try {
+        Set<Account.Id> c = emails.get().getAccountFor(emailExpander.expand(accountName));
+        if (1 == c.size()) {
+          return c.iterator().next();
+        }
+      } catch (OrmException e) {
+        throw new IOException("Failed to query accounts by email", e);
       }
     }
     return null;

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

@@ -17,6 +17,7 @@ package com.google.gerrit.server.account;
 import com.google.gerrit.extensions.client.AccountFieldName;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.server.IdentifiedUser;
+import java.io.IOException;
 import java.util.Set;
 
 public interface Realm {
@@ -43,5 +44,5 @@ public interface Realm {
    * where there is an {@link EmailExpander} configured that knows how to convert the accountName
    * into an email address, and then locate the user by that email address.
    */
-  Account.Id lookup(String accountName);
+  Account.Id lookup(String accountName) throws IOException;
 }

gerrit-server/src/main/java/com/google/gerrit/server/events/EventFactory.java

@@ -35,8 +35,8 @@ import com.google.gerrit.reviewdb.client.UserIdentity;
 import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.ApprovalsUtil;
 import com.google.gerrit.server.GerritPersonIdent;
-import com.google.gerrit.server.account.AccountByEmailCache;
 import com.google.gerrit.server.account.AccountCache;
+import com.google.gerrit.server.account.Emails;
 import com.google.gerrit.server.change.ChangeKindCache;
 import com.google.gerrit.server.config.CanonicalWebUrl;
 import com.google.gerrit.server.data.AccountAttribute;
@@ -83,9 +83,9 @@ public class EventFactory {
   private static final Logger log = LoggerFactory.getLogger(EventFactory.class);
 
   private final AccountCache accountCache;
+  private final Emails emails;
   private final Provider<String> urlProvider;
   private final PatchListCache patchListCache;
-  private final AccountByEmailCache byEmailCache;
   private final PersonIdent myIdent;
   private final ChangeData.Factory changeDataFactory;
   private final ApprovalsUtil approvalsUtil;
@@ -96,8 +96,8 @@ public class EventFactory {
   @Inject
   EventFactory(
       AccountCache accountCache,
+      Emails emails,
       @CanonicalWebUrl @Nullable Provider<String> urlProvider,
-      AccountByEmailCache byEmailCache,
       PatchListCache patchListCache,
       @GerritPersonIdent PersonIdent myIdent,
       ChangeData.Factory changeDataFactory,
@@ -106,9 +106,9 @@ public class EventFactory {
       Provider<InternalChangeQuery> queryProvider,
       SchemaFactory<ReviewDb> schema) {
     this.accountCache = accountCache;
+    this.emails = emails;
     this.urlProvider = urlProvider;
     this.patchListCache = patchListCache;
-    this.byEmailCache = byEmailCache;
     this.myIdent = myIdent;
     this.changeDataFactory = changeDataFactory;
     this.approvalsUtil = approvalsUtil;
@@ -497,7 +497,7 @@ public class EventFactory {
         }
       }
       p.kind = changeKindCache.getChangeKind(db, change, patchSet);
-    } catch (IOException e) {
+    } catch (IOException | OrmException e) {
       log.error("Cannot load patch set data for " + patchSet.getId(), e);
     } catch (PatchListNotAvailableException e) {
       log.error(String.format("Cannot get size information for %s.", pId), e);
@@ -507,7 +507,7 @@ public class EventFactory {
 
   // TODO: The same method exists in PatchSetInfoFactory, find a common place
   // for it
-  private UserIdentity toUserIdentity(PersonIdent who) {
+  private UserIdentity toUserIdentity(PersonIdent who) throws IOException, OrmException {
     UserIdentity u = new UserIdentity();
     u.setName(who.getName());
     u.setEmail(who.getEmailAddress());
@@ -517,7 +517,7 @@ public class EventFactory {
     // If only one account has access to this email address, select it
     // as the identity of the user.
     //
-    Set<Account.Id> a = byEmailCache.get(u.getEmail());
+    Set<Account.Id> a = emails.getAccountFor(u.getEmail());
     if (a.size() == 1) {
       u.setAccount(a.iterator().next());
     }

gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java

@@ -425,6 +425,7 @@ public class MergeOp implements AutoCloseable {
    * @throws OrmException an error occurred reading or writing the database.
    * @throws RestApiException if an error occurred.
    * @throws PermissionBackendException if permissions can't be checked
+   * @throws IOException an error occurred reading from NoteDb.
    */
   public void merge(
       ReviewDb db,

gerrit-server/src/main/java/com/google/gerrit/server/git/MergedByPushOp.java

@@ -198,7 +198,7 @@ public class MergedByPushOp implements BatchUpdateOp {
         change, patchSet, ctx.getAccount(), patchSet.getRevision().get(), ctx.getWhen());
   }
 
-  private PatchSetInfo getPatchSetInfo(ChangeContext ctx) throws IOException {
+  private PatchSetInfo getPatchSetInfo(ChangeContext ctx) throws IOException, OrmException {
     RevWalk rw = ctx.getRevWalk();
     RevCommit commit =
         rw.parseCommit(ObjectId.fromString(checkNotNull(patchSet).getRevision().get()));

gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java

@@ -2546,7 +2546,7 @@ public class ReceiveCommits {
               RefNames.refsEdit(user.getAccountId(), notes.getChangeId(), psId));
     }
 
-    private void newPatchSet() throws IOException {
+    private void newPatchSet() throws IOException, OrmException {
       RevCommit newCommit = rp.getRevWalk().parseCommit(newCommitId);
       psId =
           ChangeUtil.nextPatchSetIdFromAllRefsMap(allRefs, notes.getChange().currentPatchSetId());

gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/CherryPick.java

@@ -89,7 +89,8 @@ public class CherryPick extends SubmitStrategy {
     }
 
     @Override
-    protected void updateRepoImpl(RepoContext ctx) throws IntegrationException, IOException {
+    protected void updateRepoImpl(RepoContext ctx)
+        throws IntegrationException, IOException, OrmException {
       // If there is only one parent, a cherry-pick can be done by taking the
       // delta relative to that one parent and redoing that on the current merge
       // tip.

gerrit-server/src/main/java/com/google/gerrit/server/mail/MailUtil.java

@@ -24,6 +24,7 @@ import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.ReviewerSet;
 import com.google.gerrit.server.account.AccountResolver;
 import com.google.gwtorm.server.OrmException;
+import java.io.IOException;
 import java.time.format.DateTimeFormatter;
 import java.util.Collections;
 import java.util.HashSet;
@@ -42,7 +43,7 @@ public class MailUtil {
       AccountResolver accountResolver,
       boolean draftPatchSet,
       List<FooterLine> footerLines)
-      throws OrmException {
+      throws OrmException, IOException {
     MailRecipients recipients = new MailRecipients();
     if (!draftPatchSet) {
       for (FooterLine footerLine : footerLines) {
@@ -70,7 +71,7 @@ public class MailUtil {
 
   private static Account.Id toAccountId(
       ReviewDb db, AccountResolver accountResolver, String nameOrEmail)
-      throws OrmException, NoSuchAccountException {
+      throws OrmException, NoSuchAccountException, IOException {
     Account a = accountResolver.findByNameOrEmail(db, nameOrEmail);
     if (a == null) {
       throw new NoSuchAccountException("\"" + nameOrEmail + "\" is not registered");

gerrit-server/src/main/java/com/google/gerrit/server/mail/receive/MailProcessor.java

@@ -36,8 +36,8 @@ import com.google.gerrit.server.ApprovalsUtil;
 import com.google.gerrit.server.ChangeMessagesUtil;
 import com.google.gerrit.server.CommentsUtil;
 import com.google.gerrit.server.PatchSetUtil;
-import com.google.gerrit.server.account.AccountByEmailCache;
 import com.google.gerrit.server.account.AccountCache;
+import com.google.gerrit.server.account.Emails;
 import com.google.gerrit.server.change.EmailReviewComments;
 import com.google.gerrit.server.config.CanonicalWebUrl;
 import com.google.gerrit.server.extensions.events.CommentAdded;
@@ -58,6 +58,7 @@ import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
 import com.google.inject.Singleton;
+import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.HashMap;
@@ -73,7 +74,7 @@ import org.slf4j.LoggerFactory;
 public class MailProcessor {
   private static final Logger log = LoggerFactory.getLogger(MailProcessor.class);
 
-  private final AccountByEmailCache accountByEmailCache;
+  private final Emails emails;
   private final RetryHelper retryHelper;
   private final ChangeMessagesUtil changeMessagesUtil;
   private final CommentsUtil commentsUtil;
@@ -90,7 +91,7 @@ public class MailProcessor {
 
   @Inject
   public MailProcessor(
-      AccountByEmailCache accountByEmailCache,
+      Emails emails,
       RetryHelper retryHelper,
       ChangeMessagesUtil changeMessagesUtil,
       CommentsUtil commentsUtil,
@@ -104,7 +105,7 @@ public class MailProcessor {
       CommentAdded commentAdded,
       AccountCache accountCache,
       @CanonicalWebUrl Provider<String> canonicalUrl) {
-    this.accountByEmailCache = accountByEmailCache;
+    this.emails = emails;
     this.retryHelper = retryHelper;
     this.changeMessagesUtil = changeMessagesUtil;
     this.commentsUtil = commentsUtil;
@@ -134,7 +135,7 @@ public class MailProcessor {
   }
 
   private void processImpl(BatchUpdate.Factory buf, MailMessage message)
-      throws OrmException, UpdateException, RestApiException {
+      throws OrmException, UpdateException, RestApiException, IOException {
     for (DynamicMap.Entry<MailFilter> filter : mailFilters) {
       if (!filter.getProvider().get().shouldProcessMessage(message)) {
         log.warn(
@@ -154,15 +155,15 @@ public class MailProcessor {
       return;
     }
 
-    Set<Account.Id> accounts = accountByEmailCache.get(metadata.author);
-    if (accounts.size() != 1) {
+    Set<Account.Id> accountIds = emails.getAccountFor(metadata.author);
+    if (accountIds.size() != 1) {
       log.error(
           String.format(
               "Address %s could not be matched to a unique account. It was matched to %s. Will delete message.",
-              metadata.author, accounts));
+              metadata.author, accountIds));
       return;
     }
-    Account.Id account = accounts.iterator().next();
+    Account.Id account = accountIds.iterator().next();
     if (!accountCache.get(account).getAccount().isActive()) {
       log.warn(String.format("Mail: Account %s is inactive. Will delete message.", account));
       return;

gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchSetInfoFactory.java

@@ -22,7 +22,7 @@ import com.google.gerrit.reviewdb.client.RevId;
 import com.google.gerrit.reviewdb.client.UserIdentity;
 import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.PatchSetUtil;
-import com.google.gerrit.server.account.AccountByEmailCache;
+import com.google.gerrit.server.account.Emails;
 import com.google.gerrit.server.git.GitRepositoryManager;
 import com.google.gerrit.server.notedb.ChangeNotes;
 import com.google.gwtorm.server.OrmException;
@@ -45,17 +45,17 @@ import org.eclipse.jgit.revwalk.RevWalk;
 public class PatchSetInfoFactory {
   private final GitRepositoryManager repoManager;
   private final PatchSetUtil psUtil;
-  private final AccountByEmailCache byEmailCache;
+  private final Emails emails;
 
   @Inject
-  public PatchSetInfoFactory(
-      GitRepositoryManager repoManager, PatchSetUtil psUtil, AccountByEmailCache byEmailCache) {
+  public PatchSetInfoFactory(GitRepositoryManager repoManager, PatchSetUtil psUtil, Emails emails) {
     this.repoManager = repoManager;
     this.psUtil = psUtil;
-    this.byEmailCache = byEmailCache;
+    this.emails = emails;
   }
 
-  public PatchSetInfo get(RevWalk rw, RevCommit src, PatchSet.Id psi) throws IOException {
+  public PatchSetInfo get(RevWalk rw, RevCommit src, PatchSet.Id psi)
+      throws IOException, OrmException {
     rw.parseBody(src);
     PatchSetInfo info = new PatchSetInfo(psi);
     info.setSubject(src.getShortMessage());
@@ -84,13 +84,13 @@ public class PatchSetInfoFactory {
       PatchSetInfo info = get(rw, src, patchSet.getId());
       info.setParents(toParentInfos(src.getParents(), rw));
       return info;
-    } catch (IOException e) {
+    } catch (IOException | OrmException e) {
       throw new PatchSetInfoNotAvailableException(e);
     }
   }
 
   // TODO: The same method exists in EventFactory, find a common place for it
-  private UserIdentity toUserIdentity(PersonIdent who) {
+  private UserIdentity toUserIdentity(PersonIdent who) throws IOException, OrmException {
     final UserIdentity u = new UserIdentity();
     u.setName(who.getName());
     u.setEmail(who.getEmailAddress());
@@ -100,7 +100,7 @@ public class PatchSetInfoFactory {
     // If only one account has access to this email address, select it
     // as the identity of the user.
     //
-    final Set<Account.Id> a = byEmailCache.get(u.getEmail());
+    Set<Account.Id> a = emails.getAccountFor(u.getEmail());
     if (a.size() == 1) {
       u.setAccount(a.iterator().next());
     }