diff --git a/java/com/google/gerrit/elasticsearch/ElasticAccountIndex.java b/java/com/google/gerrit/elasticsearch/ElasticAccountIndex.java index ba5178e596..1bcda730b7 100644 --- a/java/com/google/gerrit/elasticsearch/ElasticAccountIndex.java +++ b/java/com/google/gerrit/elasticsearch/ElasticAccountIndex.java @@ -123,10 +123,11 @@ public class ElasticAccountIndex extends AbstractElasticIndex from the state() + // method and let callers handle the missing account case explicitly. But this would be a lot + // of work too. + state = accountCache.getEvenIfMissing(getAccountId()); } return state; } @@ -310,6 +329,11 @@ public class IdentifiedUser extends CurrentUser { () -> firstNonNull(getAccount().getPreferredEmail(), "a/" + getAccountId().get())); } + /** + * Returns the account of the identified user. + * + * @return the account of the identified user, an empty account if the account is missing + */ public Account getAccount() { return state().getAccount(); } diff --git a/java/com/google/gerrit/server/account/AccountCache.java b/java/com/google/gerrit/server/account/AccountCache.java index 73bb8e19ab..5e5ead7d37 100644 --- a/java/com/google/gerrit/server/account/AccountCache.java +++ b/java/com/google/gerrit/server/account/AccountCache.java @@ -21,26 +21,31 @@ import java.util.Optional; /** 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 {@link Optional#empty()} 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 {@link Optional#empty()} is returned + */ + Optional maybeGet(Account.Id accountId); + /** * 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. * + *

This method should only be used in exceptional cases where it is required to get an account + * state even if the account is missing. Callers should leave a comment with the method invocation + * explaining why this method is used. Most callers of {@link AccountCache} should use {@link + * #maybeGet(Account.Id)} instead and handle the missing account case explicitly. + * * @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); - - /** - * Returns an {@code AccountState} instance for the given account ID. If not cached yet the - * account is loaded. Returns {@link Optional#empty()} 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 {@link Optional#empty()}is returned - */ - Optional maybeGet(Account.Id accountId); + AccountState getEvenIfMissing(Account.Id accountId); /** * Returns an {@code AccountState} instance for the given username. diff --git a/java/com/google/gerrit/server/account/AccountCacheImpl.java b/java/com/google/gerrit/server/account/AccountCacheImpl.java index b383cb6541..747a0a4f09 100644 --- a/java/com/google/gerrit/server/account/AccountCacheImpl.java +++ b/java/com/google/gerrit/server/account/AccountCacheImpl.java @@ -77,7 +77,7 @@ public class AccountCacheImpl implements AccountCache { } @Override - public AccountState get(Account.Id accountId) { + public AccountState getEvenIfMissing(Account.Id accountId) { try { return byId.get(accountId).orElse(missing(accountId)); } catch (ExecutionException e) { diff --git a/java/com/google/gerrit/server/account/AccountState.java b/java/com/google/gerrit/server/account/AccountState.java index 34f4eb14ce..356033351e 100644 --- a/java/com/google/gerrit/server/account/AccountState.java +++ b/java/com/google/gerrit/server/account/AccountState.java @@ -51,7 +51,7 @@ import org.slf4j.LoggerFactory; * and properties from the account config file. AccountState maps one-to-one to Account. * *

Most callers should not construct AccountStates directly but rather lookup accounts via the - * account cache (see {@link AccountCache#get(Account.Id)}). + * account cache (see {@link AccountCache#maybeGet(Account.Id)}). */ public class AccountState { private static final Logger logger = LoggerFactory.getLogger(AccountState.class); diff --git a/java/com/google/gerrit/testing/FakeAccountCache.java b/java/com/google/gerrit/testing/FakeAccountCache.java index c31275335c..808e955ab4 100644 --- a/java/com/google/gerrit/testing/FakeAccountCache.java +++ b/java/com/google/gerrit/testing/FakeAccountCache.java @@ -34,7 +34,7 @@ public class FakeAccountCache implements AccountCache { } @Override - public synchronized AccountState get(Account.Id accountId) { + public synchronized AccountState getEvenIfMissing(Account.Id accountId) { AccountState state = byId.get(accountId); if (state != null) { return state;