opendev/gerrit
Code Issues Proposed changes

Fixes NPE when username not found and matches by external ID

Browse Source 
In Ia6561200 we changed to look up users in the cache by the username
from both the "gerrit:" and "user:" schemes. However, the username
cache only contains keys for the "user:" scheme, so we need to
double-check that there is actually a corresponding external ID in the
cached AccountState for the actual ID we're looking up. This was not
the case in e.g. LuceneQueryChangesTest, causing an NPE.

If the requested key does not match exactly, we fall back to fetching
the external ID directly from the database.

Change-Id: Id3b1b9b76d6ba415abee2051409ff0843caf1c85
This commit is contained in:
Luca Milanesio 2015-09-29 15:25:42 +01:00 committed by Dave Borowitz
parent f977e3d489
commit 7dadcbf091
1 changed files with 7 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

18
gerrit-server/src/main/java/com/google/gerrit/server/account/AccountManager.java
View File

@ -38,7 +38,6 @@ import org.slf4j.Logger;
import org.slf4j.LoggerFactory; import org.slf4j.LoggerFactory;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections; import java.util.Collections;
import java.util.List; import java.util.List;
import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicBoolean;
@ -141,18 +140,15 @@ public class AccountManager {
// without having to query the DB every time // without having to query the DB every time
if (keyScheme.equals(AccountExternalId.SCHEME_GERRIT) if (keyScheme.equals(AccountExternalId.SCHEME_GERRIT)
|| keyScheme.equals(AccountExternalId.SCHEME_USERNAME)) { || keyScheme.equals(AccountExternalId.SCHEME_USERNAME)) {
String username = keyValue.substring(keyScheme.length()); AccountState state = byIdCache.getByUsername(
Collection<AccountExternalId> externalIds = keyValue.substring(keyScheme.length()));
byIdCache.getByUsername(username).getExternalIds(); if (state != null) {
for (AccountExternalId accountExternalId : externalIds) { for (AccountExternalId accountExternalId : state.getExternalIds()) {
if (accountExternalId.isScheme(keyScheme)) { if (accountExternalId.getKey().equals(key)) {
return accountExternalId; return accountExternalId;
}
} }
} }
log.warn(
"Cannot find account external id for user {} in cache, possibly a stale entry",
username);
} }
return db.accountExternalIds().get(key); return db.accountExternalIds().get(key);
} }