From cabb39e1a94e6321e32d3f33aeee5914b646cbb0 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Wed, 15 Apr 2020 13:20:25 -0300 Subject: [PATCH 01/13] "Bump build to 3.8.0.16." (Internal) --- NotificationServiceExtension/Info.plist | 2 +- Signal/Signal-Info.plist | 2 +- SignalShareExtension/Info.plist | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/NotificationServiceExtension/Info.plist b/NotificationServiceExtension/Info.plist index 60a67f4a64..c597e3c59c 100644 --- a/NotificationServiceExtension/Info.plist +++ b/NotificationServiceExtension/Info.plist @@ -19,7 +19,7 @@ CFBundleShortVersionString 3.8.0 CFBundleVersion - 3.8.0.15 + 3.8.0.16 NSAppTransportSecurity NSExceptionDomains diff --git a/Signal/Signal-Info.plist b/Signal/Signal-Info.plist index ae14803dbd..8143081faf 100644 --- a/Signal/Signal-Info.plist +++ b/Signal/Signal-Info.plist @@ -45,7 +45,7 @@ CFBundleVersion - 3.8.0.15 + 3.8.0.16 ITSAppUsesNonExemptEncryption LOGS_EMAIL diff --git a/SignalShareExtension/Info.plist b/SignalShareExtension/Info.plist index 275311161f..5221bb4c82 100644 --- a/SignalShareExtension/Info.plist +++ b/SignalShareExtension/Info.plist @@ -19,7 +19,7 @@ CFBundleShortVersionString 3.8.0 CFBundleVersion - 3.8.0.15 + 3.8.0.16 ITSAppUsesNonExemptEncryption NSAppTransportSecurity From 35e4518d625f3b2f8ed027dff5b61e4349d43b0a Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Wed, 15 Apr 2020 13:20:34 -0300 Subject: [PATCH 02/13] "Bump build to 3.8.0.17." (Internal) --- NotificationServiceExtension/Info.plist | 2 +- Signal/Signal-Info.plist | 2 +- SignalShareExtension/Info.plist | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/NotificationServiceExtension/Info.plist b/NotificationServiceExtension/Info.plist index c597e3c59c..7b4305c696 100644 --- a/NotificationServiceExtension/Info.plist +++ b/NotificationServiceExtension/Info.plist @@ -19,7 +19,7 @@ CFBundleShortVersionString 3.8.0 CFBundleVersion - 3.8.0.16 + 3.8.0.17 NSAppTransportSecurity NSExceptionDomains diff --git a/Signal/Signal-Info.plist b/Signal/Signal-Info.plist index 8143081faf..be82ccb125 100644 --- a/Signal/Signal-Info.plist +++ b/Signal/Signal-Info.plist @@ -45,7 +45,7 @@ CFBundleVersion - 3.8.0.16 + 3.8.0.17 ITSAppUsesNonExemptEncryption LOGS_EMAIL diff --git a/SignalShareExtension/Info.plist b/SignalShareExtension/Info.plist index 5221bb4c82..c8b0bf2d5e 100644 --- a/SignalShareExtension/Info.plist +++ b/SignalShareExtension/Info.plist @@ -19,7 +19,7 @@ CFBundleShortVersionString 3.8.0 CFBundleVersion - 3.8.0.16 + 3.8.0.17 ITSAppUsesNonExemptEncryption NSAppTransportSecurity From 6910a0ac198b64ba85429d470ee1a0226ebbf624 Mon Sep 17 00:00:00 2001 From: Nora Trapp Date: Tue, 14 Apr 2020 16:29:20 -0700 Subject: [PATCH 03/13] Treat iPhone XR as plus size phone --- SignalMessaging/categories/UIDevice+featureSupport.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/SignalMessaging/categories/UIDevice+featureSupport.swift b/SignalMessaging/categories/UIDevice+featureSupport.swift index c02fcbfcad..3c73579ee1 100644 --- a/SignalMessaging/categories/UIDevice+featureSupport.swift +++ b/SignalMessaging/categories/UIDevice+featureSupport.swift @@ -58,7 +58,7 @@ public extension UIDevice { return false case 1792: // iPhone XR - return false + return true case 1920, 2208: // iPhone 6+/6S+/7+/8+// return true From a4d802a2a25bee6963dd3f512351da6e4b0ed184 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Tue, 14 Apr 2020 15:33:28 -0300 Subject: [PATCH 04/13] Avoid rate limits while doing profile fetches. --- .../ConversationViewController.m | 7 +- .../BaseGroupMemberViewController.swift | 11 +- .../NewGroupView/NewGroupViewController.m | 1 + .../AddToBlockListViewController.swift | 1 + .../ComposeViewController.swift | 1 + .../UpdateGroupViewController.m | 1 + SignalMessaging/environment/AppSetup.m | 4 +- SignalMessaging/groups/GroupsV2Impl.swift | 4 - SignalMessaging/profiles/OWSProfileManager.m | 9 + .../profiles/ProfileFetcherJob.swift | 29 +-- .../Messages/Stickers/DefaultStickers.swift | 3 + .../src/Messages/UD/OWSRequestMaker.swift | 10 + .../src/Protocols/ProfileManagerProtocol.h | 3 + SignalServiceKit/src/SSKEnvironment.h | 6 +- SignalServiceKit/src/SSKEnvironment.m | 6 +- .../src/TestUtils/MockSSKEnvironment.m | 4 +- .../src/TestUtils/OWSFakeProfileManager.m | 7 + .../src/Util/BulkProfileFetch.swift | 230 ++++++++++++++++++ SignalServiceKit/src/Util/OrderedSet.swift | 4 + .../src/groups/GroupManager.swift | 29 ++- 20 files changed, 332 insertions(+), 38 deletions(-) create mode 100644 SignalServiceKit/src/Util/BulkProfileFetch.swift diff --git a/Signal/src/ViewControllers/ConversationView/ConversationViewController.m b/Signal/src/ViewControllers/ConversationView/ConversationViewController.m index 9dd54341dc..51fff43375 100644 --- a/Signal/src/ViewControllers/ConversationView/ConversationViewController.m +++ b/Signal/src/ViewControllers/ConversationView/ConversationViewController.m @@ -362,6 +362,11 @@ typedef enum : NSUInteger { return SSKEnvironment.shared.syncManager; } +- (BulkProfileFetch *)bulkProfileFetch +{ + return SSKEnvironment.shared.bulkProfileFetch; +} + #pragma mark - - (void)addNotificationListeners @@ -1202,7 +1207,7 @@ typedef enum : NSUInteger { // recover status bar when returning from PhotoPicker, which is dark (uses light status bar) [self setNeedsStatusBarAppearanceUpdate]; - [ProfileFetcherJob fetchAndUpdateProfilesWithThread:self.thread]; + [self.bulkProfileFetch fetchAndUpdateProfilesWithThread:self.thread]; [self markVisibleMessagesAsRead]; [self startReadTimer]; [self updateNavigationBarSubtitleLabel]; diff --git a/Signal/src/ViewControllers/NewGroupView/BaseGroupMemberViewController.swift b/Signal/src/ViewControllers/NewGroupView/BaseGroupMemberViewController.swift index f2dd4a1296..69e619bda2 100644 --- a/Signal/src/ViewControllers/NewGroupView/BaseGroupMemberViewController.swift +++ b/Signal/src/ViewControllers/NewGroupView/BaseGroupMemberViewController.swift @@ -395,9 +395,10 @@ extension BaseGroupMemberViewController: RecipientPickerDelegate { RemoteConfig.groupsV2IncomingMessages else { return } - // GroupsV2 TODO: We could debounce this effort. if !doesRecipientSupportGroupsV2(recipient) { - tryToEnableGroupsV2ForAddress(address, ignoreErrors: true).retainUntilComplete() + tryToEnableGroupsV2ForAddress(address, + isBlocking: false, + ignoreErrors: true).retainUntilComplete() } } @@ -420,7 +421,9 @@ extension BaseGroupMemberViewController: RecipientPickerDelegate { return AnyPromise(Promise.value(())) } let ignoreErrors = !groupMemberViewDelegate.groupMemberViewIsGroupsV2Required() - return AnyPromise(tryToEnableGroupsV2ForAddress(address, ignoreErrors: ignoreErrors)) + return AnyPromise(tryToEnableGroupsV2ForAddress(address, + isBlocking: true, + ignoreErrors: ignoreErrors)) } func recipientPicker(_ recipientPickerViewController: RecipientPickerViewController, @@ -440,9 +443,11 @@ extension BaseGroupMemberViewController: RecipientPickerDelegate { } func tryToEnableGroupsV2ForAddress(_ address: SignalServiceAddress, + isBlocking: Bool, ignoreErrors: Bool) -> Promise { return firstly { () -> Promise in return GroupManager.tryToEnableGroupsV2(for: [address], + isBlocking: isBlocking, ignoreErrors: ignoreErrors) }.done { [weak self] _ in // Reload view content. diff --git a/Signal/src/ViewControllers/NewGroupView/NewGroupViewController.m b/Signal/src/ViewControllers/NewGroupView/NewGroupViewController.m index dfea4b4edc..e10d974e45 100644 --- a/Signal/src/ViewControllers/NewGroupView/NewGroupViewController.m +++ b/Signal/src/ViewControllers/NewGroupView/NewGroupViewController.m @@ -445,6 +445,7 @@ NS_ASSUME_NONNULL_BEGIN - (AnyPromise *)recipientPicker:(RecipientPickerViewController *)recipientPickerViewController prepareToSelectRecipient:(PickedRecipient *)recipient { + OWSFailDebug(@"This method should not called."); return [AnyPromise promiseWithValue:@(1)]; } diff --git a/Signal/src/ViewControllers/RecipientPicker/AddToBlockListViewController.swift b/Signal/src/ViewControllers/RecipientPicker/AddToBlockListViewController.swift index 3697ddc186..6d68d75097 100644 --- a/Signal/src/ViewControllers/RecipientPicker/AddToBlockListViewController.swift +++ b/Signal/src/ViewControllers/RecipientPicker/AddToBlockListViewController.swift @@ -88,6 +88,7 @@ extension AddToBlockListViewController: RecipientPickerDelegate { func recipientPicker(_ recipientPickerViewController: RecipientPickerViewController, prepareToSelectRecipient recipient: PickedRecipient) -> AnyPromise { + owsFailDebug("This method should not called.") return AnyPromise(Promise.value(())) } diff --git a/Signal/src/ViewControllers/RecipientPicker/ComposeViewController.swift b/Signal/src/ViewControllers/RecipientPicker/ComposeViewController.swift index 1e23a1f46b..bcf1a22758 100644 --- a/Signal/src/ViewControllers/RecipientPicker/ComposeViewController.swift +++ b/Signal/src/ViewControllers/RecipientPicker/ComposeViewController.swift @@ -86,6 +86,7 @@ extension ComposeViewController: RecipientPickerDelegate { func recipientPicker(_ recipientPickerViewController: RecipientPickerViewController, prepareToSelectRecipient recipient: PickedRecipient) -> AnyPromise { + owsFailDebug("This method should not called.") return AnyPromise(Promise.value(())) } diff --git a/Signal/src/ViewControllers/ThreadSettings/UpdateGroupViewController.m b/Signal/src/ViewControllers/ThreadSettings/UpdateGroupViewController.m index 4de6d91e60..02a6730859 100644 --- a/Signal/src/ViewControllers/ThreadSettings/UpdateGroupViewController.m +++ b/Signal/src/ViewControllers/ThreadSettings/UpdateGroupViewController.m @@ -521,6 +521,7 @@ NS_ASSUME_NONNULL_BEGIN - (AnyPromise *)recipientPicker:(RecipientPickerViewController *)recipientPickerViewController prepareToSelectRecipient:(PickedRecipient *)recipient { + OWSFailDebug(@"This method should not called."); return [AnyPromise promiseWithValue:@(1)]; } diff --git a/SignalMessaging/environment/AppSetup.m b/SignalMessaging/environment/AppSetup.m index abde31502a..04017fabfb 100644 --- a/SignalMessaging/environment/AppSetup.m +++ b/SignalMessaging/environment/AppSetup.m @@ -113,6 +113,7 @@ NS_ASSUME_NONNULL_BEGIN OWSWindowManager *windowManager = [[OWSWindowManager alloc] initDefault]; MessageProcessing *messageProcessing = [MessageProcessing new]; MessageFetcherJob *messageFetcherJob = [MessageFetcherJob new]; + BulkProfileFetch *bulkProfileFetch = [BulkProfileFetch new]; [Environment setShared:[[Environment alloc] initWithAudioSession:audioSession incomingContactSyncJobQueue:incomingContactSyncJobQueue @@ -167,7 +168,8 @@ NS_ASSUME_NONNULL_BEGIN groupsV2:groupsV2 groupV2Updates:groupV2Updates messageProcessing:messageProcessing - messageFetcherJob:messageFetcherJob]]; + messageFetcherJob:messageFetcherJob + bulkProfileFetch:bulkProfileFetch]]; appSpecificSingletonBlock(); diff --git a/SignalMessaging/groups/GroupsV2Impl.swift b/SignalMessaging/groups/GroupsV2Impl.swift index aee60f63b8..032dc4956a 100644 --- a/SignalMessaging/groups/GroupsV2Impl.swift +++ b/SignalMessaging/groups/GroupsV2Impl.swift @@ -45,10 +45,6 @@ public class GroupsV2Impl: NSObject, GroupsV2Swift { return SSKEnvironment.shared.contactsUpdater } - private var reachabilityManager: SSKReachabilityManager { - return SSKEnvironment.shared.reachabilityManager - } - // MARK: - public typealias ProfileKeyCredentialMap = [UUID: ProfileKeyCredential] diff --git a/SignalMessaging/profiles/OWSProfileManager.m b/SignalMessaging/profiles/OWSProfileManager.m index e9b4b17f00..288c206a15 100644 --- a/SignalMessaging/profiles/OWSProfileManager.m +++ b/SignalMessaging/profiles/OWSProfileManager.m @@ -449,6 +449,15 @@ const NSString *kNSNotificationKey_WasLocallyInitiated = @"kNSNotificationKey_Wa return [ProfileFetcherJob fetchAndUpdateProfilePromiseObjcWithAddress:address mainAppOnly:NO ignoreThrottling:YES]; } +- (AnyPromise *)updateProfileForAddressPromise:(SignalServiceAddress *)address + mainAppOnly:(BOOL)mainAppOnly + ignoreThrottling:(BOOL)ignoreThrottling +{ + return [ProfileFetcherJob fetchAndUpdateProfilePromiseObjcWithAddress:address + mainAppOnly:mainAppOnly + ignoreThrottling:ignoreThrottling]; +} + - (void)fetchAndUpdateProfileForUsername:(NSString *)username success:(void (^)(SignalServiceAddress *))success notFound:(void (^)(void))notFound diff --git a/SignalMessaging/profiles/ProfileFetcherJob.swift b/SignalMessaging/profiles/ProfileFetcherJob.swift index 0d269ac2ff..15417e8d34 100644 --- a/SignalMessaging/profiles/ProfileFetcherJob.swift +++ b/SignalMessaging/profiles/ProfileFetcherJob.swift @@ -155,19 +155,6 @@ public class ProfileFetcherJob: NSObject { .retainUntilComplete() } - @objc(fetchAndUpdateProfilesWithThread:) - public class func fetchAndUpdateProfiles(thread: TSThread) { - let addresses = thread.recipientAddresses - let subjects = addresses.map { ProfileRequestSubject.address(address: $0) } - let options = ProfileFetchOptions() - var promises = [Promise]() - for subject in subjects { - let job = ProfileFetcherJob(subject: subject, options: options) - promises.append(job.runAsPromise()) - } - when(fulfilled: promises).retainUntilComplete() - } - private init(subject: ProfileRequestSubject, options: ProfileFetchOptions) { self.subject = subject @@ -255,7 +242,7 @@ public class ProfileFetcherJob: NSObject { return requestProfileWithRetries() } - private func requestProfileWithRetries(remainingRetries: Int = 3) -> Promise { + private func requestProfileWithRetries(retryCount: Int = 0) -> Promise { let subject = self.subject let (promise, resolver) = Promise.pending() @@ -281,14 +268,22 @@ public class ProfileFetcherJob: NSObject { resolver.reject(error) return default: - guard remainingRetries > 0 else { + let maxRetries = 3 + guard retryCount < maxRetries else { Logger.warn("failed to get profile with error: \(error)") resolver.reject(error) return } - firstly { - self.requestProfileWithRetries(remainingRetries: remainingRetries - 1) + firstly { () -> Guarantee in + // Backoff if we've hit a rate limit. + if error.httpStatusCode == 413 { + let backoffDelay = TimeInterval(retryCount + 1) * 30 + return after(seconds: backoffDelay) + } + return Guarantee.value(()) + }.then(on: DispatchQueue.global()) { _ in + self.requestProfileWithRetries(retryCount: retryCount + 1) }.done(on: DispatchQueue.global()) { fetchedProfile in resolver.fulfill(fetchedProfile) }.catch(on: DispatchQueue.global()) { error in diff --git a/SignalServiceKit/src/Messages/Stickers/DefaultStickers.swift b/SignalServiceKit/src/Messages/Stickers/DefaultStickers.swift index 01b7f24532..993cc14321 100644 --- a/SignalServiceKit/src/Messages/Stickers/DefaultStickers.swift +++ b/SignalServiceKit/src/Messages/Stickers/DefaultStickers.swift @@ -19,6 +19,9 @@ class DefaultStickerPack { } private class func parseAll() -> [StickerPackInfo: DefaultStickerPack] { + guard FeatureFlags.isUsingProductionService else { + return [:] + } let packs = [ // Bandit the Cat DefaultStickerPack(packIdHex: "9acc9e8aba563d26a4994e69263e3b25", packKeyHex: "5a6dff3948c28efb9b7aaf93ecc375c69fc316e78077ed26867a14d10a0f6a12", shouldAutoInstall: false)!, diff --git a/SignalServiceKit/src/Messages/UD/OWSRequestMaker.swift b/SignalServiceKit/src/Messages/UD/OWSRequestMaker.swift index ff70a2dc27..971e55fea8 100644 --- a/SignalServiceKit/src/Messages/UD/OWSRequestMaker.swift +++ b/SignalServiceKit/src/Messages/UD/OWSRequestMaker.swift @@ -145,6 +145,11 @@ public class RequestMaker: NSObject { resolver.reject(RequestMakerError.websocketRequestError(statusCode: statusCode, responseData: responseData, underlyingError: error)) }) }.recover { (error: Error) -> Promise in + if error.httpStatusCode == 413 { + // We've hit rate limit; don't retry. + throw error + } + switch error { case RequestMakerError.websocketRequestError(let statusCode, _, _): if isUDRequest && (statusCode == 401 || statusCode == 403) { @@ -193,6 +198,11 @@ public class RequestMaker: NSObject { wasSentByUD: isUDRequest, wasSentByWebsocket: false) }.recover { (error: Error) -> Promise in + if error.httpStatusCode == 413 { + // We've hit rate limit; don't retry. + throw error + } + if isUDRequest, let statusCode = error.httpStatusCode, statusCode == 401 || statusCode == 403 { diff --git a/SignalServiceKit/src/Protocols/ProfileManagerProtocol.h b/SignalServiceKit/src/Protocols/ProfileManagerProtocol.h index 56686da68f..bdf8e988f4 100644 --- a/SignalServiceKit/src/Protocols/ProfileManagerProtocol.h +++ b/SignalServiceKit/src/Protocols/ProfileManagerProtocol.h @@ -85,6 +85,9 @@ NS_ASSUME_NONNULL_BEGIN - (void)updateProfileForAddress:(SignalServiceAddress *)address; - (AnyPromise *)updateProfileForAddressPromise:(SignalServiceAddress *)address; +- (AnyPromise *)updateProfileForAddressPromise:(SignalServiceAddress *)address + mainAppOnly:(BOOL)mainAppOnly + ignoreThrottling:(BOOL)ignoreThrottling; - (BOOL)recipientAddressIsUuidCapable:(SignalServiceAddress *)address transaction:(SDSAnyReadTransaction *)transaction; diff --git a/SignalServiceKit/src/SSKEnvironment.h b/SignalServiceKit/src/SSKEnvironment.h index 6b701200d4..aef5c9687f 100644 --- a/SignalServiceKit/src/SSKEnvironment.h +++ b/SignalServiceKit/src/SSKEnvironment.h @@ -5,6 +5,7 @@ NS_ASSUME_NONNULL_BEGIN @class AccountServiceClient; +@class BulkProfileFetch; @class ContactsUpdater; @class GroupsV2MessageProcessor; @class MessageFetcherJob; @@ -98,7 +99,8 @@ NS_ASSUME_NONNULL_BEGIN groupsV2:(id)groupsV2 groupV2Updates:(id)groupV2Updates messageProcessing:(MessageProcessing *)messageProcessing - messageFetcherJob:(MessageFetcherJob *)messageFetcherJob NS_DESIGNATED_INITIALIZER; + messageFetcherJob:(MessageFetcherJob *)messageFetcherJob + bulkProfileFetch:(BulkProfileFetch *)bulkProfileFetch NS_DESIGNATED_INITIALIZER; @property (nonatomic, readonly, class) SSKEnvironment *shared; @@ -147,13 +149,13 @@ NS_ASSUME_NONNULL_BEGIN @property (nonatomic, readonly) id storageServiceManager; @property (nonatomic, readonly) id groupsV2; @property (nonatomic, readonly) id groupV2Updates; - @property (nonatomic, readonly) StickerManager *stickerManager; @property (nonatomic, readonly) SDSDatabaseStorage *databaseStorage; @property (nonatomic, readonly) StorageCoordinator *storageCoordinator; @property (nonatomic, readonly) SSKPreferences *sskPreferences; @property (nonatomic, readonly) MessageProcessing *messageProcessing; @property (nonatomic, readonly) MessageFetcherJob *messageFetcherJob; +@property (nonatomic, readonly) BulkProfileFetch *bulkProfileFetch; @property (nonatomic, readonly, nullable) OWSPrimaryStorage *primaryStorage; diff --git a/SignalServiceKit/src/SSKEnvironment.m b/SignalServiceKit/src/SSKEnvironment.m index 4b73b0214a..48cc9b3a1b 100644 --- a/SignalServiceKit/src/SSKEnvironment.m +++ b/SignalServiceKit/src/SSKEnvironment.m @@ -49,6 +49,7 @@ static SSKEnvironment *sharedSSKEnvironment; @property (nonatomic) id groupV2Updates; @property (nonatomic) MessageProcessing *messageProcessing; @property (nonatomic) MessageFetcherJob *messageFetcherJob; +@property (nonatomic) BulkProfileFetch *bulkProfileFetch; @end @@ -103,6 +104,7 @@ static SSKEnvironment *sharedSSKEnvironment; groupV2Updates:(id)groupV2Updates messageProcessing:(MessageProcessing *)messageProcessing messageFetcherJob:(MessageFetcherJob *)messageFetcherJob + bulkProfileFetch:(BulkProfileFetch *)bulkProfileFetch { self = [super init]; if (!self) { @@ -150,7 +152,8 @@ static SSKEnvironment *sharedSSKEnvironment; OWSAssertDebug(groupsV2); OWSAssertDebug(groupV2Updates); OWSAssertDebug(messageProcessing); - OWSCAssertDebug(messageFetcherJob); + OWSAssertDebug(messageFetcherJob); + OWSAssertDebug(bulkProfileFetch); _contactsManager = contactsManager; _linkPreviewManager = linkPreviewManager; @@ -195,6 +198,7 @@ static SSKEnvironment *sharedSSKEnvironment; _groupV2Updates = groupV2Updates; _messageProcessing = messageProcessing; _messageFetcherJob = messageFetcherJob; + _bulkProfileFetch = bulkProfileFetch; return self; } diff --git a/SignalServiceKit/src/TestUtils/MockSSKEnvironment.m b/SignalServiceKit/src/TestUtils/MockSSKEnvironment.m index 22a7c626c4..5f008a9bf1 100644 --- a/SignalServiceKit/src/TestUtils/MockSSKEnvironment.m +++ b/SignalServiceKit/src/TestUtils/MockSSKEnvironment.m @@ -104,6 +104,7 @@ NS_ASSUME_NONNULL_BEGIN id groupV2Updates = [[MockGroupV2Updates alloc] init]; MessageProcessing *messageProcessing = [MessageProcessing new]; MessageFetcherJob *messageFetcherJob = [MessageFetcherJob new]; + BulkProfileFetch *bulkProfileFetch = [BulkProfileFetch new]; self = [super initWithContactsManager:contactsManager linkPreviewManager:linkPreviewManager @@ -147,7 +148,8 @@ NS_ASSUME_NONNULL_BEGIN groupsV2:groupsV2 groupV2Updates:groupV2Updates messageProcessing:messageProcessing - messageFetcherJob:messageFetcherJob]; + messageFetcherJob:messageFetcherJob + bulkProfileFetch:bulkProfileFetch]; if (!self) { return nil; diff --git a/SignalServiceKit/src/TestUtils/OWSFakeProfileManager.m b/SignalServiceKit/src/TestUtils/OWSFakeProfileManager.m index 07c499b392..7743c7fe3a 100644 --- a/SignalServiceKit/src/TestUtils/OWSFakeProfileManager.m +++ b/SignalServiceKit/src/TestUtils/OWSFakeProfileManager.m @@ -216,6 +216,13 @@ NS_ASSUME_NONNULL_BEGIN return [AnyPromise promiseWithValue:@(1)]; } +- (AnyPromise *)updateProfileForAddressPromise:(SignalServiceAddress *)address + mainAppOnly:(BOOL)mainAppOnly + ignoreThrottling:(BOOL)ignoreThrottling +{ + return [AnyPromise promiseWithValue:@(1)]; +} + - (void)warmCaches { // Do nothing. diff --git a/SignalServiceKit/src/Util/BulkProfileFetch.swift b/SignalServiceKit/src/Util/BulkProfileFetch.swift new file mode 100644 index 0000000000..d139d9fd71 --- /dev/null +++ b/SignalServiceKit/src/Util/BulkProfileFetch.swift @@ -0,0 +1,230 @@ +// +// Copyright (c) 2020 Open Whisper Systems. All rights reserved. +// + +import PromiseKit + +@objc +public class BulkProfileFetch: NSObject { + + // MARK: - Dependencies + + private var profileManager: ProfileManagerProtocol { + return SSKEnvironment.shared.profileManager + } + + private var tsAccountManager: TSAccountManager { + return .sharedInstance() + } + + private var reachabilityManager: SSKReachabilityManager { + return SSKEnvironment.shared.reachabilityManager + } + + // MARK: - + + private let serialQueue = DispatchQueue(label: "BulkProfileFetch") + + // This property should only be accessed on serialQueue. + private var addressQueue = OrderedSet() + + // This property should only be accessed on serialQueue. + private var isUpdateInFlight = false + + private enum UpdateOutcome { + case networkFailure(date: Date) + case retryLimit(date: Date) + case noProfile(date: Date) + case serviceError(date: Date) + case success(date: Date) + } + + // This property should only be accessed on serialQueue. + private var lastOutcomeMap = [SignalServiceAddress: UpdateOutcome]() + + // This property should only be accessed on serialQueue. + private var lastRateLimitErrorDate: Date? + + @objc + public required override init() { + super.init() + + SwiftSingletons.register(self) + + AppReadiness.runNowOrWhenAppDidBecomeReady { + // TODO: There would be benefit to trying to update + // missing & stale profiles on launch. + } + + observeNotifications() + } + + private func observeNotifications() { + NotificationCenter.default.addObserver(forName: SSKReachability.owsReachabilityDidChange, + object: nil, queue: nil) { [weak self] _ in + guard let self = self else { return } + self.serialQueue.async { + self.process() + } + } + NotificationCenter.default.addObserver(forName: .registrationStateDidChange, object: nil, queue: nil) { [weak self] _ in + guard let self = self else { return } + self.serialQueue.async { + self.process() + } + } + } + + // This should be used for non-urgent profile updates. + @objc + public func fetchAndUpdateProfiles(thread: TSThread) { + fetchAndUpdateProfiles(addresses: thread.recipientAddresses) + } + + // This should be used for non-urgent profile updates. + @objc + public func fetchAndUpdateProfile(address: SignalServiceAddress) { + fetchAndUpdateProfiles(addresses: [address]) + } + + // This should be used for non-urgent profile updates. + @objc + public func fetchAndUpdateProfiles(addresses: [SignalServiceAddress]) { + serialQueue.async { + guard let localAddress = self.tsAccountManager.localAddress else { + owsFailDebug("missing local address") + return + } + for address in addresses { + guard address != localAddress else { + continue + } + guard !self.addressQueue.contains(address) else { + continue + } + self.addressQueue.append(address) + } + self.process() + } + } + + private func process() { + assertOnQueue(serialQueue) + + guard CurrentAppContext().isMainApp else { + return + } + guard reachabilityManager.isReachable else { + return + } + guard tsAccountManager.isRegisteredAndReady else { + return + } + + // Only one update in flight at a time. + guard !self.isUpdateInFlight else { + return + } + + // Dequeue. + guard let address = self.addressQueue.first else { + return + } + self.addressQueue.remove(address) + + // De-bounce. + guard self.shouldUpdateAddress(address) else { + return + } + + Logger.verbose("Updating: \(address)") + + // Perform update. + isUpdateInFlight = true + // We need to throttle these jobs. + // Always wait N seconds between update jobs. + let updateDelaySeconds: TimeInterval = 3 + firstly { + self.profileManager.updateProfile(forAddressPromise: address, + mainAppOnly: true, + ignoreThrottling: false).asVoid() + }.done { + self.serialQueue.asyncAfter(deadline: DispatchTime.now() + updateDelaySeconds) { + self.isUpdateInFlight = false + self.lastOutcomeMap[address] = .success(date: Date()) + self.process() + } + }.catch { error in + self.serialQueue.asyncAfter(deadline: DispatchTime.now() + updateDelaySeconds) { + self.isUpdateInFlight = false + let now = Date() + switch error { + default: + if IsNetworkConnectivityFailure(error) { + Logger.warn("Error: \(error)") + self.lastOutcomeMap[address] = .networkFailure(date: now) + } else if error.httpStatusCode == 413 { + Logger.error("Error: \(error)") + self.lastOutcomeMap[address] = .retryLimit(date: now) + self.lastRateLimitErrorDate = now + } else if error.httpStatusCode == 404 { + Logger.error("Error: \(error)") + self.lastOutcomeMap[address] = .noProfile(date: now) + } else { + // TODO: We may need to handle more status codes. + owsFailDebug("Error: \(error)") + self.lastOutcomeMap[address] = .serviceError(date: now) + } + } +// public enum ProfileFetchError: Int, Error { +// case missing +// case throttled +// case notMainApp +// case cantRequestVersionedProfile +// } + + self.process() + } + }.retainUntilComplete() + } + + private func shouldUpdateAddress(_ address: SignalServiceAddress) -> Bool { + assertOnQueue(serialQueue) + + // Skip if we've recently had a rate limit error. + if let lastRateLimitErrorDate = self.lastRateLimitErrorDate { + let minElapsedSeconds = 5 * kMinuteInterval + let elapsedSeconds = lastRateLimitErrorDate.timeIntervalSinceNow + guard elapsedSeconds >= minElapsedSeconds else { + return false + } + } + + guard let lastOutcome = lastOutcomeMap[address] else { + return true + } + + let minElapsedSeconds: TimeInterval + let elapsedSeconds: TimeInterval + + switch lastOutcome { + case .networkFailure(let date): + minElapsedSeconds = 1 * kMinuteInterval + elapsedSeconds = date.timeIntervalSinceNow + case .retryLimit(let date): + minElapsedSeconds = 5 * kMinuteInterval + elapsedSeconds = date.timeIntervalSinceNow + case .noProfile(let date): + minElapsedSeconds = 6 * kHourInterval + elapsedSeconds = date.timeIntervalSinceNow + case .serviceError(let date): + minElapsedSeconds = 30 * kMinuteInterval + elapsedSeconds = date.timeIntervalSinceNow + case .success(let date): + minElapsedSeconds = 15 * kMinuteInterval + elapsedSeconds = date.timeIntervalSinceNow + } + + return elapsedSeconds >= minElapsedSeconds + } +} diff --git a/SignalServiceKit/src/Util/OrderedSet.swift b/SignalServiceKit/src/Util/OrderedSet.swift index d1cf8d819f..040eb79ce5 100644 --- a/SignalServiceKit/src/Util/OrderedSet.swift +++ b/SignalServiceKit/src/Util/OrderedSet.swift @@ -45,6 +45,10 @@ public struct OrderedSet where Element: Hashable { return set.count } + public var first: Element? { + return orderedMembers.first + } + // TODO: I only implemented the minimum API that I needed. There's lots more that could // reasonably be added to an OrderedSet (union, etc.) } diff --git a/SignalServiceKit/src/groups/GroupManager.swift b/SignalServiceKit/src/groups/GroupManager.swift index 20cb303f07..c1fe0255b8 100644 --- a/SignalServiceKit/src/groups/GroupManager.swift +++ b/SignalServiceKit/src/groups/GroupManager.swift @@ -72,6 +72,10 @@ public class GroupManager: NSObject { return SSKEnvironment.shared.messageProcessing } + private class var bulkProfileFetch: BulkProfileFetch { + return SSKEnvironment.shared.bulkProfileFetch + } + // MARK: - // Never instantiate this class. @@ -222,7 +226,7 @@ public class GroupManager: NSObject { }.then(on: .global()) { () -> Promise in var memberSet = Set(membersParam) memberSet.insert(localAddress) - return self.tryToEnableGroupsV2(for: Array(memberSet), ignoreErrors: true) + return self.tryToEnableGroupsV2(for: Array(memberSet), isBlocking: true, ignoreErrors: true) }.map(on: .global()) { () throws -> GroupMembership in // Build member list. // @@ -719,7 +723,7 @@ public class GroupManager: NSObject { guard RemoteConfig.groupsV2CreateGroups else { return Promise.value(()) } - return self.tryToEnableGroupsV2(for: Array(proposedGroupModel.groupMembership.allUsers), ignoreErrors: true) + return self.tryToEnableGroupsV2(for: Array(proposedGroupModel.groupMembership.allUsers), isBlocking: true, ignoreErrors: true) }.then(on: .global()) { () -> Promise in return self.ensureLocalProfileHasCommitmentIfNecessary() }.then(on: DispatchQueue.global()) { () -> Promise in @@ -1197,8 +1201,9 @@ public class GroupManager: NSObject { // MARK: - UUIDs public static func tryToEnableGroupsV2(for addresses: [SignalServiceAddress], + isBlocking: Bool, ignoreErrors: Bool) -> Promise { - let promise = tryToEnableGroupsV2(for: addresses) + let promise = tryToEnableGroupsV2(for: addresses, isBlocking: isBlocking) if ignoreErrors { return promise.recover { error -> Guarantee in Logger.warn("Error: \(error).") @@ -1209,7 +1214,8 @@ public class GroupManager: NSObject { } } - private static func tryToEnableGroupsV2(for addresses: [SignalServiceAddress]) -> Promise { + private static func tryToEnableGroupsV2(for addresses: [SignalServiceAddress], + isBlocking: Bool) -> Promise { return firstly { () -> Promise in for address in addresses { guard address.isValid else { @@ -1233,11 +1239,18 @@ public class GroupManager: NSObject { guard !addressesWithoutCapability.isEmpty else { return Promise.value(()) } - var promises = [Promise]() - for address in addressesWithoutCapability { - promises.append(self.profileManager.updateProfile(forAddressPromise: address).asVoid()) + if isBlocking { + // Block on the outcome of the profile updates. + var promises = [Promise]() + for address in addressesWithoutCapability { + promises.append(self.profileManager.updateProfile(forAddressPromise: address).asVoid()) + } + return when(fulfilled: promises) + } else { + // This will throttle, de-bounce, etc. + self.bulkProfileFetch.fetchAndUpdateProfiles(addresses: addressesWithoutCapability) + return Promise.value(()) } - return when(fulfilled: promises) } } From c38929dae5b2bb2080bbcae34329d89d6f8f2c43 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Tue, 14 Apr 2020 16:47:20 -0300 Subject: [PATCH 05/13] Avoid rate limits while doing profile fetches. --- Signal.xcodeproj/project.pbxproj | 20 ++- SignalMessaging/environment/AppSetup.m | 4 +- .../migrations/YDBToGRDBMigration.swift | 2 +- SignalMessaging/groups/GroupsV2Impl.swift | 20 +-- SignalMessaging/profiles/OWSProfileManager.h | 6 - SignalMessaging/profiles/OWSProfileManager.m | 9 +- .../profiles/OWSProfileManager.swift | 126 ++++++++++-------- ...iles.swift => VersionedProfilesImpl.swift} | 105 +++++++-------- .../src/Protocols/ProfileManagerProtocol.h | 6 + SignalServiceKit/src/SSKEnvironment.h | 5 +- SignalServiceKit/src/SSKEnvironment.m | 4 + .../src/TestUtils/MockSSKEnvironment.m | 4 +- .../src/TestUtils/OWSFakeProfileManager.m | 9 ++ .../{ => Profiles}/BulkProfileFetch.swift | 19 ++- .../Util/Profiles}/ProfileFetcherJob.swift | 46 +++++-- .../src/Util/Profiles/VersionedProfiles.swift | 76 +++++++++++ 16 files changed, 297 insertions(+), 164 deletions(-) rename SignalMessaging/profiles/{VersionedProfiles.swift => VersionedProfilesImpl.swift} (75%) rename SignalServiceKit/src/Util/{ => Profiles}/BulkProfileFetch.swift (91%) rename {SignalMessaging/profiles => SignalServiceKit/src/Util/Profiles}/ProfileFetcherJob.swift (93%) create mode 100644 SignalServiceKit/src/Util/Profiles/VersionedProfiles.swift diff --git a/Signal.xcodeproj/project.pbxproj b/Signal.xcodeproj/project.pbxproj index 74fd553048..d8e36df75d 100644 --- a/Signal.xcodeproj/project.pbxproj +++ b/Signal.xcodeproj/project.pbxproj @@ -132,7 +132,6 @@ 346129AF1FD1F5D900532771 /* SystemContactsFetcher.swift in Sources */ = {isa = PBXBuildFile; fileRef = 346129AE1FD1F5D900532771 /* SystemContactsFetcher.swift */; }; 346129B41FD1F7E800532771 /* OWSProfileManager.h in Headers */ = {isa = PBXBuildFile; fileRef = 346129B11FD1F7E800532771 /* OWSProfileManager.h */; settings = {ATTRIBUTES = (Public, ); }; }; 346129B51FD1F7E800532771 /* OWSProfileManager.m in Sources */ = {isa = PBXBuildFile; fileRef = 346129B21FD1F7E800532771 /* OWSProfileManager.m */; }; - 346129B61FD1F7E800532771 /* ProfileFetcherJob.swift in Sources */ = {isa = PBXBuildFile; fileRef = 346129B31FD1F7E800532771 /* ProfileFetcherJob.swift */; }; 346129D51FD20ADC00532771 /* UIViewController+OWS.m in Sources */ = {isa = PBXBuildFile; fileRef = 346129D31FD20ADB00532771 /* UIViewController+OWS.m */; }; 346129D61FD20ADC00532771 /* UIViewController+OWS.h in Headers */ = {isa = PBXBuildFile; fileRef = 346129D41FD20ADC00532771 /* UIViewController+OWS.h */; settings = {ATTRIBUTES = (Public, ); }; }; 346129E21FD5C0BE00532771 /* VersionMigrations.h in Headers */ = {isa = PBXBuildFile; fileRef = 346129E01FD5C0BE00532771 /* VersionMigrations.h */; settings = {ATTRIBUTES = (Public, ); }; }; @@ -177,7 +176,7 @@ 346E9D5421B040B700562252 /* RegistrationController.swift in Sources */ = {isa = PBXBuildFile; fileRef = 346E9D5321B040B600562252 /* RegistrationController.swift */; }; 3470249C238367EA0078D72C /* TextApprovalViewController.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3470249B238367E90078D72C /* TextApprovalViewController.swift */; }; 3470249E2385B6360078D72C /* OWSProfileManager.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3470249D2385B6360078D72C /* OWSProfileManager.swift */; }; - 347024A0238C85850078D72C /* VersionedProfiles.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3470249F238C85850078D72C /* VersionedProfiles.swift */; }; + 347024A0238C85850078D72C /* VersionedProfilesImpl.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3470249F238C85850078D72C /* VersionedProfilesImpl.swift */; }; 347137FC22A1BCE800F43A63 /* OWSMessageViewOnceView.m in Sources */ = {isa = PBXBuildFile; fileRef = 347137FB22A1BCE800F43A63 /* OWSMessageViewOnceView.m */; }; 347191F923F457BD003A3106 /* GroupsV2AvatarDownloadOperation.swift in Sources */ = {isa = PBXBuildFile; fileRef = 347191F823F457BD003A3106 /* GroupsV2AvatarDownloadOperation.swift */; }; 3476236224081F8900D4FB6F /* GroupManager+SignalMessaging.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3476236124081F8900D4FB6F /* GroupManager+SignalMessaging.swift */; }; @@ -1007,7 +1006,6 @@ 346129AE1FD1F5D900532771 /* SystemContactsFetcher.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = SystemContactsFetcher.swift; sourceTree = ""; }; 346129B11FD1F7E800532771 /* OWSProfileManager.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = OWSProfileManager.h; sourceTree = ""; }; 346129B21FD1F7E800532771 /* OWSProfileManager.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = OWSProfileManager.m; sourceTree = ""; }; - 346129B31FD1F7E800532771 /* ProfileFetcherJob.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ProfileFetcherJob.swift; sourceTree = ""; }; 346129BD1FD2068600532771 /* ThreadUtil.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = ThreadUtil.h; sourceTree = ""; }; 346129BE1FD2068600532771 /* ThreadUtil.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = ThreadUtil.m; sourceTree = ""; }; 346129D31FD20ADB00532771 /* UIViewController+OWS.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = "UIViewController+OWS.m"; sourceTree = ""; }; @@ -1057,7 +1055,7 @@ 346E9D5321B040B600562252 /* RegistrationController.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = RegistrationController.swift; sourceTree = ""; }; 3470249B238367E90078D72C /* TextApprovalViewController.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = TextApprovalViewController.swift; sourceTree = ""; }; 3470249D2385B6360078D72C /* OWSProfileManager.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = OWSProfileManager.swift; sourceTree = ""; }; - 3470249F238C85850078D72C /* VersionedProfiles.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = VersionedProfiles.swift; sourceTree = ""; }; + 3470249F238C85850078D72C /* VersionedProfilesImpl.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = VersionedProfilesImpl.swift; sourceTree = ""; }; 347137FA22A1BCE800F43A63 /* OWSMessageViewOnceView.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = OWSMessageViewOnceView.h; sourceTree = ""; }; 347137FB22A1BCE800F43A63 /* OWSMessageViewOnceView.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = OWSMessageViewOnceView.m; sourceTree = ""; }; 347191F823F457BD003A3106 /* GroupsV2AvatarDownloadOperation.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = GroupsV2AvatarDownloadOperation.swift; sourceTree = ""; }; @@ -2235,8 +2233,7 @@ 346129B11FD1F7E800532771 /* OWSProfileManager.h */, 346129B21FD1F7E800532771 /* OWSProfileManager.m */, 3470249D2385B6360078D72C /* OWSProfileManager.swift */, - 346129B31FD1F7E800532771 /* ProfileFetcherJob.swift */, - 3470249F238C85850078D72C /* VersionedProfiles.swift */, + 3470249F238C85850078D72C /* VersionedProfilesImpl.swift */, ); path = profiles; sourceTree = ""; @@ -2750,12 +2747,12 @@ 454A96571FD600B4008D2A0E /* attachments */ = { isa = PBXGroup; children = ( + 8809CE8422F8DB2D00D38867 /* AttachmentKeyboard.swift */, 34B3F8391E8DF1700035BE1A /* AttachmentSharing.h */, 34B3F83A1E8DF1700035BE1A /* AttachmentSharing.m */, - 34D913491F62D4A500722898 /* SignalAttachment.swift */, - 45BC829C1FD9C4B400011CF3 /* ShareViewDelegate.swift */, 45F59A092029140500E8D2B0 /* OWSVideoPlayer.swift */, - 8809CE8422F8DB2D00D38867 /* AttachmentKeyboard.swift */, + 45BC829C1FD9C4B400011CF3 /* ShareViewDelegate.swift */, + 34D913491F62D4A500722898 /* SignalAttachment.swift */, ); path = attachments; sourceTree = ""; @@ -2765,9 +2762,9 @@ children = ( 451166BF1FD86B98000739BA /* AccountManager.swift */, 45DF5DF11DDB843F00C936C7 /* CompareSafetyNumbersActivity.swift */, + 4CB5F26820F7D060004D1B42 /* MessageActions.swift */, 458E38351D668EBF0094BD24 /* OWSDeviceProvisioningURLParser.h */, 458E38361D668EBF0094BD24 /* OWSDeviceProvisioningURLParser.m */, - 4CB5F26820F7D060004D1B42 /* MessageActions.swift */, 4C5250D121E7BD7D00CE3D95 /* PhoneNumberValidator.swift */, 4C0C36F7226647FE0083F19A /* ThreadMapping.swift */, ); @@ -4392,7 +4389,6 @@ 34AC0A10211B39EA00997B47 /* TappableView.swift in Sources */, 346129F91FD5F31400532771 /* OWS104CreateRecipientIdentities.m in Sources */, 34C2EEAC2270A69400BCA1D0 /* CircleView.swift in Sources */, - 346129B61FD1F7E800532771 /* ProfileFetcherJob.swift in Sources */, 34AC09E9211B39B100997B47 /* OWSTableViewController.m in Sources */, 340872CE2239596100CB25B0 /* AttachmentApprovalInputAccessoryView.swift in Sources */, 346129F51FD5F31400532771 /* OWS102MoveLoggingPreferenceToUserDefaults.m in Sources */, @@ -4464,7 +4460,7 @@ 34C529092361D3540009DB2A /* ApprovalFooterView.swift in Sources */, 451F8A471FD715BA005CB9DA /* OWSAvatarBuilder.m in Sources */, 349ED992221EE80D008045B0 /* AppPreferences.swift in Sources */, - 347024A0238C85850078D72C /* VersionedProfiles.swift in Sources */, + 347024A0238C85850078D72C /* VersionedProfilesImpl.swift in Sources */, 4C9C50FE22F36FA50054A33F /* OutboundMessage+OWS.swift in Sources */, 34480B591FD0A7A400BC14EF /* OWSScrubbingLogFormatter.m in Sources */, 342BF24D227A03F8005CEAA9 /* StickerHorizontalListView.swift in Sources */, diff --git a/SignalMessaging/environment/AppSetup.m b/SignalMessaging/environment/AppSetup.m index 04017fabfb..0e5d30ef21 100644 --- a/SignalMessaging/environment/AppSetup.m +++ b/SignalMessaging/environment/AppSetup.m @@ -114,6 +114,7 @@ NS_ASSUME_NONNULL_BEGIN MessageProcessing *messageProcessing = [MessageProcessing new]; MessageFetcherJob *messageFetcherJob = [MessageFetcherJob new]; BulkProfileFetch *bulkProfileFetch = [BulkProfileFetch new]; + id versionedProfiles = [VersionedProfilesImpl new]; [Environment setShared:[[Environment alloc] initWithAudioSession:audioSession incomingContactSyncJobQueue:incomingContactSyncJobQueue @@ -169,7 +170,8 @@ NS_ASSUME_NONNULL_BEGIN groupV2Updates:groupV2Updates messageProcessing:messageProcessing messageFetcherJob:messageFetcherJob - bulkProfileFetch:bulkProfileFetch]]; + bulkProfileFetch:bulkProfileFetch + versionedProfiles:versionedProfiles]]; appSpecificSingletonBlock(); diff --git a/SignalMessaging/environment/migrations/YDBToGRDBMigration.swift b/SignalMessaging/environment/migrations/YDBToGRDBMigration.swift index 81ef5987ec..d7e8e4f148 100644 --- a/SignalMessaging/environment/migrations/YDBToGRDBMigration.swift +++ b/SignalMessaging/environment/migrations/YDBToGRDBMigration.swift @@ -299,7 +299,7 @@ extension YDBToGRDBMigration { GRDBKeyValueStoreMigrator(label: "OWSSyncManager", keyStore: OWSSyncManager.keyValueStore(), ydbTransaction: ydbTransaction), GRDBKeyValueStoreMigrator(label: "OWSOutgoingReceiptManager.deliveryReceiptStore", keyStore: OWSOutgoingReceiptManager.deliveryReceiptStore(), ydbTransaction: ydbTransaction), GRDBKeyValueStoreMigrator(label: "OWSOutgoingReceiptManager.readReceiptStore", keyStore: OWSOutgoingReceiptManager.readReceiptStore(), ydbTransaction: ydbTransaction), - GRDBKeyValueStoreMigrator(label: "VersionedProfiles.credentialStore", keyStore: VersionedProfiles.credentialStore, ydbTransaction: ydbTransaction), + GRDBKeyValueStoreMigrator(label: "VersionedProfiles.credentialStore", keyStore: VersionedProfilesImpl.credentialStore, ydbTransaction: ydbTransaction), GRDBKeyValueStoreMigrator(label: "KeyBackupService.keyValueStore", keyStore: KeyBackupService.keyValueStore, ydbTransaction: ydbTransaction), GRDBKeyValueStoreMigrator(label: "KeyBackupService.tokenStore", keyStore: KeyBackupService.tokenStore, ydbTransaction: ydbTransaction), GRDBKeyValueStoreMigrator(label: "StorageServiceOperation", keyStore: StorageServiceOperation.keyValueStore, ydbTransaction: ydbTransaction) diff --git a/SignalMessaging/groups/GroupsV2Impl.swift b/SignalMessaging/groups/GroupsV2Impl.swift index 032dc4956a..4b61f7dace 100644 --- a/SignalMessaging/groups/GroupsV2Impl.swift +++ b/SignalMessaging/groups/GroupsV2Impl.swift @@ -45,6 +45,10 @@ public class GroupsV2Impl: NSObject, GroupsV2Swift { return SSKEnvironment.shared.contactsUpdater } + private var versionedProfiles: VersionedProfilesImpl { + return SSKEnvironment.shared.versionedProfiles as! VersionedProfilesImpl + } + // MARK: - public typealias ProfileKeyCredentialMap = [UUID: ProfileKeyCredential] @@ -110,7 +114,7 @@ public class GroupsV2Impl: NSObject, GroupsV2Swift { databaseStorage.write { transaction in self.clearTemporalCredentials(transaction: transaction) - VersionedProfiles.clearProfileKeyCredentials(transaction: transaction) + self.versionedProfiles.clearProfileKeyCredentials(transaction: transaction) self.serviceStore.setString(serverPublicParamsBase64, key: lastServerPublicParamsKey, transaction: transaction) self.serviceStore.setInt(zkgroupVersionCounter, key: lastZKgroupVersionCounterKey, transaction: transaction) } @@ -929,8 +933,8 @@ public class GroupsV2Impl: NSObject, GroupsV2Swift { for uuid in Set(uuids) { do { let address = SignalServiceAddress(uuid: uuid) - if let credential = try VersionedProfiles.profileKeyCredential(for: address, - transaction: transaction) { + if let credential = try self.versionedProfiles.profileKeyCredential(for: address, + transaction: transaction) { credentialMap[uuid] = credential continue } @@ -973,9 +977,9 @@ public class GroupsV2Impl: NSObject, GroupsV2Swift { try self.databaseStorage.read { transaction in for uuid in uuids { let address = SignalServiceAddress(uuid: uuid) - guard let credential = try VersionedProfiles.profileKeyCredential(for: address, - transaction: transaction) else { - throw OWSAssertionError("Could load credential.") + guard let credential = try self.versionedProfiles.profileKeyCredential(for: address, + transaction: transaction) else { + throw OWSAssertionError("Could load credential.") } credentialMap[uuid] = credential } @@ -988,8 +992,8 @@ public class GroupsV2Impl: NSObject, GroupsV2Swift { public func hasProfileKeyCredential(for address: SignalServiceAddress, transaction: SDSAnyReadTransaction) -> Bool { do { - return try VersionedProfiles.profileKeyCredential(for: address, - transaction: transaction) != nil + return try self.versionedProfiles.profileKeyCredential(for: address, + transaction: transaction) != nil } catch { owsFailDebug("Error: \(error)") return false diff --git a/SignalMessaging/profiles/OWSProfileManager.h b/SignalMessaging/profiles/OWSProfileManager.h index b75e689976..7fbf75fe94 100644 --- a/SignalMessaging/profiles/OWSProfileManager.h +++ b/SignalMessaging/profiles/OWSProfileManager.h @@ -139,12 +139,6 @@ typedef void (^ProfileManagerFailureBlock)(NSError *error); - (nullable NSString *)usernameForAddress:(SignalServiceAddress *)address transaction:(SDSAnyReadTransaction *)transaction; -- (void)updateProfileForAddress:(SignalServiceAddress *)address - profileNameEncrypted:(nullable NSData *)profileNameEncrypted - username:(nullable NSString *)username - isUuidCapable:(BOOL)isUuidCapable - avatarUrlPath:(nullable NSString *)avatarUrlPath; - #pragma mark - Clean Up + (NSSet *)allProfileAvatarFilePathsWithTransaction:(SDSAnyReadTransaction *)transaction; diff --git a/SignalMessaging/profiles/OWSProfileManager.m b/SignalMessaging/profiles/OWSProfileManager.m index 288c206a15..22e770ce4b 100644 --- a/SignalMessaging/profiles/OWSProfileManager.m +++ b/SignalMessaging/profiles/OWSProfileManager.m @@ -77,6 +77,11 @@ const NSString *kNSNotificationKey_WasLocallyInitiated = @"kNSNotificationKey_Wa return SSKEnvironment.shared.storageServiceManager; } +- (id)versionedProfiles +{ + return SSKEnvironment.shared.versionedProfiles; +} + #pragma mark - @synthesize localUserProfile = _localUserProfile; @@ -665,7 +670,7 @@ const NSString *kNSNotificationKey_WasLocallyInitiated = @"kNSNotificationKey_Wa // Whenever a user's profile key changes, we need to fetch a new // profile key credential for them. - [VersionedProfiles clearProfileKeyCredentialForAddress:localAddress transaction:transaction]; + [self.versionedProfiles clearProfileKeyCredentialForAddress:localAddress transaction:transaction]; // We schedule the updates here but process them below using processProfileKeyUpdates. // It's more efficient to process them after the intermediary steps are done. @@ -1391,7 +1396,7 @@ const NSString *kNSNotificationKey_WasLocallyInitiated = @"kNSNotificationKey_Wa // Whenever a user's profile key changes, we need to fetch a new // profile key credential for them. - [VersionedProfiles clearProfileKeyCredentialForAddress:address transaction:transaction]; + [self.versionedProfiles clearProfileKeyCredentialForAddress:address transaction:transaction]; [userProfile clearWithProfileKey:profileKey wasLocallyInitiated:wasLocallyInitiated diff --git a/SignalMessaging/profiles/OWSProfileManager.swift b/SignalMessaging/profiles/OWSProfileManager.swift index 996e9e84fa..904e22f732 100644 --- a/SignalMessaging/profiles/OWSProfileManager.swift +++ b/SignalMessaging/profiles/OWSProfileManager.swift @@ -8,6 +8,14 @@ import SignalServiceKit public extension OWSProfileManager { + // MARK: - Dependencies + + private class var versionedProfiles: VersionedProfilesSwift { + return SSKEnvironment.shared.versionedProfiles as! VersionedProfilesSwift + } + + // MARK: - + // The main entry point for updating the local profile. It will: // // * Update local state optimistically. @@ -22,15 +30,15 @@ public extension OWSProfileManager { return DispatchQueue.global().async(.promise) { return enqueueProfileUpdate(profileGivenName: profileGivenName, profileFamilyName: profileFamilyName, profileAvatarData: profileAvatarData) - }.then { update in - return self.attemptToUpdateProfileOnService(update: update) - }.then { (_) throws -> Promise in - guard let localAddress = TSAccountManager.sharedInstance().localAddress else { - throw OWSAssertionError("missing local address") - } - return ProfileFetcherJob.fetchAndUpdateProfilePromise(address: localAddress, mainAppOnly: false, ignoreThrottling: true, fetchType: .default).asVoid() - }.done(on: .global()) { () -> Void in - Logger.verbose("Profile update did complete.") + }.then { update in + return self.attemptToUpdateProfileOnService(update: update) + }.then { (_) throws -> Promise in + guard let localAddress = TSAccountManager.sharedInstance().localAddress else { + throw OWSAssertionError("missing local address") + } + return ProfileFetcherJob.fetchAndUpdateProfilePromise(address: localAddress, mainAppOnly: false, ignoreThrottling: true, fetchType: .default).asVoid() + }.done(on: .global()) { () -> Void in + Logger.verbose("Profile update did complete.") } } @@ -113,9 +121,9 @@ extension OWSProfileManager { retryDelay: retryDelay) .done { _ in Logger.info("Update succeeded.") - }.catch { error in - Logger.error("Update failed: \(error)") - }.retainUntilComplete() + }.catch { error in + Logger.error("Update failed: \(error)") + }.retainUntilComplete() } fileprivate class func attemptToUpdateProfileOnService(update: PendingProfileUpdate, @@ -145,48 +153,48 @@ extension OWSProfileManager { } else { return updateProfileOnServiceUnversioned(attempt: attempt) } - }.done(on: DispatchQueue.global()) { _ in - _ = self.databaseStorage.write { (transaction: SDSAnyWriteTransaction) -> Void in - guard tryToDequeueProfileUpdate(update: attempt.update, transaction: transaction) else { - return - } - - if attempt.update.profileAvatarData != nil { - if attempt.avatarFilename == nil { - owsFailDebug("Missing avatarFilename.") - } - if attempt.avatarUrlPath == nil { - owsFailDebug("Missing avatarUrlPath.") - } - } - - self.updateLocalProfile(with: attempt, transaction: transaction) + }.done(on: DispatchQueue.global()) { _ in + _ = self.databaseStorage.write { (transaction: SDSAnyWriteTransaction) -> Void in + guard tryToDequeueProfileUpdate(update: attempt.update, transaction: transaction) else { + return } - self.attemptDidComplete(retryDelay: retryDelay, didSucceed: true) - }.recover(on: .global()) { error in - // We retry network errors forever (with exponential backoff). - // Other errors cause us to give up immediately. - // Note that we only ever retry the latest profile update. - if IsNetworkConnectivityFailure(error) { - Logger.warn("Retrying after error: \(error)") - } else { - owsFailDebug("Error: \(error)") - - // Dequeue to avoid getting stuck in retry loop. - self.databaseStorage.write { transaction in - _ = tryToDequeueProfileUpdate(update: attempt.update, transaction: transaction) + if attempt.update.profileAvatarData != nil { + if attempt.avatarFilename == nil { + owsFailDebug("Missing avatarFilename.") + } + if attempt.avatarUrlPath == nil { + owsFailDebug("Missing avatarUrlPath.") } } - self.attemptDidComplete(retryDelay: retryDelay, didSucceed: false) - // We don't actually want to recover; in this block we - // handle the business logic consequences of the error, - // but we re-throw so that the UI can distinguish - // success and failure. - throw error + self.updateLocalProfile(with: attempt, transaction: transaction) } + self.attemptDidComplete(retryDelay: retryDelay, didSucceed: true) + }.recover(on: .global()) { error in + // We retry network errors forever (with exponential backoff). + // Other errors cause us to give up immediately. + // Note that we only ever retry the latest profile update. + if IsNetworkConnectivityFailure(error) { + Logger.warn("Retrying after error: \(error)") + } else { + owsFailDebug("Error: \(error)") + + // Dequeue to avoid getting stuck in retry loop. + self.databaseStorage.write { transaction in + _ = tryToDequeueProfileUpdate(update: attempt.update, transaction: transaction) + } + } + self.attemptDidComplete(retryDelay: retryDelay, didSucceed: false) + + // We don't actually want to recover; in this block we + // handle the business logic consequences of the error, + // but we re-throw so that the UI can distinguish + // success and failure. + throw error + } + return promise } @@ -277,9 +285,9 @@ extension OWSProfileManager { } private class func updateProfileOnServiceVersioned(attempt: ProfileUpdateAttempt) -> Promise { - return VersionedProfiles.updateProfilePromise(profileGivenName: attempt.update.profileGivenName, - profileFamilyName: attempt.update.profileFamilyName, - profileAvatarData: attempt.update.profileAvatarData) + return self.versionedProfiles.updateProfilePromise(profileGivenName: attempt.update.profileGivenName, + profileFamilyName: attempt.update.profileFamilyName, + profileAvatarData: attempt.update.profileAvatarData) .map(on: .global()) { versionedUpdate in attempt.avatarUrlPath = versionedUpdate.avatarUrlPath } @@ -290,11 +298,11 @@ extension OWSProfileManager { Logger.verbose("profile givenName: \(attempt.update.profileGivenName), familyName: \(attempt.update.profileFamilyName), avatarFilename: \(attempt.avatarFilename)") attempt.userProfile.updateWith(givenName: attempt.update.profileGivenName, - familyName: attempt.update.profileFamilyName, - avatarUrlPath: attempt.avatarUrlPath, - avatarFileName: attempt.avatarFilename, - transaction: transaction, - completion: nil) + familyName: attempt.update.profileFamilyName, + avatarUrlPath: attempt.avatarUrlPath, + avatarFileName: attempt.avatarFilename, + transaction: transaction, + completion: nil) } // MARK: - Update Queue @@ -383,8 +391,8 @@ class PendingProfileUpdate: NSObject, NSCoding { public required init?(coder aDecoder: NSCoder) { guard let idString = aDecoder.decodeObject(forKey: "id") as? String, let id = UUID(uuidString: idString) else { - owsFailDebug("Missing id") - return nil + owsFailDebug("Missing id") + return nil } self.id = id self.profileGivenName = aDecoder.decodeObject(forKey: "profileGivenName") as? String @@ -442,8 +450,8 @@ public extension OWSProfileManager { // Given name is required guard let givenNameData = nameSegments[safe: 0], let givenName = String(data: givenNameData, encoding: .utf8), !givenName.isEmpty else { - owsFailDebug("unexpectedly missing first name") - return nil + owsFailDebug("unexpectedly missing first name") + return nil } // Family name is optional diff --git a/SignalMessaging/profiles/VersionedProfiles.swift b/SignalMessaging/profiles/VersionedProfilesImpl.swift similarity index 75% rename from SignalMessaging/profiles/VersionedProfiles.swift rename to SignalMessaging/profiles/VersionedProfilesImpl.swift index 9904fcfed4..ef71df736c 100644 --- a/SignalMessaging/profiles/VersionedProfiles.swift +++ b/SignalMessaging/profiles/VersionedProfilesImpl.swift @@ -8,44 +8,37 @@ import SignalServiceKit import SignalMetadataKit import ZKGroup -@objc -public class VersionedProfileUpdate: NSObject { - // This will only be set if there is a profile avatar. - @objc - public let avatarUrlPath: String? +public class VersionedProfileRequestImpl: NSObject, VersionedProfileRequest { + public let request: TSRequest + public let requestContext: ProfileKeyCredentialRequestContext? - required init(avatarUrlPath: String? = nil) { - self.avatarUrlPath = avatarUrlPath + public required init(request: TSRequest, + requestContext: ProfileKeyCredentialRequestContext?) { + self.request = request + self.requestContext = requestContext } } // MARK: - -public struct VersionedProfileRequest { - public let request: TSRequest - public let requestContext: ProfileKeyCredentialRequestContext? -} - -// MARK: - - @objc -public class VersionedProfiles: NSObject { +public class VersionedProfilesImpl: NSObject, VersionedProfilesSwift { // MARK: - Dependencies - private class var profileManager: OWSProfileManager { + private var profileManager: OWSProfileManager { return OWSProfileManager.shared() } - private class var networkManager: TSNetworkManager { + private var networkManager: TSNetworkManager { return SSKEnvironment.shared.networkManager } - private class var databaseStorage: SDSDatabaseStorage { + private var databaseStorage: SDSDatabaseStorage { return SDSDatabaseStorage.shared } - private class var tsAccountManager: TSAccountManager { + private var tsAccountManager: TSAccountManager { return .sharedInstance() } @@ -53,21 +46,18 @@ public class VersionedProfiles: NSObject { public static let credentialStore = SDSKeyValueStore(collection: "VersionedProfiles.credentialStore") - // Never instantiate this class. - private override init() {} - // MARK: - - public class func clientZkProfileOperations() throws -> ClientZkProfileOperations { + public func clientZkProfileOperations() throws -> ClientZkProfileOperations { return ClientZkProfileOperations(serverPublicParams: try GroupsV2Protos.serverPublicParams()) } // MARK: - Update @objc - public class func updateProfileOnService(profileGivenName: String?, - profileFamilyName: String?, - profileAvatarData: Data?) { + public func updateProfileOnService(profileGivenName: String?, + profileFamilyName: String?, + profileAvatarData: Data?) { updateProfilePromise(profileGivenName: profileGivenName, profileFamilyName: profileFamilyName, profileAvatarData: profileAvatarData) @@ -82,17 +72,17 @@ public class VersionedProfiles: NSObject { fetchType: .versioned) .done { _ in Logger.verbose("success") - }.catch { error in - owsFailDebug("error: \(error)") - }.retainUntilComplete() - }.catch { error in - owsFailDebug("error: \(error)") - }.retainUntilComplete() + }.catch { error in + owsFailDebug("error: \(error)") + }.retainUntilComplete() + }.catch { error in + owsFailDebug("error: \(error)") + }.retainUntilComplete() } - public class func updateProfilePromise(profileGivenName: String?, - profileFamilyName: String?, - profileAvatarData: Data?) -> Promise { + public func updateProfilePromise(profileGivenName: String?, + profileFamilyName: String?, + profileAvatarData: Data?) -> Promise { return DispatchQueue.global().async(.promise) { guard let localUuid = self.tsAccountManager.localUuid else { @@ -114,7 +104,7 @@ public class VersionedProfiles: NSObject { guard let encryptedPaddedProfileName = self.profileManager.encrypt(profileNameComponents: nameComponents, profileKey: profileKey) else { - throw OWSAssertionError("Could not encrypt profile name.") + throw OWSAssertionError("Could not encrypt profile name.") } nameData = encryptedPaddedProfileName } @@ -128,7 +118,7 @@ public class VersionedProfiles: NSObject { let profileKey: OWSAES256Key = self.profileManager.localProfileKey() guard let encryptedProfileAvatarData = self.profileManager.encrypt(profileData: profileAvatarData, profileKey: profileKey) else { - throw OWSAssertionError("Could not encrypt profile avatar.") + throw OWSAssertionError("Could not encrypt profile avatar.") } return self.parseFormAndUpload(formResponseObject: responseObject, profileAvatarData: encryptedProfileAvatarData) @@ -137,8 +127,8 @@ public class VersionedProfiles: NSObject { } } - private class func parseFormAndUpload(formResponseObject: Any?, - profileAvatarData: Data) -> Promise { + private func parseFormAndUpload(formResponseObject: Any?, + profileAvatarData: Data) -> Promise { return firstly { () throws -> Promise in guard let response = formResponseObject as? [AnyHashable: Any] else { throw OWSAssertionError("Unexpected response.") @@ -156,8 +146,8 @@ public class VersionedProfiles: NSObject { // MARK: - Get - public class func versionedProfileRequest(address: SignalServiceAddress, - udAccessKey: SMKUDAccessKey?) throws -> VersionedProfileRequest { + public func versionedProfileRequest(address: SignalServiceAddress, + udAccessKey: SMKUDAccessKey?) throws -> VersionedProfileRequest { guard address.isValid, let uuid: UUID = address.uuid else { throw OWSAssertionError("Invalid address: \(address)") @@ -193,20 +183,23 @@ public class VersionedProfiles: NSObject { credentialRequest: credentialRequestArg, udAccessKey: udAccessKey) - return VersionedProfileRequest(request: request, requestContext: requestContext) + return VersionedProfileRequestImpl(request: request, requestContext: requestContext) } // MARK: - - public class func parseProfileKey(profileKey: OWSAES256Key) throws -> ProfileKey { + public func parseProfileKey(profileKey: OWSAES256Key) throws -> ProfileKey { let profileKeyData: Data = profileKey.keyData let profileKeyDataBytes = [UInt8](profileKeyData) return try ProfileKey(contents: profileKeyDataBytes) } - public class func didFetchProfile(profile: SignalServiceProfile, - profileRequest: VersionedProfileRequest) { + public func didFetchProfile(profile: SignalServiceProfile, + profileRequest: VersionedProfileRequest) { do { + guard let profileRequest = profileRequest as? VersionedProfileRequestImpl else { + return + } guard let credentialResponseData = profile.credential else { return } @@ -233,7 +226,7 @@ public class VersionedProfiles: NSObject { Logger.verbose("Updating credential for: \(uuid)") databaseStorage.write { transaction in - credentialStore.setData(credentialData, key: uuid.uuidString, transaction: transaction) + Self.credentialStore.setData(credentialData, key: uuid.uuidString, transaction: transaction) } } catch { owsFailDebug("Invalid credential: \(error).") @@ -243,17 +236,17 @@ public class VersionedProfiles: NSObject { // MARK: - Credentials - public class func profileKeyCredentialData(for address: SignalServiceAddress, - transaction: SDSAnyReadTransaction) throws -> Data? { + public func profileKeyCredentialData(for address: SignalServiceAddress, + transaction: SDSAnyReadTransaction) throws -> Data? { guard address.isValid, let uuid = address.uuid else { throw OWSAssertionError("Invalid address: \(address)") } - return credentialStore.getData(uuid.uuidString, transaction: transaction) + return Self.credentialStore.getData(uuid.uuidString, transaction: transaction) } - public class func profileKeyCredential(for address: SignalServiceAddress, - transaction: SDSAnyReadTransaction) throws -> ProfileKeyCredential? { + public func profileKeyCredential(for address: SignalServiceAddress, + transaction: SDSAnyReadTransaction) throws -> ProfileKeyCredential? { guard let data = try profileKeyCredentialData(for: address, transaction: transaction) else { return nil } @@ -263,8 +256,8 @@ public class VersionedProfiles: NSObject { } @objc(clearProfileKeyCredentialForAddress:transaction:) - public class func clearProfileKeyCredential(for address: SignalServiceAddress, - transaction: SDSAnyWriteTransaction) { + public func clearProfileKeyCredential(for address: SignalServiceAddress, + transaction: SDSAnyWriteTransaction) { guard address.isValid else { owsFailDebug("Invalid address: \(address)") return @@ -272,11 +265,11 @@ public class VersionedProfiles: NSObject { guard let uuid = address.uuid else { return } - credentialStore.removeValue(forKey: uuid.uuidString, transaction: transaction) + Self.credentialStore.removeValue(forKey: uuid.uuidString, transaction: transaction) } - public class func clearProfileKeyCredentials(transaction: SDSAnyWriteTransaction) { - credentialStore.removeAll(transaction: transaction) + public func clearProfileKeyCredentials(transaction: SDSAnyWriteTransaction) { + Self.credentialStore.removeAll(transaction: transaction) } } diff --git a/SignalServiceKit/src/Protocols/ProfileManagerProtocol.h b/SignalServiceKit/src/Protocols/ProfileManagerProtocol.h index bdf8e988f4..bd940610ea 100644 --- a/SignalServiceKit/src/Protocols/ProfileManagerProtocol.h +++ b/SignalServiceKit/src/Protocols/ProfileManagerProtocol.h @@ -89,6 +89,12 @@ NS_ASSUME_NONNULL_BEGIN mainAppOnly:(BOOL)mainAppOnly ignoreThrottling:(BOOL)ignoreThrottling; +- (void)updateProfileForAddress:(SignalServiceAddress *)address + profileNameEncrypted:(nullable NSData *)profileNameEncrypted + username:(nullable NSString *)username + isUuidCapable:(BOOL)isUuidCapable + avatarUrlPath:(nullable NSString *)avatarUrlPath; + - (BOOL)recipientAddressIsUuidCapable:(SignalServiceAddress *)address transaction:(SDSAnyReadTransaction *)transaction; - (void)warmCaches; diff --git a/SignalServiceKit/src/SSKEnvironment.h b/SignalServiceKit/src/SSKEnvironment.h index aef5c9687f..515ca272f9 100644 --- a/SignalServiceKit/src/SSKEnvironment.h +++ b/SignalServiceKit/src/SSKEnvironment.h @@ -52,6 +52,7 @@ NS_ASSUME_NONNULL_BEGIN @protocol GroupsV2; @protocol GroupV2Updates; @protocol PendingReadReceiptRecorder; +@protocol VersionedProfiles; @interface SSKEnvironment : NSObject @@ -100,7 +101,8 @@ NS_ASSUME_NONNULL_BEGIN groupV2Updates:(id)groupV2Updates messageProcessing:(MessageProcessing *)messageProcessing messageFetcherJob:(MessageFetcherJob *)messageFetcherJob - bulkProfileFetch:(BulkProfileFetch *)bulkProfileFetch NS_DESIGNATED_INITIALIZER; + bulkProfileFetch:(BulkProfileFetch *)bulkProfileFetch + versionedProfiles:(id)versionedProfiles NS_DESIGNATED_INITIALIZER; @property (nonatomic, readonly, class) SSKEnvironment *shared; @@ -156,6 +158,7 @@ NS_ASSUME_NONNULL_BEGIN @property (nonatomic, readonly) MessageProcessing *messageProcessing; @property (nonatomic, readonly) MessageFetcherJob *messageFetcherJob; @property (nonatomic, readonly) BulkProfileFetch *bulkProfileFetch; +@property (nonatomic, readonly) id versionedProfiles; @property (nonatomic, readonly, nullable) OWSPrimaryStorage *primaryStorage; diff --git a/SignalServiceKit/src/SSKEnvironment.m b/SignalServiceKit/src/SSKEnvironment.m index 48cc9b3a1b..c37e014a21 100644 --- a/SignalServiceKit/src/SSKEnvironment.m +++ b/SignalServiceKit/src/SSKEnvironment.m @@ -50,6 +50,7 @@ static SSKEnvironment *sharedSSKEnvironment; @property (nonatomic) MessageProcessing *messageProcessing; @property (nonatomic) MessageFetcherJob *messageFetcherJob; @property (nonatomic) BulkProfileFetch *bulkProfileFetch; +@property (nonatomic) id versionedProfiles; @end @@ -105,6 +106,7 @@ static SSKEnvironment *sharedSSKEnvironment; messageProcessing:(MessageProcessing *)messageProcessing messageFetcherJob:(MessageFetcherJob *)messageFetcherJob bulkProfileFetch:(BulkProfileFetch *)bulkProfileFetch + versionedProfiles:(id)versionedProfiles { self = [super init]; if (!self) { @@ -154,6 +156,7 @@ static SSKEnvironment *sharedSSKEnvironment; OWSAssertDebug(messageProcessing); OWSAssertDebug(messageFetcherJob); OWSAssertDebug(bulkProfileFetch); + OWSAssertDebug(versionedProfiles); _contactsManager = contactsManager; _linkPreviewManager = linkPreviewManager; @@ -199,6 +202,7 @@ static SSKEnvironment *sharedSSKEnvironment; _messageProcessing = messageProcessing; _messageFetcherJob = messageFetcherJob; _bulkProfileFetch = bulkProfileFetch; + _versionedProfiles = versionedProfiles; return self; } diff --git a/SignalServiceKit/src/TestUtils/MockSSKEnvironment.m b/SignalServiceKit/src/TestUtils/MockSSKEnvironment.m index 5f008a9bf1..a6250e2806 100644 --- a/SignalServiceKit/src/TestUtils/MockSSKEnvironment.m +++ b/SignalServiceKit/src/TestUtils/MockSSKEnvironment.m @@ -105,6 +105,7 @@ NS_ASSUME_NONNULL_BEGIN MessageProcessing *messageProcessing = [MessageProcessing new]; MessageFetcherJob *messageFetcherJob = [MessageFetcherJob new]; BulkProfileFetch *bulkProfileFetch = [BulkProfileFetch new]; + id versionedProfiles = [MockVersionedProfiles new]; self = [super initWithContactsManager:contactsManager linkPreviewManager:linkPreviewManager @@ -149,7 +150,8 @@ NS_ASSUME_NONNULL_BEGIN groupV2Updates:groupV2Updates messageProcessing:messageProcessing messageFetcherJob:messageFetcherJob - bulkProfileFetch:bulkProfileFetch]; + bulkProfileFetch:bulkProfileFetch + versionedProfiles:versionedProfiles]; if (!self) { return nil; diff --git a/SignalServiceKit/src/TestUtils/OWSFakeProfileManager.m b/SignalServiceKit/src/TestUtils/OWSFakeProfileManager.m index 7743c7fe3a..9c92fe3f39 100644 --- a/SignalServiceKit/src/TestUtils/OWSFakeProfileManager.m +++ b/SignalServiceKit/src/TestUtils/OWSFakeProfileManager.m @@ -264,6 +264,15 @@ NS_ASSUME_NONNULL_BEGIN return self.hasLocalProfile; } +- (void)updateProfileForAddress:(SignalServiceAddress *)address + profileNameEncrypted:(nullable NSData *)profileNameEncrypted + username:(nullable NSString *)username + isUuidCapable:(BOOL)isUuidCapable + avatarUrlPath:(nullable NSString *)avatarUrlPath +{ + // Do nothing. +} + @end #endif diff --git a/SignalServiceKit/src/Util/BulkProfileFetch.swift b/SignalServiceKit/src/Util/Profiles/BulkProfileFetch.swift similarity index 91% rename from SignalServiceKit/src/Util/BulkProfileFetch.swift rename to SignalServiceKit/src/Util/Profiles/BulkProfileFetch.swift index d139d9fd71..8471ecdb93 100644 --- a/SignalServiceKit/src/Util/BulkProfileFetch.swift +++ b/SignalServiceKit/src/Util/Profiles/BulkProfileFetch.swift @@ -37,6 +37,7 @@ public class BulkProfileFetch: NSObject { case noProfile(date: Date) case serviceError(date: Date) case success(date: Date) + case throttled(date: Date) } // This property should only be accessed on serialQueue. @@ -159,6 +160,15 @@ public class BulkProfileFetch: NSObject { self.isUpdateInFlight = false let now = Date() switch error { + case ProfileFetchError.missing: + Logger.error("Error: \(error)") + self.lastOutcomeMap[address] = .noProfile(date: now) + case ProfileFetchError.throttled: + self.lastOutcomeMap[address] = .throttled(date: now) + case ProfileFetchError.rateLimit: + Logger.error("Error: \(error)") + self.lastOutcomeMap[address] = .retryLimit(date: now) + self.lastRateLimitErrorDate = now default: if IsNetworkConnectivityFailure(error) { Logger.warn("Error: \(error)") @@ -176,12 +186,6 @@ public class BulkProfileFetch: NSObject { self.lastOutcomeMap[address] = .serviceError(date: now) } } -// public enum ProfileFetchError: Int, Error { -// case missing -// case throttled -// case notMainApp -// case cantRequestVersionedProfile -// } self.process() } @@ -214,6 +218,9 @@ public class BulkProfileFetch: NSObject { case .retryLimit(let date): minElapsedSeconds = 5 * kMinuteInterval elapsedSeconds = date.timeIntervalSinceNow + case .throttled(let date): + minElapsedSeconds = 5 * kMinuteInterval + elapsedSeconds = date.timeIntervalSinceNow case .noProfile(let date): minElapsedSeconds = 6 * kHourInterval elapsedSeconds = date.timeIntervalSinceNow diff --git a/SignalMessaging/profiles/ProfileFetcherJob.swift b/SignalServiceKit/src/Util/Profiles/ProfileFetcherJob.swift similarity index 93% rename from SignalMessaging/profiles/ProfileFetcherJob.swift rename to SignalServiceKit/src/Util/Profiles/ProfileFetcherJob.swift index 15417e8d34..b8d5315be7 100644 --- a/SignalMessaging/profiles/ProfileFetcherJob.swift +++ b/SignalServiceKit/src/Util/Profiles/ProfileFetcherJob.swift @@ -4,7 +4,6 @@ import Foundation import PromiseKit -import SignalServiceKit import SignalMetadataKit @objc @@ -13,6 +12,26 @@ public enum ProfileFetchError: Int, Error { case throttled case notMainApp case cantRequestVersionedProfile + case rateLimit +} + +// MARK: - + +extension ProfileFetchError: CustomStringConvertible { + public var description: String { + switch self { + case .missing: + return "ProfileFetchError.missing" + case .throttled: + return "ProfileFetchError.throttled" + case .notMainApp: + return "ProfileFetchError.notMainApp" + case .cantRequestVersionedProfile: + return "ProfileFetchError.cantRequestVersionedProfile" + case .rateLimit: + return "ProfileFetchError.rateLimit" + } + } } // MARK: - @@ -175,8 +194,8 @@ public class ProfileFetcherJob: NSObject { return SSKEnvironment.shared.udManager } - private var profileManager: OWSProfileManager { - return OWSProfileManager.shared() + private var profileManager: ProfileManagerProtocol { + return SSKEnvironment.shared.profileManager } private var identityManager: OWSIdentityManager { @@ -200,6 +219,10 @@ public class ProfileFetcherJob: NSObject { return SDSDatabaseStorage.shared } + private var versionedProfiles: VersionedProfiles { + return SSKEnvironment.shared.versionedProfiles + } + // MARK: - private func runAsPromise() -> Promise { @@ -207,9 +230,12 @@ public class ProfileFetcherJob: NSObject { self.addBackgroundTask() }.then(on: DispatchQueue.global()) { _ in return self.requestProfile() - }.map(on: DispatchQueue.global()) { fetchedProfile in - self.updateProfile(fetchedProfile: fetchedProfile) - return fetchedProfile.profile + }.then(on: DispatchQueue.global()) { fetchedProfile in + return firstly { + self.updateProfile(fetchedProfile: fetchedProfile) + }.map(on: DispatchQueue.global()) { _ in + return fetchedProfile.profile + } } } @@ -276,10 +302,8 @@ public class ProfileFetcherJob: NSObject { } firstly { () -> Guarantee in - // Backoff if we've hit a rate limit. if error.httpStatusCode == 413 { - let backoffDelay = TimeInterval(retryCount + 1) * 30 - return after(seconds: backoffDelay) + return resolver.reject(ProfileFetchError.rateLimit) } return Guarantee.value(()) }.then(on: DispatchQueue.global()) { _ in @@ -357,7 +381,7 @@ public class ProfileFetcherJob: NSObject { if shouldUseVersionedFetch { do { - let request = try VersionedProfiles.versionedProfileRequest(address: address, udAccessKey: udAccessKeyForRequest) + let request = try self.versionedProfiles.versionedProfileRequest(address: address, udAccessKey: udAccessKeyForRequest) currentVersionedProfileRequest = request return request.request } catch { @@ -390,7 +414,7 @@ public class ProfileFetcherJob: NSObject { let address = profile.address if let profileRequest = fetchedProfile.versionedProfileRequest { - VersionedProfiles.didFetchProfile(profile: profile, profileRequest: profileRequest) + self.versionedProfiles.didFetchProfile(profile: profile, profileRequest: profileRequest) } profileManager.updateProfile(for: address, diff --git a/SignalServiceKit/src/Util/Profiles/VersionedProfiles.swift b/SignalServiceKit/src/Util/Profiles/VersionedProfiles.swift new file mode 100644 index 0000000000..f289d5d18c --- /dev/null +++ b/SignalServiceKit/src/Util/Profiles/VersionedProfiles.swift @@ -0,0 +1,76 @@ +// +// Copyright (c) 2020 Open Whisper Systems. All rights reserved. +// + +import Foundation +import PromiseKit +//import SignalServiceKit +import SignalMetadataKit +//import ZKGroup + +@objc +public class VersionedProfileUpdate: NSObject { + // This will only be set if there is a profile avatar. + @objc + public let avatarUrlPath: String? + + public required init(avatarUrlPath: String? = nil) { + self.avatarUrlPath = avatarUrlPath + } +} + +// MARK: - + +@objc +public protocol VersionedProfileRequest: class { + var request: TSRequest { get } +} + +// MARK: - + +@objc +public protocol VersionedProfiles: class { + @objc(clearProfileKeyCredentialForAddress:transaction:) + func clearProfileKeyCredential(for address: SignalServiceAddress, + transaction: SDSAnyWriteTransaction) + + func clearProfileKeyCredentials(transaction: SDSAnyWriteTransaction) + + func versionedProfileRequest(address: SignalServiceAddress, + udAccessKey: SMKUDAccessKey?) throws -> VersionedProfileRequest + + func didFetchProfile(profile: SignalServiceProfile, + profileRequest: VersionedProfileRequest) +} + +// MARK: - + +public protocol VersionedProfilesSwift: VersionedProfiles { + func updateProfilePromise(profileGivenName: String?, + profileFamilyName: String?, + profileAvatarData: Data?) -> Promise +} + +// MARK: - + +@objc +public class MockVersionedProfiles: NSObject, VersionedProfilesSwift { + public func clearProfileKeyCredential(for address: SignalServiceAddress, + transaction: SDSAnyWriteTransaction) {} + + public func clearProfileKeyCredentials(transaction: SDSAnyWriteTransaction) {} + + public func versionedProfileRequest(address: SignalServiceAddress, + udAccessKey: SMKUDAccessKey?) throws -> VersionedProfileRequest { + owsFail("Not implemented.") + } + + public func didFetchProfile(profile: SignalServiceProfile, + profileRequest: VersionedProfileRequest) {} + + public func updateProfilePromise(profileGivenName: String?, + profileFamilyName: String?, + profileAvatarData: Data?) -> Promise { + owsFail("Not implemented.") + } +} From 0f2bd92eabaf5ccbff1c082ecbd04c5f2469b91b Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Tue, 14 Apr 2020 17:12:27 -0300 Subject: [PATCH 06/13] Avoid rate limits while doing profile fetches. --- .../BaseGroupMemberViewController.swift | 26 ++++++++++++++----- .../src/Util/Profiles/ProfileFetcherJob.swift | 13 ++++------ 2 files changed, 24 insertions(+), 15 deletions(-) diff --git a/Signal/src/ViewControllers/NewGroupView/BaseGroupMemberViewController.swift b/Signal/src/ViewControllers/NewGroupView/BaseGroupMemberViewController.swift index 69e619bda2..14b9c60dba 100644 --- a/Signal/src/ViewControllers/NewGroupView/BaseGroupMemberViewController.swift +++ b/Signal/src/ViewControllers/NewGroupView/BaseGroupMemberViewController.swift @@ -395,10 +395,12 @@ extension BaseGroupMemberViewController: RecipientPickerDelegate { RemoteConfig.groupsV2IncomingMessages else { return } - if !doesRecipientSupportGroupsV2(recipient) { - tryToEnableGroupsV2ForAddress(address, - isBlocking: false, - ignoreErrors: true).retainUntilComplete() + DispatchQueue.global().async { + if !self.doesRecipientSupportGroupsV2(recipient) { + self.tryToEnableGroupsV2ForAddress(address, + isBlocking: false, + ignoreErrors: true).retainUntilComplete() + } } } @@ -437,6 +439,10 @@ extension BaseGroupMemberViewController: RecipientPickerDelegate { owsFailDebug("Invalid recipient.") return false } + return doesRecipientSupportGroupsV2(address) + } + + private func doesRecipientSupportGroupsV2(_ address: SignalServiceAddress) -> Bool { return databaseStorage.read { transaction in return GroupManager.doesUserSupportGroupsV2(address: address, transaction: transaction) } @@ -449,9 +455,15 @@ extension BaseGroupMemberViewController: RecipientPickerDelegate { return GroupManager.tryToEnableGroupsV2(for: [address], isBlocking: isBlocking, ignoreErrors: ignoreErrors) - }.done { [weak self] _ in - // Reload view content. - self?.recipientPicker.reloadContent() + }.done(on: .global() ) { [weak self] _ in + // If we succeeded in enable groups v2 for this address, + // reload the recipient picker to reflect that. + if self?.doesRecipientSupportGroupsV2(address) ?? false { + DispatchQueue.main.async { + // Reload view content. + self?.recipientPicker.reloadContent() + } + } } } diff --git a/SignalServiceKit/src/Util/Profiles/ProfileFetcherJob.swift b/SignalServiceKit/src/Util/Profiles/ProfileFetcherJob.swift index b8d5315be7..a0aaf783dc 100644 --- a/SignalServiceKit/src/Util/Profiles/ProfileFetcherJob.swift +++ b/SignalServiceKit/src/Util/Profiles/ProfileFetcherJob.swift @@ -278,8 +278,10 @@ public class ProfileFetcherJob: NSObject { resolver.fulfill(fetchedProfile) }.catch(on: DispatchQueue.global()) { error in if error.httpStatusCode == 404 { - resolver.reject(ProfileFetchError.missing) - return + return resolver.reject(ProfileFetchError.missing) + } + if error.httpStatusCode == 413 { + return resolver.reject(ProfileFetchError.rateLimit) } switch error { @@ -301,12 +303,7 @@ public class ProfileFetcherJob: NSObject { return } - firstly { () -> Guarantee in - if error.httpStatusCode == 413 { - return resolver.reject(ProfileFetchError.rateLimit) - } - return Guarantee.value(()) - }.then(on: DispatchQueue.global()) { _ in + firstly { self.requestProfileWithRetries(retryCount: retryCount + 1) }.done(on: DispatchQueue.global()) { fetchedProfile in resolver.fulfill(fetchedProfile) From a13436d699090b6c175a8d410cbaaaa6192da3f6 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Tue, 14 Apr 2020 17:31:02 -0300 Subject: [PATCH 07/13] Avoid rate limits while doing profile fetches. --- .../profiles/OWSProfileManager.swift | 32 ++++++++++--------- .../profiles/VersionedProfilesImpl.swift | 26 ++++++++------- 2 files changed, 31 insertions(+), 27 deletions(-) diff --git a/SignalMessaging/profiles/OWSProfileManager.swift b/SignalMessaging/profiles/OWSProfileManager.swift index 904e22f732..9ac6ba57cc 100644 --- a/SignalMessaging/profiles/OWSProfileManager.swift +++ b/SignalMessaging/profiles/OWSProfileManager.swift @@ -117,10 +117,11 @@ extension OWSProfileManager { guard let update = pendingUpdate else { return } - attemptToUpdateProfileOnService(update: update, - retryDelay: retryDelay) - .done { _ in - Logger.info("Update succeeded.") + firstly { + attemptToUpdateProfileOnService(update: update, + retryDelay: retryDelay) + }.done { _ in + Logger.info("Update succeeded.") }.catch { error in Logger.error("Update failed: \(error)") }.retainUntilComplete() @@ -141,18 +142,19 @@ extension OWSProfileManager { let attempt = ProfileUpdateAttempt(update: update, userProfile: userProfile) - let promise = writeProfileAvatarToDisk(attempt: attempt) - .then(on: DispatchQueue.global()) { () -> Promise in - // Optimistically update local profile state. - databaseStorage.write { transaction in - self.updateLocalProfile(with: attempt, transaction: transaction) - } + let promise = firstly { + writeProfileAvatarToDisk(attempt: attempt) + }.then(on: DispatchQueue.global()) { () -> Promise in + // Optimistically update local profile state. + databaseStorage.write { transaction in + self.updateLocalProfile(with: attempt, transaction: transaction) + } - if FeatureFlags.versionedProfiledUpdate { - return updateProfileOnServiceVersioned(attempt: attempt) - } else { - return updateProfileOnServiceUnversioned(attempt: attempt) - } + if FeatureFlags.versionedProfiledUpdate { + return updateProfileOnServiceVersioned(attempt: attempt) + } else { + return updateProfileOnServiceUnversioned(attempt: attempt) + } }.done(on: DispatchQueue.global()) { _ in _ = self.databaseStorage.write { (transaction: SDSAnyWriteTransaction) -> Void in guard tryToDequeueProfileUpdate(update: attempt.update, transaction: transaction) else { diff --git a/SignalMessaging/profiles/VersionedProfilesImpl.swift b/SignalMessaging/profiles/VersionedProfilesImpl.swift index ef71df736c..69b3a79124 100644 --- a/SignalMessaging/profiles/VersionedProfilesImpl.swift +++ b/SignalMessaging/profiles/VersionedProfilesImpl.swift @@ -58,23 +58,25 @@ public class VersionedProfilesImpl: NSObject, VersionedProfilesSwift { public func updateProfileOnService(profileGivenName: String?, profileFamilyName: String?, profileAvatarData: Data?) { - updateProfilePromise(profileGivenName: profileGivenName, - profileFamilyName: profileFamilyName, - profileAvatarData: profileAvatarData) - .done { _ in - Logger.verbose("success") + firstly { + updateProfilePromise(profileGivenName: profileGivenName, + profileFamilyName: profileFamilyName, + profileAvatarData: profileAvatarData) + }.done { _ in + Logger.verbose("success") - // TODO: This is temporary for testing. - let localAddress = TSAccountManager.sharedInstance().localAddress! + // TODO: This is temporary for testing. + let localAddress = TSAccountManager.sharedInstance().localAddress! + firstly { ProfileFetcherJob.fetchAndUpdateProfilePromise(address: localAddress, mainAppOnly: false, ignoreThrottling: true, fetchType: .versioned) - .done { _ in - Logger.verbose("success") - }.catch { error in - owsFailDebug("error: \(error)") - }.retainUntilComplete() + }.done { _ in + Logger.verbose("success") + }.catch { error in + owsFailDebug("error: \(error)") + }.retainUntilComplete() }.catch { error in owsFailDebug("error: \(error)") }.retainUntilComplete() From b3f5f8d3d7a6554fb007605235f53a7e8c79c9fd Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Wed, 15 Apr 2020 17:10:28 -0300 Subject: [PATCH 08/13] Respond to CR. --- Pods | 2 +- .../src/Util/Profiles/BulkProfileFetch.swift | 66 ++++++++++--------- 2 files changed, 35 insertions(+), 33 deletions(-) diff --git a/Pods b/Pods index 6da0e3f112..ca3f9cdb29 160000 --- a/Pods +++ b/Pods @@ -1 +1 @@ -Subproject commit 6da0e3f112b795f22277415fdcebb6d7c7ff83a8 +Subproject commit ca3f9cdb29b13d71754e193b4b7965dad22921c4 diff --git a/SignalServiceKit/src/Util/Profiles/BulkProfileFetch.swift b/SignalServiceKit/src/Util/Profiles/BulkProfileFetch.swift index 8471ecdb93..4fe47fae51 100644 --- a/SignalServiceKit/src/Util/Profiles/BulkProfileFetch.swift +++ b/SignalServiceKit/src/Util/Profiles/BulkProfileFetch.swift @@ -31,13 +31,22 @@ public class BulkProfileFetch: NSObject { // This property should only be accessed on serialQueue. private var isUpdateInFlight = false - private enum UpdateOutcome { - case networkFailure(date: Date) - case retryLimit(date: Date) - case noProfile(date: Date) - case serviceError(date: Date) - case success(date: Date) - case throttled(date: Date) + struct UpdateOutcome { + let outcome: Outcome + enum Outcome { + case networkFailure + case retryLimit + case noProfile + case serviceError + case success + case throttled + } + let date: Date + + init(_ outcome: Outcome) { + self.outcome = outcome + self.date = Date() + } } // This property should only be accessed on serialQueue. @@ -152,38 +161,37 @@ public class BulkProfileFetch: NSObject { }.done { self.serialQueue.asyncAfter(deadline: DispatchTime.now() + updateDelaySeconds) { self.isUpdateInFlight = false - self.lastOutcomeMap[address] = .success(date: Date()) + self.lastOutcomeMap[address] = UpdateOutcome(.success) self.process() } }.catch { error in self.serialQueue.asyncAfter(deadline: DispatchTime.now() + updateDelaySeconds) { self.isUpdateInFlight = false - let now = Date() switch error { case ProfileFetchError.missing: Logger.error("Error: \(error)") - self.lastOutcomeMap[address] = .noProfile(date: now) + self.lastOutcomeMap[address] = UpdateOutcome(.noProfile) case ProfileFetchError.throttled: - self.lastOutcomeMap[address] = .throttled(date: now) + self.lastOutcomeMap[address] = UpdateOutcome(.throttled) case ProfileFetchError.rateLimit: Logger.error("Error: \(error)") - self.lastOutcomeMap[address] = .retryLimit(date: now) - self.lastRateLimitErrorDate = now + self.lastOutcomeMap[address] = UpdateOutcome(.retryLimit) + self.lastRateLimitErrorDate = Date() default: if IsNetworkConnectivityFailure(error) { Logger.warn("Error: \(error)") - self.lastOutcomeMap[address] = .networkFailure(date: now) + self.lastOutcomeMap[address] = UpdateOutcome(.networkFailure) } else if error.httpStatusCode == 413 { Logger.error("Error: \(error)") - self.lastOutcomeMap[address] = .retryLimit(date: now) - self.lastRateLimitErrorDate = now + self.lastOutcomeMap[address] = UpdateOutcome(.retryLimit) + self.lastRateLimitErrorDate = Date() } else if error.httpStatusCode == 404 { Logger.error("Error: \(error)") - self.lastOutcomeMap[address] = .noProfile(date: now) + self.lastOutcomeMap[address] = UpdateOutcome(.noProfile) } else { // TODO: We may need to handle more status codes. owsFailDebug("Error: \(error)") - self.lastOutcomeMap[address] = .serviceError(date: now) + self.lastOutcomeMap[address] = UpdateOutcome(.serviceError) } } @@ -209,27 +217,21 @@ public class BulkProfileFetch: NSObject { } let minElapsedSeconds: TimeInterval - let elapsedSeconds: TimeInterval + let elapsedSeconds: TimeInterval = lastOutcome.date.timeIntervalSinceNow - switch lastOutcome { - case .networkFailure(let date): + switch lastOutcome.outcome { + case .networkFailure: minElapsedSeconds = 1 * kMinuteInterval - elapsedSeconds = date.timeIntervalSinceNow - case .retryLimit(let date): + case .retryLimit: minElapsedSeconds = 5 * kMinuteInterval - elapsedSeconds = date.timeIntervalSinceNow - case .throttled(let date): + case .throttled: minElapsedSeconds = 5 * kMinuteInterval - elapsedSeconds = date.timeIntervalSinceNow - case .noProfile(let date): + case .noProfile: minElapsedSeconds = 6 * kHourInterval - elapsedSeconds = date.timeIntervalSinceNow - case .serviceError(let date): + case .serviceError: minElapsedSeconds = 30 * kMinuteInterval - elapsedSeconds = date.timeIntervalSinceNow - case .success(let date): + case .success: minElapsedSeconds = 15 * kMinuteInterval - elapsedSeconds = date.timeIntervalSinceNow } return elapsedSeconds >= minElapsedSeconds From a0eb61535a27ed9fc94890ab28ae2630a2c59a30 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Wed, 15 Apr 2020 11:09:20 -0300 Subject: [PATCH 09/13] Bulk UUID lookup. --- .../BaseGroupMemberViewController.swift | 10 + SignalMessaging/environment/AppSetup.m | 2 + SignalMessaging/groups/GroupsV2Impl.swift | 19 -- SignalServiceKit/src/SSKEnvironment.h | 3 + SignalServiceKit/src/SSKEnvironment.m | 4 + .../src/TestUtils/MockSSKEnvironment.m | 2 + .../src/Util/BulkUUIDLookup.swift | 220 ++++++++++++++++++ .../src/groups/GroupManager.swift | 48 +++- SignalServiceKit/src/groups/GroupsV2.swift | 6 - 9 files changed, 287 insertions(+), 27 deletions(-) create mode 100644 SignalServiceKit/src/Util/BulkUUIDLookup.swift diff --git a/Signal/src/ViewControllers/NewGroupView/BaseGroupMemberViewController.swift b/Signal/src/ViewControllers/NewGroupView/BaseGroupMemberViewController.swift index 14b9c60dba..8d6c8066cb 100644 --- a/Signal/src/ViewControllers/NewGroupView/BaseGroupMemberViewController.swift +++ b/Signal/src/ViewControllers/NewGroupView/BaseGroupMemberViewController.swift @@ -127,6 +127,16 @@ public class BaseGroupMemberViewController: OWSViewController { autoPinView(toBottomOfViewControllerOrKeyboard: recipientPicker.view, avoidNotch: false) updateMemberCount() + tryToFillInMissingUuuids() + } + + private func tryToFillInMissingUuuids() { + let addresses = recipientPicker.contactsViewHelper.signalAccounts.map { $0.recipientAddress } + firstly { + GroupManager.tryToFillInMissingUuuids(for: addresses, isBlocking: false) + }.catch { error in + owsFailDebug("Error: \(error)") + }.retainUntilComplete() } @objc diff --git a/SignalMessaging/environment/AppSetup.m b/SignalMessaging/environment/AppSetup.m index 0e5d30ef21..1757dc84a4 100644 --- a/SignalMessaging/environment/AppSetup.m +++ b/SignalMessaging/environment/AppSetup.m @@ -114,6 +114,7 @@ NS_ASSUME_NONNULL_BEGIN MessageProcessing *messageProcessing = [MessageProcessing new]; MessageFetcherJob *messageFetcherJob = [MessageFetcherJob new]; BulkProfileFetch *bulkProfileFetch = [BulkProfileFetch new]; + BulkUUIDLookup *bulkUUIDLookup = [BulkUUIDLookup new]; id versionedProfiles = [VersionedProfilesImpl new]; [Environment setShared:[[Environment alloc] initWithAudioSession:audioSession @@ -171,6 +172,7 @@ NS_ASSUME_NONNULL_BEGIN messageProcessing:messageProcessing messageFetcherJob:messageFetcherJob bulkProfileFetch:bulkProfileFetch + bulkUUIDLookup:bulkUUIDLookup versionedProfiles:versionedProfiles]]; appSpecificSingletonBlock(); diff --git a/SignalMessaging/groups/GroupsV2Impl.swift b/SignalMessaging/groups/GroupsV2Impl.swift index 4b61f7dace..5f5b02a23b 100644 --- a/SignalMessaging/groups/GroupsV2Impl.swift +++ b/SignalMessaging/groups/GroupsV2Impl.swift @@ -1049,25 +1049,6 @@ public class GroupsV2Impl: NSObject, GroupsV2Swift { return when(fulfilled: promises).asVoid() } - // MARK: - UUIDs - - public func tryToEnsureUuidsForGroupMembers(for addresses: [SignalServiceAddress]) -> Promise { - guard FeatureFlags.useOnlyModernContactDiscovery else { - // Can't fill in UUIDs using legacy contact intersections. - return Promise.value(()) - } - - let phoneNumbersWithoutUuids = addresses.filter { $0.uuid == nil }.compactMap { $0.phoneNumber } - guard phoneNumbersWithoutUuids.count > 0 else { - return Promise.value(()) - } - - return firstly { - contactsUpdater.lookupIdentifiersPromise(phoneNumbers: - phoneNumbersWithoutUuids) - }.asVoid() - } - // MARK: - Auth Credentials private let authCredentialStore = SDSKeyValueStore(collection: "GroupsV2Impl.authCredentialStoreStore") diff --git a/SignalServiceKit/src/SSKEnvironment.h b/SignalServiceKit/src/SSKEnvironment.h index 515ca272f9..f50669f1ac 100644 --- a/SignalServiceKit/src/SSKEnvironment.h +++ b/SignalServiceKit/src/SSKEnvironment.h @@ -6,6 +6,7 @@ NS_ASSUME_NONNULL_BEGIN @class AccountServiceClient; @class BulkProfileFetch; +@class BulkUUIDLookup; @class ContactsUpdater; @class GroupsV2MessageProcessor; @class MessageFetcherJob; @@ -102,6 +103,7 @@ NS_ASSUME_NONNULL_BEGIN messageProcessing:(MessageProcessing *)messageProcessing messageFetcherJob:(MessageFetcherJob *)messageFetcherJob bulkProfileFetch:(BulkProfileFetch *)bulkProfileFetch + bulkUUIDLookup:(BulkUUIDLookup *)bulkUUIDLookup versionedProfiles:(id)versionedProfiles NS_DESIGNATED_INITIALIZER; @property (nonatomic, readonly, class) SSKEnvironment *shared; @@ -158,6 +160,7 @@ NS_ASSUME_NONNULL_BEGIN @property (nonatomic, readonly) MessageProcessing *messageProcessing; @property (nonatomic, readonly) MessageFetcherJob *messageFetcherJob; @property (nonatomic, readonly) BulkProfileFetch *bulkProfileFetch; +@property (nonatomic, readonly) BulkUUIDLookup *bulkUUIDLookup; @property (nonatomic, readonly) id versionedProfiles; @property (nonatomic, readonly, nullable) OWSPrimaryStorage *primaryStorage; diff --git a/SignalServiceKit/src/SSKEnvironment.m b/SignalServiceKit/src/SSKEnvironment.m index c37e014a21..8d0df16a57 100644 --- a/SignalServiceKit/src/SSKEnvironment.m +++ b/SignalServiceKit/src/SSKEnvironment.m @@ -50,6 +50,7 @@ static SSKEnvironment *sharedSSKEnvironment; @property (nonatomic) MessageProcessing *messageProcessing; @property (nonatomic) MessageFetcherJob *messageFetcherJob; @property (nonatomic) BulkProfileFetch *bulkProfileFetch; +@property (nonatomic) BulkUUIDLookup *bulkUUIDLookup; @property (nonatomic) id versionedProfiles; @end @@ -106,6 +107,7 @@ static SSKEnvironment *sharedSSKEnvironment; messageProcessing:(MessageProcessing *)messageProcessing messageFetcherJob:(MessageFetcherJob *)messageFetcherJob bulkProfileFetch:(BulkProfileFetch *)bulkProfileFetch + bulkUUIDLookup:(BulkUUIDLookup *)bulkUUIDLookup versionedProfiles:(id)versionedProfiles { self = [super init]; @@ -157,6 +159,7 @@ static SSKEnvironment *sharedSSKEnvironment; OWSAssertDebug(messageFetcherJob); OWSAssertDebug(bulkProfileFetch); OWSAssertDebug(versionedProfiles); + OWSAssertDebug(bulkUUIDLookup); _contactsManager = contactsManager; _linkPreviewManager = linkPreviewManager; @@ -203,6 +206,7 @@ static SSKEnvironment *sharedSSKEnvironment; _messageFetcherJob = messageFetcherJob; _bulkProfileFetch = bulkProfileFetch; _versionedProfiles = versionedProfiles; + _bulkUUIDLookup = bulkUUIDLookup; return self; } diff --git a/SignalServiceKit/src/TestUtils/MockSSKEnvironment.m b/SignalServiceKit/src/TestUtils/MockSSKEnvironment.m index a6250e2806..4595da36f8 100644 --- a/SignalServiceKit/src/TestUtils/MockSSKEnvironment.m +++ b/SignalServiceKit/src/TestUtils/MockSSKEnvironment.m @@ -105,6 +105,7 @@ NS_ASSUME_NONNULL_BEGIN MessageProcessing *messageProcessing = [MessageProcessing new]; MessageFetcherJob *messageFetcherJob = [MessageFetcherJob new]; BulkProfileFetch *bulkProfileFetch = [BulkProfileFetch new]; + BulkUUIDLookup *bulkUUIDLookup = [BulkUUIDLookup new]; id versionedProfiles = [MockVersionedProfiles new]; self = [super initWithContactsManager:contactsManager @@ -151,6 +152,7 @@ NS_ASSUME_NONNULL_BEGIN messageProcessing:messageProcessing messageFetcherJob:messageFetcherJob bulkProfileFetch:bulkProfileFetch + bulkUUIDLookup:bulkUUIDLookup versionedProfiles:versionedProfiles]; if (!self) { diff --git a/SignalServiceKit/src/Util/BulkUUIDLookup.swift b/SignalServiceKit/src/Util/BulkUUIDLookup.swift new file mode 100644 index 0000000000..d6eb58f1db --- /dev/null +++ b/SignalServiceKit/src/Util/BulkUUIDLookup.swift @@ -0,0 +1,220 @@ +// +// Copyright (c) 2020 Open Whisper Systems. All rights reserved. +// + +import PromiseKit + +@objc +public class BulkUUIDLookup: NSObject { + + // MARK: - Dependencies + + private var tsAccountManager: TSAccountManager { + return .sharedInstance() + } + + private var reachabilityManager: SSKReachabilityManager { + return SSKEnvironment.shared.reachabilityManager + } + + private var contactsUpdater: ContactsUpdater { + return SSKEnvironment.shared.contactsUpdater + } + + // MARK: - + + private let serialQueue = DispatchQueue(label: "BulkUUIDLookup") + + // This property should only be accessed on serialQueue. + private var phoneNumberQueue = Set() + + // This property should only be accessed on serialQueue. + private var isUpdateInFlight = false + + private enum UpdateOutcome { + case networkFailure(date: Date) + case retryLimit(date: Date) + case serviceError(date: Date) + case unknownError(date: Date) + case success(date: Date) + } + + // This property should only be accessed on serialQueue. + private var lastOutcomeMap = [String: UpdateOutcome]() + + // This property should only be accessed on serialQueue. + private var lastRateLimitErrorDate: Date? + + @objc + public required override init() { + super.init() + + SwiftSingletons.register(self) + + observeNotifications() + } + + private func observeNotifications() { + NotificationCenter.default.addObserver(forName: SSKReachability.owsReachabilityDidChange, + object: nil, queue: nil) { [weak self] _ in + guard let self = self else { return } + self.serialQueue.async { + self.process() + } + } + NotificationCenter.default.addObserver(forName: .registrationStateDidChange, object: nil, queue: nil) { [weak self] _ in + guard let self = self else { return } + self.serialQueue.async { + self.process() + } + } + } + + // This should be used for non-urgent uuid lookups. + @objc + public func lookupUuids(phoneNumbers: [String]) { + serialQueue.async { + self.phoneNumberQueue.formUnion(phoneNumbers) + self.process() + } + } + + private func process() { + assertOnQueue(serialQueue) + + guard CurrentAppContext().isMainApp else { + return + } + guard reachabilityManager.isReachable else { + return + } + guard tsAccountManager.isRegisteredAndReady else { + return + } + guard FeatureFlags.useOnlyModernContactDiscovery || + FeatureFlags.compareLegacyContactDiscoveryAgainstModern else { + // Can't fill in UUIDs using legacy contact intersections. + return + } + + // Only one update in flight at a time. + guard !self.isUpdateInFlight else { + return + } + + // Build batch. + let phoneNumbers = phoneNumberQueue.filter { self.shouldUpdatePhoneNumber($0) } + phoneNumberQueue.removeAll() + + // De-bounce. + + Logger.verbose("Updating: \(phoneNumbers)") + + // Perform update. + isUpdateInFlight = true + firstly { + return contactsUpdater.lookupIdentifiersPromise(phoneNumbers: Array(phoneNumbers)).asVoid() + }.done { + self.serialQueue.async { + self.isUpdateInFlight = false + let now = Date() + for phoneNumber in phoneNumbers { + self.lastOutcomeMap[phoneNumber] = .success(date: now) + } + self.process() + } + }.catch { error in + self.serialQueue.async { + self.isUpdateInFlight = false + let now = Date() + + let outcome: UpdateOutcome + let nsError = error as NSError + if nsError.domain == OWSSignalServiceKitErrorDomain && + nsError.code == OWSErrorCode.contactsUpdaterRateLimit.rawValue { + Logger.error("Error: \(error)") + outcome = .retryLimit(date: now) + self.lastRateLimitErrorDate = now + } else { + switch error { + case ContactDiscoveryService.ServiceError.error4xx, + ContactDiscoveryService.ServiceError.error5xx: + owsFailDebug("Error: \(error)") + outcome = .serviceError(date: now) + case ContactDiscoveryService.ServiceError.tooManyRequests: + Logger.error("Error: \(error)") + outcome = .retryLimit(date: now) + self.lastRateLimitErrorDate = now + default: + if IsNetworkConnectivityFailure(error) { + Logger.warn("Error: \(error)") + outcome = .networkFailure(date: now) + } else if error.httpStatusCode == 413 { + Logger.error("Error: \(error)") + outcome = .retryLimit(date: now) + self.lastRateLimitErrorDate = now + } else if let httpStatusCode = error.httpStatusCode, + httpStatusCode >= 400, + httpStatusCode <= 599 { + owsFailDebug("Error: \(error)") + outcome = .serviceError(date: now) + } else { + owsFailDebug("Error: \(error)") + outcome = .unknownError(date: now) + } + } + } + + for phoneNumber in phoneNumbers { + self.lastOutcomeMap[phoneNumber] = outcome + } + + self.process() + } + }.retainUntilComplete() + } + + private func shouldUpdatePhoneNumber(_ phoneNumber: String) -> Bool { + assertOnQueue(serialQueue) + + guard SignalServiceAddress(phoneNumber: phoneNumber).uuid == nil else { + return false + } + + // Skip if we've recently had a rate limit error. + if let lastRateLimitErrorDate = self.lastRateLimitErrorDate { + let minElapsedSeconds = 5 * kMinuteInterval + let elapsedSeconds = lastRateLimitErrorDate.timeIntervalSinceNow + guard elapsedSeconds >= minElapsedSeconds else { + return false + } + } + + guard let lastOutcome = lastOutcomeMap[phoneNumber] else { + return true + } + + let minElapsedSeconds: TimeInterval + let elapsedSeconds: TimeInterval + + switch lastOutcome { + case .networkFailure(let date): + minElapsedSeconds = 1 * kMinuteInterval + elapsedSeconds = date.timeIntervalSinceNow + case .retryLimit(let date): + minElapsedSeconds = 15 * kMinuteInterval + elapsedSeconds = date.timeIntervalSinceNow + case .serviceError(let date): + minElapsedSeconds = 30 * kMinuteInterval + elapsedSeconds = date.timeIntervalSinceNow + case .unknownError(let date): + minElapsedSeconds = 60 * kMinuteInterval + elapsedSeconds = date.timeIntervalSinceNow + case .success(let date): + minElapsedSeconds = 60 * kMinuteInterval + elapsedSeconds = date.timeIntervalSinceNow + } + + return elapsedSeconds >= minElapsedSeconds + } +} diff --git a/SignalServiceKit/src/groups/GroupManager.swift b/SignalServiceKit/src/groups/GroupManager.swift index c1fe0255b8..36c638fbb0 100644 --- a/SignalServiceKit/src/groups/GroupManager.swift +++ b/SignalServiceKit/src/groups/GroupManager.swift @@ -76,6 +76,14 @@ public class GroupManager: NSObject { return SSKEnvironment.shared.bulkProfileFetch } + private class var bulkUUIDLookup: BulkUUIDLookup { + return SSKEnvironment.shared.bulkUUIDLookup + } + + private class var contactsUpdater: ContactsUpdater { + return SSKEnvironment.shared.contactsUpdater + } + // MARK: - // Never instantiate this class. @@ -1224,9 +1232,45 @@ public class GroupManager: NSObject { } return Promise.value(()) }.then(on: .global()) { _ -> Promise in - // Try to ensure UUIDs. - return self.groupsV2.tryToEnsureUuidsForGroupMembers(for: addresses) + return self.tryToFillInMissingUuuids(for: addresses, isBlocking: isBlocking) }.then(on: .global()) { _ -> Promise in + return self.tryToEnableGroupsV2Capability(for: addresses, isBlocking: isBlocking) + } + } + + public static func tryToFillInMissingUuuids(for addresses: [SignalServiceAddress], + isBlocking: Bool) -> Promise { + guard FeatureFlags.useOnlyModernContactDiscovery || + FeatureFlags.compareLegacyContactDiscoveryAgainstModern else { + // Can't fill in UUIDs using legacy contact intersections. + return Promise.value(()) + } + + let phoneNumbersWithoutUuids = addresses.filter { $0.uuid == nil }.compactMap { $0.phoneNumber } + guard phoneNumbersWithoutUuids.count > 0 else { + return Promise.value(()) + } + + if isBlocking { + // Block on the outcome. + return contactsUpdater.lookupIdentifiersPromise(phoneNumbers: + phoneNumbersWithoutUuids).asVoid() + } else { + // This will throttle, de-bounce, etc. + self.bulkUUIDLookup.lookupUuids(phoneNumbers: phoneNumbersWithoutUuids) + return Promise.value(()) + } + } + + private static func tryToEnableGroupsV2Capability(for addresses: [SignalServiceAddress], + isBlocking: Bool) -> Promise { + return firstly { () -> Promise<[SignalServiceAddress]> in + let validAddresses = addresses.filter { $0.isValid } + if validAddresses.count < addresses.count { + owsFailDebug("Invalid addresses.") + } + return Promise.value(validAddresses) + }.then(on: .global()) { (addresses: [SignalServiceAddress]) -> Promise in // Try to ensure groups v2 capability. var addressesWithoutCapability = [SignalServiceAddress]() self.databaseStorage.read { transaction in diff --git a/SignalServiceKit/src/groups/GroupsV2.swift b/SignalServiceKit/src/groups/GroupsV2.swift index 425afebba0..4bde2f5d29 100644 --- a/SignalServiceKit/src/groups/GroupsV2.swift +++ b/SignalServiceKit/src/groups/GroupsV2.swift @@ -66,8 +66,6 @@ public protocol GroupsV2Swift: GroupsV2 { func tryToEnsureProfileKeyCredentials(for addresses: [SignalServiceAddress]) -> Promise - func tryToEnsureUuidsForGroupMembers(for addresses: [SignalServiceAddress]) -> Promise - func fetchCurrentGroupV2Snapshot(groupModel: TSGroupModelV2) -> Promise func fetchCurrentGroupV2Snapshot(groupSecretParamsData: Data) -> Promise @@ -326,10 +324,6 @@ public class MockGroupsV2: NSObject, GroupsV2Swift { owsFail("Not implemented.") } - public func tryToEnsureUuidsForGroupMembers(for addresses: [SignalServiceAddress]) -> Promise { - owsFail("Not implemented.") - } - public func fetchCurrentGroupV2Snapshot(groupModel: TSGroupModelV2) -> Promise { owsFail("Not implemented.") } From ff1a73b932b895ff97a08db9445898fa8e99d73e Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Wed, 15 Apr 2020 17:17:56 -0300 Subject: [PATCH 10/13] Respond to CR. --- .../src/Util/BulkUUIDLookup.swift | 65 ++++++++++--------- .../src/Util/Profiles/BulkProfileFetch.swift | 2 +- 2 files changed, 35 insertions(+), 32 deletions(-) diff --git a/SignalServiceKit/src/Util/BulkUUIDLookup.swift b/SignalServiceKit/src/Util/BulkUUIDLookup.swift index d6eb58f1db..bc4b8905ef 100644 --- a/SignalServiceKit/src/Util/BulkUUIDLookup.swift +++ b/SignalServiceKit/src/Util/BulkUUIDLookup.swift @@ -31,12 +31,21 @@ public class BulkUUIDLookup: NSObject { // This property should only be accessed on serialQueue. private var isUpdateInFlight = false - private enum UpdateOutcome { - case networkFailure(date: Date) - case retryLimit(date: Date) - case serviceError(date: Date) - case unknownError(date: Date) - case success(date: Date) + struct UpdateOutcome { + let outcome: Outcome + enum Outcome { + case networkFailure + case retryLimit + case serviceError + case success + case unknownError + } + let date: Date + + init(_ outcome: Outcome) { + self.outcome = outcome + self.date = Date() + } } // This property should only be accessed on serialQueue. @@ -117,50 +126,49 @@ public class BulkUUIDLookup: NSObject { }.done { self.serialQueue.async { self.isUpdateInFlight = false - let now = Date() + let outcome = UpdateOutcome(.success) for phoneNumber in phoneNumbers { - self.lastOutcomeMap[phoneNumber] = .success(date: now) + self.lastOutcomeMap[phoneNumber] = outcome } self.process() } }.catch { error in self.serialQueue.async { self.isUpdateInFlight = false - let now = Date() let outcome: UpdateOutcome let nsError = error as NSError if nsError.domain == OWSSignalServiceKitErrorDomain && nsError.code == OWSErrorCode.contactsUpdaterRateLimit.rawValue { Logger.error("Error: \(error)") - outcome = .retryLimit(date: now) - self.lastRateLimitErrorDate = now + outcome = UpdateOutcome(.retryLimit) + self.lastRateLimitErrorDate = Date() } else { switch error { case ContactDiscoveryService.ServiceError.error4xx, ContactDiscoveryService.ServiceError.error5xx: owsFailDebug("Error: \(error)") - outcome = .serviceError(date: now) + outcome = UpdateOutcome(.serviceError) case ContactDiscoveryService.ServiceError.tooManyRequests: Logger.error("Error: \(error)") - outcome = .retryLimit(date: now) - self.lastRateLimitErrorDate = now + outcome = UpdateOutcome(.retryLimit) + self.lastRateLimitErrorDate = Date() default: if IsNetworkConnectivityFailure(error) { Logger.warn("Error: \(error)") - outcome = .networkFailure(date: now) + outcome = UpdateOutcome(.networkFailure) } else if error.httpStatusCode == 413 { Logger.error("Error: \(error)") - outcome = .retryLimit(date: now) - self.lastRateLimitErrorDate = now + outcome = UpdateOutcome(.retryLimit) + self.lastRateLimitErrorDate = Date() } else if let httpStatusCode = error.httpStatusCode, httpStatusCode >= 400, httpStatusCode <= 599 { owsFailDebug("Error: \(error)") - outcome = .serviceError(date: now) + outcome = UpdateOutcome(.serviceError) } else { owsFailDebug("Error: \(error)") - outcome = .unknownError(date: now) + outcome = UpdateOutcome(.unknownError) } } } @@ -195,24 +203,19 @@ public class BulkUUIDLookup: NSObject { } let minElapsedSeconds: TimeInterval - let elapsedSeconds: TimeInterval + let elapsedSeconds = lastOutcome.date.timeIntervalSinceNow - switch lastOutcome { - case .networkFailure(let date): + switch lastOutcome.outcome { + case .networkFailure: minElapsedSeconds = 1 * kMinuteInterval - elapsedSeconds = date.timeIntervalSinceNow - case .retryLimit(let date): + case .retryLimit: minElapsedSeconds = 15 * kMinuteInterval - elapsedSeconds = date.timeIntervalSinceNow - case .serviceError(let date): + case .serviceError: minElapsedSeconds = 30 * kMinuteInterval - elapsedSeconds = date.timeIntervalSinceNow - case .unknownError(let date): + case .unknownError: minElapsedSeconds = 60 * kMinuteInterval - elapsedSeconds = date.timeIntervalSinceNow - case .success(let date): + case .success: minElapsedSeconds = 60 * kMinuteInterval - elapsedSeconds = date.timeIntervalSinceNow } return elapsedSeconds >= minElapsedSeconds diff --git a/SignalServiceKit/src/Util/Profiles/BulkProfileFetch.swift b/SignalServiceKit/src/Util/Profiles/BulkProfileFetch.swift index 4fe47fae51..ca9dcd486a 100644 --- a/SignalServiceKit/src/Util/Profiles/BulkProfileFetch.swift +++ b/SignalServiceKit/src/Util/Profiles/BulkProfileFetch.swift @@ -217,7 +217,7 @@ public class BulkProfileFetch: NSObject { } let minElapsedSeconds: TimeInterval - let elapsedSeconds: TimeInterval = lastOutcome.date.timeIntervalSinceNow + let elapsedSeconds = lastOutcome.date.timeIntervalSinceNow switch lastOutcome.outcome { case .networkFailure: From 7eecc851470b163bf98542e93e8b52d2336f6770 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Wed, 15 Apr 2020 17:26:37 -0300 Subject: [PATCH 11/13] Update Cocoapods. --- Pods | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Pods b/Pods index ca3f9cdb29..4106600d50 160000 --- a/Pods +++ b/Pods @@ -1 +1 @@ -Subproject commit ca3f9cdb29b13d71754e193b4b7965dad22921c4 +Subproject commit 4106600d5046a4c28a2aea8fba7c054cb8091ba8 From f6ce0ddfb447a5993de30b45e63fb215294e8df7 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Wed, 15 Apr 2020 17:28:24 -0300 Subject: [PATCH 12/13] "Bump build to 3.8.0.18." (Internal) --- NotificationServiceExtension/Info.plist | 2 +- Signal/Signal-Info.plist | 2 +- SignalShareExtension/Info.plist | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/NotificationServiceExtension/Info.plist b/NotificationServiceExtension/Info.plist index 7b4305c696..d09843e3dc 100644 --- a/NotificationServiceExtension/Info.plist +++ b/NotificationServiceExtension/Info.plist @@ -19,7 +19,7 @@ CFBundleShortVersionString 3.8.0 CFBundleVersion - 3.8.0.17 + 3.8.0.18 NSAppTransportSecurity NSExceptionDomains diff --git a/Signal/Signal-Info.plist b/Signal/Signal-Info.plist index be82ccb125..a4bf36cee3 100644 --- a/Signal/Signal-Info.plist +++ b/Signal/Signal-Info.plist @@ -45,7 +45,7 @@ CFBundleVersion - 3.8.0.17 + 3.8.0.18 ITSAppUsesNonExemptEncryption LOGS_EMAIL diff --git a/SignalShareExtension/Info.plist b/SignalShareExtension/Info.plist index c8b0bf2d5e..a6c5c79819 100644 --- a/SignalShareExtension/Info.plist +++ b/SignalShareExtension/Info.plist @@ -19,7 +19,7 @@ CFBundleShortVersionString 3.8.0 CFBundleVersion - 3.8.0.17 + 3.8.0.18 ITSAppUsesNonExemptEncryption NSAppTransportSecurity From 478225f7c2d6cb288597bcc4ccbba965bce8be57 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Wed, 15 Apr 2020 17:28:38 -0300 Subject: [PATCH 13/13] "Bump build to 3.8.0.19." (Internal) --- NotificationServiceExtension/Info.plist | 2 +- Signal/Signal-Info.plist | 2 +- SignalShareExtension/Info.plist | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/NotificationServiceExtension/Info.plist b/NotificationServiceExtension/Info.plist index d09843e3dc..916ce8418c 100644 --- a/NotificationServiceExtension/Info.plist +++ b/NotificationServiceExtension/Info.plist @@ -19,7 +19,7 @@ CFBundleShortVersionString 3.8.0 CFBundleVersion - 3.8.0.18 + 3.8.0.19 NSAppTransportSecurity NSExceptionDomains diff --git a/Signal/Signal-Info.plist b/Signal/Signal-Info.plist index a4bf36cee3..d6de9cb989 100644 --- a/Signal/Signal-Info.plist +++ b/Signal/Signal-Info.plist @@ -45,7 +45,7 @@ CFBundleVersion - 3.8.0.18 + 3.8.0.19 ITSAppUsesNonExemptEncryption LOGS_EMAIL diff --git a/SignalShareExtension/Info.plist b/SignalShareExtension/Info.plist index a6c5c79819..4c1031e4ca 100644 --- a/SignalShareExtension/Info.plist +++ b/SignalShareExtension/Info.plist @@ -19,7 +19,7 @@ CFBundleShortVersionString 3.8.0 CFBundleVersion - 3.8.0.18 + 3.8.0.19 ITSAppUsesNonExemptEncryption NSAppTransportSecurity