From 473c8b199e0c58fad0afabe6e03fd83ecc26bb7f Mon Sep 17 00:00:00 2001 From: Greyson Parrelli Date: Tue, 1 Nov 2022 17:47:35 -0400 Subject: [PATCH] Fix PNP CDS sync bug. --- .../sync/ContactDiscoveryRefreshV2.kt | 9 ++++-- .../securesms/database/RecipientDatabase.kt | 28 +++++++++++++++---- .../securesms/jobs/StorageForcePushJob.java | 7 ++++- 3 files changed, 34 insertions(+), 10 deletions(-) diff --git a/app/src/main/java/org/thoughtcrime/securesms/contacts/sync/ContactDiscoveryRefreshV2.kt b/app/src/main/java/org/thoughtcrime/securesms/contacts/sync/ContactDiscoveryRefreshV2.kt index 75ebf5cc8b..0c915f743a 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/contacts/sync/ContactDiscoveryRefreshV2.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/contacts/sync/ContactDiscoveryRefreshV2.kt @@ -186,13 +186,16 @@ object ContactDiscoveryRefreshV2 { SignalDatabase.recipients.rewritePhoneNumbers(fuzzyOutput.rewrites) stopwatch.split("rewrite-e164") - val existingIds: Set = SignalDatabase.recipients.getAllPossiblyRegisteredByE164(recipientE164s + rewrites.values) - val inactiveIds: Set = (existingIds - registeredIds).removeRegisteredButUnlisted() - registeredIds += SignalDatabase.recipients.bulkProcessCdsV2Result(fuzzyOutput.numbers) rewrites += fuzzyOutput.rewrites stopwatch.split("process-result") + val existingIds: Set = SignalDatabase.recipients.getAllPossiblyRegisteredByE164(recipientE164s + rewrites.values) + stopwatch.split("get-ids") + + val inactiveIds: Set = (existingIds - registeredIds).removeRegisteredButUnlisted() + stopwatch.split("registered-but-unlisted") + SignalDatabase.recipients.bulkUpdatedRegisteredStatusV2(registeredIds, inactiveIds) stopwatch.split("update-registered") } diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/RecipientDatabase.kt b/app/src/main/java/org/thoughtcrime/securesms/database/RecipientDatabase.kt index 4444d78064..cd7ba0a1be 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/database/RecipientDatabase.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/database/RecipientDatabase.kt @@ -475,8 +475,8 @@ open class RecipientDatabase(context: Context, databaseHelper: SignalDatabase) : else -> processPnpTuple(e164 = e164, pni = pni, aci = ACI.fromNullable(serviceId), pniVerified = pniVerified, changeSelf = changeSelf) } - if (result.operations.isNotEmpty()) { - Log.i(TAG, "[getAndPossiblyMergePnp] ($serviceId, $pni, $e164) BreadCrumbs: ${result.breadCrumbs}, Operations: ${result.operations}") + if (result.operations.isNotEmpty() || result.requiredInsert) { + Log.i(TAG, "[getAndPossiblyMerge] ($serviceId, $pni, $e164) BreadCrumbs: ${result.breadCrumbs}, Operations: ${result.operations}, RequiredInsert: ${result.requiredInsert}, FinalId: ${result.finalId}") } db.setTransactionSuccessful() @@ -601,11 +601,11 @@ open class RecipientDatabase(context: Context, databaseHelper: SignalDatabase) : } fun getOrInsertFromServiceId(serviceId: ServiceId): RecipientId { - return getOrInsertByColumn(SERVICE_ID, serviceId.toString()).recipientId + return getAndPossiblyMerge(serviceId = serviceId, e164 = null) } fun getOrInsertFromE164(e164: String): RecipientId { - return getOrInsertByColumn(PHONE, e164).recipientId + return getAndPossiblyMerge(serviceId = null, e164 = e164) } fun getOrInsertFromEmail(email: String): RecipientId { @@ -2183,6 +2183,7 @@ open class RecipientDatabase(context: Context, databaseHelper: SignalDatabase) : UNREGISTERED_TIMESTAMP to 0 ) if (update(id, contentValues)) { + Log.i(TAG, "Newly marked $id as registered.") setStorageIdIfNotSet(id) ApplicationDependencies.getDatabaseObserver().notifyRecipientChanged(id) } @@ -2191,11 +2192,11 @@ open class RecipientDatabase(context: Context, databaseHelper: SignalDatabase) : fun markUnregistered(id: RecipientId) { val contentValues = contentValuesOf( REGISTERED to RegisteredState.NOT_REGISTERED.id, - STORAGE_SERVICE_ID to null, UNREGISTERED_TIMESTAMP to System.currentTimeMillis() ) if (update(id, contentValues)) { + Log.i(TAG, "Newly marked $id as unregistered.") ApplicationDependencies.getDatabaseObserver().notifyRecipientChanged(id) } } @@ -2306,24 +2307,37 @@ open class RecipientDatabase(context: Context, databaseHelper: SignalDatabase) : UNREGISTERED_TIMESTAMP to 0 ) + val newlyRegistered: MutableSet = mutableSetOf() + for (id in registered) { if (update(id, registeredValues)) { + newlyRegistered += id setStorageIdIfNotSet(id) ApplicationDependencies.getDatabaseObserver().notifyRecipientChanged(id) } } + if (newlyRegistered.isNotEmpty()) { + Log.i(TAG, "Newly marked the following as registered: $newlyRegistered") + } + + val newlyUnregistered: MutableSet = mutableSetOf() + val unregisteredValues = contentValuesOf( REGISTERED to RegisteredState.NOT_REGISTERED.id, - STORAGE_SERVICE_ID to null, UNREGISTERED_TIMESTAMP to System.currentTimeMillis() ) for (id in unregistered) { if (update(id, unregisteredValues)) { + newlyUnregistered += id ApplicationDependencies.getDatabaseObserver().notifyRecipientChanged(id) } } + + if (newlyUnregistered.isNotEmpty()) { + Log.i(TAG, "Newly marked the following as unregistered: $newlyUnregistered") + } } } @@ -2364,6 +2378,7 @@ open class RecipientDatabase(context: Context, databaseHelper: SignalDatabase) : return ProcessPnpTupleResult( finalId = finalId, + requiredInsert = changeSet.id is PnpIdResolver.PnpInsert, affectedIds = affectedIds, oldIds = oldIds, changedNumberId = changedNumberId, @@ -4408,6 +4423,7 @@ open class RecipientDatabase(context: Context, databaseHelper: SignalDatabase) : data class ProcessPnpTupleResult( val finalId: RecipientId, + val requiredInsert: Boolean, val affectedIds: Set, val oldIds: Set, val changedNumberId: RecipientId?, diff --git a/app/src/main/java/org/thoughtcrime/securesms/jobs/StorageForcePushJob.java b/app/src/main/java/org/thoughtcrime/securesms/jobs/StorageForcePushJob.java index b8c62fc07e..bd450e4120 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/jobs/StorageForcePushJob.java +++ b/app/src/main/java/org/thoughtcrime/securesms/jobs/StorageForcePushJob.java @@ -76,11 +76,16 @@ public class StorageForcePushJob extends BaseJob { return; } - if (!SignalStore.account().isRegistered() || SignalStore.account().getE164() == null || Recipient.self().getStorageServiceId() == null) { + if (!SignalStore.account().isRegistered() || SignalStore.account().getE164() == null) { Log.w(TAG, "User not registered. Skipping."); return; } + if (Recipient.self().getStorageServiceId() == null) { + Log.w(TAG, "No storage ID set for self! Skipping."); + return; + } + StorageKey storageServiceKey = SignalStore.storageService().getOrCreateStorageKey(); SignalServiceAccountManager accountManager = ApplicationDependencies.getSignalServiceAccountManager(); RecipientDatabase recipientDatabase = SignalDatabase.recipients();