From 911c4d380b35fca007c65ead55cfcb3b9d6afb9c Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Fri, 8 Dec 2017 13:22:01 -0500 Subject: [PATCH 01/11] Rework thread safety in profile manager. --- Signal.xcodeproj/project.pbxproj | 8 + Signal/src/AppDelegate.m | 1 - SignalMessaging/profiles/OWSProfileManager.m | 1049 +++++++----------- SignalMessaging/profiles/OWSUserProfile.h | 83 ++ SignalMessaging/profiles/OWSUserProfile.m | 250 +++++ 5 files changed, 762 insertions(+), 629 deletions(-) create mode 100644 SignalMessaging/profiles/OWSUserProfile.h create mode 100644 SignalMessaging/profiles/OWSUserProfile.m diff --git a/Signal.xcodeproj/project.pbxproj b/Signal.xcodeproj/project.pbxproj index 337d6bb963..85ec492cb0 100644 --- a/Signal.xcodeproj/project.pbxproj +++ b/Signal.xcodeproj/project.pbxproj @@ -107,6 +107,8 @@ 3478506A1FD9B78A007B8332 /* AppSetup.h in Headers */ = {isa = PBXBuildFile; fileRef = 347850661FD9B789007B8332 /* AppSetup.h */; settings = {ATTRIBUTES = (Public, ); }; }; 3478506B1FD9B78A007B8332 /* NoopCallMessageHandler.swift in Sources */ = {isa = PBXBuildFile; fileRef = 347850671FD9B78A007B8332 /* NoopCallMessageHandler.swift */; }; 3478506C1FD9B78A007B8332 /* NoopNotificationsManager.swift in Sources */ = {isa = PBXBuildFile; fileRef = 347850681FD9B78A007B8332 /* NoopNotificationsManager.swift */; }; + 347850711FDAEB17007B8332 /* OWSUserProfile.m in Sources */ = {isa = PBXBuildFile; fileRef = 3478506F1FDAEB16007B8332 /* OWSUserProfile.m */; }; + 347850721FDAEB17007B8332 /* OWSUserProfile.h in Headers */ = {isa = PBXBuildFile; fileRef = 347850701FDAEB16007B8332 /* OWSUserProfile.h */; }; 3497DBEC1ECE257500DB2605 /* OWSCountryMetadata.m in Sources */ = {isa = PBXBuildFile; fileRef = 3497DBEB1ECE257500DB2605 /* OWSCountryMetadata.m */; }; 3497DBEF1ECE2E4700DB2605 /* DomainFrontingCountryViewController.m in Sources */ = {isa = PBXBuildFile; fileRef = 3497DBEE1ECE2E4700DB2605 /* DomainFrontingCountryViewController.m */; }; 34B0796D1FCF46B100E248C2 /* MainAppContext.m in Sources */ = {isa = PBXBuildFile; fileRef = 34B0796B1FCF46B000E248C2 /* MainAppContext.m */; }; @@ -584,6 +586,8 @@ 347850661FD9B789007B8332 /* AppSetup.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = AppSetup.h; sourceTree = ""; }; 347850671FD9B78A007B8332 /* NoopCallMessageHandler.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = NoopCallMessageHandler.swift; sourceTree = ""; }; 347850681FD9B78A007B8332 /* NoopNotificationsManager.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = NoopNotificationsManager.swift; sourceTree = ""; }; + 3478506F1FDAEB16007B8332 /* OWSUserProfile.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = OWSUserProfile.m; sourceTree = ""; }; + 347850701FDAEB16007B8332 /* OWSUserProfile.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = OWSUserProfile.h; sourceTree = ""; }; 348F2EAD1F0D21BC00D4ECE0 /* DeviceSleepManager.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = DeviceSleepManager.swift; sourceTree = ""; }; 3495BC911F1426B800B478F5 /* ar */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = ar; path = translations/ar.lproj/Localizable.strings; sourceTree = ""; }; 3497DBEA1ECE257500DB2605 /* OWSCountryMetadata.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = OWSCountryMetadata.h; sourceTree = ""; }; @@ -1225,6 +1229,8 @@ children = ( 346129B11FD1F7E800532771 /* OWSProfileManager.h */, 346129B21FD1F7E800532771 /* OWSProfileManager.m */, + 347850701FDAEB16007B8332 /* OWSUserProfile.h */, + 3478506F1FDAEB16007B8332 /* OWSUserProfile.m */, 346129B31FD1F7E800532771 /* ProfileFetcherJob.swift */, ); path = profiles; @@ -2041,6 +2047,7 @@ 3461295A1FD1D74C00532771 /* Environment.h in Headers */, 34480B631FD0A98800BC14EF /* UIView+OWS.h in Headers */, 451F8A4B1FD715E1005CB9DA /* OWSGroupAvatarBuilder.h in Headers */, + 347850721FDAEB17007B8332 /* OWSUserProfile.h in Headers */, 451F8A371FD71179005CB9DA /* OWSViewController.h in Headers */, 454A965D1FD602B1008D2A0E /* OWSAudioAttachmentPlayer.h in Headers */, 451F8A3E1FD713D2005CB9DA /* ThreadViewHelper.h in Headers */, @@ -2730,6 +2737,7 @@ 346129F71FD5F31400532771 /* OWS105AttachmentFilePaths.m in Sources */, 45194F931FD7215C00333B2C /* OWSContactOffersInteraction.m in Sources */, 450998681FD8C0FF00D89EB3 /* AttachmentSharing.m in Sources */, + 347850711FDAEB17007B8332 /* OWSUserProfile.m in Sources */, 346129761FD1E0B500532771 /* WeakTimer.swift in Sources */, 346129F81FD5F31400532771 /* OWS100RemoveTSRecipientsMigration.m in Sources */, 45E547201FD755E700DFC09E /* AttachmentApprovalViewController.swift in Sources */, diff --git a/Signal/src/AppDelegate.m b/Signal/src/AppDelegate.m index 4bd30344ce..b6b3d4f5a6 100644 --- a/Signal/src/AppDelegate.m +++ b/Signal/src/AppDelegate.m @@ -356,7 +356,6 @@ static NSString *const kURLHostVerifyPrefix = @"verify"; return NO; } - - (void)applicationDidBecomeActive:(UIApplication *)application { DDLogWarn(@"%@ applicationDidBecomeActive.", self.logTag); diff --git a/SignalMessaging/profiles/OWSProfileManager.m b/SignalMessaging/profiles/OWSProfileManager.m index 908e1d4eb6..f0f0910fea 100644 --- a/SignalMessaging/profiles/OWSProfileManager.m +++ b/SignalMessaging/profiles/OWSProfileManager.m @@ -5,6 +5,7 @@ #import "OWSProfileManager.h" #import "Environment.h" #import "NSString+OWS.h" +#import "OWSUserProfile.h" #import "UIImage+OWS.h" #import #import @@ -32,69 +33,6 @@ NS_ASSUME_NONNULL_BEGIN -// UserProfile properties may be read from any thread, but should -// only be mutated when synchronized on the profile manager. -@interface UserProfile : TSYapDatabaseObject - -@property (atomic, readonly) NSString *recipientId; -@property (atomic, nullable) OWSAES256Key *profileKey; -@property (atomic, nullable) NSString *profileName; -@property (atomic, nullable) NSString *avatarUrlPath; -// This filename is relative to OWSProfileManager.profileAvatarsDirPath. -@property (atomic, nullable) NSString *avatarFileName; - -// This should reflect when either: -// -// * The last successful update finished. -// * The current in-flight update began. -@property (atomic, nullable) NSDate *lastUpdateDate; - -- (instancetype)init NS_UNAVAILABLE; - -@end - -#pragma mark - - -@implementation UserProfile - -@synthesize profileName = _profileName; - -- (instancetype)initWithRecipientId:(NSString *)recipientId -{ - self = [super initWithUniqueId:recipientId]; - - if (!self) { - return self; - } - - OWSAssert(recipientId.length > 0); - _recipientId = recipientId; - - return self; -} - -- (nullable NSString *)profileName -{ - @synchronized(self) - { - return _profileName; - } -} - -- (void)setProfileName:(nullable NSString *)profileName -{ - @synchronized(self) - { - _profileName = [profileName ows_stripped]; - } -} - -@end - -#pragma mark - - -NSString *const kLocalProfileUniqueId = @"kLocalProfileUniqueId"; - NSString *const kNSNotificationName_LocalProfileDidChange = @"kNSNotificationName_LocalProfileDidChange"; NSString *const kNSNotificationName_OtherUsersProfileWillChange = @"kNSNotificationName_OtherUsersProfileWillChange"; NSString *const kNSNotificationName_OtherUsersProfileDidChange = @"kNSNotificationName_OtherUsersProfileDidChange"; @@ -118,19 +56,12 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640; @property (nonatomic, readonly) OWSIdentityManager *identityManager; // This property can be accessed on any thread, while synchronized on self. -@property (nonatomic, readonly) UserProfile *localUserProfile; -// This property can be accessed on any thread, while synchronized on self. -@property (atomic, nullable) UIImage *localCachedAvatarImage; - -// These caches are lazy-populated. The single point of truth is the database. -// -// This property can be accessed on any thread, while synchronized on self. -@property (atomic, readonly) NSMutableDictionary *userProfileWhitelistCache; -// This property can be accessed on any thread, while synchronized on self. -@property (atomic, readonly) NSMutableDictionary *groupProfileWhitelistCache; +@property (atomic, readonly) OWSUserProfile *localUserProfile; // This property can be accessed on any thread, while synchronized on self. -@property (atomic, readonly) NSCache *otherUsersProfileAvatarImageCache; +//@property (atomic, readonly) NSCache *otherUsersProfileAvatarImageCache; +@property (atomic, readonly) NSCache *profileAvatarImageCache; + // This property can be accessed on any thread, while synchronized on self. @property (atomic, readonly) NSMutableSet *currentAvatarDownloads; @@ -182,9 +113,7 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640; _dbConnection = storageManager.newDatabaseConnection; _networkManager = networkManager; - _userProfileWhitelistCache = [NSMutableDictionary new]; - _groupProfileWhitelistCache = [NSMutableDictionary new]; - _otherUsersProfileAvatarImageCache = [NSCache new]; + _profileAvatarImageCache = [NSCache new]; _currentAvatarDownloads = [NSMutableSet new]; OWSSingletonAssert(); @@ -217,46 +146,11 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640; #pragma mark - User Profile Accessor -// This method can be safely called from any thread. -- (UserProfile *)getOrBuildUserProfileForRecipientId:(NSString *)recipientId -{ - OWSAssert(recipientId.length > 0); - - __block UserProfile *instance; - // Make sure to read on the local db connection for consistency. - [self.dbConnection readWithBlock:^(YapDatabaseReadTransaction *transaction) { - instance = [UserProfile fetchObjectWithUniqueID:recipientId transaction:transaction]; - }]; - - if (!instance) { - instance = [[UserProfile alloc] initWithRecipientId:recipientId]; - } - - OWSAssert(instance); - - return instance; -} - -- (void)saveUserProfile:(UserProfile *)userProfile +- (OWSUserProfileCompletion)userProfileSaveCompletion:(OWSUserProfile *)userProfile { OWSAssert(userProfile); - // Make a copy to use inside the transaction. - // To avoid deadlock, we want to avoid creating a new transaction while sync'd on self. - UserProfile *userProfileCopy; - @synchronized(self) - { - userProfileCopy = [userProfile copy]; - // Other threads may modify this profile's properties - OWSAssert([userProfile isEqual:userProfileCopy]); - } - - dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{ - // Make sure to save on the local db connection for consistency. - [self.dbConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { - [userProfileCopy saveWithTransaction:transaction]; - }]; - + return ^{ BOOL isLocalUserProfile = userProfile == self.localUserProfile; dispatch_async(dispatch_get_main_queue(), ^{ @@ -287,51 +181,38 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640; }]; } }); - }); + }; } - (void)ensureLocalProfileCached { // Since localUserProfile can create a transaction, we want to make sure it's not called for the first // time unexpectedly (e.g. in a nested transaction.) - __unused UserProfile *profile = [self localUserProfile]; + __unused OWSUserProfile *profile = [self localUserProfile]; } #pragma mark - Local Profile -- (UserProfile *)localUserProfile +- (OWSUserProfile *)localUserProfile { @synchronized(self) { - if (_localUserProfile == nil) { - // Make sure to read on the local db connection for consistency. - [self.dbConnection readWithBlock:^(YapDatabaseReadTransaction *transaction) { - _localUserProfile = [UserProfile fetchObjectWithUniqueID:kLocalProfileUniqueId transaction:transaction]; - }]; - - if (_localUserProfile == nil) { - DDLogInfo(@"%@ Building local profile.", self.logTag); - _localUserProfile = [[UserProfile alloc] initWithRecipientId:kLocalProfileUniqueId]; - _localUserProfile.profileKey = [OWSAES256Key generateRandomKey]; - - dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{ - [self saveUserProfile:_localUserProfile]; - }); - } + if (!_localUserProfile) { + _localUserProfile = [OWSUserProfile getOrBuildUserProfileForRecipientId:kLocalProfileUniqueId + dbConnection:self.dbConnection]; } - - return _localUserProfile; } + + OWSAssert(_localUserProfile.profileKey); + + return _localUserProfile; } - (OWSAES256Key *)localProfileKey { - @synchronized(self) - { - OWSAssert(self.localUserProfile.profileKey.keyData.length == kAES256_KeyByteLength); + OWSAssert(self.localUserProfile.profileKey.keyData.length == kAES256_KeyByteLength); - return self.localUserProfile.profileKey; - } + return self.localUserProfile.profileKey; } - (BOOL)hasLocalProfile @@ -341,24 +222,12 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640; - (nullable NSString *)localProfileName { - @synchronized(self) - { - return self.localUserProfile.profileName; - } + return self.localUserProfile.profileName; } - (nullable UIImage *)localProfileAvatarImage { - @synchronized(self) - { - if (!self.localCachedAvatarImage) { - if (self.localUserProfile.avatarFileName) { - self.localCachedAvatarImage = [self loadProfileAvatarWithFilename:self.localUserProfile.avatarFileName]; - } - } - - return self.localCachedAvatarImage; - } + return [self loadProfileAvatarWithFilename:self.localUserProfile.avatarFileName]; } - (void)updateLocalProfileName:(nullable NSString *)profileName @@ -369,108 +238,97 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640; OWSAssert(successBlockParameter); OWSAssert(failureBlockParameter); - dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{ - @synchronized(self) - { - // Ensure that the success and failure blocks are called on the main thread. - void (^failureBlock)(void) = ^{ - DDLogError(@"%@ Updating service with profile failed.", self.logTag); + // Ensure that the success and failure blocks are called on the main thread. + void (^failureBlock)(void) = ^{ + DDLogError(@"%@ Updating service with profile failed.", self.logTag); - dispatch_async(dispatch_get_main_queue(), ^{ - failureBlockParameter(); - }); - }; - void (^successBlock)(void) = ^{ - DDLogInfo(@"%@ Successfully updated service with profile.", self.logTag); + dispatch_async(dispatch_get_main_queue(), ^{ + failureBlockParameter(); + }); + }; + void (^successBlock)(void) = ^{ + DDLogInfo(@"%@ Successfully updated service with profile.", self.logTag); - dispatch_async(dispatch_get_main_queue(), ^{ - successBlockParameter(); - }); - }; + dispatch_async(dispatch_get_main_queue(), ^{ + successBlockParameter(); + }); + }; - // The final steps are to: - // - // * Try to update the service. - // * Update client state on success. - void (^tryToUpdateService)(NSString *_Nullable, NSString *_Nullable) - = ^(NSString *_Nullable avatarUrlPath, NSString *_Nullable avatarFileName) { - [self updateServiceWithProfileName:profileName - success:^{ - dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{ - @synchronized(self) - { - UserProfile *userProfile = self.localUserProfile; - OWSAssert(userProfile); - userProfile.profileName = profileName; + // The final steps are to: + // + // * Try to update the service. + // * Update client state on success. + void (^tryToUpdateService)(NSString *_Nullable, NSString *_Nullable) = ^( + NSString *_Nullable avatarUrlPath, NSString *_Nullable avatarFileName) { + [self updateServiceWithProfileName:profileName + success:^{ + OWSUserProfile *userProfile = self.localUserProfile; + OWSAssert(userProfile); - // TODO remote avatarUrlPath changes as result of fetching form - - // we should probably invalidate it at that point, and refresh again when - // uploading file completes. - userProfile.avatarUrlPath = avatarUrlPath; - userProfile.avatarFileName = avatarFileName; + [userProfile updateWithProfileName:profileName + avatarUrlPath:avatarUrlPath + avatarFileName:avatarFileName + dbConnection:self.dbConnection + completion:^{ + if (avatarFileName) { + [self updateProfileAvatarCache:avatarImage filename:avatarFileName]; + } - [self saveUserProfile:userProfile]; + successBlock(); + }]; + } + failure:^{ + failureBlock(); + }]; + }; - self.localCachedAvatarImage = avatarImage; + OWSUserProfile *userProfile = self.localUserProfile; + OWSAssert(userProfile); - successBlock(); - } - }); - } - failure:^{ - failureBlock(); - }]; - }; + if (avatarImage) { + // If we have a new avatar image, we must first: + // + // * Encode it to JPEG. + // * Write it to disk. + // * Encrypt it + // * Upload it to asset service + // * Send asset service info to Signal Service + if (self.localProfileAvatarImage == avatarImage) { + OWSAssert(userProfile.avatarUrlPath.length > 0); + OWSAssert(userProfile.avatarFileName.length > 0); - UserProfile *userProfile = self.localUserProfile; - OWSAssert(userProfile); - - if (avatarImage) { - // If we have a new avatar image, we must first: - // - // * Encode it to JPEG. - // * Write it to disk. - // * Encrypt it - // * Upload it to asset service - // * Send asset service info to Signal Service - if (self.localCachedAvatarImage == avatarImage) { - OWSAssert(userProfile.avatarUrlPath.length > 0); - OWSAssert(userProfile.avatarFileName.length > 0); - - DDLogVerbose(@"%@ Updating local profile on service with unchanged avatar.", self.logTag); - // If the avatar hasn't changed, reuse the existing metadata. - tryToUpdateService(userProfile.avatarUrlPath, userProfile.avatarFileName); - } else { - DDLogVerbose(@"%@ Updating local profile on service with new avatar.", self.logTag); - [self writeAvatarToDisk:avatarImage - success:^(NSData *data, NSString *fileName) { - [self uploadAvatarToService:data - success:^(NSString *_Nullable avatarUrlPath) { - tryToUpdateService(avatarUrlPath, fileName); - } - failure:^{ - failureBlock(); - }]; + DDLogVerbose(@"%@ Updating local profile on service with unchanged avatar.", self.logTag); + // If the avatar hasn't changed, reuse the existing metadata. + tryToUpdateService(userProfile.avatarUrlPath, userProfile.avatarFileName); + } else { + DDLogVerbose(@"%@ Updating local profile on service with new avatar.", self.logTag); + [self writeAvatarToDisk:avatarImage + success:^(NSData *data, NSString *fileName) { + [self uploadAvatarToService:data + success:^(NSString *_Nullable avatarUrlPath) { + tryToUpdateService(avatarUrlPath, fileName); } failure:^{ failureBlock(); }]; } - } else if (userProfile.avatarUrlPath) { - DDLogVerbose(@"%@ Updating local profile on service with cleared avatar.", self.logTag); - [self uploadAvatarToService:nil - success:^(NSString *_Nullable avatarUrlPath) { - tryToUpdateService(nil, nil); - } - failure:^{ - failureBlock(); - }]; - } else { - DDLogVerbose(@"%@ Updating local profile on service with no avatar.", self.logTag); + failure:^{ + failureBlock(); + }]; + } + } else if (userProfile.avatarUrlPath) { + DDLogVerbose(@"%@ Updating local profile on service with cleared avatar.", self.logTag); + [self uploadAvatarToService:nil + success:^(NSString *_Nullable avatarUrlPath) { tryToUpdateService(nil, nil); } - } - }); + failure:^{ + failureBlock(); + }]; + } else { + DDLogVerbose(@"%@ Updating local profile on service with no avatar.", self.logTag); + tryToUpdateService(nil, nil); + } } - (void)writeAvatarToDisk:(UIImage *)avatar @@ -539,23 +397,8 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640; // TODO: Revisit this so that failed profile updates don't leave // the profile avatar blank, etc. void (^clearLocalAvatar)(void) = ^{ - dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{ - @synchronized(self) - { - UserProfile *userProfile = self.localUserProfile; - OWSAssert(userProfile); - - // TODO remote avatarUrlPath changes as result of fetching form - - // we should probably invalidate it at that point, and refresh again when - // uploading file completes. - userProfile.avatarUrlPath = nil; - userProfile.avatarFileName = nil; - - [self saveUserProfile:userProfile]; - - self.localCachedAvatarImage = nil; - } - }); + OWSUserProfile *userProfile = self.localUserProfile; + [userProfile updateWithAvatarUrlPath:nil avatarFileName:nil dbConnection:self.dbConnection completion:nil]; }; dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{ @@ -718,62 +561,36 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640; { DDLogWarn(@"%@ Clearing the profile whitelist.", self.logTag); - dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{ - @synchronized(self) - { - [self.userProfileWhitelistCache removeAllObjects]; - [self.groupProfileWhitelistCache removeAllObjects]; - - [self.dbConnection purgeCollection:kOWSProfileManager_UserWhitelistCollection]; - [self.dbConnection purgeCollection:kOWSProfileManager_GroupWhitelistCollection]; - OWSAssert(0 == [self.dbConnection numberOfKeysInCollection:kOWSProfileManager_UserWhitelistCollection]); - OWSAssert(0 == [self.dbConnection numberOfKeysInCollection:kOWSProfileManager_GroupWhitelistCollection]); - } - }); + [self.dbConnection asyncReadWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { + [transaction removeAllObjectsInCollection:kOWSProfileManager_UserWhitelistCollection]; + [transaction removeAllObjectsInCollection:kOWSProfileManager_GroupWhitelistCollection]; + OWSAssert(0 == [transaction numberOfKeysInCollection:kOWSProfileManager_UserWhitelistCollection]); + OWSAssert(0 == [transaction numberOfKeysInCollection:kOWSProfileManager_GroupWhitelistCollection]); + }]; } - (void)logProfileWhitelist { - dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{ - @synchronized(self) - { - DDLogError(@"userProfileWhitelistCache: %zd", self.userProfileWhitelistCache.count); - DDLogError(@"groupProfileWhitelistCache: %zd", self.groupProfileWhitelistCache.count); - DDLogError(@"kOWSProfileManager_UserWhitelistCollection: %zd", - [self.dbConnection numberOfKeysInCollection:kOWSProfileManager_UserWhitelistCollection]); - [self.dbConnection readWithBlock:^(YapDatabaseReadTransaction *transaction) { - [transaction enumerateKeysInCollection:kOWSProfileManager_UserWhitelistCollection - usingBlock:^(NSString *_Nonnull key, BOOL *_Nonnull stop) { - DDLogError(@"\t profile whitelist user: %@", key); - }]; - }]; - DDLogError(@"kOWSProfileManager_GroupWhitelistCollection: %zd", - [self.dbConnection numberOfKeysInCollection:kOWSProfileManager_GroupWhitelistCollection]); - [self.dbConnection readWithBlock:^(YapDatabaseReadTransaction *transaction) { - [transaction enumerateKeysInCollection:kOWSProfileManager_GroupWhitelistCollection - usingBlock:^(NSString *_Nonnull key, BOOL *_Nonnull stop) { - DDLogError(@"\t profile whitelist group: %@", key); - }]; - }]; - } - }); + [self.dbConnection asyncReadWithBlock:^(YapDatabaseReadTransaction *transaction) { + DDLogError(@"kOWSProfileManager_UserWhitelistCollection: %zd", + [transaction numberOfKeysInCollection:kOWSProfileManager_UserWhitelistCollection]); + [transaction enumerateKeysInCollection:kOWSProfileManager_UserWhitelistCollection + usingBlock:^(NSString *_Nonnull key, BOOL *_Nonnull stop) { + DDLogError(@"\t profile whitelist user: %@", key); + }]; + DDLogError(@"kOWSProfileManager_GroupWhitelistCollection: %zd", + [transaction numberOfKeysInCollection:kOWSProfileManager_GroupWhitelistCollection]); + [transaction enumerateKeysInCollection:kOWSProfileManager_GroupWhitelistCollection + usingBlock:^(NSString *_Nonnull key, BOOL *_Nonnull stop) { + DDLogError(@"\t profile whitelist group: %@", key); + }]; + }]; } - (void)regenerateLocalProfile { - dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{ - @synchronized(self) - { - _localUserProfile = nil; - DDLogWarn(@"%@ Removing local user profile", self.logTag); - [self.dbConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *_Nonnull transaction) { - [transaction removeObjectForKey:kLocalProfileUniqueId inCollection:[UserProfile collection]]; - }]; - - // rebuild localUserProfile - OWSAssert(self.localUserProfile); - } - }); + OWSUserProfile *userProfile = self.localUserProfile; + [userProfile clearWithProfileKey:[OWSAES256Key generateRandomKey] dbConnection:self.dbConnection completion:nil]; } - (void)addUserToProfileWhitelist:(NSString *)recipientId @@ -787,87 +604,70 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640; { OWSAssert(recipientIds); - dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{ - NSMutableArray *newRecipientIds = [NSMutableArray new]; - - @synchronized(self) - { - for (NSString *recipientId in recipientIds) { - if (![self isUserInProfileWhitelist:recipientId]) { - [newRecipientIds addObject:recipientId]; - } - } - - if (newRecipientIds.count < 1) { - return; - } - - for (NSString *recipientId in recipientIds) { - self.userProfileWhitelistCache[recipientId] = @(YES); + NSMutableSet *newRecipientIds = [NSMutableSet new]; + [self.dbConnection asyncReadWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { + for (NSString *recipientId in recipientIds) { + NSNumber *_Nullable oldValue = + [transaction objectForKey:recipientId inCollection:kOWSProfileManager_UserWhitelistCollection]; + if (oldValue && oldValue.boolValue) { + continue; } + [transaction setObject:@(YES) forKey:recipientId inCollection:kOWSProfileManager_UserWhitelistCollection]; + [newRecipientIds addObject:recipientId]; } - - [self.dbConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { - for (NSString *recipientId in recipientIds) { - [transaction setObject:@(YES) - forKey:recipientId - inCollection:kOWSProfileManager_UserWhitelistCollection]; + } + completionBlock:^{ + for (NSString *recipientId in newRecipientIds) { + [[NSNotificationCenter defaultCenter] + postNotificationNameAsync:kNSNotificationName_ProfileWhitelistDidChange + object:nil + userInfo:@{ + kNSNotificationKey_ProfileRecipientId : recipientId, + }]; } }]; - - for (NSString *recipientId in newRecipientIds) { - [[NSNotificationCenter defaultCenter] - postNotificationNameAsync:kNSNotificationName_ProfileWhitelistDidChange - object:nil - userInfo:@{ - kNSNotificationKey_ProfileRecipientId : recipientId, - }]; - } - }); } - (BOOL)isUserInProfileWhitelist:(NSString *)recipientId { OWSAssert(recipientId.length > 0); - @synchronized(self) - { - NSNumber *_Nullable value = self.userProfileWhitelistCache[recipientId]; - if (value) { - return [value boolValue]; - } - - value = - @([self.dbConnection hasObjectForKey:recipientId inCollection:kOWSProfileManager_UserWhitelistCollection]); - self.userProfileWhitelistCache[recipientId] = value; - return [value boolValue]; - } + __block BOOL result = NO; + [self.dbConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { + NSNumber *_Nullable oldValue = + [transaction objectForKey:recipientId inCollection:kOWSProfileManager_UserWhitelistCollection]; + result = (oldValue && oldValue.boolValue); + }]; + return result; } - (void)addGroupIdToProfileWhitelist:(NSData *)groupId { OWSAssert(groupId.length > 0); - dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{ - NSString *groupIdKey = [groupId hexadecimalString]; + NSString *groupIdKey = [groupId hexadecimalString]; - @synchronized(self) - { - if ([self isGroupIdInProfileWhitelist:groupId]) { - return; - } - - self.groupProfileWhitelistCache[groupIdKey] = @(YES); + __block BOOL didChange = NO; + [self.dbConnection asyncReadWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { + NSNumber *_Nullable oldValue = + [transaction objectForKey:groupIdKey inCollection:kOWSProfileManager_GroupWhitelistCollection]; + if (oldValue && oldValue.boolValue) { + // Do nothing. + } else { + [transaction setObject:@(YES) forKey:groupIdKey inCollection:kOWSProfileManager_GroupWhitelistCollection]; + didChange = YES; } - - [self.dbConnection setBool:YES forKey:groupIdKey inCollection:kOWSProfileManager_GroupWhitelistCollection]; - - [[NSNotificationCenter defaultCenter] postNotificationNameAsync:kNSNotificationName_ProfileWhitelistDidChange - object:nil - userInfo:@{ - kNSNotificationKey_ProfileGroupId : groupId, - }]; - }); + } + completionBlock:^{ + if (didChange) { + [[NSNotificationCenter defaultCenter] + postNotificationNameAsync:kNSNotificationName_ProfileWhitelistDidChange + object:nil + userInfo:@{ + kNSNotificationKey_ProfileGroupId : groupId, + }]; + } + }]; } - (void)addThreadToProfileWhitelist:(TSThread *)thread @@ -896,19 +696,15 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640; { OWSAssert(groupId.length > 0); - @synchronized(self) - { - NSString *groupIdKey = [groupId hexadecimalString]; - NSNumber *_Nullable value = self.groupProfileWhitelistCache[groupIdKey]; - if (value) { - return [value boolValue]; - } + NSString *groupIdKey = [groupId hexadecimalString]; - value = @(nil != - [self.dbConnection objectForKey:groupIdKey inCollection:kOWSProfileManager_GroupWhitelistCollection]); - self.groupProfileWhitelistCache[groupIdKey] = value; - return [value boolValue]; - } + __block BOOL result = NO; + [self.dbConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { + NSNumber *_Nullable oldValue = + [transaction objectForKey:groupIdKey inCollection:kOWSProfileManager_GroupWhitelistCollection]; + result = (oldValue && oldValue.boolValue); + }]; + return result; } - (BOOL)isThreadInProfileWhitelist:(TSThread *)thread @@ -936,59 +732,49 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640; - (void)logUserProfiles { - dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{ - @synchronized(self) - { - DDLogError(@"logUserProfiles: %zd", [self.dbConnection numberOfKeysInCollection:UserProfile.collection]); - [self.dbConnection readWithBlock:^(YapDatabaseReadTransaction *transaction) { - [transaction - enumerateKeysAndObjectsInCollection:UserProfile.collection - usingBlock:^( - NSString *_Nonnull key, id _Nonnull object, BOOL *_Nonnull stop) { - OWSAssert([object isKindOfClass:[UserProfile class]]); - UserProfile *userProfile = object; - DDLogError(@"\t [%@]: has profile key: %d, has avatar URL: %d, has " - @"avatar file: %d, name: %@", - userProfile.recipientId, - userProfile.profileKey != nil, - userProfile.avatarUrlPath != nil, - userProfile.avatarFileName != nil, - userProfile.profileName); - }]; - }]; - } - }); + [self.dbConnection asyncReadWithBlock:^(YapDatabaseReadTransaction *transaction) { + DDLogError(@"logUserProfiles: %zd", [transaction numberOfKeysInCollection:OWSUserProfile.collection]); + [transaction + enumerateKeysAndObjectsInCollection:OWSUserProfile.collection + usingBlock:^(NSString *_Nonnull key, id _Nonnull object, BOOL *_Nonnull stop) { + OWSAssert([object isKindOfClass:[OWSUserProfile class]]); + OWSUserProfile *userProfile = object; + DDLogError(@"\t [%@]: has profile key: %d, has avatar URL: %d, has " + @"avatar file: %d, name: %@", + userProfile.recipientId, + userProfile.profileKey != nil, + userProfile.avatarUrlPath != nil, + userProfile.avatarFileName != nil, + userProfile.profileName); + }]; + }]; } - (void)setProfileKeyData:(NSData *)profileKeyData forRecipientId:(NSString *)recipientId; { dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{ - @synchronized(self) - { - OWSAES256Key *_Nullable profileKey = [OWSAES256Key keyWithData:profileKeyData]; - if (profileKey == nil) { - OWSFail(@"Failed to make profile key for key data"); - return; - } - - UserProfile *userProfile = [self getOrBuildUserProfileForRecipientId:recipientId]; - OWSAssert(userProfile); - if (userProfile.profileKey && [userProfile.profileKey.keyData isEqual:profileKey.keyData]) { - // Ignore redundant update. - return; - } - - userProfile.profileKey = profileKey; - - // Clear profile state. - userProfile.profileName = nil; - userProfile.avatarUrlPath = nil; - userProfile.avatarFileName = nil; - - [self saveUserProfile:userProfile]; - - [self refreshProfileForRecipientId:recipientId ignoreThrottling:YES]; + OWSAES256Key *_Nullable profileKey = [OWSAES256Key keyWithData:profileKeyData]; + if (profileKey == nil) { + OWSFail(@"Failed to make profile key for key data"); + return; } + + 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]; + }]; }); } @@ -1001,12 +787,10 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640; { OWSAssert(recipientId.length > 0); - @synchronized(self) - { - UserProfile *userProfile = [self getOrBuildUserProfileForRecipientId:recipientId]; - OWSAssert(userProfile); - return userProfile.profileKey; - } + OWSUserProfile *userProfile = + [OWSUserProfile getOrBuildUserProfileForRecipientId:recipientId dbConnection:self.dbConnection]; + OWSAssert(userProfile); + return userProfile.profileKey; } - (nullable NSString *)profileNameForRecipientId:(NSString *)recipientId @@ -1015,12 +799,10 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640; [self refreshProfileForRecipientId:recipientId]; - @synchronized(self) - { - UserProfile *userProfile = [self getOrBuildUserProfileForRecipientId:recipientId]; - return userProfile.profileName; - return self.localUserProfile.profileName; - } + OWSUserProfile *userProfile = + [OWSUserProfile getOrBuildUserProfileForRecipientId:recipientId dbConnection:self.dbConnection]; + return userProfile.profileName; + return self.localUserProfile.profileName; } - (nullable UIImage *)profileAvatarForRecipientId:(NSString *)recipientId @@ -1029,131 +811,123 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640; [self refreshProfileForRecipientId:recipientId]; - @synchronized(self) - { - UIImage *_Nullable image = [self.otherUsersProfileAvatarImageCache objectForKey:recipientId]; - if (image) { - return image; - } - - UserProfile *userProfile = [self getOrBuildUserProfileForRecipientId:recipientId]; - if (userProfile.avatarFileName.length > 0) { - image = [self loadProfileAvatarWithFilename:userProfile.avatarFileName]; - if (image) { - [self.otherUsersProfileAvatarImageCache setObject:image forKey:recipientId]; - } - } else if (userProfile.avatarUrlPath.length > 0) { - [self downloadAvatarForUserProfile:userProfile]; - } - - return image; + OWSUserProfile *userProfile = + [OWSUserProfile getOrBuildUserProfileForRecipientId:recipientId dbConnection:self.dbConnection]; + if (userProfile.avatarFileName.length > 0) { + return [self loadProfileAvatarWithFilename:userProfile.avatarFileName]; + } else if (userProfile.avatarUrlPath.length > 0) { + [self downloadAvatarForUserProfile:userProfile]; } + + return nil; } -- (void)downloadAvatarForUserProfile:(UserProfile *)userProfile +- (void)downloadAvatarForUserProfile:(OWSUserProfile *)userProfileParameter { - OWSAssert(userProfile); + OWSAssert(userProfileParameter); + + // Make a local copy. + OWSUserProfile *userProfile = [userProfileParameter copy]; dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{ + if (userProfile.avatarUrlPath.length < 1) { + OWSFail(@"%@ Malformed avatar URL: %@", self.logTag, userProfile.avatarUrlPath); + return; + } + NSString *_Nullable avatarUrlPathAtStart = userProfile.avatarUrlPath; + + if (userProfile.profileKey.keyData.length < 1 || userProfile.avatarUrlPath.length < 1) { + return; + } + + OWSAES256Key *profileKeyAtStart = userProfile.profileKey; + + NSString *fileName = [[NSUUID UUID].UUIDString stringByAppendingPathExtension:@"jpg"]; + NSString *filePath = [self.profileAvatarsDirPath stringByAppendingPathComponent:fileName]; + @synchronized(self) { - if (userProfile.avatarUrlPath.length < 1) { - OWSFail(@"%@ Malformed avatar URL: %@", self.logTag, userProfile.avatarUrlPath); - return; - } - NSString *_Nullable avatarUrlPathAtStart = userProfile.avatarUrlPath; - - if (userProfile.profileKey.keyData.length < 1 || userProfile.avatarUrlPath.length < 1) { - return; - } - - OWSAES256Key *profileKeyAtStart = userProfile.profileKey; - - NSString *fileName = [[NSUUID UUID].UUIDString stringByAppendingPathExtension:@"jpg"]; - NSString *filePath = [self.profileAvatarsDirPath stringByAppendingPathComponent:fileName]; - if ([self.currentAvatarDownloads containsObject:userProfile.recipientId]) { // Download already in flight; ignore. return; } [self.currentAvatarDownloads addObject:userProfile.recipientId]; - - NSString *tempDirectory = NSTemporaryDirectory(); - NSString *tempFilePath = [tempDirectory stringByAppendingPathComponent:fileName]; - - NSURL *avatarUrlPath = - [NSURL URLWithString:userProfile.avatarUrlPath relativeToURL:self.avatarHTTPManager.baseURL]; - NSURLRequest *request = [NSURLRequest requestWithURL:avatarUrlPath]; - NSURLSessionDownloadTask *downloadTask = [self.avatarHTTPManager downloadTaskWithRequest:request - progress:^(NSProgress *_Nonnull downloadProgress) { - DDLogVerbose(@"%@ Downloading avatar for %@ %f", - self.logTag, - userProfile.recipientId, - downloadProgress.fractionCompleted); - } - destination:^NSURL *_Nonnull(NSURL *_Nonnull targetPath, NSURLResponse *_Nonnull response) { - return [NSURL fileURLWithPath:tempFilePath]; - } - completionHandler:^( - NSURLResponse *_Nonnull response, NSURL *_Nullable filePathParam, NSError *_Nullable error) { - // Ensure disk IO and decryption occurs off the main thread. - dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{ - NSData *_Nullable encryptedData = (error ? nil : [NSData dataWithContentsOfFile:tempFilePath]); - NSData *_Nullable decryptedData = - [self decryptProfileData:encryptedData profileKey:profileKeyAtStart]; - UIImage *_Nullable image = nil; - if (decryptedData) { - BOOL success = [decryptedData writeToFile:filePath atomically:YES]; - if (success) { - image = [UIImage imageWithContentsOfFile:filePath]; - } - } - - @synchronized(self) - { - [self.currentAvatarDownloads removeObject:userProfile.recipientId]; - - UserProfile *latestUserProfile = - [self getOrBuildUserProfileForRecipientId:userProfile.recipientId]; - if (latestUserProfile.profileKey.keyData.length < 1 - || ![latestUserProfile.profileKey isEqual:userProfile.profileKey]) { - DDLogWarn(@"%@ Ignoring avatar download for obsolete user profile.", self.logTag); - } else if (![avatarUrlPathAtStart isEqualToString:latestUserProfile.avatarUrlPath]) { - DDLogInfo(@"%@ avatar url has changed during download", self.logTag); - if (latestUserProfile.avatarUrlPath.length > 0) { - [self downloadAvatarForUserProfile:latestUserProfile]; - } - } else if (error) { - DDLogError(@"%@ avatar download failed: %@", self.logTag, error); - } else if (!encryptedData) { - DDLogError(@"%@ avatar encrypted data could not be read.", self.logTag); - } else if (!decryptedData) { - DDLogError(@"%@ avatar data could not be decrypted.", self.logTag); - } else if (!image) { - DDLogError(@"%@ avatar image could not be loaded: %@", self.logTag, error); - } else { - [self.otherUsersProfileAvatarImageCache setObject:image forKey:userProfile.recipientId]; - - userProfile.avatarFileName = fileName; - - [self saveUserProfile:userProfile]; - } - - // If we're updating the profile that corresponds to our local number, - // update the local profile as well. - NSString *_Nullable localNumber = [TSAccountManager sharedInstance].localNumber; - if (localNumber && [localNumber isEqualToString:userProfile.recipientId]) { - UserProfile *localUserProfile = self.localUserProfile; - OWSAssert(localUserProfile); - localUserProfile.avatarFileName = fileName; - [self saveUserProfile:localUserProfile]; - self.localCachedAvatarImage = image; - } - } - }); - }]; - [downloadTask resume]; } + + NSString *tempDirectory = NSTemporaryDirectory(); + NSString *tempFilePath = [tempDirectory stringByAppendingPathComponent:fileName]; + + void (^completionHandler)(NSURLResponse *_Nonnull, NSURL *_Nullable, NSError *_Nullable) = ^( + NSURLResponse *_Nonnull response, NSURL *_Nullable filePathParam, NSError *_Nullable error) { + // Ensure disk IO and decryption occurs off the main thread. + dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{ + NSData *_Nullable encryptedData = (error ? nil : [NSData dataWithContentsOfFile:tempFilePath]); + NSData *_Nullable decryptedData = [self decryptProfileData:encryptedData profileKey:profileKeyAtStart]; + UIImage *_Nullable image = nil; + if (decryptedData) { + BOOL success = [decryptedData writeToFile:filePath atomically:YES]; + if (success) { + image = [UIImage imageWithContentsOfFile:filePath]; + } + } + + @synchronized(self) + { + [self.currentAvatarDownloads removeObject:userProfile.recipientId]; + } + + OWSUserProfile *latestUserProfile = + [OWSUserProfile getOrBuildUserProfileForRecipientId:userProfile.recipientId + dbConnection:self.dbConnection]; + if (latestUserProfile.profileKey.keyData.length < 1 + || ![latestUserProfile.profileKey isEqual:userProfile.profileKey]) { + DDLogWarn(@"%@ Ignoring avatar download for obsolete user profile.", self.logTag); + } else if (![avatarUrlPathAtStart isEqualToString:latestUserProfile.avatarUrlPath]) { + DDLogInfo(@"%@ avatar url has changed during download", self.logTag); + if (latestUserProfile.avatarUrlPath.length > 0) { + [self downloadAvatarForUserProfile:latestUserProfile]; + } + } else if (error) { + DDLogError(@"%@ avatar download failed: %@", self.logTag, error); + } else if (!encryptedData) { + DDLogError(@"%@ avatar encrypted data could not be read.", self.logTag); + } else if (!decryptedData) { + DDLogError(@"%@ avatar data could not be decrypted.", self.logTag); + } else if (!image) { + DDLogError(@"%@ avatar image could not be loaded: %@", self.logTag, error); + } else { + [self updateProfileAvatarCache:image filename:fileName]; + + [latestUserProfile updateWithAvatarFileName:fileName dbConnection:self.dbConnection completion:nil]; + } + + // If we're updating the profile that corresponds to our local number, + // update the local profile as well. + NSString *_Nullable localNumber = [TSAccountManager sharedInstance].localNumber; + if (localNumber && [localNumber isEqualToString:userProfile.recipientId]) { + OWSUserProfile *localUserProfile = self.localUserProfile; + OWSAssert(localUserProfile); + [localUserProfile updateWithAvatarFileName:fileName dbConnection:self.dbConnection completion:nil]; + [self updateProfileAvatarCache:image filename:fileName]; + } + }); + }; + + NSURL *avatarUrlPath = + [NSURL URLWithString:userProfile.avatarUrlPath relativeToURL:self.avatarHTTPManager.baseURL]; + NSURLRequest *request = [NSURLRequest requestWithURL:avatarUrlPath]; + NSURLSessionDownloadTask *downloadTask = [self.avatarHTTPManager downloadTaskWithRequest:request + progress:^(NSProgress *_Nonnull downloadProgress) { + DDLogVerbose(@"%@ Downloading avatar for %@ %f", + self.logTag, + userProfile.recipientId, + downloadProgress.fractionCompleted); + } + destination:^NSURL *_Nonnull(NSURL *_Nonnull targetPath, NSURLResponse *_Nonnull response) { + return [NSURL fileURLWithPath:tempFilePath]; + } + completionHandler:completionHandler]; + [downloadTask resume]; }); } @@ -1167,35 +941,33 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640; OWSAssert(recipientId.length > 0); dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{ - @synchronized(self) - { - UserProfile *userProfile = [self getOrBuildUserProfileForRecipientId:recipientId]; + 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.lastUpdateDate = [NSDate new]; - - [self saveUserProfile:userProfile]; - - dispatch_async(dispatch_get_main_queue(), ^{ - [ProfileFetcherJob runWithRecipientId:recipientId - networkManager:self.networkManager - ignoreThrottling:ignoreThrottling]; - }); + 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]; + }); + }]; }); } @@ -1213,50 +985,47 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640; // Ensure decryption, etc. off main thread. dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{ - @synchronized(self) - { - UserProfile *userProfile = [self getOrBuildUserProfileForRecipientId:recipientId]; - if (!userProfile.profileKey) { - return; + OWSUserProfile *userProfile = + [OWSUserProfile getOrBuildUserProfileForRecipientId:recipientId dbConnection:self.dbConnection]; + if (!userProfile.profileKey) { + return; + } + + 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] + dbConnection:self.dbConnection + completion:nil]; + + // If we're updating the profile that corresponds to our local number, + // update the local profile as well. + NSString *_Nullable localNumber = [TSAccountManager sharedInstance].localNumber; + if (localNumber && [localNumber isEqualToString:recipientId]) { + OWSUserProfile *localUserProfile = self.localUserProfile; + OWSAssert(localUserProfile); + + // Don't clear avatarFileName optimistically. + // * The profile avatar probably isn't out of sync. + // * If the profile avatar is out of sync, it can be synced on next app launch. + // * We don't want to touch local avatar state until we've + // 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]; } - - NSString *_Nullable profileName = - [self decryptProfileNameData:profileNameEncrypted profileKey:userProfile.profileKey]; - - BOOL isAvatarSame = [self isNullableStringEqual:userProfile.avatarUrlPath toString:avatarUrlPath]; - - userProfile.profileName = profileName; - userProfile.avatarUrlPath = avatarUrlPath; - userProfile.avatarFileName = nil; - - // If we're updating the profile that corresponds to our local number, - // update the local profile as well. - NSString *_Nullable localNumber = [TSAccountManager sharedInstance].localNumber; - if (localNumber && [localNumber isEqualToString:recipientId]) { - UserProfile *localUserProfile = self.localUserProfile; - OWSAssert(localUserProfile); - localUserProfile.profileName = profileName; - localUserProfile.avatarUrlPath = avatarUrlPath; - // Don't clear avatarFileName and localCachedAvatarImage optimistically. - // * The profile avatar probably isn't out of sync. - // * If the profile avatar is out of sync, it can be synced on next app launch. - // * We don't want to touch local avatar state until we've - // downloaded the latest avatar by downloadAvatarForUserProfile. - [self saveUserProfile:localUserProfile]; - } - - if (!isAvatarSame) { - // Evacuate avatar image cache. - [self.otherUsersProfileAvatarImageCache removeObjectForKey:recipientId]; - - if (avatarUrlPath) { - [self downloadAvatarForUserProfile:userProfile]; - } - } - - userProfile.lastUpdateDate = [NSDate new]; - - [self saveUserProfile:userProfile]; } }); } @@ -1379,16 +1148,43 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640; - (nullable UIImage *)loadProfileAvatarWithFilename:(NSString *)filename { - OWSAssert(filename.length > 0); + if (filename.length == 0) { + return nil; + } + + UIImage *_Nullable image = nil; + @synchronized(self) + { + image = [self.profileAvatarImageCache objectForKey:filename]; + } + if (image) { + return image; + } NSData *data = [self loadProfileDataWithFilename:filename]; if (![data ows_isValidImage]) { return nil; } - UIImage *_Nullable image = [UIImage imageWithData:data]; + image = [UIImage imageWithData:data]; + [self updateProfileAvatarCache:image filename:filename]; return image; } +- (void)updateProfileAvatarCache:(nullable UIImage *)image filename:(NSString *)filename +{ + OWSAssert(filename.length > 0); + OWSAssert(image); + + @synchronized(self) + { + if (image) { + [self.profileAvatarImageCache setObject:image forKey:filename]; + } else { + [self.profileAvatarImageCache removeObjectForKey:filename]; + } + } +} + + (NSString *)legacyProfileAvatarsDirPath { return [[OWSFileSystem appDocumentDirectoryPath] stringByAppendingPathComponent:@"ProfileAvatars"]; @@ -1412,9 +1208,9 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640; static dispatch_once_t onceToken; dispatch_once(&onceToken, ^{ profileAvatarsDirPath = OWSProfileManager.sharedDataProfileAvatarsDirPath; - + [OWSFileSystem ensureDirectoryExists:profileAvatarsDirPath]; - + [OWSFileSystem protectFolderAtPath:profileAvatarsDirPath]; }); return profileAvatarsDirPath; @@ -1495,10 +1291,7 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640; { OWSAssert([NSThread isMainThread]); - @synchronized(self) - { - // TODO: Sync if necessary. - } + // TODO: Sync if necessary. } @end diff --git a/SignalMessaging/profiles/OWSUserProfile.h b/SignalMessaging/profiles/OWSUserProfile.h new file mode 100644 index 0000000000..81281aaa6e --- /dev/null +++ b/SignalMessaging/profiles/OWSUserProfile.h @@ -0,0 +1,83 @@ +// +// Copyright (c) 2017 Open Whisper Systems. All rights reserved. +// + +#import + +NS_ASSUME_NONNULL_BEGIN + +typedef void (^OWSUserProfileCompletion)(void); + +@class OWSAES256Key; + +extern NSString *const kLocalProfileUniqueId; + +// This class should be completely thread-safe. +@interface OWSUserProfile : TSYapDatabaseObject + +@property (atomic, readonly) NSString *recipientId; +@property (atomic, readonly, nullable) OWSAES256Key *profileKey; +@property (atomic, readonly, nullable) NSString *profileName; +@property (atomic, readonly, nullable) NSString *avatarUrlPath; +// 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 + dbConnection:(YapDatabaseConnection *)dbConnection; + +#pragma mark - Update With... Methods + +- (void)updateWithProfileName:(nullable NSString *)profileName + profileKey:(OWSAES256Key *)profileKey + 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 + 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; + +- (void)updateWithAvatarUrlPath:(nullable NSString *)avatarUrlPath + avatarFileName:(nullable NSString *)avatarFileName + dbConnection:(YapDatabaseConnection *)dbConnection + completion:(nullable OWSUserProfileCompletion)completion; + +- (void)updateWithAvatarFileName:(nullable NSString *)avatarFileName + 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; + +@end + +NS_ASSUME_NONNULL_END diff --git a/SignalMessaging/profiles/OWSUserProfile.m b/SignalMessaging/profiles/OWSUserProfile.m new file mode 100644 index 0000000000..675ce5ea53 --- /dev/null +++ b/SignalMessaging/profiles/OWSUserProfile.m @@ -0,0 +1,250 @@ +// +// Copyright (c) 2017 Open Whisper Systems. All rights reserved. +// + +#import "OWSUserProfile.h" +#import "NSString+OWS.h" +#import +#import + +NS_ASSUME_NONNULL_BEGIN + +NSString *const kLocalProfileUniqueId = @"kLocalProfileUniqueId"; + +@interface OWSUserProfile () + +@property (atomic, nullable) OWSAES256Key *profileKey; +@property (atomic, nullable) NSString *profileName; +@property (atomic, nullable) NSString *avatarUrlPath; +@property (atomic, nullable) NSString *avatarFileName; +@property (atomic, nullable) NSDate *lastUpdateDate; + +@end + +#pragma mark - + +@implementation OWSUserProfile + ++ (OWSUserProfile *)getOrBuildUserProfileForRecipientId:(NSString *)recipientId + dbConnection:(YapDatabaseConnection *)dbConnection +{ + OWSAssert(recipientId.length > 0); + + __block OWSUserProfile *userProfile; + [dbConnection readWithBlock:^(YapDatabaseReadTransaction *transaction) { + userProfile = [OWSUserProfile fetchObjectWithUniqueID:recipientId transaction:transaction]; + }]; + + if (!userProfile) { + userProfile = [[OWSUserProfile alloc] initWithRecipientId:recipientId]; + + if ([recipientId isEqualToString:kLocalProfileUniqueId]) { + [userProfile updateImmediatelyWithProfileKey:[OWSAES256Key generateRandomKey] + dbConnection:dbConnection + completion:nil]; + } + } + + OWSAssert(userProfile); + + return userProfile; +} + +- (instancetype)initWithRecipientId:(NSString *)recipientId +{ + self = [super initWithUniqueId:recipientId]; + + if (!self) { + return self; + } + + OWSAssert(recipientId.length > 0); + _recipientId = recipientId; + + return self; +} + +#pragma mark - Update With... Methods + +- (void)updateWithProfileName:(nullable NSString *)profileName + avatarUrlPath:(nullable NSString *)avatarUrlPath + avatarFileName:(nullable NSString *)avatarFileName + dbConnection:(YapDatabaseConnection *)dbConnection + completion:(nullable OWSUserProfileCompletion)completion +{ + [dbConnection asyncReadWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { + [self applyChangeToSelfAndLatestCopy:transaction + changeBlock:^(OWSUserProfile *userProfile) { + [userProfile setProfileName:[profileName ows_stripped]]; + [userProfile setAvatarUrlPath:avatarUrlPath]; + [userProfile setAvatarFileName:avatarFileName]; + }]; + } + completionBlock: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 +{ + [dbConnection asyncReadWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { + [self applyChangeToSelfAndLatestCopy:transaction + changeBlock:^(OWSUserProfile *userProfile) { + [userProfile setProfileName:[profileName ows_stripped]]; + [userProfile setAvatarUrlPath:avatarUrlPath]; + [userProfile setAvatarFileName:avatarFileName]; + [userProfile setLastUpdateDate:lastUpdateDate]; + }]; + } + completionBlock:completion]; +} + +- (void)updateWithProfileName:(nullable NSString *)profileName + avatarUrlPath:(nullable NSString *)avatarUrlPath + lastUpdateDate:(nullable NSDate *)lastUpdateDate + dbConnection:(YapDatabaseConnection *)dbConnection + completion:(nullable OWSUserProfileCompletion)completion +{ + [dbConnection asyncReadWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { + [self applyChangeToSelfAndLatestCopy:transaction + changeBlock:^(OWSUserProfile *userProfile) { + [userProfile setProfileName:[profileName ows_stripped]]; + [userProfile setAvatarUrlPath:avatarUrlPath]; + [userProfile setLastUpdateDate:lastUpdateDate]; + }]; + } + completionBlock:completion]; +} + +- (void)updateWithProfileName:(nullable NSString *)profileName + profileKey:(OWSAES256Key *)profileKey + avatarUrlPath:(nullable NSString *)avatarUrlPath + avatarFileName:(nullable NSString *)avatarFileName + dbConnection:(YapDatabaseConnection *)dbConnection + completion:(nullable OWSUserProfileCompletion)completion +{ + [dbConnection asyncReadWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { + [self applyChangeToSelfAndLatestCopy:transaction + changeBlock:^(OWSUserProfile *userProfile) { + [userProfile setProfileName:[profileName ows_stripped]]; + [userProfile setProfileKey:profileKey]; + [userProfile setAvatarUrlPath:avatarUrlPath]; + [userProfile setAvatarFileName:avatarFileName]; + }]; + } + completionBlock:completion]; +} + +- (void)updateWithAvatarUrlPath:(nullable NSString *)avatarUrlPath + avatarFileName:(nullable NSString *)avatarFileName + dbConnection:(YapDatabaseConnection *)dbConnection + completion:(nullable OWSUserProfileCompletion)completion +{ + [dbConnection asyncReadWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { + [self applyChangeToSelfAndLatestCopy:transaction + changeBlock:^(OWSUserProfile *userProfile) { + [userProfile setAvatarUrlPath:avatarUrlPath]; + [userProfile setAvatarFileName:avatarFileName]; + }]; + } + completionBlock:completion]; +} + +- (void)updateWithAvatarFileName:(nullable NSString *)avatarFileName + dbConnection:(YapDatabaseConnection *)dbConnection + completion:(nullable OWSUserProfileCompletion)completion +{ + [dbConnection asyncReadWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { + [self applyChangeToSelfAndLatestCopy:transaction + changeBlock:^(OWSUserProfile *userProfile) { + [userProfile setAvatarFileName:avatarFileName]; + }]; + } + completionBlock:completion]; +} + +- (void)updateWithLastUpdateDate:(nullable NSDate *)lastUpdateDate + dbConnection:(YapDatabaseConnection *)dbConnection + completion:(nullable OWSUserProfileCompletion)completion +{ + [dbConnection asyncReadWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { + [self applyChangeToSelfAndLatestCopy:transaction + changeBlock:^(OWSUserProfile *userProfile) { + [userProfile setLastUpdateDate:lastUpdateDate]; + }]; + } + completionBlock:completion]; +} + +- (void)clearWithProfileKey:(OWSAES256Key *)profileKey + dbConnection:(YapDatabaseConnection *)dbConnection + completion:(nullable OWSUserProfileCompletion)completion; +{ + OWSAssert(profileKey); + + [dbConnection asyncReadWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { + [self applyChangeToSelfAndLatestCopy:transaction + changeBlock:^(OWSUserProfile *userProfile) { + [userProfile setProfileKey:profileKey]; + [userProfile setProfileName:nil]; + [userProfile setAvatarUrlPath:nil]; + [userProfile setAvatarFileName:nil]; + [userProfile setLastUpdateDate:nil]; + }]; + } + completionBlock:completion]; +} + +- (void)updateImmediatelyWithProfileKey:(OWSAES256Key *)profileKey + dbConnection:(YapDatabaseConnection *)dbConnection + completion:(nullable OWSUserProfileCompletion)completion; +{ + OWSAssert(profileKey); + + self.profileKey = profileKey; + + [dbConnection asyncReadWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { + [self applyChangeToSelfAndLatestCopy:transaction + changeBlock:^(OWSUserProfile *userProfile) { + [userProfile setProfileKey:profileKey]; + }]; + } + completionBlock:completion]; +} + +#pragma mark - Database Connection Accessors + +- (YapDatabaseConnection *)dbReadConnection +{ + OWSFail(@"%@ UserProfile should always use OWSProfileManager's database connection.", self.logTag); + + return TSYapDatabaseObject.dbReadConnection; +} + ++ (YapDatabaseConnection *)dbReadConnection +{ + OWSFail(@"%@ UserProfile should always use OWSProfileManager's database connection.", self.logTag); + + return TSYapDatabaseObject.dbReadConnection; +} + +- (YapDatabaseConnection *)dbReadWriteConnection +{ + OWSFail(@"%@ UserProfile should always use OWSProfileManager's database connection.", self.logTag); + + return TSYapDatabaseObject.dbReadWriteConnection; +} + ++ (YapDatabaseConnection *)dbReadWriteConnection +{ + OWSFail(@"%@ UserProfile should always use OWSProfileManager's database connection.", self.logTag); + + return TSYapDatabaseObject.dbReadWriteConnection; +} + +@end + +NS_ASSUME_NONNULL_END From ee300590e307174d79670e9aca68b8af62fff842 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Fri, 8 Dec 2017 13:27:53 -0500 Subject: [PATCH 02/11] Rework thread safety in profile manager. --- SignalMessaging/profiles/OWSProfileManager.m | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/SignalMessaging/profiles/OWSProfileManager.m b/SignalMessaging/profiles/OWSProfileManager.m index f0f0910fea..d59085e119 100644 --- a/SignalMessaging/profiles/OWSProfileManager.m +++ b/SignalMessaging/profiles/OWSProfileManager.m @@ -845,7 +845,7 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640; NSString *fileName = [[NSUUID UUID].UUIDString stringByAppendingPathExtension:@"jpg"]; NSString *filePath = [self.profileAvatarsDirPath stringByAppendingPathComponent:fileName]; - @synchronized(self) + @synchronized(self.currentAvatarDownloads) { if ([self.currentAvatarDownloads containsObject:userProfile.recipientId]) { // Download already in flight; ignore. @@ -871,7 +871,7 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640; } } - @synchronized(self) + @synchronized(self.currentAvatarDownloads) { [self.currentAvatarDownloads removeObject:userProfile.recipientId]; } @@ -1153,7 +1153,7 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640; } UIImage *_Nullable image = nil; - @synchronized(self) + @synchronized(self.profileAvatarImageCache) { image = [self.profileAvatarImageCache objectForKey:filename]; } @@ -1175,7 +1175,7 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640; OWSAssert(filename.length > 0); OWSAssert(image); - @synchronized(self) + @synchronized(self.profileAvatarImageCache) { if (image) { [self.profileAvatarImageCache setObject:image forKey:filename]; From 74efcb90414db3e2b5acdc843e2b3bfce71a5b67 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Fri, 8 Dec 2017 13:45:24 -0500 Subject: [PATCH 03/11] Rework thread safety in profile manager. --- Signal.xcodeproj/project.pbxproj | 2 +- .../ConversationViewController.m | 1 + .../src/ViewControllers/InboxTableViewCell.m | 1 + .../OWSConversationSettingsViewController.m | 1 + SignalMessaging/SignalMessaging.h | 1 + SignalMessaging/Views/ContactTableViewCell.m | 1 + SignalMessaging/contacts/OWSContactsManager.m | 1 + SignalMessaging/profiles/OWSProfileManager.h | 5 -- SignalMessaging/profiles/OWSProfileManager.m | 43 ------------ SignalMessaging/profiles/OWSUserProfile.h | 7 ++ SignalMessaging/profiles/OWSUserProfile.m | 68 ++++++++++++++++--- 11 files changed, 73 insertions(+), 58 deletions(-) diff --git a/Signal.xcodeproj/project.pbxproj b/Signal.xcodeproj/project.pbxproj index 85ec492cb0..1db27bc0fb 100644 --- a/Signal.xcodeproj/project.pbxproj +++ b/Signal.xcodeproj/project.pbxproj @@ -108,7 +108,7 @@ 3478506B1FD9B78A007B8332 /* NoopCallMessageHandler.swift in Sources */ = {isa = PBXBuildFile; fileRef = 347850671FD9B78A007B8332 /* NoopCallMessageHandler.swift */; }; 3478506C1FD9B78A007B8332 /* NoopNotificationsManager.swift in Sources */ = {isa = PBXBuildFile; fileRef = 347850681FD9B78A007B8332 /* NoopNotificationsManager.swift */; }; 347850711FDAEB17007B8332 /* OWSUserProfile.m in Sources */ = {isa = PBXBuildFile; fileRef = 3478506F1FDAEB16007B8332 /* OWSUserProfile.m */; }; - 347850721FDAEB17007B8332 /* OWSUserProfile.h in Headers */ = {isa = PBXBuildFile; fileRef = 347850701FDAEB16007B8332 /* OWSUserProfile.h */; }; + 347850721FDAEB17007B8332 /* OWSUserProfile.h in Headers */ = {isa = PBXBuildFile; fileRef = 347850701FDAEB16007B8332 /* OWSUserProfile.h */; settings = {ATTRIBUTES = (Public, ); }; }; 3497DBEC1ECE257500DB2605 /* OWSCountryMetadata.m in Sources */ = {isa = PBXBuildFile; fileRef = 3497DBEB1ECE257500DB2605 /* OWSCountryMetadata.m */; }; 3497DBEF1ECE2E4700DB2605 /* DomainFrontingCountryViewController.m in Sources */ = {isa = PBXBuildFile; fileRef = 3497DBEE1ECE2E4700DB2605 /* DomainFrontingCountryViewController.m */; }; 34B0796D1FCF46B100E248C2 /* MainAppContext.m in Sources */ = {isa = PBXBuildFile; fileRef = 34B0796B1FCF46B000E248C2 /* MainAppContext.m */; }; diff --git a/Signal/src/ViewControllers/ConversationView/ConversationViewController.m b/Signal/src/ViewControllers/ConversationView/ConversationViewController.m index 089addd834..28ae423b70 100644 --- a/Signal/src/ViewControllers/ConversationView/ConversationViewController.m +++ b/Signal/src/ViewControllers/ConversationView/ConversationViewController.m @@ -64,6 +64,7 @@ #import #import #import +#import #import #import #import diff --git a/Signal/src/ViewControllers/InboxTableViewCell.m b/Signal/src/ViewControllers/InboxTableViewCell.m index 50ced4ff09..de7c8fcfd8 100644 --- a/Signal/src/ViewControllers/InboxTableViewCell.m +++ b/Signal/src/ViewControllers/InboxTableViewCell.m @@ -7,6 +7,7 @@ #import "Signal-Swift.h" #import "ViewControllerUtils.h" #import +#import #import #import #import diff --git a/Signal/src/ViewControllers/OWSConversationSettingsViewController.m b/Signal/src/ViewControllers/OWSConversationSettingsViewController.m index afacae8f22..0d850c793d 100644 --- a/Signal/src/ViewControllers/OWSConversationSettingsViewController.m +++ b/Signal/src/ViewControllers/OWSConversationSettingsViewController.m @@ -19,6 +19,7 @@ #import #import #import +#import #import #import #import diff --git a/SignalMessaging/SignalMessaging.h b/SignalMessaging/SignalMessaging.h index b049c7e2e7..f8c0772e4a 100644 --- a/SignalMessaging/SignalMessaging.h +++ b/SignalMessaging/SignalMessaging.h @@ -30,6 +30,7 @@ FOUNDATION_EXPORT const unsigned char SignalMessagingVersionString[]; #import #import #import +#import #import #import #import diff --git a/SignalMessaging/Views/ContactTableViewCell.m b/SignalMessaging/Views/ContactTableViewCell.m index 4c95fac78e..d67b1d2c61 100644 --- a/SignalMessaging/Views/ContactTableViewCell.m +++ b/SignalMessaging/Views/ContactTableViewCell.m @@ -6,6 +6,7 @@ #import "Environment.h" #import "OWSContactAvatarBuilder.h" #import "OWSContactsManager.h" +#import "OWSUserProfile.h" #import "UIFont+OWS.h" #import "UIUtil.h" #import "UIView+OWS.h" diff --git a/SignalMessaging/contacts/OWSContactsManager.m b/SignalMessaging/contacts/OWSContactsManager.m index f67c8dbdf1..1a7bf4fb0a 100644 --- a/SignalMessaging/contacts/OWSContactsManager.m +++ b/SignalMessaging/contacts/OWSContactsManager.m @@ -6,6 +6,7 @@ #import "Environment.h" #import "OWSFormat.h" #import "OWSProfileManager.h" +#import "OWSUserProfile.h" #import "ViewControllerUtils.h" #import #import diff --git a/SignalMessaging/profiles/OWSProfileManager.h b/SignalMessaging/profiles/OWSProfileManager.h index 331dddb230..659ac94925 100644 --- a/SignalMessaging/profiles/OWSProfileManager.h +++ b/SignalMessaging/profiles/OWSProfileManager.h @@ -6,12 +6,7 @@ NS_ASSUME_NONNULL_BEGIN -extern NSString *const kNSNotificationName_LocalProfileDidChange; -extern NSString *const kNSNotificationName_OtherUsersProfileWillChange; -extern NSString *const kNSNotificationName_OtherUsersProfileDidChange; extern NSString *const kNSNotificationName_ProfileWhitelistDidChange; -extern NSString *const kNSNotificationKey_ProfileRecipientId; -extern NSString *const kNSNotificationKey_ProfileGroupId; extern const NSUInteger kOWSProfileManager_NameDataLength; extern const NSUInteger kOWSProfileManager_MaxAvatarDiameter; diff --git a/SignalMessaging/profiles/OWSProfileManager.m b/SignalMessaging/profiles/OWSProfileManager.m index d59085e119..bd991ae536 100644 --- a/SignalMessaging/profiles/OWSProfileManager.m +++ b/SignalMessaging/profiles/OWSProfileManager.m @@ -33,12 +33,7 @@ NS_ASSUME_NONNULL_BEGIN -NSString *const kNSNotificationName_LocalProfileDidChange = @"kNSNotificationName_LocalProfileDidChange"; -NSString *const kNSNotificationName_OtherUsersProfileWillChange = @"kNSNotificationName_OtherUsersProfileWillChange"; -NSString *const kNSNotificationName_OtherUsersProfileDidChange = @"kNSNotificationName_OtherUsersProfileDidChange"; NSString *const kNSNotificationName_ProfileWhitelistDidChange = @"kNSNotificationName_ProfileWhitelistDidChange"; -NSString *const kNSNotificationKey_ProfileRecipientId = @"kNSNotificationKey_ProfileRecipientId"; -NSString *const kNSNotificationKey_ProfileGroupId = @"kNSNotificationKey_ProfileGroupId"; NSString *const kOWSProfileManager_UserWhitelistCollection = @"kOWSProfileManager_UserWhitelistCollection"; NSString *const kOWSProfileManager_GroupWhitelistCollection = @"kOWSProfileManager_GroupWhitelistCollection"; @@ -146,44 +141,6 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640; #pragma mark - User Profile Accessor -- (OWSUserProfileCompletion)userProfileSaveCompletion:(OWSUserProfile *)userProfile -{ - OWSAssert(userProfile); - - return ^{ - BOOL isLocalUserProfile = userProfile == self.localUserProfile; - - dispatch_async(dispatch_get_main_queue(), ^{ - if (isLocalUserProfile) { - // We populate an initial (empty) profile on launch of a new install, but until - // we have a registered account, syncing will fail (and there could not be any - // linked device to sync to at this point anyway). - if ([TSAccountManager isRegistered]) { - [CurrentAppContext() doMultiDeviceUpdateWithProfileKey:userProfile.profileKey]; - } - - [[NSNotificationCenter defaultCenter] - postNotificationNameAsync:kNSNotificationName_LocalProfileDidChange - object:nil - userInfo:nil]; - } else { - [[NSNotificationCenter defaultCenter] - postNotificationNameAsync:kNSNotificationName_OtherUsersProfileWillChange - object:nil - userInfo:@{ - kNSNotificationKey_ProfileRecipientId : userProfile.recipientId, - }]; - [[NSNotificationCenter defaultCenter] - postNotificationNameAsync:kNSNotificationName_OtherUsersProfileDidChange - object:nil - userInfo:@{ - kNSNotificationKey_ProfileRecipientId : userProfile.recipientId, - }]; - } - }); - }; -} - - (void)ensureLocalProfileCached { // Since localUserProfile can create a transaction, we want to make sure it's not called for the first diff --git a/SignalMessaging/profiles/OWSUserProfile.h b/SignalMessaging/profiles/OWSUserProfile.h index 81281aaa6e..cae7284c80 100644 --- a/SignalMessaging/profiles/OWSUserProfile.h +++ b/SignalMessaging/profiles/OWSUserProfile.h @@ -10,6 +10,13 @@ typedef void (^OWSUserProfileCompletion)(void); @class OWSAES256Key; +extern NSString *const kNSNotificationName_LocalProfileDidChange; +extern NSString *const kNSNotificationName_OtherUsersProfileWillChange; +extern NSString *const kNSNotificationName_OtherUsersProfileDidChange; + +extern NSString *const kNSNotificationKey_ProfileRecipientId; +extern NSString *const kNSNotificationKey_ProfileGroupId; + extern NSString *const kLocalProfileUniqueId; // This class should be completely thread-safe. diff --git a/SignalMessaging/profiles/OWSUserProfile.m b/SignalMessaging/profiles/OWSUserProfile.m index 675ce5ea53..834de26db6 100644 --- a/SignalMessaging/profiles/OWSUserProfile.m +++ b/SignalMessaging/profiles/OWSUserProfile.m @@ -4,11 +4,21 @@ #import "OWSUserProfile.h" #import "NSString+OWS.h" +#import #import +#import +#import #import NS_ASSUME_NONNULL_BEGIN +NSString *const kNSNotificationName_LocalProfileDidChange = @"kNSNotificationName_LocalProfileDidChange"; +NSString *const kNSNotificationName_OtherUsersProfileWillChange = @"kNSNotificationName_OtherUsersProfileWillChange"; +NSString *const kNSNotificationName_OtherUsersProfileDidChange = @"kNSNotificationName_OtherUsersProfileDidChange"; + +NSString *const kNSNotificationKey_ProfileRecipientId = @"kNSNotificationKey_ProfileRecipientId"; +NSString *const kNSNotificationKey_ProfileGroupId = @"kNSNotificationKey_ProfileGroupId"; + NSString *const kLocalProfileUniqueId = @"kLocalProfileUniqueId"; @interface OWSUserProfile () @@ -66,6 +76,46 @@ NSString *const kLocalProfileUniqueId = @"kLocalProfileUniqueId"; #pragma mark - Update With... Methods +- (OWSUserProfileCompletion)updateWithCompletion:(nullable OWSUserProfileCompletion)externalCompletion +{ + return ^{ + if (externalCompletion) { + externalCompletion(); + } + + BOOL isLocalUserProfile = [self.recipientId isEqualToString:kLocalProfileUniqueId]; + + dispatch_async(dispatch_get_main_queue(), ^{ + if (isLocalUserProfile) { + // We populate an initial (empty) profile on launch of a new install, but until + // we have a registered account, syncing will fail (and there could not be any + // linked device to sync to at this point anyway). + if ([TSAccountManager isRegistered]) { + [CurrentAppContext() doMultiDeviceUpdateWithProfileKey:self.profileKey]; + } + + [[NSNotificationCenter defaultCenter] + postNotificationNameAsync:kNSNotificationName_LocalProfileDidChange + object:nil + userInfo:nil]; + } else { + [[NSNotificationCenter defaultCenter] + postNotificationNameAsync:kNSNotificationName_OtherUsersProfileWillChange + object:nil + userInfo:@{ + kNSNotificationKey_ProfileRecipientId : self.recipientId, + }]; + [[NSNotificationCenter defaultCenter] + postNotificationNameAsync:kNSNotificationName_OtherUsersProfileDidChange + object:nil + userInfo:@{ + kNSNotificationKey_ProfileRecipientId : self.recipientId, + }]; + } + }); + }; +} + - (void)updateWithProfileName:(nullable NSString *)profileName avatarUrlPath:(nullable NSString *)avatarUrlPath avatarFileName:(nullable NSString *)avatarFileName @@ -80,7 +130,7 @@ NSString *const kLocalProfileUniqueId = @"kLocalProfileUniqueId"; [userProfile setAvatarFileName:avatarFileName]; }]; } - completionBlock:completion]; + completionBlock:[self updateWithCompletion:completion]]; } - (void)updateWithProfileName:(nullable NSString *)profileName @@ -99,7 +149,7 @@ NSString *const kLocalProfileUniqueId = @"kLocalProfileUniqueId"; [userProfile setLastUpdateDate:lastUpdateDate]; }]; } - completionBlock:completion]; + completionBlock:[self updateWithCompletion:completion]]; } - (void)updateWithProfileName:(nullable NSString *)profileName @@ -116,7 +166,7 @@ NSString *const kLocalProfileUniqueId = @"kLocalProfileUniqueId"; [userProfile setLastUpdateDate:lastUpdateDate]; }]; } - completionBlock:completion]; + completionBlock:[self updateWithCompletion:completion]]; } - (void)updateWithProfileName:(nullable NSString *)profileName @@ -135,7 +185,7 @@ NSString *const kLocalProfileUniqueId = @"kLocalProfileUniqueId"; [userProfile setAvatarFileName:avatarFileName]; }]; } - completionBlock:completion]; + completionBlock:[self updateWithCompletion:completion]]; } - (void)updateWithAvatarUrlPath:(nullable NSString *)avatarUrlPath @@ -150,7 +200,7 @@ NSString *const kLocalProfileUniqueId = @"kLocalProfileUniqueId"; [userProfile setAvatarFileName:avatarFileName]; }]; } - completionBlock:completion]; + completionBlock:[self updateWithCompletion:completion]]; } - (void)updateWithAvatarFileName:(nullable NSString *)avatarFileName @@ -163,7 +213,7 @@ NSString *const kLocalProfileUniqueId = @"kLocalProfileUniqueId"; [userProfile setAvatarFileName:avatarFileName]; }]; } - completionBlock:completion]; + completionBlock:[self updateWithCompletion:completion]]; } - (void)updateWithLastUpdateDate:(nullable NSDate *)lastUpdateDate @@ -176,7 +226,7 @@ NSString *const kLocalProfileUniqueId = @"kLocalProfileUniqueId"; [userProfile setLastUpdateDate:lastUpdateDate]; }]; } - completionBlock:completion]; + completionBlock:[self updateWithCompletion:completion]]; } - (void)clearWithProfileKey:(OWSAES256Key *)profileKey @@ -195,7 +245,7 @@ NSString *const kLocalProfileUniqueId = @"kLocalProfileUniqueId"; [userProfile setLastUpdateDate:nil]; }]; } - completionBlock:completion]; + completionBlock:[self updateWithCompletion:completion]]; } - (void)updateImmediatelyWithProfileKey:(OWSAES256Key *)profileKey @@ -212,7 +262,7 @@ NSString *const kLocalProfileUniqueId = @"kLocalProfileUniqueId"; [userProfile setProfileKey:profileKey]; }]; } - completionBlock:completion]; + completionBlock:[self updateWithCompletion:completion]]; } #pragma mark - Database Connection Accessors From 3ea901044a8f4d2b3f72f703c2845d3b482e18ee Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Fri, 8 Dec 2017 14:00:04 -0500 Subject: [PATCH 04/11] Rework thread safety in profile manager. --- SignalMessaging/profiles/OWSUserProfile.m | 119 +++++++++++----------- 1 file changed, 58 insertions(+), 61 deletions(-) diff --git a/SignalMessaging/profiles/OWSUserProfile.m b/SignalMessaging/profiles/OWSUserProfile.m index 834de26db6..a5b51e9f1d 100644 --- a/SignalMessaging/profiles/OWSUserProfile.m +++ b/SignalMessaging/profiles/OWSUserProfile.m @@ -76,44 +76,41 @@ NSString *const kLocalProfileUniqueId = @"kLocalProfileUniqueId"; #pragma mark - Update With... Methods -- (OWSUserProfileCompletion)updateWithCompletion:(nullable OWSUserProfileCompletion)externalCompletion +- (void)finalizeWithCompletion:(nullable OWSUserProfileCompletion)externalCompletion { - return ^{ - if (externalCompletion) { - externalCompletion(); - } + if (externalCompletion) { + dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), externalCompletion); + } - BOOL isLocalUserProfile = [self.recipientId isEqualToString:kLocalProfileUniqueId]; + BOOL isLocalUserProfile = [self.recipientId isEqualToString:kLocalProfileUniqueId]; - dispatch_async(dispatch_get_main_queue(), ^{ - if (isLocalUserProfile) { - // We populate an initial (empty) profile on launch of a new install, but until - // we have a registered account, syncing will fail (and there could not be any - // linked device to sync to at this point anyway). - if ([TSAccountManager isRegistered]) { - [CurrentAppContext() doMultiDeviceUpdateWithProfileKey:self.profileKey]; - } - - [[NSNotificationCenter defaultCenter] - postNotificationNameAsync:kNSNotificationName_LocalProfileDidChange - object:nil - userInfo:nil]; - } else { - [[NSNotificationCenter defaultCenter] - postNotificationNameAsync:kNSNotificationName_OtherUsersProfileWillChange - object:nil - userInfo:@{ - kNSNotificationKey_ProfileRecipientId : self.recipientId, - }]; - [[NSNotificationCenter defaultCenter] - postNotificationNameAsync:kNSNotificationName_OtherUsersProfileDidChange - object:nil - userInfo:@{ - kNSNotificationKey_ProfileRecipientId : self.recipientId, - }]; + dispatch_async(dispatch_get_main_queue(), ^{ + if (isLocalUserProfile) { + // We populate an initial (empty) profile on launch of a new install, but until + // we have a registered account, syncing will fail (and there could not be any + // linked device to sync to at this point anyway). + if ([TSAccountManager isRegistered]) { + [CurrentAppContext() doMultiDeviceUpdateWithProfileKey:self.profileKey]; } - }); - }; + + [[NSNotificationCenter defaultCenter] postNotificationNameAsync:kNSNotificationName_LocalProfileDidChange + object:nil + userInfo:nil]; + } else { + [[NSNotificationCenter defaultCenter] + postNotificationNameAsync:kNSNotificationName_OtherUsersProfileWillChange + object:nil + userInfo:@{ + kNSNotificationKey_ProfileRecipientId : self.recipientId, + }]; + [[NSNotificationCenter defaultCenter] + postNotificationNameAsync:kNSNotificationName_OtherUsersProfileDidChange + object:nil + userInfo:@{ + kNSNotificationKey_ProfileRecipientId : self.recipientId, + }]; + } + }); } - (void)updateWithProfileName:(nullable NSString *)profileName @@ -122,15 +119,15 @@ NSString *const kLocalProfileUniqueId = @"kLocalProfileUniqueId"; dbConnection:(YapDatabaseConnection *)dbConnection completion:(nullable OWSUserProfileCompletion)completion { - [dbConnection asyncReadWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { + [dbConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { [self applyChangeToSelfAndLatestCopy:transaction changeBlock:^(OWSUserProfile *userProfile) { [userProfile setProfileName:[profileName ows_stripped]]; [userProfile setAvatarUrlPath:avatarUrlPath]; [userProfile setAvatarFileName:avatarFileName]; }]; - } - completionBlock:[self updateWithCompletion:completion]]; + }]; + [self finalizeWithCompletion:completion]; } - (void)updateWithProfileName:(nullable NSString *)profileName @@ -140,7 +137,7 @@ NSString *const kLocalProfileUniqueId = @"kLocalProfileUniqueId"; dbConnection:(YapDatabaseConnection *)dbConnection completion:(nullable OWSUserProfileCompletion)completion { - [dbConnection asyncReadWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { + [dbConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { [self applyChangeToSelfAndLatestCopy:transaction changeBlock:^(OWSUserProfile *userProfile) { [userProfile setProfileName:[profileName ows_stripped]]; @@ -148,8 +145,8 @@ NSString *const kLocalProfileUniqueId = @"kLocalProfileUniqueId"; [userProfile setAvatarFileName:avatarFileName]; [userProfile setLastUpdateDate:lastUpdateDate]; }]; - } - completionBlock:[self updateWithCompletion:completion]]; + }]; + [self finalizeWithCompletion:completion]; } - (void)updateWithProfileName:(nullable NSString *)profileName @@ -158,15 +155,15 @@ NSString *const kLocalProfileUniqueId = @"kLocalProfileUniqueId"; dbConnection:(YapDatabaseConnection *)dbConnection completion:(nullable OWSUserProfileCompletion)completion { - [dbConnection asyncReadWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { + [dbConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { [self applyChangeToSelfAndLatestCopy:transaction changeBlock:^(OWSUserProfile *userProfile) { [userProfile setProfileName:[profileName ows_stripped]]; [userProfile setAvatarUrlPath:avatarUrlPath]; [userProfile setLastUpdateDate:lastUpdateDate]; }]; - } - completionBlock:[self updateWithCompletion:completion]]; + }]; + [self finalizeWithCompletion:completion]; } - (void)updateWithProfileName:(nullable NSString *)profileName @@ -176,7 +173,7 @@ NSString *const kLocalProfileUniqueId = @"kLocalProfileUniqueId"; dbConnection:(YapDatabaseConnection *)dbConnection completion:(nullable OWSUserProfileCompletion)completion { - [dbConnection asyncReadWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { + [dbConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { [self applyChangeToSelfAndLatestCopy:transaction changeBlock:^(OWSUserProfile *userProfile) { [userProfile setProfileName:[profileName ows_stripped]]; @@ -184,8 +181,8 @@ NSString *const kLocalProfileUniqueId = @"kLocalProfileUniqueId"; [userProfile setAvatarUrlPath:avatarUrlPath]; [userProfile setAvatarFileName:avatarFileName]; }]; - } - completionBlock:[self updateWithCompletion:completion]]; + }]; + [self finalizeWithCompletion:completion]; } - (void)updateWithAvatarUrlPath:(nullable NSString *)avatarUrlPath @@ -193,40 +190,40 @@ NSString *const kLocalProfileUniqueId = @"kLocalProfileUniqueId"; dbConnection:(YapDatabaseConnection *)dbConnection completion:(nullable OWSUserProfileCompletion)completion { - [dbConnection asyncReadWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { + [dbConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { [self applyChangeToSelfAndLatestCopy:transaction changeBlock:^(OWSUserProfile *userProfile) { [userProfile setAvatarUrlPath:avatarUrlPath]; [userProfile setAvatarFileName:avatarFileName]; }]; - } - completionBlock:[self updateWithCompletion:completion]]; + }]; + [self finalizeWithCompletion:completion]; } - (void)updateWithAvatarFileName:(nullable NSString *)avatarFileName dbConnection:(YapDatabaseConnection *)dbConnection completion:(nullable OWSUserProfileCompletion)completion { - [dbConnection asyncReadWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { + [dbConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { [self applyChangeToSelfAndLatestCopy:transaction changeBlock:^(OWSUserProfile *userProfile) { [userProfile setAvatarFileName:avatarFileName]; }]; - } - completionBlock:[self updateWithCompletion:completion]]; + }]; + [self finalizeWithCompletion:completion]; } - (void)updateWithLastUpdateDate:(nullable NSDate *)lastUpdateDate dbConnection:(YapDatabaseConnection *)dbConnection completion:(nullable OWSUserProfileCompletion)completion { - [dbConnection asyncReadWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { + [dbConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { [self applyChangeToSelfAndLatestCopy:transaction changeBlock:^(OWSUserProfile *userProfile) { [userProfile setLastUpdateDate:lastUpdateDate]; }]; - } - completionBlock:[self updateWithCompletion:completion]]; + }]; + [self finalizeWithCompletion:completion]; } - (void)clearWithProfileKey:(OWSAES256Key *)profileKey @@ -235,7 +232,7 @@ NSString *const kLocalProfileUniqueId = @"kLocalProfileUniqueId"; { OWSAssert(profileKey); - [dbConnection asyncReadWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { + [dbConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { [self applyChangeToSelfAndLatestCopy:transaction changeBlock:^(OWSUserProfile *userProfile) { [userProfile setProfileKey:profileKey]; @@ -244,8 +241,8 @@ NSString *const kLocalProfileUniqueId = @"kLocalProfileUniqueId"; [userProfile setAvatarFileName:nil]; [userProfile setLastUpdateDate:nil]; }]; - } - completionBlock:[self updateWithCompletion:completion]]; + }]; + [self finalizeWithCompletion:completion]; } - (void)updateImmediatelyWithProfileKey:(OWSAES256Key *)profileKey @@ -256,13 +253,13 @@ NSString *const kLocalProfileUniqueId = @"kLocalProfileUniqueId"; self.profileKey = profileKey; - [dbConnection asyncReadWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { + [dbConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { [self applyChangeToSelfAndLatestCopy:transaction changeBlock:^(OWSUserProfile *userProfile) { [userProfile setProfileKey:profileKey]; }]; - } - completionBlock:[self updateWithCompletion:completion]]; + }]; + [self finalizeWithCompletion:completion]; } #pragma mark - Database Connection Accessors From 97ce1a667545bed883ccc4bb59159b008d79c747 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Fri, 8 Dec 2017 14:43:11 -0500 Subject: [PATCH 05/11] Rework user profile saves; block SAE if no local user profile key. --- SignalMessaging/profiles/OWSProfileManager.h | 3 ++ SignalMessaging/profiles/OWSProfileManager.m | 5 +++ SignalMessaging/profiles/OWSUserProfile.h | 2 + SignalMessaging/profiles/OWSUserProfile.m | 38 ++++++++++++++----- .../src/Storage/TSYapDatabaseObject.h | 4 ++ .../src/Storage/TSYapDatabaseObject.m | 11 +++++- .../ShareViewController.swift | 10 +++-- 7 files changed, 59 insertions(+), 14 deletions(-) diff --git a/SignalMessaging/profiles/OWSProfileManager.h b/SignalMessaging/profiles/OWSProfileManager.h index 659ac94925..149439bba0 100644 --- a/SignalMessaging/profiles/OWSProfileManager.h +++ b/SignalMessaging/profiles/OWSProfileManager.h @@ -30,6 +30,9 @@ extern const NSUInteger kOWSProfileManager_MaxAvatarDiameter; // These two methods should only be called from the main thread. - (OWSAES256Key *)localProfileKey; +// localUserProfileExists is true if there is _ANY_ local profile. +- (BOOL)localProfileExists; +// localUserProfileExists is true if there is a local profile with a name or avatar. - (BOOL)hasLocalProfile; - (nullable NSString *)localProfileName; - (nullable UIImage *)localProfileAvatarImage; diff --git a/SignalMessaging/profiles/OWSProfileManager.m b/SignalMessaging/profiles/OWSProfileManager.m index bd991ae536..4f4e54e121 100644 --- a/SignalMessaging/profiles/OWSProfileManager.m +++ b/SignalMessaging/profiles/OWSProfileManager.m @@ -165,6 +165,11 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640; return _localUserProfile; } +- (BOOL)localProfileExists +{ + return [OWSUserProfile localUserProfileExists:self.dbConnection]; +} + - (OWSAES256Key *)localProfileKey { OWSAssert(self.localUserProfile.profileKey.keyData.length == kAES256_KeyByteLength); diff --git a/SignalMessaging/profiles/OWSUserProfile.h b/SignalMessaging/profiles/OWSUserProfile.h index cae7284c80..804482d749 100644 --- a/SignalMessaging/profiles/OWSUserProfile.h +++ b/SignalMessaging/profiles/OWSUserProfile.h @@ -40,6 +40,8 @@ extern NSString *const kLocalProfileUniqueId; + (OWSUserProfile *)getOrBuildUserProfileForRecipientId:(NSString *)recipientId dbConnection:(YapDatabaseConnection *)dbConnection; ++ (BOOL)localUserProfileExists:(YapDatabaseConnection *)dbConnection; + #pragma mark - Update With... Methods - (void)updateWithProfileName:(nullable NSString *)profileName diff --git a/SignalMessaging/profiles/OWSUserProfile.m b/SignalMessaging/profiles/OWSUserProfile.m index a5b51e9f1d..6038dddb1f 100644 --- a/SignalMessaging/profiles/OWSUserProfile.m +++ b/SignalMessaging/profiles/OWSUserProfile.m @@ -6,6 +6,7 @@ #import "NSString+OWS.h" #import #import +#import #import #import #import @@ -60,6 +61,16 @@ NSString *const kLocalProfileUniqueId = @"kLocalProfileUniqueId"; return userProfile; } ++ (BOOL)localUserProfileExists:(YapDatabaseConnection *)dbConnection +{ + __block BOOL result = NO; + [dbConnection readWithBlock:^(YapDatabaseReadTransaction *transaction) { + result = [OWSUserProfile fetchObjectWithUniqueID:kLocalProfileUniqueId transaction:transaction] != nil; + }]; + + return result; +} + - (instancetype)initWithRecipientId:(NSString *)recipientId { self = [super initWithUniqueId:recipientId]; @@ -125,7 +136,8 @@ NSString *const kLocalProfileUniqueId = @"kLocalProfileUniqueId"; [userProfile setProfileName:[profileName ows_stripped]]; [userProfile setAvatarUrlPath:avatarUrlPath]; [userProfile setAvatarFileName:avatarFileName]; - }]; + } + saveIfMissing:YES]; }]; [self finalizeWithCompletion:completion]; } @@ -144,7 +156,8 @@ NSString *const kLocalProfileUniqueId = @"kLocalProfileUniqueId"; [userProfile setAvatarUrlPath:avatarUrlPath]; [userProfile setAvatarFileName:avatarFileName]; [userProfile setLastUpdateDate:lastUpdateDate]; - }]; + } + saveIfMissing:YES]; }]; [self finalizeWithCompletion:completion]; } @@ -161,7 +174,8 @@ NSString *const kLocalProfileUniqueId = @"kLocalProfileUniqueId"; [userProfile setProfileName:[profileName ows_stripped]]; [userProfile setAvatarUrlPath:avatarUrlPath]; [userProfile setLastUpdateDate:lastUpdateDate]; - }]; + } + saveIfMissing:YES]; }]; [self finalizeWithCompletion:completion]; } @@ -180,7 +194,8 @@ NSString *const kLocalProfileUniqueId = @"kLocalProfileUniqueId"; [userProfile setProfileKey:profileKey]; [userProfile setAvatarUrlPath:avatarUrlPath]; [userProfile setAvatarFileName:avatarFileName]; - }]; + } + saveIfMissing:YES]; }]; [self finalizeWithCompletion:completion]; } @@ -195,7 +210,8 @@ NSString *const kLocalProfileUniqueId = @"kLocalProfileUniqueId"; changeBlock:^(OWSUserProfile *userProfile) { [userProfile setAvatarUrlPath:avatarUrlPath]; [userProfile setAvatarFileName:avatarFileName]; - }]; + } + saveIfMissing:YES]; }]; [self finalizeWithCompletion:completion]; } @@ -208,7 +224,8 @@ NSString *const kLocalProfileUniqueId = @"kLocalProfileUniqueId"; [self applyChangeToSelfAndLatestCopy:transaction changeBlock:^(OWSUserProfile *userProfile) { [userProfile setAvatarFileName:avatarFileName]; - }]; + } + saveIfMissing:YES]; }]; [self finalizeWithCompletion:completion]; } @@ -221,7 +238,8 @@ NSString *const kLocalProfileUniqueId = @"kLocalProfileUniqueId"; [self applyChangeToSelfAndLatestCopy:transaction changeBlock:^(OWSUserProfile *userProfile) { [userProfile setLastUpdateDate:lastUpdateDate]; - }]; + } + saveIfMissing:YES]; }]; [self finalizeWithCompletion:completion]; } @@ -240,7 +258,8 @@ NSString *const kLocalProfileUniqueId = @"kLocalProfileUniqueId"; [userProfile setAvatarUrlPath:nil]; [userProfile setAvatarFileName:nil]; [userProfile setLastUpdateDate:nil]; - }]; + } + saveIfMissing:YES]; }]; [self finalizeWithCompletion:completion]; } @@ -257,7 +276,8 @@ NSString *const kLocalProfileUniqueId = @"kLocalProfileUniqueId"; [self applyChangeToSelfAndLatestCopy:transaction changeBlock:^(OWSUserProfile *userProfile) { [userProfile setProfileKey:profileKey]; - }]; + } + saveIfMissing:YES]; }]; [self finalizeWithCompletion:completion]; } diff --git a/SignalServiceKit/src/Storage/TSYapDatabaseObject.h b/SignalServiceKit/src/Storage/TSYapDatabaseObject.h index 03f8f115c6..728b7b38df 100644 --- a/SignalServiceKit/src/Storage/TSYapDatabaseObject.h +++ b/SignalServiceKit/src/Storage/TSYapDatabaseObject.h @@ -151,6 +151,10 @@ NS_ASSUME_NONNULL_BEGIN - (void)applyChangeToSelfAndLatestCopy:(YapDatabaseReadWriteTransaction *)transaction changeBlock:(void (^)(id))changeBlock; +- (void)applyChangeToSelfAndLatestCopy:(YapDatabaseReadWriteTransaction *)transaction + changeBlock:(void (^)(id))changeBlock + saveIfMissing:(BOOL)saveIfMissing; + @end NS_ASSUME_NONNULL_END diff --git a/SignalServiceKit/src/Storage/TSYapDatabaseObject.m b/SignalServiceKit/src/Storage/TSYapDatabaseObject.m index 22c637e089..0b6d4e5410 100644 --- a/SignalServiceKit/src/Storage/TSYapDatabaseObject.m +++ b/SignalServiceKit/src/Storage/TSYapDatabaseObject.m @@ -200,10 +200,15 @@ NS_ASSUME_NONNULL_BEGIN #pragma mark Update With... -// This method does the work for the "updateWith..." methods. Please see -// the header for a discussion of those methods. - (void)applyChangeToSelfAndLatestCopy:(YapDatabaseReadWriteTransaction *)transaction changeBlock:(void (^)(id))changeBlock +{ + [self applyChangeToSelfAndLatestCopy:transaction changeBlock:changeBlock saveIfMissing:NO]; +} + +- (void)applyChangeToSelfAndLatestCopy:(YapDatabaseReadWriteTransaction *)transaction + changeBlock:(void (^)(id))changeBlock + saveIfMissing:(BOOL)saveIfMissing { OWSAssert(transaction); @@ -214,6 +219,8 @@ NS_ASSUME_NONNULL_BEGIN if (latestInstance) { changeBlock(latestInstance); [latestInstance saveWithTransaction:transaction]; + } else if (saveIfMissing) { + [self saveWithTransaction:transaction]; } } diff --git a/SignalShareExtension/ShareViewController.swift b/SignalShareExtension/ShareViewController.swift index 020579fe49..453cbc407c 100644 --- a/SignalShareExtension/ShareViewController.swift +++ b/SignalShareExtension/ShareViewController.swift @@ -232,10 +232,14 @@ public class ShareViewController: UINavigationController, ShareViewDelegate, SAE Logger.info("Presenting initial root view controller") - if TSAccountManager.isRegistered() { - presentConversationPicker() - } else { + if !TSAccountManager.isRegistered() { showNotRegisteredView() + } else if !OWSProfileManager.shared().localProfileExists() { + // This is a rare edge case, but we want to ensure that the user + // is has already saved their local profile key in the main app. + showNotReadyView() + } else { + presentConversationPicker() } // We don't use the AppUpdateNag in the SAE. From 7b4aa4056c9ded529cbb319fbb7c108e7ef60d41 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Fri, 8 Dec 2017 14:59:36 -0500 Subject: [PATCH 06/11] Don't emit "user profile changed" notifications if nothing changed. --- SignalMessaging/profiles/OWSUserProfile.m | 156 +++++++++++++++++----- 1 file changed, 125 insertions(+), 31 deletions(-) diff --git a/SignalMessaging/profiles/OWSUserProfile.m b/SignalMessaging/profiles/OWSUserProfile.m index 6038dddb1f..1b9e4edf14 100644 --- a/SignalMessaging/profiles/OWSUserProfile.m +++ b/SignalMessaging/profiles/OWSUserProfile.m @@ -50,9 +50,9 @@ NSString *const kLocalProfileUniqueId = @"kLocalProfileUniqueId"; userProfile = [[OWSUserProfile alloc] initWithRecipientId:recipientId]; if ([recipientId isEqualToString:kLocalProfileUniqueId]) { - [userProfile updateImmediatelyWithProfileKey:[OWSAES256Key generateRandomKey] - dbConnection:dbConnection - completion:nil]; + [userProfile updateWithProfileKey:[OWSAES256Key generateRandomKey] + dbConnection:dbConnection + completion:nil]; } } @@ -87,12 +87,16 @@ NSString *const kLocalProfileUniqueId = @"kLocalProfileUniqueId"; #pragma mark - Update With... Methods -- (void)finalizeWithCompletion:(nullable OWSUserProfileCompletion)externalCompletion +- (void)finalizeWithCompletion:(nullable OWSUserProfileCompletion)externalCompletion didChange:(BOOL)didChange { if (externalCompletion) { dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), externalCompletion); } + if (!didChange) { + return; + } + BOOL isLocalUserProfile = [self.recipientId isEqualToString:kLocalProfileUniqueId]; dispatch_async(dispatch_get_main_queue(), ^{ @@ -130,16 +134,24 @@ NSString *const kLocalProfileUniqueId = @"kLocalProfileUniqueId"; dbConnection:(YapDatabaseConnection *)dbConnection completion:(nullable OWSUserProfileCompletion)completion { + __block BOOL didChange = NO; [dbConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { [self applyChangeToSelfAndLatestCopy:transaction changeBlock:^(OWSUserProfile *userProfile) { + didChange |= [self didStringChange:userProfile.profileName + newValue:[profileName ows_stripped]]; + didChange |= + [self didStringChange:userProfile.avatarUrlPath newValue:avatarUrlPath]; + didChange |= + [self didStringChange:userProfile.avatarFileName newValue:avatarFileName]; + [userProfile setProfileName:[profileName ows_stripped]]; [userProfile setAvatarUrlPath:avatarUrlPath]; [userProfile setAvatarFileName:avatarFileName]; } saveIfMissing:YES]; }]; - [self finalizeWithCompletion:completion]; + [self finalizeWithCompletion:completion didChange:didChange]; } - (void)updateWithProfileName:(nullable NSString *)profileName @@ -149,17 +161,26 @@ NSString *const kLocalProfileUniqueId = @"kLocalProfileUniqueId"; dbConnection:(YapDatabaseConnection *)dbConnection completion:(nullable OWSUserProfileCompletion)completion { + __block BOOL didChange = NO; [dbConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { - [self applyChangeToSelfAndLatestCopy:transaction - changeBlock:^(OWSUserProfile *userProfile) { - [userProfile setProfileName:[profileName ows_stripped]]; - [userProfile setAvatarUrlPath:avatarUrlPath]; - [userProfile setAvatarFileName:avatarFileName]; - [userProfile setLastUpdateDate:lastUpdateDate]; - } - saveIfMissing:YES]; + [self + applyChangeToSelfAndLatestCopy:transaction + changeBlock:^(OWSUserProfile *userProfile) { + didChange |= [self didStringChange:userProfile.profileName + newValue:[profileName ows_stripped]]; + didChange |= [self didStringChange:userProfile.avatarUrlPath newValue:avatarUrlPath]; + didChange |= + [self didStringChange:userProfile.avatarFileName newValue:avatarFileName]; + didChange |= [self didDateChange:userProfile.lastUpdateDate newValue:lastUpdateDate]; + + [userProfile setProfileName:[profileName ows_stripped]]; + [userProfile setAvatarUrlPath:avatarUrlPath]; + [userProfile setAvatarFileName:avatarFileName]; + [userProfile setLastUpdateDate:lastUpdateDate]; + } + saveIfMissing:YES]; }]; - [self finalizeWithCompletion:completion]; + [self finalizeWithCompletion:completion didChange:didChange]; } - (void)updateWithProfileName:(nullable NSString *)profileName @@ -168,16 +189,23 @@ NSString *const kLocalProfileUniqueId = @"kLocalProfileUniqueId"; dbConnection:(YapDatabaseConnection *)dbConnection completion:(nullable OWSUserProfileCompletion)completion { + __block BOOL didChange = NO; [dbConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { - [self applyChangeToSelfAndLatestCopy:transaction - changeBlock:^(OWSUserProfile *userProfile) { - [userProfile setProfileName:[profileName ows_stripped]]; - [userProfile setAvatarUrlPath:avatarUrlPath]; - [userProfile setLastUpdateDate:lastUpdateDate]; - } - saveIfMissing:YES]; + [self + applyChangeToSelfAndLatestCopy:transaction + changeBlock:^(OWSUserProfile *userProfile) { + didChange |= [self didStringChange:userProfile.profileName + newValue:[profileName ows_stripped]]; + didChange |= [self didStringChange:userProfile.avatarUrlPath newValue:avatarUrlPath]; + didChange |= [self didDateChange:userProfile.lastUpdateDate newValue:lastUpdateDate]; + + [userProfile setProfileName:[profileName ows_stripped]]; + [userProfile setAvatarUrlPath:avatarUrlPath]; + [userProfile setLastUpdateDate:lastUpdateDate]; + } + saveIfMissing:YES]; }]; - [self finalizeWithCompletion:completion]; + [self finalizeWithCompletion:completion didChange:didChange]; } - (void)updateWithProfileName:(nullable NSString *)profileName @@ -187,9 +215,18 @@ NSString *const kLocalProfileUniqueId = @"kLocalProfileUniqueId"; dbConnection:(YapDatabaseConnection *)dbConnection completion:(nullable OWSUserProfileCompletion)completion { + __block BOOL didChange = NO; [dbConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { [self applyChangeToSelfAndLatestCopy:transaction changeBlock:^(OWSUserProfile *userProfile) { + didChange |= [self didStringChange:userProfile.profileName + newValue:[profileName ows_stripped]]; + didChange |= [self didKeyChange:userProfile.profileKey newValue:profileKey]; + didChange |= + [self didStringChange:userProfile.avatarUrlPath newValue:avatarUrlPath]; + didChange |= + [self didStringChange:userProfile.avatarFileName newValue:avatarFileName]; + [userProfile setProfileName:[profileName ows_stripped]]; [userProfile setProfileKey:profileKey]; [userProfile setAvatarUrlPath:avatarUrlPath]; @@ -197,7 +234,7 @@ NSString *const kLocalProfileUniqueId = @"kLocalProfileUniqueId"; } saveIfMissing:YES]; }]; - [self finalizeWithCompletion:completion]; + [self finalizeWithCompletion:completion didChange:didChange]; } - (void)updateWithAvatarUrlPath:(nullable NSString *)avatarUrlPath @@ -205,43 +242,57 @@ NSString *const kLocalProfileUniqueId = @"kLocalProfileUniqueId"; dbConnection:(YapDatabaseConnection *)dbConnection completion:(nullable OWSUserProfileCompletion)completion { + __block BOOL didChange = NO; [dbConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { [self applyChangeToSelfAndLatestCopy:transaction changeBlock:^(OWSUserProfile *userProfile) { + didChange |= + [self didStringChange:userProfile.avatarUrlPath newValue:avatarUrlPath]; + didChange |= + [self didStringChange:userProfile.avatarFileName newValue:avatarFileName]; + [userProfile setAvatarUrlPath:avatarUrlPath]; [userProfile setAvatarFileName:avatarFileName]; } saveIfMissing:YES]; }]; - [self finalizeWithCompletion:completion]; + [self finalizeWithCompletion:completion didChange:didChange]; } - (void)updateWithAvatarFileName:(nullable NSString *)avatarFileName dbConnection:(YapDatabaseConnection *)dbConnection completion:(nullable OWSUserProfileCompletion)completion { + __block BOOL didChange = NO; [dbConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { [self applyChangeToSelfAndLatestCopy:transaction changeBlock:^(OWSUserProfile *userProfile) { + didChange |= + [self didStringChange:userProfile.avatarFileName newValue:avatarFileName]; + [userProfile setAvatarFileName:avatarFileName]; } saveIfMissing:YES]; }]; - [self finalizeWithCompletion:completion]; + [self finalizeWithCompletion:completion didChange:didChange]; } - (void)updateWithLastUpdateDate:(nullable NSDate *)lastUpdateDate dbConnection:(YapDatabaseConnection *)dbConnection completion:(nullable OWSUserProfileCompletion)completion { + __block BOOL didChange = NO; [dbConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { [self applyChangeToSelfAndLatestCopy:transaction changeBlock:^(OWSUserProfile *userProfile) { + didChange |= + [self didDateChange:userProfile.lastUpdateDate newValue:lastUpdateDate]; + [userProfile setLastUpdateDate:lastUpdateDate]; } saveIfMissing:YES]; }]; - [self finalizeWithCompletion:completion]; + [self finalizeWithCompletion:completion didChange:didChange]; } - (void)clearWithProfileKey:(OWSAES256Key *)profileKey @@ -250,9 +301,16 @@ NSString *const kLocalProfileUniqueId = @"kLocalProfileUniqueId"; { OWSAssert(profileKey); + __block BOOL didChange = NO; [dbConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { [self applyChangeToSelfAndLatestCopy:transaction changeBlock:^(OWSUserProfile *userProfile) { + didChange |= [self didKeyChange:userProfile.profileKey newValue:profileKey]; + didChange |= [self didStringChange:userProfile.profileName newValue:nil]; + didChange |= [self didStringChange:userProfile.avatarUrlPath newValue:nil]; + didChange |= [self didStringChange:userProfile.avatarFileName newValue:nil]; + didChange |= [self didDateChange:userProfile.lastUpdateDate newValue:nil]; + [userProfile setProfileKey:profileKey]; [userProfile setProfileName:nil]; [userProfile setAvatarUrlPath:nil]; @@ -261,25 +319,61 @@ NSString *const kLocalProfileUniqueId = @"kLocalProfileUniqueId"; } saveIfMissing:YES]; }]; - [self finalizeWithCompletion:completion]; + [self finalizeWithCompletion:completion didChange:didChange]; } -- (void)updateImmediatelyWithProfileKey:(OWSAES256Key *)profileKey - dbConnection:(YapDatabaseConnection *)dbConnection - completion:(nullable OWSUserProfileCompletion)completion; +- (void)updateWithProfileKey:(OWSAES256Key *)profileKey + dbConnection:(YapDatabaseConnection *)dbConnection + completion:(nullable OWSUserProfileCompletion)completion; { OWSAssert(profileKey); self.profileKey = profileKey; + __block BOOL didChange = NO; [dbConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { [self applyChangeToSelfAndLatestCopy:transaction changeBlock:^(OWSUserProfile *userProfile) { + didChange |= [self didKeyChange:userProfile.profileKey newValue:profileKey]; + [userProfile setProfileKey:profileKey]; } saveIfMissing:YES]; }]; - [self finalizeWithCompletion:completion]; + [self finalizeWithCompletion:completion didChange:didChange]; +} + +- (BOOL)didStringChange:(NSString *_Nullable)oldValue newValue:(NSString *_Nullable)newValue +{ + if (!oldValue && !newValue) { + return NO; + } else if (!oldValue || !newValue) { + return YES; + } else { + return ![oldValue isEqualToString:newValue]; + } +} + +- (BOOL)didDateChange:(NSDate *_Nullable)oldValue newValue:(NSDate *_Nullable)newValue +{ + if (!oldValue && !newValue) { + return NO; + } else if (!oldValue || !newValue) { + return YES; + } else { + return ![oldValue isEqualToDate:newValue]; + } +} + +- (BOOL)didKeyChange:(OWSAES256Key *_Nullable)oldValue newValue:(OWSAES256Key *_Nullable)newValue +{ + if (!oldValue && !newValue) { + return NO; + } else if (!oldValue || !newValue) { + return YES; + } else { + return ![oldValue.keyData isEqualToData:newValue.keyData]; + } } #pragma mark - Database Connection Accessors From f684482c54244a9a8b70408a3470d6725726a0f5 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Fri, 8 Dec 2017 15:21:08 -0500 Subject: [PATCH 07/11] Don't emit "user profile changed" notifications if nothing changed. --- Signal/src/ViewControllers/AvatarViewHelper.m | 4 +- .../ViewControllers/NewGroupViewController.m | 1 + .../ViewControllers/ProfileViewController.m | 1 + .../UpdateGroupViewController.m | 1 + .../ExperienceUpgrade.swift | 2 +- SignalMessaging/profiles/OWSUserProfile.m | 257 +++++++++++++----- .../src/Storage/TSYapDatabaseObject.h | 2 + .../src/Storage/TSYapDatabaseObject.m | 19 +- 8 files changed, 215 insertions(+), 72 deletions(-) diff --git a/Signal/src/ViewControllers/AvatarViewHelper.m b/Signal/src/ViewControllers/AvatarViewHelper.m index ce3d23f7f5..9b8848729b 100644 --- a/Signal/src/ViewControllers/AvatarViewHelper.m +++ b/Signal/src/ViewControllers/AvatarViewHelper.m @@ -130,7 +130,9 @@ NS_ASSUME_NONNULL_BEGIN CropScaleImageViewController *vc = [[CropScaleImageViewController alloc] initWithSrcImage:rawAvatar successCompletion:^(UIImage *_Nonnull dstImage) { - [self.delegate avatarDidChange:dstImage]; + dispatch_async(dispatch_get_main_queue(), ^{ + [self.delegate avatarDidChange:dstImage]; + }); }]; [self.delegate.fromViewController presentViewController:vc diff --git a/Signal/src/ViewControllers/NewGroupViewController.m b/Signal/src/ViewControllers/NewGroupViewController.m index 85281dd269..1841547404 100644 --- a/Signal/src/ViewControllers/NewGroupViewController.m +++ b/Signal/src/ViewControllers/NewGroupViewController.m @@ -619,6 +619,7 @@ const NSUInteger kNewGroupViewControllerAvatarWidth = 68; - (void)avatarDidChange:(UIImage *)image { + OWSAssert([NSThread isMainThread]); OWSAssert(image); self.groupAvatar = image; diff --git a/Signal/src/ViewControllers/ProfileViewController.m b/Signal/src/ViewControllers/ProfileViewController.m index 9d25acc0f6..210579c3e1 100644 --- a/Signal/src/ViewControllers/ProfileViewController.m +++ b/Signal/src/ViewControllers/ProfileViewController.m @@ -577,6 +577,7 @@ NSString *const kProfileView_LastPresentedDate = @"kProfileView_LastPresentedDat - (void)avatarDidChange:(UIImage *)image { + OWSAssert([NSThread isMainThread]); OWSAssert(image); self.avatar = [image resizedImageToFillPixelSize:CGSizeMake(kOWSProfileManager_MaxAvatarDiameter, diff --git a/Signal/src/ViewControllers/UpdateGroupViewController.m b/Signal/src/ViewControllers/UpdateGroupViewController.m index 5d2f3f9c01..dd8bdbaa5d 100644 --- a/Signal/src/ViewControllers/UpdateGroupViewController.m +++ b/Signal/src/ViewControllers/UpdateGroupViewController.m @@ -496,6 +496,7 @@ NS_ASSUME_NONNULL_BEGIN - (void)avatarDidChange:(UIImage *)image { + OWSAssert([NSThread isMainThread]); OWSAssert(image); self.groupAvatar = image; diff --git a/Signal/src/environment/ExperienceUpgrades/ExperienceUpgrade.swift b/Signal/src/environment/ExperienceUpgrades/ExperienceUpgrade.swift index d3e5735bc0..9a325efd22 100644 --- a/Signal/src/environment/ExperienceUpgrades/ExperienceUpgrade.swift +++ b/Signal/src/environment/ExperienceUpgrades/ExperienceUpgrade.swift @@ -54,7 +54,7 @@ class ExperienceUpgrade: TSYapDatabaseObject { // these models in a "change log" archive. if propertyKey == "title" || propertyKey == "body" || propertyKey == "image" { return MTLPropertyStorageNone - } else if propertyKey == "uniqueId" || propertyKey == "seenAt" { + } else if propertyKey == "uniqueId" || propertyKey == "seenAt" || propertyKey == "hasEverBeenSaved" { return super.storageBehaviorForProperty(withKey: propertyKey) } else { // Being conservative here in case we rename a property. diff --git a/SignalMessaging/profiles/OWSUserProfile.m b/SignalMessaging/profiles/OWSUserProfile.m index 1b9e4edf14..00593e0716 100644 --- a/SignalMessaging/profiles/OWSUserProfile.m +++ b/SignalMessaging/profiles/OWSUserProfile.m @@ -87,18 +87,21 @@ NSString *const kLocalProfileUniqueId = @"kLocalProfileUniqueId"; #pragma mark - Update With... Methods -- (void)finalizeWithCompletion:(nullable OWSUserProfileCompletion)externalCompletion didChange:(BOOL)didChange +- (void)finalizeWithCompletion:(nullable OWSUserProfileCompletion)externalCompletion { + DDLogVerbose(@"%@, after: %@", self.logTag, self.debugDescription); + if (externalCompletion) { dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), externalCompletion); } - if (!didChange) { - return; - } - BOOL isLocalUserProfile = [self.recipientId isEqualToString:kLocalProfileUniqueId]; + // if (isLocalUserProfile) { + // [DDLog flushLog]; + // DDLogError(@"%@", self.logTag); + // } + dispatch_async(dispatch_get_main_queue(), ^{ if (isLocalUserProfile) { // We populate an initial (empty) profile on launch of a new install, but until @@ -134,24 +137,34 @@ NSString *const kLocalProfileUniqueId = @"kLocalProfileUniqueId"; dbConnection:(YapDatabaseConnection *)dbConnection completion:(nullable OWSUserProfileCompletion)completion { - __block BOOL didChange = NO; + DDLogVerbose(@"%@ %s, before: %@", self.logTag, __PRETTY_FUNCTION__, self.debugDescription); + + if (self.hasEverBeenSaved) { + BOOL didChange = NO; + + didChange |= [self didStringChange:self.profileName newValue:[profileName ows_stripped]]; + didChange |= [self didStringChange:self.avatarUrlPath newValue:avatarUrlPath]; + didChange |= [self didStringChange:self.avatarFileName newValue:avatarFileName]; + + if (!didChange) { + DDLogVerbose(@"%@ Ignoring update in %s: %@", self.logTag, __PRETTY_FUNCTION__, self.debugDescription); + if (completion) { + dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), completion); + } + return; + } + } + [dbConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { [self applyChangeToSelfAndLatestCopy:transaction changeBlock:^(OWSUserProfile *userProfile) { - didChange |= [self didStringChange:userProfile.profileName - newValue:[profileName ows_stripped]]; - didChange |= - [self didStringChange:userProfile.avatarUrlPath newValue:avatarUrlPath]; - didChange |= - [self didStringChange:userProfile.avatarFileName newValue:avatarFileName]; - [userProfile setProfileName:[profileName ows_stripped]]; [userProfile setAvatarUrlPath:avatarUrlPath]; [userProfile setAvatarFileName:avatarFileName]; } saveIfMissing:YES]; }]; - [self finalizeWithCompletion:completion didChange:didChange]; + [self finalizeWithCompletion:completion]; } - (void)updateWithProfileName:(nullable NSString *)profileName @@ -161,18 +174,29 @@ NSString *const kLocalProfileUniqueId = @"kLocalProfileUniqueId"; dbConnection:(YapDatabaseConnection *)dbConnection completion:(nullable OWSUserProfileCompletion)completion { - __block BOOL didChange = NO; + DDLogVerbose(@"%@ %s, before: %@", self.logTag, __PRETTY_FUNCTION__, self.debugDescription); + + if (self.hasEverBeenSaved) { + BOOL didChange = NO; + + didChange |= [self didStringChange:self.profileName newValue:[profileName ows_stripped]]; + didChange |= [self didStringChange:self.avatarUrlPath newValue:avatarUrlPath]; + didChange |= [self didStringChange:self.avatarFileName newValue:avatarFileName]; + didChange |= [self didDateChange:self.lastUpdateDate newValue:lastUpdateDate]; + + if (!didChange) { + DDLogVerbose(@"%@ Ignoring update in %s: %@", self.logTag, __PRETTY_FUNCTION__, self.debugDescription); + if (completion) { + dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), completion); + } + return; + } + } + [dbConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { [self applyChangeToSelfAndLatestCopy:transaction changeBlock:^(OWSUserProfile *userProfile) { - didChange |= [self didStringChange:userProfile.profileName - newValue:[profileName ows_stripped]]; - didChange |= [self didStringChange:userProfile.avatarUrlPath newValue:avatarUrlPath]; - didChange |= - [self didStringChange:userProfile.avatarFileName newValue:avatarFileName]; - didChange |= [self didDateChange:userProfile.lastUpdateDate newValue:lastUpdateDate]; - [userProfile setProfileName:[profileName ows_stripped]]; [userProfile setAvatarUrlPath:avatarUrlPath]; [userProfile setAvatarFileName:avatarFileName]; @@ -180,7 +204,7 @@ NSString *const kLocalProfileUniqueId = @"kLocalProfileUniqueId"; } saveIfMissing:YES]; }]; - [self finalizeWithCompletion:completion didChange:didChange]; + [self finalizeWithCompletion:completion]; } - (void)updateWithProfileName:(nullable NSString *)profileName @@ -189,23 +213,35 @@ NSString *const kLocalProfileUniqueId = @"kLocalProfileUniqueId"; dbConnection:(YapDatabaseConnection *)dbConnection completion:(nullable OWSUserProfileCompletion)completion { - __block BOOL didChange = NO; + DDLogVerbose(@"%@ %s, before: %@", self.logTag, __PRETTY_FUNCTION__, self.debugDescription); + + if (self.hasEverBeenSaved) { + BOOL didChange = NO; + + didChange |= [self didStringChange:self.profileName newValue:[profileName ows_stripped]]; + didChange |= [self didStringChange:self.avatarUrlPath newValue:avatarUrlPath]; + didChange |= [self didDateChange:self.lastUpdateDate newValue:lastUpdateDate]; + + if (!didChange) { + DDLogVerbose(@"%@ Ignoring update in %s: %@", self.logTag, __PRETTY_FUNCTION__, self.debugDescription); + if (completion) { + dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), completion); + } + return; + } + } + [dbConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { [self applyChangeToSelfAndLatestCopy:transaction changeBlock:^(OWSUserProfile *userProfile) { - didChange |= [self didStringChange:userProfile.profileName - newValue:[profileName ows_stripped]]; - didChange |= [self didStringChange:userProfile.avatarUrlPath newValue:avatarUrlPath]; - didChange |= [self didDateChange:userProfile.lastUpdateDate newValue:lastUpdateDate]; - [userProfile setProfileName:[profileName ows_stripped]]; [userProfile setAvatarUrlPath:avatarUrlPath]; [userProfile setLastUpdateDate:lastUpdateDate]; } saveIfMissing:YES]; }]; - [self finalizeWithCompletion:completion didChange:didChange]; + [self finalizeWithCompletion:completion]; } - (void)updateWithProfileName:(nullable NSString *)profileName @@ -215,18 +251,28 @@ NSString *const kLocalProfileUniqueId = @"kLocalProfileUniqueId"; dbConnection:(YapDatabaseConnection *)dbConnection completion:(nullable OWSUserProfileCompletion)completion { - __block BOOL didChange = NO; + DDLogVerbose(@"%@ %s, before: %@", self.logTag, __PRETTY_FUNCTION__, self.debugDescription); + + if (self.hasEverBeenSaved) { + BOOL didChange = NO; + + didChange |= [self didStringChange:self.profileName newValue:[profileName ows_stripped]]; + didChange |= [self didKeyChange:self.profileKey newValue:profileKey]; + didChange |= [self didStringChange:self.avatarUrlPath newValue:avatarUrlPath]; + didChange |= [self didStringChange:self.avatarFileName newValue:avatarFileName]; + + if (!didChange) { + DDLogVerbose(@"%@ Ignoring update in %s: %@", self.logTag, __PRETTY_FUNCTION__, self.debugDescription); + if (completion) { + dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), completion); + } + return; + } + } + [dbConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { [self applyChangeToSelfAndLatestCopy:transaction changeBlock:^(OWSUserProfile *userProfile) { - didChange |= [self didStringChange:userProfile.profileName - newValue:[profileName ows_stripped]]; - didChange |= [self didKeyChange:userProfile.profileKey newValue:profileKey]; - didChange |= - [self didStringChange:userProfile.avatarUrlPath newValue:avatarUrlPath]; - didChange |= - [self didStringChange:userProfile.avatarFileName newValue:avatarFileName]; - [userProfile setProfileName:[profileName ows_stripped]]; [userProfile setProfileKey:profileKey]; [userProfile setAvatarUrlPath:avatarUrlPath]; @@ -234,7 +280,7 @@ NSString *const kLocalProfileUniqueId = @"kLocalProfileUniqueId"; } saveIfMissing:YES]; }]; - [self finalizeWithCompletion:completion didChange:didChange]; + [self finalizeWithCompletion:completion]; } - (void)updateWithAvatarUrlPath:(nullable NSString *)avatarUrlPath @@ -242,75 +288,124 @@ NSString *const kLocalProfileUniqueId = @"kLocalProfileUniqueId"; dbConnection:(YapDatabaseConnection *)dbConnection completion:(nullable OWSUserProfileCompletion)completion { - __block BOOL didChange = NO; + DDLogVerbose(@"%@ %s, before: %@", self.logTag, __PRETTY_FUNCTION__, self.debugDescription); + + if (self.hasEverBeenSaved) { + BOOL didChange = NO; + + didChange |= [self didStringChange:self.avatarUrlPath newValue:avatarUrlPath]; + didChange |= [self didStringChange:self.avatarFileName newValue:avatarFileName]; + + if (!didChange) { + DDLogVerbose(@"%@ Ignoring update in %s: %@", self.logTag, __PRETTY_FUNCTION__, self.debugDescription); + if (completion) { + dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), completion); + } + return; + } + } + [dbConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { [self applyChangeToSelfAndLatestCopy:transaction changeBlock:^(OWSUserProfile *userProfile) { - didChange |= - [self didStringChange:userProfile.avatarUrlPath newValue:avatarUrlPath]; - didChange |= - [self didStringChange:userProfile.avatarFileName newValue:avatarFileName]; [userProfile setAvatarUrlPath:avatarUrlPath]; [userProfile setAvatarFileName:avatarFileName]; } saveIfMissing:YES]; }]; - [self finalizeWithCompletion:completion didChange:didChange]; + [self finalizeWithCompletion:completion]; } - (void)updateWithAvatarFileName:(nullable NSString *)avatarFileName dbConnection:(YapDatabaseConnection *)dbConnection completion:(nullable OWSUserProfileCompletion)completion { - __block BOOL didChange = NO; + DDLogVerbose(@"%@ %s, before: %@", self.logTag, __PRETTY_FUNCTION__, self.debugDescription); + + if (self.hasEverBeenSaved) { + BOOL didChange = NO; + + didChange |= [self didStringChange:self.avatarFileName newValue:avatarFileName]; + + if (!didChange) { + DDLogVerbose(@"%@ Ignoring update in %s: %@", self.logTag, __PRETTY_FUNCTION__, self.debugDescription); + if (completion) { + dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), completion); + } + return; + } + } + [dbConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { [self applyChangeToSelfAndLatestCopy:transaction changeBlock:^(OWSUserProfile *userProfile) { - didChange |= - [self didStringChange:userProfile.avatarFileName newValue:avatarFileName]; - [userProfile setAvatarFileName:avatarFileName]; } saveIfMissing:YES]; }]; - [self finalizeWithCompletion:completion didChange:didChange]; + [self finalizeWithCompletion:completion]; } - (void)updateWithLastUpdateDate:(nullable NSDate *)lastUpdateDate dbConnection:(YapDatabaseConnection *)dbConnection completion:(nullable OWSUserProfileCompletion)completion { - __block BOOL didChange = NO; + DDLogVerbose(@"%@ %s, before: %@", self.logTag, __PRETTY_FUNCTION__, self.debugDescription); + + if (self.hasEverBeenSaved) { + BOOL didChange = NO; + + didChange |= [self didDateChange:self.lastUpdateDate newValue:lastUpdateDate]; + + if (!didChange) { + DDLogVerbose(@"%@ Ignoring update in %s: %@", self.logTag, __PRETTY_FUNCTION__, self.debugDescription); + if (completion) { + dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), completion); + } + return; + } + } + [dbConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { [self applyChangeToSelfAndLatestCopy:transaction changeBlock:^(OWSUserProfile *userProfile) { - didChange |= - [self didDateChange:userProfile.lastUpdateDate newValue:lastUpdateDate]; - [userProfile setLastUpdateDate:lastUpdateDate]; } saveIfMissing:YES]; }]; - [self finalizeWithCompletion:completion didChange:didChange]; + [self finalizeWithCompletion:completion]; } - (void)clearWithProfileKey:(OWSAES256Key *)profileKey dbConnection:(YapDatabaseConnection *)dbConnection completion:(nullable OWSUserProfileCompletion)completion; { + DDLogVerbose(@"%@ %s, before: %@", self.logTag, __PRETTY_FUNCTION__, self.debugDescription); + OWSAssert(profileKey); - __block BOOL didChange = NO; + if (self.hasEverBeenSaved) { + BOOL didChange = NO; + + didChange |= [self didKeyChange:self.profileKey newValue:profileKey]; + didChange |= [self didStringChange:self.profileName newValue:nil]; + didChange |= [self didStringChange:self.avatarUrlPath newValue:nil]; + didChange |= [self didStringChange:self.avatarFileName newValue:nil]; + didChange |= [self didDateChange:self.lastUpdateDate newValue:nil]; + + if (!didChange) { + DDLogVerbose(@"%@ Ignoring update in %s: %@", self.logTag, __PRETTY_FUNCTION__, self.debugDescription); + if (completion) { + dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), completion); + } + return; + } + } + [dbConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { [self applyChangeToSelfAndLatestCopy:transaction changeBlock:^(OWSUserProfile *userProfile) { - didChange |= [self didKeyChange:userProfile.profileKey newValue:profileKey]; - didChange |= [self didStringChange:userProfile.profileName newValue:nil]; - didChange |= [self didStringChange:userProfile.avatarUrlPath newValue:nil]; - didChange |= [self didStringChange:userProfile.avatarFileName newValue:nil]; - didChange |= [self didDateChange:userProfile.lastUpdateDate newValue:nil]; - [userProfile setProfileKey:profileKey]; [userProfile setProfileName:nil]; [userProfile setAvatarUrlPath:nil]; @@ -319,28 +414,39 @@ NSString *const kLocalProfileUniqueId = @"kLocalProfileUniqueId"; } saveIfMissing:YES]; }]; - [self finalizeWithCompletion:completion didChange:didChange]; + [self finalizeWithCompletion:completion]; } - (void)updateWithProfileKey:(OWSAES256Key *)profileKey dbConnection:(YapDatabaseConnection *)dbConnection completion:(nullable OWSUserProfileCompletion)completion; { + DDLogVerbose(@"%@ %s, before: %@", self.logTag, __PRETTY_FUNCTION__, self.debugDescription); + OWSAssert(profileKey); - self.profileKey = profileKey; + if (self.hasEverBeenSaved) { + BOOL didChange = NO; + + didChange |= [self didKeyChange:self.profileKey newValue:profileKey]; + + if (!didChange) { + DDLogVerbose(@"%@ Ignoring update in %s: %@", self.logTag, __PRETTY_FUNCTION__, self.debugDescription); + if (completion) { + dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), completion); + } + return; + } + } - __block BOOL didChange = NO; [dbConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { [self applyChangeToSelfAndLatestCopy:transaction changeBlock:^(OWSUserProfile *userProfile) { - didChange |= [self didKeyChange:userProfile.profileKey newValue:profileKey]; - [userProfile setProfileKey:profileKey]; } saveIfMissing:YES]; }]; - [self finalizeWithCompletion:completion didChange:didChange]; + [self finalizeWithCompletion:completion]; } - (BOOL)didStringChange:(NSString *_Nullable)oldValue newValue:(NSString *_Nullable)newValue @@ -406,6 +512,19 @@ NSString *const kLocalProfileUniqueId = @"kLocalProfileUniqueId"; return TSYapDatabaseObject.dbReadWriteConnection; } +- (NSString *)debugDescription +{ + return [NSString stringWithFormat:@"%@ %p %@ %@ %@ %@ %@ %f", + self.logTag, + self, + self.recipientId, + self.profileKey.keyData.hexadecimalString, + self.profileName, + self.avatarUrlPath, + self.avatarFileName, + self.lastUpdateDate.timeIntervalSinceNow]; +} + @end NS_ASSUME_NONNULL_END diff --git a/SignalServiceKit/src/Storage/TSYapDatabaseObject.h b/SignalServiceKit/src/Storage/TSYapDatabaseObject.h index 728b7b38df..c516edd0ba 100644 --- a/SignalServiceKit/src/Storage/TSYapDatabaseObject.h +++ b/SignalServiceKit/src/Storage/TSYapDatabaseObject.h @@ -13,6 +13,8 @@ NS_ASSUME_NONNULL_BEGIN @interface TSYapDatabaseObject : MTLModel +@property (atomic, readonly) BOOL hasEverBeenSaved; + /** * Initializes a new database object with a unique identifier * diff --git a/SignalServiceKit/src/Storage/TSYapDatabaseObject.m b/SignalServiceKit/src/Storage/TSYapDatabaseObject.m index 0b6d4e5410..b189d8f3af 100644 --- a/SignalServiceKit/src/Storage/TSYapDatabaseObject.m +++ b/SignalServiceKit/src/Storage/TSYapDatabaseObject.m @@ -8,6 +8,14 @@ NS_ASSUME_NONNULL_BEGIN +@interface TSYapDatabaseObject () + +@property (atomic) BOOL hasEverBeenSaved; + +@end + +#pragma mark - + @implementation TSYapDatabaseObject - (instancetype)init @@ -29,12 +37,21 @@ NS_ASSUME_NONNULL_BEGIN - (nullable instancetype)initWithCoder:(NSCoder *)coder { - return [super initWithCoder:coder]; + self = [super initWithCoder:coder]; + if (!self) { + return self; + } + + self.hasEverBeenSaved = YES; + + return self; } - (void)saveWithTransaction:(YapDatabaseReadWriteTransaction *)transaction { [transaction setObject:self forKey:self.uniqueId inCollection:[[self class] collection]]; + + self.hasEverBeenSaved = YES; } - (void)save From ee92efd4a8c3af79783ae7db0339fa6d7542229e Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Fri, 8 Dec 2017 15:21:38 -0500 Subject: [PATCH 08/11] Don't emit "user profile changed" notifications if nothing changed. --- SignalMessaging/profiles/OWSUserProfile.m | 23 ----------------------- 1 file changed, 23 deletions(-) diff --git a/SignalMessaging/profiles/OWSUserProfile.m b/SignalMessaging/profiles/OWSUserProfile.m index 00593e0716..0b310639f8 100644 --- a/SignalMessaging/profiles/OWSUserProfile.m +++ b/SignalMessaging/profiles/OWSUserProfile.m @@ -97,11 +97,6 @@ NSString *const kLocalProfileUniqueId = @"kLocalProfileUniqueId"; BOOL isLocalUserProfile = [self.recipientId isEqualToString:kLocalProfileUniqueId]; - // if (isLocalUserProfile) { - // [DDLog flushLog]; - // DDLogError(@"%@", self.logTag); - // } - dispatch_async(dispatch_get_main_queue(), ^{ if (isLocalUserProfile) { // We populate an initial (empty) profile on launch of a new install, but until @@ -137,8 +132,6 @@ NSString *const kLocalProfileUniqueId = @"kLocalProfileUniqueId"; dbConnection:(YapDatabaseConnection *)dbConnection completion:(nullable OWSUserProfileCompletion)completion { - DDLogVerbose(@"%@ %s, before: %@", self.logTag, __PRETTY_FUNCTION__, self.debugDescription); - if (self.hasEverBeenSaved) { BOOL didChange = NO; @@ -174,8 +167,6 @@ NSString *const kLocalProfileUniqueId = @"kLocalProfileUniqueId"; dbConnection:(YapDatabaseConnection *)dbConnection completion:(nullable OWSUserProfileCompletion)completion { - DDLogVerbose(@"%@ %s, before: %@", self.logTag, __PRETTY_FUNCTION__, self.debugDescription); - if (self.hasEverBeenSaved) { BOOL didChange = NO; @@ -213,8 +204,6 @@ NSString *const kLocalProfileUniqueId = @"kLocalProfileUniqueId"; dbConnection:(YapDatabaseConnection *)dbConnection completion:(nullable OWSUserProfileCompletion)completion { - DDLogVerbose(@"%@ %s, before: %@", self.logTag, __PRETTY_FUNCTION__, self.debugDescription); - if (self.hasEverBeenSaved) { BOOL didChange = NO; @@ -251,8 +240,6 @@ NSString *const kLocalProfileUniqueId = @"kLocalProfileUniqueId"; dbConnection:(YapDatabaseConnection *)dbConnection completion:(nullable OWSUserProfileCompletion)completion { - DDLogVerbose(@"%@ %s, before: %@", self.logTag, __PRETTY_FUNCTION__, self.debugDescription); - if (self.hasEverBeenSaved) { BOOL didChange = NO; @@ -288,8 +275,6 @@ NSString *const kLocalProfileUniqueId = @"kLocalProfileUniqueId"; dbConnection:(YapDatabaseConnection *)dbConnection completion:(nullable OWSUserProfileCompletion)completion { - DDLogVerbose(@"%@ %s, before: %@", self.logTag, __PRETTY_FUNCTION__, self.debugDescription); - if (self.hasEverBeenSaved) { BOOL didChange = NO; @@ -321,8 +306,6 @@ NSString *const kLocalProfileUniqueId = @"kLocalProfileUniqueId"; dbConnection:(YapDatabaseConnection *)dbConnection completion:(nullable OWSUserProfileCompletion)completion { - DDLogVerbose(@"%@ %s, before: %@", self.logTag, __PRETTY_FUNCTION__, self.debugDescription); - if (self.hasEverBeenSaved) { BOOL didChange = NO; @@ -351,8 +334,6 @@ NSString *const kLocalProfileUniqueId = @"kLocalProfileUniqueId"; dbConnection:(YapDatabaseConnection *)dbConnection completion:(nullable OWSUserProfileCompletion)completion { - DDLogVerbose(@"%@ %s, before: %@", self.logTag, __PRETTY_FUNCTION__, self.debugDescription); - if (self.hasEverBeenSaved) { BOOL didChange = NO; @@ -381,8 +362,6 @@ NSString *const kLocalProfileUniqueId = @"kLocalProfileUniqueId"; dbConnection:(YapDatabaseConnection *)dbConnection completion:(nullable OWSUserProfileCompletion)completion; { - DDLogVerbose(@"%@ %s, before: %@", self.logTag, __PRETTY_FUNCTION__, self.debugDescription); - OWSAssert(profileKey); if (self.hasEverBeenSaved) { @@ -421,8 +400,6 @@ NSString *const kLocalProfileUniqueId = @"kLocalProfileUniqueId"; dbConnection:(YapDatabaseConnection *)dbConnection completion:(nullable OWSUserProfileCompletion)completion; { - DDLogVerbose(@"%@ %s, before: %@", self.logTag, __PRETTY_FUNCTION__, self.debugDescription); - OWSAssert(profileKey); if (self.hasEverBeenSaved) { From 429312523f60055b5ae747c2db8a399110079c60 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Fri, 8 Dec 2017 15:43:22 -0500 Subject: [PATCH 09/11] Simplify OWSUserProfile's "apply changes" logic using model diffing. --- .../ExperienceUpgrade.swift | 2 +- SignalMessaging/profiles/OWSUserProfile.m | 362 +++++------------- .../src/Storage/TSYapDatabaseObject.h | 2 - .../src/Storage/TSYapDatabaseObject.m | 12 - 4 files changed, 102 insertions(+), 276 deletions(-) diff --git a/Signal/src/environment/ExperienceUpgrades/ExperienceUpgrade.swift b/Signal/src/environment/ExperienceUpgrades/ExperienceUpgrade.swift index 9a325efd22..d3e5735bc0 100644 --- a/Signal/src/environment/ExperienceUpgrades/ExperienceUpgrade.swift +++ b/Signal/src/environment/ExperienceUpgrades/ExperienceUpgrade.swift @@ -54,7 +54,7 @@ class ExperienceUpgrade: TSYapDatabaseObject { // these models in a "change log" archive. if propertyKey == "title" || propertyKey == "body" || propertyKey == "image" { return MTLPropertyStorageNone - } else if propertyKey == "uniqueId" || propertyKey == "seenAt" || propertyKey == "hasEverBeenSaved" { + } else if propertyKey == "uniqueId" || propertyKey == "seenAt" { return super.storageBehaviorForProperty(withKey: propertyKey) } else { // Being conservative here in case we rename a property. diff --git a/SignalMessaging/profiles/OWSUserProfile.m b/SignalMessaging/profiles/OWSUserProfile.m index 0b310639f8..460ca7d193 100644 --- a/SignalMessaging/profiles/OWSUserProfile.m +++ b/SignalMessaging/profiles/OWSUserProfile.m @@ -10,6 +10,7 @@ #import #import #import +#import NS_ASSUME_NONNULL_BEGIN @@ -87,12 +88,48 @@ NSString *const kLocalProfileUniqueId = @"kLocalProfileUniqueId"; #pragma mark - Update With... Methods -- (void)finalizeWithCompletion:(nullable OWSUserProfileCompletion)externalCompletion +// Similar in spirit to [TSYapDatabaseObject applyChangeToSelfAndLatestCopy], +// but with significant differences. +// +// * We save if this entity is not in the database. +// * We skip redundant saves by diffing. +// * We kick off multi-device synchronization. +// * We fire "did change" notifications. +- (void)applyChanges:(void (^)(id))changeBlock + functionName:(const char *)functionName + dbConnection:(YapDatabaseConnection *)dbConnection + completion:(nullable OWSUserProfileCompletion)completion { - DDLogVerbose(@"%@, after: %@", self.logTag, self.debugDescription); + id beforeSnapshot = self.dictionaryValue; - if (externalCompletion) { - dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), externalCompletion); + changeBlock(self); + + __block BOOL didChange = YES; + [dbConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { + NSString *collection = [[self class] collection]; + OWSUserProfile *latestInstance = [transaction objectForKey:self.uniqueId inCollection:collection]; + if (latestInstance) { + changeBlock(latestInstance); + + id afterSnapshot = latestInstance.dictionaryValue; + if ([beforeSnapshot isEqual:afterSnapshot]) { + DDLogVerbose( + @"%@ Ignoring redundant update in %s: %@", self.logTag, functionName, self.debugDescription); + didChange = NO; + } else { + [latestInstance saveWithTransaction:transaction]; + } + } else { + [self saveWithTransaction:transaction]; + } + }]; + + if (completion) { + dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), completion); + } + + if (!didChange) { + return; } BOOL isLocalUserProfile = [self.recipientId isEqualToString:kLocalProfileUniqueId]; @@ -132,32 +169,14 @@ NSString *const kLocalProfileUniqueId = @"kLocalProfileUniqueId"; dbConnection:(YapDatabaseConnection *)dbConnection completion:(nullable OWSUserProfileCompletion)completion { - if (self.hasEverBeenSaved) { - BOOL didChange = NO; - - didChange |= [self didStringChange:self.profileName newValue:[profileName ows_stripped]]; - didChange |= [self didStringChange:self.avatarUrlPath newValue:avatarUrlPath]; - didChange |= [self didStringChange:self.avatarFileName newValue:avatarFileName]; - - if (!didChange) { - DDLogVerbose(@"%@ Ignoring update in %s: %@", self.logTag, __PRETTY_FUNCTION__, self.debugDescription); - if (completion) { - dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), completion); - } - return; - } + [self applyChanges:^(OWSUserProfile *userProfile) { + [userProfile setProfileName:[profileName ows_stripped]]; + [userProfile setAvatarUrlPath:avatarUrlPath]; + [userProfile setAvatarFileName:avatarFileName]; } - - [dbConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { - [self applyChangeToSelfAndLatestCopy:transaction - changeBlock:^(OWSUserProfile *userProfile) { - [userProfile setProfileName:[profileName ows_stripped]]; - [userProfile setAvatarUrlPath:avatarUrlPath]; - [userProfile setAvatarFileName:avatarFileName]; - } - saveIfMissing:YES]; - }]; - [self finalizeWithCompletion:completion]; + functionName:__PRETTY_FUNCTION__ + dbConnection:dbConnection + completion:completion]; } - (void)updateWithProfileName:(nullable NSString *)profileName @@ -167,35 +186,15 @@ NSString *const kLocalProfileUniqueId = @"kLocalProfileUniqueId"; dbConnection:(YapDatabaseConnection *)dbConnection completion:(nullable OWSUserProfileCompletion)completion { - if (self.hasEverBeenSaved) { - BOOL didChange = NO; - - didChange |= [self didStringChange:self.profileName newValue:[profileName ows_stripped]]; - didChange |= [self didStringChange:self.avatarUrlPath newValue:avatarUrlPath]; - didChange |= [self didStringChange:self.avatarFileName newValue:avatarFileName]; - didChange |= [self didDateChange:self.lastUpdateDate newValue:lastUpdateDate]; - - if (!didChange) { - DDLogVerbose(@"%@ Ignoring update in %s: %@", self.logTag, __PRETTY_FUNCTION__, self.debugDescription); - if (completion) { - dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), completion); - } - return; - } + [self applyChanges:^(OWSUserProfile *userProfile) { + [userProfile setProfileName:[profileName ows_stripped]]; + [userProfile setAvatarUrlPath:avatarUrlPath]; + [userProfile setAvatarFileName:avatarFileName]; + [userProfile setLastUpdateDate:lastUpdateDate]; } - - [dbConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { - [self - applyChangeToSelfAndLatestCopy:transaction - changeBlock:^(OWSUserProfile *userProfile) { - [userProfile setProfileName:[profileName ows_stripped]]; - [userProfile setAvatarUrlPath:avatarUrlPath]; - [userProfile setAvatarFileName:avatarFileName]; - [userProfile setLastUpdateDate:lastUpdateDate]; - } - saveIfMissing:YES]; - }]; - [self finalizeWithCompletion:completion]; + functionName:__PRETTY_FUNCTION__ + dbConnection:dbConnection + completion:completion]; } - (void)updateWithProfileName:(nullable NSString *)profileName @@ -204,33 +203,14 @@ NSString *const kLocalProfileUniqueId = @"kLocalProfileUniqueId"; dbConnection:(YapDatabaseConnection *)dbConnection completion:(nullable OWSUserProfileCompletion)completion { - if (self.hasEverBeenSaved) { - BOOL didChange = NO; - - didChange |= [self didStringChange:self.profileName newValue:[profileName ows_stripped]]; - didChange |= [self didStringChange:self.avatarUrlPath newValue:avatarUrlPath]; - didChange |= [self didDateChange:self.lastUpdateDate newValue:lastUpdateDate]; - - if (!didChange) { - DDLogVerbose(@"%@ Ignoring update in %s: %@", self.logTag, __PRETTY_FUNCTION__, self.debugDescription); - if (completion) { - dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), completion); - } - return; - } + [self applyChanges:^(OWSUserProfile *userProfile) { + [userProfile setProfileName:[profileName ows_stripped]]; + [userProfile setAvatarUrlPath:avatarUrlPath]; + [userProfile setLastUpdateDate:lastUpdateDate]; } - - [dbConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { - [self - applyChangeToSelfAndLatestCopy:transaction - changeBlock:^(OWSUserProfile *userProfile) { - [userProfile setProfileName:[profileName ows_stripped]]; - [userProfile setAvatarUrlPath:avatarUrlPath]; - [userProfile setLastUpdateDate:lastUpdateDate]; - } - saveIfMissing:YES]; - }]; - [self finalizeWithCompletion:completion]; + functionName:__PRETTY_FUNCTION__ + dbConnection:dbConnection + completion:completion]; } - (void)updateWithProfileName:(nullable NSString *)profileName @@ -240,34 +220,15 @@ NSString *const kLocalProfileUniqueId = @"kLocalProfileUniqueId"; dbConnection:(YapDatabaseConnection *)dbConnection completion:(nullable OWSUserProfileCompletion)completion { - if (self.hasEverBeenSaved) { - BOOL didChange = NO; - - didChange |= [self didStringChange:self.profileName newValue:[profileName ows_stripped]]; - didChange |= [self didKeyChange:self.profileKey newValue:profileKey]; - didChange |= [self didStringChange:self.avatarUrlPath newValue:avatarUrlPath]; - didChange |= [self didStringChange:self.avatarFileName newValue:avatarFileName]; - - if (!didChange) { - DDLogVerbose(@"%@ Ignoring update in %s: %@", self.logTag, __PRETTY_FUNCTION__, self.debugDescription); - if (completion) { - dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), completion); - } - return; - } + [self applyChanges:^(OWSUserProfile *userProfile) { + [userProfile setProfileName:[profileName ows_stripped]]; + [userProfile setProfileKey:profileKey]; + [userProfile setAvatarUrlPath:avatarUrlPath]; + [userProfile setAvatarFileName:avatarFileName]; } - - [dbConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { - [self applyChangeToSelfAndLatestCopy:transaction - changeBlock:^(OWSUserProfile *userProfile) { - [userProfile setProfileName:[profileName ows_stripped]]; - [userProfile setProfileKey:profileKey]; - [userProfile setAvatarUrlPath:avatarUrlPath]; - [userProfile setAvatarFileName:avatarFileName]; - } - saveIfMissing:YES]; - }]; - [self finalizeWithCompletion:completion]; + functionName:__PRETTY_FUNCTION__ + dbConnection:dbConnection + completion:completion]; } - (void)updateWithAvatarUrlPath:(nullable NSString *)avatarUrlPath @@ -275,125 +236,53 @@ NSString *const kLocalProfileUniqueId = @"kLocalProfileUniqueId"; dbConnection:(YapDatabaseConnection *)dbConnection completion:(nullable OWSUserProfileCompletion)completion { - if (self.hasEverBeenSaved) { - BOOL didChange = NO; - - didChange |= [self didStringChange:self.avatarUrlPath newValue:avatarUrlPath]; - didChange |= [self didStringChange:self.avatarFileName newValue:avatarFileName]; - - if (!didChange) { - DDLogVerbose(@"%@ Ignoring update in %s: %@", self.logTag, __PRETTY_FUNCTION__, self.debugDescription); - if (completion) { - dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), completion); - } - return; - } + [self applyChanges:^(OWSUserProfile *userProfile) { + [userProfile setAvatarUrlPath:avatarUrlPath]; + [userProfile setAvatarFileName:avatarFileName]; } - - [dbConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { - [self applyChangeToSelfAndLatestCopy:transaction - changeBlock:^(OWSUserProfile *userProfile) { - - [userProfile setAvatarUrlPath:avatarUrlPath]; - [userProfile setAvatarFileName:avatarFileName]; - } - saveIfMissing:YES]; - }]; - [self finalizeWithCompletion:completion]; + functionName:__PRETTY_FUNCTION__ + dbConnection:dbConnection + completion:completion]; } - (void)updateWithAvatarFileName:(nullable NSString *)avatarFileName dbConnection:(YapDatabaseConnection *)dbConnection completion:(nullable OWSUserProfileCompletion)completion { - if (self.hasEverBeenSaved) { - BOOL didChange = NO; - - didChange |= [self didStringChange:self.avatarFileName newValue:avatarFileName]; - - if (!didChange) { - DDLogVerbose(@"%@ Ignoring update in %s: %@", self.logTag, __PRETTY_FUNCTION__, self.debugDescription); - if (completion) { - dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), completion); - } - return; - } + [self applyChanges:^(OWSUserProfile *userProfile) { + [userProfile setAvatarFileName:avatarFileName]; } - - [dbConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { - [self applyChangeToSelfAndLatestCopy:transaction - changeBlock:^(OWSUserProfile *userProfile) { - [userProfile setAvatarFileName:avatarFileName]; - } - saveIfMissing:YES]; - }]; - [self finalizeWithCompletion:completion]; + functionName:__PRETTY_FUNCTION__ + dbConnection:dbConnection + completion:completion]; } - (void)updateWithLastUpdateDate:(nullable NSDate *)lastUpdateDate dbConnection:(YapDatabaseConnection *)dbConnection completion:(nullable OWSUserProfileCompletion)completion { - if (self.hasEverBeenSaved) { - BOOL didChange = NO; - - didChange |= [self didDateChange:self.lastUpdateDate newValue:lastUpdateDate]; - - if (!didChange) { - DDLogVerbose(@"%@ Ignoring update in %s: %@", self.logTag, __PRETTY_FUNCTION__, self.debugDescription); - if (completion) { - dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), completion); - } - return; - } + [self applyChanges:^(OWSUserProfile *userProfile) { + [userProfile setLastUpdateDate:lastUpdateDate]; } - - [dbConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { - [self applyChangeToSelfAndLatestCopy:transaction - changeBlock:^(OWSUserProfile *userProfile) { - [userProfile setLastUpdateDate:lastUpdateDate]; - } - saveIfMissing:YES]; - }]; - [self finalizeWithCompletion:completion]; + functionName:__PRETTY_FUNCTION__ + dbConnection:dbConnection + completion:completion]; } - (void)clearWithProfileKey:(OWSAES256Key *)profileKey dbConnection:(YapDatabaseConnection *)dbConnection completion:(nullable OWSUserProfileCompletion)completion; { - OWSAssert(profileKey); - - if (self.hasEverBeenSaved) { - BOOL didChange = NO; - - didChange |= [self didKeyChange:self.profileKey newValue:profileKey]; - didChange |= [self didStringChange:self.profileName newValue:nil]; - didChange |= [self didStringChange:self.avatarUrlPath newValue:nil]; - didChange |= [self didStringChange:self.avatarFileName newValue:nil]; - didChange |= [self didDateChange:self.lastUpdateDate newValue:nil]; - - if (!didChange) { - DDLogVerbose(@"%@ Ignoring update in %s: %@", self.logTag, __PRETTY_FUNCTION__, self.debugDescription); - if (completion) { - dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), completion); - } - return; - } + [self applyChanges:^(OWSUserProfile *userProfile) { + [userProfile setProfileKey:profileKey]; + [userProfile setProfileName:nil]; + [userProfile setAvatarUrlPath:nil]; + [userProfile setAvatarFileName:nil]; + [userProfile setLastUpdateDate:nil]; } - - [dbConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { - [self applyChangeToSelfAndLatestCopy:transaction - changeBlock:^(OWSUserProfile *userProfile) { - [userProfile setProfileKey:profileKey]; - [userProfile setProfileName:nil]; - [userProfile setAvatarUrlPath:nil]; - [userProfile setAvatarFileName:nil]; - [userProfile setLastUpdateDate:nil]; - } - saveIfMissing:YES]; - }]; - [self finalizeWithCompletion:completion]; + functionName:__PRETTY_FUNCTION__ + dbConnection:dbConnection + completion:completion]; } - (void)updateWithProfileKey:(OWSAES256Key *)profileKey @@ -402,61 +291,12 @@ NSString *const kLocalProfileUniqueId = @"kLocalProfileUniqueId"; { OWSAssert(profileKey); - if (self.hasEverBeenSaved) { - BOOL didChange = NO; - - didChange |= [self didKeyChange:self.profileKey newValue:profileKey]; - - if (!didChange) { - DDLogVerbose(@"%@ Ignoring update in %s: %@", self.logTag, __PRETTY_FUNCTION__, self.debugDescription); - if (completion) { - dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), completion); - } - return; - } - } - - [dbConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { - [self applyChangeToSelfAndLatestCopy:transaction - changeBlock:^(OWSUserProfile *userProfile) { - [userProfile setProfileKey:profileKey]; - } - saveIfMissing:YES]; - }]; - [self finalizeWithCompletion:completion]; -} - -- (BOOL)didStringChange:(NSString *_Nullable)oldValue newValue:(NSString *_Nullable)newValue -{ - if (!oldValue && !newValue) { - return NO; - } else if (!oldValue || !newValue) { - return YES; - } else { - return ![oldValue isEqualToString:newValue]; - } -} - -- (BOOL)didDateChange:(NSDate *_Nullable)oldValue newValue:(NSDate *_Nullable)newValue -{ - if (!oldValue && !newValue) { - return NO; - } else if (!oldValue || !newValue) { - return YES; - } else { - return ![oldValue isEqualToDate:newValue]; - } -} - -- (BOOL)didKeyChange:(OWSAES256Key *_Nullable)oldValue newValue:(OWSAES256Key *_Nullable)newValue -{ - if (!oldValue && !newValue) { - return NO; - } else if (!oldValue || !newValue) { - return YES; - } else { - return ![oldValue.keyData isEqualToData:newValue.keyData]; + [self applyChanges:^(OWSUserProfile *userProfile) { + [userProfile setProfileKey:profileKey]; } + functionName:__PRETTY_FUNCTION__ + dbConnection:dbConnection + completion:completion]; } #pragma mark - Database Connection Accessors diff --git a/SignalServiceKit/src/Storage/TSYapDatabaseObject.h b/SignalServiceKit/src/Storage/TSYapDatabaseObject.h index c516edd0ba..728b7b38df 100644 --- a/SignalServiceKit/src/Storage/TSYapDatabaseObject.h +++ b/SignalServiceKit/src/Storage/TSYapDatabaseObject.h @@ -13,8 +13,6 @@ NS_ASSUME_NONNULL_BEGIN @interface TSYapDatabaseObject : MTLModel -@property (atomic, readonly) BOOL hasEverBeenSaved; - /** * Initializes a new database object with a unique identifier * diff --git a/SignalServiceKit/src/Storage/TSYapDatabaseObject.m b/SignalServiceKit/src/Storage/TSYapDatabaseObject.m index b189d8f3af..4bcd1707a2 100644 --- a/SignalServiceKit/src/Storage/TSYapDatabaseObject.m +++ b/SignalServiceKit/src/Storage/TSYapDatabaseObject.m @@ -8,14 +8,6 @@ NS_ASSUME_NONNULL_BEGIN -@interface TSYapDatabaseObject () - -@property (atomic) BOOL hasEverBeenSaved; - -@end - -#pragma mark - - @implementation TSYapDatabaseObject - (instancetype)init @@ -42,16 +34,12 @@ NS_ASSUME_NONNULL_BEGIN return self; } - self.hasEverBeenSaved = YES; - return self; } - (void)saveWithTransaction:(YapDatabaseReadWriteTransaction *)transaction { [transaction setObject:self forKey:self.uniqueId inCollection:[[self class] collection]]; - - self.hasEverBeenSaved = YES; } - (void)save From b9b3eb0541ed522d65110a850b5273398cd6e4d1 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Fri, 8 Dec 2017 15:45:56 -0500 Subject: [PATCH 10/11] Clean up ahead of PR. --- SignalServiceKit/src/Storage/TSYapDatabaseObject.h | 4 ---- SignalServiceKit/src/Storage/TSYapDatabaseObject.m | 9 --------- 2 files changed, 13 deletions(-) diff --git a/SignalServiceKit/src/Storage/TSYapDatabaseObject.h b/SignalServiceKit/src/Storage/TSYapDatabaseObject.h index 728b7b38df..03f8f115c6 100644 --- a/SignalServiceKit/src/Storage/TSYapDatabaseObject.h +++ b/SignalServiceKit/src/Storage/TSYapDatabaseObject.h @@ -151,10 +151,6 @@ NS_ASSUME_NONNULL_BEGIN - (void)applyChangeToSelfAndLatestCopy:(YapDatabaseReadWriteTransaction *)transaction changeBlock:(void (^)(id))changeBlock; -- (void)applyChangeToSelfAndLatestCopy:(YapDatabaseReadWriteTransaction *)transaction - changeBlock:(void (^)(id))changeBlock - saveIfMissing:(BOOL)saveIfMissing; - @end NS_ASSUME_NONNULL_END diff --git a/SignalServiceKit/src/Storage/TSYapDatabaseObject.m b/SignalServiceKit/src/Storage/TSYapDatabaseObject.m index 4bcd1707a2..023cf90df6 100644 --- a/SignalServiceKit/src/Storage/TSYapDatabaseObject.m +++ b/SignalServiceKit/src/Storage/TSYapDatabaseObject.m @@ -207,13 +207,6 @@ NS_ASSUME_NONNULL_BEGIN - (void)applyChangeToSelfAndLatestCopy:(YapDatabaseReadWriteTransaction *)transaction changeBlock:(void (^)(id))changeBlock -{ - [self applyChangeToSelfAndLatestCopy:transaction changeBlock:changeBlock saveIfMissing:NO]; -} - -- (void)applyChangeToSelfAndLatestCopy:(YapDatabaseReadWriteTransaction *)transaction - changeBlock:(void (^)(id))changeBlock - saveIfMissing:(BOOL)saveIfMissing { OWSAssert(transaction); @@ -224,8 +217,6 @@ NS_ASSUME_NONNULL_BEGIN if (latestInstance) { changeBlock(latestInstance); [latestInstance saveWithTransaction:transaction]; - } else if (saveIfMissing) { - [self saveWithTransaction:transaction]; } } From 8642a708e79e12737ad644b9f42aabb0766c677a Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Mon, 11 Dec 2017 12:37:47 -0500 Subject: [PATCH 11/11] Respond to CR. --- SignalMessaging/profiles/OWSProfileManager.h | 2 +- SignalMessaging/profiles/OWSProfileManager.m | 5 ++--- SignalMessaging/profiles/OWSUserProfile.h | 3 +++ SignalMessaging/profiles/OWSUserProfile.m | 11 +++++++++-- 4 files changed, 15 insertions(+), 6 deletions(-) diff --git a/SignalMessaging/profiles/OWSProfileManager.h b/SignalMessaging/profiles/OWSProfileManager.h index 149439bba0..f252f44ab5 100644 --- a/SignalMessaging/profiles/OWSProfileManager.h +++ b/SignalMessaging/profiles/OWSProfileManager.h @@ -32,7 +32,7 @@ extern const NSUInteger kOWSProfileManager_MaxAvatarDiameter; - (OWSAES256Key *)localProfileKey; // localUserProfileExists is true if there is _ANY_ local profile. - (BOOL)localProfileExists; -// localUserProfileExists is true if there is a local profile with a name or avatar. +// hasLocalProfile is true if there is a local profile with a name or avatar. - (BOOL)hasLocalProfile; - (nullable NSString *)localProfileName; - (nullable UIImage *)localProfileAvatarImage; diff --git a/SignalMessaging/profiles/OWSProfileManager.m b/SignalMessaging/profiles/OWSProfileManager.m index 4f4e54e121..07c633bcdb 100644 --- a/SignalMessaging/profiles/OWSProfileManager.m +++ b/SignalMessaging/profiles/OWSProfileManager.m @@ -54,7 +54,6 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640; @property (atomic, readonly) OWSUserProfile *localUserProfile; // This property can be accessed on any thread, while synchronized on self. -//@property (atomic, readonly) NSCache *otherUsersProfileAvatarImageCache; @property (atomic, readonly) NSCache *profileAvatarImageCache; // This property can be accessed on any thread, while synchronized on self. @@ -595,7 +594,7 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640; OWSAssert(recipientId.length > 0); __block BOOL result = NO; - [self.dbConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { + [self.dbConnection readWithBlock:^(YapDatabaseReadTransaction *transaction) { NSNumber *_Nullable oldValue = [transaction objectForKey:recipientId inCollection:kOWSProfileManager_UserWhitelistCollection]; result = (oldValue && oldValue.boolValue); @@ -661,7 +660,7 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640; NSString *groupIdKey = [groupId hexadecimalString]; __block BOOL result = NO; - [self.dbConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { + [self.dbConnection readWithBlock:^(YapDatabaseReadTransaction *transaction) { NSNumber *_Nullable oldValue = [transaction objectForKey:groupIdKey inCollection:kOWSProfileManager_GroupWhitelistCollection]; result = (oldValue && oldValue.boolValue); diff --git a/SignalMessaging/profiles/OWSUserProfile.h b/SignalMessaging/profiles/OWSUserProfile.h index 804482d749..0f9b5715b3 100644 --- a/SignalMessaging/profiles/OWSUserProfile.h +++ b/SignalMessaging/profiles/OWSUserProfile.h @@ -20,6 +20,9 @@ extern NSString *const kNSNotificationKey_ProfileGroupId; extern NSString *const kLocalProfileUniqueId; // This class should be completely thread-safe. +// +// It is critical for coherency that all DB operations for this +// class should be done on OWSProfileManager's dbConnection. @interface OWSUserProfile : TSYapDatabaseObject @property (atomic, readonly) NSString *recipientId; diff --git a/SignalMessaging/profiles/OWSUserProfile.m b/SignalMessaging/profiles/OWSUserProfile.m index 460ca7d193..cb77272138 100644 --- a/SignalMessaging/profiles/OWSUserProfile.m +++ b/SignalMessaging/profiles/OWSUserProfile.m @@ -37,6 +37,12 @@ NSString *const kLocalProfileUniqueId = @"kLocalProfileUniqueId"; @implementation OWSUserProfile ++ (NSString *)collection +{ + // Legacy class name. + return @"UserProfile"; +} + + (OWSUserProfile *)getOrBuildUserProfileForRecipientId:(NSString *)recipientId dbConnection:(YapDatabaseConnection *)dbConnection { @@ -329,13 +335,14 @@ NSString *const kLocalProfileUniqueId = @"kLocalProfileUniqueId"; return TSYapDatabaseObject.dbReadWriteConnection; } +// This should only be used in verbose, developer-only logs. - (NSString *)debugDescription { - return [NSString stringWithFormat:@"%@ %p %@ %@ %@ %@ %@ %f", + return [NSString stringWithFormat:@"%@ %p %@ %zd %@ %@ %@ %f", self.logTag, self, self.recipientId, - self.profileKey.keyData.hexadecimalString, + self.profileKey.keyData.length, self.profileName, self.avatarUrlPath, self.avatarFileName,