From 7146a02821c71228bfb3c830ef87c8a3d69ce8ad Mon Sep 17 00:00:00 2001 From: Sasha Weiss <105739406+sashaweiss-signal@users.noreply.github.com> Date: Wed, 17 Aug 2022 13:18:31 -0700 Subject: [PATCH] Serialize group updates using OWSOperation --- SignalMessaging/groups/GroupsV2Impl.swift | 24 ++-- .../src/groups/GroupManager.swift | 118 +++++++++++++++--- SignalServiceKit/src/groups/GroupsV2.swift | 14 ++- 3 files changed, 130 insertions(+), 26 deletions(-) diff --git a/SignalMessaging/groups/GroupsV2Impl.swift b/SignalMessaging/groups/GroupsV2Impl.swift index c87cd0bf42..e0cf4cf7b1 100644 --- a/SignalMessaging/groups/GroupsV2Impl.swift +++ b/SignalMessaging/groups/GroupsV2Impl.swift @@ -830,16 +830,23 @@ public class GroupsV2Impl: NSObject, GroupsV2Swift, GroupsV2 { // MARK: - Generic Group Change - public func updateGroupV2(groupModel: TSGroupModelV2, - changesBlock: @escaping (GroupsV2OutgoingChanges) -> Void) -> Promise { + public func updateGroupV2( + groupId: Data, + groupSecretParamsData: Data, + changesBlock: @escaping (GroupsV2OutgoingChanges) -> Void + ) -> Promise { return firstly(on: .global()) { () throws -> GroupsV2OutgoingChanges in - let changes = GroupsV2OutgoingChangesImpl(groupId: groupModel.groupId, - groupSecretParamsData: groupModel.secretParamsData) + let changes = GroupsV2OutgoingChangesImpl( + groupId: groupId, + groupSecretParamsData: groupSecretParamsData + ) changesBlock(changes) return changes }.then(on: .global()) { (changes: GroupsV2OutgoingChanges) -> Promise in - return self.updateExistingGroupOnService(changes: changes, - requiredRevision: nil) + return self.updateExistingGroupOnService( + changes: changes, + requiredRevision: nil + ) } } @@ -1660,7 +1667,10 @@ public class GroupsV2Impl: NSObject, GroupsV2Swift, GroupsV2 { // We're already an invited member; try to join by accepting the invite. // That will make us a full member; requesting to join via // the invite link might make us a requesting member. - return self.updateGroupV2(groupModel: groupModelV2) { groupChangeSet in + return self.updateGroupV2( + groupId: groupModelV2.groupId, + groupSecretParamsData: groupModelV2.secretParamsData + ) { groupChangeSet in groupChangeSet.promoteInvitedMember(localUuid) } } else { diff --git a/SignalServiceKit/src/groups/GroupManager.swift b/SignalServiceKit/src/groups/GroupManager.swift index 812b44f258..0554f3cebd 100644 --- a/SignalServiceKit/src/groups/GroupManager.swift +++ b/SignalServiceKit/src/groups/GroupManager.swift @@ -1139,21 +1139,6 @@ public class GroupManager: NSObject { } } - // MARK: - Generic Group Change - - public static func updateGroupV2(groupModel: TSGroupModelV2, - description: String, - changesBlock: @escaping (GroupsV2OutgoingChanges) -> Void) -> Promise { - return firstly { - self.ensureLocalProfileHasCommitmentIfNecessary() - }.then(on: .global()) { () throws -> Promise in - self.groupsV2Swift.updateGroupV2(groupModel: groupModel, changesBlock: changesBlock) - }.timeout(seconds: GroupManager.groupUpdateTimeoutDuration, - description: description) { - GroupsV2Error.timeout - } - } - // MARK: - Removed from Group or Invite Revoked public static func handleNotInGroup(groupId: Data, @@ -2224,3 +2209,106 @@ extension GroupManager { } } } + +// MARK: - Serialized generic group changes + +extension GroupManager { + // Serialize group updates by group ID + private static var groupUpdateOperationQueues: [Data: OperationQueue] = [:] + + private static func operationQueue( + forUpdatingGroup groupModel: TSGroupModel + ) -> OperationQueue { + if let queue = groupUpdateOperationQueues[groupModel.groupId] { + return queue + } + + let newQueue = OperationQueue() + newQueue.name = "GroupManager.updateQueueForGroup.\(UUID().uuidString)" + newQueue.maxConcurrentOperationCount = 1 + + groupUpdateOperationQueues[groupModel.groupId] = newQueue + return newQueue + } + + private class GenericGroupUpdateOperation: OWSOperation { + private let groupId: Data + private let groupSecretParamsData: Data + private let updateDescription: String + private let changesBlock: (GroupsV2OutgoingChanges) -> Void + + let promise: Promise + private let future: Future + + init( + groupId: Data, + groupSecretParamsData: Data, + updateDescription: String, + changesBlock: @escaping (GroupsV2OutgoingChanges) -> Void + ) { + self.groupId = groupId + self.groupSecretParamsData = groupSecretParamsData + self.updateDescription = updateDescription + self.changesBlock = changesBlock + + let (promise, future) = Promise.pending() + self.promise = promise + self.future = future + + super.init() + + self.remainingRetries = 1 + } + + public override func run() { + firstly { + GroupManager.ensureLocalProfileHasCommitmentIfNecessary() + }.then(on: .global()) { () throws -> Promise in + self.groupsV2Swift.updateGroupV2( + groupId: self.groupId, + groupSecretParamsData: self.groupSecretParamsData, + changesBlock: self.changesBlock + ) + }.done(on: .global()) { groupThread in + self.reportSuccess() + self.future.resolve(groupThread) + }.timeout( + seconds: GroupManager.groupUpdateTimeoutDuration, + description: description + ) { + GroupsV2Error.timeout + }.catch(on: .global()) { error in + switch error { + case GroupsV2Error.redundantChange: + // From an operation perspective, this is a success! + self.reportSuccess() + self.future.reject(error) + default: + owsFailDebug("Group update failed: \(error)") + self.reportError(error) + } + } + } + + public override func didFail(error: Error) { + future.reject(error) + } + } + + public static func updateGroupV2( + groupModel: TSGroupModelV2, + description: String, + changesBlock: @escaping (GroupsV2OutgoingChanges) -> Void + ) -> Promise { + let operation = GenericGroupUpdateOperation( + groupId: groupModel.groupId, + groupSecretParamsData: groupModel.secretParamsData, + updateDescription: description, + changesBlock: changesBlock + ) + + operationQueue(forUpdatingGroup: groupModel).addOperation(operation) + + return operation.promise + } +} diff --git a/SignalServiceKit/src/groups/GroupsV2.swift b/SignalServiceKit/src/groups/GroupsV2.swift index e542fcb892..728766c20e 100644 --- a/SignalServiceKit/src/groups/GroupsV2.swift +++ b/SignalServiceKit/src/groups/GroupsV2.swift @@ -111,8 +111,11 @@ public protocol GroupsV2Swift: GroupsV2 { func updateExistingGroupOnService(changes: GroupsV2OutgoingChanges, requiredRevision: UInt32?) -> Promise - func updateGroupV2(groupModel: TSGroupModelV2, - changesBlock: @escaping (GroupsV2OutgoingChanges) -> Void) -> Promise + func updateGroupV2( + groupId: Data, + groupSecretParamsData: Data, + changesBlock: @escaping (GroupsV2OutgoingChanges) -> Void + ) -> Promise func reuploadLocalProfilePromise() -> Promise @@ -566,8 +569,11 @@ public class MockGroupsV2: NSObject, GroupsV2Swift, GroupsV2 { owsFail("Not implemented.") } - public func updateGroupV2(groupModel: TSGroupModelV2, - changesBlock: @escaping (GroupsV2OutgoingChanges) -> Void) -> Promise { + public func updateGroupV2( + groupId: Data, + groupSecretParamsData: Data, + changesBlock: @escaping (GroupsV2OutgoingChanges) -> Void + ) -> Promise { owsFail("Not implemented.") }