From 2881c0fd7e2e87ff2c2c97e7aa0e14d119625d81 Mon Sep 17 00:00:00 2001
From: Jon Chambers <jon@signal.org>
Date: Thu, 10 Nov 2022 12:42:38 -0500
Subject: [PATCH] Allow the account cleaner to act on all accounts in a crawled
 chunk

---
 .../textsecuregcm/storage/AccountCleaner.java | 19 ++++----------
 .../storage/AccountCleanerTest.java           | 26 +------------------
 2 files changed, 6 insertions(+), 39 deletions(-)

diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/storage/AccountCleaner.java b/service/src/main/java/org/whispersystems/textsecuregcm/storage/AccountCleaner.java
index b5fd726c..b532b320 100644
--- a/service/src/main/java/org/whispersystems/textsecuregcm/storage/AccountCleaner.java
+++ b/service/src/main/java/org/whispersystems/textsecuregcm/storage/AccountCleaner.java
@@ -25,9 +25,6 @@ public class AccountCleaner extends AccountDatabaseCrawlerListener {
   private static final String DELETED_ACCOUNT_COUNTER_NAME = name(AccountCleaner.class, "deletedAccounts");
   private static final String DELETION_REASON_TAG_NAME = "reason";
 
-  @VisibleForTesting
-  static final int MAX_ACCOUNT_DELETIONS_PER_CHUNK = 256;
-
   private final AccountsManager accountsManager;
 
   public AccountCleaner(AccountsManager accountsManager) {
@@ -44,8 +41,6 @@ public class AccountCleaner extends AccountDatabaseCrawlerListener {
 
   @Override
   protected void onCrawlChunk(Optional<UUID> fromUuid, List<Account> chunkAccounts) {
-    int accountUpdateCount = 0;
-
     for (Account account : chunkAccounts) {
       if (isExpired(account) || needsExplicitRemoval(account)) {
         final Tag deletionReason;
@@ -56,15 +51,11 @@ public class AccountCleaner extends AccountDatabaseCrawlerListener {
           deletionReason = Tag.of(DELETION_REASON_TAG_NAME, "previouslyExpired");
         }
 
-        if (accountUpdateCount < MAX_ACCOUNT_DELETIONS_PER_CHUNK) {
-          try {
-            accountsManager.delete(account, AccountsManager.DeletionReason.EXPIRED);
-            accountUpdateCount++;
-
-            Metrics.counter(DELETED_ACCOUNT_COUNTER_NAME, Tags.of(deletionReason)).increment();
-          } catch (final Exception e) {
-            log.warn("Failed to delete account {}", account.getUuid(), e);
-          }
+        try {
+          accountsManager.delete(account, AccountsManager.DeletionReason.EXPIRED);
+          Metrics.counter(DELETED_ACCOUNT_COUNTER_NAME, Tags.of(deletionReason)).increment();
+        } catch (final Exception e) {
+          log.warn("Failed to delete account {}", account.getUuid(), e);
         }
       }
     }
diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountCleanerTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountCleanerTest.java
index 7c439647..ef64e4a8 100644
--- a/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountCleanerTest.java
+++ b/service/src/test/java/org/whispersystems/textsecuregcm/storage/AccountCleanerTest.java
@@ -8,19 +8,16 @@ import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.never;
-import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.verifyNoMoreInteractions;
 import static org.mockito.Mockito.when;
 
 import java.util.Arrays;
-import java.util.LinkedList;
-import java.util.List;
 import java.util.Optional;
 import java.util.UUID;
 import java.util.concurrent.TimeUnit;
-import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
 import org.whispersystems.textsecuregcm.storage.AccountsManager.DeletionReason;
 
 class AccountCleanerTest {
@@ -80,25 +77,4 @@ class AccountCleanerTest {
     verifyNoMoreInteractions(accountsManager);
   }
 
-  @Test
-  void testMaxAccountUpdates() throws AccountDatabaseCrawlerRestartException, InterruptedException {
-    List<Account> accounts = new LinkedList<>();
-    accounts.add(undeletedEnabledAccount);
-
-    int activeExpiredAccountCount = AccountCleaner.MAX_ACCOUNT_DELETIONS_PER_CHUNK + 1;
-    for (int addedAccountCount = 0; addedAccountCount < activeExpiredAccountCount; addedAccountCount++) {
-      accounts.add(undeletedDisabledAccount);
-    }
-
-    accounts.add(deletedDisabledAccount);
-
-    AccountCleaner accountCleaner = new AccountCleaner(accountsManager);
-    accountCleaner.onCrawlStart();
-    accountCleaner.timeAndProcessCrawlChunk(Optional.empty(), accounts);
-    accountCleaner.onCrawlEnd(Optional.empty());
-
-    verify(accountsManager, times(AccountCleaner.MAX_ACCOUNT_DELETIONS_PER_CHUNK)).delete(undeletedDisabledAccount, AccountsManager.DeletionReason.EXPIRED);
-    verifyNoMoreInteractions(accountsManager);
-  }
-
 }