diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/storage/AccountsManager.java b/service/src/main/java/org/whispersystems/textsecuregcm/storage/AccountsManager.java index 68a327adc..097dae31b 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/storage/AccountsManager.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/storage/AccountsManager.java @@ -30,7 +30,6 @@ import java.util.Collection; import java.util.Collections; import java.util.List; import java.util.Map; -import java.util.NoSuchElementException; import java.util.Objects; import java.util.Optional; import java.util.Queue; @@ -85,9 +84,6 @@ public class AccountsManager { private static final Timer deleteTimer = metricRegistry.timer(name(AccountsManager.class, "delete")); private static final Timer redisSetTimer = metricRegistry.timer(name(AccountsManager.class, "redisSet")); - private static final Timer redisNumberGetTimer = metricRegistry.timer(name(AccountsManager.class, "redisNumberGet")); - private static final Timer redisUsernameHashGetTimer = metricRegistry.timer(name(AccountsManager.class, "redisUsernameHashGet")); - private static final Timer redisUsernameLinkHandleGetTimer = metricRegistry.timer(name(AccountsManager.class, "redisUsernameLinkHandleGet")); private static final Timer redisPniGetTimer = metricRegistry.timer(name(AccountsManager.class, "redisPniGet")); private static final Timer redisUuidGetTimer = metricRegistry.timer(name(AccountsManager.class, "redisUuidGet")); private static final Timer redisDeleteTimer = metricRegistry.timer(name(AccountsManager.class, "redisDelete")); @@ -875,19 +871,13 @@ public class AccountsManager { } public Optional getByE164(final String number) { - return checkRedisThenAccounts( - getByNumberTimer, - () -> redisGetBySecondaryKey(getAccountMapKey(number), redisNumberGetTimer), - () -> accounts.getByE164(number) - ); + return getByNumberTimer.timeSupplier(() -> accounts.getByE164(number)); } public CompletableFuture> getByE164Async(final String number) { - return checkRedisThenAccountsAsync( - getByNumberTimer, - () -> redisGetBySecondaryKeyAsync(getAccountMapKey(number), redisNumberGetTimer), - () -> accounts.getByE164Async(number) - ); + final Timer.Context context = getByNumberTimer.time(); + return accounts.getByE164Async(number) + .whenComplete((ignoredResult, ignoredThrowable) -> context.close()); } public Optional getByPhoneNumberIdentifier(final UUID pni) { @@ -907,19 +897,15 @@ public class AccountsManager { } public CompletableFuture> getByUsernameLinkHandle(final UUID usernameLinkHandle) { - return checkRedisThenAccountsAsync( - getByUsernameLinkHandleTimer, - () -> redisGetBySecondaryKeyAsync(getAccountMapKey(usernameLinkHandle.toString()), redisUsernameLinkHandleGetTimer), - () -> accounts.getByUsernameLinkHandle(usernameLinkHandle) - ); + final Timer.Context context = getByUsernameLinkHandleTimer.time(); + return accounts.getByUsernameLinkHandle(usernameLinkHandle) + .whenComplete((ignoredResult, ignoredThrowable) -> context.close()); } public CompletableFuture> getByUsernameHash(final byte[] usernameHash) { - return checkRedisThenAccountsAsync( - getByUsernameHashTimer, - () -> redisGetBySecondaryKeyAsync(getUsernameHashAccountMapKey(usernameHash), redisUsernameHashGetTimer), - () -> accounts.getByUsernameHash(usernameHash) - ); + final Timer.Context context = getByUsernameHashTimer.time(); + return accounts.getByUsernameHash(usernameHash) + .whenComplete((ignoredResult, ignoredThrowable) -> context.close()); } public Optional getByServiceIdentifier(final ServiceIdentifier serviceIdentifier) { @@ -1030,11 +1016,7 @@ public class AccountsManager { final RedisAdvancedClusterCommands commands = connection.sync(); commands.setex(getAccountMapKey(account.getPhoneNumberIdentifier().toString()), CACHE_TTL_SECONDS, account.getUuid().toString()); - commands.setex(getAccountMapKey(account.getNumber()), CACHE_TTL_SECONDS, account.getUuid().toString()); commands.setex(getAccountEntityKey(account.getUuid()), CACHE_TTL_SECONDS, accountJson); - - account.getUsernameHash().ifPresent(usernameHash -> - commands.setex(getUsernameHashAccountMapKey(usernameHash), CACHE_TTL_SECONDS, account.getUuid().toString())); }); } catch (JsonProcessingException e) { throw new IllegalStateException(e); @@ -1055,20 +1037,8 @@ public class AccountsManager { getAccountMapKey(account.getPhoneNumberIdentifier().toString()), CACHE_TTL_SECONDS, account.getUuid().toString()) .toCompletableFuture(), - - connection.async() - .setex(getAccountMapKey(account.getNumber()), CACHE_TTL_SECONDS, account.getUuid().toString()) - .toCompletableFuture(), - connection.async().setex(getAccountEntityKey(account.getUuid()), CACHE_TTL_SECONDS, accountJson) - .toCompletableFuture(), - - account.getUsernameHash() - .map(usernameHash -> connection.async() - .setex(getUsernameHashAccountMapKey(usernameHash), CACHE_TTL_SECONDS, account.getUuid().toString()) - .toCompletableFuture()) - .orElseGet(() -> CompletableFuture.completedFuture(null)) - )); + .toCompletableFuture())); } private Optional checkRedisThenAccounts( @@ -1186,11 +1156,8 @@ public class AccountsManager { try (final Timer.Context ignored = redisDeleteTimer.time()) { cacheCluster.useCluster(connection -> { connection.sync().del( - getAccountMapKey(account.getNumber()), getAccountMapKey(account.getPhoneNumberIdentifier().toString()), getAccountEntityKey(account.getUuid())); - - account.getUsernameHash().ifPresent(usernameHash -> connection.sync().del(getUsernameHashAccountMapKey(usernameHash))); }); } } @@ -1198,15 +1165,14 @@ public class AccountsManager { private CompletableFuture redisDeleteAsync(final Account account) { @SuppressWarnings("resource") final Timer.Context timerContext = redisDeleteTimer.time(); - final List keysToDelete = new ArrayList<>(4); - keysToDelete.add(getAccountMapKey(account.getNumber())); - keysToDelete.add(getAccountMapKey(account.getPhoneNumberIdentifier().toString())); - keysToDelete.add(getAccountEntityKey(account.getUuid())); + final String[] keysToDelete = new String[]{ + getAccountMapKey(account.getPhoneNumberIdentifier().toString()), + getAccountEntityKey(account.getUuid()) + }; - account.getUsernameHash().ifPresent(usernameHash -> keysToDelete.add(getUsernameHashAccountMapKey(usernameHash))); - - return cacheCluster.withCluster(connection -> connection.async().del(keysToDelete.toArray(new String[0]))) + return cacheCluster.withCluster(connection -> connection.async().del(keysToDelete)) .toCompletableFuture() - .thenRun(timerContext::close); + .whenComplete((ignoredResult, ignoredException) -> timerContext.close()) + .thenRun(Util.NOOP); } } diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerTest.java index 98ceace50..a118f832f 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerTest.java @@ -280,51 +280,6 @@ class AccountsManagerTest { assertFalse(accountsManager.getByServiceIdentifierAsync(new PniServiceIdentifier(aci)).join().isPresent()); } - @Test - void testGetAccountByNumberInCache() { - UUID uuid = UUID.randomUUID(); - - when(commands.get(eq("AccountMap::+14152222222"))).thenReturn(uuid.toString()); - when(commands.get(eq("Account3::" + uuid))).thenReturn( - "{\"number\": \"+14152222222\", \"pni\": \"de24dc73-fbd8-41be-a7d5-764c70d9da7e\"}"); - - Optional account = accountsManager.getByE164("+14152222222"); - - assertTrue(account.isPresent()); - assertEquals(account.get().getNumber(), "+14152222222"); - assertEquals(UUID.fromString("de24dc73-fbd8-41be-a7d5-764c70d9da7e"), account.get().getPhoneNumberIdentifier()); - - verify(commands, times(1)).get(eq("AccountMap::+14152222222")); - verify(commands, times(1)).get(eq("Account3::" + uuid)); - verifyNoMoreInteractions(commands); - - verifyNoInteractions(accounts); - } - - @Test - void testGetAccountByNumberAsyncInCache() { - UUID uuid = UUID.randomUUID(); - - when(asyncCommands.get(eq("AccountMap::+14152222222"))) - .thenReturn(MockRedisFuture.completedFuture(uuid.toString())); - - when(asyncCommands.get(eq("Account3::" + uuid))).thenReturn(MockRedisFuture.completedFuture( - "{\"number\": \"+14152222222\", \"pni\": \"de24dc73-fbd8-41be-a7d5-764c70d9da7e\"}")); - - when(asyncCommands.setex(any(), anyLong(), any())).thenReturn(MockRedisFuture.completedFuture("OK")); - - Optional account = accountsManager.getByE164Async("+14152222222").join(); - - assertTrue(account.isPresent()); - assertEquals(account.get().getNumber(), "+14152222222"); - assertEquals(UUID.fromString("de24dc73-fbd8-41be-a7d5-764c70d9da7e"), account.get().getPhoneNumberIdentifier()); - - verify(asyncCommands).get(eq("AccountMap::+14152222222")); - verify(asyncCommands).get(eq("Account3::" + uuid)); - verifyNoMoreInteractions(asyncCommands); - - verifyNoInteractions(accounts); - } @Test void testGetAccountByUuidInCache() { @@ -416,80 +371,6 @@ class AccountsManagerTest { verifyNoInteractions(accounts); } - @Test - void testGetAccountByUsernameHashInCache() { - UUID uuid = UUID.randomUUID(); - when(asyncCommands.get(eq("UAccountMap::" + BASE_64_URL_USERNAME_HASH_1))) - .thenReturn(MockRedisFuture.completedFuture(uuid.toString())); - - when(asyncCommands.get(eq("Account3::" + uuid))).thenReturn(MockRedisFuture.completedFuture( - String.format("{\"number\": \"+14152222222\", \"pni\": \"de24dc73-fbd8-41be-a7d5-764c70d9da7e\", \"usernameHash\": \"%s\"}", - BASE_64_URL_USERNAME_HASH_1))); - - Optional account = accountsManager.getByUsernameHash(USERNAME_HASH_1).join(); - - assertTrue(account.isPresent()); - assertEquals(account.get().getNumber(), "+14152222222"); - assertEquals(UUID.fromString("de24dc73-fbd8-41be-a7d5-764c70d9da7e"), account.get().getPhoneNumberIdentifier()); - assertArrayEquals(USERNAME_HASH_1, account.get().getUsernameHash().get()); - - verify(asyncCommands).get(eq("UAccountMap::" + BASE_64_URL_USERNAME_HASH_1)); - verify(asyncCommands).get(eq("Account3::" + uuid)); - verifyNoMoreInteractions(asyncCommands); - - verifyNoInteractions(accounts); - } - - @Test - void testGetAccountByNumberNotInCache() { - 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(commands.get(eq("AccountMap::+14152222222"))).thenReturn(null); - when(accounts.getByE164(eq("+14152222222"))).thenReturn(Optional.of(account)); - - Optional retrieved = accountsManager.getByE164("+14152222222"); - - assertTrue(retrieved.isPresent()); - assertSame(retrieved.get(), account); - - verify(commands, times(1)).get(eq("AccountMap::+14152222222")); - verify(commands, times(1)).setex(eq("AccountMap::+14152222222"), anyLong(), eq(uuid.toString())); - verify(commands, times(1)).setex(eq("AccountMap::" + pni), anyLong(), eq(uuid.toString())); - verify(commands, times(1)).setex(eq("Account3::" + uuid), anyLong(), anyString()); - verifyNoMoreInteractions(commands); - - verify(accounts, times(1)).getByE164(eq("+14152222222")); - verifyNoMoreInteractions(accounts); - } - - @Test - void testGetAccountByNumberNotInCacheAsync() { - 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(asyncCommands.get(eq("AccountMap::+14152222222"))).thenReturn(MockRedisFuture.completedFuture(null)); - when(asyncCommands.setex(any(), anyLong(), any())).thenReturn(MockRedisFuture.completedFuture("OK")); - when(accounts.getByE164Async(eq("+14152222222"))) - .thenReturn(MockRedisFuture.completedFuture(Optional.of(account))); - - Optional retrieved = accountsManager.getByE164Async("+14152222222").join(); - - assertTrue(retrieved.isPresent()); - assertSame(retrieved.get(), account); - - verify(asyncCommands).get(eq("AccountMap::+14152222222")); - verify(asyncCommands).setex(eq("AccountMap::+14152222222"), anyLong(), eq(uuid.toString())); - verify(asyncCommands).setex(eq("AccountMap::" + pni), anyLong(), eq(uuid.toString())); - verify(asyncCommands).setex(eq("Account3::" + uuid), anyLong(), anyString()); - verifyNoMoreInteractions(asyncCommands); - - verify(accounts).getByE164Async(eq("+14152222222")); - verifyNoMoreInteractions(accounts); - } - @Test void testGetAccountByUuidNotInCache() { UUID uuid = UUID.randomUUID(); @@ -505,7 +386,6 @@ class AccountsManagerTest { assertSame(retrieved.get(), account); verify(commands, times(1)).get(eq("Account3::" + uuid)); - verify(commands, times(1)).setex(eq("AccountMap::+14152222222"), anyLong(), eq(uuid.toString())); verify(commands, times(1)).setex(eq("AccountMap::" + pni), anyLong(), eq(uuid.toString())); verify(commands, times(1)).setex(eq("Account3::" + uuid), anyLong(), anyString()); verifyNoMoreInteractions(commands); @@ -531,7 +411,6 @@ class AccountsManagerTest { assertSame(retrieved.get(), account); verify(asyncCommands).get(eq("Account3::" + uuid)); - verify(asyncCommands).setex(eq("AccountMap::+14152222222"), anyLong(), eq(uuid.toString())); verify(asyncCommands).setex(eq("AccountMap::" + pni), anyLong(), eq(uuid.toString())); verify(asyncCommands).setex(eq("Account3::" + uuid), anyLong(), anyString()); verifyNoMoreInteractions(asyncCommands); @@ -557,7 +436,6 @@ class AccountsManagerTest { verify(commands).get(eq("AccountMap::" + pni)); verify(commands).setex(eq("AccountMap::" + pni), anyLong(), eq(uuid.toString())); - verify(commands).setex(eq("AccountMap::+14152222222"), anyLong(), eq(uuid.toString())); verify(commands).setex(eq("Account3::" + uuid), anyLong(), anyString()); verifyNoMoreInteractions(commands); @@ -584,7 +462,6 @@ class AccountsManagerTest { verify(asyncCommands).get(eq("AccountMap::" + pni)); verify(asyncCommands).setex(eq("AccountMap::" + pni), anyLong(), eq(uuid.toString())); - verify(asyncCommands).setex(eq("AccountMap::+14152222222"), anyLong(), eq(uuid.toString())); verify(asyncCommands).setex(eq("Account3::" + uuid), anyLong(), anyString()); verifyNoMoreInteractions(asyncCommands); @@ -593,86 +470,20 @@ class AccountsManagerTest { } @Test - void testGetAccountByUsernameHashNotInCache() { + void testGetAccountByUsernameHash() { UUID uuid = UUID.randomUUID(); - - Account account = AccountsHelper.generateTestAccount("+14152222222", uuid, UUID.randomUUID(), new ArrayList<>(), new byte[UnidentifiedAccessUtil.UNIDENTIFIED_ACCESS_KEY_LENGTH]); + Account account = AccountsHelper.generateTestAccount("+14152222222", uuid, UUID.randomUUID(), new ArrayList<>(), + new byte[UnidentifiedAccessUtil.UNIDENTIFIED_ACCESS_KEY_LENGTH]); account.setUsernameHash(USERNAME_HASH_1); - - when(asyncCommands.get(eq("UAccountMap::" + BASE_64_URL_USERNAME_HASH_1))) - .thenReturn(MockRedisFuture.completedFuture(null)); - when(accounts.getByUsernameHash(USERNAME_HASH_1)) .thenReturn(CompletableFuture.completedFuture(Optional.of(account))); - Optional retrieved = accountsManager.getByUsernameHash(USERNAME_HASH_1).join(); - assertTrue(retrieved.isPresent()); assertSame(retrieved.get(), account); - - verify(asyncCommands).get(eq("UAccountMap::" + BASE_64_URL_USERNAME_HASH_1)); - verify(asyncCommands).setex(eq("UAccountMap::" + BASE_64_URL_USERNAME_HASH_1), anyLong(), eq(uuid.toString())); - verify(asyncCommands).setex(eq("AccountMap::" + account.getPhoneNumberIdentifier()), anyLong(), eq(uuid.toString())); - verify(asyncCommands).setex(eq("AccountMap::+14152222222"), anyLong(), eq(uuid.toString())); - verify(asyncCommands).setex(eq("Account3::" + uuid), anyLong(), anyString()); - verifyNoMoreInteractions(asyncCommands); - verify(accounts).getByUsernameHash(USERNAME_HASH_1); verifyNoMoreInteractions(accounts); } - @Test - void testGetAccountByNumberBrokenCache() { - 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(commands.get(eq("AccountMap::+14152222222"))).thenThrow(new RedisException("Connection lost!")); - when(accounts.getByE164(eq("+14152222222"))).thenReturn(Optional.of(account)); - - Optional retrieved = accountsManager.getByE164("+14152222222"); - - assertTrue(retrieved.isPresent()); - assertSame(retrieved.get(), account); - - verify(commands, times(1)).get(eq("AccountMap::+14152222222")); - verify(commands, times(1)).setex(eq("AccountMap::+14152222222"), anyLong(), eq(uuid.toString())); - verify(commands, times(1)).setex(eq("AccountMap::" + pni), anyLong(), eq(uuid.toString())); - verify(commands, times(1)).setex(eq("Account3::" + uuid), anyLong(), anyString()); - verifyNoMoreInteractions(commands); - - verify(accounts, times(1)).getByE164(eq("+14152222222")); - verifyNoMoreInteractions(accounts); - } - - @Test - void testGetAccountByNumberBrokenCacheAsync() { - 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(asyncCommands.get(eq("AccountMap::+14152222222"))) - .thenReturn(MockRedisFuture.failedFuture(new RedisException("Connection lost!"))); - - when(asyncCommands.setex(any(), anyLong(), any())).thenReturn(MockRedisFuture.completedFuture("OK")); - - when(accounts.getByE164Async(eq("+14152222222"))).thenReturn(CompletableFuture.completedFuture(Optional.of(account))); - - Optional retrieved = accountsManager.getByE164Async("+14152222222").join(); - - assertTrue(retrieved.isPresent()); - assertSame(retrieved.get(), account); - - verify(asyncCommands).get(eq("AccountMap::+14152222222")); - verify(asyncCommands).setex(eq("AccountMap::+14152222222"), anyLong(), eq(uuid.toString())); - verify(asyncCommands).setex(eq("AccountMap::" + pni), anyLong(), eq(uuid.toString())); - verify(asyncCommands).setex(eq("Account3::" + uuid), anyLong(), anyString()); - verifyNoMoreInteractions(asyncCommands); - - verify(accounts).getByE164Async(eq("+14152222222")); - verifyNoMoreInteractions(accounts); - } - @Test void testGetAccountByUuidBrokenCache() { UUID uuid = UUID.randomUUID(); @@ -688,7 +499,6 @@ class AccountsManagerTest { assertSame(retrieved.get(), account); verify(commands, times(1)).get(eq("Account3::" + uuid)); - verify(commands, times(1)).setex(eq("AccountMap::+14152222222"), anyLong(), eq(uuid.toString())); verify(commands, times(1)).setex(eq("AccountMap::" + pni), anyLong(), eq(uuid.toString())); verify(commands, times(1)).setex(eq("Account3::" + uuid), anyLong(), anyString()); verifyNoMoreInteractions(commands); @@ -717,7 +527,6 @@ class AccountsManagerTest { assertSame(retrieved.get(), account); verify(asyncCommands).get(eq("Account3::" + uuid)); - verify(asyncCommands).setex(eq("AccountMap::+14152222222"), anyLong(), eq(uuid.toString())); verify(asyncCommands).setex(eq("AccountMap::" + pni), anyLong(), eq(uuid.toString())); verify(asyncCommands).setex(eq("Account3::" + uuid), anyLong(), anyString()); verifyNoMoreInteractions(asyncCommands); @@ -743,7 +552,6 @@ class AccountsManagerTest { verify(commands).get(eq("AccountMap::" + pni)); verify(commands).setex(eq("AccountMap::" + pni), anyLong(), eq(uuid.toString())); - verify(commands).setex(eq("AccountMap::+14152222222"), anyLong(), eq(uuid.toString())); verify(commands).setex(eq("Account3::" + uuid), anyLong(), anyString()); verifyNoMoreInteractions(commands); @@ -773,7 +581,6 @@ class AccountsManagerTest { verify(asyncCommands).get(eq("AccountMap::" + pni)); verify(asyncCommands).setex(eq("AccountMap::" + pni), anyLong(), eq(uuid.toString())); - verify(asyncCommands).setex(eq("AccountMap::+14152222222"), anyLong(), eq(uuid.toString())); verify(asyncCommands).setex(eq("Account3::" + uuid), anyLong(), anyString()); verifyNoMoreInteractions(asyncCommands); @@ -781,35 +588,6 @@ class AccountsManagerTest { verifyNoMoreInteractions(accounts); } - @Test - void testGetAccountByUsernameBrokenCache() { - UUID uuid = UUID.randomUUID(); - - Account account = AccountsHelper.generateTestAccount("+14152222222", uuid, UUID.randomUUID(), new ArrayList<>(), new byte[UnidentifiedAccessUtil.UNIDENTIFIED_ACCESS_KEY_LENGTH]); - account.setUsernameHash(USERNAME_HASH_1); - - when(asyncCommands.get(eq("UAccountMap::" + BASE_64_URL_USERNAME_HASH_1))) - .thenReturn(MockRedisFuture.failedFuture(new RedisException("OH NO"))); - - when(accounts.getByUsernameHash(USERNAME_HASH_1)) - .thenReturn(CompletableFuture.completedFuture(Optional.of(account))); - - Optional retrieved = accountsManager.getByUsernameHash(USERNAME_HASH_1).join(); - - assertTrue(retrieved.isPresent()); - assertSame(retrieved.get(), account); - - verify(asyncCommands).get(eq("UAccountMap::" + BASE_64_URL_USERNAME_HASH_1)); - verify(asyncCommands).setex(eq("UAccountMap::" + BASE_64_URL_USERNAME_HASH_1), anyLong(), eq(uuid.toString())); - verify(asyncCommands).setex(eq("AccountMap::" + account.getPhoneNumberIdentifier()), anyLong(), eq(uuid.toString())); - verify(asyncCommands).setex(eq("AccountMap::+14152222222"), anyLong(), eq(uuid.toString())); - verify(asyncCommands).setex(eq("Account3::" + uuid), anyLong(), anyString()); - verifyNoMoreInteractions(asyncCommands); - - verify(accounts).getByUsernameHash(USERNAME_HASH_1); - verifyNoMoreInteractions(accounts); - } - @Test void testUpdate_optimisticLockingFailure() { UUID uuid = UUID.randomUUID(); diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerUsernameIntegrationTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerUsernameIntegrationTest.java index 7bce806d4..363d329fa 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerUsernameIntegrationTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerUsernameIntegrationTest.java @@ -11,11 +11,8 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.doAnswer; -import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.eq; import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.times; -import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import java.time.Clock; @@ -31,7 +28,7 @@ import java.util.Map; import java.util.Optional; import java.util.UUID; import java.util.concurrent.CompletableFuture; -import java.util.concurrent.Executor; +import java.util.concurrent.Executors; import java.util.function.Supplier; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -135,21 +132,26 @@ class AccountsManagerUsernameIntegrationTest { when(experimentEnrollmentManager.isEnrolled(any(UUID.class), eq(AccountsManager.USERNAME_EXPERIMENT_NAME))) .thenReturn(true); + final MessagesManager messageManager = mock(MessagesManager.class); + final ProfilesManager profileManager = mock(ProfilesManager.class); + when(messageManager.clear(any())).thenReturn(CompletableFuture.completedFuture(null)); + when(profileManager.deleteAll(any())).thenReturn(CompletableFuture.completedFuture(null)); + accountsManager = new AccountsManager( accounts, phoneNumberIdentifiers, CACHE_CLUSTER_EXTENSION.getRedisCluster(), accountLockManager, keysManager, - mock(MessagesManager.class), - mock(ProfilesManager.class), + messageManager, + profileManager, mock(SecureStorageClient.class), mock(SecureValueRecovery2Client.class), mock(ClientPresenceManager.class), experimentEnrollmentManager, mock(RegistrationRecoveryPasswordsManager.class), - mock(Executor.class), - mock(Executor.class), + Executors.newSingleThreadExecutor(), + Executors.newSingleThreadExecutor(), mock(Clock.class)); } @@ -299,6 +301,28 @@ class AccountsManagerUsernameIntegrationTest { assertArrayEquals(account.getEncryptedUsername().orElseThrow(), ENCRYPTED_USERNAME_2); } + @Test + public void testReclaim() throws InterruptedException { + Account account = AccountsHelper.createAccount(accountsManager, "+18005551111"); + final AccountsManager.UsernameReservation reservation1 = + accountsManager.reserveUsernameHash(account, List.of(USERNAME_HASH_1)).join(); + account = accountsManager.confirmReservedUsernameHash(reservation1.account(), USERNAME_HASH_1, ENCRYPTED_USERNAME_1) + .join(); + + // "reclaim" the account by re-registering + Account reclaimed = AccountsHelper.createAccount(accountsManager, "+18005551111"); + + // the username should still be reserved, but no longer on our account. + assertThat(reclaimed.getUsernameHash()).isEmpty(); + + // Make sure we can't lookup the account + assertThat(accountsManager.getByUsernameHash(USERNAME_HASH_1).join()).isEmpty(); + + // confirm it again + accountsManager.confirmReservedUsernameHash(reclaimed, USERNAME_HASH_1, ENCRYPTED_USERNAME_1).join(); + assertThat(accountsManager.getByUsernameHash(USERNAME_HASH_1).join()).isPresent(); + } + @Test public void testUsernameLinks() throws InterruptedException, AccountAlreadyExistsException { final Account account = AccountsHelper.createAccount(accountsManager, "+18005551111");