diff --git a/app/src/androidTest/java/org/thoughtcrime/securesms/database/RecipientTableTest.kt b/app/src/androidTest/java/org/thoughtcrime/securesms/database/RecipientTableTest.kt index 0b456cc899..3b2ce69942 100644 --- a/app/src/androidTest/java/org/thoughtcrime/securesms/database/RecipientTableTest.kt +++ b/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() diff --git a/app/src/androidTest/java/org/thoughtcrime/securesms/storage/ContactRecordProcessorTest.kt b/app/src/androidTest/java/org/thoughtcrime/securesms/storage/ContactRecordProcessorTest.kt index f63fd8c667..782c75b419 100644 --- a/app/src/androidTest/java/org/thoughtcrime/securesms/storage/ContactRecordProcessorTest.kt +++ b/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 diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/RecipientTable.kt b/app/src/main/java/org/thoughtcrime/securesms/database/RecipientTable.kt index d095395beb..c15ed20db4 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/database/RecipientTable.kt +++ b/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) { - val record = getByStorageId(storageId)!! - if (record.aci == null || record.pni == null) { - Log.w(TAG, "Invalid state for split, ignoring.") + fun splitForStorageSyncIfNecessary(aci: ACI) { + val recipientId = getByAci(aci).getOrNull() ?: return + val record = getRecord(recipientId) + + if (record.pni == null && record.e164 == null) { return } + Log.i(TAG, "Splitting $recipientId for storage sync", true) + writableDatabase .update(TABLE_NAME) .values( diff --git a/app/src/main/java/org/thoughtcrime/securesms/storage/ContactRecordProcessor.java b/app/src/main/java/org/thoughtcrime/securesms/storage/ContactRecordProcessor.java index b090198589..aaa67cdc14 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/storage/ContactRecordProcessor.java +++ b/app/src/main/java/org/thoughtcrime/securesms/storage/ContactRecordProcessor.java @@ -56,17 +56,13 @@ public class ContactRecordProcessor extends DefaultStorageRecordProcessor remoteRecords, @NonNull StorageKeyGenerator keyGenerator) throws IOException { List unregisteredAciOnly = new ArrayList<>(); - List pniE164Only = new ArrayList<>(); for (SignalContactRecord remoteRecord : remoteRecords) { if (isInvalid(remoteRecord)) { @@ -75,39 +71,15 @@ public class ContactRecordProcessor extends DefaultStorageRecordProcessor 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()) { - super.process(remoteRecords, keyGenerator); - return; - } - - 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 localMatches = new TreeSet<>(this); - - for (SignalContactRecord aciOnly : unregisteredAciOnly) { - Optional localMatch = getMatching(aciOnly, keyGenerator); - - if (localMatch.isPresent()) { - localMatches.add(localMatch.get()); + if (unregisteredAciOnly.size() > 0) { + for (SignalContactRecord aciOnly : unregisteredAciOnly) { + SignalDatabase.recipients().splitForStorageSyncIfNecessary(aciOnly.getAci().get()); } } - for (SignalContactRecord pniOnly : pniE164Only) { - Optional 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); }