revert to phone number-based account crawler

This commit is contained in:
Jeffrey Griffin
2019-08-26 14:00:15 -07:00
parent 284428a45a
commit cf78047830
16 changed files with 118 additions and 160 deletions

View File

@@ -29,7 +29,6 @@ import org.whispersystems.textsecuregcm.storage.AccountsManager;
import java.util.Arrays;
import java.util.Collections;
import java.util.Optional;
import java.util.UUID;
import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.ArgumentMatchers.any;
@@ -40,8 +39,8 @@ import static org.mockito.Mockito.*;
public class AccountDatabaseCrawlerTest {
private static final UUID ACCOUNT1 = UUID.randomUUID();
private static final UUID ACCOUNT2 = UUID.randomUUID();
private static final String ACCOUNT1 = "+1";
private static final String ACCOUNT2 = "+2";
private static final int CHUNK_SIZE = 1000;
private static final long CHUNK_INTERVAL_MS = 30_000L;
@@ -57,8 +56,8 @@ public class AccountDatabaseCrawlerTest {
@Before
public void setup() {
when(account1.getUuid()).thenReturn(ACCOUNT1);
when(account2.getUuid()).thenReturn(ACCOUNT2);
when(account1.getNumber()).thenReturn(ACCOUNT1);
when(account2.getNumber()).thenReturn(ACCOUNT2);
when(accounts.getAllFrom(anyInt())).thenReturn(Arrays.asList(account1, account2));
when(accounts.getAllFrom(eq(ACCOUNT1), anyInt())).thenReturn(Arrays.asList(account2));
@@ -70,20 +69,20 @@ public class AccountDatabaseCrawlerTest {
@Test
public void testCrawlStart() throws AccountDatabaseCrawlerRestartException {
when(cache.getLastUuid()).thenReturn(Optional.empty());
when(cache.getLastNumber()).thenReturn(Optional.empty());
boolean accelerated = crawler.doPeriodicWork();
assertThat(accelerated).isFalse();
verify(cache, times(1)).claimActiveWork(any(String.class), anyLong());
verify(cache, times(1)).getLastUuid();
verify(cache, times(1)).getLastNumber();
verify(listener, times(1)).onCrawlStart();
verify(accounts, times(1)).getAllFrom(eq(CHUNK_SIZE));
verify(accounts, times(0)).getAllFrom(any(UUID.class), eq(CHUNK_SIZE));
verify(account1, times(0)).getUuid();
verify(account2, times(1)).getUuid();
verify(accounts, times(0)).getAllFrom(any(String.class), eq(CHUNK_SIZE));
verify(account1, times(0)).getNumber();
verify(account2, times(1)).getNumber();
verify(listener, times(1)).onCrawlChunk(eq(Optional.empty()), eq(Arrays.asList(account1, account2)));
verify(cache, times(1)).setLastUuid(eq(Optional.of(ACCOUNT2)));
verify(cache, times(1)).setLastNumber(eq(Optional.of(ACCOUNT2)));
verify(cache, times(1)).isAccelerated();
verify(cache, times(1)).releaseActiveWork(any(String.class));
@@ -96,18 +95,18 @@ public class AccountDatabaseCrawlerTest {
@Test
public void testCrawlChunk() throws AccountDatabaseCrawlerRestartException {
when(cache.getLastUuid()).thenReturn(Optional.of(ACCOUNT1));
when(cache.getLastNumber()).thenReturn(Optional.of(ACCOUNT1));
boolean accelerated = crawler.doPeriodicWork();
assertThat(accelerated).isFalse();
verify(cache, times(1)).claimActiveWork(any(String.class), anyLong());
verify(cache, times(1)).getLastUuid();
verify(cache, times(1)).getLastNumber();
verify(accounts, times(0)).getAllFrom(eq(CHUNK_SIZE));
verify(accounts, times(1)).getAllFrom(eq(ACCOUNT1), eq(CHUNK_SIZE));
verify(account2, times(1)).getUuid();
verify(account2, times(1)).getNumber();
verify(listener, times(1)).onCrawlChunk(eq(Optional.of(ACCOUNT1)), eq(Arrays.asList(account2)));
verify(cache, times(1)).setLastUuid(eq(Optional.of(ACCOUNT2)));
verify(cache, times(1)).setLastNumber(eq(Optional.of(ACCOUNT2)));
verify(cache, times(1)).isAccelerated();
verify(cache, times(1)).releaseActiveWork(any(String.class));
@@ -122,18 +121,18 @@ public class AccountDatabaseCrawlerTest {
@Test
public void testCrawlChunkAccelerated() throws AccountDatabaseCrawlerRestartException {
when(cache.isAccelerated()).thenReturn(true);
when(cache.getLastUuid()).thenReturn(Optional.of(ACCOUNT1));
when(cache.getLastNumber()).thenReturn(Optional.of(ACCOUNT1));
boolean accelerated = crawler.doPeriodicWork();
assertThat(accelerated).isTrue();
verify(cache, times(1)).claimActiveWork(any(String.class), anyLong());
verify(cache, times(1)).getLastUuid();
verify(cache, times(1)).getLastNumber();
verify(accounts, times(0)).getAllFrom(eq(CHUNK_SIZE));
verify(accounts, times(1)).getAllFrom(eq(ACCOUNT1), eq(CHUNK_SIZE));
verify(account2, times(1)).getUuid();
verify(account2, times(1)).getNumber();
verify(listener, times(1)).onCrawlChunk(eq(Optional.of(ACCOUNT1)), eq(Arrays.asList(account2)));
verify(cache, times(1)).setLastUuid(eq(Optional.of(ACCOUNT2)));
verify(cache, times(1)).setLastNumber(eq(Optional.of(ACCOUNT2)));
verify(cache, times(1)).isAccelerated();
verify(cache, times(1)).releaseActiveWork(any(String.class));
@@ -147,19 +146,19 @@ public class AccountDatabaseCrawlerTest {
@Test
public void testCrawlChunkRestart() throws AccountDatabaseCrawlerRestartException {
when(cache.getLastUuid()).thenReturn(Optional.of(ACCOUNT1));
when(cache.getLastNumber()).thenReturn(Optional.of(ACCOUNT1));
doThrow(AccountDatabaseCrawlerRestartException.class).when(listener).onCrawlChunk(eq(Optional.of(ACCOUNT1)), eq(Arrays.asList(account2)));
boolean accelerated = crawler.doPeriodicWork();
assertThat(accelerated).isFalse();
verify(cache, times(1)).claimActiveWork(any(String.class), anyLong());
verify(cache, times(1)).getLastUuid();
verify(cache, times(1)).getLastNumber();
verify(accounts, times(0)).getAllFrom(eq(CHUNK_SIZE));
verify(accounts, times(1)).getAllFrom(eq(ACCOUNT1), eq(CHUNK_SIZE));
verify(account2, times(0)).getNumber();
verify(listener, times(1)).onCrawlChunk(eq(Optional.of(ACCOUNT1)), eq(Arrays.asList(account2)));
verify(cache, times(1)).setLastUuid(eq(Optional.empty()));
verify(cache, times(1)).setLastNumber(eq(Optional.empty()));
verify(cache, times(1)).clearAccelerate();
verify(cache, times(1)).isAccelerated();
verify(cache, times(1)).releaseActiveWork(any(String.class));
@@ -174,19 +173,19 @@ public class AccountDatabaseCrawlerTest {
@Test
public void testCrawlEnd() {
when(cache.getLastUuid()).thenReturn(Optional.of(ACCOUNT2));
when(cache.getLastNumber()).thenReturn(Optional.of(ACCOUNT2));
boolean accelerated = crawler.doPeriodicWork();
assertThat(accelerated).isFalse();
verify(cache, times(1)).claimActiveWork(any(String.class), anyLong());
verify(cache, times(1)).getLastUuid();
verify(cache, times(1)).getLastNumber();
verify(accounts, times(0)).getAllFrom(eq(CHUNK_SIZE));
verify(accounts, times(1)).getAllFrom(eq(ACCOUNT2), eq(CHUNK_SIZE));
verify(account1, times(0)).getNumber();
verify(account2, times(0)).getNumber();
verify(listener, times(1)).onCrawlEnd(eq(Optional.of(ACCOUNT2)));
verify(cache, times(1)).setLastUuid(eq(Optional.empty()));
verify(cache, times(1)).setLastNumber(eq(Optional.empty()));
verify(cache, times(1)).clearAccelerate();
verify(cache, times(1)).isAccelerated();
verify(cache, times(1)).releaseActiveWork(any(String.class));

View File

@@ -173,8 +173,6 @@ public class AccountsTest {
accounts.create(account);
}
users.sort((account, t1) -> UUIDComparator.staticCompare(account.getUuid(), t1.getUuid()));
List<Account> retrieved = accounts.getAllFrom(10);
assertThat(retrieved.size()).isEqualTo(10);
@@ -183,7 +181,7 @@ public class AccountsTest {
}
for (int j=0;j<9;j++) {
retrieved = accounts.getAllFrom(retrieved.get(9).getUuid(), 10);
retrieved = accounts.getAllFrom(retrieved.get(9).getNumber(), 10);
assertThat(retrieved.size()).isEqualTo(10);
for (int i=0;i<retrieved.size();i++) {

View File

@@ -31,7 +31,6 @@ import org.junit.Test;
import redis.clients.jedis.Jedis;
import java.util.Arrays;
import java.util.UUID;
import java.util.concurrent.TimeUnit;
import java.util.Optional;
@@ -47,13 +46,9 @@ import static org.mockito.Mockito.when;
public class ActiveUserCounterTest {
private final UUID UUID_IOS = UUID.randomUUID();
private final UUID UUID_ANDROID = UUID.randomUUID();
private final UUID UUID_NODEVICE = UUID.randomUUID();
private final String ACCOUNT_NUMBER_IOS = "+15551234567";
private final String ACCOUNT_NUMBER_ANDROID = "+5511987654321";
private final String ACCOUNT_NUMBER_NODEVICE = "+5215551234567";
private final String NUMBER_IOS = "+15551234567";
private final String NUMBER_ANDROID = "+5511987654321";
private final String NUMBER_NODEVICE = "+5215551234567";
private final String TALLY_KEY = "active_user_tally";
@@ -84,17 +79,14 @@ public class ActiveUserCounterTest {
when(iosDevice.getGcmId()).thenReturn(null);
when(iosDevice.getLastSeen()).thenReturn(halfDayAgo);
when(iosAccount.getUuid()).thenReturn(UUID_IOS);
when(iosAccount.getNumber()).thenReturn(NUMBER_IOS);
when(iosAccount.getMasterDevice()).thenReturn(Optional.of(iosDevice));
when(iosAccount.getNumber()).thenReturn(ACCOUNT_NUMBER_IOS);
when(androidAccount.getUuid()).thenReturn(UUID_ANDROID);
when(androidAccount.getNumber()).thenReturn(NUMBER_ANDROID);
when(androidAccount.getMasterDevice()).thenReturn(Optional.of(androidDevice));
when(androidAccount.getNumber()).thenReturn(ACCOUNT_NUMBER_ANDROID);
when(noDeviceAccount.getUuid()).thenReturn(UUID_NODEVICE);
when(noDeviceAccount.getNumber()).thenReturn(NUMBER_NODEVICE);
when(noDeviceAccount.getMasterDevice()).thenReturn(Optional.ofNullable(null));
when(noDeviceAccount.getNumber()).thenReturn(ACCOUNT_NUMBER_NODEVICE);
when(jedis.get(any(String.class))).thenReturn("{\"fromNumber\":\"+\",\"platforms\":{},\"countries\":{}}");
when(jedisPool.getWriteResource()).thenReturn(jedis);
@@ -145,7 +137,7 @@ public class ActiveUserCounterTest {
@Test
public void testCrawlChunkValidAccount() throws AccountDatabaseCrawlerRestartException {
activeUserCounter.onCrawlChunk(Optional.of(UUID_IOS), Arrays.asList(iosAccount));
activeUserCounter.onCrawlChunk(Optional.of(NUMBER_IOS), Arrays.asList(iosAccount));
verify(iosAccount, times(1)).getMasterDevice();
verify(iosAccount, times(1)).getNumber();
@@ -156,7 +148,7 @@ public class ActiveUserCounterTest {
verify(jedisPool, times(1)).getWriteResource();
verify(jedis, times(1)).get(any(String.class));
verify(jedis, times(1)).set(any(String.class), eq("{\"fromUuid\":\""+UUID_IOS.toString()+"\",\"platforms\":{\"ios\":[1,1,1,1,1]},\"countries\":{\"1\":[1,1,1,1,1]}}"));
verify(jedis, times(1)).set(any(String.class), eq("{\"fromNumber\":\""+NUMBER_IOS+"\",\"platforms\":{\"ios\":[1,1,1,1,1]},\"countries\":{\"1\":[1,1,1,1,1]}}"));
verify(jedis, times(1)).close();
verify(metricsFactory, times(0)).getReporters();
@@ -174,13 +166,13 @@ public class ActiveUserCounterTest {
@Test
public void testCrawlChunkNoDeviceAccount() throws AccountDatabaseCrawlerRestartException {
activeUserCounter.onCrawlChunk(Optional.of(UUID_NODEVICE), Arrays.asList(noDeviceAccount));
activeUserCounter.onCrawlChunk(Optional.of(NUMBER_NODEVICE), Arrays.asList(noDeviceAccount));
verify(noDeviceAccount, times(1)).getMasterDevice();
verify(jedisPool, times(1)).getWriteResource();
verify(jedis, times(1)).get(eq(TALLY_KEY));
verify(jedis, times(1)).set(any(String.class), eq("{\"fromUuid\":\""+UUID_NODEVICE+"\",\"platforms\":{},\"countries\":{}}"));
verify(jedis, times(1)).set(any(String.class), eq("{\"fromNumber\":\""+NUMBER_NODEVICE+"\",\"platforms\":{},\"countries\":{}}"));
verify(jedis, times(1)).close();
verify(metricsFactory, times(0)).getReporters();
@@ -198,7 +190,7 @@ public class ActiveUserCounterTest {
@Test
public void testCrawlChunkMixedAccount() throws AccountDatabaseCrawlerRestartException {
activeUserCounter.onCrawlChunk(Optional.of(UUID_IOS), Arrays.asList(iosAccount, androidAccount, noDeviceAccount));
activeUserCounter.onCrawlChunk(Optional.of(NUMBER_IOS), Arrays.asList(iosAccount, androidAccount, noDeviceAccount));
verify(iosAccount, times(1)).getMasterDevice();
verify(iosAccount, times(1)).getNumber();
@@ -216,7 +208,7 @@ public class ActiveUserCounterTest {
verify(jedisPool, times(1)).getWriteResource();
verify(jedis, times(1)).get(eq(TALLY_KEY));
verify(jedis, times(1)).set(any(String.class), eq("{\"fromUuid\":\""+UUID_IOS+"\",\"platforms\":{\"android\":[0,0,0,1,1],\"ios\":[1,1,1,1,1]},\"countries\":{\"55\":[0,0,0,1,1],\"1\":[1,1,1,1,1]}}"));
verify(jedis, times(1)).set(any(String.class), eq("{\"fromNumber\":\""+NUMBER_IOS+"\",\"platforms\":{\"android\":[0,0,0,1,1],\"ios\":[1,1,1,1,1]},\"countries\":{\"55\":[0,0,0,1,1],\"1\":[1,1,1,1,1]}}"));
verify(jedis, times(1)).close();
verify(metricsFactory, times(0)).getReporters();

View File

@@ -33,7 +33,6 @@ import org.whispersystems.textsecuregcm.util.Util;
import java.util.Arrays;
import java.util.Optional;
import java.util.UUID;
import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.ArgumentMatchers.any;
@@ -41,10 +40,8 @@ import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.*;
public class DirectoryReconcilerTest {
private static final UUID VALID_UUID = UUID.randomUUID();
private static final String VALID_NUMBERRR = "+14152222222";
private static final UUID INACTIVE_UUID = UUID.randomUUID();
private static final String INACTIVE_NUMBERRR = "+14151111111";
private static final String VALID_NUMBER = "valid";
private static final String INACTIVE_NUMBER = "inactive";
private final Account activeAccount = mock(Account.class);
private final Account inactiveAccount = mock(Account.class);
@@ -58,11 +55,9 @@ public class DirectoryReconcilerTest {
@Before
public void setup() {
when(activeAccount.getUuid()).thenReturn(VALID_UUID);
when(activeAccount.getNumber()).thenReturn(VALID_NUMBER);
when(activeAccount.isEnabled()).thenReturn(true);
when(activeAccount.getNumber()).thenReturn(VALID_NUMBERRR);
when(inactiveAccount.getUuid()).thenReturn(INACTIVE_UUID);
when(inactiveAccount.getNumber()).thenReturn(INACTIVE_NUMBERRR);
when(inactiveAccount.getNumber()).thenReturn(INACTIVE_NUMBER);
when(inactiveAccount.isEnabled()).thenReturn(false);
when(directoryManager.startBatchOperation()).thenReturn(batchOperationHandle);
}
@@ -70,29 +65,27 @@ public class DirectoryReconcilerTest {
@Test
public void testCrawlChunkValid() throws AccountDatabaseCrawlerRestartException {
when(reconciliationClient.sendChunk(any())).thenReturn(successResponse);
directoryReconciler.onCrawlChunk(Optional.of(VALID_UUID), Arrays.asList(activeAccount, inactiveAccount));
directoryReconciler.onCrawlChunk(Optional.of(VALID_NUMBER), Arrays.asList(activeAccount, inactiveAccount));
verify(activeAccount, atLeastOnce()).getUuid();
verify(activeAccount, atLeastOnce()).getNumber();
verify(activeAccount, atLeastOnce()).isEnabled();
verify(inactiveAccount, atLeastOnce()).getUuid();
verify(inactiveAccount, atLeastOnce()).getNumber();
verify(inactiveAccount, atLeastOnce()).isEnabled();
ArgumentCaptor<DirectoryReconciliationRequest> request = ArgumentCaptor.forClass(DirectoryReconciliationRequest.class);
verify(reconciliationClient, times(1)).sendChunk(request.capture());
assertThat(request.getValue().getFromUuid()).isEqualTo(VALID_UUID);
assertThat(request.getValue().getToUuid()).isEqualTo(INACTIVE_UUID);
assertThat(request.getValue().getNumbers()).isEqualTo(Arrays.asList(VALID_NUMBERRR));
assertThat(request.getValue().getFromNumber()).isEqualTo(VALID_NUMBER);
assertThat(request.getValue().getToNumber()).isEqualTo(INACTIVE_NUMBER);
assertThat(request.getValue().getNumbers()).isEqualTo(Arrays.asList(VALID_NUMBER));
ArgumentCaptor<ClientContact> addedContact = ArgumentCaptor.forClass(ClientContact.class);
verify(directoryManager, times(1)).startBatchOperation();
verify(directoryManager, times(1)).add(eq(batchOperationHandle), addedContact.capture());
verify(directoryManager, times(1)).remove(eq(batchOperationHandle), eq(INACTIVE_NUMBERRR));
verify(directoryManager, times(1)).remove(eq(batchOperationHandle), eq(INACTIVE_NUMBER));
verify(directoryManager, times(1)).stopBatchOperation(eq(batchOperationHandle));
assertThat(addedContact.getValue().getToken()).isEqualTo(Util.getContactToken(VALID_NUMBERRR));
assertThat(addedContact.getValue().getToken()).isEqualTo(Util.getContactToken(VALID_NUMBER));
verifyNoMoreInteractions(activeAccount);
verifyNoMoreInteractions(inactiveAccount);

View File

@@ -13,7 +13,6 @@ import java.util.Collections;
import java.util.List;
import java.util.Optional;
import java.util.Set;
import java.util.UUID;
import java.util.concurrent.TimeUnit;
import static org.mockito.Mockito.*;
@@ -63,7 +62,7 @@ public class PushFeedbackProcessorTest {
@Test
public void testEmpty() {
PushFeedbackProcessor processor = new PushFeedbackProcessor(accountsManager, directoryQueue);
processor.onCrawlChunk(Optional.of(UUID.randomUUID()), Collections.emptyList());
processor.onCrawlChunk(Optional.of("+14152222222"), Collections.emptyList());
verifyZeroInteractions(accountsManager);
verifyZeroInteractions(directoryQueue);
@@ -72,7 +71,7 @@ public class PushFeedbackProcessorTest {
@Test
public void testUpdate() {
PushFeedbackProcessor processor = new PushFeedbackProcessor(accountsManager, directoryQueue);
processor.onCrawlChunk(Optional.of(UUID.randomUUID()), List.of(uninstalledAccount, mixedAccount, stillActiveAccount, freshAccount, cleanAccount));
processor.onCrawlChunk(Optional.of("+14153333333"), List.of(uninstalledAccount, mixedAccount, stillActiveAccount, freshAccount, cleanAccount));
verify(uninstalledDevice).setApnId(isNull());
verify(uninstalledDevice).setGcmId(isNull());