Simplify contact splitting when reading from storage service.

app/src/androidTest/java/org/thoughtcrime/securesms/database/RecipientTableTest.kt

@@ -190,7 +190,7 @@ class RecipientTableTest {
     val mainId = SignalDatabase.recipients.getAndPossiblyMerge(ACI_A, PNI_A, E164_A)
     val mainRecord = SignalDatabase.recipients.getRecord(mainId)
 
-    SignalDatabase.recipients.splitForStorageSync(mainRecord.storageId!!)
+    SignalDatabase.recipients.splitForStorageSyncIfNecessary(mainRecord.aci!!)
 
     val byAci: RecipientId = SignalDatabase.recipients.getByAci(ACI_A).get()
 

app/src/androidTest/java/org/thoughtcrime/securesms/storage/ContactRecordProcessorTest.kt

@@ -33,7 +33,7 @@ class ContactRecordProcessorTest {
   }
 
   @Test
-  fun process_splitContact_normalSplit() {
+  fun process_splitContact_normalSplit_twoRecords() {
     // GIVEN
     val originalId = SignalDatabase.recipients.getAndPossiblyMerge(ACI_A, PNI_A, E164_A)
     setStorageId(originalId, STORAGE_ID_A)
@@ -69,6 +69,35 @@ class ContactRecordProcessorTest {
     assertNotEquals(byAci, byE164)
   }
 
+  @Test
+  fun process_splitContact_normalSplit_oneRecord() {
+    // GIVEN
+    val originalId = SignalDatabase.recipients.getAndPossiblyMerge(ACI_A, PNI_A, E164_A)
+    setStorageId(originalId, STORAGE_ID_A)
+
+    val remote = buildRecord(
+      STORAGE_ID_B,
+      ContactRecord(
+        aci = ACI_A.toString(),
+        unregisteredAtTimestamp = 100
+      )
+    )
+
+    // WHEN
+    val subject = ContactRecordProcessor()
+    subject.process(listOf(remote), StorageSyncHelper.KEY_GENERATOR)
+
+    // THEN
+    val byAci: RecipientId = SignalDatabase.recipients.getByAci(ACI_A).get()
+
+    val byE164: RecipientId = SignalDatabase.recipients.getByE164(E164_A).get()
+    val byPni: RecipientId = SignalDatabase.recipients.getByPni(PNI_A).get()
+
+    assertEquals(originalId, byAci)
+    assertEquals(byE164, byPni)
+    assertNotEquals(byAci, byE164)
+  }
+
   @Test
   fun process_splitContact_doNotSplitIfAciRecordIsRegistered() {
     // GIVEN

app/src/main/java/org/thoughtcrime/securesms/database/RecipientTable.kt

@@ -113,6 +113,7 @@ import java.util.LinkedList
 import java.util.Objects
 import java.util.Optional
 import java.util.concurrent.TimeUnit
+import kotlin.jvm.optionals.getOrNull
 import kotlin.math.max
 
 open class RecipientTable(context: Context, databaseHelper: SignalDatabase) : DatabaseTable(context, databaseHelper) {
@@ -2246,13 +2247,16 @@ open class RecipientTable(context: Context, databaseHelper: SignalDatabase) : Da
    * Removes the target recipient's E164+PNI, then creates a new recipient with that E164+PNI.
    * Done so we can match a split contact during storage sync.
    */
-  fun splitForStorageSync(storageId: ByteArray) {
+  fun splitForStorageSyncIfNecessary(aci: ACI) {
-    val record = getByStorageId(storageId)!!
+    val recipientId = getByAci(aci).getOrNull() ?: return
-    if (record.aci == null || record.pni == null) {
+    val record = getRecord(recipientId)
-      Log.w(TAG, "Invalid state for split, ignoring.")
+
+    if (record.pni == null && record.e164 == null) {
       return
     }
 
+    Log.i(TAG, "Splitting $recipientId for storage sync", true)
+
     writableDatabase
       .update(TABLE_NAME)
       .values(

app/src/main/java/org/thoughtcrime/securesms/storage/ContactRecordProcessor.java

@@ -56,17 +56,13 @@ public class ContactRecordProcessor extends DefaultStorageRecordProcessor<Signal
   /**
    * For contact records specifically, we have some extra work that needs to be done before we process all of the records.
    *
-   * We have to look and see if there is an unregistered ACI-only record and another E164/PNI-only record that points to the
+   * We have to find all unregistered ACI-only records and split them into two separate contact rows locally, if necessary.
-   * same local contact row.
+   * The reasons are nuanced, but the TL;DR is that we want to split unregistered users into separate rows so that a user
-   *
+   * could re-register and get a different ACI.
-   * If so, we actually want to mimic the split and turn them into two separate contact rows locally. The reasons are nuanced,
-   * but the TL;DR is that we want to split unregistered users into separate rows so that a user could re-register and get a
-   * different ACI.
    */
   @Override
   public void process(@NonNull Collection<SignalContactRecord> remoteRecords, @NonNull StorageKeyGenerator keyGenerator) throws IOException {
     List<SignalContactRecord> unregisteredAciOnly = new ArrayList<>();
-    List<SignalContactRecord> pniE164Only         = new ArrayList<>();
 
     for (SignalContactRecord remoteRecord : remoteRecords) {
       if (isInvalid(remoteRecord)) {
@@ -75,39 +71,15 @@ public class ContactRecordProcessor extends DefaultStorageRecordProcessor<Signal
 
       if (remoteRecord.getUnregisteredTimestamp() > 0 && remoteRecord.getAci().isPresent() && remoteRecord.getPni().isEmpty() && remoteRecord.getNumber().isEmpty()) {
         unregisteredAciOnly.add(remoteRecord);
-      } else if (remoteRecord.getAci().isEmpty()) {
-        pniE164Only.add(remoteRecord);
       }
     }
 
-    if (unregisteredAciOnly.isEmpty() || pniE164Only.isEmpty()) {
+    if (unregisteredAciOnly.size() > 0) {
-      super.process(remoteRecords, keyGenerator);
+      for (SignalContactRecord aciOnly : unregisteredAciOnly) {
-      return;
+        SignalDatabase.recipients().splitForStorageSyncIfNecessary(aciOnly.getAci().get());
-    }
-
-    Log.i(TAG, "We have some unregistered ACI-only contacts as well as some PNI-only contacts. Need to do an intersection to detect any possible required splits.");
-
-    TreeSet<SignalContactRecord> localMatches = new TreeSet<>(this);
-
-    for (SignalContactRecord aciOnly : unregisteredAciOnly) {
-      Optional<SignalContactRecord> localMatch = getMatching(aciOnly, keyGenerator);
-
-      if (localMatch.isPresent()) {
-        localMatches.add(localMatch.get());
       }
     }
 
-    for (SignalContactRecord pniOnly : pniE164Only) {
-      Optional<SignalContactRecord> localMatch = getMatching(pniOnly, keyGenerator);
-
-      if (localMatch.isPresent() && localMatches.contains(localMatch.get())) {
-        Log.w(TAG, "Found a situation where we need to split our local record in two in order to match the remote state.");
-
-        SignalDatabase.recipients().splitForStorageSync(localMatch.get().getId().getRaw());
-      }
-    }
-
-
     super.process(remoteRecords, keyGenerator);
   }