Evict account from cache on post/delete of GPG key
For each GPG key there is an external ID. This external ID is created/deleted/updated when a GPG key is posted/deleted. Since external IDs are cached with AccountState in the account cache, the account must be evicted from the account cache whenever an external ID is modified. This change also adapts AccountIT to read external IDs from the cache rather than looking them up directly from the database. This revealed the problem with not evicting the account from the account cache when a GPG key is posted/deleted. Change-Id: Ie65ef07bdd3511ded03edb2b2e01bef550ca09a2 Signed-off-by: Edwin Kempin <ekempin@google.com>
This commit is contained in:
		@@ -88,6 +88,7 @@ import org.junit.Test;
 | 
			
		||||
import java.io.ByteArrayOutputStream;
 | 
			
		||||
import java.util.ArrayList;
 | 
			
		||||
import java.util.Arrays;
 | 
			
		||||
import java.util.Collection;
 | 
			
		||||
import java.util.Collections;
 | 
			
		||||
import java.util.Iterator;
 | 
			
		||||
import java.util.List;
 | 
			
		||||
@@ -121,6 +122,8 @@ public class AccountIT extends AbstractDaemonTest {
 | 
			
		||||
    db.accountExternalIds().delete(getExternalIds(admin));
 | 
			
		||||
    db.accountExternalIds().delete(getExternalIds(user));
 | 
			
		||||
    db.accountExternalIds().insert(savedExternalIds);
 | 
			
		||||
    accountCache.evict(admin.getId());
 | 
			
		||||
    accountCache.evict(user.getId());
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
  @After
 | 
			
		||||
@@ -135,9 +138,9 @@ public class AccountIT extends AbstractDaemonTest {
 | 
			
		||||
    }
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
  private List<AccountExternalId> getExternalIds(TestAccount account)
 | 
			
		||||
  private Collection<AccountExternalId> getExternalIds(TestAccount account)
 | 
			
		||||
      throws Exception {
 | 
			
		||||
    return db.accountExternalIds().byAccount(account.getId()).toList();
 | 
			
		||||
    return accountCache.get(account.getId()).getExternalIds();
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
  @After
 | 
			
		||||
@@ -513,6 +516,7 @@ public class AccountIT extends AbstractDaemonTest {
 | 
			
		||||
        user.getId(), new AccountExternalId.Key("foo:myId"));
 | 
			
		||||
 | 
			
		||||
    db.accountExternalIds().insert(Collections.singleton(extId));
 | 
			
		||||
    accountCache.evict(user.getId());
 | 
			
		||||
 | 
			
		||||
    TestKey key = validKeyWithSecondUserId();
 | 
			
		||||
    addGpgKey(key.getPublicKeyArmored());
 | 
			
		||||
 
 | 
			
		||||
@@ -25,6 +25,7 @@ import com.google.gerrit.gpg.server.DeleteGpgKey.Input;
 | 
			
		||||
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.gwtorm.server.OrmException;
 | 
			
		||||
import com.google.inject.Inject;
 | 
			
		||||
import com.google.inject.Provider;
 | 
			
		||||
@@ -45,14 +46,17 @@ public class DeleteGpgKey implements RestModifyView<GpgKey, Input> {
 | 
			
		||||
  private final Provider<PersonIdent> serverIdent;
 | 
			
		||||
  private final Provider<ReviewDb> db;
 | 
			
		||||
  private final Provider<PublicKeyStore> storeProvider;
 | 
			
		||||
  private final AccountCache accountCache;
 | 
			
		||||
 | 
			
		||||
  @Inject
 | 
			
		||||
  DeleteGpgKey(@GerritPersonIdent Provider<PersonIdent> serverIdent,
 | 
			
		||||
      Provider<ReviewDb> db,
 | 
			
		||||
      Provider<PublicKeyStore> storeProvider) {
 | 
			
		||||
      Provider<PublicKeyStore> storeProvider,
 | 
			
		||||
      AccountCache accountCache) {
 | 
			
		||||
    this.serverIdent = serverIdent;
 | 
			
		||||
    this.db = db;
 | 
			
		||||
    this.storeProvider = storeProvider;
 | 
			
		||||
    this.accountCache = accountCache;
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
  @Override
 | 
			
		||||
@@ -64,6 +68,7 @@ public class DeleteGpgKey implements RestModifyView<GpgKey, Input> {
 | 
			
		||||
        AccountExternalId.SCHEME_GPGKEY,
 | 
			
		||||
        BaseEncoding.base16().encode(key.getFingerprint()));
 | 
			
		||||
    db.get().accountExternalIds().deleteKeys(Collections.singleton(extIdKey));
 | 
			
		||||
    accountCache.evict(rsrc.getUser().getAccountId());
 | 
			
		||||
 | 
			
		||||
    try (PublicKeyStore store = storeProvider.get()) {
 | 
			
		||||
      store.remove(rsrc.getKeyRing().getPublicKey().getFingerprint());
 | 
			
		||||
 
 | 
			
		||||
@@ -44,6 +44,7 @@ import com.google.gerrit.reviewdb.server.ReviewDb;
 | 
			
		||||
import com.google.gerrit.server.CurrentUser;
 | 
			
		||||
import com.google.gerrit.server.GerritPersonIdent;
 | 
			
		||||
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.mail.AddKeySender;
 | 
			
		||||
import com.google.gwtorm.server.OrmException;
 | 
			
		||||
@@ -85,6 +86,7 @@ public class PostGpgKeys implements RestModifyView<AccountResource, Input> {
 | 
			
		||||
  private final Provider<PublicKeyStore> storeProvider;
 | 
			
		||||
  private final GerritPublicKeyChecker.Factory checkerFactory;
 | 
			
		||||
  private final AddKeySender.Factory addKeyFactory;
 | 
			
		||||
  private final AccountCache accountCache;
 | 
			
		||||
 | 
			
		||||
  @Inject
 | 
			
		||||
  PostGpgKeys(@GerritPersonIdent Provider<PersonIdent> serverIdent,
 | 
			
		||||
@@ -92,13 +94,15 @@ public class PostGpgKeys implements RestModifyView<AccountResource, Input> {
 | 
			
		||||
      Provider<CurrentUser> self,
 | 
			
		||||
      Provider<PublicKeyStore> storeProvider,
 | 
			
		||||
      GerritPublicKeyChecker.Factory checkerFactory,
 | 
			
		||||
      AddKeySender.Factory addKeyFactory) {
 | 
			
		||||
      AddKeySender.Factory addKeyFactory,
 | 
			
		||||
      AccountCache accountCache) {
 | 
			
		||||
    this.serverIdent = serverIdent;
 | 
			
		||||
    this.db = db;
 | 
			
		||||
    this.self = self;
 | 
			
		||||
    this.storeProvider = storeProvider;
 | 
			
		||||
    this.checkerFactory = checkerFactory;
 | 
			
		||||
    this.addKeyFactory = addKeyFactory;
 | 
			
		||||
    this.accountCache = accountCache;
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
  @Override
 | 
			
		||||
@@ -141,6 +145,7 @@ public class PostGpgKeys implements RestModifyView<AccountResource, Input> {
 | 
			
		||||
              return toExtIdKey(fp.get());
 | 
			
		||||
            }
 | 
			
		||||
          }));
 | 
			
		||||
      accountCache.evict(rsrc.getUser().getAccountId());
 | 
			
		||||
      return toJson(newKeys, toRemove, store, rsrc.getUser());
 | 
			
		||||
    }
 | 
			
		||||
  }
 | 
			
		||||
 
 | 
			
		||||
		Reference in New Issue
	
	Block a user