More Accounts cleanup

* Remove `AccountStore`
* Clean up `AccountsDynamoDb#delete`
* Rename `AccountsDynamoDb` → `Accounts`
* Remove unused configuration
* Move Accounts scan page size to static configuration
* Remove disabled tests and related methods
This commit is contained in:
Chris Eager
2021-09-21 15:25:16 -07:00
committed by GitHub
parent 75661fa800
commit 6a71d369e2
16 changed files with 232 additions and 513 deletions

View File

@@ -313,29 +313,6 @@ class DynamicConfigurationTest {
}
}
@Test
void testParseAccountsDynamoDbMigrationConfiguration() throws JsonProcessingException {
{
final String emptyConfigYaml = "test: true";
final DynamicConfiguration emptyConfig =
DynamicConfigurationManager.parseConfiguration(emptyConfigYaml).orElseThrow();
assertEquals(10, emptyConfig.getAccountsDynamoDbMigrationConfiguration().getDynamoCrawlerScanPageSize());
}
{
final String accountsDynamoDbMigrationConfig =
"accountsDynamoDbMigration:\n"
+ " dynamoCrawlerScanPageSize: 5000";
final DynamicAccountsDynamoDbMigrationConfiguration config =
DynamicConfigurationManager.parseConfiguration(accountsDynamoDbMigrationConfig).orElseThrow()
.getAccountsDynamoDbMigrationConfiguration();
assertEquals(5000, config.getDynamoCrawlerScanPageSize());
}
}
@Test
void testParseLimits() throws JsonProcessingException {
{

View File

@@ -33,8 +33,6 @@ public class AccountDatabaseCrawlerIntegrationTest extends AbstractRedisClusterT
private AccountsManager accountsManager;
private AccountDatabaseCrawlerListener listener;
private DynamicConfigurationManager dynamicConfigurationManager;
private AccountDatabaseCrawler accountDatabaseCrawler;
private static final int CHUNK_SIZE = 1;
@@ -50,8 +48,6 @@ public class AccountDatabaseCrawlerIntegrationTest extends AbstractRedisClusterT
accountsManager = mock(AccountsManager.class);
listener = mock(AccountDatabaseCrawlerListener.class);
dynamicConfigurationManager = mock(DynamicConfigurationManager.class);
when(firstAccount.getUuid()).thenReturn(FIRST_UUID);
when(secondAccount.getUuid()).thenReturn(SECOND_UUID);

View File

@@ -15,11 +15,7 @@ import static org.mockito.Mockito.atLeast;
import static org.mockito.Mockito.doAnswer;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
import com.opentable.db.postgres.embedded.LiquibasePreparer;
import com.opentable.db.postgres.junit5.EmbeddedPostgresExtension;
import com.opentable.db.postgres.junit5.PreparedDbExtension;
import io.lettuce.core.cluster.api.sync.RedisAdvancedClusterCommands;
import java.io.IOException;
import java.time.Instant;
@@ -38,7 +34,6 @@ import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;
import org.mockito.ArgumentCaptor;
import org.whispersystems.textsecuregcm.auth.AuthenticationCredentials;
import org.whispersystems.textsecuregcm.configuration.dynamic.DynamicConfiguration;
import org.whispersystems.textsecuregcm.entities.AccountAttributes;
import org.whispersystems.textsecuregcm.entities.SignedPreKey;
import org.whispersystems.textsecuregcm.securebackup.SecureBackupClient;
@@ -55,23 +50,22 @@ import software.amazon.awssdk.services.dynamodb.model.ScalarAttributeType;
class AccountsManagerConcurrentModificationIntegrationTest {
@RegisterExtension
static PreparedDbExtension db = EmbeddedPostgresExtension.preparedDatabase(LiquibasePreparer.forClasspathLocation("accountsdb.xml"));
private static final String ACCOUNTS_TABLE_NAME = "accounts_test";
private static final String NUMBERS_TABLE_NAME = "numbers_test";
private static final int SCAN_PAGE_SIZE = 1;
@RegisterExtension
static DynamoDbExtension dynamoDbExtension = DynamoDbExtension.builder()
.tableName(ACCOUNTS_TABLE_NAME)
.hashKey(AccountsDynamoDb.KEY_ACCOUNT_UUID)
.hashKey(Accounts.KEY_ACCOUNT_UUID)
.attributeDefinition(AttributeDefinition.builder()
.attributeName(AccountsDynamoDb.KEY_ACCOUNT_UUID)
.attributeName(Accounts.KEY_ACCOUNT_UUID)
.attributeType(ScalarAttributeType.B)
.build())
.build();
private AccountsDynamoDb accountsDynamoDb;
private Accounts accounts;
private AccountsManager accountsManager;
@@ -86,11 +80,11 @@ class AccountsManagerConcurrentModificationIntegrationTest {
CreateTableRequest createNumbersTableRequest = CreateTableRequest.builder()
.tableName(NUMBERS_TABLE_NAME)
.keySchema(KeySchemaElement.builder()
.attributeName(AccountsDynamoDb.ATTR_ACCOUNT_E164)
.attributeName(Accounts.ATTR_ACCOUNT_E164)
.keyType(KeyType.HASH)
.build())
.attributeDefinitions(AttributeDefinition.builder()
.attributeName(AccountsDynamoDb.ATTR_ACCOUNT_E164)
.attributeName(Accounts.ATTR_ACCOUNT_E164)
.attributeType(ScalarAttributeType.S)
.build())
.provisionedThroughput(DynamoDbExtension.DEFAULT_PROVISIONED_THROUGHPUT)
@@ -99,18 +93,14 @@ class AccountsManagerConcurrentModificationIntegrationTest {
dynamoDbExtension.getDynamoDbClient().createTable(createNumbersTableRequest);
}
accountsDynamoDb = new AccountsDynamoDb(
accounts = new Accounts(
dynamoDbExtension.getDynamoDbClient(),
dynamoDbExtension.getTableName(),
NUMBERS_TABLE_NAME
);
NUMBERS_TABLE_NAME,
SCAN_PAGE_SIZE);
{
final DynamicConfigurationManager dynamicConfigurationManager = mock(DynamicConfigurationManager.class);
DynamicConfiguration dynamicConfiguration = new DynamicConfiguration();
when(dynamicConfigurationManager.getConfiguration()).thenReturn(dynamicConfiguration);
//noinspection unchecked
commands = mock(RedisAdvancedClusterCommands.class);
final DeletedAccountsManager deletedAccountsManager = mock(DeletedAccountsManager.class);
@@ -122,7 +112,7 @@ class AccountsManagerConcurrentModificationIntegrationTest {
}).when(deletedAccountsManager).lockAndTake(anyString(), any());
accountsManager = new AccountsManager(
accountsDynamoDb,
accounts,
RedisClusterHelper.buildMockRedisCluster(commands),
deletedAccountsManager,
mock(DirectoryQueue.class),
@@ -132,8 +122,8 @@ class AccountsManagerConcurrentModificationIntegrationTest {
mock(ProfilesManager.class),
mock(StoredVerificationCodeManager.class),
mock(SecureStorageClient.class),
mock(SecureBackupClient.class),
dynamicConfigurationManager);
mock(SecureBackupClient.class)
);
}
}
@@ -186,12 +176,12 @@ class AccountsManagerConcurrentModificationIntegrationTest {
modifyAccount(uuid, account -> account.setUnidentifiedAccessKey(unidentifiedAccessKey)),
modifyAccount(uuid, account -> account.setRegistrationLock(credentials.getHashedAuthenticationToken(), credentials.getSalt())),
modifyAccount(uuid, account -> account.setUnrestrictedUnidentifiedAccess(unrestrictedUnidentifiedAccess)),
modifyDevice(uuid, Device.MASTER_ID, device-> device.setLastSeen(lastSeen)),
modifyDevice(uuid, Device.MASTER_ID, device-> device.setName("deviceName"))
modifyDevice(uuid, Device.MASTER_ID, device -> device.setLastSeen(lastSeen)),
modifyDevice(uuid, Device.MASTER_ID, device -> device.setName("deviceName"))
).join();
final Account managerAccount = accountsManager.get(uuid).get();
final Account dynamoAccount = accountsDynamoDb.get(uuid).get();
final Account managerAccount = accountsManager.get(uuid).orElseThrow();
final Account dynamoAccount = accounts.get(uuid).orElseThrow();
final Account redisAccount = getLastAccountFromRedisMock(commands);
@@ -200,10 +190,9 @@ class AccountsManagerConcurrentModificationIntegrationTest {
new Pair<>("dynamo", dynamoAccount),
new Pair<>("redis", redisAccount)
).forEach(pair ->
verifyAccount(pair.first(), pair.second(), profileName, avatar, discoverableByPhoneNumber,
currentProfileVersion, identityKey, unidentifiedAccessKey, pin, registrationLock,
unrestrictedUnidentifiedAccess, lastSeen)
);
verifyAccount(pair.first(), pair.second(), profileName, avatar, discoverableByPhoneNumber,
currentProfileVersion, identityKey, unidentifiedAccessKey, pin, registrationLock,
unrestrictedUnidentifiedAccess, lastSeen));
}
private Account getLastAccountFromRedisMock(RedisAdvancedClusterCommands<String, String> commands) throws IOException {
@@ -220,9 +209,9 @@ class AccountsManagerConcurrentModificationIntegrationTest {
() -> assertEquals(profileName, account.getProfileName()),
() -> assertEquals(avatar, account.getAvatar()),
() -> assertEquals(discoverableByPhoneNumber, account.isDiscoverableByPhoneNumber()),
() -> assertEquals(currentProfileVersion, account.getCurrentProfileVersion().get()),
() -> assertEquals(currentProfileVersion, account.getCurrentProfileVersion().orElseThrow()),
() -> assertEquals(identityKey, account.getIdentityKey()),
() -> assertArrayEquals(unidentifiedAccessKey, account.getUnidentifiedAccessKey().get()),
() -> assertArrayEquals(unidentifiedAccessKey, account.getUnidentifiedAccessKey().orElseThrow()),
() -> assertTrue(account.getRegistrationLock().verify(clientRegistrationLock)),
() -> assertEquals(unrestrictedUnidentifiedAcces, account.isUnrestrictedUnidentifiedAccess())
);
@@ -231,7 +220,7 @@ class AccountsManagerConcurrentModificationIntegrationTest {
private CompletableFuture<?> modifyAccount(final UUID uuid, final Consumer<Account> accountMutation) {
return CompletableFuture.runAsync(() -> {
final Account account = accountsManager.get(uuid).get();
final Account account = accountsManager.get(uuid).orElseThrow();
accountsManager.update(account, accountMutation);
}, mutationExecutor);
}
@@ -239,7 +228,7 @@ class AccountsManagerConcurrentModificationIntegrationTest {
private CompletableFuture<?> modifyDevice(final UUID uuid, final long deviceId, final Consumer<Device> deviceMutation) {
return CompletableFuture.runAsync(() -> {
final Account account = accountsManager.get(uuid).get();
final Account account = accountsManager.get(uuid).orElseThrow();
accountsManager.updateDevice(account, deviceId, deviceMutation);
}, mutationExecutor);
}

View File

@@ -45,33 +45,35 @@ import software.amazon.awssdk.services.dynamodb.model.TransactWriteItemsRequest;
import software.amazon.awssdk.services.dynamodb.model.TransactionConflictException;
import software.amazon.awssdk.services.dynamodb.model.UpdateItemRequest;
class AccountsDynamoDbTest {
class AccountsTest {
private static final String ACCOUNTS_TABLE_NAME = "accounts_test";
private static final String NUMBERS_TABLE_NAME = "numbers_test";
private static final int SCAN_PAGE_SIZE = 1;
@RegisterExtension
static DynamoDbExtension dynamoDbExtension = DynamoDbExtension.builder()
.tableName(ACCOUNTS_TABLE_NAME)
.hashKey(AccountsDynamoDb.KEY_ACCOUNT_UUID)
.hashKey(Accounts.KEY_ACCOUNT_UUID)
.attributeDefinition(AttributeDefinition.builder()
.attributeName(AccountsDynamoDb.KEY_ACCOUNT_UUID)
.attributeName(Accounts.KEY_ACCOUNT_UUID)
.attributeType(ScalarAttributeType.B)
.build())
.build();
private AccountsDynamoDb accountsDynamoDb;
private Accounts accounts;
@BeforeEach
void setupAccountsDao() {
CreateTableRequest createNumbersTableRequest = CreateTableRequest.builder()
.tableName(NUMBERS_TABLE_NAME)
.keySchema(KeySchemaElement.builder()
.attributeName(AccountsDynamoDb.ATTR_ACCOUNT_E164)
.attributeName(Accounts.ATTR_ACCOUNT_E164)
.keyType(KeyType.HASH)
.build())
.attributeDefinitions(AttributeDefinition.builder()
.attributeName(AccountsDynamoDb.ATTR_ACCOUNT_E164)
.attributeName(Accounts.ATTR_ACCOUNT_E164)
.attributeType(ScalarAttributeType.S)
.build())
.provisionedThroughput(DynamoDbExtension.DEFAULT_PROVISIONED_THROUGHPUT)
@@ -79,11 +81,11 @@ class AccountsDynamoDbTest {
dynamoDbExtension.getDynamoDbClient().createTable(createNumbersTableRequest);
this.accountsDynamoDb = new AccountsDynamoDb(
this.accounts = new Accounts(
dynamoDbExtension.getDynamoDbClient(),
dynamoDbExtension.getTableName(),
NUMBERS_TABLE_NAME
);
NUMBERS_TABLE_NAME,
SCAN_PAGE_SIZE);
}
@Test
@@ -91,12 +93,12 @@ class AccountsDynamoDbTest {
Device device = generateDevice (1 );
Account account = generateAccount("+14151112222", UUID.randomUUID(), Collections.singleton(device));
boolean freshUser = accountsDynamoDb.create(account);
boolean freshUser = accounts.create(account);
assertThat(freshUser).isTrue();
verifyStoredState("+14151112222", account.getUuid(), account, true);
freshUser = accountsDynamoDb.create(account);
freshUser = accounts.create(account);
assertThat(freshUser).isTrue();
verifyStoredState("+14151112222", account.getUuid(), account, true);
@@ -110,7 +112,7 @@ class AccountsDynamoDbTest {
Account account = generateAccount("+14151112222", UUID.randomUUID(), devices);
accountsDynamoDb.create(account);
accounts.create(account);
verifyStoredState("+14151112222", account.getUuid(), account, true);
}
@@ -131,11 +133,11 @@ class AccountsDynamoDbTest {
UUID uuidSecond = UUID.randomUUID();
Account accountSecond = generateAccount("+14152221111", uuidSecond, devicesSecond);
accountsDynamoDb.create(accountFirst);
accountsDynamoDb.create(accountSecond);
accounts.create(accountFirst);
accounts.create(accountSecond);
Optional<Account> retrievedFirst = accountsDynamoDb.get("+14151112222");
Optional<Account> retrievedSecond = accountsDynamoDb.get("+14152221111");
Optional<Account> retrievedFirst = accounts.get("+14151112222");
Optional<Account> retrievedSecond = accounts.get("+14152221111");
assertThat(retrievedFirst.isPresent()).isTrue();
assertThat(retrievedSecond.isPresent()).isTrue();
@@ -143,8 +145,8 @@ class AccountsDynamoDbTest {
verifyStoredState("+14151112222", uuidFirst, retrievedFirst.get(), accountFirst);
verifyStoredState("+14152221111", uuidSecond, retrievedSecond.get(), accountSecond);
retrievedFirst = accountsDynamoDb.get(uuidFirst);
retrievedSecond = accountsDynamoDb.get(uuidSecond);
retrievedFirst = accounts.get(uuidFirst);
retrievedSecond = accounts.get(uuidSecond);
assertThat(retrievedFirst.isPresent()).isTrue();
assertThat(retrievedSecond.isPresent()).isTrue();
@@ -159,27 +161,27 @@ class AccountsDynamoDbTest {
UUID firstUuid = UUID.randomUUID();
Account account = generateAccount("+14151112222", firstUuid, Collections.singleton(device));
accountsDynamoDb.create(account);
accounts.create(account);
verifyStoredState("+14151112222", account.getUuid(), account, true);
account.setProfileName("name");
accountsDynamoDb.update(account);
accounts.update(account);
UUID secondUuid = UUID.randomUUID();
device = generateDevice(1);
account = generateAccount("+14151112222", secondUuid, Collections.singleton(device));
final boolean freshUser = accountsDynamoDb.create(account);
final boolean freshUser = accounts.create(account);
assertThat(freshUser).isFalse();
verifyStoredState("+14151112222", firstUuid, account, true);
device = generateDevice(1);
Account invalidAccount = generateAccount("+14151113333", firstUuid, Collections.singleton(device));
assertThatThrownBy(() -> accountsDynamoDb.create(invalidAccount));
assertThatThrownBy(() -> accounts.create(invalidAccount));
}
@Test
@@ -187,18 +189,18 @@ class AccountsDynamoDbTest {
Device device = generateDevice (1 );
Account account = generateAccount("+14151112222", UUID.randomUUID(), Collections.singleton(device));
accountsDynamoDb.create(account);
accounts.create(account);
device.setName("foobar");
accountsDynamoDb.update(account);
accounts.update(account);
Optional<Account> retrieved = accountsDynamoDb.get("+14151112222");
Optional<Account> retrieved = accounts.get("+14151112222");
assertThat(retrieved.isPresent()).isTrue();
verifyStoredState("+14151112222", account.getUuid(), retrieved.get(), account);
retrieved = accountsDynamoDb.get(account.getUuid());
retrieved = accounts.get(account.getUuid());
assertThat(retrieved.isPresent()).isTrue();
verifyStoredState("+14151112222", account.getUuid(), account, true);
@@ -206,11 +208,11 @@ class AccountsDynamoDbTest {
device = generateDevice(1);
Account unknownAccount = generateAccount("+14151113333", UUID.randomUUID(), Collections.singleton(device));
assertThatThrownBy(() -> accountsDynamoDb.update(unknownAccount)).isInstanceOfAny(ConditionalCheckFailedException.class);
assertThatThrownBy(() -> accounts.update(unknownAccount)).isInstanceOfAny(ConditionalCheckFailedException.class);
account.setProfileName("name");
accountsDynamoDb.update(account);
accounts.update(account);
assertThat(account.getVersion()).isEqualTo(2);
@@ -218,12 +220,12 @@ class AccountsDynamoDbTest {
account.setVersion(1);
assertThatThrownBy(() -> accountsDynamoDb.update(account)).isInstanceOfAny(ContestedOptimisticLockException.class);
assertThatThrownBy(() -> accounts.update(account)).isInstanceOfAny(ContestedOptimisticLockException.class);
account.setVersion(2);
account.setProfileName("name2");
accountsDynamoDb.update(account);
accounts.update(account);
verifyStoredState("+14151112222", account.getUuid(), account, true);
}
@@ -232,8 +234,8 @@ class AccountsDynamoDbTest {
void testUpdateWithMockTransactionConflictException() {
final DynamoDbClient dynamoDbClient = mock(DynamoDbClient.class);
accountsDynamoDb = new AccountsDynamoDb(dynamoDbClient,
dynamoDbExtension.getTableName(), NUMBERS_TABLE_NAME);
accounts = new Accounts(dynamoDbClient,
dynamoDbExtension.getTableName(), NUMBERS_TABLE_NAME, SCAN_PAGE_SIZE);
when(dynamoDbClient.updateItem(any(UpdateItemRequest.class)))
.thenThrow(TransactionConflictException.class);
@@ -241,7 +243,7 @@ class AccountsDynamoDbTest {
Device device = generateDevice(1);
Account account = generateAccount("+14151112222", UUID.randomUUID(), Collections.singleton(device));
assertThatThrownBy(() -> accountsDynamoDb.update(account)).isInstanceOfAny(ContestedOptimisticLockException.class);
assertThatThrownBy(() -> accounts.update(account)).isInstanceOfAny(ContestedOptimisticLockException.class);
}
@Test
@@ -251,12 +253,12 @@ class AccountsDynamoDbTest {
for (int i = 1; i <= 100; i++) {
Account account = generateAccount("+1" + String.format("%03d", i), UUID.randomUUID());
users.add(account);
accountsDynamoDb.create(account);
accounts.create(account);
}
users.sort((account, t1) -> UUIDComparator.staticCompare(account.getUuid(), t1.getUuid()));
AccountCrawlChunk retrieved = accountsDynamoDb.getAllFromStart(10, 1);
AccountCrawlChunk retrieved = accounts.getAllFromStart(10);
assertThat(retrieved.getAccounts().size()).isEqualTo(10);
for (int i = 0; i < retrieved.getAccounts().size(); i++) {
@@ -273,7 +275,7 @@ class AccountsDynamoDbTest {
}
for (int j = 0; j < 9; j++) {
retrieved = accountsDynamoDb.getAllFrom(retrieved.getLastUuid().orElseThrow(), 10, 1);
retrieved = accounts.getAllFrom(retrieved.getLastUuid().orElseThrow(), 10);
assertThat(retrieved.getAccounts().size()).isEqualTo(10);
for (int i = 0; i < retrieved.getAccounts().size(); i++) {
@@ -295,33 +297,36 @@ class AccountsDynamoDbTest {
@Test
void testDelete() {
final Device deletedDevice = generateDevice (1);
final Account deletedAccount = generateAccount("+14151112222", UUID.randomUUID(), Collections.singleton(deletedDevice));
final Device retainedDevice = generateDevice (1);
final Account retainedAccount = generateAccount("+14151112345", UUID.randomUUID(), Collections.singleton(retainedDevice));
final Device deletedDevice = generateDevice(1);
final Account deletedAccount = generateAccount("+14151112222", UUID.randomUUID(),
Collections.singleton(deletedDevice));
final Device retainedDevice = generateDevice(1);
final Account retainedAccount = generateAccount("+14151112345", UUID.randomUUID(),
Collections.singleton(retainedDevice));
accountsDynamoDb.create(deletedAccount);
accountsDynamoDb.create(retainedAccount);
accounts.create(deletedAccount);
accounts.create(retainedAccount);
assertThat(accountsDynamoDb.get(deletedAccount.getUuid())).isPresent();
assertThat(accountsDynamoDb.get(retainedAccount.getUuid())).isPresent();
assertThat(accounts.get(deletedAccount.getUuid())).isPresent();
assertThat(accounts.get(retainedAccount.getUuid())).isPresent();
accountsDynamoDb.delete(deletedAccount.getUuid());
accounts.delete(deletedAccount.getUuid());
assertThat(accountsDynamoDb.get(deletedAccount.getUuid())).isNotPresent();
assertThat(accounts.get(deletedAccount.getUuid())).isNotPresent();
verifyStoredState(retainedAccount.getNumber(), retainedAccount.getUuid(), accountsDynamoDb.get(retainedAccount.getUuid()).get(), retainedAccount);
verifyStoredState(retainedAccount.getNumber(), retainedAccount.getUuid(),
accounts.get(retainedAccount.getUuid()).get(), retainedAccount);
{
final Account recreatedAccount = generateAccount(deletedAccount.getNumber(), UUID.randomUUID(),
Collections.singleton(generateDevice(1)));
final boolean freshUser = accountsDynamoDb.create(recreatedAccount);
final boolean freshUser = accounts.create(recreatedAccount);
assertThat(freshUser).isTrue();
assertThat(accountsDynamoDb.get(recreatedAccount.getUuid())).isPresent();
assertThat(accounts.get(recreatedAccount.getUuid())).isPresent();
verifyStoredState(recreatedAccount.getNumber(), recreatedAccount.getUuid(),
accountsDynamoDb.get(recreatedAccount.getUuid()).get(), recreatedAccount);
accounts.get(recreatedAccount.getUuid()).get(), recreatedAccount);
}
}
@@ -330,12 +335,12 @@ class AccountsDynamoDbTest {
Device device = generateDevice (1 );
Account account = generateAccount("+14151112222", UUID.randomUUID(), Collections.singleton(device));
accountsDynamoDb.create(account);
accounts.create(account);
Optional<Account> retrieved = accountsDynamoDb.get("+11111111");
Optional<Account> retrieved = accounts.get("+11111111");
assertThat(retrieved.isPresent()).isFalse();
retrieved = accountsDynamoDb.get(UUID.randomUUID());
retrieved = accounts.get(UUID.randomUUID());
assertThat(retrieved.isPresent()).isFalse();
}
@@ -357,7 +362,7 @@ class AccountsDynamoDbTest {
when(client.updateItem(any(UpdateItemRequest.class)))
.thenThrow(RuntimeException.class);
AccountsDynamoDb accounts = new AccountsDynamoDb(client, ACCOUNTS_TABLE_NAME, NUMBERS_TABLE_NAME);
Accounts accounts = new Accounts(client, ACCOUNTS_TABLE_NAME, NUMBERS_TABLE_NAME, SCAN_PAGE_SIZE);
Account account = generateAccount("+14151112222", UUID.randomUUID());
try {
@@ -397,13 +402,13 @@ class AccountsDynamoDbTest {
UUID uuid = UUID.randomUUID();
Account account = generateAccount("+14151112222", uuid, Collections.singleton(device));
account.setDiscoverableByPhoneNumber(false);
accountsDynamoDb.create(account);
accounts.create(account);
verifyStoredState("+14151112222", account.getUuid(), account, false);
account.setDiscoverableByPhoneNumber(true);
accountsDynamoDb.update(account);
accounts.update(account);
verifyStoredState("+14151112222", account.getUuid(), account, true);
account.setDiscoverableByPhoneNumber(false);
accountsDynamoDb.update(account);
accounts.update(account);
verifyStoredState("+14151112222", account.getUuid(), account, false);
}
@@ -433,20 +438,21 @@ class AccountsDynamoDbTest {
final GetItemResponse get = db.getItem(GetItemRequest.builder()
.tableName(dynamoDbExtension.getTableName())
.key(Map.of(AccountsDynamoDb.KEY_ACCOUNT_UUID, AttributeValues.fromUUID(uuid)))
.key(Map.of(Accounts.KEY_ACCOUNT_UUID, AttributeValues.fromUUID(uuid)))
.consistentRead(true)
.build());
if (get.hasItem()) {
String data = new String(get.item().get(AccountsDynamoDb.ATTR_ACCOUNT_DATA).b().asByteArray(), StandardCharsets.UTF_8);
String data = new String(get.item().get(Accounts.ATTR_ACCOUNT_DATA).b().asByteArray(), StandardCharsets.UTF_8);
assertThat(data).isNotEmpty();
assertThat(AttributeValues.getInt(get.item(), AccountsDynamoDb.ATTR_VERSION, -1))
assertThat(AttributeValues.getInt(get.item(), Accounts.ATTR_VERSION, -1))
.isEqualTo(expecting.getVersion());
assertThat(AttributeValues.getBool(get.item(), AccountsDynamoDb.ATTR_CANONICALLY_DISCOVERABLE, !canonicallyDiscoverable)).isEqualTo(canonicallyDiscoverable);
assertThat(AttributeValues.getBool(get.item(), Accounts.ATTR_CANONICALLY_DISCOVERABLE,
!canonicallyDiscoverable)).isEqualTo(canonicallyDiscoverable);
Account result = AccountsDynamoDb.fromItem(get.item());
Account result = Accounts.fromItem(get.item());
verifyStoredState(number, uuid, result, expecting);
} else {
throw new AssertionError("No data");

View File

@@ -6,9 +6,7 @@
package org.whispersystems.textsecuregcm.tests.storage;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotSame;
import static org.junit.jupiter.api.Assertions.assertSame;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.argThat;
@@ -26,20 +24,17 @@ import static org.mockito.Mockito.when;
import io.lettuce.core.RedisException;
import io.lettuce.core.cluster.api.sync.RedisAdvancedClusterCommands;
import java.io.IOException;
import java.util.HashSet;
import java.util.Optional;
import java.util.UUID;
import java.util.function.Consumer;
import java.util.stream.Stream;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;
import org.junit.jupiter.params.provider.ValueSource;
import org.mockito.ArgumentCaptor;
import org.mockito.stubbing.Answer;
import org.whispersystems.textsecuregcm.configuration.dynamic.DynamicConfiguration;
import org.whispersystems.textsecuregcm.entities.AccountAttributes;
@@ -48,7 +43,7 @@ import org.whispersystems.textsecuregcm.securebackup.SecureBackupClient;
import org.whispersystems.textsecuregcm.securestorage.SecureStorageClient;
import org.whispersystems.textsecuregcm.sqs.DirectoryQueue;
import org.whispersystems.textsecuregcm.storage.Account;
import org.whispersystems.textsecuregcm.storage.AccountsDynamoDb;
import org.whispersystems.textsecuregcm.storage.Accounts;
import org.whispersystems.textsecuregcm.storage.AccountsManager;
import org.whispersystems.textsecuregcm.storage.ContestedOptimisticLockException;
import org.whispersystems.textsecuregcm.storage.DeletedAccountsManager;
@@ -60,12 +55,11 @@ import org.whispersystems.textsecuregcm.storage.MessagesManager;
import org.whispersystems.textsecuregcm.storage.ProfilesManager;
import org.whispersystems.textsecuregcm.storage.StoredVerificationCodeManager;
import org.whispersystems.textsecuregcm.storage.UsernamesManager;
import org.whispersystems.textsecuregcm.tests.util.JsonHelpers;
import org.whispersystems.textsecuregcm.tests.util.RedisClusterHelper;
class AccountsManagerTest {
private AccountsDynamoDb accountsDynamoDb;
private Accounts accounts;
private DeletedAccountsManager deletedAccountsManager;
private DirectoryQueue directoryQueue;
private DynamicConfigurationManager dynamicConfigurationManager;
@@ -86,7 +80,7 @@ class AccountsManagerTest {
@BeforeEach
void setup() throws InterruptedException {
accountsDynamoDb = mock(AccountsDynamoDb.class);
accounts = mock(Accounts.class);
deletedAccountsManager = mock(DeletedAccountsManager.class);
directoryQueue = mock(DirectoryQueue.class);
dynamicConfigurationManager = mock(DynamicConfigurationManager.class);
@@ -107,7 +101,7 @@ class AccountsManagerTest {
}).when(deletedAccountsManager).lockAndTake(anyString(), any());
accountsManager = new AccountsManager(
accountsDynamoDb,
accounts,
RedisClusterHelper.buildMockRedisCluster(commands),
deletedAccountsManager,
directoryQueue,
@@ -117,8 +111,8 @@ class AccountsManagerTest {
profilesManager,
mock(StoredVerificationCodeManager.class),
mock(SecureStorageClient.class),
mock(SecureBackupClient.class),
dynamicConfigurationManager);
mock(SecureBackupClient.class)
);
}
@Test
@@ -138,7 +132,7 @@ class AccountsManagerTest {
verify(commands, times(1)).get(eq("Account3::" + uuid));
verifyNoMoreInteractions(commands);
verifyNoInteractions(accountsDynamoDb);
verifyNoInteractions(accounts);
}
@Test
@@ -157,7 +151,7 @@ class AccountsManagerTest {
verify(commands, times(1)).get(eq("Account3::" + uuid));
verifyNoMoreInteractions(commands);
verifyNoInteractions(accountsDynamoDb);
verifyNoInteractions(accounts);
}
@@ -168,7 +162,7 @@ class AccountsManagerTest {
Account account = new Account("+14152222222", uuid, new HashSet<>(), new byte[16]);
when(commands.get(eq("AccountMap::+14152222222"))).thenReturn(null);
when(accountsDynamoDb.get(eq("+14152222222"))).thenReturn(Optional.of(account));
when(accounts.get(eq("+14152222222"))).thenReturn(Optional.of(account));
Optional<Account> retrieved = accountsManager.get("+14152222222");
@@ -180,8 +174,8 @@ class AccountsManagerTest {
verify(commands, times(1)).set(eq("Account3::" + uuid), anyString());
verifyNoMoreInteractions(commands);
verify(accountsDynamoDb, times(1)).get(eq("+14152222222"));
verifyNoMoreInteractions(accountsDynamoDb);
verify(accounts, times(1)).get(eq("+14152222222"));
verifyNoMoreInteractions(accounts);
}
@Test
@@ -191,7 +185,7 @@ class AccountsManagerTest {
Account account = new Account("+14152222222", uuid, new HashSet<>(), new byte[16]);
when(commands.get(eq("Account3::" + uuid))).thenReturn(null);
when(accountsDynamoDb.get(eq(uuid))).thenReturn(Optional.of(account));
when(accounts.get(eq(uuid))).thenReturn(Optional.of(account));
Optional<Account> retrieved = accountsManager.get(uuid);
@@ -203,8 +197,8 @@ class AccountsManagerTest {
verify(commands, times(1)).set(eq("Account3::" + uuid), anyString());
verifyNoMoreInteractions(commands);
verify(accountsDynamoDb, times(1)).get(eq(uuid));
verifyNoMoreInteractions(accountsDynamoDb);
verify(accounts, times(1)).get(eq(uuid));
verifyNoMoreInteractions(accounts);
}
@Test
@@ -213,7 +207,7 @@ class AccountsManagerTest {
Account account = new Account("+14152222222", uuid, new HashSet<>(), new byte[16]);
when(commands.get(eq("AccountMap::+14152222222"))).thenThrow(new RedisException("Connection lost!"));
when(accountsDynamoDb.get(eq("+14152222222"))).thenReturn(Optional.of(account));
when(accounts.get(eq("+14152222222"))).thenReturn(Optional.of(account));
Optional<Account> retrieved = accountsManager.get("+14152222222");
@@ -225,18 +219,17 @@ class AccountsManagerTest {
verify(commands, times(1)).set(eq("Account3::" + uuid), anyString());
verifyNoMoreInteractions(commands);
verify(accountsDynamoDb, times(1)).get(eq("+14152222222"));
verifyNoMoreInteractions(accountsDynamoDb);
verify(accounts, times(1)).get(eq("+14152222222"));
verifyNoMoreInteractions(accounts);
}
@Test
void testGetAccountByUuidBrokenCache() {
final boolean dynamoEnabled = true;
UUID uuid = UUID.randomUUID();
Account account = new Account("+14152222222", uuid, new HashSet<>(), new byte[16]);
when(commands.get(eq("Account3::" + uuid))).thenThrow(new RedisException("Connection lost!"));
when(accountsDynamoDb.get(eq(uuid))).thenReturn(Optional.of(account));
when(accounts.get(eq(uuid))).thenReturn(Optional.of(account));
Optional<Account> retrieved = accountsManager.get(uuid);
@@ -248,76 +241,8 @@ class AccountsManagerTest {
verify(commands, times(1)).set(eq("Account3::" + uuid), anyString());
verifyNoMoreInteractions(commands);
verify(accountsDynamoDb, times(1)).get(eq(uuid));
verifyNoMoreInteractions(accountsDynamoDb);
}
// TODO delete
@Disabled("migration specific")
@Test
void testUpdate_dynamoDbMigration() throws IOException {
UUID uuid = UUID.randomUUID();
Account account = new Account("+14152222222", uuid, new HashSet<>(), new byte[16]);
when(commands.get(eq("Account3::" + uuid))).thenReturn(null);
// database fetches should always return new instances
when(accountsDynamoDb.get(uuid)).thenReturn(
Optional.of(new Account("+14152222222", uuid, new HashSet<>(), new byte[16])));
when(accountsDynamoDb.get(uuid)).thenReturn(
Optional.of(new Account("+14152222222", uuid, new HashSet<>(), new byte[16])));
doAnswer(ACCOUNT_UPDATE_ANSWER).when(accountsDynamoDb).update(any(Account.class));
Account updatedAccount = accountsManager.update(account, a -> a.setProfileName("name"));
assertThrows(AssertionError.class, account::getProfileName, "Account passed to update() should be stale");
assertNotSame(updatedAccount, account);
verify(accountsDynamoDb, times(1)).update(account);
verifyNoMoreInteractions(accountsDynamoDb);
ArgumentCaptor<Account> argumentCaptor = ArgumentCaptor.forClass(Account.class);
verify(accountsDynamoDb, times(1)).update(argumentCaptor.capture());
assertEquals(uuid, argumentCaptor.getValue().getUuid());
verify(accountsDynamoDb, times(1)).get(uuid);
verifyNoMoreInteractions(accountsDynamoDb);
ArgumentCaptor<String> redisSetArgumentCapture = ArgumentCaptor.forClass(String.class);
verify(commands, times(2)).set(anyString(), redisSetArgumentCapture.capture());
Account accountCached = JsonHelpers.fromJson(redisSetArgumentCapture.getAllValues().get(1), Account.class);
// uuid is @JsonIgnore, so we need to set it for compareAccounts to work
accountCached.setUuid(uuid);
assertEquals(Optional.empty(),
accountsManager.compareAccounts(Optional.of(updatedAccount), Optional.of(accountCached)));
}
// TODO delete
@Disabled("migration specific")
@Test
void testUpdate_dynamoMissing() {
UUID uuid = UUID.randomUUID();
Account account = new Account("+14152222222", uuid, new HashSet<>(), new byte[16]);
when(commands.get(eq("Account3::" + uuid))).thenReturn(null);
when(accountsDynamoDb.get(uuid)).thenReturn(Optional.empty());
doAnswer(ACCOUNT_UPDATE_ANSWER).when(accountsDynamoDb).update(any());
Account updatedAccount = accountsManager.update(account, a -> {
});
verify(accountsDynamoDb, times(1)).update(account);
verifyNoMoreInteractions(accountsDynamoDb);
verify(accountsDynamoDb, never()).update(account);
verify(accountsDynamoDb, times(1)).get(uuid);
verifyNoMoreInteractions(accountsDynamoDb);
assertEquals(1, updatedAccount.getVersion());
verify(accounts, times(1)).get(eq(uuid));
verifyNoMoreInteractions(accounts);
}
@Test
@@ -327,52 +252,51 @@ class AccountsManagerTest {
when(commands.get(eq("Account3::" + uuid))).thenReturn(null);
when(accountsDynamoDb.get(uuid)).thenReturn(
when(accounts.get(uuid)).thenReturn(
Optional.of(new Account("+14152222222", uuid, new HashSet<>(), new byte[16])));
doThrow(ContestedOptimisticLockException.class)
.doAnswer(ACCOUNT_UPDATE_ANSWER)
.when(accountsDynamoDb).update(any());
.when(accounts).update(any());
when(accountsDynamoDb.get(uuid)).thenReturn(
when(accounts.get(uuid)).thenReturn(
Optional.of(new Account("+14152222222", uuid, new HashSet<>(), new byte[16])));
doThrow(ContestedOptimisticLockException.class)
.doAnswer(ACCOUNT_UPDATE_ANSWER)
.when(accountsDynamoDb).update(any());
.when(accounts).update(any());
account = accountsManager.update(account, a -> a.setProfileName("name"));
assertEquals(1, account.getVersion());
assertEquals("name", account.getProfileName());
verify(accountsDynamoDb, times(1)).get(uuid);
verify(accountsDynamoDb, times(2)).update(any());
verifyNoMoreInteractions(accountsDynamoDb);
verify(accounts, times(1)).get(uuid);
verify(accounts, times(2)).update(any());
verifyNoMoreInteractions(accounts);
}
@Test
void testUpdate_dynamoOptimisticLockingFailureDuringCreate() {
UUID uuid = UUID.randomUUID();
Account account = new Account("+14152222222", uuid, new HashSet<>(), new byte[16]);
UUID uuid = UUID.randomUUID();
Account account = new Account("+14152222222", uuid, new HashSet<>(), new byte[16]);
when(commands.get(eq("Account3::" + uuid))).thenReturn(null);
when(accountsDynamoDb.get(uuid)).thenReturn(Optional.empty())
.thenReturn(Optional.of(account));
when(accountsDynamoDb.create(any())).thenThrow(ContestedOptimisticLockException.class);
when(accounts.get(uuid)).thenReturn(Optional.empty())
.thenReturn(Optional.of(account));
when(accounts.create(any())).thenThrow(ContestedOptimisticLockException.class);
accountsManager.update(account, a -> {});
accountsManager.update(account, a -> {
});
verify(accountsDynamoDb, times(1)).update(account);
verifyNoMoreInteractions(accountsDynamoDb);
verify(accounts, times(1)).update(account);
verifyNoMoreInteractions(accounts);
}
@Test
void testUpdateDevice() {
assertEquals(Optional.empty(), accountsManager.compareAccounts(Optional.empty(), Optional.empty()));
final UUID uuid = UUID.randomUUID();
Account account = new Account("+14152222222", uuid, new HashSet<>(), new byte[16]);
when(accountsDynamoDb.get(uuid)).thenReturn(
when(accounts.get(uuid)).thenReturn(
Optional.of(new Account("+14152222222", uuid, new HashSet<>(), new byte[16])));
assertTrue(account.getDevices().isEmpty());
@@ -400,90 +324,15 @@ class AccountsManagerTest {
verify(unknownDeviceUpdater, never()).accept(any(Device.class));
}
// TODO delete
@Disabled("migration specific")
@Test
void testCompareAccounts() throws Exception {
assertEquals(Optional.empty(), accountsManager.compareAccounts(Optional.empty(), Optional.empty()));
final UUID uuidA = UUID.randomUUID();
final Account a1 = new Account("+14152222222", uuidA, new HashSet<>(), new byte[16]);
assertEquals(Optional.of("primaryMissing"), accountsManager.compareAccounts(Optional.empty(), Optional.of(a1)));
final Account a2 = new Account("+14152222222", uuidA, new HashSet<>(), new byte[16]);
assertEquals(Optional.empty(), accountsManager.compareAccounts(Optional.of(a1), Optional.of(a2)));
{
Device device1 = new Device();
device1.setId(1L);
a1.addDevice(device1);
assertEquals(Optional.of("devices"), accountsManager.compareAccounts(Optional.of(a1), Optional.of(a2)));
Device device2 = new Device();
device2.setId(1L);
a2.addDevice(device2);
assertEquals(Optional.empty(), accountsManager.compareAccounts(Optional.of(a1), Optional.of(a2)));
device1.setLastSeen(1L);
assertEquals(Optional.empty(), accountsManager.compareAccounts(Optional.of(a1), Optional.of(a2)));
device1.setName("name");
assertEquals(Optional.of("devices"), accountsManager.compareAccounts(Optional.of(a1), Optional.of(a2)));
device1.setName(null);
device1.setSignedPreKey(new SignedPreKey(1L, "123", "456"));
device2.setSignedPreKey(new SignedPreKey(2L, "123", "456"));
assertEquals(Optional.of("masterDeviceSignedPreKey"), accountsManager.compareAccounts(Optional.of(a1), Optional.of(a2)));
device1.setSignedPreKey(null);
device2.setSignedPreKey(null);
assertEquals(Optional.empty(), accountsManager.compareAccounts(Optional.of(a1), Optional.of(a2)));
device1.setApnId("123");
Thread.sleep(5);
device2.setApnId("123");
assertEquals(Optional.empty(), accountsManager.compareAccounts(Optional.of(a1), Optional.of(a2)));
a1.removeDevice(1L);
a2.removeDevice(1L);
assertEquals(Optional.empty(), accountsManager.compareAccounts(Optional.of(a1), Optional.of(a2)));
}
assertEquals(Optional.empty(), accountsManager.compareAccounts(Optional.of(a1), Optional.of(a2)));
a1.setVersion(1);
assertEquals(Optional.of("version"), accountsManager.compareAccounts(Optional.of(a1), Optional.of(a2)));
a2.setVersion(1);
a2.setProfileName("name");
assertEquals(Optional.of("profileName"), accountsManager.compareAccounts(Optional.of(a1), Optional.of(a2)));
}
@Test
void testCreateFreshAccount() throws InterruptedException {
when(accountsDynamoDb.create(any())).thenReturn(true);
when(accounts.create(any())).thenReturn(true);
final String e164 = "+18005550123";
final AccountAttributes attributes = new AccountAttributes(false, 0, null, null, true, null);
accountsManager.create(e164, "password", null, attributes);
verify(accountsDynamoDb).create(argThat(account -> e164.equals(account.getNumber())));
verify(accounts).create(argThat(account -> e164.equals(account.getNumber())));
verifyNoInteractions(keys);
verifyNoInteractions(messagesManager);
verifyNoInteractions(profilesManager);
@@ -493,7 +342,7 @@ class AccountsManagerTest {
void testReregisterAccount() throws InterruptedException {
final UUID existingUuid = UUID.randomUUID();
when(accountsDynamoDb.create(any())).thenAnswer(invocation -> {
when(accounts.create(any())).thenAnswer(invocation -> {
invocation.getArgument(0, Account.class).setUuid(existingUuid);
return false;
});
@@ -502,7 +351,7 @@ class AccountsManagerTest {
final AccountAttributes attributes = new AccountAttributes(false, 0, null, null, true, null);
accountsManager.create(e164, "password", null, attributes);
verify(accountsDynamoDb).create(
verify(accounts).create(
argThat(account -> e164.equals(account.getNumber()) && existingUuid.equals(account.getUuid())));
verify(keys).delete(existingUuid);
verify(messagesManager).clear(existingUuid);
@@ -519,13 +368,13 @@ class AccountsManagerTest {
return null;
}).when(deletedAccountsManager).lockAndTake(anyString(), any());
when(accountsDynamoDb.create(any())).thenReturn(true);
when(accounts.create(any())).thenReturn(true);
final String e164 = "+18005550123";
final AccountAttributes attributes = new AccountAttributes(false, 0, null, null, true, null);
accountsManager.create(e164, "password", null, attributes);
verify(accountsDynamoDb).create(
verify(accounts).create(
argThat(account -> e164.equals(account.getNumber()) && recentlyDeletedUuid.equals(account.getUuid())));
verifyNoInteractions(keys);
verifyNoInteractions(messagesManager);
@@ -596,7 +445,7 @@ class AccountsManagerTest {
accountsManager.updateDeviceLastSeen(account, device, updatedLastSeen);
assertEquals(expectUpdate ? updatedLastSeen : initialLastSeen, device.getLastSeen());
verify(accountsDynamoDb, expectUpdate ? times(1) : never()).update(account);
verify(accounts, expectUpdate ? times(1) : never()).update(account);
}
@SuppressWarnings("unused")