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 0470d22e5a..82a19a3afe 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 6e2bf9d362..81649246ed 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(); + } } } }