From 42d0d84ae024df684f9d842e512db0c25505b0a1 Mon Sep 17 00:00:00 2001
From: Greyson Parrelli <greyson@signal.org>
Date: Thu, 18 Nov 2021 13:19:32 -0500
Subject: [PATCH] Handle the case where a number changes during a recipient
 merge.

---
 .../database/RecipientDatabaseTest.kt         | 22 ++++++++++++++
 .../securesms/database/RecipientDatabase.java | 29 ++++++++++++++-----
 2 files changed, 44 insertions(+), 7 deletions(-)

diff --git a/app/src/androidTest/java/org/thoughtcrime/securesms/database/RecipientDatabaseTest.kt b/app/src/androidTest/java/org/thoughtcrime/securesms/database/RecipientDatabaseTest.kt
index 0470d22e5..82a19a3af 100644
--- a/app/src/androidTest/java/org/thoughtcrime/securesms/database/RecipientDatabaseTest.kt
+++ b/app/src/androidTest/java/org/thoughtcrime/securesms/database/RecipientDatabaseTest.kt
@@ -302,6 +302,28 @@ class RecipientDatabaseTest {
     assertEquals(E164_A, existingRecipient2.requireE164())
   }
 
+  /**
+   * Another high trust case that results in a merge. Nothing strictly new here, but this case is called out because it’s a merge but *also* an E164 change,
+   * which clients may need to know for UX purposes.
+   */
+  @Test
+  fun getAndPossiblyMerge_bothAciAndE164MapToExistingUser_aciAndE164_mergeAndPhoneNumberChange_highTrust() {
+    val existingId1: RecipientId = recipientDatabase.getAndPossiblyMerge(ACI_A, E164_B, true)
+    val existingId2: RecipientId = recipientDatabase.getAndPossiblyMerge(null, E164_A, true)
+
+    val retrievedId: RecipientId = recipientDatabase.getAndPossiblyMerge(ACI_A, E164_A, true)
+    assertEquals(existingId1, retrievedId)
+
+    val retrievedRecipient = Recipient.resolved(retrievedId)
+    assertEquals(ACI_A, retrievedRecipient.requireAci())
+    assertEquals(E164_A, retrievedRecipient.requireE164())
+
+    assertFalse(recipientDatabase.getByE164(E164_B).isPresent)
+
+    val recipientWithId2 = Recipient.resolved(existingId2)
+    assertEquals(retrievedId, recipientWithId2.id)
+  }
+
   // ==============================================================
   // Misc
   // ==============================================================
diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/RecipientDatabase.java b/app/src/main/java/org/thoughtcrime/securesms/database/RecipientDatabase.java
index 6e2bf9d36..81649246e 100644
--- a/app/src/main/java/org/thoughtcrime/securesms/database/RecipientDatabase.java
+++ b/app/src/main/java/org/thoughtcrime/securesms/database/RecipientDatabase.java
@@ -548,14 +548,29 @@ public class RecipientDatabase extends Database {
               finalId = byAci.get();
             }
           } else {
-            if (highTrust) {
-              Log.w(TAG, "We have one contact with just an E164, and another with just an ACI. High-trust, so merging the two rows together.", true);
-              finalId                 = merge(byAci.get(), byE164.get());
-              recipientNeedingRefresh = byAci.get();
-              remapped                = new Pair<>(byE164.get(), byAci.get());
+            RecipientSettings aciSettings = getRecipientSettings(byAci.get());
+
+            if (aciSettings.getE164() != null) {
+              if (highTrust) {
+                Log.w(TAG, "We have one contact with just an E164, and another with both an ACI and a different E164. High-trust, so merging the two rows together. The E164 has also effectively changed for the ACI contact.", true);
+                finalId                 = merge(byAci.get(), byE164.get());
+                recipientNeedingRefresh = byAci.get();
+                remapped                = new Pair<>(byE164.get(), byAci.get());
+                recipientChangedNumber  = finalId;
+              } else {
+                Log.w(TAG, "We have one contact with just an E164, and another with both an ACI and a different E164. Low-trust, so doing nothing.", true);
+                finalId  = byAci.get();
+              }
             } else {
-              Log.w(TAG, "We have one contact with just an E164, and another with just an ACI. Low-trust, so doing nothing.", true);
-              finalId  = byAci.get();
+              if (highTrust) {
+                Log.w(TAG, "We have one contact with just an E164, and another with just an ACI. High-trust, so merging the two rows together.", true);
+                finalId                 = merge(byAci.get(), byE164.get());
+                recipientNeedingRefresh = byAci.get();
+                remapped                = new Pair<>(byE164.get(), byAci.get());
+              } else {
+                Log.w(TAG, "We have one contact with just an E164, and another with just an ACI. Low-trust, so doing nothing.", true);
+                finalId  = byAci.get();
+              }
             }
           }
         }