opendev/gerrit
Code Issues Proposed changes

Revert external ID cache

Browse Source 
It make more sense to include the commit SHA-1 of the notes branch from
which the external IDs have been read into the cache key. This makes it
easier to implement this cache within a multimaster setup.
However, obviously, we can include the commit SHA-1 of the notes branch
into the cache key only after the external IDs have been migrated to
NoteDb (because before this the notes branch does not exist yet).
Revert the cache for now and re-add an improved version of the cache
later with the change that implements the migration of the external IDs
to NoteDb.

This reverts the following commits:
- 2869caaf70:
  Add cache for external ids
- 48d5c9b9aa:
  Make ExternalIdCacheImpl.AllKey public
- 20e5507d46:
  ExternalIdCache: Add method to get external IDs by account ID + scheme

Change-Id: I589242aad32a9c70718542ba950c0a351c594e54
Signed-off-by: Edwin Kempin <ekempin@google.com>
This commit is contained in:
Edwin Kempin
2017-02-01 11:39:17 +01:00
parent a461a3d45a
commit 79e63a39e4
22 changed files with 93 additions and 418 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AccountCreator.java
View File

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

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

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

@@ -26,7 +26,6 @@ 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;
@@ -48,19 +47,16 @@ 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,
ExternalIdCache externalIdCache) {
AccountCache accountCache) {
this.serverIdent = serverIdent;
this.db = db;
this.storeProvider = storeProvider;
this.accountCache = accountCache;
this.externalIdCache = externalIdCache;
}
@Override
@@ -72,7 +68,6 @@ 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()) {

23
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,
ExternalIdCache externalIdCache) {
GerritPublicKeyChecker.Factory checkerFactory) {
this.views = views;
this.db = db;
this.self = self;
this.storeProvider = storeProvider;
this.checkerFactory = checkerFactory;
this.externalIdCache = externalIdCache;
}
@Override
@@ -208,14 +208,15 @@ public class GpgKeys implements
}
@VisibleForTesting
public static FluentIterable<AccountExternalId> getGpgExtIds(
ExternalIdCache externalIdCache, Account.Id accountId) {
return FluentIterable.from(
externalIdCache.byAccount(accountId, SCHEME_GPGKEY));
public static FluentIterable<AccountExternalId> getGpgExtIds(ReviewDb db,
Account.Id accountId) throws OrmException {
return FluentIterable.from(db.accountExternalIds().byAccount(accountId))
.filter(in -> in.isScheme(SCHEME_GPGKEY));
}
private Iterable<AccountExternalId> getGpgExtIds(AccountResource rsrc) {
return getGpgExtIds(externalIdCache, rsrc.getUser().getAccountId());
private Iterable<AccountExternalId> getGpgExtIds(AccountResource rsrc)
throws OrmException {
return getGpgExtIds(db.get(), 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,7 +47,6 @@ 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;
@@ -91,7 +90,6 @@ 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,
@@ -101,8 +99,7 @@ public class PostGpgKeys implements RestModifyView<AccountResource, Input> {
GerritPublicKeyChecker.Factory checkerFactory,
AddKeySender.Factory addKeyFactory,
AccountCache accountCache,
Provider<InternalAccountQuery> accountQueryProvider,
ExternalIdCache externalIdCache) {
Provider<InternalAccountQuery> accountQueryProvider) {
this.serverIdent = serverIdent;
this.db = db;
this.self = self;
@@ -111,7 +108,6 @@ public class PostGpgKeys implements RestModifyView<AccountResource, Input> {
this.addKeyFactory = addKeyFactory;
this.accountCache = accountCache;
this.accountQueryProvider = accountQueryProvider;
this.externalIdCache = externalIdCache;
}
@Override
@@ -121,8 +117,7 @@ public class PostGpgKeys implements RestModifyView<AccountResource, Input> {
GpgKeys.checkVisible(self, rsrc);
List<AccountExternalId> existingExtIds =
GpgKeys.getGpgExtIds(externalIdCache,
rsrc.getUser().getAccountId()).toList();
GpgKeys.getGpgExtIds(db.get(), rsrc.getUser().getAccountId()).toList();
try (PublicKeyStore store = storeProvider.get()) {
Set<Fingerprint> toRemove = readKeysToRemove(input, existingExtIds);
@@ -147,13 +142,9 @@ public class PostGpgKeys implements RestModifyView<AccountResource, Input> {
storeKeys(rsrc, newKeys, toRemove);
if (!newExtIds.isEmpty()) {
db.get().accountExternalIds().insert(newExtIds);
externalIdCache.onCreate(newExtIds);
}
List<AccountExternalId.Key> extIdKeysToRemove =
toRemove.stream().map(fp -> toExtIdKey(fp.get())).collect(toList());
db.get().accountExternalIds().deleteKeys(extIdKeysToRemove);
externalIdCache.onRemove(rsrc.getUser().getAccountId(), extIdKeysToRemove);
db.get().accountExternalIds().deleteKeys(
Iterables.transform(toRemove, fp -> toExtIdKey(fp.get())));
accountCache.evict(rsrc.getUser().getAccountId());
return toJson(newKeys, toRemove, store, rsrc.getUser());
}

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

17
gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/account/DeleteExternalIds.java
View File

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

19
gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/account/ExternalIdDetailFactory.java
View File

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

2
gerrit-pgm/src/main/java/com/google/gerrit/pgm/util/BatchProgramModule.java
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.CapabilityCollection;
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.GroupCacheImpl;
import com.google.gerrit.server.account.GroupIncludeCacheImpl;
@@ -154,7 +153,6 @@ public class BatchProgramModule extends FactoryModule {
install(new PrologModule());
install(AccountByEmailCacheImpl.module());
install(AccountCacheImpl.module());
install(ExternalIdCacheImpl.module());
install(GroupCacheImpl.module());
install(GroupIncludeCacheImpl.module());
install(ProjectCacheImpl.module());

19
gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/AccountExternalId.java
View File

@@ -18,8 +18,6 @@ import com.google.gerrit.extensions.client.AuthType;
import com.google.gwtorm.client.Column;
import com.google.gwtorm.client.StringKey;
import java.util.Objects;
/** Association of an external account identifier to a local {@link Account}. */
public final class AccountExternalId {
/**
@@ -169,21 +167,4 @@ public final class AccountExternalId {
public void setCanDelete(final boolean 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);
}
}

4
gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/server/AccountExternalIdAccess.java
View File

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

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

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

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

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

30
gerrit-server/src/main/java/com/google/gerrit/server/account/ChangeUserName.java
View File

@@ -30,6 +30,7 @@ import com.google.inject.Inject;
import com.google.inject.assistedinject.Assisted;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.concurrent.Callable;
@@ -50,22 +51,19 @@ public class ChangeUserName implements Callable<VoidResult> {
private final AccountCache accountCache;
private final SshKeyCache sshKeyCache;
private final ExternalIdCache externalIdCache;
private final ReviewDb db;
private final IdentifiedUser user;
private final String newUsername;
@Inject
ChangeUserName(AccountCache accountCache,
SshKeyCache sshKeyCache,
ExternalIdCache externalIdCache,
@Assisted ReviewDb db,
@Assisted IdentifiedUser user,
@Nullable @Assisted String newUsername) {
ChangeUserName(final AccountCache accountCache,
final SshKeyCache sshKeyCache,
@Assisted final ReviewDb db, @Assisted final IdentifiedUser user,
@Nullable @Assisted final String newUsername) {
this.accountCache = accountCache;
this.sshKeyCache = sshKeyCache;
this.externalIdCache = externalIdCache;
this.db = db;
this.user = user;
@@ -75,8 +73,7 @@ public class ChangeUserName implements Callable<VoidResult> {
@Override
public VoidResult call() throws OrmException, NameAlreadyUsedException,
InvalidUserNameException, IOException {
Collection<AccountExternalId> old =
externalIdCache.byAccount(user.getAccountId(), SCHEME_USERNAME);
final Collection<AccountExternalId> old = old();
if (!old.isEmpty()) {
throw new IllegalStateException(USERNAME_CANNOT_BE_CHANGED);
}
@@ -99,7 +96,6 @@ public class ChangeUserName implements Callable<VoidResult> {
}
db.accountExternalIds().insert(Collections.singleton(id));
externalIdCache.onCreate(id);
} catch (OrmDuplicateKeyException dupeErr) {
// 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.
//
db.accountExternalIds().delete(old);
externalIdCache.onRemove(old);
for (AccountExternalId i : old) {
sshKeyCache.evict(i.getSchemeRest());
accountCache.evictByUsername(i.getSchemeRest());
@@ -128,4 +123,15 @@ public class ChangeUserName implements Callable<VoidResult> {
sshKeyCache.evict(newUsername);
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;
}
}

6
gerrit-server/src/main/java/com/google/gerrit/server/account/CreateAccount.java
View File

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

10
gerrit-server/src/main/java/com/google/gerrit/server/account/DeleteExternalIds.java
View File

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

52
gerrit-server/src/main/java/com/google/gerrit/server/account/ExternalIdCache.java
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());
}
}

193
gerrit-server/src/main/java/com/google/gerrit/server/account/ExternalIdCacheImpl.java
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);
}
}
}
}

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

9
gerrit-server/src/main/java/com/google/gerrit/server/account/PutHttpPassword.java
View File

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

4
gerrit-server/src/main/java/com/google/gerrit/server/api/accounts/AccountApiImpl.java
View File

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

2
gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java
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.ChangeUserName;
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.GroupControl;
import com.google.gerrit.server.account.GroupDetailFactory;
@@ -220,7 +219,6 @@ public class GerritGlobalModule extends FactoryModule {
install(AccountCacheImpl.module());
install(ChangeKindCacheImpl.module());
install(ConflictsCacheImpl.module());
install(ExternalIdCacheImpl.module());
install(GroupCacheImpl.module());
install(GroupIncludeCacheImpl.module());
install(MergeabilityCacheImpl.module());