Merge "Revert external ID cache"

This commit is contained in:
ekempin
2017-02-02 16:32:22 +00:00
committed by Gerrit Code Review
22 changed files with 93 additions and 418 deletions

View File

@@ -26,7 +26,6 @@ import com.google.gerrit.reviewdb.client.AccountGroupMember;
import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.account.AccountByEmailCache; import com.google.gerrit.server.account.AccountByEmailCache;
import com.google.gerrit.server.account.AccountCache; import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.account.ExternalIdCache;
import com.google.gerrit.server.account.GroupCache; import com.google.gerrit.server.account.GroupCache;
import com.google.gerrit.server.account.VersionedAuthorizedKeys; import com.google.gerrit.server.account.VersionedAuthorizedKeys;
import com.google.gerrit.server.index.account.AccountIndexer; import com.google.gerrit.server.index.account.AccountIndexer;
@@ -57,7 +56,6 @@ public class AccountCreator {
private final AccountCache accountCache; private final AccountCache accountCache;
private final AccountByEmailCache byEmailCache; private final AccountByEmailCache byEmailCache;
private final AccountIndexer indexer; private final AccountIndexer indexer;
private final ExternalIdCache externalIdCache;
@Inject @Inject
AccountCreator(SchemaFactory<ReviewDb> schema, AccountCreator(SchemaFactory<ReviewDb> schema,
@@ -66,8 +64,7 @@ public class AccountCreator {
SshKeyCache sshKeyCache, SshKeyCache sshKeyCache,
AccountCache accountCache, AccountCache accountCache,
AccountByEmailCache byEmailCache, AccountByEmailCache byEmailCache,
AccountIndexer indexer, AccountIndexer indexer) {
ExternalIdCache externalIdCache) {
accounts = new HashMap<>(); accounts = new HashMap<>();
reviewDbProvider = schema; reviewDbProvider = schema;
this.authorizedKeys = authorizedKeys; this.authorizedKeys = authorizedKeys;
@@ -76,7 +73,6 @@ public class AccountCreator {
this.accountCache = accountCache; this.accountCache = accountCache;
this.byEmailCache = byEmailCache; this.byEmailCache = byEmailCache;
this.indexer = indexer; this.indexer = indexer;
this.externalIdCache = externalIdCache;
} }
public synchronized TestAccount create(String username, String email, public synchronized TestAccount create(String username, String email,
@@ -94,13 +90,11 @@ public class AccountCreator {
String httpPass = "http-pass"; String httpPass = "http-pass";
extUser.setPassword(httpPass); extUser.setPassword(httpPass);
db.accountExternalIds().insert(Collections.singleton(extUser)); db.accountExternalIds().insert(Collections.singleton(extUser));
externalIdCache.onCreate(extUser);
if (email != null) { if (email != null) {
AccountExternalId extMailto = new AccountExternalId(id, getEmailKey(email)); AccountExternalId extMailto = new AccountExternalId(id, getEmailKey(email));
extMailto.setEmailAddress(email); extMailto.setEmailAddress(email);
db.accountExternalIds().insert(Collections.singleton(extMailto)); db.accountExternalIds().insert(Collections.singleton(extMailto));
externalIdCache.onCreate(extMailto);
} }
Account a = new Account(id, TimeUtil.nowTs()); Account a = new Account(id, TimeUtil.nowTs());

View File

@@ -61,7 +61,6 @@ import com.google.gerrit.gpg.testutil.TestKey;
import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.AccountExternalId; import com.google.gerrit.reviewdb.client.AccountExternalId;
import com.google.gerrit.reviewdb.client.RefNames; import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.server.account.ExternalIdCache;
import com.google.gerrit.server.account.WatchConfig; import com.google.gerrit.server.account.WatchConfig;
import com.google.gerrit.server.account.WatchConfig.NotifyType; import com.google.gerrit.server.account.WatchConfig.NotifyType;
import com.google.gerrit.server.config.AllUsersName; import com.google.gerrit.server.config.AllUsersName;
@@ -113,9 +112,6 @@ public class AccountIT extends AbstractDaemonTest {
@Inject @Inject
private AllUsersName allUsers; private AllUsersName allUsers;
@Inject
private ExternalIdCache externalIdCache;
private List<AccountExternalId> savedExternalIds; private List<AccountExternalId> savedExternalIds;
@Before @Before
@@ -131,18 +127,10 @@ public class AccountIT extends AbstractDaemonTest {
// savedExternalIds is null when we don't run SSH tests and the assume in // savedExternalIds is null when we don't run SSH tests and the assume in
// @Before in AbstractDaemonTest prevents this class' @Before method from // @Before in AbstractDaemonTest prevents this class' @Before method from
// being executed. // being executed.
Collection<AccountExternalId> adminExtIds = getExternalIds(admin); db.accountExternalIds().delete(getExternalIds(admin));
db.accountExternalIds().delete(adminExtIds); db.accountExternalIds().delete(getExternalIds(user));
externalIdCache.onRemove(adminExtIds);
Collection<AccountExternalId> userExtIds = getExternalIds(user);
db.accountExternalIds().delete(userExtIds);
externalIdCache.onRemove(userExtIds);
db.accountExternalIds().insert(savedExternalIds); db.accountExternalIds().insert(savedExternalIds);
externalIdCache.onCreate(savedExternalIds);
} }
accountCache.evict(admin.getId()); accountCache.evict(admin.getId());
accountCache.evict(user.getId()); accountCache.evict(user.getId());
} }
@@ -641,7 +629,6 @@ public class AccountIT extends AbstractDaemonTest {
user.getId(), new AccountExternalId.Key("foo:myId")); user.getId(), new AccountExternalId.Key("foo:myId"));
db.accountExternalIds().insert(Collections.singleton(extId)); db.accountExternalIds().insert(Collections.singleton(extId));
externalIdCache.onCreate(extId);
accountCache.evict(user.getId()); accountCache.evict(user.getId());
TestKey key = validKeyWithSecondUserId(); TestKey key = validKeyWithSecondUserId();
@@ -838,9 +825,8 @@ public class AccountIT extends AbstractDaemonTest {
Account.Id currAccountId = atrScope.get().getUser().getAccountId(); Account.Id currAccountId = atrScope.get().getUser().getAccountId();
Iterable<String> expectedFps = expected.transform( Iterable<String> expectedFps = expected.transform(
k -> BaseEncoding.base16().encode(k.getPublicKey().getFingerprint())); k -> BaseEncoding.base16().encode(k.getPublicKey().getFingerprint()));
Iterable<String> actualFps = Iterable<String> actualFps = GpgKeys.getGpgExtIds(db, currAccountId)
GpgKeys.getGpgExtIds(externalIdCache, currAccountId) .transform(AccountExternalId::getSchemeRest);
.transform(AccountExternalId::getSchemeRest);
assertThat(actualFps) assertThat(actualFps)
.named("external IDs in database") .named("external IDs in database")
.containsExactlyElementsIn(expectedFps); .containsExactlyElementsIn(expectedFps);
@@ -873,7 +859,6 @@ public class AccountIT extends AbstractDaemonTest {
account.getId(), new AccountExternalId.Key(name("test"), email)); account.getId(), new AccountExternalId.Key(name("test"), email));
extId.setEmailAddress(email); extId.setEmailAddress(email);
db.accountExternalIds().insert(Collections.singleton(extId)); db.accountExternalIds().insert(Collections.singleton(extId));
externalIdCache.onCreate(extId);
// Clear saved AccountState and AccountExternalIds. // Clear saved AccountState and AccountExternalIds.
accountCache.evict(account.getId()); accountCache.evict(account.getId());
setApiUser(account); setApiUser(account);

View File

@@ -26,7 +26,6 @@ import com.google.gerrit.reviewdb.client.AccountExternalId;
import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.GerritPersonIdent; import com.google.gerrit.server.GerritPersonIdent;
import com.google.gerrit.server.account.AccountCache; import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.account.ExternalIdCache;
import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject; import com.google.inject.Inject;
import com.google.inject.Provider; import com.google.inject.Provider;
@@ -48,19 +47,16 @@ public class DeleteGpgKey implements RestModifyView<GpgKey, Input> {
private final Provider<ReviewDb> db; private final Provider<ReviewDb> db;
private final Provider<PublicKeyStore> storeProvider; private final Provider<PublicKeyStore> storeProvider;
private final AccountCache accountCache; private final AccountCache accountCache;
private final ExternalIdCache externalIdCache;
@Inject @Inject
DeleteGpgKey(@GerritPersonIdent Provider<PersonIdent> serverIdent, DeleteGpgKey(@GerritPersonIdent Provider<PersonIdent> serverIdent,
Provider<ReviewDb> db, Provider<ReviewDb> db,
Provider<PublicKeyStore> storeProvider, Provider<PublicKeyStore> storeProvider,
AccountCache accountCache, AccountCache accountCache) {
ExternalIdCache externalIdCache) {
this.serverIdent = serverIdent; this.serverIdent = serverIdent;
this.db = db; this.db = db;
this.storeProvider = storeProvider; this.storeProvider = storeProvider;
this.accountCache = accountCache; this.accountCache = accountCache;
this.externalIdCache = externalIdCache;
} }
@Override @Override
@@ -72,7 +68,6 @@ public class DeleteGpgKey implements RestModifyView<GpgKey, Input> {
AccountExternalId.SCHEME_GPGKEY, AccountExternalId.SCHEME_GPGKEY,
BaseEncoding.base16().encode(key.getFingerprint())); BaseEncoding.base16().encode(key.getFingerprint()));
db.get().accountExternalIds().deleteKeys(Collections.singleton(extIdKey)); db.get().accountExternalIds().deleteKeys(Collections.singleton(extIdKey));
externalIdCache.onRemove(rsrc.getUser().getAccountId(), extIdKey);
accountCache.evict(rsrc.getUser().getAccountId()); accountCache.evict(rsrc.getUser().getAccountId());
try (PublicKeyStore store = storeProvider.get()) { try (PublicKeyStore store = storeProvider.get()) {

View File

@@ -38,9 +38,9 @@ import com.google.gerrit.gpg.PublicKeyChecker;
import com.google.gerrit.gpg.PublicKeyStore; import com.google.gerrit.gpg.PublicKeyStore;
import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.AccountExternalId; import com.google.gerrit.reviewdb.client.AccountExternalId;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.CurrentUser; import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.account.AccountResource; import com.google.gerrit.server.account.AccountResource;
import com.google.gerrit.server.account.ExternalIdCache;
import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject; import com.google.inject.Inject;
import com.google.inject.Provider; import com.google.inject.Provider;
@@ -69,22 +69,22 @@ public class GpgKeys implements
public static String MIME_TYPE = "application/pgp-keys"; public static String MIME_TYPE = "application/pgp-keys";
private final DynamicMap<RestView<GpgKey>> views; private final DynamicMap<RestView<GpgKey>> views;
private final Provider<ReviewDb> db;
private final Provider<CurrentUser> self; private final Provider<CurrentUser> self;
private final Provider<PublicKeyStore> storeProvider; private final Provider<PublicKeyStore> storeProvider;
private final GerritPublicKeyChecker.Factory checkerFactory; private final GerritPublicKeyChecker.Factory checkerFactory;
private final ExternalIdCache externalIdCache;
@Inject @Inject
GpgKeys(DynamicMap<RestView<GpgKey>> views, GpgKeys(DynamicMap<RestView<GpgKey>> views,
Provider<ReviewDb> db,
Provider<CurrentUser> self, Provider<CurrentUser> self,
Provider<PublicKeyStore> storeProvider, Provider<PublicKeyStore> storeProvider,
GerritPublicKeyChecker.Factory checkerFactory, GerritPublicKeyChecker.Factory checkerFactory) {
ExternalIdCache externalIdCache) {
this.views = views; this.views = views;
this.db = db;
this.self = self; this.self = self;
this.storeProvider = storeProvider; this.storeProvider = storeProvider;
this.checkerFactory = checkerFactory; this.checkerFactory = checkerFactory;
this.externalIdCache = externalIdCache;
} }
@Override @Override
@@ -208,14 +208,15 @@ public class GpgKeys implements
} }
@VisibleForTesting @VisibleForTesting
public static FluentIterable<AccountExternalId> getGpgExtIds( public static FluentIterable<AccountExternalId> getGpgExtIds(ReviewDb db,
ExternalIdCache externalIdCache, Account.Id accountId) { Account.Id accountId) throws OrmException {
return FluentIterable.from( return FluentIterable.from(db.accountExternalIds().byAccount(accountId))
externalIdCache.byAccount(accountId, SCHEME_GPGKEY)); .filter(in -> in.isScheme(SCHEME_GPGKEY));
} }
private Iterable<AccountExternalId> getGpgExtIds(AccountResource rsrc) { private Iterable<AccountExternalId> getGpgExtIds(AccountResource rsrc)
return getGpgExtIds(externalIdCache, rsrc.getUser().getAccountId()); throws OrmException {
return getGpgExtIds(db.get(), rsrc.getUser().getAccountId());
} }
private static long keyId(byte[] fp) { private static long keyId(byte[] fp) {

View File

@@ -17,11 +17,11 @@ package com.google.gerrit.gpg.server;
import static com.google.gerrit.gpg.PublicKeyStore.keyIdToString; import static com.google.gerrit.gpg.PublicKeyStore.keyIdToString;
import static com.google.gerrit.gpg.PublicKeyStore.keyToString; import static com.google.gerrit.gpg.PublicKeyStore.keyToString;
import static java.nio.charset.StandardCharsets.UTF_8; import static java.nio.charset.StandardCharsets.UTF_8;
import static java.util.stream.Collectors.toList;
import com.google.common.base.Joiner; import com.google.common.base.Joiner;
import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import com.google.common.collect.Lists; import com.google.common.collect.Lists;
import com.google.common.collect.Maps; import com.google.common.collect.Maps;
import com.google.common.collect.Sets; import com.google.common.collect.Sets;
@@ -47,7 +47,6 @@ import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.account.AccountCache; import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.account.AccountResource; import com.google.gerrit.server.account.AccountResource;
import com.google.gerrit.server.account.AccountState; import com.google.gerrit.server.account.AccountState;
import com.google.gerrit.server.account.ExternalIdCache;
import com.google.gerrit.server.mail.send.AddKeySender; import com.google.gerrit.server.mail.send.AddKeySender;
import com.google.gerrit.server.query.account.InternalAccountQuery; import com.google.gerrit.server.query.account.InternalAccountQuery;
import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.OrmException;
@@ -91,7 +90,6 @@ public class PostGpgKeys implements RestModifyView<AccountResource, Input> {
private final AddKeySender.Factory addKeyFactory; private final AddKeySender.Factory addKeyFactory;
private final AccountCache accountCache; private final AccountCache accountCache;
private final Provider<InternalAccountQuery> accountQueryProvider; private final Provider<InternalAccountQuery> accountQueryProvider;
private final ExternalIdCache externalIdCache;
@Inject @Inject
PostGpgKeys(@GerritPersonIdent Provider<PersonIdent> serverIdent, PostGpgKeys(@GerritPersonIdent Provider<PersonIdent> serverIdent,
@@ -101,8 +99,7 @@ public class PostGpgKeys implements RestModifyView<AccountResource, Input> {
GerritPublicKeyChecker.Factory checkerFactory, GerritPublicKeyChecker.Factory checkerFactory,
AddKeySender.Factory addKeyFactory, AddKeySender.Factory addKeyFactory,
AccountCache accountCache, AccountCache accountCache,
Provider<InternalAccountQuery> accountQueryProvider, Provider<InternalAccountQuery> accountQueryProvider) {
ExternalIdCache externalIdCache) {
this.serverIdent = serverIdent; this.serverIdent = serverIdent;
this.db = db; this.db = db;
this.self = self; this.self = self;
@@ -111,7 +108,6 @@ public class PostGpgKeys implements RestModifyView<AccountResource, Input> {
this.addKeyFactory = addKeyFactory; this.addKeyFactory = addKeyFactory;
this.accountCache = accountCache; this.accountCache = accountCache;
this.accountQueryProvider = accountQueryProvider; this.accountQueryProvider = accountQueryProvider;
this.externalIdCache = externalIdCache;
} }
@Override @Override
@@ -121,8 +117,7 @@ public class PostGpgKeys implements RestModifyView<AccountResource, Input> {
GpgKeys.checkVisible(self, rsrc); GpgKeys.checkVisible(self, rsrc);
List<AccountExternalId> existingExtIds = List<AccountExternalId> existingExtIds =
GpgKeys.getGpgExtIds(externalIdCache, GpgKeys.getGpgExtIds(db.get(), rsrc.getUser().getAccountId()).toList();
rsrc.getUser().getAccountId()).toList();
try (PublicKeyStore store = storeProvider.get()) { try (PublicKeyStore store = storeProvider.get()) {
Set<Fingerprint> toRemove = readKeysToRemove(input, existingExtIds); Set<Fingerprint> toRemove = readKeysToRemove(input, existingExtIds);
@@ -147,13 +142,9 @@ public class PostGpgKeys implements RestModifyView<AccountResource, Input> {
storeKeys(rsrc, newKeys, toRemove); storeKeys(rsrc, newKeys, toRemove);
if (!newExtIds.isEmpty()) { if (!newExtIds.isEmpty()) {
db.get().accountExternalIds().insert(newExtIds); db.get().accountExternalIds().insert(newExtIds);
externalIdCache.onCreate(newExtIds);
} }
db.get().accountExternalIds().deleteKeys(
List<AccountExternalId.Key> extIdKeysToRemove = Iterables.transform(toRemove, fp -> toExtIdKey(fp.get())));
toRemove.stream().map(fp -> toExtIdKey(fp.get())).collect(toList());
db.get().accountExternalIds().deleteKeys(extIdKeysToRemove);
externalIdCache.onRemove(rsrc.getUser().getAccountId(), extIdKeysToRemove);
accountCache.evict(rsrc.getUser().getAccountId()); accountCache.evict(rsrc.getUser().getAccountId());
return toJson(newKeys, toRemove, store, rsrc.getUser()); return toJson(newKeys, toRemove, store, rsrc.getUser());
} }

View File

@@ -41,7 +41,6 @@ import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.account.AccountCache; import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.account.AccountManager; import com.google.gerrit.server.account.AccountManager;
import com.google.gerrit.server.account.AuthRequest; import com.google.gerrit.server.account.AuthRequest;
import com.google.gerrit.server.account.ExternalIdCache;
import com.google.gerrit.server.schema.SchemaCreator; import com.google.gerrit.server.schema.SchemaCreator;
import com.google.gerrit.server.util.RequestContext; import com.google.gerrit.server.util.RequestContext;
import com.google.gerrit.server.util.ThreadLocalRequestContext; import com.google.gerrit.server.util.ThreadLocalRequestContext;
@@ -70,7 +69,6 @@ import org.junit.Test;
import java.io.IOException; import java.io.IOException;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.Arrays; import java.util.Arrays;
import java.util.Collection;
import java.util.Collections; import java.util.Collections;
import java.util.List; import java.util.List;
@@ -97,9 +95,6 @@ public class GerritPublicKeyCheckerTest {
@Inject @Inject
private ThreadLocalRequestContext requestContext; private ThreadLocalRequestContext requestContext;
@Inject
private ExternalIdCache externalIdCache;
private LifecycleManager lifecycle; private LifecycleManager lifecycle;
private ReviewDb db; private ReviewDb db;
private Account.Id userId; private Account.Id userId;
@@ -234,10 +229,8 @@ public class GerritPublicKeyCheckerTest {
@Test @Test
public void noExternalIds() throws Exception { public void noExternalIds() throws Exception {
Collection<AccountExternalId> extIds = db.accountExternalIds().delete(
externalIdCache.byAccount(user.getAccountId()); db.accountExternalIds().byAccount(user.getAccountId()));
db.accountExternalIds().delete(extIds);
externalIdCache.onRemove(extIds);
reloadUser(); reloadUser();
TestKey key = validKeyWithSecondUserId(); TestKey key = validKeyWithSecondUserId();
@@ -255,10 +248,9 @@ public class GerritPublicKeyCheckerTest {
checker.check(key.getPublicKey()), Status.BAD, checker.check(key.getPublicKey()), Status.BAD,
"Key is not associated with any users"); "Key is not associated with any users");
AccountExternalId extId = new AccountExternalId(user.getAccountId(), db.accountExternalIds().insert(Collections.singleton(
toExtIdKey(key.getPublicKey())); new AccountExternalId(
db.accountExternalIds().insert(Collections.singleton(extId)); user.getAccountId(), toExtIdKey(key.getPublicKey()))));
externalIdCache.onCreate(extId);
reloadUser(); reloadUser();
assertProblems( assertProblems(
checker.check(key.getPublicKey()), Status.BAD, checker.check(key.getPublicKey()), Status.BAD,
@@ -435,7 +427,6 @@ public class GerritPublicKeyCheckerTest {
assertThat(store.save(cb)).isAnyOf(NEW, FAST_FORWARD, FORCED); assertThat(store.save(cb)).isAnyOf(NEW, FAST_FORWARD, FORCED);
db.accountExternalIds().insert(newExtIds); db.accountExternalIds().insert(newExtIds);
externalIdCache.onCreate(newExtIds);
accountCache.evict(user.getAccountId()); accountCache.evict(user.getAccountId());
} }
@@ -468,7 +459,6 @@ public class GerritPublicKeyCheckerTest {
extId.setEmailAddress(email); extId.setEmailAddress(email);
} }
db.accountExternalIds().insert(Collections.singleton(extId)); db.accountExternalIds().insert(Collections.singleton(extId));
externalIdCache.onCreate(extId);
reloadUser(); reloadUser();
} }
} }

View File

@@ -20,7 +20,6 @@ import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.account.AccountByEmailCache; import com.google.gerrit.server.account.AccountByEmailCache;
import com.google.gerrit.server.account.AccountCache; import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.account.ExternalIdCache;
import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject; import com.google.inject.Inject;
import com.google.inject.assistedinject.Assisted; import com.google.inject.assistedinject.Assisted;
@@ -43,24 +42,21 @@ class DeleteExternalIds extends Handler<Set<AccountExternalId.Key>> {
private final ExternalIdDetailFactory detailFactory; private final ExternalIdDetailFactory detailFactory;
private final AccountByEmailCache byEmailCache; private final AccountByEmailCache byEmailCache;
private final AccountCache accountCache; private final AccountCache accountCache;
private final ExternalIdCache externalIdCache;
private final Set<AccountExternalId.Key> keys; private final Set<AccountExternalId.Key> keys;
@Inject @Inject
DeleteExternalIds(ReviewDb db, DeleteExternalIds(final ReviewDb db, final IdentifiedUser user,
IdentifiedUser user, final ExternalIdDetailFactory detailFactory,
ExternalIdDetailFactory detailFactory, final AccountByEmailCache byEmailCache, final AccountCache accountCache,
AccountByEmailCache byEmailCache,
AccountCache accountCache, @Assisted final Set<AccountExternalId.Key> keys) {
ExternalIdCache externalIdCache,
@Assisted Set<AccountExternalId.Key> keys) {
this.db = db; this.db = db;
this.user = user; this.user = user;
this.detailFactory = detailFactory; this.detailFactory = detailFactory;
this.byEmailCache = byEmailCache; this.byEmailCache = byEmailCache;
this.accountCache = accountCache; this.accountCache = accountCache;
this.externalIdCache = externalIdCache;
this.keys = keys; this.keys = keys;
} }
@@ -78,7 +74,6 @@ class DeleteExternalIds extends Handler<Set<AccountExternalId.Key>> {
if (!toDelete.isEmpty()) { if (!toDelete.isEmpty()) {
db.accountExternalIds().delete(toDelete); db.accountExternalIds().delete(toDelete);
externalIdCache.onRemove(toDelete);
accountCache.evict(user.getAccountId()); accountCache.evict(user.getAccountId());
for (AccountExternalId e : toDelete) { for (AccountExternalId e : toDelete) {
byEmailCache.evict(e.getEmailAddress()); byEmailCache.evict(e.getEmailAddress());

View File

@@ -20,13 +20,12 @@ import com.google.gerrit.extensions.registration.DynamicItem;
import com.google.gerrit.httpd.WebSession; import com.google.gerrit.httpd.WebSession;
import com.google.gerrit.httpd.rpc.Handler; import com.google.gerrit.httpd.rpc.Handler;
import com.google.gerrit.reviewdb.client.AccountExternalId; import com.google.gerrit.reviewdb.client.AccountExternalId;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.account.ExternalIdCache;
import com.google.gerrit.server.config.AuthConfig; import com.google.gerrit.server.config.AuthConfig;
import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject; import com.google.inject.Inject;
import java.util.ArrayList;
import java.util.Collections; import java.util.Collections;
import java.util.List; import java.util.List;
@@ -35,27 +34,25 @@ class ExternalIdDetailFactory extends Handler<List<AccountExternalId>> {
ExternalIdDetailFactory create(); ExternalIdDetailFactory create();
} }
private final ReviewDb db;
private final IdentifiedUser user; private final IdentifiedUser user;
private final AuthConfig authConfig; private final AuthConfig authConfig;
private final DynamicItem<WebSession> session; private final DynamicItem<WebSession> session;
private final ExternalIdCache externalIdCache;
@Inject @Inject
ExternalIdDetailFactory(IdentifiedUser user, ExternalIdDetailFactory(final ReviewDb db, final IdentifiedUser user,
AuthConfig authConfig, final AuthConfig authConfig, final DynamicItem<WebSession> session) {
DynamicItem<WebSession> session, this.db = db;
ExternalIdCache externalIdCache) {
this.user = user; this.user = user;
this.authConfig = authConfig; this.authConfig = authConfig;
this.session = session; this.session = session;
this.externalIdCache = externalIdCache;
} }
@Override @Override
public List<AccountExternalId> call() throws OrmException { public List<AccountExternalId> call() throws OrmException {
AccountExternalId.Key last = session.get().getLastLoginExternalId(); final AccountExternalId.Key last = session.get().getLastLoginExternalId();
List<AccountExternalId> ids = final List<AccountExternalId> ids =
new ArrayList<>(externalIdCache.byAccount(user.getAccountId())); db.accountExternalIds().byAccount(user.getAccountId()).toList();
for (final AccountExternalId e : ids) { for (final AccountExternalId e : ids) {
e.setTrusted(authConfig.isIdentityTrustable(Collections.singleton(e))); e.setTrusted(authConfig.isIdentityTrustable(Collections.singleton(e)));

View File

@@ -33,7 +33,6 @@ import com.google.gerrit.server.account.AccountVisibility;
import com.google.gerrit.server.account.AccountVisibilityProvider; import com.google.gerrit.server.account.AccountVisibilityProvider;
import com.google.gerrit.server.account.CapabilityCollection; import com.google.gerrit.server.account.CapabilityCollection;
import com.google.gerrit.server.account.CapabilityControl; import com.google.gerrit.server.account.CapabilityControl;
import com.google.gerrit.server.account.ExternalIdCacheImpl;
import com.google.gerrit.server.account.FakeRealm; import com.google.gerrit.server.account.FakeRealm;
import com.google.gerrit.server.account.GroupCacheImpl; import com.google.gerrit.server.account.GroupCacheImpl;
import com.google.gerrit.server.account.GroupIncludeCacheImpl; import com.google.gerrit.server.account.GroupIncludeCacheImpl;
@@ -154,7 +153,6 @@ public class BatchProgramModule extends FactoryModule {
install(new PrologModule()); install(new PrologModule());
install(AccountByEmailCacheImpl.module()); install(AccountByEmailCacheImpl.module());
install(AccountCacheImpl.module()); install(AccountCacheImpl.module());
install(ExternalIdCacheImpl.module());
install(GroupCacheImpl.module()); install(GroupCacheImpl.module());
install(GroupIncludeCacheImpl.module()); install(GroupIncludeCacheImpl.module());
install(ProjectCacheImpl.module()); install(ProjectCacheImpl.module());

View File

@@ -18,8 +18,6 @@ import com.google.gerrit.extensions.client.AuthType;
import com.google.gwtorm.client.Column; import com.google.gwtorm.client.Column;
import com.google.gwtorm.client.StringKey; import com.google.gwtorm.client.StringKey;
import java.util.Objects;
/** Association of an external account identifier to a local {@link Account}. */ /** Association of an external account identifier to a local {@link Account}. */
public final class AccountExternalId { public final class AccountExternalId {
/** /**
@@ -169,21 +167,4 @@ public final class AccountExternalId {
public void setCanDelete(final boolean t) { public void setCanDelete(final boolean t) {
canDelete = t; canDelete = t;
} }
@Override
public boolean equals(Object o) {
if (o instanceof AccountExternalId) {
AccountExternalId extId = (AccountExternalId) o;
return Objects.equals(key, extId.key)
&& Objects.equals(accountId, extId.accountId)
&& Objects.equals(emailAddress, extId.emailAddress)
&& Objects.equals(password, extId.password);
}
return false;
}
@Override
public int hashCode() {
return Objects.hash(key, accountId, emailAddress, password);
}
} }

View File

@@ -14,6 +14,7 @@
package com.google.gerrit.reviewdb.server; package com.google.gerrit.reviewdb.server;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.AccountExternalId; import com.google.gerrit.reviewdb.client.AccountExternalId;
import com.google.gwtorm.server.Access; import com.google.gwtorm.server.Access;
import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.OrmException;
@@ -27,6 +28,9 @@ public interface AccountExternalIdAccess extends
@PrimaryKey("key") @PrimaryKey("key")
AccountExternalId get(AccountExternalId.Key key) throws OrmException; AccountExternalId get(AccountExternalId.Key key) throws OrmException;
@Query("WHERE accountId = ?")
ResultSet<AccountExternalId> byAccount(Account.Id id) throws OrmException;
@Query @Query
ResultSet<AccountExternalId> all() throws OrmException; ResultSet<AccountExternalId> all() throws OrmException;
} }

View File

@@ -155,7 +155,6 @@ public class AccountCacheImpl implements AccountCache {
private final GeneralPreferencesLoader loader; private final GeneralPreferencesLoader loader;
private final LoadingCache<String, Optional<Account.Id>> byName; private final LoadingCache<String, Optional<Account.Id>> byName;
private final Provider<WatchConfig.Accessor> watchConfig; private final Provider<WatchConfig.Accessor> watchConfig;
private final ExternalIdCache externalIdCache;
@Inject @Inject
ByIdLoader(SchemaFactory<ReviewDb> sf, ByIdLoader(SchemaFactory<ReviewDb> sf,
@@ -163,14 +162,12 @@ public class AccountCacheImpl implements AccountCache {
GeneralPreferencesLoader loader, GeneralPreferencesLoader loader,
@Named(BYUSER_NAME) LoadingCache<String, @Named(BYUSER_NAME) LoadingCache<String,
Optional<Account.Id>> byUsername, Optional<Account.Id>> byUsername,
Provider<WatchConfig.Accessor> watchConfig, Provider<WatchConfig.Accessor> watchConfig) {
ExternalIdCache externalIdCache) {
this.schema = sf; this.schema = sf;
this.groupCache = groupCache; this.groupCache = groupCache;
this.loader = loader; this.loader = loader;
this.byName = byUsername; this.byName = byUsername;
this.watchConfig = watchConfig; this.watchConfig = watchConfig;
this.externalIdCache = externalIdCache;
} }
@Override @Override
@@ -194,7 +191,8 @@ public class AccountCacheImpl implements AccountCache {
} }
Collection<AccountExternalId> externalIds = Collection<AccountExternalId> externalIds =
externalIdCache.byAccount(who); Collections.unmodifiableCollection(
db.accountExternalIds().byAccount(who).toList());
Set<AccountGroup.UUID> internalGroups = new HashSet<>(); Set<AccountGroup.UUID> internalGroups = new HashSet<>();
for (AccountGroupMember g : db.accountGroupMembers().byAccount(who)) { for (AccountGroupMember g : db.accountGroupMembers().byAccount(who)) {

View File

@@ -31,6 +31,7 @@ import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.project.ProjectCache; import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.query.account.InternalAccountQuery; import com.google.gerrit.server.query.account.InternalAccountQuery;
import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.OrmException;
import com.google.gwtorm.server.ResultSet;
import com.google.gwtorm.server.SchemaFactory; import com.google.gwtorm.server.SchemaFactory;
import com.google.inject.Inject; import com.google.inject.Inject;
import com.google.inject.Provider; import com.google.inject.Provider;
@@ -41,7 +42,6 @@ import org.slf4j.LoggerFactory;
import java.io.IOException; import java.io.IOException;
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.Optional; import java.util.Optional;
@@ -63,7 +63,6 @@ public class AccountManager {
private final AtomicBoolean awaitsFirstAccountCheck; private final AtomicBoolean awaitsFirstAccountCheck;
private final AuditService auditService; private final AuditService auditService;
private final Provider<InternalAccountQuery> accountQueryProvider; private final Provider<InternalAccountQuery> accountQueryProvider;
private final ExternalIdCache externalIdCache;
@Inject @Inject
AccountManager(SchemaFactory<ReviewDb> schema, AccountManager(SchemaFactory<ReviewDb> schema,
@@ -74,8 +73,7 @@ public class AccountManager {
ChangeUserName.Factory changeUserNameFactory, ChangeUserName.Factory changeUserNameFactory,
ProjectCache projectCache, ProjectCache projectCache,
AuditService auditService, AuditService auditService,
Provider<InternalAccountQuery> accountQueryProvider, Provider<InternalAccountQuery> accountQueryProvider) {
ExternalIdCache externalIdCache) {
this.schema = schema; this.schema = schema;
this.byIdCache = byIdCache; this.byIdCache = byIdCache;
this.byEmailCache = byEmailCache; this.byEmailCache = byEmailCache;
@@ -86,7 +84,6 @@ public class AccountManager {
this.awaitsFirstAccountCheck = new AtomicBoolean(true); this.awaitsFirstAccountCheck = new AtomicBoolean(true);
this.auditService = auditService; this.auditService = auditService;
this.accountQueryProvider = accountQueryProvider; this.accountQueryProvider = accountQueryProvider;
this.externalIdCache = externalIdCache;
} }
/** /**
@@ -174,7 +171,6 @@ public class AccountManager {
extId.setEmailAddress(newEmail); extId.setEmailAddress(newEmail);
db.accountExternalIds().update(Collections.singleton(extId)); db.accountExternalIds().update(Collections.singleton(extId));
externalIdCache.onUpdate(extId);
} }
if (!realm.allowsEdit(AccountFieldName.FULL_NAME) if (!realm.allowsEdit(AccountFieldName.FULL_NAME)
@@ -246,7 +242,6 @@ public class AccountManager {
+ "\" to account " + newId + "; external ID already in use."); + "\" to account " + newId + "; external ID already in use.");
} }
db.accountExternalIds().upsert(Collections.singleton(extId)); db.accountExternalIds().upsert(Collections.singleton(extId));
externalIdCache.onUpdate(extId);
} finally { } finally {
// If adding the account failed, it may be that it actually was the // If adding the account failed, it may be that it actually was the
// first account. So we reset the 'check for first account'-guard, as // first account. So we reset the 'check for first account'-guard, as
@@ -340,7 +335,6 @@ public class AccountManager {
// the database // the database
db.accounts().delete(Collections.singleton(account)); db.accounts().delete(Collections.singleton(account));
db.accountExternalIds().delete(Collections.singleton(extId)); db.accountExternalIds().delete(Collections.singleton(extId));
externalIdCache.onRemove(extId);
throw new AccountUserNameException(errorMessage, e); throw new AccountUserNameException(errorMessage, e);
} }
} }
@@ -373,7 +367,6 @@ public class AccountManager {
extId = createId(to, who); extId = createId(to, who);
extId.setEmailAddress(who.getEmailAddress()); extId.setEmailAddress(who.getEmailAddress());
db.accountExternalIds().insert(Collections.singleton(extId)); db.accountExternalIds().insert(Collections.singleton(extId));
externalIdCache.onCreate(extId);
if (who.getEmailAddress() != null) { if (who.getEmailAddress() != null) {
Account a = db.accounts().get(to); Account a = db.accounts().get(to);
@@ -412,12 +405,12 @@ public class AccountManager {
try (ReviewDb db = schema.open()) { try (ReviewDb db = schema.open()) {
AccountExternalId.Key key = id(who); AccountExternalId.Key key = id(who);
List<AccountExternalId.Key> filteredKeysByScheme = List<AccountExternalId.Key> filteredKeysByScheme =
filterKeysByScheme(key.getScheme(), externalIdCache.byAccount(to)); filterKeysByScheme(key.getScheme(), db.accountExternalIds()
.byAccount(to));
if (!filteredKeysByScheme.isEmpty() if (!filteredKeysByScheme.isEmpty()
&& (filteredKeysByScheme.size() > 1 || !filteredKeysByScheme && (filteredKeysByScheme.size() > 1 || !filteredKeysByScheme
.contains(key))) { .contains(key))) {
db.accountExternalIds().deleteKeys(filteredKeysByScheme); db.accountExternalIds().deleteKeys(filteredKeysByScheme);
externalIdCache.onRemove(to, filteredKeysByScheme);
} }
byIdCache.evict(to); byIdCache.evict(to);
return link(to, who); return link(to, who);
@@ -425,7 +418,7 @@ public class AccountManager {
} }
private List<AccountExternalId.Key> filterKeysByScheme( private List<AccountExternalId.Key> filterKeysByScheme(
String keyScheme, Collection<AccountExternalId> externalIds) { String keyScheme, ResultSet<AccountExternalId> externalIds) {
List<AccountExternalId.Key> filteredExternalIds = new ArrayList<>(); List<AccountExternalId.Key> filteredExternalIds = new ArrayList<>();
for (AccountExternalId accountExternalId : externalIds) { for (AccountExternalId accountExternalId : externalIds) {
if (accountExternalId.isScheme(keyScheme)) { if (accountExternalId.isScheme(keyScheme)) {
@@ -455,7 +448,6 @@ public class AccountManager {
"Identity '" + key.get() + "' in use by another account"); "Identity '" + key.get() + "' in use by another account");
} }
db.accountExternalIds().delete(Collections.singleton(extId)); db.accountExternalIds().delete(Collections.singleton(extId));
externalIdCache.onRemove(extId);
if (who.getEmailAddress() != null) { if (who.getEmailAddress() != null) {
Account a = db.accounts().get(from); Account a = db.accounts().get(from);

View File

@@ -30,6 +30,7 @@ import com.google.inject.Inject;
import com.google.inject.assistedinject.Assisted; import com.google.inject.assistedinject.Assisted;
import java.io.IOException; import java.io.IOException;
import java.util.ArrayList;
import java.util.Collection; import java.util.Collection;
import java.util.Collections; import java.util.Collections;
import java.util.concurrent.Callable; import java.util.concurrent.Callable;
@@ -50,22 +51,19 @@ public class ChangeUserName implements Callable<VoidResult> {
private final AccountCache accountCache; private final AccountCache accountCache;
private final SshKeyCache sshKeyCache; private final SshKeyCache sshKeyCache;
private final ExternalIdCache externalIdCache;
private final ReviewDb db; private final ReviewDb db;
private final IdentifiedUser user; private final IdentifiedUser user;
private final String newUsername; private final String newUsername;
@Inject @Inject
ChangeUserName(AccountCache accountCache, ChangeUserName(final AccountCache accountCache,
SshKeyCache sshKeyCache, final SshKeyCache sshKeyCache,
ExternalIdCache externalIdCache,
@Assisted ReviewDb db, @Assisted final ReviewDb db, @Assisted final IdentifiedUser user,
@Assisted IdentifiedUser user, @Nullable @Assisted final String newUsername) {
@Nullable @Assisted String newUsername) {
this.accountCache = accountCache; this.accountCache = accountCache;
this.sshKeyCache = sshKeyCache; this.sshKeyCache = sshKeyCache;
this.externalIdCache = externalIdCache;
this.db = db; this.db = db;
this.user = user; this.user = user;
@@ -75,8 +73,7 @@ public class ChangeUserName implements Callable<VoidResult> {
@Override @Override
public VoidResult call() throws OrmException, NameAlreadyUsedException, public VoidResult call() throws OrmException, NameAlreadyUsedException,
InvalidUserNameException, IOException { InvalidUserNameException, IOException {
Collection<AccountExternalId> old = final Collection<AccountExternalId> old = old();
externalIdCache.byAccount(user.getAccountId(), SCHEME_USERNAME);
if (!old.isEmpty()) { if (!old.isEmpty()) {
throw new IllegalStateException(USERNAME_CANNOT_BE_CHANGED); throw new IllegalStateException(USERNAME_CANNOT_BE_CHANGED);
} }
@@ -99,7 +96,6 @@ public class ChangeUserName implements Callable<VoidResult> {
} }
db.accountExternalIds().insert(Collections.singleton(id)); db.accountExternalIds().insert(Collections.singleton(id));
externalIdCache.onCreate(id);
} catch (OrmDuplicateKeyException dupeErr) { } catch (OrmDuplicateKeyException dupeErr) {
// If we are using this identity, don't report the exception. // If we are using this identity, don't report the exception.
// //
@@ -117,7 +113,6 @@ public class ChangeUserName implements Callable<VoidResult> {
// If we have any older user names, remove them. // If we have any older user names, remove them.
// //
db.accountExternalIds().delete(old); db.accountExternalIds().delete(old);
externalIdCache.onRemove(old);
for (AccountExternalId i : old) { for (AccountExternalId i : old) {
sshKeyCache.evict(i.getSchemeRest()); sshKeyCache.evict(i.getSchemeRest());
accountCache.evictByUsername(i.getSchemeRest()); accountCache.evictByUsername(i.getSchemeRest());
@@ -128,4 +123,15 @@ public class ChangeUserName implements Callable<VoidResult> {
sshKeyCache.evict(newUsername); sshKeyCache.evict(newUsername);
return VoidResult.INSTANCE; return VoidResult.INSTANCE;
} }
private Collection<AccountExternalId> old() throws OrmException {
final Collection<AccountExternalId> r = new ArrayList<>(1);
for (AccountExternalId i : db.accountExternalIds().byAccount(
user.getAccountId())) {
if (i.isScheme(SCHEME_USERNAME)) {
r.add(i);
}
}
return r;
}
} }

View File

@@ -73,7 +73,6 @@ public class CreateAccount
private final AccountLoader.Factory infoLoader; private final AccountLoader.Factory infoLoader;
private final DynamicSet<AccountExternalIdCreator> externalIdCreators; private final DynamicSet<AccountExternalIdCreator> externalIdCreators;
private final AuditService auditService; private final AuditService auditService;
private final ExternalIdCache externalIdCache;
private final String username; private final String username;
@Inject @Inject
@@ -88,7 +87,6 @@ public class CreateAccount
AccountLoader.Factory infoLoader, AccountLoader.Factory infoLoader,
DynamicSet<AccountExternalIdCreator> externalIdCreators, DynamicSet<AccountExternalIdCreator> externalIdCreators,
AuditService auditService, AuditService auditService,
ExternalIdCache externalIdCache,
@Assisted String username) { @Assisted String username) {
this.db = db; this.db = db;
this.currentUser = currentUser; this.currentUser = currentUser;
@@ -101,7 +99,6 @@ public class CreateAccount
this.infoLoader = infoLoader; this.infoLoader = infoLoader;
this.externalIdCreators = externalIdCreators; this.externalIdCreators = externalIdCreators;
this.auditService = auditService; this.auditService = auditService;
this.externalIdCache = externalIdCache;
this.username = username; this.username = username;
} }
@@ -156,7 +153,6 @@ public class CreateAccount
try { try {
db.accountExternalIds().insert(externalIds); db.accountExternalIds().insert(externalIds);
externalIdCache.onCreate(externalIds);
} catch (OrmDuplicateKeyException duplicateKey) { } catch (OrmDuplicateKeyException duplicateKey) {
throw new ResourceConflictException( throw new ResourceConflictException(
"username '" + username + "' already exists"); "username '" + username + "' already exists");
@@ -168,11 +164,9 @@ public class CreateAccount
extMailto.setEmailAddress(input.email); extMailto.setEmailAddress(input.email);
try { try {
db.accountExternalIds().insert(Collections.singleton(extMailto)); db.accountExternalIds().insert(Collections.singleton(extMailto));
externalIdCache.onCreate(extMailto);
} catch (OrmDuplicateKeyException duplicateKey) { } catch (OrmDuplicateKeyException duplicateKey) {
try { try {
db.accountExternalIds().delete(Collections.singleton(extUser)); db.accountExternalIds().delete(Collections.singleton(extUser));
externalIdCache.onRemove(extUser);
} catch (OrmException cleanupError) { } catch (OrmException cleanupError) {
// Ignored // Ignored
} }

View File

@@ -41,22 +41,22 @@ import java.util.stream.Collectors;
@Singleton @Singleton
public class DeleteExternalIds implements public class DeleteExternalIds implements
RestModifyView<AccountResource, List<String>> { RestModifyView<AccountResource, List<String>> {
private final Provider<ReviewDb> db;
private final AccountByEmailCache accountByEmailCache; private final AccountByEmailCache accountByEmailCache;
private final AccountCache accountCache; private final AccountCache accountCache;
private final ExternalIdCache externalIdCache;
private final Provider<CurrentUser> self; private final Provider<CurrentUser> self;
private final Provider<ReviewDb> dbProvider; private final Provider<ReviewDb> dbProvider;
@Inject @Inject
DeleteExternalIds( DeleteExternalIds(
Provider<ReviewDb> db,
AccountByEmailCache accountByEmailCache, AccountByEmailCache accountByEmailCache,
AccountCache accountCache, AccountCache accountCache,
ExternalIdCache externalIdCache,
Provider<CurrentUser> self, Provider<CurrentUser> self,
Provider<ReviewDb> dbProvider) { Provider<ReviewDb> dbProvider) {
this.db = db;
this.accountByEmailCache = accountByEmailCache; this.accountByEmailCache = accountByEmailCache;
this.accountCache = accountCache; this.accountCache = accountCache;
this.externalIdCache = externalIdCache;
this.self = self; this.self = self;
this.dbProvider = dbProvider; this.dbProvider = dbProvider;
} }
@@ -74,8 +74,9 @@ public class DeleteExternalIds implements
Account.Id accountId = resource.getUser().getAccountId(); Account.Id accountId = resource.getUser().getAccountId();
Map<AccountExternalId.Key, AccountExternalId> externalIdMap = Map<AccountExternalId.Key, AccountExternalId> externalIdMap =
externalIdCache.byAccount(resource.getUser().getAccountId()) db.get().accountExternalIds().byAccount(
.stream().collect(Collectors.toMap(i -> i.getKey(), i -> i)); resource.getUser().getAccountId()).toList()
.stream().collect(Collectors.toMap(i -> i.getKey(), i -> i));
List<AccountExternalId> toDelete = new ArrayList<>(); List<AccountExternalId> toDelete = new ArrayList<>();
AccountExternalId.Key last = resource.getUser().getLastLoginExternalIdKey(); AccountExternalId.Key last = resource.getUser().getLastLoginExternalIdKey();
@@ -99,7 +100,6 @@ public class DeleteExternalIds implements
if (!toDelete.isEmpty()) { if (!toDelete.isEmpty()) {
dbProvider.get().accountExternalIds().delete(toDelete); dbProvider.get().accountExternalIds().delete(toDelete);
externalIdCache.onRemove(toDelete);
accountCache.evict(accountId); accountCache.evict(accountId);
for (AccountExternalId e : toDelete) { for (AccountExternalId e : toDelete) {
accountByEmailCache.evict(e.getEmailAddress()); accountByEmailCache.evict(e.getEmailAddress());

View File

@@ -1,52 +0,0 @@
// Copyright (C) 2016 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.account;
import static java.util.stream.Collectors.toSet;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.AccountExternalId;
import java.util.Collection;
import java.util.Collections;
import java.util.Set;
/** Caches external ids of all accounts */
public interface ExternalIdCache {
void onCreate(Iterable<AccountExternalId> extId);
void onRemove(Iterable<AccountExternalId> extId);
void onRemove(Account.Id accountId,
Iterable<AccountExternalId.Key> extIdKeys);
void onUpdate(AccountExternalId extId);
Collection<AccountExternalId> byAccount(Account.Id accountId);
default void onCreate(AccountExternalId extId) {
onCreate(Collections.singleton(extId));
}
default void onRemove(AccountExternalId extId) {
onRemove(Collections.singleton(extId));
}
default void onRemove(Account.Id accountId, AccountExternalId.Key extIdKey) {
onRemove(accountId, Collections.singleton(extIdKey));
}
default Set<AccountExternalId> byAccount(Account.Id accountId,
String scheme) {
return byAccount(accountId).stream().filter(e -> e.isScheme(scheme))
.collect(toSet());
}
}

View File

@@ -1,193 +0,0 @@
// Copyright (C) 2016 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.account;
import com.google.common.cache.CacheLoader;
import com.google.common.cache.LoadingCache;
import com.google.common.collect.ImmutableSetMultimap;
import com.google.common.collect.ListMultimap;
import com.google.common.collect.MultimapBuilder;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.AccountExternalId;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.cache.CacheModule;
import com.google.gwtorm.server.SchemaFactory;
import com.google.inject.Inject;
import com.google.inject.Module;
import com.google.inject.Singleton;
import com.google.inject.TypeLiteral;
import com.google.inject.name.Named;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import java.util.Collection;
import java.util.Collections;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.locks.Lock;
import java.util.concurrent.locks.ReentrantLock;
@Singleton
public class ExternalIdCacheImpl implements ExternalIdCache {
private static final Logger log =
LoggerFactory.getLogger(ExternalIdCacheImpl.class);
private static final String CACHE_NAME = "external_ids_map";
public static Module module() {
return new CacheModule() {
@Override
protected void configure() {
cache(CACHE_NAME, AllKey.class,
new TypeLiteral<ImmutableSetMultimap<Account.Id, AccountExternalId>>() {})
.maximumWeight(1).loader(Loader.class);
bind(ExternalIdCacheImpl.class);
bind(ExternalIdCache.class).to(ExternalIdCacheImpl.class);
}
};
}
private final LoadingCache<AllKey,
ImmutableSetMultimap<Account.Id, AccountExternalId>> extIdsByAccount;
private final Lock lock;
@Inject
ExternalIdCacheImpl(
@Named(CACHE_NAME) LoadingCache<AllKey,
ImmutableSetMultimap<Account.Id, AccountExternalId>> extIdsByAccount) {
this.extIdsByAccount = extIdsByAccount;
this.lock = new ReentrantLock(true /* fair */);
}
@Override
public void onCreate(Iterable<AccountExternalId> extIds) {
lock.lock();
try {
ListMultimap<Account.Id, AccountExternalId> n = MultimapBuilder.hashKeys()
.arrayListValues().build(extIdsByAccount.get(AllKey.ALL));
for (AccountExternalId extId : extIds) {
n.put(extId.getAccountId(), extId);
}
extIdsByAccount.put(AllKey.ALL, ImmutableSetMultimap.copyOf(n));
} catch (ExecutionException e) {
log.warn("Cannot list external IDs", e);
} finally {
lock.unlock();
}
}
@Override
public void onRemove(Iterable<AccountExternalId> extIds) {
lock.lock();
try {
ListMultimap<Account.Id, AccountExternalId> n = MultimapBuilder.hashKeys()
.arrayListValues().build(extIdsByAccount.get(AllKey.ALL));
for (AccountExternalId extId : extIds) {
n.remove(extId.getAccountId(), extId);
}
extIdsByAccount.put(AllKey.ALL, ImmutableSetMultimap.copyOf(n));
} catch (ExecutionException e) {
log.warn("Cannot list external IDs", e);
} finally {
lock.unlock();
}
}
@Override
public void onRemove(Account.Id accountId,
Iterable<AccountExternalId.Key> extIdKeys) {
lock.lock();
try {
ListMultimap<Account.Id, AccountExternalId> n = MultimapBuilder.hashKeys()
.arrayListValues().build(extIdsByAccount.get(AllKey.ALL));
for (AccountExternalId extId : byAccount(accountId)) {
for (AccountExternalId.Key extIdKey : extIdKeys) {
if (extIdKey.equals(extId.getKey())) {
n.remove(accountId, extId);
break;
}
}
}
extIdsByAccount.put(AllKey.ALL, ImmutableSetMultimap.copyOf(n));
} catch (ExecutionException e) {
log.warn("Cannot list external IDs", e);
} finally {
lock.unlock();
}
}
@Override
public void onUpdate(AccountExternalId updatedExtId) {
lock.lock();
try {
ListMultimap<Account.Id, AccountExternalId> n = MultimapBuilder.hashKeys()
.arrayListValues().build(extIdsByAccount.get(AllKey.ALL));
for (AccountExternalId extId : byAccount(updatedExtId.getAccountId())) {
if (updatedExtId.getKey().equals(extId.getKey())) {
n.remove(updatedExtId.getAccountId(), extId);
break;
}
}
n.put(updatedExtId.getAccountId(), updatedExtId);
extIdsByAccount.put(AllKey.ALL, ImmutableSetMultimap.copyOf(n));
} catch (ExecutionException e) {
log.warn("Cannot list external IDs", e);
} finally {
lock.unlock();
}
}
@Override
public Collection<AccountExternalId> byAccount(Account.Id accountId) {
try {
return extIdsByAccount.get(AllKey.ALL).get(accountId);
} catch (ExecutionException e) {
log.warn("Cannot list external ids", e);
return Collections.emptySet();
}
}
public static class AllKey {
static final AllKey ALL = new AllKey();
private AllKey() {
}
}
static class Loader
extends CacheLoader<AllKey,
ImmutableSetMultimap<Account.Id, AccountExternalId>> {
private final SchemaFactory<ReviewDb> schema;
@Inject
Loader(SchemaFactory<ReviewDb> schema) {
this.schema = schema;
}
@Override
public ImmutableSetMultimap<Account.Id, AccountExternalId> load(AllKey key)
throws Exception {
try (ReviewDb db = schema.open()) {
ListMultimap<Account.Id, AccountExternalId> extIdsByAccount =
MultimapBuilder.hashKeys().arrayListValues().build();
for (AccountExternalId extId : db.accountExternalIds().all()) {
extIdsByAccount.put(extId.getAccountId(), extId);
}
return ImmutableSetMultimap.copyOf(extIdsByAccount);
}
}
}
}

View File

@@ -23,8 +23,10 @@ import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.RestApiException; import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.RestReadView; import com.google.gerrit.extensions.restapi.RestReadView;
import com.google.gerrit.reviewdb.client.AccountExternalId; import com.google.gerrit.reviewdb.client.AccountExternalId;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.CurrentUser; import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.config.AuthConfig; import com.google.gerrit.server.config.AuthConfig;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject; import com.google.inject.Inject;
import com.google.inject.Provider; import com.google.inject.Provider;
import com.google.inject.Singleton; import com.google.inject.Singleton;
@@ -35,28 +37,28 @@ import java.util.List;
@Singleton @Singleton
public class GetExternalIds implements RestReadView<AccountResource> { public class GetExternalIds implements RestReadView<AccountResource> {
private final ExternalIdCache externalIdCache; private final Provider<ReviewDb> db;
private final Provider<CurrentUser> self; private final Provider<CurrentUser> self;
private final AuthConfig authConfig; private final AuthConfig authConfig;
@Inject @Inject
GetExternalIds(ExternalIdCache externalIdCache, GetExternalIds(Provider<ReviewDb> db,
Provider<CurrentUser> self, Provider<CurrentUser> self,
AuthConfig authConfig) { AuthConfig authConfig) {
this.externalIdCache = externalIdCache; this.db = db;
this.self = self; this.self = self;
this.authConfig = authConfig; this.authConfig = authConfig;
} }
@Override @Override
public List<AccountExternalIdInfo> apply(AccountResource resource) public List<AccountExternalIdInfo> apply(AccountResource resource)
throws RestApiException { throws RestApiException, OrmException {
if (self.get() != resource.getUser()) { if (self.get() != resource.getUser()) {
throw new AuthException("not allowed to get external IDs"); throw new AuthException("not allowed to get external IDs");
} }
Collection<AccountExternalId> ids = externalIdCache.byAccount( Collection<AccountExternalId> ids = db.get().accountExternalIds()
resource.getUser().getAccountId()); .byAccount(resource.getUser().getAccountId()).toList();
if (ids.isEmpty()) { if (ids.isEmpty()) {
return ImmutableList.of(); return ImmutableList.of();
} }

View File

@@ -60,17 +60,13 @@ public class PutHttpPassword implements RestModifyView<AccountResource, Input> {
private final Provider<CurrentUser> self; private final Provider<CurrentUser> self;
private final Provider<ReviewDb> dbProvider; private final Provider<ReviewDb> dbProvider;
private final AccountCache accountCache; private final AccountCache accountCache;
private final ExternalIdCache externalIdCache;
@Inject @Inject
PutHttpPassword(Provider<CurrentUser> self, PutHttpPassword(Provider<CurrentUser> self, Provider<ReviewDb> dbProvider,
Provider<ReviewDb> dbProvider, AccountCache accountCache) {
AccountCache accountCache,
ExternalIdCache externalIdCache) {
this.self = self; this.self = self;
this.dbProvider = dbProvider; this.dbProvider = dbProvider;
this.accountCache = accountCache; this.accountCache = accountCache;
this.externalIdCache = externalIdCache;
} }
@Override @Override
@@ -121,7 +117,6 @@ public class PutHttpPassword implements RestModifyView<AccountResource, Input> {
} }
id.setPassword(newPassword); id.setPassword(newPassword);
dbProvider.get().accountExternalIds().update(Collections.singleton(id)); dbProvider.get().accountExternalIds().update(Collections.singleton(id));
externalIdCache.onUpdate(id);
accountCache.evict(user.getAccountId()); accountCache.evict(user.getAccountId());
return Strings.isNullOrEmpty(newPassword) return Strings.isNullOrEmpty(newPassword)

View File

@@ -473,7 +473,11 @@ public class AccountApiImpl implements AccountApi {
@Override @Override
public List<AccountExternalIdInfo> getExternalIds() throws RestApiException { public List<AccountExternalIdInfo> getExternalIds() throws RestApiException {
return getExternalIds.apply(account); try {
return getExternalIds.apply(account);
} catch (OrmException e) {
throw new RestApiException("Cannot get external IDs", e);
}
} }
@Override @Override

View File

@@ -86,7 +86,6 @@ import com.google.gerrit.server.account.CapabilityCollection;
import com.google.gerrit.server.account.CapabilityControl; import com.google.gerrit.server.account.CapabilityControl;
import com.google.gerrit.server.account.ChangeUserName; import com.google.gerrit.server.account.ChangeUserName;
import com.google.gerrit.server.account.EmailExpander; import com.google.gerrit.server.account.EmailExpander;
import com.google.gerrit.server.account.ExternalIdCacheImpl;
import com.google.gerrit.server.account.GroupCacheImpl; import com.google.gerrit.server.account.GroupCacheImpl;
import com.google.gerrit.server.account.GroupControl; import com.google.gerrit.server.account.GroupControl;
import com.google.gerrit.server.account.GroupDetailFactory; import com.google.gerrit.server.account.GroupDetailFactory;
@@ -221,7 +220,6 @@ public class GerritGlobalModule extends FactoryModule {
install(AccountCacheImpl.module()); install(AccountCacheImpl.module());
install(ChangeKindCacheImpl.module()); install(ChangeKindCacheImpl.module());
install(ConflictsCacheImpl.module()); install(ConflictsCacheImpl.module());
install(ExternalIdCacheImpl.module());
install(GroupCacheImpl.module()); install(GroupCacheImpl.module());
install(GroupIncludeCacheImpl.module()); install(GroupIncludeCacheImpl.module());
install(MergeabilityCacheImpl.module()); install(MergeabilityCacheImpl.module());