opendev/gerrit
Code Issues Proposed changes

Do not read external IDs to be deleted outside of account update transaction

Browse Source 
If the external IDs that should be deleted are read outside of the
account update transaction it is possible that a race updates the
external IDs after they have been read and before the account update is
done. AccountUpdater already provides an AccountState that was
consistently read and from which the external IDs of the account can be
accessed. Get the external IDs from this AccountState instance instead
of reading them outside of the transaction.

Change-Id: I5d14840b1b7418831c789152ec37636241323a39
Signed-off-by: Edwin Kempin <ekempin@google.com>
This commit is contained in:
Edwin Kempin
2018-01-10 10:36:37 +01:00
parent 80696e6dc3
commit 06e676fc95
3 changed files with 29 additions and 22 deletions
Download Patch File Download Diff File Expand all files Collapse all files

10
javatests/com/google/gerrit/gpg/GerritPublicKeyCheckerTest.java
View File

@@ -40,7 +40,6 @@ import com.google.gerrit.server.account.AccountManager;
import com.google.gerrit.server.account.AccountsUpdate;
import com.google.gerrit.server.account.AuthRequest;
import com.google.gerrit.server.account.externalids.ExternalId;
import com.google.gerrit.server.account.externalids.ExternalIds;
import com.google.gerrit.server.schema.SchemaCreator;
import com.google.gerrit.server.util.RequestContext;
import com.google.gerrit.server.util.ThreadLocalRequestContext;
@@ -55,7 +54,6 @@ import com.google.inject.util.Providers;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.Set;
import org.bouncycastle.openpgp.PGPPublicKey;
import org.bouncycastle.openpgp.PGPPublicKeyRing;
import org.eclipse.jgit.internal.storage.dfs.DfsRepositoryDescription;
@@ -85,8 +83,6 @@ public class GerritPublicKeyCheckerTest {
@Inject private ThreadLocalRequestContext requestContext;
@Inject private ExternalIds externalIds;
private LifecycleManager lifecycle;
private ReviewDb db;
private Account.Id userId;
@@ -222,10 +218,12 @@ public class GerritPublicKeyCheckerTest {
@Test
public void noExternalIds() throws Exception {
Set<ExternalId> extIds = externalIds.byAccount(user.getAccountId());
accountsUpdate
.create()
.update("Delete External IDs", user.getAccountId(), u -> u.deleteExternalIds(extIds));
.update(
"Delete External IDs",
user.getAccountId(),
(a, u) -> u.deleteExternalIds(a.getExternalIds()));
reloadUser();
TestKey key = validKeyWithSecondUserId();