From 6304c84cdb43385a4d4e9b0a4629713927fd5bd6 Mon Sep 17 00:00:00 2001 From: Graeme Connell Date: Mon, 13 Sep 2021 15:08:06 -0600 Subject: [PATCH] Add ContactDiscoveryWriterTest based on mock. --- .../storage/ContactDiscoveryWriter.java | 15 +++++- .../storage/AccountsDynamoDbTest.java | 34 -------------- .../storage/ContactDiscoveryWriterTest.java | 46 +++++++++++++++++++ 3 files changed, 60 insertions(+), 35 deletions(-) create mode 100644 service/src/test/java/org/whispersystems/textsecuregcm/storage/ContactDiscoveryWriterTest.java diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/storage/ContactDiscoveryWriter.java b/service/src/main/java/org/whispersystems/textsecuregcm/storage/ContactDiscoveryWriter.java index f8b954d2a..b79350fa1 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/storage/ContactDiscoveryWriter.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/storage/ContactDiscoveryWriter.java @@ -1,8 +1,15 @@ +/* + * Copyright 2013-2021 Signal Messenger, LLC + * SPDX-License-Identifier: AGPL-3.0-only + */ + package org.whispersystems.textsecuregcm.storage; +import com.google.common.annotations.VisibleForTesting; import java.util.List; import java.util.Optional; import java.util.UUID; +import java.util.function.Consumer; public class ContactDiscoveryWriter extends AccountDatabaseCrawlerListener { @@ -22,6 +29,12 @@ public class ContactDiscoveryWriter extends AccountDatabaseCrawlerListener { // nothing } + // We "update" by doing nothing, since everything about the account is already accurate except for a temporal + // change in the 'shouldBeVisible' trait. This update forces a new write of the underlying DB to reflect + // that temporal change persistently. + @VisibleForTesting + static final Consumer NOOP_UPDATER = a -> {}; + @Override protected void onCrawlChunk(final Optional fromUuid, final List chunkAccounts) throws AccountDatabaseCrawlerRestartException { @@ -30,7 +43,7 @@ public class ContactDiscoveryWriter extends AccountDatabaseCrawlerListener { // It’s less than ideal, but crawler listeners currently must not call update() // with the accounts from the chunk, because updates cause the account instance to become stale. Instead, they // must get a new copy, which they are free to update. - accounts.get(account.getUuid()).ifPresent(a -> accounts.update(a, updated -> {})); + accounts.get(account.getUuid()).ifPresent(a -> accounts.update(a, NOOP_UPDATER)); } } } diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsDynamoDbTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsDynamoDbTest.java index 907143d89..7d5367b93 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsDynamoDbTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsDynamoDbTest.java @@ -524,40 +524,6 @@ class AccountsDynamoDbTest { verifyStoredState("+14151112222", account.getUuid(), account, false); } - /* - @Test - void testContactDiscoveryWriter() throws Exception { - Device device = generateDevice(1); - UUID uuid = UUID.randomUUID(); - Account account = generateAccount("+14151112222", uuid, Collections.singleton(device)); - accountsDynamoDb.create(account); - verifyStoredState("+14151112222", account.getUuid(), account, true); - ContactDiscoveryWriter writer = new ContactDiscoveryWriter(accountsDynamoDb); - account.setCanonicallyDiscoverable(false); - writer.onCrawlChunk(null, List.of(account)); - account.setVersion(1); - verifyStoredState("+14151112222", account.getUuid(), account, true); - - // Make the stored "C" column not match reality. - final UpdateItemRequest updateItemRequest = UpdateItemRequest.builder() - .tableName(ACCOUNTS_TABLE_NAME) - .key(Map.of(AccountsDynamoDb.KEY_ACCOUNT_UUID, AttributeValues.fromUUID(account.getUuid()))) - .updateExpression("SET #cds = :cds") - .expressionAttributeNames(Map.of("#cds", AccountsDynamoDb.ATTR_CANONICALLY_DISCOVERABLE)) - .expressionAttributeValues(Map.of( - ":cds", AttributeValues.fromBool(false))) - .build(); - dynamoDbExtension.getDynamoDbClient().updateItem(updateItemRequest); - verifyStoredState("+14151112222", account.getUuid(), account, false); - - // Crawl again and make sure update happened - account.setCanonicallyDiscoverable(false); - writer.onCrawlChunk(null, List.of(account)); - account.setVersion(2); - verifyStoredState("+14151112222", account.getUuid(), account, true); - } - */ - private Device generateDevice(long id) { Random random = new Random(System.currentTimeMillis()); SignedPreKey signedPreKey = new SignedPreKey(random.nextInt(), "testPublicKey-" + random.nextInt(), "testSignature-" + random.nextInt()); diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/storage/ContactDiscoveryWriterTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/storage/ContactDiscoveryWriterTest.java new file mode 100644 index 000000000..a726e3b86 --- /dev/null +++ b/service/src/test/java/org/whispersystems/textsecuregcm/storage/ContactDiscoveryWriterTest.java @@ -0,0 +1,46 @@ +/* + * Copyright 2013-2021 Signal Messenger, LLC + * SPDX-License-Identifier: AGPL-3.0-only + */ + +package org.whispersystems.textsecuregcm.storage; + +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; + +import java.util.List; +import java.util.Optional; +import java.util.UUID; +import java.util.stream.Stream; + +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +public class ContactDiscoveryWriterTest { + + @ParameterizedTest + @MethodSource + void testUpdatesOnChange(boolean canonicallyDiscoverable, boolean shouldBeVisible, boolean updateCalled) throws AccountDatabaseCrawlerRestartException { + AccountsManager mgr = mock(AccountsManager.class); + UUID uuid = UUID.randomUUID(); + Account acct = mock(Account.class); + when(acct.getUuid()).thenReturn(uuid); + when(acct.isCanonicallyDiscoverable()).thenReturn(canonicallyDiscoverable); + when(acct.shouldBeVisibleInDirectory()).thenReturn(shouldBeVisible); + when(mgr.get(uuid)).thenReturn(Optional.of(acct)); + ContactDiscoveryWriter writer = new ContactDiscoveryWriter(mgr); + writer.onCrawlChunk(Optional.empty(), List.of(acct)); + verify(mgr, times(updateCalled ? 1 : 0)).update(acct, ContactDiscoveryWriter.NOOP_UPDATER); + } + + static Stream testUpdatesOnChange() { + return Stream.of( + Arguments.of(true, true, false), + Arguments.of(false, false, false), + Arguments.of(true, false, true), + Arguments.of(false, true, true)); + } +}