From ffe05d93aa2b3ee994fce2930c6a32ff0fa9a5d4 Mon Sep 17 00:00:00 2001 From: Max Radermacher Date: Mon, 5 Jan 2026 12:47:46 -0600 Subject: [PATCH] Use failIfThrows in Blocked*Stores --- .../Privacy/BlockListViewController.swift | 2 +- .../Archivers/BackupArchive+Errors.swift | 3 - ...ackupArchiveContactRecipientArchiver.swift | 7 +- .../BackupArchiveGroupRecipientArchiver.swift | 7 +- .../Archiving/BackupArchive+Shims.swift | 12 +-- .../Messages/BlockedGroupStore.swift | 39 ++++----- .../Messages/BlockedRecipientStore.swift | 47 ++++------- .../Messages/BlockingManager.swift | 82 ++++++++----------- .../tests/Contacts/BlockingManagerTests.swift | 2 +- 9 files changed, 79 insertions(+), 122 deletions(-) diff --git a/Signal/src/ViewControllers/AppSettings/Privacy/BlockListViewController.swift b/Signal/src/ViewControllers/AppSettings/Privacy/BlockListViewController.swift index da58bb4eeb..0b4e78eb5f 100644 --- a/Signal/src/ViewControllers/AppSettings/Privacy/BlockListViewController.swift +++ b/Signal/src/ViewControllers/AppSettings/Privacy/BlockListViewController.swift @@ -62,7 +62,7 @@ class BlockListViewController: OWSTableViewController2 { transaction: transaction, ) let groups: [(groupId: Data, groupName: String, groupModel: TSGroupModel?, groupAvatarImage: UIImage?)] - groups = ((try? blockingManager.blockedGroupIds(transaction: transaction)) ?? []).map { groupId in + groups = blockingManager.blockedGroupIds(transaction: transaction).map { groupId in let groupModel = TSGroupThread.fetch(groupId: groupId, transaction: transaction)?.groupModel let groupName = groupModel?.groupNameOrDefault ?? TSGroupThread.defaultGroupName let groupAvatarImage: UIImage? = { diff --git a/SignalServiceKit/Backups/Archiving/Archivers/BackupArchive+Errors.swift b/SignalServiceKit/Backups/Archiving/Archivers/BackupArchive+Errors.swift index 9e7d0eaeef..3f08c210fd 100644 --- a/SignalServiceKit/Backups/Archiving/Archivers/BackupArchive+Errors.swift +++ b/SignalServiceKit/Backups/Archiving/Archivers/BackupArchive+Errors.swift @@ -545,9 +545,6 @@ extension BackupArchive { /// Error iterating over all ad hoc calls for backup purposes. case adHocCallIteratorError(RawError) - case blockedRecipientFetchError(RawError) - case blockedGroupFetchError(RawError) - case oversizedTextCacheFetchError(RawError) /// These should never happen; it means some invariant in the backup code diff --git a/SignalServiceKit/Backups/Archiving/Archivers/Recipient/BackupArchiveContactRecipientArchiver.swift b/SignalServiceKit/Backups/Archiving/Archivers/Recipient/BackupArchiveContactRecipientArchiver.swift index b0f6708f81..22aced5633 100644 --- a/SignalServiceKit/Backups/Archiving/Archivers/Recipient/BackupArchiveContactRecipientArchiver.swift +++ b/SignalServiceKit/Backups/Archiving/Archivers/Recipient/BackupArchiveContactRecipientArchiver.swift @@ -73,12 +73,7 @@ public class BackupArchiveContactRecipientArchiver: BackupArchiveProtoStreamWrit ) throws(CancellationError) -> ArchiveMultiFrameResult { let whitelistedAddresses = Set(profileManager.allWhitelistedAddresses(tx: context.tx)) - let blockedRecipientIds: Set - do { - blockedRecipientIds = try blockingManager.blockedRecipientIds(tx: context.tx) - } catch { - return .completeFailure(.fatalArchiveError(.blockedRecipientFetchError(error))) - } + let blockedRecipientIds = blockingManager.blockedRecipientIds(tx: context.tx) var errors = [ArchiveFrameError]() diff --git a/SignalServiceKit/Backups/Archiving/Archivers/Recipient/BackupArchiveGroupRecipientArchiver.swift b/SignalServiceKit/Backups/Archiving/Archivers/Recipient/BackupArchiveGroupRecipientArchiver.swift index b007607439..0a26654379 100644 --- a/SignalServiceKit/Backups/Archiving/Archivers/Recipient/BackupArchiveGroupRecipientArchiver.swift +++ b/SignalServiceKit/Backups/Archiving/Archivers/Recipient/BackupArchiveGroupRecipientArchiver.swift @@ -58,12 +58,7 @@ public class BackupArchiveGroupRecipientArchiver: BackupArchiveProtoStreamWriter ) throws(CancellationError) -> ArchiveMultiFrameResult { var errors = [ArchiveFrameError]() - let blockedGroupIds: Set - do { - blockedGroupIds = Set(try blockingManager.blockedGroupIds(tx: context.tx)) - } catch { - return .completeFailure(.fatalArchiveError(.blockedGroupFetchError(error))) - } + let blockedGroupIds = Set(blockingManager.blockedGroupIds(tx: context.tx)) do { try context.bencher.wrapEnumeration( diff --git a/SignalServiceKit/Backups/Archiving/BackupArchive+Shims.swift b/SignalServiceKit/Backups/Archiving/BackupArchive+Shims.swift index 04ad4ca8b1..c45ba37275 100644 --- a/SignalServiceKit/Backups/Archiving/BackupArchive+Shims.swift +++ b/SignalServiceKit/Backups/Archiving/BackupArchive+Shims.swift @@ -51,8 +51,8 @@ extension BackupArchive { public protocol _MessageBackup_BlockingManagerShim { - func blockedRecipientIds(tx: DBReadTransaction) throws -> Set - func blockedGroupIds(tx: DBReadTransaction) throws -> [Data] + func blockedRecipientIds(tx: DBReadTransaction) -> Set + func blockedGroupIds(tx: DBReadTransaction) -> [Data] func addBlockedAddress(_ address: SignalServiceAddress, tx: DBWriteTransaction) func addBlockedGroupId(_ groupId: Data, tx: DBWriteTransaction) @@ -66,12 +66,12 @@ public class _MessageBackup_BlockingManagerWrapper: _MessageBackup_BlockingManag self.blockingManager = blockingManager } - public func blockedRecipientIds(tx: DBReadTransaction) throws -> Set { - return try blockingManager.blockedRecipientIds(tx: tx) + public func blockedRecipientIds(tx: DBReadTransaction) -> Set { + return blockingManager.blockedRecipientIds(tx: tx) } - public func blockedGroupIds(tx: DBReadTransaction) throws -> [Data] { - return try blockingManager.blockedGroupIds(transaction: tx) + public func blockedGroupIds(tx: DBReadTransaction) -> [Data] { + return blockingManager.blockedGroupIds(transaction: tx) } public func addBlockedAddress(_ address: SignalServiceAddress, tx: DBWriteTransaction) { diff --git a/SignalServiceKit/Messages/BlockedGroupStore.swift b/SignalServiceKit/Messages/BlockedGroupStore.swift index 6e38ef3654..b92d6d9e5f 100644 --- a/SignalServiceKit/Messages/BlockedGroupStore.swift +++ b/SignalServiceKit/Messages/BlockedGroupStore.swift @@ -7,36 +7,29 @@ import Foundation import GRDB struct BlockedGroupStore { - func blockedGroupIds(tx: DBReadTransaction) throws -> [Data] { - let db = tx.database - do { - return try BlockedGroup.fetchAll(db).map(\.groupId) - } catch { - throw error.grdbErrorForLogging + func blockedGroupIds(tx: DBReadTransaction) -> [Data] { + return failIfThrows { + return try BlockedGroup.fetchAll(tx.database).map(\.groupId) } } - func isBlocked(groupId: Data, tx: DBReadTransaction) throws -> Bool { - let db = tx.database - do { - return try BlockedGroup.filter(key: groupId).fetchOne(db) != nil - } catch { - throw error.grdbErrorForLogging + func isBlocked(groupId: Data, tx: DBReadTransaction) -> Bool { + return failIfThrows { + return try BlockedGroup.filter(key: groupId).fetchOne(tx.database) != nil } } - func setBlocked(_ isBlocked: Bool, groupId: Data, tx: DBWriteTransaction) throws { - let db = tx.database - do { - if isBlocked { - try BlockedGroup(groupId: groupId).insert(db) - } else { - try BlockedGroup(groupId: groupId).delete(db) + func setBlocked(_ isBlocked: Bool, groupId: Data, tx: DBWriteTransaction) { + return failIfThrows { + do { + if isBlocked { + try BlockedGroup(groupId: groupId).insert(tx.database) + } else { + try BlockedGroup(groupId: groupId).delete(tx.database) + } + } catch DatabaseError.SQLITE_CONSTRAINT { + // It's already blocked -- this is fine. } - } catch DatabaseError.SQLITE_CONSTRAINT { - // It's already blocked -- this is fine. - } catch { - throw error.grdbErrorForLogging } } } diff --git a/SignalServiceKit/Messages/BlockedRecipientStore.swift b/SignalServiceKit/Messages/BlockedRecipientStore.swift index 83a45fe357..4737405796 100644 --- a/SignalServiceKit/Messages/BlockedRecipientStore.swift +++ b/SignalServiceKit/Messages/BlockedRecipientStore.swift @@ -7,46 +7,35 @@ import Foundation import GRDB struct BlockedRecipientStore { - func blockedRecipientIds(tx: DBReadTransaction) throws -> [SignalRecipient.RowId] { - let db = tx.database - do { - return try BlockedRecipient.fetchAll(db).map(\.recipientId) - } catch { - throw error.grdbErrorForLogging + func blockedRecipientIds(tx: DBReadTransaction) -> [SignalRecipient.RowId] { + return failIfThrows { + return try BlockedRecipient.fetchAll(tx.database).map(\.recipientId) } } - func isBlocked(recipientId: SignalRecipient.RowId, tx: DBReadTransaction) throws -> Bool { - let db = tx.database - do { - return try BlockedRecipient.filter(key: recipientId).fetchOne(db) != nil - } catch { - throw error.grdbErrorForLogging + func isBlocked(recipientId: SignalRecipient.RowId, tx: DBReadTransaction) -> Bool { + return failIfThrows { + return try BlockedRecipient.filter(key: recipientId).fetchOne(tx.database) != nil } } - func setBlocked(_ isBlocked: Bool, recipientId: SignalRecipient.RowId, tx: DBWriteTransaction) throws { - let db = tx.database - do { - if isBlocked { - try BlockedRecipient(recipientId: recipientId).insert(db) - } else { - try BlockedRecipient(recipientId: recipientId).delete(db) + func setBlocked(_ isBlocked: Bool, recipientId: SignalRecipient.RowId, tx: DBWriteTransaction) { + failIfThrows { + do { + if isBlocked { + try BlockedRecipient(recipientId: recipientId).insert(tx.database) + } else { + try BlockedRecipient(recipientId: recipientId).delete(tx.database) + } + } catch DatabaseError.SQLITE_CONSTRAINT { + // It's already blocked -- this is fine. } - } catch DatabaseError.SQLITE_CONSTRAINT { - // It's already blocked -- this is fine. - } catch { - throw error.grdbErrorForLogging } } func mergeRecipientId(_ recipientId: SignalRecipient.RowId, into targetRecipientId: SignalRecipient.RowId, tx: DBWriteTransaction) { - do { - if try self.isBlocked(recipientId: recipientId, tx: tx) { - try self.setBlocked(true, recipientId: targetRecipientId, tx: tx) - } - } catch { - Logger.warn("Couldn't merge BlockedRecipient") + if self.isBlocked(recipientId: recipientId, tx: tx) { + self.setBlocked(true, recipientId: targetRecipientId, tx: tx) } } } diff --git a/SignalServiceKit/Messages/BlockingManager.swift b/SignalServiceKit/Messages/BlockingManager.swift index e207c65484..2d95c6648a 100644 --- a/SignalServiceKit/Messages/BlockingManager.swift +++ b/SignalServiceKit/Messages/BlockingManager.swift @@ -80,7 +80,7 @@ public class BlockingManager { guard let recipientId = recipientDatabaseTable.fetchRecipient(address: address, tx: transaction)?.id else { return false } - return (try? blockedRecipientStore.isBlocked(recipientId: recipientId, tx: transaction)) ?? false + return blockedRecipientStore.isBlocked(recipientId: recipientId, tx: transaction) } public func isGroupIdBlocked(_ groupId: GroupIdentifier, transaction tx: DBReadTransaction) -> Bool { @@ -92,24 +92,24 @@ public class BlockingManager { } private func _isGroupIdBlocked(_ groupId: Data, tx: DBReadTransaction) -> Bool { - return (try? blockedGroupStore.isBlocked(groupId: groupId, tx: tx)) ?? false + return blockedGroupStore.isBlocked(groupId: groupId, tx: tx) } - public func blockedRecipientIds(tx: DBReadTransaction) throws -> Set { - return Set(try blockedRecipientStore.blockedRecipientIds(tx: tx)) + public func blockedRecipientIds(tx: DBReadTransaction) -> Set { + return Set(blockedRecipientStore.blockedRecipientIds(tx: tx)) } public func blockedAddresses(transaction: DBReadTransaction) -> Set { let recipientDatabaseTable = DependenciesBridge.shared.recipientDatabaseTable - let blockedRecipientIds = (try? self.blockedRecipientIds(tx: transaction)) ?? [] + let blockedRecipientIds = self.blockedRecipientIds(tx: transaction) return Set(blockedRecipientIds.compactMap { return recipientDatabaseTable.fetchRecipient(rowId: $0, tx: transaction)?.address }) } - public func blockedGroupIds(transaction: DBReadTransaction) throws -> [Data] { - return try blockedGroupStore.blockedGroupIds(tx: transaction) + public func blockedGroupIds(transaction: DBReadTransaction) -> [Data] { + return blockedGroupStore.blockedGroupIds(tx: transaction) } public func addBlockedAci(_ aci: Aci, blockMode: BlockMode, tx: DBWriteTransaction) { @@ -137,13 +137,11 @@ public class BlockingManager { return } - let isBlocked = failIfThrows { try blockedRecipientStore.isBlocked(recipientId: recipient.id, tx: tx) } + let isBlocked = blockedRecipientStore.isBlocked(recipientId: recipient.id, tx: tx) guard !isBlocked else { return } - failIfThrows { - try blockedRecipientStore.setBlocked(true, recipientId: recipient.id, tx: tx) - } + blockedRecipientStore.setBlocked(true, recipientId: recipient.id, tx: tx) Logger.info("Added blocked address: \(address)") @@ -199,13 +197,11 @@ public class BlockingManager { return } - let isBlocked = failIfThrows { try blockedRecipientStore.isBlocked(recipientId: recipient.id, tx: tx) } + let isBlocked = blockedRecipientStore.isBlocked(recipientId: recipient.id, tx: tx) guard isBlocked else { return } - failIfThrows { - try blockedRecipientStore.setBlocked(false, recipientId: recipient.id, tx: tx) - } + blockedRecipientStore.setBlocked(false, recipientId: recipient.id, tx: tx) Logger.info("Removed blocked address: \(address)") @@ -232,13 +228,11 @@ public class BlockingManager { return } - let isBlocked = failIfThrows { try blockedGroupStore.isBlocked(groupId: groupId, tx: transaction) } + let isBlocked = blockedGroupStore.isBlocked(groupId: groupId, tx: transaction) guard !isBlocked else { return } - failIfThrows { - try blockedGroupStore.setBlocked(true, groupId: groupId, tx: transaction) - } + blockedGroupStore.setBlocked(true, groupId: groupId, tx: transaction) Logger.info("Added blocked groupId: \(groupId.toHex())") @@ -279,13 +273,11 @@ public class BlockingManager { } public func removeBlockedGroup(groupId: Data, wasLocallyInitiated: Bool, transaction: DBWriteTransaction) { - let isBlocked = failIfThrows { try blockedGroupStore.isBlocked(groupId: groupId, tx: transaction) } + let isBlocked = blockedGroupStore.isBlocked(groupId: groupId, tx: transaction) guard isBlocked else { return } - failIfThrows { - try blockedGroupStore.setBlocked(false, groupId: groupId, tx: transaction) - } + blockedGroupStore.setBlocked(false, groupId: groupId, tx: transaction) Logger.info("Removed blocked groupId: \(groupId.toHex())") @@ -372,17 +364,15 @@ public class BlockingManager { var didChange = false - failIfThrows { - let oldBlockedGroupIds = Set(try blockedGroupStore.blockedGroupIds(tx: transaction)) - let newBlockedGroupIds = blockedGroupIds - try oldBlockedGroupIds.subtracting(newBlockedGroupIds).forEach { - didChange = true - try blockedGroupStore.setBlocked(false, groupId: $0, tx: transaction) - } - try newBlockedGroupIds.subtracting(oldBlockedGroupIds).forEach { - didChange = true - try blockedGroupStore.setBlocked(true, groupId: $0, tx: transaction) - } + let oldBlockedGroupIds = Set(blockedGroupStore.blockedGroupIds(tx: transaction)) + let newBlockedGroupIds = blockedGroupIds + oldBlockedGroupIds.subtracting(newBlockedGroupIds).forEach { + didChange = true + blockedGroupStore.setBlocked(false, groupId: $0, tx: transaction) + } + newBlockedGroupIds.subtracting(oldBlockedGroupIds).forEach { + didChange = true + blockedGroupStore.setBlocked(true, groupId: $0, tx: transaction) } var blockedRecipientIds = Set() @@ -394,17 +384,15 @@ public class BlockingManager { blockedRecipientIds.insert(recipientFetcher.fetchOrCreate(phoneNumber: blockedPhoneNumber, tx: transaction).id) } - failIfThrows { - let oldBlockedRecipientIds = Set(try blockedRecipientStore.blockedRecipientIds(tx: transaction)) - let newBlockedRecipientIds = blockedRecipientIds - try oldBlockedRecipientIds.subtracting(newBlockedRecipientIds).forEach { - didChange = true - try blockedRecipientStore.setBlocked(false, recipientId: $0, tx: transaction) - } - try newBlockedRecipientIds.subtracting(oldBlockedRecipientIds).forEach { - didChange = true - try blockedRecipientStore.setBlocked(true, recipientId: $0, tx: transaction) - } + let oldBlockedRecipientIds = Set(blockedRecipientStore.blockedRecipientIds(tx: transaction)) + let newBlockedRecipientIds = blockedRecipientIds + oldBlockedRecipientIds.subtracting(newBlockedRecipientIds).forEach { + didChange = true + blockedRecipientStore.setBlocked(false, recipientId: $0, tx: transaction) + } + newBlockedRecipientIds.subtracting(oldBlockedRecipientIds).forEach { + didChange = true + blockedRecipientStore.setBlocked(true, recipientId: $0, tx: transaction) } if didChange { @@ -435,11 +423,11 @@ public class BlockingManager { ) let recipientDatabaseTable = DependenciesBridge.shared.recipientDatabaseTable - let blockedRecipients = try blockedRecipientStore.blockedRecipientIds(tx: tx).compactMap { + let blockedRecipients = blockedRecipientStore.blockedRecipientIds(tx: tx).compactMap { return recipientDatabaseTable.fetchRecipient(rowId: $0, tx: tx) } - let blockedGroupIds = try blockedGroupStore.blockedGroupIds(tx: tx) + let blockedGroupIds = blockedGroupStore.blockedGroupIds(tx: tx) let message = OWSBlockedPhoneNumbersMessage( localThread: localThread, diff --git a/SignalServiceKit/tests/Contacts/BlockingManagerTests.swift b/SignalServiceKit/tests/Contacts/BlockingManagerTests.swift index 322b212424..ba788005f1 100644 --- a/SignalServiceKit/tests/Contacts/BlockingManagerTests.swift +++ b/SignalServiceKit/tests/Contacts/BlockingManagerTests.swift @@ -195,7 +195,7 @@ class BlockingManagerTests: SSKBaseTest { SignalServiceAddress(newlyBlockedPhoneNumber), ] XCTAssertEqual(Set(otherBlockedAddresses), Set(expectedBlockedAddresses)) - let otherBlockedGroupIds = try otherBlockingManager.blockedGroupIds(transaction: readTx) + let otherBlockedGroupIds = otherBlockingManager.blockedGroupIds(transaction: readTx) let expectedBlockedGroupIds = [ try stillBlockedGroupParams.getPublicParams().getGroupIdentifier().serialize(), try newlyBlockedGroupParams.getPublicParams().getGroupIdentifier().serialize(),