Merge changes I6984e3c9,Id5689dc5,Ie25b44c5,I51dcca79,I23b9f69c

* changes:
  Don't index missing accounts
  AccountCache: Remove unused getIfPresent
  ExternalIdsConsistencyChecker: Use AccountCache#getOrNull to check if account exists
  AccountCache: Add getOrNull method that returns null for missing account
  AccountCache: Document get and getIfPresent methods

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

diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountCache.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountCache.java
index c28021c..0a154c9b 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountCache.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountCache.java
@@ -14,14 +14,33 @@
 
 package com.google.gerrit.server.account;
 
+import com.google.gerrit.common.Nullable;
 import com.google.gerrit.reviewdb.client.Account;
 import java.io.IOException;
 
 /** Caches important (but small) account state to avoid database hits. */
 public interface AccountCache {
+  /**
+   * Returns an {@code AccountState} instance for the given account ID. If not cached yet the
+   * account is loaded. Returns an empty {@code AccountState} instance to represent a missing
+   * account.
+   *
+   * @param accountId ID of the account that should be retrieved
+   * @return {@code AccountState} instance for the given account ID, if no account with this ID
+   *     exists an empty {@code AccountState} instance is returned to represent the missing account
+   */
   AccountState get(Account.Id accountId);
 
-  AccountState getIfPresent(Account.Id accountId);
+  /**
+   * Returns an {@code AccountState} instance for the given account ID. If not cached yet the
+   * account is loaded. Returns {@code null} if the account is missing.
+   *
+   * @param accountId ID of the account that should be retrieved
+   * @return {@code AccountState} instance for the given account ID, if no account with this ID
+   *     exists {@code null} is returned
+   */
+  @Nullable
+  AccountState getOrNull(Account.Id accountId);
 
   AccountState getByUsername(String username);
 

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

diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountCacheImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountCacheImpl.java
index 866a423..12fc938 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountCacheImpl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountCacheImpl.java
@@ -19,6 +19,7 @@
 import com.google.common.cache.CacheLoader;
 import com.google.common.cache.LoadingCache;
 import com.google.common.collect.ImmutableSet;
+import com.google.gerrit.common.Nullable;
 import com.google.gerrit.common.TimeUtil;
 import com.google.gerrit.extensions.client.GeneralPreferencesInfo;
 import com.google.gerrit.reviewdb.client.Account;
@@ -62,7 +63,8 @@
     return new CacheModule() {
       @Override
       protected void configure() {
-        cache(BYID_NAME, Account.Id.class, AccountState.class).loader(ByIdLoader.class);
+        cache(BYID_NAME, Account.Id.class, new TypeLiteral<Optional<AccountState>>() {})
+            .loader(ByIdLoader.class);
 
         cache(BYUSER_NAME, String.class, new TypeLiteral<Optional<Account.Id>>() {})
             .loader(ByNameLoader.class);
@@ -73,13 +75,13 @@
     };
   }
 
-  private final LoadingCache<Account.Id, AccountState> byId;
+  private final LoadingCache<Account.Id, Optional<AccountState>> byId;
   private final LoadingCache<String, Optional<Account.Id>> byName;
   private final Provider<AccountIndexer> indexer;
 
   @Inject
   AccountCacheImpl(
-      @Named(BYID_NAME) LoadingCache<Account.Id, AccountState> byId,
+      @Named(BYID_NAME) LoadingCache<Account.Id, Optional<AccountState>> byId,
       @Named(BYUSER_NAME) LoadingCache<String, Optional<Account.Id>> byUsername,
       Provider<AccountIndexer> indexer) {
     this.byId = byId;
@@ -90,7 +92,7 @@
   @Override
   public AccountState get(Account.Id accountId) {
     try {
-      return byId.get(accountId);
+      return byId.get(accountId).orElse(missing(accountId));
     } catch (ExecutionException e) {
       log.warn("Cannot load AccountState for " + accountId, e);
       return missing(accountId);
@@ -98,15 +100,21 @@
   }
 
   @Override
-  public AccountState getIfPresent(Account.Id accountId) {
-    return byId.getIfPresent(accountId);
+  @Nullable
+  public AccountState getOrNull(Account.Id accountId) {
+    try {
+      return byId.get(accountId).orElse(null);
+    } catch (ExecutionException e) {
+      log.warn("Cannot load AccountState for " + accountId, e);
+      return null;
+    }
   }
 
   @Override
   public AccountState getByUsername(String username) {
     try {
       Optional<Account.Id> id = byName.get(username);
-      return id != null && id.isPresent() ? byId.get(id.get()) : null;
+      return id != null && id.isPresent() ? getOrNull(id.get()) : null;
     } catch (ExecutionException e) {
       log.warn("Cannot load AccountState for " + username, e);
       return null;
@@ -144,7 +152,7 @@
         account, anon, Collections.emptySet(), new HashMap<ProjectWatchKey, Set<NotifyType>>());
   }
 
-  static class ByIdLoader extends CacheLoader<Account.Id, AccountState> {
+  static class ByIdLoader extends CacheLoader<Account.Id, Optional<AccountState>> {
     private final SchemaFactory<ReviewDb> schema;
     private final GroupCache groupCache;
     private final GeneralPreferencesLoader loader;
@@ -169,23 +177,25 @@
     }
 
     @Override
-    public AccountState load(Account.Id key) throws Exception {
+    public Optional<AccountState> load(Account.Id key) throws Exception {
       try (ReviewDb db = schema.open()) {
-        final AccountState state = load(db, key);
-        String user = state.getUserName();
+        Optional<AccountState> state = load(db, key);
+        if (!state.isPresent()) {
+          return state;
+        }
+        String user = state.get().getUserName();
         if (user != null) {
-          byName.put(user, Optional.of(state.getAccount().getId()));
+          byName.put(user, Optional.of(state.get().getAccount().getId()));
         }
         return state;
       }
     }
 
-    private AccountState load(final ReviewDb db, final Account.Id who)
+    private Optional<AccountState> load(final ReviewDb db, final Account.Id who)
         throws OrmException, IOException, ConfigInvalidException {
       Account account = db.accounts().get(who);
       if (account == null) {
-        // Account no longer exists? They are anonymous.
-        return missing(who);
+        return Optional.empty();
       }
 
       Set<AccountGroup.UUID> internalGroups = new HashSet<>();
@@ -205,11 +215,12 @@
         account.setGeneralPreferences(GeneralPreferencesInfo.defaults());
       }
 
-      return new AccountState(
-          account,
-          internalGroups,
-          externalIds.byAccount(who),
-          watchConfig.get().getProjectWatches(who));
+      return Optional.of(
+          new AccountState(
+              account,
+              internalGroups,
+              externalIds.byAccount(who),
+              watchConfig.get().getProjectWatches(who)));
     }
   }
 

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

diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/externalids/ExternalIdsConsistencyChecker.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/externalids/ExternalIdsConsistencyChecker.java
index 928349d..f74210f 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/externalids/ExternalIdsConsistencyChecker.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/externalids/ExternalIdsConsistencyChecker.java
@@ -120,7 +120,7 @@
   private List<ConsistencyProblemInfo> validateExternalId(ExternalId extId) {
     List<ConsistencyProblemInfo> problems = new ArrayList<>();
 
-    if (accountCache.getIfPresent(extId.accountId()) == null) {
+    if (accountCache.getOrNull(extId.accountId()) == null) {
       addError(
           String.format(
               "External ID '%s' belongs to account that doesn't exist: %s",

gerrit-server/src/main/java/com/google/gerrit/server/index/account/AccountIndexerImpl.java

diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/index/account/AccountIndexerImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/index/account/AccountIndexerImpl.java
index 58dbd34..8796360 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/index/account/AccountIndexerImpl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/index/account/AccountIndexerImpl.java
@@ -64,8 +64,13 @@
 
   @Override
   public void index(Account.Id id) throws IOException {
-    for (Index<?, AccountState> i : getWriteIndexes()) {
-      i.replace(byIdCache.get(id));
+    for (Index<Account.Id, AccountState> i : getWriteIndexes()) {
+      AccountState accountState = byIdCache.getOrNull(id);
+      if (accountState != null) {
+        i.replace(accountState);
+      } else {
+        i.delete(id);
+      }
     }
     fireAccountIndexedEvent(id.get());
   }

gerrit-server/src/test/java/com/google/gerrit/testutil/FakeAccountCache.java

diff --git a/gerrit-server/src/test/java/com/google/gerrit/testutil/FakeAccountCache.java b/gerrit-server/src/test/java/com/google/gerrit/testutil/FakeAccountCache.java
index d81d441..066b7b1 100644
--- a/gerrit-server/src/test/java/com/google/gerrit/testutil/FakeAccountCache.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/testutil/FakeAccountCache.java
@@ -15,6 +15,7 @@
 package com.google.gerrit.testutil;
 
 import com.google.common.collect.ImmutableSet;
+import com.google.gerrit.common.Nullable;
 import com.google.gerrit.common.TimeUtil;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.server.account.AccountCache;
@@ -34,7 +35,7 @@
 
   @Override
   public synchronized AccountState get(Account.Id accountId) {
-    AccountState state = getIfPresent(accountId);
+    AccountState state = byId.get(accountId);
     if (state != null) {
       return state;
     }
@@ -42,7 +43,8 @@
   }
 
   @Override
-  public synchronized AccountState getIfPresent(Account.Id accountId) {
+  @Nullable
+  public synchronized AccountState getOrNull(Account.Id accountId) {
     return byId.get(accountId);
   }