Skip redundant merge storage service update

This commit is contained in:
Max Radermacher 2026-05-06 15:01:33 -05:00 committed by GitHub
parent 489271dae8
commit 4f23af1cdd
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 43 additions and 16 deletions

View File

@ -104,7 +104,7 @@ public class RegistrationStateChangeManagerImpl: RegistrationStateChangeManager
tx: tx,
)
didUpdateLocalIdentifiers(e164: e164, aci: aci, pni: pni, deviceId: .primary, tx: tx)
didUpdateLocalIdentifiers(e164: e164, aci: aci, pni: pni, deviceId: .primary, shouldUpdateStorageService: true, tx: tx)
tx.addSyncCompletion {
self.postLocalNumberDidChangeNotification()
@ -128,7 +128,7 @@ public class RegistrationStateChangeManagerImpl: RegistrationStateChangeManager
serverAuthToken: authToken,
tx: tx,
)
didUpdateLocalIdentifiers(e164: e164, aci: aci, pni: pni, deviceId: deviceId, tx: tx)
didUpdateLocalIdentifiers(e164: e164, aci: aci, pni: pni, deviceId: deviceId, shouldUpdateStorageService: false, tx: tx)
tx.addSyncCompletion {
self.postLocalNumberDidChangeNotification()
@ -144,7 +144,7 @@ public class RegistrationStateChangeManagerImpl: RegistrationStateChangeManager
) {
tsAccountManager.changeLocalNumber(newE164: e164, aci: aci, pni: pni, tx: tx)
didUpdateLocalIdentifiers(e164: e164, aci: aci, pni: pni, deviceId: .primary, tx: tx)
didUpdateLocalIdentifiers(e164: e164, aci: aci, pni: pni, deviceId: .primary, shouldUpdateStorageService: false, tx: tx)
tx.addSyncCompletion {
self.postLocalNumberDidChangeNotification()
@ -331,6 +331,7 @@ public class RegistrationStateChangeManagerImpl: RegistrationStateChangeManager
aci: Aci,
pni: Pni,
deviceId: DeviceId,
shouldUpdateStorageService: Bool,
tx: DBWriteTransaction,
) {
udManager.removeSenderCertificates(tx: tx)
@ -346,6 +347,7 @@ public class RegistrationStateChangeManagerImpl: RegistrationStateChangeManager
aci: aci,
phoneNumber: e164,
pni: pni,
shouldUpdateStorageService: shouldUpdateStorageService,
tx: tx,
)
// Always add the .primary DeviceId as well as our own. This is how linked
@ -354,7 +356,7 @@ public class RegistrationStateChangeManagerImpl: RegistrationStateChangeManager
&recipient,
deviceIdsToAdd: [deviceId, .primary],
deviceIdsToRemove: [],
shouldUpdateStorageService: false,
shouldUpdateStorageService: shouldUpdateStorageService,
tx: tx,
)
// Always make sure we haven't blocked ourselves. (It's logically
@ -404,6 +406,7 @@ extension RegistrationStateChangeManagerImpl {
aci: localIdentifiers.aci,
pni: localIdentifiers.pni!,
deviceId: .primary,
shouldUpdateStorageService: false,
tx: tx,
)
}

View File

@ -95,6 +95,7 @@ public class AccountChecker {
recipientMerger.splitUnregisteredRecipientIfNeeded(
localIdentifiers: localIdentifiers,
unregisteredRecipient: &recipient,
shouldUpdateStorageService: true,
tx: tx,
)
}

View File

