AccountByEmailCacheImpl: Don't use account index to find accounts by email

Instead rather get accounts by email from either the database (if
external IDs are read from ReviewDb) or from the ExternalIdCache (if
external IDs are read from NoteDb).

For NoteDb Using the in-memory cache is faster than doing the lookup via
the account index.

To support querying accounts by email from ReviewDb the
account_external_ids_byEmail index is added back temporarily. The
follow-up change will drop the account_external_ids table completely and
hence this index will be gone again.

The ReviewDb method to query accounts by email is also needed to perform
the external ID migration on the https://*-review.googlesource.com
Gerrit servers at Google.

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

gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/server/AccountExternalIdAccess.java

@@ -30,6 +30,9 @@ public interface AccountExternalIdAccess extends Access<AccountExternalId, Accou
   @Query("WHERE accountId = ?")
   ResultSet<AccountExternalId> byAccount(Account.Id id) throws OrmException;
 
+  @Query("WHERE emailAddress = ?")
+  ResultSet<AccountExternalId> byEmailAddress(String email) throws OrmException;
+
   @Query
   ResultSet<AccountExternalId> all() throws OrmException;
 }

gerrit-reviewdb/src/main/resources/com/google/gerrit/reviewdb/server/index_generic.sql

@@ -21,6 +21,9 @@ ON accounts (full_name);
 CREATE INDEX account_external_ids_byAccount
 ON account_external_ids (account_id);
 
+--    covers:             byEmailAddress
+CREATE INDEX account_external_ids_byEmail
+ON account_external_ids (email_address);
 
 -- *********************************************************************
 -- AccountGroupMemberAccess

gerrit-reviewdb/src/main/resources/com/google/gerrit/reviewdb/server/index_maxdb.sql

@@ -25,6 +25,10 @@ CREATE INDEX account_external_ids_byAccount
 ON account_external_ids (account_id)
 #
 
+--    covers:             byEmailAddress
+CREATE INDEX account_external_ids_byEmail
+ON account_external_ids (email_address)
+#
 
 -- *********************************************************************
 -- AccountGroupMemberAccess

gerrit-reviewdb/src/main/resources/com/google/gerrit/reviewdb/server/index_postgres.sql

@@ -68,6 +68,10 @@ ON accounts (full_name);
 CREATE INDEX account_external_ids_byAccount
 ON account_external_ids (account_id);
 
+--    covers:             byEmailAddress
+CREATE INDEX account_external_ids_byEmail
+ON account_external_ids (email_address);
+
 
 -- *********************************************************************
 -- AccountGroupMemberAccess

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

@@ -14,13 +14,15 @@
 
 package com.google.gerrit.server.account;
 
+import static com.google.common.collect.ImmutableSet.toImmutableSet;
+
 import com.google.common.cache.CacheLoader;
 import com.google.common.cache.LoadingCache;
-import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Streams;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gerrit.server.account.externalids.ExternalIds;
 import com.google.gerrit.server.cache.CacheModule;
-import com.google.gerrit.server.query.account.InternalAccountQuery;
 import com.google.gwtorm.server.SchemaFactory;
 import com.google.inject.Inject;
 import com.google.inject.Module;
@@ -29,7 +31,6 @@ import com.google.inject.Singleton;
 import com.google.inject.TypeLiteral;
 import com.google.inject.name.Named;
 import java.util.Collections;
-import java.util.HashSet;
 import java.util.Set;
 import java.util.concurrent.ExecutionException;
 import org.slf4j.Logger;
