From b7288b25659109c3f6ea0e3843e460dba50dd356 Mon Sep 17 00:00:00 2001 From: Michael Kirk Date: Wed, 18 Jul 2018 14:46:46 -0600 Subject: [PATCH] Move contact intersection into batched operation // FREEBIE --- .../src/Contacts/ContactsUpdater.m | 107 ++++------ .../OWSContactDiscoveryOperation.swift | 196 ++++++++++++++++++ .../src/Messages/OWSMessageSender.m | 18 +- .../Network/API/Requests/OWSRequestFactory.h | 2 +- .../Network/API/Requests/OWSRequestFactory.m | 2 +- SignalServiceKit/src/SignalServiceKit.h | 6 + SignalServiceKit/src/Util/OWSOperation.m | 20 +- 7 files changed, 264 insertions(+), 87 deletions(-) create mode 100644 SignalServiceKit/src/Contacts/OWSContactDiscoveryOperation.swift create mode 100644 SignalServiceKit/src/SignalServiceKit.h diff --git a/SignalServiceKit/src/Contacts/ContactsUpdater.m b/SignalServiceKit/src/Contacts/ContactsUpdater.m index dfd0d69c0..8117b5845 100644 --- a/SignalServiceKit/src/Contacts/ContactsUpdater.m +++ b/SignalServiceKit/src/Contacts/ContactsUpdater.m @@ -9,10 +9,18 @@ #import "OWSRequestFactory.h" #import "PhoneNumber.h" #import "TSNetworkManager.h" +#import #import NS_ASSUME_NONNULL_BEGIN +@interface ContactsUpdater () + +@property (nonatomic, readonly) NSOperationQueue *contactIntersectionQueue; + +@end + + @implementation ContactsUpdater + (instancetype)sharedUpdater { @@ -32,6 +40,8 @@ NS_ASSUME_NONNULL_BEGIN return self; } + _contactIntersectionQueue = [NSOperationQueue new]; + OWSSingletonAssert(); return self; @@ -88,75 +98,36 @@ NS_ASSUME_NONNULL_BEGIN - (void)contactIntersectionWithSet:(NSSet *)recipientIdsToLookup success:(void (^)(NSSet *recipients))success - failure:(void (^)(NSError *error))failure { + failure:(void (^)(NSError *error))failure +{ dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{ - NSMutableDictionary *phoneNumbersByHashes = [NSMutableDictionary new]; - for (NSString *recipientId in recipientIdsToLookup) { - NSString *hash = [Cryptography truncatedSHA1Base64EncodedWithoutPadding:recipientId]; - phoneNumbersByHashes[hash] = recipientId; - } - NSArray *hashes = [phoneNumbersByHashes allKeys]; - - TSRequest *request = [OWSRequestFactory contactsIntersectionRequestWithHashesArray:hashes]; - [[TSNetworkManager sharedManager] makeRequest:request - success:^(NSURLSessionDataTask *task, id responseDict) { - NSMutableSet *registeredRecipientIds = [NSMutableSet new]; - - if ([responseDict isKindOfClass:[NSDictionary class]]) { - NSArray *_Nullable contactsArray = responseDict[@"contacts"]; - if ([contactsArray isKindOfClass:[NSArray class]]) { - for (NSDictionary *contactDict in contactsArray) { - if (![contactDict isKindOfClass:[NSDictionary class]]) { - OWSProdLogAndFail(@"%@ invalid contact dictionary.", self.logTag); - continue; - } - NSString *_Nullable hash = contactDict[@"token"]; - if (hash.length < 1) { - OWSProdLogAndFail(@"%@ contact missing hash.", self.logTag); - continue; - } - NSString *_Nullable recipientId = phoneNumbersByHashes[hash]; - if (recipientId.length < 1) { - OWSProdLogAndFail(@"%@ An intersecting hash wasn't found in the mapping.", self.logTag); - continue; - } - if (![recipientIdsToLookup containsObject:recipientId]) { - OWSProdLogAndFail(@"%@ Intersection response included unexpected recipient.", self.logTag); - continue; - } - [registeredRecipientIds addObject:recipientId]; - } - } - } - - NSMutableSet *recipients = [NSMutableSet new]; - [OWSPrimaryStorage.dbReadWriteConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { - for (NSString *recipientId in recipientIdsToLookup) { - if ([registeredRecipientIds containsObject:recipientId]) { - SignalRecipient *recipient = - [SignalRecipient markRecipientAsRegisteredAndGet:recipientId transaction:transaction]; - [recipients addObject:recipient]; - } else { - [SignalRecipient removeUnregisteredRecipient:recipientId transaction:transaction]; - } - } - }]; - - success([recipients copy]); - } - failure:^(NSURLSessionDataTask *task, NSError *error) { - if (!IsNSErrorNetworkFailure(error)) { - OWSProdError([OWSAnalyticsEvents contactsErrorContactsIntersectionFailed]); - } - - NSHTTPURLResponse *response = (NSHTTPURLResponse *)task.response; - if (response.statusCode == 413) { - failure(OWSErrorWithCodeDescription( - OWSErrorCodeContactsUpdaterRateLimit, @"Contacts Intersection Rate Limit")); - } else { - failure(error); - } - }]; + OWSContactDiscoveryOperation *operation = + [[OWSContactDiscoveryOperation alloc] initWithRecipientIdsToLookup:recipientIdsToLookup.allObjects]; + + NSArray *operationAndDependencies = [operation.dependencies arrayByAddingObject:operation]; + [self.contactIntersectionQueue addOperations:operationAndDependencies waitUntilFinished:YES]; + + if (operation.failingError != nil) { + failure(operation.failingError); + return; + } + + NSSet *registeredRecipientIds = operation.registeredRecipientIds; + + NSMutableSet *recipients = [NSMutableSet new]; + [OWSPrimaryStorage.dbReadWriteConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { + for (NSString *recipientId in recipientIdsToLookup) { + if ([registeredRecipientIds containsObject:recipientId]) { + SignalRecipient *recipient = + [SignalRecipient markRecipientAsRegisteredAndGet:recipientId transaction:transaction]; + [recipients addObject:recipient]; + } else { + [SignalRecipient removeUnregisteredRecipient:recipientId transaction:transaction]; + } + } + }]; + + success([recipients copy]); }); } diff --git a/SignalServiceKit/src/Contacts/OWSContactDiscoveryOperation.swift b/SignalServiceKit/src/Contacts/OWSContactDiscoveryOperation.swift new file mode 100644 index 000000000..58c741d35 --- /dev/null +++ b/SignalServiceKit/src/Contacts/OWSContactDiscoveryOperation.swift @@ -0,0 +1,196 @@ +// +// Copyright (c) 2018 Open Whisper Systems. All rights reserved. +// + +import Foundation + +extension Array { + func chunked(by chunkSize: Int) -> [[Element]] { + return stride(from: 0, to: self.count, by: chunkSize).map { + Array(self[$0.. + + @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 = OWSContactDiscoveryBatchOperation(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? OWSContactDiscoveryBatchOperation else { + owsFail("\(self.logTag) in \(#function) unexpected dependency: \(dependency)") + continue + } + + self.registeredRecipientIds.formUnion(batchOperation.registeredRecipientIds) + } + + self.reportSuccess() + } + + // MARK: Optional Overrides + + // Called one time only + override func checkForPreconditionError() -> Error? { + return super.checkForPreconditionError() + } + + // Called at most one time. + override func didSucceed() { + super.didSucceed() + } + + // Called at most one time, once retry is no longer possible. + override func didFail(error: Error) { + super.didFail(error: error) + } +} + +class OWSContactDiscoveryBatchOperation: OWSOperation { + + private let recipientIdsToLookup: [String] + var registeredRecipientIds: Set + + required init(recipientIdsToLookup: [String]) { + self.recipientIdsToLookup = recipientIdsToLookup + self.registeredRecipientIds = Set() + + super.init() + + Logger.debug("\(logTag) in \(#function) with recipientIdsToLookup: \(recipientIdsToLookup.count)") + } + + private var networkManager: TSNetworkManager { + return TSNetworkManager.shared() + } + + private func parse(response: Any?, phoneNumbersByHashes: [String: String]) throws -> Set { + + guard let responseDict = response as? [String: AnyObject] else { + let responseError: NSError = OWSErrorMakeUnableToProcessServerResponseError() as NSError + responseError.isRetryable = true + + throw responseError + } + + guard let contactDicts = responseDict["contacts"] as? [[String: AnyObject]] else { + let responseError: NSError = OWSErrorMakeUnableToProcessServerResponseError() as NSError + responseError.isRetryable = true + + throw responseError + } + + var registeredRecipientIds: Set = Set() + + for contactDict in contactDicts { + guard let hash = contactDict["token"] as? String, hash.count > 0 else { + owsFail("\(self.logTag) in \(#function) hash was unexpectedly nil") + continue + } + + guard let recipientId = phoneNumbersByHashes[hash], recipientId.count > 0 else { + owsFail("\(self.logTag) in \(#function) recipientId was unexpectedly nil") + continue + } + + guard recipientIdsToLookup.contains(recipientId) else { + owsFail("\(self.logTag) in \(#function) unexpected recipientId") + continue + } + + registeredRecipientIds.insert(recipientId) + } + + return registeredRecipientIds + } + + // 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)") + + var phoneNumbersByHashes: [String: String] = [:] + + for recipientId in recipientIdsToLookup { + let hash = Cryptography.truncatedSHA1Base64EncodedWithoutPadding(recipientId) + phoneNumbersByHashes[hash] = recipientId + } + + let hashes: [String] = Array(phoneNumbersByHashes.keys) + + let request = OWSRequestFactory.contactsIntersectionRequest(withHashesArray: hashes) + + self.networkManager.makeRequest(request, + success: { (task, responseDict) in + do { + self.registeredRecipientIds = try self.parse(response: responseDict, phoneNumbersByHashes: phoneNumbersByHashes) + self.reportSuccess() + } catch { + self.reportError(error) + } + }, + failure: { (task, error) in + if (!IsNSErrorNetworkFailure(error)) { + // FIXME not accessible in swift for some reason. +// OWSProdError(OWSAnalyticsEvents.contactsErrorContactsIntersectionFailed) + } + + guard let response = task.response as? HTTPURLResponse else { + let responseError: NSError = OWSErrorMakeUnableToProcessServerResponseError() as NSError + responseError.isRetryable = true + self.reportError(responseError) + return + } + + if (response.statusCode == 413) { + let rateLimitError = OWSErrorWithCodeDescription(OWSErrorCode.contactsUpdaterRateLimit, "Contacts Intersection Rate Limit") + self.reportError(rateLimitError) + } + self.reportError(error) + + }) + } + + // MARK: Optional Overrides + + // Called one time only + override func checkForPreconditionError() -> Error? { + return super.checkForPreconditionError() + } + + // Called at most one time. + override func didSucceed() { + super.didSucceed() + } + + // Called at most one time, once retry is no longer possible. + override func didFail(error: Error) { + super.didFail(error: error) + } +} diff --git a/SignalServiceKit/src/Messages/OWSMessageSender.m b/SignalServiceKit/src/Messages/OWSMessageSender.m index 018977468..24240aba8 100644 --- a/SignalServiceKit/src/Messages/OWSMessageSender.m +++ b/SignalServiceKit/src/Messages/OWSMessageSender.m @@ -132,21 +132,9 @@ void AssertIsOnSendingQueue() - (nullable NSError *)checkForPreconditionError { - for (NSOperation *dependency in self.dependencies) { - if (![dependency isKindOfClass:[OWSOperation class]]) { - NSString *errorDescription = - [NSString stringWithFormat:@"%@ unknown dependency: %@", self.logTag, dependency.class]; - NSError *assertionError = OWSErrorMakeAssertionError(errorDescription); - return assertionError; - } - - OWSOperation *upload = (OWSOperation *)dependency; - - // Cannot proceed if dependency failed - surface the dependency's error. - NSError *_Nullable dependencyError = upload.failingError; - if (dependencyError) { - return dependencyError; - } + NSError *_Nullable error = [super checkForPreconditionError]; + if (error) { + return error; } // Sanity check preconditions diff --git a/SignalServiceKit/src/Network/API/Requests/OWSRequestFactory.h b/SignalServiceKit/src/Network/API/Requests/OWSRequestFactory.h index 9ea9864c1..d6597ed89 100644 --- a/SignalServiceKit/src/Network/API/Requests/OWSRequestFactory.h +++ b/SignalServiceKit/src/Network/API/Requests/OWSRequestFactory.h @@ -41,7 +41,7 @@ typedef NS_ENUM(NSUInteger, TSVerificationTransport) { TSVerificationTransportVo + (TSRequest *)availablePreKeysCountRequest; -+ (TSRequest *)contactsIntersectionRequestWithHashesArray:(NSArray *)hashes; ++ (TSRequest *)contactsIntersectionRequestWithHashesArray:(NSArray *)hashes; + (TSRequest *)currentSignedPreKeyRequest; diff --git a/SignalServiceKit/src/Network/API/Requests/OWSRequestFactory.m b/SignalServiceKit/src/Network/API/Requests/OWSRequestFactory.m index 334252fc0..e00460a0a 100644 --- a/SignalServiceKit/src/Network/API/Requests/OWSRequestFactory.m +++ b/SignalServiceKit/src/Network/API/Requests/OWSRequestFactory.m @@ -116,7 +116,7 @@ NS_ASSUME_NONNULL_BEGIN return [TSRequest requestWithUrl:[NSURL URLWithString:path] method:@"GET" parameters:@{}]; } -+ (TSRequest *)contactsIntersectionRequestWithHashesArray:(NSArray *)hashes ++ (TSRequest *)contactsIntersectionRequestWithHashesArray:(NSArray *)hashes { OWSAssert(hashes.count > 0); diff --git a/SignalServiceKit/src/SignalServiceKit.h b/SignalServiceKit/src/SignalServiceKit.h new file mode 100644 index 000000000..695adba43 --- /dev/null +++ b/SignalServiceKit/src/SignalServiceKit.h @@ -0,0 +1,6 @@ +// +// Copyright (c) 2018 Open Whisper Systems. All rights reserved. +// + +// ObjC classes from which Swift classes inherit must be included in this framework header. +#import "OWSOperation.h" diff --git a/SignalServiceKit/src/Util/OWSOperation.m b/SignalServiceKit/src/Util/OWSOperation.m index 7b081f0b3..d70cbb4c6 100644 --- a/SignalServiceKit/src/Util/OWSOperation.m +++ b/SignalServiceKit/src/Util/OWSOperation.m @@ -5,6 +5,7 @@ #import "OWSOperation.h" #import "NSError+MessageSending.h" #import "OWSBackgroundTask.h" +#import "OWSError.h" NS_ASSUME_NONNULL_BEGIN @@ -47,8 +48,23 @@ NSString *const OWSOperationKeyIsFinished = @"isFinished"; // Called one time only - (nullable NSError *)checkForPreconditionError { - // no-op - // Override in subclass if necessary + for (NSOperation *dependency in self.dependencies) { + if (![dependency isKindOfClass:[OWSOperation class]]) { + NSString *errorDescription = + [NSString stringWithFormat:@"%@ unknown dependency: %@", self.logTag, dependency.class]; + + return OWSErrorMakeAssertionError(errorDescription); + } + + OWSOperation *dependentOperation = (OWSOperation *)dependency; + + // Don't proceed if dependency failed - surface the dependency's error. + NSError *_Nullable dependencyError = dependentOperation.failingError; + if (dependencyError != nil) { + return dependencyError; + } + } + return nil; }