From 2059bb5ef83cb907d1892a7b68a67feb0846ee35 Mon Sep 17 00:00:00 2001
From: Graeme Connell <gram@signal.org>
Date: Tue, 7 Sep 2021 11:16:43 -0600
Subject: [PATCH] Update test to handle read-then-write in
 ContactDiscoveryWriter.

---
 .../storage/ContactDiscoveryWriter.java       |  3 +++
 .../storage/AccountsDynamoDbTest.java         | 23 ++++++++++++++++---
 2 files changed, 23 insertions(+), 3 deletions(-)

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 18f230c14..e50f41855 100644
--- a/service/src/main/java/org/whispersystems/textsecuregcm/storage/ContactDiscoveryWriter.java
+++ b/service/src/main/java/org/whispersystems/textsecuregcm/storage/ContactDiscoveryWriter.java
@@ -27,6 +27,9 @@ public class ContactDiscoveryWriter extends AccountDatabaseCrawlerListener {
       throws AccountDatabaseCrawlerRestartException {
     for (Account account : chunkAccounts) {
       if (account.isCanonicallyDiscoverable() != account.shouldBeVisibleInDirectory()) {
+        // 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(accounts::update);
       }
     }
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 bb118e479..f82769e66 100644
--- a/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsDynamoDbTest.java
+++ b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountsDynamoDbTest.java
@@ -36,6 +36,7 @@ import org.junit.jupiter.api.extension.RegisterExtension;
 import org.whispersystems.textsecuregcm.configuration.CircuitBreakerConfiguration;
 import org.whispersystems.textsecuregcm.entities.SignedPreKey;
 import org.whispersystems.textsecuregcm.util.AttributeValues;
+import org.whispersystems.textsecuregcm.util.SystemMapper;
 import software.amazon.awssdk.services.dynamodb.DynamoDbAsyncClient;
 import software.amazon.awssdk.services.dynamodb.DynamoDbClient;
 import software.amazon.awssdk.services.dynamodb.model.AttributeDefinition;
@@ -46,6 +47,7 @@ import software.amazon.awssdk.services.dynamodb.model.GetItemRequest;
 import software.amazon.awssdk.services.dynamodb.model.GetItemResponse;
 import software.amazon.awssdk.services.dynamodb.model.KeySchemaElement;
 import software.amazon.awssdk.services.dynamodb.model.KeyType;
+import software.amazon.awssdk.services.dynamodb.model.ReturnValue;
 import software.amazon.awssdk.services.dynamodb.model.ScalarAttributeType;
 import software.amazon.awssdk.services.dynamodb.model.ScanRequest;
 import software.amazon.awssdk.services.dynamodb.model.ScanResponse;
@@ -532,11 +534,26 @@ class AccountsDynamoDbTest {
     ContactDiscoveryWriter writer = new ContactDiscoveryWriter(accountsDynamoDb);
     account.setCanonicallyDiscoverable(false);
     writer.onCrawlChunk(null, List.of(account));
+    account.setVersion(1);
     verifyStoredState("+14151112222", account.getUuid(), account, true);
-    account.setCanonicallyDiscoverable(true);
-    account.setDiscoverableByPhoneNumber(false);
-    writer.onCrawlChunk(null, List.of(account));
+
+    // 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) {