@@ -78,32 +79,23 @@ public class AccountByEmailCacheImpl implements AccountByEmailCache {
 
   static class Loader extends CacheLoader<String, Set<Account.Id>> {
     private final SchemaFactory<ReviewDb> schema;
-    private final Provider<InternalAccountQuery> accountQueryProvider;
+
+    // This must be a provider to prevent a cyclic dependency within Google-internal glue code.
+    private final Provider<ExternalIds> externalIds;
 
     @Inject
-    Loader(SchemaFactory<ReviewDb> schema, Provider<InternalAccountQuery> accountQueryProvider) {
+    Loader(SchemaFactory<ReviewDb> schema, Provider<ExternalIds> externalIds) {
       this.schema = schema;
-      this.accountQueryProvider = accountQueryProvider;
+      this.externalIds = externalIds;
     }
 
     @Override
     public Set<Account.Id> load(String email) throws Exception {
       try (ReviewDb db = schema.open()) {
-        Set<Account.Id> r = new HashSet<>();
-        for (Account a : db.accounts().byPreferredEmail(email)) {
-          r.add(a.getId());
-        }
-        for (AccountState accountState : accountQueryProvider.get().byEmailPrefix(email)) {
-          if (accountState
-              .getExternalIds()
-              .stream()
-              .filter(e -> email.equals(e.email()))
-              .findAny()
-              .isPresent()) {
-            r.add(accountState.getAccount().getId());
-          }
-        }
-        return ImmutableSet.copyOf(r);
+        return Streams.concat(
+                Streams.stream(db.accounts().byPreferredEmail(email)).map(a -> a.getId()),
+                externalIds.get().byEmail(db, email).stream().map(e -> e.accountId()))
+            .collect(toImmutableSet());
       }
     }
   }

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

@@ -17,6 +17,7 @@ package com.google.gerrit.server.account.externalids;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.inject.AbstractModule;
 import com.google.inject.Module;
+import java.io.IOException;
 import java.util.Set;
 import org.eclipse.jgit.lib.ObjectId;
 
@@ -73,4 +74,9 @@ public class DisabledExternalIdCache implements ExternalIdCache {
   public Set<ExternalId> byAccount(Account.Id accountId) {
     throw new UnsupportedOperationException();
   }
+
+  @Override
+  public Set<ExternalId> byEmail(String email) throws IOException {
+    throw new UnsupportedOperationException();
+  }
 }

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

@@ -57,6 +57,8 @@ interface ExternalIdCache {
 
   Set<ExternalId> byAccount(Account.Id accountId) throws IOException;
 
+  Set<ExternalId> byEmail(String email) throws IOException;
+
   default void onCreate(ObjectId newNotesRev, ExternalId extId) throws IOException {
     onCreate(newNotesRev, Collections.singleton(extId));
   }

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

@@ -14,6 +14,8 @@
 
 package com.google.gerrit.server.account.externalids;
 
+import static java.util.stream.Collectors.toSet;
+
 import com.google.common.cache.CacheLoader;
 import com.google.common.cache.LoadingCache;
 import com.google.common.collect.ImmutableSetMultimap;
@@ -220,7 +222,22 @@ class ExternalIdCacheImpl implements ExternalIdCache {
     try {
       return extIdsByAccount.get(externalIdReader.readRevision()).get(accountId);
     } catch (ExecutionException e) {
-      log.warn("Cannot list external ids", e);
+      log.warn("Cannot list external ids by account", e);
+      return Collections.emptySet();
+    }
+  }
+
+  @Override
+  public Set<ExternalId> byEmail(String email) throws IOException {
+    try {
+      return extIdsByAccount
+          .get(externalIdReader.readRevision())
+          .values()
+          .stream()
+          .filter(e -> email.equals(e.email()))
+          .collect(toSet());
+    } catch (ExecutionException e) {
+      log.warn("Cannot list external ids by email", e);
       return Collections.emptySet();
     }
   }

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

@@ -82,4 +82,12 @@ public class ExternalIds {
       throws IOException, OrmException {
     return byAccount(db, accountId).stream().filter(e -> e.key().isScheme(scheme)).collect(toSet());
   }
+
+  public Set<ExternalId> byEmail(ReviewDb db, String email) throws IOException, OrmException {
+    if (externalIdReader.readFromGit()) {
+      return externalIdCache.byEmail(email);
+    }
+
+    return ExternalId.from(db.accountExternalIds().byEmailAddress(email).toList());
+  }
 }

gerrit-server/src/main/java/com/google/gerrit/server/query/account/InternalAccountQuery.java

@@ -68,10 +68,6 @@ public class InternalAccountQuery extends InternalQuery<AccountState> {
     return query(AccountPredicates.defaultPredicate(query));
   }
 
-  public List<AccountState> byEmailPrefix(String emailPrefix) throws OrmException {
-    return query(AccountPredicates.email(emailPrefix));
-  }
-
   public List<AccountState> byExternalId(String scheme, String id) throws OrmException {
     return byExternalId(ExternalId.Key.create(scheme, id));
   }

gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaVersion.java

@@ -35,7 +35,7 @@ import java.util.concurrent.TimeUnit;
 /** A version of the database schema. */
 public abstract class SchemaVersion {
   /** The current schema version. */
-  public static final Class<Schema_144> C = Schema_144.class;
+  public static final Class<Schema_145> C = Schema_145.class;
 
   public static int getBinaryVersion() {
     return guessVersion(C);

gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_145.java

@@ -0,0 +1,46 @@
+// Copyright (C) 2017 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.server.schema;
+
+import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gwtorm.jdbc.JdbcSchema;
+import com.google.gwtorm.schema.sql.SqlDialect;
+import com.google.gwtorm.server.OrmException;
+import com.google.gwtorm.server.StatementExecutor;
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+import java.sql.SQLException;
+
+/** Create account_external_ids_byEmail index. */
+public class Schema_145 extends SchemaVersion {
+
+  @Inject
+  Schema_145(Provider<Schema_144> prior) {
+    super(prior);
+  }
+
+  @Override
+  protected void migrateData(ReviewDb db, UpdateUI ui) throws OrmException, SQLException {
+    JdbcSchema schema = (JdbcSchema) db;
+    SqlDialect dialect = schema.getDialect();
+    try (StatementExecutor e = newExecutor(db)) {
+      dialect.dropIndex(e, "account_external_ids", "account_external_ids_byEmail");
+      e.execute(
+          "CREATE INDEX account_external_ids_byEmail"
+              + " ON account_external_ids"
+              + " (email_address)");
+    }
+  }
+}