From 2e38fa145ce5c235c8bf6ff93588cc862a730688 Mon Sep 17 00:00:00 2001 From: Michael Kirk Date: Wed, 25 Jul 2018 19:56:28 -0600 Subject: [PATCH 1/2] Unbatch legacy contact requests // FREEBIE --- .../src/Contacts/ContactsUpdater.m | 4 +- .../OWSContactDiscoveryOperation.swift | 53 +++++++------------ 2 files changed, 21 insertions(+), 36 deletions(-) diff --git a/SignalServiceKit/src/Contacts/ContactsUpdater.m b/SignalServiceKit/src/Contacts/ContactsUpdater.m index 8ac744224f..5e638e94e0 100644 --- a/SignalServiceKit/src/Contacts/ContactsUpdater.m +++ b/SignalServiceKit/src/Contacts/ContactsUpdater.m @@ -82,8 +82,8 @@ NS_ASSUME_NONNULL_BEGIN failure:(void (^)(NSError *error))failure { dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{ - OWSContactDiscoveryOperation *operation = - [[OWSContactDiscoveryOperation alloc] initWithRecipientIdsToLookup:recipientIdsToLookup.allObjects]; + OWSLegacyContactDiscoveryOperation *operation = + [[OWSLegacyContactDiscoveryOperation alloc] initWithRecipientIdsToLookup:recipientIdsToLookup.allObjects]; NSArray *operationAndDependencies = [operation.dependencies arrayByAddingObject:operation]; [self.contactIntersectionQueue addOperations:operationAndDependencies waitUntilFinished:YES]; diff --git a/SignalServiceKit/src/Contacts/OWSContactDiscoveryOperation.swift b/SignalServiceKit/src/Contacts/OWSContactDiscoveryOperation.swift index bbbfb08ce2..e7399cb1e1 100644 --- a/SignalServiceKit/src/Contacts/OWSContactDiscoveryOperation.swift +++ b/SignalServiceKit/src/Contacts/OWSContactDiscoveryOperation.swift @@ -4,10 +4,17 @@ import Foundation -@objc(OWSContactDiscoveryOperation) -class ContactDiscoveryOperation: OWSOperation, LegacyContactDiscoveryBatchOperationDelegate { +@objc(OWSCDSOperation) +class CDSOperation: OWSOperation { let batchSize = 2048 + static let operationQueue: OperationQueue = { + let queue = OperationQueue() + queue.maxConcurrentOperationCount = 5 + + return queue + }() + let recipientIdsToLookup: [String] @objc @@ -22,8 +29,7 @@ class ContactDiscoveryOperation: OWSOperation, LegacyContactDiscoveryBatchOperat Logger.debug("\(logTag) in \(#function) with recipientIdsToLookup: \(recipientIdsToLookup.count)") for batchIds in recipientIdsToLookup.chunked(by: batchSize) { - let batchOperation = LegacyContactDiscoveryBatchOperation(recipientIdsToLookup: batchIds) - batchOperation.delegate = self + let batchOperation = CDSBatchOperation(recipientIdsToLookup: batchIds) self.addDependency(batchOperation) } } @@ -35,7 +41,7 @@ class ContactDiscoveryOperation: OWSOperation, LegacyContactDiscoveryBatchOperat Logger.debug("\(logTag) in \(#function)") for dependency in self.dependencies { - guard let batchOperation = dependency as? LegacyContactDiscoveryBatchOperation else { + guard let batchOperation = dependency as? CDSBatchOperation else { owsFail("\(self.logTag) in \(#function) unexpected dependency: \(dependency)") continue } @@ -46,23 +52,13 @@ class ContactDiscoveryOperation: OWSOperation, LegacyContactDiscoveryBatchOperat self.reportSuccess() } - // MARK: LegacyContactDiscoveryBatchOperationDelegate - func contactDiscoverBatchOperation(_ contactDiscoverBatchOperation: LegacyContactDiscoveryBatchOperation, didFailWithError error: Error) { - Logger.debug("\(logTag) in \(#function) canceling self and all dependencies.") - - self.dependencies.forEach { $0.cancel() } - self.cancel() - } -} - -protocol LegacyContactDiscoveryBatchOperationDelegate: class { - func contactDiscoverBatchOperation(_ contactDiscoverBatchOperation: LegacyContactDiscoveryBatchOperation, didFailWithError error: Error) } +@objc(OWSLegacyContactDiscoveryOperation) class LegacyContactDiscoveryBatchOperation: OWSOperation { + @objc var registeredRecipientIds: Set - weak var delegate: LegacyContactDiscoveryBatchOperationDelegate? private let recipientIdsToLookup: [String] private var networkManager: TSNetworkManager { @@ -71,6 +67,7 @@ class LegacyContactDiscoveryBatchOperation: OWSOperation { // MARK: Initializers + @objc required init(recipientIdsToLookup: [String]) { self.recipientIdsToLookup = recipientIdsToLookup self.registeredRecipientIds = Set() @@ -134,16 +131,12 @@ class LegacyContactDiscoveryBatchOperation: OWSOperation { // Called at most one time. override func didSucceed() { // Compare against new CDS service - let newCDSBatchOperation = CDSBatchOperation(recipientIdsToLookup: self.recipientIdsToLookup) + let modernCDSOperation = CDSOperation(recipientIdsToLookup: self.recipientIdsToLookup) let cdsFeedbackOperation = CDSFeedbackOperation(legacyRegisteredRecipientIds: self.registeredRecipientIds) - cdsFeedbackOperation.addDependency(newCDSBatchOperation) + cdsFeedbackOperation.addDependency(modernCDSOperation) - CDSBatchOperation.operationQueue.addOperations([newCDSBatchOperation, cdsFeedbackOperation], waitUntilFinished: false) - } - - // Called at most one time. - override func didFail(error: Error) { - self.delegate?.contactDiscoverBatchOperation(self, didFailWithError: error) + let operations = modernCDSOperation.dependencies + [modernCDSOperation, cdsFeedbackOperation] + CDSOperation.operationQueue.addOperations(operations, waitUntilFinished: false) } // MARK: Private Helpers @@ -204,13 +197,6 @@ class CDSBatchOperation: OWSOperation { private let recipientIdsToLookup: [String] private(set) var registeredRecipientIds: Set - static let operationQueue: OperationQueue = { - let queue = OperationQueue() - queue.maxConcurrentOperationCount = 1 - - return queue - }() - private var networkManager: TSNetworkManager { return TSNetworkManager.shared() } @@ -411,7 +397,6 @@ class CDSBatchOperation: OWSOperation { additionalAuthenticatedData: nil, authTag: authTag, key: remoteAttestation.keys.serverKey) else { - throw ContactDiscoveryError.parseError(description: "decryption failed") } @@ -462,7 +447,7 @@ class CDSFeedbackOperation: OWSOperation { return } - guard let cdsOperation = dependencies.first as? CDSBatchOperation else { + guard let cdsOperation = dependencies.first as? CDSOperation else { let error = OWSErrorMakeAssertionError("\(self.logTag) in \(#function) cdsOperation was unexpectedly nil") self.reportError(error) return From 6d46ed0e3f788c98085779f2342b710b2b5a7c2a Mon Sep 17 00:00:00 2001 From: Michael Kirk Date: Wed, 25 Jul 2018 20:22:13 -0600 Subject: [PATCH 2/2] No change in behavior: move class down --- .../OWSContactDiscoveryOperation.swift | 100 +++++++++--------- 1 file changed, 50 insertions(+), 50 deletions(-) diff --git a/SignalServiceKit/src/Contacts/OWSContactDiscoveryOperation.swift b/SignalServiceKit/src/Contacts/OWSContactDiscoveryOperation.swift index e7399cb1e1..f7e37e1d5c 100644 --- a/SignalServiceKit/src/Contacts/OWSContactDiscoveryOperation.swift +++ b/SignalServiceKit/src/Contacts/OWSContactDiscoveryOperation.swift @@ -4,56 +4,6 @@ import Foundation -@objc(OWSCDSOperation) -class CDSOperation: OWSOperation { - - let batchSize = 2048 - static let operationQueue: OperationQueue = { - let queue = OperationQueue() - queue.maxConcurrentOperationCount = 5 - - return queue - }() - - let recipientIdsToLookup: [String] - - @objc - var registeredRecipientIds: Set - - @objc - required init(recipientIdsToLookup: [String]) { - self.recipientIdsToLookup = recipientIdsToLookup - self.registeredRecipientIds = Set() - - super.init() - - Logger.debug("\(logTag) in \(#function) with recipientIdsToLookup: \(recipientIdsToLookup.count)") - for batchIds in recipientIdsToLookup.chunked(by: batchSize) { - let batchOperation = CDSBatchOperation(recipientIdsToLookup: batchIds) - self.addDependency(batchOperation) - } - } - - // MARK: Mandatory overrides - - // Called every retry, this is where the bulk of the operation's work should go. - override func run() { - Logger.debug("\(logTag) in \(#function)") - - for dependency in self.dependencies { - guard let batchOperation = dependency as? CDSBatchOperation else { - owsFail("\(self.logTag) in \(#function) unexpected dependency: \(dependency)") - continue - } - - self.registeredRecipientIds.formUnion(batchOperation.registeredRecipientIds) - } - - self.reportSuccess() - } - -} - @objc(OWSLegacyContactDiscoveryOperation) class LegacyContactDiscoveryBatchOperation: OWSOperation { @@ -191,6 +141,56 @@ enum ContactDiscoveryError: Error { case serverError(underlyingError: Error) } +@objc(OWSCDSOperation) +class CDSOperation: OWSOperation { + + let batchSize = 2048 + static let operationQueue: OperationQueue = { + let queue = OperationQueue() + queue.maxConcurrentOperationCount = 5 + + return queue + }() + + let recipientIdsToLookup: [String] + + @objc + var registeredRecipientIds: Set + + @objc + required init(recipientIdsToLookup: [String]) { + self.recipientIdsToLookup = recipientIdsToLookup + self.registeredRecipientIds = Set() + + super.init() + + Logger.debug("\(logTag) in \(#function) with recipientIdsToLookup: \(recipientIdsToLookup.count)") + for batchIds in recipientIdsToLookup.chunked(by: batchSize) { + let batchOperation = CDSBatchOperation(recipientIdsToLookup: batchIds) + self.addDependency(batchOperation) + } + } + + // MARK: Mandatory overrides + + // Called every retry, this is where the bulk of the operation's work should go. + override func run() { + Logger.debug("\(logTag) in \(#function)") + + for dependency in self.dependencies { + guard let batchOperation = dependency as? CDSBatchOperation else { + owsFail("\(self.logTag) in \(#function) unexpected dependency: \(dependency)") + continue + } + + self.registeredRecipientIds.formUnion(batchOperation.registeredRecipientIds) + } + + self.reportSuccess() + } + +} + public class CDSBatchOperation: OWSOperation {