@ -14,6 +14,7 @@ public protocol RecipientMerger {
aci: Aci,
phoneNumber: E164,
pni: Pni?,
shouldUpdateStorageService: Bool,
tx: DBWriteTransaction,
) -> SignalRecipient
@ -63,6 +64,7 @@ public protocol RecipientMerger {
func splitUnregisteredRecipientIfNeeded(
localIdentifiers: LocalIdentifiers,
unregisteredRecipient: inout SignalRecipient,
shouldUpdateStorageService: Bool,
tx: DBWriteTransaction,
)
}
@ -207,11 +209,12 @@ class RecipientMergerImpl: RecipientMerger {
aci: Aci,
phoneNumber: E164,
pni: Pni?,
shouldUpdateStorageService: Bool,
tx: DBWriteTransaction,
) -> SignalRecipient {
let aciResult = mergeAlways(aci: aci, phoneNumber: phoneNumber, isLocalRecipient: true, tx: tx)
let aciResult = mergeAlways(aci: aci, phoneNumber: phoneNumber, isLocalRecipient: true, shouldUpdateStorageService: shouldUpdateStorageService, tx: tx)
if let pni {
return mergeAlways(phoneNumber: phoneNumber, pni: pni, isLocalRecipient: true, tx: tx)
return mergeAlways(phoneNumber: phoneNumber, pni: pni, isLocalRecipient: true, shouldUpdateStorageService: shouldUpdateStorageService, tx: tx)
}
return aciResult
}
@ -270,14 +273,14 @@ class RecipientMergerImpl: RecipientMerger {
return nil
}
// Explicit cast to guarantee this method doesn't return an Optional.
return mergeAlways(aci: aci, phoneNumber: phoneNumber, isLocalRecipient: false, tx: tx) as SignalRecipient
return mergeAlways(aci: aci, phoneNumber: phoneNumber, isLocalRecipient: false, shouldUpdateStorageService: false, tx: tx) as SignalRecipient
}()
let phoneNumberPniRecipient: SignalRecipient? = {
guard let phoneNumber, let pni = serviceIds.pni else {
return nil
}
// Explicit cast to guarantee this method doesn't return an Optional.
return mergeAlways(phoneNumber: phoneNumber, pni: pni, isLocalRecipient: false, tx: tx) as SignalRecipient
return mergeAlways(phoneNumber: phoneNumber, pni: pni, isLocalRecipient: false, shouldUpdateStorageService: false, tx: tx) as SignalRecipient
}()
if let phoneNumberResult = phoneNumberPniRecipient ?? aciPhoneNumberRecipient {
return phoneNumberResult
@ -304,7 +307,7 @@ class RecipientMergerImpl: RecipientMerger {
guard let phoneNumber else {
return recipientFetcher.fetchOrCreate(serviceId: aci, tx: tx)
}
return mergeIfNotLocalIdentifier(localIdentifiers: localIdentifiers, aci: aci, phoneNumber: phoneNumber, tx: tx)
return mergeIfNotLocalIdentifier(localIdentifiers: localIdentifiers, aci: aci, phoneNumber: phoneNumber, shouldUpdateStorageService: false, tx: tx)
}
func applyMergeFromSealedSender(
@ -316,7 +319,7 @@ class RecipientMergerImpl: RecipientMerger {
guard let phoneNumber else {
return recipientFetcher.fetchOrCreate(serviceId: aci, tx: tx)
}
return mergeIfNotLocalIdentifier(localIdentifiers: localIdentifiers, aci: aci, phoneNumber: phoneNumber, tx: tx)
return mergeIfNotLocalIdentifier(localIdentifiers: localIdentifiers, aci: aci, phoneNumber: phoneNumber, shouldUpdateStorageService: true, tx: tx)
}
func applyMergeFromPniSignature(
@ -345,6 +348,7 @@ class RecipientMergerImpl: RecipientMerger {
mightReplaceNonnilPhoneNumber: true,
insertSessionSwitchoverIfNeeded: false,
isLocalMerge: false,
shouldUpdateStorageService: true,
tx: tx,
) { _ in
aciRecipient.phoneNumber = pniRecipient.phoneNumber
@ -384,14 +388,15 @@ class RecipientMergerImpl: RecipientMerger {
aci = nil
}
if let aci {
_ = mergeAlways(aci: aci, phoneNumber: phoneNumber, isLocalRecipient: false, tx: tx)
_ = mergeAlways(aci: aci, phoneNumber: phoneNumber, isLocalRecipient: false, shouldUpdateStorageService: true, tx: tx)
}
return mergeAlways(phoneNumber: phoneNumber, pni: pni, isLocalRecipient: false, tx: tx)
return mergeAlways(phoneNumber: phoneNumber, pni: pni, isLocalRecipient: false, shouldUpdateStorageService: true, tx: tx)
}
func splitUnregisteredRecipientIfNeeded(
localIdentifiers: LocalIdentifiers,
unregisteredRecipient: inout SignalRecipient,
shouldUpdateStorageService: Bool,
tx: DBWriteTransaction,
) {
// We can't split if they're registered or lacking an ACI.
@ -411,6 +416,7 @@ class RecipientMergerImpl: RecipientMerger {
mightReplaceNonnilPhoneNumber: true,
insertSessionSwitchoverIfNeeded: true,
isLocalMerge: false,
shouldUpdateStorageService: shouldUpdateStorageService,
tx: tx,
) { tx in
var splitRecipient = failIfThrowsDatabaseError { () throws(GRDB.DatabaseError) in
@ -433,12 +439,19 @@ class RecipientMergerImpl: RecipientMerger {
localIdentifiers: LocalIdentifiers,
aci: Aci,
phoneNumber: E164,
shouldUpdateStorageService: Bool,
tx: DBWriteTransaction,
) -> SignalRecipient {
if localIdentifiers.containsAnyOf(aci: aci, phoneNumber: phoneNumber, pni: nil) {
return recipientFetcher.fetchOrCreate(serviceId: aci, tx: tx)
}
return mergeAlways(aci: aci, phoneNumber: phoneNumber, isLocalRecipient: false, tx: tx)
return mergeAlways(
aci: aci,
phoneNumber: phoneNumber,
isLocalRecipient: false,
shouldUpdateStorageService: shouldUpdateStorageService,
tx: tx,
)
}
// MARK: - Merge Logic
@ -466,6 +479,7 @@ class RecipientMergerImpl: RecipientMerger {
aci: Aci,
phoneNumber: E164,
isLocalRecipient: Bool,
shouldUpdateStorageService: Bool,
tx: DBWriteTransaction,
) -> SignalRecipient {
let aciRecipient = recipientDatabaseTable.fetchRecipient(serviceId: aci, transaction: tx)
@ -494,6 +508,7 @@ class RecipientMergerImpl: RecipientMerger {
mightReplaceNonnilPhoneNumber: true,
insertSessionSwitchoverIfNeeded: true,
isLocalMerge: isLocalRecipient,
shouldUpdateStorageService: shouldUpdateStorageService,
tx: tx,
) { tx in
let mergeResult = _mergeHighTrust(
@ -565,6 +580,7 @@ class RecipientMergerImpl: RecipientMerger {
phoneNumber: E164,
pni: Pni,
isLocalRecipient: Bool,
shouldUpdateStorageService: Bool,
tx: DBWriteTransaction,
) -> SignalRecipient {
let phoneNumberRecipient = recipientDatabaseTable.fetchRecipient(phoneNumber: phoneNumber.stringValue, transaction: tx)
@ -583,6 +599,7 @@ class RecipientMergerImpl: RecipientMerger {
mightReplaceNonnilPhoneNumber: false,
insertSessionSwitchoverIfNeeded: true,
isLocalMerge: isLocalRecipient,
shouldUpdateStorageService: shouldUpdateStorageService,
tx: tx,
) { tx in
let mergeResult = _mergeAlways(
@ -669,6 +686,7 @@ class RecipientMergerImpl: RecipientMerger {
mightReplaceNonnilPhoneNumber: false,
insertSessionSwitchoverIfNeeded: true,
isLocalMerge: false,
shouldUpdateStorageService: false,
tx: tx,
) { tx in
let mergeResult = _mergeAlwaysFromStorageService(
@ -760,6 +778,7 @@ class RecipientMergerImpl: RecipientMerger {
mightReplaceNonnilPhoneNumber: Bool,
insertSessionSwitchoverIfNeeded: Bool,
isLocalMerge: Bool,
shouldUpdateStorageService: Bool,
tx: DBWriteTransaction,
applyMerge: (DBWriteTransaction) -> (mergedRecipient: SignalRecipient, otherUpdatedRecipients: [SignalRecipient]),
) -> SignalRecipient {
@ -814,7 +833,9 @@ class RecipientMergerImpl: RecipientMerger {
}
}
storageServiceManager.recordPendingUpdates(updatedRecipientUniqueIds: affectedRecipients.map { $0.uniqueId })
if shouldUpdateStorageService {
storageServiceManager.recordPendingUpdates(updatedRecipientUniqueIds: affectedRecipients.map { $0.uniqueId })
}
let threadMergeEventCount = observers.didLearnAssociation(
mergedRecipient: MergedRecipient(

View File

@ -310,6 +310,7 @@ public class SSKEnvironment: NSObject {
aci: localIdentifiers.aci,
phoneNumber: phoneNumber,
pni: localIdentifiers.pni,
shouldUpdateStorageService: true,
tx: tx,
)
blockedRecipientStore.setBlocked(false, recipientId: localRecipient.id, tx: tx)

View File

@ -463,6 +463,7 @@ class StorageServiceContactRecordUpdater: StorageServiceRecordUpdater {
recipientMerger.splitUnregisteredRecipientIfNeeded(
localIdentifiers: localIdentifiers,
unregisteredRecipient: &recipient,
shouldUpdateStorageService: false,
tx: transaction,
)
}

View File

@ -9,7 +9,7 @@ import XCTest
@testable import SignalServiceKit
private class MockRecipientMerger: RecipientMerger {
func applyMergeForLocalAccount(aci: Aci, phoneNumber: E164, pni: Pni?, tx: DBWriteTransaction) -> SignalRecipient {
func applyMergeForLocalAccount(aci: Aci, phoneNumber: E164, pni: Pni?, shouldUpdateStorageService: Bool, tx: DBWriteTransaction) -> SignalRecipient {
fatalError()
}
@ -34,7 +34,7 @@ private class MockRecipientMerger: RecipientMerger {
appliedMergesFromPniSignatures += 1
}
func splitUnregisteredRecipientIfNeeded(localIdentifiers: LocalIdentifiers, unregisteredRecipient: inout SignalRecipient, tx: DBWriteTransaction) {
func splitUnregisteredRecipientIfNeeded(localIdentifiers: LocalIdentifiers, unregisteredRecipient: inout SignalRecipient, shouldUpdateStorageService: Bool, tx: DBWriteTransaction) {
fatalError()
}
}