Signal/Signal-Server
1
0
Fork 0
mirror of https://github.com/signalapp/Signal-Server synced 2026-04-22 01:08:05 +01:00
Code Issues Packages Projects Releases Wiki Activity

Remove some secondary keys from account cache

Browse Source 
Remove e164, usernameHash, and usernameLink secondary mappings from the
accounts redis cache.
This commit is contained in:
Ravi Khadiwala
2024-01-10 11:05:57 -06:00
committed by ravi-signal
parent bf05e47e26
commit 323bfd9a6e
3 changed files with 53 additions and 285 deletions
Download Patch File Download Diff File Expand all files Collapse all files

228
service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerTest.java
View File

@@ -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> 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> 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> 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<Account> 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<Account> 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<Account> 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<Account> 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<Account> 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<Account> 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();

40
service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsManagerUsernameIntegrationTest.java
View File

@@ -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");