mirror of
https://github.com/signalapp/Signal-Server
synced 2026-04-20 13:27:59 +01:00
Remove asynchronous account update plumbing
This commit is contained in:
committed by
Jon Chambers
parent
de458c3e6a
commit
31de1fc1a3
@@ -241,7 +241,7 @@ class AccountsGrpcServiceTest extends SimpleBaseGrpcTest<AccountsGrpcService, Ac
|
||||
GrpcTestUtils.assertStatusException(Status.INVALID_ARGUMENT,
|
||||
() -> authenticatedServiceStub().clearRegistrationLock(ClearRegistrationLockRequest.newBuilder().build()));
|
||||
|
||||
verify(accountsManager, never()).updateAsync(any(), any());
|
||||
verify(accountsManager, never()).update(any(), any());
|
||||
}
|
||||
|
||||
@Test
|
||||
|
||||
@@ -138,15 +138,6 @@ class AccountsManagerTest {
|
||||
return null;
|
||||
};
|
||||
|
||||
private static final Answer<CompletableFuture<Void>> ACCOUNT_UPDATE_ASYNC_ANSWER = invocation -> {
|
||||
// it is implicit in the update() contract is that a successful call will
|
||||
// result in an incremented version
|
||||
final Account updatedAccount = invocation.getArgument(0, Account.class);
|
||||
updatedAccount.setVersion(updatedAccount.getVersion() + 1);
|
||||
|
||||
return CompletableFuture.completedFuture(null);
|
||||
};
|
||||
|
||||
@BeforeEach
|
||||
void setup() throws Exception {
|
||||
accounts = mock(Accounts.class);
|
||||
@@ -169,7 +160,6 @@ class AccountsManagerTest {
|
||||
when(asyncClusterCommands.set(any(), any(), any())).thenReturn(MockRedisFuture.completedFuture("OK"));
|
||||
when(asyncClusterCommands.setex(any(), anyLong(), any())).thenReturn(MockRedisFuture.completedFuture("OK"));
|
||||
|
||||
when(accounts.updateAsync(any())).thenReturn(CompletableFuture.completedFuture(null));
|
||||
when(accounts.updateTransactionallyAsync(any(), any())).thenReturn(CompletableFuture.completedFuture(null));
|
||||
when(accounts.delete(any(), any())).thenReturn(CompletableFuture.completedFuture(null));
|
||||
|
||||
@@ -643,33 +633,6 @@ class AccountsManagerTest {
|
||||
verifyNoMoreInteractions(accounts);
|
||||
}
|
||||
|
||||
@Test
|
||||
void testUpdateAsync_optimisticLockingFailure() {
|
||||
UUID uuid = UUID.randomUUID();
|
||||
UUID pni = UUID.randomUUID();
|
||||
Account account = AccountsHelper.generateTestAccount("+14152222222", uuid, pni, new ArrayList<>(), new byte[UnidentifiedAccessUtil.UNIDENTIFIED_ACCESS_KEY_LENGTH]);
|
||||
|
||||
when(asyncClusterCommands.get(eq("Account3::" + uuid))).thenReturn(null);
|
||||
|
||||
when(accounts.getByAccountIdentifierAsync(uuid)).thenReturn(CompletableFuture.completedFuture(
|
||||
Optional.of(AccountsHelper.generateTestAccount("+14152222222", uuid, pni, new ArrayList<>(), new byte[UnidentifiedAccessUtil.UNIDENTIFIED_ACCESS_KEY_LENGTH]))));
|
||||
|
||||
when(accounts.updateAsync(any()))
|
||||
.thenReturn(CompletableFuture.failedFuture(new ContestedOptimisticLockException()))
|
||||
.thenAnswer(ACCOUNT_UPDATE_ASYNC_ANSWER);
|
||||
|
||||
final IdentityKey identityKey = new IdentityKey(ECKeyPair.generate().getPublicKey());
|
||||
|
||||
account = accountsManager.updateAsync(account, a -> a.setIdentityKey(identityKey)).join();
|
||||
|
||||
assertEquals(1, account.getVersion());
|
||||
assertEquals(identityKey, account.getIdentityKey(IdentityType.ACI));
|
||||
|
||||
verify(accounts, times(1)).getByAccountIdentifierAsync(uuid);
|
||||
verify(accounts, times(2)).updateAsync(any());
|
||||
verifyNoMoreInteractions(accounts);
|
||||
}
|
||||
|
||||
@Test
|
||||
void testUpdate_dynamoOptimisticLockingFailureDuringCreate() throws AccountAlreadyExistsException {
|
||||
UUID uuid = UUID.randomUUID();
|
||||
|
||||
@@ -604,15 +604,14 @@ class AccountsTest {
|
||||
verifyStoredState("+14151112222", account.getUuid(), account.getPhoneNumberIdentifier(), null, account, true);
|
||||
}
|
||||
|
||||
@ParameterizedTest
|
||||
@ValueSource(booleans = {true, false})
|
||||
void testUpdateWithMockTransactionConflictException(boolean wrapException) {
|
||||
@Test
|
||||
void testUpdateWithMockTransactionConflictException() {
|
||||
|
||||
final DynamoDbAsyncClient dynamoDbAsyncClient = mock(DynamoDbAsyncClient.class);
|
||||
final DynamoDbClient dynamoDbClient = mock(DynamoDbClient.class);
|
||||
accounts = new Accounts(
|
||||
clock,
|
||||
mock(DynamoDbClient.class),
|
||||
dynamoDbAsyncClient,
|
||||
dynamoDbClient,
|
||||
mock(DynamoDbAsyncClient.class),
|
||||
Tables.ACCOUNTS.tableName(),
|
||||
Tables.NUMBERS.tableName(),
|
||||
Tables.PNI_ASSIGNMENTS.tableName(),
|
||||
@@ -620,13 +619,10 @@ class AccountsTest {
|
||||
Tables.DELETED_ACCOUNTS.tableName(),
|
||||
Tables.USED_LINK_DEVICE_TOKENS.tableName());
|
||||
|
||||
Exception e = TransactionConflictException.builder().build();
|
||||
e = wrapException ? new CompletionException(e) : e;
|
||||
when(dynamoDbClient.updateItem(any(UpdateItemRequest.class)))
|
||||
.thenThrow(TransactionConflictException.builder().build());
|
||||
|
||||
when(dynamoDbAsyncClient.updateItem(any(UpdateItemRequest.class)))
|
||||
.thenReturn(CompletableFuture.failedFuture(e));
|
||||
|
||||
Account account = generateAccount("+14151112222", UUID.randomUUID(), UUID.randomUUID());
|
||||
final Account account = generateAccount("+14151112222", UUID.randomUUID(), UUID.randomUUID());
|
||||
|
||||
assertThatThrownBy(() -> accounts.update(account)).isInstanceOfAny(ContestedOptimisticLockException.class);
|
||||
}
|
||||
|
||||
@@ -20,7 +20,6 @@ import java.util.List;
|
||||
import java.util.Optional;
|
||||
import java.util.Set;
|
||||
import java.util.UUID;
|
||||
import java.util.concurrent.CompletableFuture;
|
||||
import java.util.function.Consumer;
|
||||
import org.mockito.MockingDetails;
|
||||
import org.mockito.stubbing.Stubbing;
|
||||
@@ -65,7 +64,6 @@ public class AccountsHelper {
|
||||
* Sets up stubbing for:
|
||||
* <ul>
|
||||
* <li>{@link AccountsManager#update(Account, Consumer)}</li>
|
||||
* <li>{@link AccountsManager#updateAsync(Account, Consumer)}</li>
|
||||
* <li>{@link AccountsManager#updateDevice(Account, byte, Consumer)}</li>
|
||||
* </ul>
|
||||
*
|
||||
@@ -91,16 +89,6 @@ public class AccountsHelper {
|
||||
return copyAndMarkStale(account);
|
||||
});
|
||||
|
||||
when(mockAccountsManager.updateAsync(any(), any())).thenAnswer(answer -> {
|
||||
final Account account = answer.getArgument(0, Account.class);
|
||||
|
||||
for (int i = 0; i < retryCount; i++) {
|
||||
answer.getArgument(1, Consumer.class).accept(account);
|
||||
}
|
||||
|
||||
return CompletableFuture.completedFuture(copyAndMarkStale(account));
|
||||
});
|
||||
|
||||
when(mockAccountsManager.updateDevice(any(), anyByte(), any())).thenAnswer(answer -> {
|
||||
final Account account = answer.getArgument(0, Account.class);
|
||||
final byte deviceId = answer.getArgument(1, Byte.class);
|
||||
@@ -122,13 +110,6 @@ public class AccountsHelper {
|
||||
return markStale ? copyAndMarkStale(account) : account;
|
||||
});
|
||||
|
||||
when(mockAccountsManager.updateAsync(any(), any())).thenAnswer(answer -> {
|
||||
final Account account = answer.getArgument(0, Account.class);
|
||||
answer.getArgument(1, Consumer.class).accept(account);
|
||||
|
||||
return CompletableFuture.completedFuture(markStale ? copyAndMarkStale(account) : account);
|
||||
});
|
||||
|
||||
when(mockAccountsManager.updateDevice(any(), anyByte(), any())).thenAnswer(answer -> {
|
||||
final Account account = answer.getArgument(0, Account.class);
|
||||
final byte deviceId = answer.getArgument(1, Byte.class);
|
||||
|
||||
@@ -6,7 +6,6 @@
|
||||
package org.whispersystems.textsecuregcm.workers;
|
||||
|
||||
import static org.assertj.core.api.Assertions.assertThat;
|
||||
import static org.mockito.ArgumentMatchers.any;
|
||||
import static org.mockito.ArgumentMatchers.eq;
|
||||
import static org.mockito.Mockito.argThat;
|
||||
import static org.mockito.Mockito.mock;
|
||||
@@ -24,7 +23,6 @@ import java.util.Arrays;
|
||||
import java.util.Collections;
|
||||
import java.util.Iterator;
|
||||
import java.util.List;
|
||||
import java.util.concurrent.CompletableFuture;
|
||||
import java.util.concurrent.ThreadLocalRandom;
|
||||
import java.util.function.Consumer;
|
||||
import java.util.stream.IntStream;
|
||||
@@ -75,9 +73,6 @@ class RemoveExpiredUsernameHoldsCommandTest {
|
||||
final TestClock clock = TestClock.pinned(Instant.EPOCH.plus(Duration.ofSeconds(1)));
|
||||
|
||||
final AccountsManager accountsManager = mock(AccountsManager.class);
|
||||
when(accountsManager.updateAsync(any(), any()))
|
||||
.thenReturn(CompletableFuture.completedFuture(null));
|
||||
|
||||
final RemoveExpiredUsernameHoldsCommand removeExpiredUsernameHoldsCommand =
|
||||
new TestRemoveExpiredUsernameHoldsCommand(clock, accountsManager, isDryRun);
|
||||
|
||||
@@ -97,7 +92,7 @@ class RemoveExpiredUsernameHoldsCommandTest {
|
||||
verifyNoInteractions(accountsManager);
|
||||
} else {
|
||||
ArgumentCaptor<Consumer<Account>> updaterCaptor = ArgumentCaptor.forClass(Consumer.class);
|
||||
verify(accountsManager, times(1)).updateAsync(eq(hasHolds), updaterCaptor.capture());
|
||||
verify(accountsManager, times(1)).update(eq(hasHolds), updaterCaptor.capture());
|
||||
final Consumer<Account> consumer = updaterCaptor.getValue();
|
||||
consumer.accept(hasHolds);
|
||||
verify(hasHolds, times(1)).setUsernameHolds(argThat(holds ->
|
||||
|
||||
Reference in New Issue
Block a user