diff --git a/SignalMessaging/profiles/OWSProfileManager.h b/SignalMessaging/profiles/OWSProfileManager.h index f252f44ab5..64fba73cf1 100644 --- a/SignalMessaging/profiles/OWSProfileManager.h +++ b/SignalMessaging/profiles/OWSProfileManager.h @@ -76,8 +76,6 @@ extern const NSUInteger kOWSProfileManager_MaxAvatarDiameter; - (nullable UIImage *)profileAvatarForRecipientId:(NSString *)recipientId; -- (void)refreshProfileForRecipientId:(NSString *)recipientId; - - (void)updateProfileForRecipientId:(NSString *)recipientId profileNameEncrypted:(nullable NSData *)profileNameEncrypted avatarUrlPath:(nullable NSString *)avatarUrlPath; diff --git a/SignalMessaging/profiles/OWSProfileManager.m b/SignalMessaging/profiles/OWSProfileManager.m index 3ea178e507..5bc3f644da 100644 --- a/SignalMessaging/profiles/OWSProfileManager.m +++ b/SignalMessaging/profiles/OWSProfileManager.m @@ -722,20 +722,22 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640; OWSUserProfile *userProfile = [OWSUserProfile getOrBuildUserProfileForRecipientId:recipientId dbConnection:self.dbConnection]; + OWSAssert(userProfile); if (userProfile.profileKey && [userProfile.profileKey.keyData isEqual:profileKey.keyData]) { // Ignore redundant update. return; } - [userProfile updateWithProfileName:nil - profileKey:profileKey - avatarUrlPath:nil - avatarFileName:nil - dbConnection:self.dbConnection - completion:^{ - [self refreshProfileForRecipientId:recipientId ignoreThrottling:YES]; - }]; + [userProfile clearWithProfileKey:profileKey + dbConnection:self.dbConnection + completion:^{ + dispatch_async(dispatch_get_main_queue(), ^(void) { + [ProfileFetcherJob runWithRecipientId:recipientId + networkManager:self.networkManager + ignoreThrottling:YES]; + }); + }]; }); } @@ -751,6 +753,7 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640; OWSUserProfile *userProfile = [OWSUserProfile getOrBuildUserProfileForRecipientId:recipientId dbConnection:self.dbConnection]; OWSAssert(userProfile); + return userProfile.profileKey; } @@ -758,25 +761,24 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640; { OWSAssert(recipientId.length > 0); - [self refreshProfileForRecipientId:recipientId]; - OWSUserProfile *userProfile = [OWSUserProfile getOrBuildUserProfileForRecipientId:recipientId dbConnection:self.dbConnection]; + return userProfile.profileName; - return self.localUserProfile.profileName; } - (nullable UIImage *)profileAvatarForRecipientId:(NSString *)recipientId { OWSAssert(recipientId.length > 0); - [self refreshProfileForRecipientId:recipientId]; - OWSUserProfile *userProfile = [OWSUserProfile getOrBuildUserProfileForRecipientId:recipientId dbConnection:self.dbConnection]; + if (userProfile.avatarFileName.length > 0) { return [self loadProfileAvatarWithFilename:userProfile.avatarFileName]; - } else if (userProfile.avatarUrlPath.length > 0) { + } + + if (userProfile.avatarUrlPath.length > 0) { [self downloadAvatarForUserProfile:userProfile]; } @@ -892,46 +894,6 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640; }); } -- (void)refreshProfileForRecipientId:(NSString *)recipientId -{ - [self refreshProfileForRecipientId:recipientId ignoreThrottling:NO]; -} - -- (void)refreshProfileForRecipientId:(NSString *)recipientId ignoreThrottling:(BOOL)ignoreThrottling -{ - OWSAssert(recipientId.length > 0); - - dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{ - OWSUserProfile *userProfile = - [OWSUserProfile getOrBuildUserProfileForRecipientId:recipientId dbConnection:self.dbConnection]; - - if (!userProfile.profileKey) { - // There's no point in fetching the profile for a user - // if we don't have their profile key; we won't be able - // to decrypt it. - return; - } - - // Throttle and debounce the updates. - const NSTimeInterval kMaxRefreshFrequency = 5 * kMinuteInterval; - if (userProfile.lastUpdateDate - && fabs([userProfile.lastUpdateDate timeIntervalSinceNow]) < kMaxRefreshFrequency) { - // This profile was updated recently or already has an update in flight. - return; - } - - [userProfile updateWithLastUpdateDate:[NSDate new] - dbConnection:self.dbConnection - completion:^{ - dispatch_async(dispatch_get_main_queue(), ^{ - [ProfileFetcherJob runWithRecipientId:recipientId - networkManager:self.networkManager - ignoreThrottling:ignoreThrottling]; - }); - }]; - }); -} - - (void)updateProfileForRecipientId:(NSString *)recipientId profileNameEncrypted:(nullable NSData *)profileNameEncrypted avatarUrlPath:(nullable NSString *)avatarUrlPath; @@ -948,6 +910,7 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640; dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{ OWSUserProfile *userProfile = [OWSUserProfile getOrBuildUserProfileForRecipientId:recipientId dbConnection:self.dbConnection]; + if (!userProfile.profileKey) { return; } @@ -955,12 +918,9 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640; NSString *_Nullable profileName = [self decryptProfileNameData:profileNameEncrypted profileKey:userProfile.profileKey]; - BOOL isAvatarSame = [self isNullableStringEqual:userProfile.avatarUrlPath toString:avatarUrlPath]; - [userProfile updateWithProfileName:profileName avatarUrlPath:avatarUrlPath - avatarFileName:nil - lastUpdateDate:[NSDate new] + avatarFileName:userProfile.avatarFileName // use existing file name if already downloaded dbConnection:self.dbConnection completion:nil]; @@ -978,15 +938,12 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640; // downloaded the latest avatar by downloadAvatarForUserProfile. [localUserProfile updateWithProfileName:profileName avatarUrlPath:avatarUrlPath - lastUpdateDate:[NSDate new] dbConnection:self.dbConnection completion:nil]; } - if (!isAvatarSame) { - if (avatarUrlPath) { - [self downloadAvatarForUserProfile:userProfile]; - } + if (userProfile.avatarUrlPath.length > 0 && userProfile.avatarFileName.length == 0) { + [self downloadAvatarForUserProfile:userProfile]; } }); } diff --git a/SignalMessaging/profiles/OWSUserProfile.h b/SignalMessaging/profiles/OWSUserProfile.h index 0f9b5715b3..c58e59e792 100644 --- a/SignalMessaging/profiles/OWSUserProfile.h +++ b/SignalMessaging/profiles/OWSUserProfile.h @@ -32,12 +32,6 @@ extern NSString *const kLocalProfileUniqueId; // This filename is relative to OWSProfileManager.profileAvatarsDirPath. @property (atomic, readonly, nullable) NSString *avatarFileName; -// This should reflect when either: -// -// * The last successful update finished. -// * The current in-flight update began. -@property (atomic, readonly, nullable) NSDate *lastUpdateDate; - - (instancetype)init NS_UNAVAILABLE; + (OWSUserProfile *)getOrBuildUserProfileForRecipientId:(NSString *)recipientId @@ -48,7 +42,6 @@ extern NSString *const kLocalProfileUniqueId; #pragma mark - Update With... Methods - (void)updateWithProfileName:(nullable NSString *)profileName - profileKey:(OWSAES256Key *)profileKey avatarUrlPath:(nullable NSString *)avatarUrlPath avatarFileName:(nullable NSString *)avatarFileName dbConnection:(YapDatabaseConnection *)dbConnection @@ -56,20 +49,6 @@ extern NSString *const kLocalProfileUniqueId; - (void)updateWithProfileName:(nullable NSString *)profileName avatarUrlPath:(nullable NSString *)avatarUrlPath - avatarFileName:(nullable NSString *)avatarFileName - dbConnection:(YapDatabaseConnection *)dbConnection - completion:(nullable OWSUserProfileCompletion)completion; - -- (void)updateWithProfileName:(nullable NSString *)profileName - avatarUrlPath:(nullable NSString *)avatarUrlPath - avatarFileName:(nullable NSString *)avatarFileName - lastUpdateDate:(nullable NSDate *)lastUpdateDate - dbConnection:(YapDatabaseConnection *)dbConnection - completion:(nullable OWSUserProfileCompletion)completion; - -- (void)updateWithProfileName:(nullable NSString *)profileName - avatarUrlPath:(nullable NSString *)avatarUrlPath - lastUpdateDate:(nullable NSDate *)lastUpdateDate dbConnection:(YapDatabaseConnection *)dbConnection completion:(nullable OWSUserProfileCompletion)completion; @@ -82,10 +61,6 @@ extern NSString *const kLocalProfileUniqueId; dbConnection:(YapDatabaseConnection *)dbConnection completion:(nullable OWSUserProfileCompletion)completion; -- (void)updateWithLastUpdateDate:(nullable NSDate *)lastUpdateDate - dbConnection:(YapDatabaseConnection *)dbConnection - completion:(nullable OWSUserProfileCompletion)completion; - - (void)clearWithProfileKey:(OWSAES256Key *)profileKey dbConnection:(YapDatabaseConnection *)dbConnection completion:(nullable OWSUserProfileCompletion)completion; diff --git a/SignalMessaging/profiles/OWSUserProfile.m b/SignalMessaging/profiles/OWSUserProfile.m index cb77272138..2e4294bada 100644 --- a/SignalMessaging/profiles/OWSUserProfile.m +++ b/SignalMessaging/profiles/OWSUserProfile.m @@ -29,7 +29,6 @@ NSString *const kLocalProfileUniqueId = @"kLocalProfileUniqueId"; @property (atomic, nullable) NSString *profileName; @property (atomic, nullable) NSString *avatarUrlPath; @property (atomic, nullable) NSString *avatarFileName; -@property (atomic, nullable) NSDate *lastUpdateDate; @end @@ -106,7 +105,7 @@ NSString *const kLocalProfileUniqueId = @"kLocalProfileUniqueId"; dbConnection:(YapDatabaseConnection *)dbConnection completion:(nullable OWSUserProfileCompletion)completion { - id beforeSnapshot = self.dictionaryValue; + NSDictionary *beforeSnapshot = self.dictionaryValue; changeBlock(self); @@ -117,12 +116,19 @@ NSString *const kLocalProfileUniqueId = @"kLocalProfileUniqueId"; if (latestInstance) { changeBlock(latestInstance); - id afterSnapshot = latestInstance.dictionaryValue; + NSDictionary *afterSnapshot = latestInstance.dictionaryValue; if ([beforeSnapshot isEqual:afterSnapshot]) { DDLogVerbose( @"%@ Ignoring redundant update in %s: %@", self.logTag, functionName, self.debugDescription); didChange = NO; } else { + NSString *_Nullable oldAvatarUrlPath = beforeSnapshot[@"avatarUrlPath"]; + if (!latestInstance.avatarUrlPath || ![latestInstance.avatarUrlPath isEqual:oldAvatarUrlPath]) { + // If the avatarURL changed, the avatarFileName can't be valid. + // Clear it. + latestInstance.avatarFileName = nil; + } + [latestInstance saveWithTransaction:transaction]; } } else { @@ -187,50 +193,12 @@ NSString *const kLocalProfileUniqueId = @"kLocalProfileUniqueId"; - (void)updateWithProfileName:(nullable NSString *)profileName avatarUrlPath:(nullable NSString *)avatarUrlPath - avatarFileName:(nullable NSString *)avatarFileName - lastUpdateDate:(nullable NSDate *)lastUpdateDate dbConnection:(YapDatabaseConnection *)dbConnection completion:(nullable OWSUserProfileCompletion)completion { [self applyChanges:^(OWSUserProfile *userProfile) { [userProfile setProfileName:[profileName ows_stripped]]; [userProfile setAvatarUrlPath:avatarUrlPath]; - [userProfile setAvatarFileName:avatarFileName]; - [userProfile setLastUpdateDate:lastUpdateDate]; - } - functionName:__PRETTY_FUNCTION__ - dbConnection:dbConnection - completion:completion]; -} - -- (void)updateWithProfileName:(nullable NSString *)profileName - avatarUrlPath:(nullable NSString *)avatarUrlPath - lastUpdateDate:(nullable NSDate *)lastUpdateDate - dbConnection:(YapDatabaseConnection *)dbConnection - completion:(nullable OWSUserProfileCompletion)completion -{ - [self applyChanges:^(OWSUserProfile *userProfile) { - [userProfile setProfileName:[profileName ows_stripped]]; - [userProfile setAvatarUrlPath:avatarUrlPath]; - [userProfile setLastUpdateDate:lastUpdateDate]; - } - functionName:__PRETTY_FUNCTION__ - dbConnection:dbConnection - completion:completion]; -} - -- (void)updateWithProfileName:(nullable NSString *)profileName - profileKey:(OWSAES256Key *)profileKey - avatarUrlPath:(nullable NSString *)avatarUrlPath - avatarFileName:(nullable NSString *)avatarFileName - dbConnection:(YapDatabaseConnection *)dbConnection - completion:(nullable OWSUserProfileCompletion)completion -{ - [self applyChanges:^(OWSUserProfile *userProfile) { - [userProfile setProfileName:[profileName ows_stripped]]; - [userProfile setProfileKey:profileKey]; - [userProfile setAvatarUrlPath:avatarUrlPath]; - [userProfile setAvatarFileName:avatarFileName]; } functionName:__PRETTY_FUNCTION__ dbConnection:dbConnection @@ -263,18 +231,6 @@ NSString *const kLocalProfileUniqueId = @"kLocalProfileUniqueId"; completion:completion]; } -- (void)updateWithLastUpdateDate:(nullable NSDate *)lastUpdateDate - dbConnection:(YapDatabaseConnection *)dbConnection - completion:(nullable OWSUserProfileCompletion)completion -{ - [self applyChanges:^(OWSUserProfile *userProfile) { - [userProfile setLastUpdateDate:lastUpdateDate]; - } - functionName:__PRETTY_FUNCTION__ - dbConnection:dbConnection - completion:completion]; -} - - (void)clearWithProfileKey:(OWSAES256Key *)profileKey dbConnection:(YapDatabaseConnection *)dbConnection completion:(nullable OWSUserProfileCompletion)completion; @@ -284,7 +240,6 @@ NSString *const kLocalProfileUniqueId = @"kLocalProfileUniqueId"; [userProfile setProfileName:nil]; [userProfile setAvatarUrlPath:nil]; [userProfile setAvatarFileName:nil]; - [userProfile setLastUpdateDate:nil]; } functionName:__PRETTY_FUNCTION__ dbConnection:dbConnection @@ -338,15 +293,14 @@ NSString *const kLocalProfileUniqueId = @"kLocalProfileUniqueId"; // This should only be used in verbose, developer-only logs. - (NSString *)debugDescription { - return [NSString stringWithFormat:@"%@ %p %@ %zd %@ %@ %@ %f", + return [NSString stringWithFormat:@"%@ %p %@ %zd %@ %@ %@", self.logTag, self, self.recipientId, self.profileKey.keyData.length, self.profileName, self.avatarUrlPath, - self.avatarFileName, - self.lastUpdateDate.timeIntervalSinceNow]; + self.avatarFileName]; } @end diff --git a/SignalMessaging/profiles/ProfileFetcherJob.swift b/SignalMessaging/profiles/ProfileFetcherJob.swift index 8b4f9e0e1e..ddf3afa74d 100644 --- a/SignalMessaging/profiles/ProfileFetcherJob.swift +++ b/SignalMessaging/profiles/ProfileFetcherJob.swift @@ -38,6 +38,13 @@ public class ProfileFetcherJob: NSObject { public func run(recipientIds: [String]) { AssertIsOnMainThread() + if (!CurrentAppContext().isMainApp) { + // Only refresh profiles in the MainApp to decrease the chance of missed SN notifications + // in the AppExtension for our users who choose not to verify contacts. + owsFail("Should only fetch profiles in the main app") + return + } + DispatchQueue.main.async { for recipientId in recipientIds { self.updateProfile(recipientId: recipientId) @@ -74,11 +81,11 @@ public class ProfileFetcherJob: NSObject { if !ignoreThrottling { if let lastDate = ProfileFetcherJob.fetchDateMap[recipientId] { let lastTimeInterval = fabs(lastDate.timeIntervalSinceNow) - // Don't check a profile more often than every N minutes. + // Don't check a profile more often than every N seconds. // - // Only throttle profile fetch in production builds in order to - // facilitate debugging. - let kGetProfileMaxFrequencySeconds = _isDebugAssertConfiguration() ? 0 : 60.0 * 5.0 + // Throttle less in debug to make it easier to test problems + // with our fetching logic. + let kGetProfileMaxFrequencySeconds = _isDebugAssertConfiguration() ? 60 : 60.0 * 5.0 guard lastTimeInterval > kGetProfileMaxFrequencySeconds else { return Promise(error: ProfileFetcherJobError.throttled(lastTimeInterval: lastTimeInterval)) } diff --git a/SignalShareExtension/ShareViewController.swift b/SignalShareExtension/ShareViewController.swift index 0c331488e6..1bdf28ad41 100644 --- a/SignalShareExtension/ShareViewController.swift +++ b/SignalShareExtension/ShareViewController.swift @@ -206,7 +206,7 @@ public class ShareViewController: UINavigationController, ShareViewDelegate, SAE // We don't need to use OWSOrphanedDataCleaner in the SAE. - OWSProfileManager.shared().fetchLocalUsersProfile() + // We don't need to fetch the local profile in the SAE OWSReadReceiptManager.shared().prepareCachedValues()