From 98151dfd0df5a7410be26a24e49f0ed449b2103e Mon Sep 17 00:00:00 2001 From: "Sasha Weiss (Signal)" <105739406+sashaweiss-signal@users.noreply.github.com> Date: Tue, 12 Jul 2022 10:23:12 -0700 Subject: [PATCH] Don't need transactions to build group models --- .../DebugUI/DebugUIGroupsV2.swift | 26 ++++---- .../DebugUI/DebugUIStress.swift | 21 +++---- .../ViewControllers/GroupInviteLinksUI.swift | 4 +- .../BaseGroupMemberViewController.swift | 4 +- .../NewGroupConfirmViewController.swift | 10 ++- .../AddGroupMembersViewController.swift | 6 +- .../AddToGroupViewController.swift | 4 +- .../groups/GroupV2UpdatesImpl.swift | 12 ++-- SignalMessaging/groups/GroupsV2Impl.swift | 8 +-- .../groups/GroupsV2IncomingChanges.swift | 5 +- .../groups/GroupsV2Migration.swift | 6 +- .../src/Messages/BlockingManager.swift | 4 +- .../Storage/Database/GRDBSchemaMigrator.swift | 2 +- .../src/groups/GroupManager.swift | 62 ++++++++----------- .../src/groups/TSGroupModelBuilder.swift | 16 +++-- .../Contacts/BlockingManagerStateTests.swift | 4 +- 16 files changed, 81 insertions(+), 113 deletions(-) diff --git a/Signal/src/ViewControllers/DebugUI/DebugUIGroupsV2.swift b/Signal/src/ViewControllers/DebugUI/DebugUIGroupsV2.swift index 1c83af2379..a413b3d55c 100644 --- a/Signal/src/ViewControllers/DebugUI/DebugUIGroupsV2.swift +++ b/Signal/src/ViewControllers/DebugUI/DebugUIGroupsV2.swift @@ -176,7 +176,7 @@ class DebugUIGroupsV2: DebugUIPage { var defaultMembershipBuilder = GroupMembership.Builder() defaultMembershipBuilder.addFullMember(localAddress, role: .administrator) defaultModelBuilder.groupMembership = defaultMembershipBuilder.build() - let defaultModel = try defaultModelBuilder.build(transaction: transaction) + let defaultModel = try defaultModelBuilder.build() let defaultDMToken = DisappearingMessageToken.disabledToken do { @@ -199,7 +199,7 @@ class DebugUIGroupsV2: DebugUIPage { var modelBuilder2 = model1.asBuilder modelBuilder2.groupV2Revision = 1 - let model2 = try modelBuilder2.build(transaction: transaction) + let model2 = try modelBuilder2.build() let dmToken2 = defaultDMToken GroupManager.insertGroupUpdateInfoMessage(groupThread: groupThread, @@ -253,7 +253,7 @@ class DebugUIGroupsV2: DebugUIPage { modelBuilder2.groupMembership = groupMembershipBuilder1.build() modelBuilder2.groupAccess = .adminOnly - let model2 = try modelBuilder2.build(transaction: transaction) + let model2 = try modelBuilder2.build() let dmToken2 = DisappearingMessageToken(isEnabled: true, durationSeconds: 30) GroupManager.insertGroupUpdateInfoMessage(groupThread: groupThread, @@ -319,15 +319,15 @@ class DebugUIGroupsV2: DebugUIPage { modelBuilder1.groupsVersion = groupsVersion modelBuilder1.groupAccess = .defaultForV2 modelBuilder1.groupMembership = defaultMembershipBuilder.build() - let model1 = try modelBuilder1.build(transaction: transaction) + let model1 = try modelBuilder1.build() var modelBuilder2 = model1.asBuilder modelBuilder2.groupAccess = .adminOnly - let model2 = try modelBuilder2.build(transaction: transaction) + let model2 = try modelBuilder2.build() var modelBuilder3 = model1.asBuilder modelBuilder3.groupAccess = .allAccess - let model3 = try modelBuilder3.build(transaction: transaction) + let model3 = try modelBuilder3.build() let dmToken = defaultDMToken @@ -376,7 +376,7 @@ class DebugUIGroupsV2: DebugUIPage { groupMembershipBuilder1.addFullMember(member, role: .normal) } modelBuilder1.groupMembership = groupMembershipBuilder1.build() - let model1 = try modelBuilder1.build(transaction: transaction) + let model1 = try modelBuilder1.build() var modelBuilder2 = defaultModel.asBuilder var groupMembershipBuilder2 = defaultModel.groupMembership.asBuilder @@ -385,7 +385,7 @@ class DebugUIGroupsV2: DebugUIPage { groupMembershipBuilder2.addFullMember(member, role: .administrator) } modelBuilder2.groupMembership = groupMembershipBuilder2.build() - let model2 = try modelBuilder2.build(transaction: transaction) + let model2 = try modelBuilder2.build() let dmToken = defaultDMToken @@ -421,7 +421,7 @@ class DebugUIGroupsV2: DebugUIPage { var modelBuilder1 = defaultModel.asBuilder modelBuilder1.groupMembership = GroupMembership() - let model1 = try modelBuilder1.build(transaction: transaction) + let model1 = try modelBuilder1.build() var modelBuilder2 = defaultModel.asBuilder var groupMembershipBuilder2 = defaultModel.groupMembership.asBuilder @@ -430,7 +430,7 @@ class DebugUIGroupsV2: DebugUIPage { groupMembershipBuilder2.addFullMember(member, role: .administrator) } modelBuilder2.groupMembership = groupMembershipBuilder2.build() - let model2 = try modelBuilder2.build(transaction: transaction) + let model2 = try modelBuilder2.build() let dmToken = defaultDMToken @@ -478,7 +478,7 @@ class DebugUIGroupsV2: DebugUIPage { // Model 1: Empty. var modelBuilder1 = defaultModel.asBuilder modelBuilder1.groupMembership = GroupMembership() - let model1 = try modelBuilder1.build(transaction: transaction) + let model1 = try modelBuilder1.build() // Model 2: Invited. var modelBuilder2 = defaultModel.asBuilder @@ -488,7 +488,7 @@ class DebugUIGroupsV2: DebugUIPage { groupMembershipBuilder2.addInvitedMember(member, role: .normal, addedByUuid: inviterUuid) } modelBuilder2.groupMembership = groupMembershipBuilder2.build() - let model2 = try modelBuilder2.build(transaction: transaction) + let model2 = try modelBuilder2.build() // Model 3: Active members. var modelBuilder3 = defaultModel.asBuilder @@ -498,7 +498,7 @@ class DebugUIGroupsV2: DebugUIPage { groupMembershipBuilder3.addFullMember(member, role: .administrator) } modelBuilder3.groupMembership = groupMembershipBuilder3.build() - let model3 = try modelBuilder3.build(transaction: transaction) + let model3 = try modelBuilder3.build() let dmToken = defaultDMToken diff --git a/Signal/src/ViewControllers/DebugUI/DebugUIStress.swift b/Signal/src/ViewControllers/DebugUI/DebugUIStress.swift index b23ea86835..9d49e0c495 100644 --- a/Signal/src/ViewControllers/DebugUI/DebugUIStress.swift +++ b/Signal/src/ViewControllers/DebugUI/DebugUIStress.swift @@ -87,14 +87,11 @@ public extension DebugUIStress { guard GroupManager.defaultGroupsVersion == .V2 else { throw OWSAssertionError("Groups v2 not enabled.") } - let members = try self.databaseStorage.read { (transaction: SDSAnyReadTransaction) throws -> [SignalServiceAddress] in - let members: [SignalServiceAddress] = oldGroupThread.groupModel.groupMembers.filter { address in - GroupManager.doesUserSupportGroupsV2(address: address, transaction: transaction) - } - guard GroupManager.canUseV2(for: Set(members), transaction: transaction) else { - throw OWSAssertionError("Error filtering users.") - } - return members + let members: [SignalServiceAddress] = oldGroupThread.groupModel.groupMembers.filter { address in + GroupManager.doesUserSupportGroupsV2(address: address) + } + guard GroupManager.canUseV2(for: Set(members)) else { + throw OWSAssertionError("Error filtering users.") } for member in members { Logger.verbose("Member: \(member)") @@ -150,13 +147,13 @@ public extension DebugUIStress { } return GroupManager.tryToEnableGroupsV2(for: Array(membersToAdd), isBlocking: true, ignoreErrors: true) }.then { () -> Promise in - let uuidsToAdd = try self.databaseStorage.read { transaction -> [UUID] in + let uuidsToAdd: [UUID] = try { let validMembersToAdd: [SignalServiceAddress] if dstGroupThread.isGroupV1Thread { validMembersToAdd = membersToAdd.filter { $0.phoneNumber != nil } } else { validMembersToAdd = membersToAdd.filter { address in - GroupManager.doesUserSupportGroupsV2(address: address, transaction: transaction) + GroupManager.doesUserSupportGroupsV2(address: address) } } @@ -168,8 +165,8 @@ public extension DebugUIStress { throw OWSAssertionError("No valid members to add.") } - return membersToAdd.compactMap { $0.uuid } - } + return validMembersToAdd.compactMap { $0.uuid } + }() return GroupManager.updateExistingGroup(existingGroupModel: dstGroupThread.groupModel, update: .addNormalMembers(uuids: uuidsToAdd)) diff --git a/Signal/src/ViewControllers/GroupInviteLinksUI.swift b/Signal/src/ViewControllers/GroupInviteLinksUI.swift index 84de95fa72..9258b060e6 100644 --- a/Signal/src/ViewControllers/GroupInviteLinksUI.swift +++ b/Signal/src/ViewControllers/GroupInviteLinksUI.swift @@ -432,8 +432,6 @@ private class GroupInviteLinksActionSheet: ActionSheetController { owsFailDebug("missing local address") return false } - return databaseStorage.read { transaction in - GroupManager.doesUserSupportGroupsV2(address: localAddress, transaction: transaction) - } + return GroupManager.doesUserSupportGroupsV2(address: localAddress) } } diff --git a/Signal/src/ViewControllers/NewGroupView/BaseGroupMemberViewController.swift b/Signal/src/ViewControllers/NewGroupView/BaseGroupMemberViewController.swift index 1c25b323e2..2f97260028 100644 --- a/Signal/src/ViewControllers/NewGroupView/BaseGroupMemberViewController.swift +++ b/Signal/src/ViewControllers/NewGroupView/BaseGroupMemberViewController.swift @@ -204,9 +204,7 @@ extension BaseGroupMemberViewController: MemberViewDelegate { } private func doesRecipientSupportGroupsV2(_ address: SignalServiceAddress) -> Bool { - return databaseStorage.read { transaction in - return GroupManager.doesUserSupportGroupsV2(address: address, transaction: transaction) - } + return GroupManager.doesUserSupportGroupsV2(address: address) } func tryToEnableGroupsV2ForAddress(_ address: SignalServiceAddress, diff --git a/Signal/src/ViewControllers/NewGroupView/NewGroupConfirmViewController.swift b/Signal/src/ViewControllers/NewGroupView/NewGroupConfirmViewController.swift index 50e995c423..16bda21fb1 100644 --- a/Signal/src/ViewControllers/NewGroupView/NewGroupConfirmViewController.swift +++ b/Signal/src/ViewControllers/NewGroupView/NewGroupConfirmViewController.swift @@ -81,13 +81,11 @@ public class NewGroupConfirmViewController: OWSTableViewController2 { } private var membersDoNotSupportGroupsV2: [PickedRecipient] { - return databaseStorage.read { transaction in - self.recipientSet.orderedMembers.filter { - guard let address = $0.address else { - return false - } - return !GroupManager.doesUserSupportGroupsV2(address: address, transaction: transaction) + recipientSet.orderedMembers.filter { + guard let address = $0.address else { + return false } + return !GroupManager.doesUserSupportGroupsV2(address: address) } } diff --git a/Signal/src/ViewControllers/ThreadSettings/AddGroupMembersViewController.swift b/Signal/src/ViewControllers/ThreadSettings/AddGroupMembersViewController.swift index deee0742a5..2fe1b1fdf7 100644 --- a/Signal/src/ViewControllers/ThreadSettings/AddGroupMembersViewController.swift +++ b/Signal/src/ViewControllers/ThreadSettings/AddGroupMembersViewController.swift @@ -188,9 +188,7 @@ extension AddGroupMembersViewController: GroupMemberViewDelegate { owsFailDebug("Invalid recipient.") return false } - return databaseStorage.read { transaction in - return GroupManager.doesUserSupportGroupsV2(address: address, transaction: transaction) - } + return GroupManager.doesUserSupportGroupsV2(address: address) } func groupMemberViewShouldShowMemberCount() -> Bool { @@ -225,7 +223,7 @@ extension AddGroupMembersViewController: GroupMemberViewDelegate { // We can "add" pending or requesting members if they support gv2 // and we know their profile key credential. let canAddMember: Bool = { - guard GroupManager.doesUserSupportGroupsV2(address: address, transaction: transaction) else { + guard GroupManager.doesUserSupportGroupsV2(address: address) else { return false } return self.groupsV2.hasProfileKeyCredential(for: address, transaction: transaction) diff --git a/Signal/src/ViewControllers/ThreadSettings/AddToGroupViewController.swift b/Signal/src/ViewControllers/ThreadSettings/AddToGroupViewController.swift index 84a48430e7..9909544b2a 100644 --- a/Signal/src/ViewControllers/ThreadSettings/AddToGroupViewController.swift +++ b/Signal/src/ViewControllers/ThreadSettings/AddToGroupViewController.swift @@ -178,9 +178,7 @@ public class AddToGroupViewController: OWSTableViewController2 { addToGroupStep2(groupThread, shortName: shortName) return } - let doesUserSupportGroupsV2 = databaseStorage.read { transaction in - GroupManager.doesUserSupportGroupsV2(address: self.address, transaction: transaction) - } + let doesUserSupportGroupsV2 = GroupManager.doesUserSupportGroupsV2(address: self.address) guard doesUserSupportGroupsV2 else { GroupViewUtils.showInvalidGroupMemberAlert(fromViewController: self) return diff --git a/SignalMessaging/groups/GroupV2UpdatesImpl.swift b/SignalMessaging/groups/GroupV2UpdatesImpl.swift index b32562928a..13fb42993a 100644 --- a/SignalMessaging/groups/GroupV2UpdatesImpl.swift +++ b/SignalMessaging/groups/GroupV2UpdatesImpl.swift @@ -456,8 +456,7 @@ public class GroupV2UpdatesImpl: NSObject, GroupV2UpdatesSwift, GroupV2Updates { let changedGroupModel = try GroupsV2IncomingChanges.applyChangesToGroupModel(groupThread: groupThread, changeActionsProto: changeActionsProto, downloadedAvatars: downloadedAvatars, - groupModelOptions: [], - transaction: transaction) + groupModelOptions: []) guard changedGroupModel.newGroupModel.revision > changedGroupModel.oldGroupModel.revision else { throw OWSAssertionError("Invalid groupV2Revision: \(changedGroupModel.newGroupModel.revision).") } @@ -760,7 +759,7 @@ public class GroupV2UpdatesImpl: NSObject, GroupV2UpdatesSwift, GroupV2Updates { builder.wasJustCreatedByLocalUser = true } - let newGroupModel = try builder.build(transaction: transaction) + let newGroupModel = try builder.build() let newDisappearingMessageToken = snapshot.disappearingMessageToken let didAddLocalUserToV2Group = self.didAddLocalUserToV2Group(groupChange: firstGroupChange, @@ -846,7 +845,7 @@ public class GroupV2UpdatesImpl: NSObject, GroupV2UpdatesSwift, GroupV2Updates { var builder = try TSGroupModelBuilder.builderForSnapshot(groupV2Snapshot: snapshot, transaction: transaction) builder.apply(options: groupModelOptions) - newGroupModel = try builder.build(transaction: transaction) + newGroupModel = try builder.build() newDisappearingMessageToken = snapshot.disappearingMessageToken newProfileKeys = snapshot.profileKeys } else if let changeActionsProto = groupChange.changeActionsProto { @@ -854,8 +853,7 @@ public class GroupV2UpdatesImpl: NSObject, GroupV2UpdatesSwift, GroupV2Updates { groupThread: groupThread, changeActionsProto: changeActionsProto, downloadedAvatars: groupChange.downloadedAvatars, - groupModelOptions: groupModelOptions, - transaction: transaction) + groupModelOptions: groupModelOptions) newGroupModel = changedGroupModel.newGroupModel newDisappearingMessageToken = changedGroupModel.newDisappearingMessageToken newProfileKeys = changedGroupModel.profileKeys @@ -1025,7 +1023,7 @@ public class GroupV2UpdatesImpl: NSObject, GroupV2UpdatesSwift, GroupV2Updates { } } - let newGroupModel = try builder.buildAsV2(transaction: transaction) + let newGroupModel = try builder.buildAsV2() let newDisappearingMessageToken = groupV2Snapshot.disappearingMessageToken // groupUpdateSourceAddress is nil because we don't know the // author(s) of changes reflected in the snapshot. diff --git a/SignalMessaging/groups/GroupsV2Impl.swift b/SignalMessaging/groups/GroupsV2Impl.swift index addc592e64..a97f85483f 100644 --- a/SignalMessaging/groups/GroupsV2Impl.swift +++ b/SignalMessaging/groups/GroupsV2Impl.swift @@ -1809,7 +1809,7 @@ public class GroupsV2Impl: NSObject, GroupsV2Swift, GroupsV2 { membershipBuilder.remove(localUuid) membershipBuilder.addRequestingMember(localUuid) builder.groupMembership = membershipBuilder.build() - let newGroupModel = try builder.build(transaction: transaction) + let newGroupModel = try builder.build() groupThread.update(with: newGroupModel, transaction: transaction) @@ -1853,7 +1853,7 @@ public class GroupsV2Impl: NSObject, GroupsV2Swift, GroupsV2 { membershipBuilder.addRequestingMember(localUuid) builder.groupMembership = membershipBuilder.build() - let groupModel = try builder.build(transaction: transaction) + let groupModel = try builder.build() let groupThread = TSGroupThread(groupModelPrivate: groupModel, transaction: transaction) groupThread.anyInsert(transaction: transaction) @@ -2002,7 +2002,7 @@ public class GroupsV2Impl: NSObject, GroupsV2Swift, GroupsV2 { var membershipBuilder = oldGroupMembership.asBuilder membershipBuilder.remove(localUuid) builder.groupMembership = membershipBuilder.build() - let newGroupModel = try builder.build(transaction: transaction) + let newGroupModel = try builder.build() groupThread.update(with: newGroupModel, transaction: transaction) @@ -2144,7 +2144,7 @@ public class GroupsV2Impl: NSObject, GroupsV2Swift, GroupsV2 { membershipBuilder.addRequestingMember(localUuid) } builder.groupMembership = membershipBuilder.build() - let newGroupModel = try builder.build(transaction: transaction) + let newGroupModel = try builder.build() groupThread.update(with: newGroupModel, transaction: transaction) diff --git a/SignalMessaging/groups/GroupsV2IncomingChanges.swift b/SignalMessaging/groups/GroupsV2IncomingChanges.swift index 74ba28a44c..7a8204116f 100644 --- a/SignalMessaging/groups/GroupsV2IncomingChanges.swift +++ b/SignalMessaging/groups/GroupsV2IncomingChanges.swift @@ -50,8 +50,7 @@ public class GroupsV2IncomingChanges: Dependencies { class func applyChangesToGroupModel(groupThread: TSGroupThread, changeActionsProto: GroupsProtoGroupChangeActions, downloadedAvatars: GroupV2DownloadedAvatars, - groupModelOptions: TSGroupModelOptions, - transaction: SDSAnyReadTransaction) throws -> ChangedGroupModel { + groupModelOptions: TSGroupModelOptions) throws -> ChangedGroupModel { guard let oldGroupModel = groupThread.groupModel as? TSGroupModelV2 else { throw OWSAssertionError("Invalid group model.") } @@ -596,7 +595,7 @@ public class GroupsV2IncomingChanges: Dependencies { builder.apply(options: groupModelOptions) - let newGroupModel = try builder.buildAsV2(transaction: transaction) + let newGroupModel = try builder.buildAsV2() return ChangedGroupModel(oldGroupModel: oldGroupModel, newGroupModel: newGroupModel, diff --git a/SignalMessaging/groups/GroupsV2Migration.swift b/SignalMessaging/groups/GroupsV2Migration.swift index 66c44ca9f2..e552fb9889 100644 --- a/SignalMessaging/groups/GroupsV2Migration.swift +++ b/SignalMessaging/groups/GroupsV2Migration.swift @@ -486,7 +486,7 @@ fileprivate extension GroupsV2Migration { try self.databaseStorage.write { transaction in let builder = try TSGroupModelBuilder.builderForSnapshot(groupV2Snapshot: groupV2Snapshot, transaction: transaction) - return try builder.buildAsV2(transaction: transaction) + return try builder.buildAsV2() } }.then(on: .global()) { (newGroupModelV2: TSGroupModelV2) throws -> Promise in let newDisappearingMessageToken = groupV2Snapshot.disappearingMessageToken @@ -673,7 +673,7 @@ fileprivate extension GroupsV2Migration { groupModelBuilder.addedByAddress = nil - return try groupModelBuilder.buildAsV2(transaction: transaction) + return try groupModelBuilder.buildAsV2() } static func migrateGroupOnService(proposedGroupModel: TSGroupModelV2, @@ -687,7 +687,7 @@ fileprivate extension GroupsV2Migration { let createdGroupModel = try self.databaseStorage.write { (transaction) throws -> TSGroupModelV2 in let builder = try TSGroupModelBuilder.builderForSnapshot(groupV2Snapshot: groupV2Snapshot, transaction: transaction) - return try builder.buildAsV2(transaction: transaction) + return try builder.buildAsV2() } if proposedGroupModel != createdGroupModel { Logger.verbose("proposedGroupModel: \(proposedGroupModel.debugDescription)") diff --git a/SignalServiceKit/src/Messages/BlockingManager.swift b/SignalServiceKit/src/Messages/BlockingManager.swift index c7f415b91f..139460eb74 100644 --- a/SignalServiceKit/src/Messages/BlockingManager.swift +++ b/SignalServiceKit/src/Messages/BlockingManager.swift @@ -296,7 +296,7 @@ extension BlockingManager { } else if let currentThread = TSGroupThread.fetch(groupId: groupId, transaction: transaction) { return currentThread.groupModel } else { - return GroupManager.fakeGroupModel(groupId: groupId, transaction: transaction) + return GroupManager.fakeGroupModel(groupId: groupId) } } @@ -345,7 +345,7 @@ extension BlockingManager { } else if let currentThread = TSGroupThread.fetch(groupId: blockedGroupId, transaction: transaction) { return currentThread.groupModel } else { - return GroupManager.fakeGroupModel(groupId: blockedGroupId, transaction: transaction) + return GroupManager.fakeGroupModel(groupId: blockedGroupId) } }.compactMapValues { $0 } diff --git a/SignalServiceKit/src/Storage/Database/GRDBSchemaMigrator.swift b/SignalServiceKit/src/Storage/Database/GRDBSchemaMigrator.swift index ac43623c3f..4211791d14 100644 --- a/SignalServiceKit/src/Storage/Database/GRDBSchemaMigrator.swift +++ b/SignalServiceKit/src/Storage/Database/GRDBSchemaMigrator.swift @@ -1985,7 +1985,7 @@ public class GRDBSchemaMigrator: NSObject { builder.avatarUrlPath = nil do { - let newGroupModel = try builder.build(transaction: transaction.asAnyWrite) + let newGroupModel = try builder.build() groupThread.update(with: newGroupModel, transaction: transaction.asAnyWrite) } catch { owsFail("Failed to remove invalid group avatar during migration: \(error)") diff --git a/SignalServiceKit/src/groups/GroupManager.swift b/SignalServiceKit/src/groups/GroupManager.swift index 7de8ea2291..84372551a2 100644 --- a/SignalServiceKit/src/groups/GroupManager.swift +++ b/SignalServiceKit/src/groups/GroupManager.swift @@ -138,8 +138,7 @@ public class GroupManager: NSObject { // MARK: - Group Models @objc - public static func fakeGroupModel(groupId: Data, - transaction: SDSAnyReadTransaction) -> TSGroupModel? { + public static func fakeGroupModel(groupId: Data) -> TSGroupModel? { do { var builder = TSGroupModelBuilder() builder.groupId = groupId @@ -152,18 +151,17 @@ public class GroupManager: NSObject { throw OWSAssertionError("Invalid group id: \(groupId).") } - return try builder.build(transaction: transaction) + return try builder.build() } catch { owsFailDebug("Error: \(error)") return nil } } - public static func canUseV2(for members: Set, - transaction: SDSAnyReadTransaction) -> Bool { + public static func canUseV2(for members: Set) -> Bool { for recipientAddress in members { - guard doesUserSupportGroupsV2(address: recipientAddress, transaction: transaction) else { + guard doesUserSupportGroupsV2(address: recipientAddress) else { Logger.warn("Creating legacy group; member missing UUID.") return false } @@ -173,8 +171,7 @@ public class GroupManager: NSObject { return true } - public static func doesUserSupportGroupsV2(address: SignalServiceAddress, - transaction: SDSAnyReadTransaction) -> Bool { + public static func doesUserSupportGroupsV2(address: SignalServiceAddress) -> Bool { guard address.isValid else { Logger.warn("Invalid address: \(address).") @@ -293,7 +290,7 @@ public class GroupManager: NSObject { builder.groupMembership = groupMembership builder.groupAccess = groupAccess builder.newGroupSeed = newGroupSeed - return try builder.build(transaction: transaction) + return try builder.build() } return groupModel }.then(on: DispatchQueue.global()) { (proposedGroupModel: TSGroupModel) -> Promise in @@ -311,11 +308,9 @@ public class GroupManager: NSObject { groupSecretParamsData: proposedGroupModelV2.secretParamsData) }.map(on: DispatchQueue.global()) { (avatarUrlPath: String) -> TSGroupModel in // Fill in the avatarUrl on the group model. - return try self.databaseStorage.read { transaction in - var builder = proposedGroupModel.asBuilder - builder.avatarUrlPath = avatarUrlPath - return try builder.build(transaction: transaction) - } + var builder = proposedGroupModel.asBuilder + builder.avatarUrlPath = avatarUrlPath + return try builder.build() } }.then(on: .global()) { (proposedGroupModel: TSGroupModel) -> Promise in guard let proposedGroupModelV2 = proposedGroupModel as? TSGroupModelV2 else { @@ -332,7 +327,7 @@ public class GroupManager: NSObject { var builder = try TSGroupModelBuilder.builderForSnapshot(groupV2Snapshot: groupV2Snapshot, transaction: transaction) builder.wasJustCreatedByLocalUser = true - return try builder.build(transaction: transaction) + return try builder.build() } if proposedGroupModel != createdGroupModel { Logger.verbose("proposedGroupModel: \(proposedGroupModel.debugDescription)") @@ -393,7 +388,7 @@ public class GroupManager: NSObject { let localAddress = SignalServiceAddress(uuid: localUuid) var builder = GroupMembership.Builder() - guard canUseV2(for: newGroupMembership.allMembersOfAnyKind, transaction: transaction) else { + guard canUseV2(for: newGroupMembership.allMembersOfAnyKind) else { // We should never be in this position anymore, since V1 groups are // dead, but we do this just in case until we revisit the GV1 code // more holistically. @@ -408,7 +403,7 @@ public class GroupManager: NSObject { // We only need to separate new members. for address in newMembers { - guard doesUserSupportGroupsV2(address: address, transaction: transaction) else { + guard doesUserSupportGroupsV2(address: address) else { // Members of v2 groups must support groups v2. // This should never happen. We prevent this // when creating new groups above by checking @@ -574,7 +569,7 @@ public class GroupManager: NSObject { builder.groupMembership = groupMembership builder.groupAccess = groupAccess builder.groupsVersion = groupsVersion - let groupModel = try builder.build(transaction: transaction) + let groupModel = try builder.build() // Just create it in the database, don't create it on the service. return try remoteUpsertExistingGroup(groupModel: groupModel, @@ -613,7 +608,7 @@ public class GroupManager: NSObject { builder.avatarUrlPath = nil builder.groupMembership = groupMembership builder.groupsVersion = .V1 - let groupModel = try builder.build(transaction: transaction) + let groupModel = try builder.build() return try remoteUpsertExistingGroup(groupModel: groupModel, disappearingMessageToken: disappearingMessageToken, @@ -675,7 +670,7 @@ public class GroupManager: NSObject { var builder = oldGroupModel.asBuilder builder.avatarData = avatarData builder.avatarUrlPath = nil - let newGroupModel = try builder.build(transaction: transaction) + let newGroupModel = try builder.build() return try remoteUpdateToExistingGroupV1(groupModel: newGroupModel, disappearingMessageToken: nil, groupUpdateSourceAddress: groupUpdateSourceAddress, @@ -740,7 +735,7 @@ public class GroupManager: NSObject { var groupModelBuilder = proposedGroupModel.asBuilder groupModelBuilder.groupMembership = groupMembership - let newGroupModel = try groupModelBuilder.build(transaction: transaction) + let newGroupModel = try groupModelBuilder.build() if currentGroupModel.isEqual(to: newGroupModel, comparisonMode: .compareAll) { // Skip redundant update. @@ -926,7 +921,7 @@ public class GroupManager: NSObject { var builder = oldGroupModel.asBuilder builder.groupMembership = newGroupMembership - var newGroupModel = try builder.build(transaction: transaction) + var newGroupModel = try builder.build() // We're leaving, so clear out who added us. If we're re-added it may change. if newGroupModel.groupsVersion == .V1 { @@ -1211,7 +1206,7 @@ public class GroupManager: NSObject { var groupModelBuilder = groupThread.groupModel.asBuilder do { groupModelBuilder.groupMembership = groupMembershipBuilder.build() - let newGroupModel = try groupModelBuilder.build(transaction: transaction) + let newGroupModel = try groupModelBuilder.build() // groupUpdateSourceAddress is nil because we don't (and can't) know who // removed us or revoked our invite. @@ -1319,8 +1314,7 @@ public class GroupManager: NSObject { messageBuilder.changeActionsProtoData = changeActionsProtoData if singleRecipient == nil { self.addAdditionalRecipients(to: messageBuilder, - groupThread: thread, - transaction: transaction) + groupThread: thread) } } return messageBuilder.build(transaction: transaction) @@ -1390,8 +1384,7 @@ public class GroupManager: NSObject { messageBuilder.groupMetaMessage = .new messageBuilder.expiresInSeconds = expiresInSeconds self.addAdditionalRecipients(to: messageBuilder, - groupThread: thread, - transaction: transaction) + groupThread: thread) let message = messageBuilder.build(transaction: transaction) self.messageSenderJobQueue.add(message: message.asPreparer, transaction: transaction) @@ -1409,8 +1402,7 @@ public class GroupManager: NSObject { } private static func addAdditionalRecipients(to messageBuilder: TSOutgoingMessageBuilder, - groupThread: TSGroupThread, - transaction: SDSAnyReadTransaction) { + groupThread: TSGroupThread) { guard groupThread.groupModel.groupsVersion == .V2 else { // No need to add "additional recipients" to v1 groups. return @@ -1420,8 +1412,7 @@ public class GroupManager: NSObject { assert(messageBuilder.additionalRecipients == nil) let groupMembership = groupThread.groupModel.groupMembership let additionalRecipients = groupMembership.invitedMembers.filter { address in - return doesUserSupportGroupsV2(address: address, - transaction: transaction) + return doesUserSupportGroupsV2(address: address) } messageBuilder.additionalRecipients = Array(additionalRecipients) } @@ -1472,8 +1463,7 @@ public class GroupManager: NSObject { assert(messageBuilder.additionalRecipients == nil) let groupMembership = groupThread.groupModel.groupMembership let additionalRecipients = groupMembership.invitedOrRequestMembers.filter { address in - return doesUserSupportGroupsV2(address: address, - transaction: transaction) + return doesUserSupportGroupsV2(address: address) } messageBuilder.additionalRecipients = Array(additionalRecipients) let message = messageBuilder.build(transaction: transaction) @@ -1616,7 +1606,7 @@ public class GroupManager: NSObject { // Set droppedMembers on the model. groupModelBuilder.droppedMembers = Array(droppedMembers) } - let newGroupModelV2 = try groupModelBuilder.buildAsV2(transaction: transaction) + let newGroupModelV2 = try groupModelBuilder.buildAsV2() guard newGroupModelV2.groupsVersion == .V2 else { throw OWSAssertionError("Invalid v2 group model.") @@ -2389,9 +2379,7 @@ extension GroupManager { newGroupModelBuilder.groupMembership = newGroupMembershipBuilder.build() - let newGroupModel = try databaseStorage.read { transaction in - try newGroupModelBuilder.build(transaction: transaction) - } + let newGroupModel = try newGroupModelBuilder.build() return .value(newGroupModel) }.then(on: .global()) { (newGroupModel: TSGroupModel) -> Promise in diff --git a/SignalServiceKit/src/groups/TSGroupModelBuilder.swift b/SignalServiceKit/src/groups/TSGroupModelBuilder.swift index 08fb077ab4..9cd90b8a9c 100644 --- a/SignalServiceKit/src/groups/TSGroupModelBuilder.swift +++ b/SignalServiceKit/src/groups/TSGroupModelBuilder.swift @@ -61,7 +61,7 @@ public struct TSGroupModelBuilder: Dependencies { } TSGroupThread.ensureGroupIdMapping(forGroupId: groupId, transaction: transaction) guard let oldGroupThread = TSGroupThread.fetch(groupId: groupId, transaction: transaction) else { - // Group not uet in db. + // Group not yet in db. return builder } let oldGroupModel = oldGroupThread.groupModel @@ -108,13 +108,12 @@ public struct TSGroupModelBuilder: Dependencies { groupSecretParamsData: groupSecretParamsData) } - public func build(transaction: SDSAnyReadTransaction) throws -> TSGroupModel { + public func build() throws -> TSGroupModel { try checkUsers() let allUsers = groupMembership.allMembersOfAnyKind - let groupsVersion = buildGroupsVersion(for: allUsers, - transaction: transaction) + let groupsVersion = buildGroupsVersion(for: allUsers) let newGroupSeed = self.newGroupSeed ?? NewGroupSeed() @@ -204,8 +203,8 @@ public struct TSGroupModelBuilder: Dependencies { } } - public func buildAsV2(transaction: SDSAnyReadTransaction) throws -> TSGroupModelV2 { - guard let model = try build(transaction: transaction) as? TSGroupModelV2 else { + public func buildAsV2() throws -> TSGroupModelV2 { + guard let model = try build() as? TSGroupModelV2 else { throw OWSAssertionError("Invalid group model.") } return model @@ -252,8 +251,7 @@ public struct TSGroupModelBuilder: Dependencies { } } - private func buildGroupsVersion(for members: Set, - transaction: SDSAnyReadTransaction) -> GroupsVersion { + private func buildGroupsVersion(for members: Set) -> GroupsVersion { if let value = groupsVersion { return value } @@ -262,7 +260,7 @@ public struct TSGroupModelBuilder: Dependencies { Logger.info("Creating v1 group due to debug flag.") return .V1 } - let canUseV2 = GroupManager.canUseV2(for: members, transaction: transaction) + let canUseV2 = GroupManager.canUseV2(for: members) if canUseV2 { Logger.info("Creating v2 group.") return GroupManager.defaultGroupsVersion diff --git a/SignalServiceKit/tests/Contacts/BlockingManagerStateTests.swift b/SignalServiceKit/tests/Contacts/BlockingManagerStateTests.swift index f0c0e28a2d..4aa6ff2e6b 100644 --- a/SignalServiceKit/tests/Contacts/BlockingManagerStateTests.swift +++ b/SignalServiceKit/tests/Contacts/BlockingManagerStateTests.swift @@ -354,8 +354,6 @@ class BlockingManagerStateTests: SSKBaseTestSwift { } func generateRandomGroupModel() -> TSGroupModel { - databaseStorage.read { - GroupManager.fakeGroupModel(groupId: TSGroupModel.generateRandomV1GroupId(), transaction: $0)! - } + GroupManager.fakeGroupModel(groupId: TSGroupModel.generateRandomV1GroupId())! } }