opendev/gerrit
Code Issues Proposed changes

Add cache for external ids

Browse Source 
Introducing a cache for external ids is a preparation step for moving
the external ids from ReviewDb into NoteDb. For NoteDb it is planned
to store the external ids in a git note branch, where the note keys
are the sha1's of the external ids and the note values contain the
external id, the account id and optionally email and password. With
this format we can easily lookup external ids by the external id, but
listing all external ids of an account requires parsing all external
ids. Looking up the external ids of an account is possible from the
account index, however for reindexing an account we would still need
to lookup all external ids of the account from git. Having a cache for
the external ids ensures that the external ids are only loaded once
from git. If there is any update to external ids, we take care to
update the cache as well. For this change it means that all code that
modifies external ids must do an extra call to update the external id
cache. This is not optimal since updating the cache can be easily
forgotten. This is why the follow-up change cleans this up by
introducing a dedicated class that handles all external id updates and
then this is the only class that must take care to update the external
id cache.

Change-Id: I9ea979c646cddb9b39e723de5c061a70a2ce6fd6
Signed-off-by: Edwin Kempin <ekempin@google.com>
This commit is contained in:
Edwin Kempin
2016-12-21 14:34:32 +01:00
parent 57e0378dab
commit 2869caaf70
19 changed files with 395 additions and 62 deletions
Download Patch File Download Diff File Expand all files Collapse all files

7
gerrit-gpg/src/main/java/com/google/gerrit/gpg/server/DeleteGpgKey.java
View File

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

21
gerrit-gpg/src/main/java/com/google/gerrit/gpg/server/GpgKeys.java
View File

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

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