From f84e7aebd0c90364f50e547bd6447f68678293a7 Mon Sep 17 00:00:00 2001
From: Jon Chambers <jon@signal.org>
Date: Mon, 11 Oct 2021 17:38:09 -0400
Subject: [PATCH] Count numbers that can't be normalized because another
 account has the normalized form of the number

---
 .../textsecuregcm/WhisperServerService.java   |  2 +-
 .../NonNormalizedAccountCrawlerListener.java  | 44 +++++++++++++++----
 2 files changed, 37 insertions(+), 9 deletions(-)

diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java b/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java
index d47ab4731..01a45eb55 100644
--- a/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java
+++ b/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java
@@ -514,7 +514,7 @@ public class WhisperServerService extends Application<WhisperServerConfiguration
           directoryServerConfiguration.getReplicationName(), directoryReconciliationClient);
       deletedAccountsDirectoryReconcilers.add(deletedAccountsDirectoryReconciler);
     }
-    accountDatabaseCrawlerListeners.add(new NonNormalizedAccountCrawlerListener(metricsCluster));
+    accountDatabaseCrawlerListeners.add(new NonNormalizedAccountCrawlerListener(accountsManager, metricsCluster));
     accountDatabaseCrawlerListeners.add(new ContactDiscoveryWriter(accountsManager));
     // PushFeedbackProcessor may update device properties
     accountDatabaseCrawlerListeners.add(new PushFeedbackProcessor(accountsManager));
diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/storage/NonNormalizedAccountCrawlerListener.java b/service/src/main/java/org/whispersystems/textsecuregcm/storage/NonNormalizedAccountCrawlerListener.java
index da255c25e..62fa75940 100644
--- a/service/src/main/java/org/whispersystems/textsecuregcm/storage/NonNormalizedAccountCrawlerListener.java
+++ b/service/src/main/java/org/whispersystems/textsecuregcm/storage/NonNormalizedAccountCrawlerListener.java
@@ -19,24 +19,29 @@ import java.util.UUID;
 
 public class NonNormalizedAccountCrawlerListener extends AccountDatabaseCrawlerListener {
 
+  private final AccountsManager accountsManager;
   private final FaultTolerantRedisCluster metricsCluster;
 
   private static final String NORMALIZED_NUMBER_COUNT_KEY = "NonNormalizedAccountCrawlerListener::normalized";
   private static final String NON_NORMALIZED_NUMBER_COUNT_KEY = "NonNormalizedAccountCrawlerListener::nonNormalized";
+  private static final String CONFLICTING_NUMBER_COUNT_KEY = "NonNormalizedAccountCrawlerListener::conflicts";
 
   private static final PhoneNumberUtil PHONE_NUMBER_UTIL = PhoneNumberUtil.getInstance();
 
   private static final Logger log = LoggerFactory.getLogger(NonNormalizedAccountCrawlerListener.class);
 
-  public NonNormalizedAccountCrawlerListener(final FaultTolerantRedisCluster metricsCluster) {
+  public NonNormalizedAccountCrawlerListener(
+      final AccountsManager accountsManager,
+      final FaultTolerantRedisCluster metricsCluster) {
+
+    this.accountsManager = accountsManager;
     this.metricsCluster = metricsCluster;
   }
 
   @Override
   public void onCrawlStart() {
-    metricsCluster.useCluster(connection -> {
-      connection.sync().del(NORMALIZED_NUMBER_COUNT_KEY, NON_NORMALIZED_NUMBER_COUNT_KEY);
-    });
+    metricsCluster.useCluster(connection ->
+        connection.sync().del(NORMALIZED_NUMBER_COUNT_KEY, NON_NORMALIZED_NUMBER_COUNT_KEY, CONFLICTING_NUMBER_COUNT_KEY));
   }
 
   @Override
@@ -44,25 +49,41 @@ public class NonNormalizedAccountCrawlerListener extends AccountDatabaseCrawlerL
 
     final int normalizedNumbers;
     final int nonNormalizedNumbers;
+    final int conflictingNumbers;
     {
       int workingNormalizedNumbers = 0;
       int workingNonNormalizedNumbers = 0;
+      int workingConflictingNumbers = 0;
 
       for (final Account account : chunkAccounts) {
         if (hasNumberNormalized(account)) {
           workingNormalizedNumbers++;
         } else {
           workingNonNormalizedNumbers++;
+
+          try {
+            final Optional<Account> maybeConflictingAccount = accountsManager.get(getNormalizedNumber(account));
+
+            if (maybeConflictingAccount.isPresent()) {
+              workingConflictingNumbers++;
+              log.info("Normalized form of number for account {} conflicts with number for account {}",
+                  account.getUuid(), maybeConflictingAccount.get().getUuid());
+            }
+          } catch (final NumberParseException e) {
+            log.warn("Failed to parse phone number for account {}", account.getUuid(), e);
+          }
         }
       }
 
       normalizedNumbers = workingNormalizedNumbers;
       nonNormalizedNumbers = workingNonNormalizedNumbers;
+      conflictingNumbers = workingConflictingNumbers;
     }
 
     metricsCluster.useCluster(connection -> {
       connection.sync().incrby(NORMALIZED_NUMBER_COUNT_KEY, normalizedNumbers);
       connection.sync().incrby(NON_NORMALIZED_NUMBER_COUNT_KEY, nonNormalizedNumbers);
+      connection.sync().incrby(CONFLICTING_NUMBER_COUNT_KEY, conflictingNumbers);
     });
   }
 
@@ -74,18 +95,25 @@ public class NonNormalizedAccountCrawlerListener extends AccountDatabaseCrawlerL
     final int nonNormalizedNumbers = metricsCluster.withCluster(connection ->
         Integer.parseInt(connection.sync().get(NON_NORMALIZED_NUMBER_COUNT_KEY)));
 
-    log.info("Crawl completed. Normalized numbers: {}; non-normalized numbers: {}",
-        normalizedNumbers, nonNormalizedNumbers);
+    final int conflictingNumbers = metricsCluster.withCluster(connection ->
+        Integer.parseInt(connection.sync().get(CONFLICTING_NUMBER_COUNT_KEY)));
+
+    log.info("Crawl completed. Normalized numbers: {}; non-normalized numbers: {}; conflicting numbers: {}",
+        normalizedNumbers, nonNormalizedNumbers, conflictingNumbers);
   }
 
   @VisibleForTesting
   static boolean hasNumberNormalized(final Account account) {
     try {
-      final PhoneNumber phoneNumber = PHONE_NUMBER_UTIL.parse(account.getNumber(), null);
-      return account.getNumber().equals(PHONE_NUMBER_UTIL.format(phoneNumber, PhoneNumberFormat.E164));
+      return account.getNumber().equals(getNormalizedNumber(account));
     } catch (final NumberParseException e) {
       log.warn("Failed to parse phone number for account {}", account.getUuid(), e);
       return false;
     }
   }
+
+  private static String getNormalizedNumber(final Account account) throws NumberParseException {
+    final PhoneNumber phoneNumber = PHONE_NUMBER_UTIL.parse(account.getNumber(), null);
+    return PHONE_NUMBER_UTIL.format(phoneNumber, PhoneNumberFormat.E164);
+  }
 }