From c603a2651d7f6b8ab6ebb3b7831e78031a86d536 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Wed, 2 Aug 2017 10:36:54 -0400 Subject: [PATCH] =?UTF-8?q?Rework=20how=20user=20profiles=20are=20updated?= =?UTF-8?q?=20and=20persisted.=20Persist=20other=20user=E2=80=99s=20profil?= =?UTF-8?q?es.=20Load=20and=20cache=20other=20user=E2=80=99s=20profile=20a?= =?UTF-8?q?vatars.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit // FREEBIE --- .../src/Profiles/OWSProfilesManager.h | 15 +- .../src/Profiles/OWSProfilesManager.m | 335 ++++++++++-------- 2 files changed, 192 insertions(+), 158 deletions(-) diff --git a/SignalServiceKit/src/Profiles/OWSProfilesManager.h b/SignalServiceKit/src/Profiles/OWSProfilesManager.h index d7cc6f9fe..3c02ac00a 100644 --- a/SignalServiceKit/src/Profiles/OWSProfilesManager.h +++ b/SignalServiceKit/src/Profiles/OWSProfilesManager.h @@ -17,17 +17,22 @@ extern NSString *const kNSNotificationName_OtherUsersProfileDidChange; #pragma mark - Local Profile @property (atomic, readonly) NSData *localProfileKey; -@property (atomic, nullable, readonly) NSString *localProfileName; -@property (atomic, nullable, readonly) UIImage *localProfileAvatarImage; + +// These two methods should only be called from the main thread. +- (NSString *)localProfileName; +- (UIImage *)localProfileAvatarImage; // This method is used to update the "local profile" state on the client // and the service. Client state is only updated if service state is // successfully updated. -- (void)updateLocalProfileName:(nullable NSString *)localProfileName - localProfileAvatarImage:(nullable UIImage *)localProfileAvatarImage +// +// This method should only be called from the main thread. +- (void)updateLocalProfileName:(nullable NSString *)profileName + avatarImage:(nullable UIImage *)avatarImage success:(void (^)())successBlock failure:(void (^)())failureBlock; +// This method should only be called from the main thread. - (void)appLaunchDidBegin; #pragma mark - Profile Whitelist @@ -52,6 +57,8 @@ extern NSString *const kNSNotificationName_OtherUsersProfileDidChange; - (nullable UIImage *)profileAvatarForRecipientId:(NSString *)recipientId; +- (void)fetchProfileForRecipientId:(NSString *)recipientId; + @end NS_ASSUME_NONNULL_END diff --git a/SignalServiceKit/src/Profiles/OWSProfilesManager.m b/SignalServiceKit/src/Profiles/OWSProfilesManager.m index 097186b95..80bc6e346 100644 --- a/SignalServiceKit/src/Profiles/OWSProfilesManager.m +++ b/SignalServiceKit/src/Profiles/OWSProfilesManager.m @@ -4,22 +4,31 @@ #import "OWSProfilesManager.h" #import "NSData+hexString.h" +#import "NSDate+OWS.h" #import "OWSMessageSender.h" #import "SecurityUtils.h" +#import "TSAccountManager.h" #import "TSStorageManager.h" #import "TSYapDatabaseObject.h" #import "TextSecureKitEnv.h" NS_ASSUME_NONNULL_BEGIN -@class TSThread; +@interface UserProfile : TSYapDatabaseObject -@interface AvatarMetadata : TSYapDatabaseObject +@property (nonatomic, readonly) NSString *recipientId; +@property (nonatomic, nullable) NSString *profileName; +@property (nonatomic, nullable) NSString *avatarUrl; +@property (nonatomic, nullable) NSString *avatarDigest; // This filename is relative to OWSProfilesManager.profileAvatarsDirPath. -@property (nonatomic, readonly) NSString *fileName; -@property (nonatomic, readonly) NSString *avatarUrl; -@property (nonatomic, readonly) NSString *avatarDigest; +@property (nonatomic, nullable) NSString *avatarFileName; + +// This should reflect when either: +// +// * The last successful update was started. +// * The in-flight update was started. +@property (nonatomic, nullable) NSDate *lastUpdateDate; - (instancetype)init NS_UNAVAILABLE; @@ -27,46 +36,49 @@ NS_ASSUME_NONNULL_BEGIN #pragma mark - -@implementation AvatarMetadata - -+ (NSString *)collection -{ - return @"AvatarMetadata"; -} +@implementation UserProfile -- (instancetype)initWithFileName:(NSString *)fileName - avatarUrl:(NSString *)avatarUrl - avatarDigest:(NSString *)avatarDigest +- (instancetype)initWithRecipientId:(NSString *)recipientId + profileName:(NSString *_Nullable)profileName + avatarUrl:(NSString *_Nullable)avatarUrl + avatarDigest:(NSString *_Nullable)avatarDigest + avatarFileName:(NSString *_Nullable)avatarFileName { - // TODO: Local filenames for avatars are guaranteed to be unique. - self = [super initWithUniqueId:fileName]; + self = [super initWithUniqueId:recipientId]; if (!self) { return self; } - OWSAssert(fileName.length > 0); - OWSAssert(avatarUrl.length > 0); - OWSAssert(avatarDigest.length > 0); - _fileName = fileName; + OWSAssert(recipientId.length > 0); + _recipientId = recipientId; + _profileName = profileName; _avatarUrl = avatarUrl; _avatarDigest = avatarDigest; + _avatarFileName = avatarFileName; return self; } ++ (NSString *)collection +{ + return @"UserProfile"; +} #pragma mark - NSObject -- (BOOL)isEqual:(AvatarMetadata *)other +- (BOOL)isEqual:(UserProfile *)other { - return ([other isKindOfClass:[AvatarMetadata class]] && [self.fileName isEqualToString:other.fileName] && - [self.avatarUrl isEqualToString:other.avatarUrl] && [self.avatarDigest isEqualToString:other.avatarDigest]); + return ([other isKindOfClass:[UserProfile class]] && [self.recipientId isEqualToString:other.recipientId] && + [self.profileName isEqualToString:other.profileName] && [self.avatarUrl isEqualToString:other.avatarUrl] && + [self.avatarDigest isEqualToString:other.avatarDigest] && + [self.avatarFileName isEqualToString:other.avatarFileName]); } - (NSUInteger)hash { - return self.fileName.hash ^ self.avatarUrl.hash ^ self.avatarDigest.hash; + return self.recipientId.hash ^ self.profileName.hash ^ self.avatarUrl.hash ^ self.avatarDigest.hash + ^ self.avatarFileName.hash; } @end @@ -79,19 +91,12 @@ NSString *const kNSNotificationName_OtherUsersProfileDidChange = @"kNSNotificati NSString *const kOWSProfilesManager_Collection = @"kOWSProfilesManager_Collection"; // This key is used to persist the local user's profile key. NSString *const kOWSProfilesManager_LocalProfileSecretKey = @"kOWSProfilesManager_LocalProfileSecretKey"; -NSString *const kOWSProfilesManager_LocalProfileNameKey = @"kOWSProfilesManager_LocalProfileNameKey"; -NSString *const kOWSProfilesManager_LocalProfileAvatarMetadataKey - = @"kOWSProfilesManager_LocalProfileAvatarMetadataKey"; NSString *const kOWSProfilesManager_UserWhitelistCollection = @"kOWSProfilesManager_UserWhitelistCollection"; NSString *const kOWSProfilesManager_GroupWhitelistCollection = @"kOWSProfilesManager_GroupWhitelistCollection"; NSString *const kOWSProfilesManager_OtherUsersProfileKeysCollection = @"kOWSProfilesManager_OtherUsersProfileKeysCollection"; -NSString *const kOWSProfilesManager_OtherUsersProfileNamesCollection - = @"kOWSProfilesManager_OtherUsersProfileNamesCollection"; -NSString *const kOWSProfilesManager_OtherUsersProfileAvatarMetadataCollection - = @"kOWSProfilesManager_OtherUsersProfileAvatarMetadataCollection"; // TODO: static const NSInteger kProfileKeyLength = 16; @@ -101,19 +106,22 @@ static const NSInteger kProfileKeyLength = 16; @property (nonatomic, readonly) OWSMessageSender *messageSender; @property (nonatomic, readonly) YapDatabaseConnection *dbConnection; -// These properties should only be mutated on the main thread, -// but they may be accessed on other threads. -@property (atomic, nullable) NSString *localProfileName; -@property (atomic, nullable) UIImage *localProfileAvatarImage; -@property (atomic, nullable) AvatarMetadata *localProfileAvatarMetadata; +// This property should only be mutated on the main thread, +// +// NOTE: Do not access this property directly; use getOrCreateLocalUserProfile instead. +@property (nonatomic, nullable) UserProfile *localUserProfile; +// This property should only be mutated on the main thread, +@property (nonatomic, nullable) UIImage *localCachedAvatarImage; // These caches are lazy-populated. The single point truth is the database. -@property (nonatomic, readonly) NSMutableDictionary *userProfileWhitelistCache; -@property (nonatomic, readonly) NSMutableDictionary *groupProfileWhitelistCache; -@property (nonatomic, readonly) NSMutableDictionary *otherUsersProfileKeyCache; -@property (nonatomic, readonly) NSMutableDictionary *otherUsersProfileNameCache; -// TODO: Replace with NSCache. -@property (nonatomic, readonly) NSMutableDictionary *otherUsersProfileAvatarImageCache; +// +// These three properties can be accessed on any thread. +@property (atomic, readonly) NSMutableDictionary *userProfileWhitelistCache; +@property (atomic, readonly) NSMutableDictionary *groupProfileWhitelistCache; +@property (atomic, readonly) NSMutableDictionary *otherUsersProfileKeyCache; + +// This property should only be mutated on the main thread, +@property (nonatomic, readonly) NSCache *otherUsersProfileAvatarImageCache; @end @@ -157,8 +165,7 @@ static const NSInteger kProfileKeyLength = 16; _userProfileWhitelistCache = [NSMutableDictionary new]; _groupProfileWhitelistCache = [NSMutableDictionary new]; _otherUsersProfileKeyCache = [NSMutableDictionary new]; - _otherUsersProfileNameCache = [NSMutableDictionary new]; - _otherUsersProfileAvatarImageCache = [NSMutableDictionary new]; + _otherUsersProfileAvatarImageCache = [NSCache new]; OWSSingletonAssert(); @@ -179,8 +186,6 @@ static const NSInteger kProfileKeyLength = 16; } OWSAssert(_localProfileKey.length == kProfileKeyLength); - [self loadLocalProfileAsync]; - return self; } @@ -202,6 +207,48 @@ static const NSInteger kProfileKeyLength = 16; // Do nothing; we only want to make sure this singleton is created on startup. } +#pragma mark - User Profile Accessor + +- (UserProfile *)getOrCreateUserProfileForRecipientId:(NSString *)recipientId +{ + OWSAssert([NSThread isMainThread]); + OWSAssert(recipientId.length > 0); + + __block UserProfile *instance; + // Make sure to read on the local db connection for consistency. + [self.dbConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { + instance = [UserProfile fetchObjectWithUniqueID:recipientId transaction:transaction]; + + if (!instance) { + instance = [[UserProfile alloc] initWithRecipientId:recipientId + profileName:nil + avatarUrl:nil + avatarDigest:nil + avatarFileName:nil]; + [instance saveWithTransaction:transaction]; + } + }]; + + OWSAssert(instance); + + return instance; +} + +- (nullable UserProfile *)getOrCreateLocalUserProfile +{ + OWSAssert([NSThread isMainThread]); + + if (!self.localUserProfile) { + NSString *_Nullable recipientId = [TSAccountManager localNumber]; + if (!recipientId) { + OWSFail(@"Missing local number."); + return nil; + } + self.localUserProfile = [self getOrCreateUserProfileForRecipientId:recipientId]; + } + return self.localUserProfile; +} + #pragma mark - Local Profile Key + (NSData *)generateLocalProfileKey @@ -213,51 +260,22 @@ static const NSInteger kProfileKeyLength = 16; #pragma mark - Local Profile -// This method is use to update client "local profile" state. -- (void)updateLocalProfileName:(nullable NSString *)localProfileName - localProfileAvatarImage:(nullable UIImage *)localProfileAvatarImage - localProfileAvatarMetadata:(nullable AvatarMetadata *)localProfileAvatarMetadata +- (NSString *)localProfileName { OWSAssert([NSThread isMainThread]); - // The avatar image and filename should both be set, or neither should be set. - if (!localProfileAvatarMetadata && localProfileAvatarImage) { - OWSFail(@"Missing avatar metadata."); - localProfileAvatarImage = nil; - } - if (localProfileAvatarMetadata && !localProfileAvatarImage) { - OWSFail(@"Missing avatar image."); - localProfileAvatarMetadata = nil; - } - - self.localProfileName = localProfileName; - self.localProfileAvatarImage = localProfileAvatarImage; - self.localProfileAvatarMetadata = localProfileAvatarMetadata; + return [self getOrCreateLocalUserProfile].profileName; +} - if (localProfileName) { - [self.dbConnection setObject:localProfileName - forKey:kOWSProfilesManager_LocalProfileNameKey - inCollection:kOWSProfilesManager_Collection]; - } else { - [self.dbConnection removeObjectForKey:kOWSProfilesManager_LocalProfileNameKey - inCollection:kOWSProfilesManager_Collection]; - } - if (localProfileAvatarMetadata) { - [self.dbConnection setObject:localProfileAvatarMetadata - forKey:kOWSProfilesManager_LocalProfileAvatarMetadataKey - inCollection:kOWSProfilesManager_Collection]; - } else { - [self.dbConnection removeObjectForKey:kOWSProfilesManager_LocalProfileAvatarMetadataKey - inCollection:kOWSProfilesManager_Collection]; - } +- (UIImage *)localProfileAvatarImage +{ + OWSAssert([NSThread isMainThread]); - [[NSNotificationCenter defaultCenter] postNotificationName:kNSNotificationName_LocalProfileDidChange - object:nil - userInfo:nil]; + return self.localCachedAvatarImage; } -- (void)updateLocalProfileName:(nullable NSString *)localProfileName - localProfileAvatarImage:(nullable UIImage *)localProfileAvatarImage +- (void)updateLocalProfileName:(nullable NSString *)profileName + avatarImage:(nullable UIImage *)avatarImage success:(void (^)())successBlock failure:(void (^)())failureBlockParameter { @@ -276,15 +294,31 @@ static const NSInteger kProfileKeyLength = 16; // // * Try to update the service. // * Update client state on success. - void (^tryToUpdateService)(AvatarMetadata *_Nullable) = ^(AvatarMetadata *_Nullable avatarMetadata) { - [self updateProfileOnService:localProfileName - avatarMetadata:avatarMetadata + void (^tryToUpdateService)(NSString *_Nullable, NSString *_Nullable, NSString *_Nullable) = ^( + NSString *_Nullable avatarUrl, NSString *_Nullable avatarDigest, NSString *_Nullable avatarFileName) { + [self updateProfileOnService:profileName + avatarUrl:avatarUrl + avatarDigest:avatarDigest success:^{ + // All reads and writes to user profiles should happen on the main thread. dispatch_async(dispatch_get_main_queue(), ^{ - [self updateLocalProfileName:localProfileName - localProfileAvatarImage:localProfileAvatarImage - localProfileAvatarMetadata:avatarMetadata]; + UserProfile *userProfile = [self getOrCreateLocalUserProfile]; + OWSAssert(userProfile); + userProfile.profileName = profileName; + userProfile.avatarUrl = avatarUrl; + userProfile.avatarDigest = avatarDigest; + userProfile.avatarFileName = avatarFileName; + // Make sure to save on the local db connection for consistency. + [self.dbConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { + [userProfile saveWithTransaction:transaction]; + }]; + self.localCachedAvatarImage = avatarImage; + successBlock(); + + [[NSNotificationCenter defaultCenter] postNotificationName:kNSNotificationName_LocalProfileDidChange + object:nil + userInfo:nil]; }); } failure:^{ @@ -292,24 +326,31 @@ static const NSInteger kProfileKeyLength = 16; }]; }; + UserProfile *userProfile = [self getOrCreateLocalUserProfile]; + OWSAssert(userProfile); + // If we have a new avatar image, we must first: // // * Encode it to JPEG. // * Write it to disk. // * Upload it to service. - if (localProfileAvatarImage) { - if (self.localProfileAvatarMetadata && self.localProfileAvatarImage == localProfileAvatarImage) { + if (avatarImage) { + if (self.localCachedAvatarImage == avatarImage) { + OWSAssert(userProfile.avatarUrl.length > 0); + OWSAssert(userProfile.avatarDigest.length > 0); + OWSAssert(userProfile.avatarFileName.length > 0); + DDLogVerbose(@"%@ Updating local profile on service with unchanged avatar.", self.tag); // If the avatar hasn't changed, reuse the existing metadata. - tryToUpdateService(self.localProfileAvatarMetadata); + tryToUpdateService(userProfile.avatarUrl, userProfile.avatarDigest, userProfile.avatarFileName); } else { DDLogVerbose(@"%@ Updating local profile on service with new avatar.", self.tag); - [self writeAvatarToDisk:localProfileAvatarImage + [self writeAvatarToDisk:avatarImage success:^(NSData *data, NSString *fileName) { [self uploadAvatarToService:data fileName:fileName - success:^(AvatarMetadata *avatarMetadata) { - tryToUpdateService(avatarMetadata); + success:^(NSString *avatarUrl, NSString *avatarDigest) { + tryToUpdateService(avatarUrl, avatarDigest, fileName); } failure:^{ failureBlock(); @@ -321,7 +362,7 @@ static const NSInteger kProfileKeyLength = 16; } } else { DDLogVerbose(@"%@ Updating local profile on service with no avatar.", self.tag); - tryToUpdateService(nil); + tryToUpdateService(nil, nil, nil); } } @@ -355,7 +396,7 @@ static const NSInteger kProfileKeyLength = 16; // TODO: The exact API & encryption scheme for avatars is not yet settled. - (void)uploadAvatarToService:(NSData *)data fileName:(NSString *)fileName - success:(void (^)(AvatarMetadata *avatarMetadata))successBlock + success:(void (^)(NSString *avatarUrl, NSString *avatarDigest))successBlock failure:(void (^)())failureBlock { OWSAssert(data.length > 0); @@ -366,11 +407,9 @@ static const NSInteger kProfileKeyLength = 16; dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{ // TODO: NSString *avatarUrl = @"avatarUrl"; - NSString *avatarDigest = @"digest"; - AvatarMetadata *avatarMetadata = - [[AvatarMetadata alloc] initWithFileName:fileName avatarUrl:avatarUrl avatarDigest:avatarDigest]; + NSString *avatarDigest = @"avatarDigest"; if (YES) { - successBlock(avatarMetadata); + successBlock(avatarUrl, avatarDigest); return; } failureBlock(); @@ -379,7 +418,8 @@ static const NSInteger kProfileKeyLength = 16; // TODO: The exact API & encryption scheme for profiles is not yet settled. - (void)updateProfileOnService:(nullable NSString *)localProfileName - avatarMetadata:(nullable AvatarMetadata *)avatarMetadata + avatarUrl:(nullable NSString *)avatarUrl + avatarDigest:(nullable NSString *)avatarDigest success:(void (^)())successBlock failure:(void (^)())failureBlock { @@ -396,34 +436,6 @@ static const NSInteger kProfileKeyLength = 16; }); } -- (void)loadLocalProfileAsync -{ - dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{ - NSString *_Nullable localProfileName = [self.dbConnection objectForKey:kOWSProfilesManager_LocalProfileNameKey - inCollection:kOWSProfilesManager_Collection]; - AvatarMetadata *_Nullable localProfileAvatarMetadata = - [self.dbConnection objectForKey:kOWSProfilesManager_LocalProfileAvatarMetadataKey - inCollection:kOWSProfilesManager_Collection]; - UIImage *_Nullable localProfileAvatarImage = nil; - if (localProfileAvatarMetadata) { - localProfileAvatarImage = [self loadProfileAvatarWithFilename:localProfileAvatarMetadata.fileName]; - if (!localProfileAvatarImage) { - localProfileAvatarMetadata = nil; - } - } - - dispatch_async(dispatch_get_main_queue(), ^{ - self.localProfileName = localProfileName; - self.localProfileAvatarImage = localProfileAvatarImage; - self.localProfileAvatarMetadata = localProfileAvatarMetadata; - - [[NSNotificationCenter defaultCenter] postNotificationName:kNSNotificationName_LocalProfileDidChange - object:nil - userInfo:nil]; - }); - }); -} - #pragma mark - Profile Whitelist - (void)addUserToProfileWhitelist:(NSString *)recipientId @@ -531,43 +543,58 @@ static const NSInteger kProfileKeyLength = 16; - (nullable NSString *)profileNameForRecipientId:(NSString *)recipientId { + OWSAssert([NSThread isMainThread]); OWSAssert(recipientId.length > 0); - NSString *_Nullable profileName = self.otherUsersProfileNameCache[recipientId]; - if (profileName.length > 0) { - return profileName; - } + [self fetchProfileForRecipientId:recipientId]; - profileName = - [self.dbConnection objectForKey:recipientId inCollection:kOWSProfilesManager_OtherUsersProfileNamesCollection]; - if (profileName) { - OWSAssert(profileName.length == kProfileKeyLength); - self.otherUsersProfileNameCache[recipientId] = profileName; - } else { - [self fetchProfileForRecipientId:recipientId]; - } - return profileName; + UserProfile *userProfile = [self getOrCreateUserProfileForRecipientId:recipientId]; + return userProfile.profileName; } - (nullable UIImage *)profileAvatarForRecipientId:(NSString *)recipientId { - // TODO: - return nil; + OWSAssert([NSThread isMainThread]); + OWSAssert(recipientId.length > 0); + + [self fetchProfileForRecipientId:recipientId]; + + UIImage *_Nullable image = [self.otherUsersProfileAvatarImageCache objectForKey:recipientId]; + if (image) { + return image; + } + + UserProfile *userProfile = [self getOrCreateUserProfileForRecipientId:recipientId]; + if (userProfile.avatarFileName) { + image = [self loadProfileAvatarWithFilename:userProfile.avatarFileName]; + if (image) { + [self.otherUsersProfileAvatarImageCache setObject:image forKey:recipientId]; + } + } + + return image; } - (void)fetchProfileForRecipientId:(NSString *)recipientId { OWSAssert(recipientId.length > 0); - // TODO: -} + UserProfile *userProfile = [self getOrCreateUserProfileForRecipientId:recipientId]; -// NSString *const kOWSProfilesManager_OtherUsersProfileNamesCollection = -// @"kOWSProfilesManager_OtherUsersProfileNamesCollection"; NSString *const -// kOWSProfilesManager_OtherUsersProfileAvatarMetadataCollection = -// @"kOWSProfilesManager_OtherUsersProfileAvatarMetadataCollection"; kNSNotificationName_OtherUsersProfileDidChange -//@property (nonatomic, readonly) NSMutableDictionary *otherUsersProfileNameCache; -//@property (nonatomic, readonly) NSMutableDictionary *otherUsersProfileAvatarImageCache; + // 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.dbConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { + [userProfile saveWithTransaction:transaction]; + }]; + + // TODO: Actually update the profile. +} #pragma mark - Avatar Disk Cache