From 0b14f87575c3ad8a1d9e2765a2ca71b29334a3a5 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Wed, 16 Aug 2017 11:40:43 -0400 Subject: [PATCH 1/4] Improve comments about mapping consistency in conversation view. // FREEBIE --- .../ConversationView/MessagesViewController.m | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/Signal/src/ViewControllers/ConversationView/MessagesViewController.m b/Signal/src/ViewControllers/ConversationView/MessagesViewController.m index 7680d5966..2d4a97c2a 100644 --- a/Signal/src/ViewControllers/ConversationView/MessagesViewController.m +++ b/Signal/src/ViewControllers/ConversationView/MessagesViewController.m @@ -166,6 +166,16 @@ typedef enum : NSUInteger { @property (nonatomic) TSThread *thread; @property (nonatomic) TSMessageAdapter *lastDeliveredMessage; @property (nonatomic) YapDatabaseConnection *editingDatabaseConnection; + +// These two properties must be updated in lockstep. +// +// * The first step is to update uiDatabaseConnection using beginLongLivedReadTransaction. +// * The second step is to update messageMappings. +// * We can't do the first step without doing the second step soon afterward. +// * We can't do the second step without doing the first step first. +// * We can't use messageMappings in between the first and second steps; e.g. +// we can't do any layout, since that uses numberOfItemsInSection: and +// interactionAtIndexPath: which use the messageMappings. @property (nonatomic) YapDatabaseConnection *uiDatabaseConnection; @property (nonatomic) YapDatabaseViewMappings *messageMappings; From d476bc286d27f8926a2d727afb1e210a6e10a4ba Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Wed, 16 Aug 2017 15:32:56 -0400 Subject: [PATCH 2/4] =?UTF-8?q?*=20Add=20debug=20UI=20tools=20for=20cleari?= =?UTF-8?q?ng=20and=20logging=20the=20profile=20whitelist.=20*=20Auto-add?= =?UTF-8?q?=20new=20contact=20threads=20to=20profile=20whitelist=20when=20?= =?UTF-8?q?local=20user=20sends=20first=20message=20to=20that=20thread.=20?= =?UTF-8?q?*=20Ensure=20dynamic=20interactions=20have=20a=20non-negative?= =?UTF-8?q?=20timestamp=20even=20if=20the=20conversation=20was=20empty.=20?= =?UTF-8?q?*=20Only=20call=20updateMessageMappingRangeOptions=20=5Fafter?= =?UTF-8?q?=5F=20beginLongLivedReadTransaction=20and=20updating=20messageM?= =?UTF-8?q?appings.=20*=20Improve=20documentation=20around=20how=20to=20av?= =?UTF-8?q?oid=20corrupt=20mappings=20in=20conversation=20view.=20*=20Fix?= =?UTF-8?q?=20edge=20cases=20around=20large=20initial=20range=20sizes.=20*?= =?UTF-8?q?=20Always=20treat=20dynamic=20interactions=20as=20read.=20*=20R?= =?UTF-8?q?ebuild=20the=20=E2=80=9Cunseen=E2=80=9D=20database=20views=20to?= =?UTF-8?q?=20remove=20dynamic=20interactions=20(see=20above).?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit // FREEBIE --- Signal/src/AppDelegate.m | 2 +- Signal/src/Profiles/OWSProfileManager.h | 6 ++ Signal/src/Profiles/OWSProfileManager.m | 33 ++++++++++ .../ConversationView/MessagesViewController.m | 66 ++++++++++++------- .../src/ViewControllers/DebugUI/DebugUIMisc.m | 10 +++ .../SendExternalFileViewController.m | 1 + Signal/src/util/ThreadUtil.h | 9 +++ Signal/src/util/ThreadUtil.m | 37 +++++++++-- .../Messages/Interactions/TSErrorMessage.m | 8 +++ .../src/Messages/Interactions/TSInfoMessage.m | 8 +++ SignalServiceKit/src/Storage/TSDatabaseView.m | 4 +- .../src/Storage/TSStorageManager.m | 4 +- .../src/Storage/YapDatabaseConnection+OWS.h | 5 +- .../src/Storage/YapDatabaseConnection+OWS.m | 9 +++ 14 files changed, 165 insertions(+), 37 deletions(-) diff --git a/Signal/src/AppDelegate.m b/Signal/src/AppDelegate.m index aceb8d0c0..b97adf95b 100644 --- a/Signal/src/AppDelegate.m +++ b/Signal/src/AppDelegate.m @@ -176,7 +176,7 @@ static NSString *const kURLHostVerifyPrefix = @"verify"; // we need to bump this constant. // // We added a number of database views in v2.13.0. - NSString *kLastVersionWithDatabaseViewChange = @"2.13.0"; + NSString *kLastVersionWithDatabaseViewChange = @"2.16.0"; BOOL mayNeedUpgrade = ([TSAccountManager isRegistered] && lastLaunchedAppVersion && (!lastCompletedLaunchAppVersion || [VersionMigrations isVersion:lastCompletedLaunchAppVersion diff --git a/Signal/src/Profiles/OWSProfileManager.h b/Signal/src/Profiles/OWSProfileManager.h index 5d3166b88..c5ad52c07 100644 --- a/Signal/src/Profiles/OWSProfileManager.h +++ b/Signal/src/Profiles/OWSProfileManager.h @@ -43,6 +43,12 @@ extern const NSUInteger kOWSProfileManager_MaxAvatarDiameter; #pragma mark - Profile Whitelist +#ifdef DEBUG +// These methods are for debugging. +- (void)clearProfileWhitelist; +- (void)logProfileWhitelist; +#endif + - (void)addThreadToProfileWhitelist:(TSThread *)thread; - (BOOL)isThreadInProfileWhitelist:(TSThread *)thread; diff --git a/Signal/src/Profiles/OWSProfileManager.m b/Signal/src/Profiles/OWSProfileManager.m index c7156a265..45a35609c 100644 --- a/Signal/src/Profiles/OWSProfileManager.m +++ b/Signal/src/Profiles/OWSProfileManager.m @@ -595,6 +595,39 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640; #pragma mark - Profile Whitelist +#ifdef DEBUG +- (void)clearProfileWhitelist +{ + 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]); + } + }); +} + +- (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]); + DDLogError(@"kOWSProfileManager_GroupWhitelistCollection: %zd", + [self.dbConnection numberOfKeysInCollection:kOWSProfileManager_GroupWhitelistCollection]); + } + }); +} +#endif + - (void)addUserToProfileWhitelist:(NSString *)recipientId { OWSAssert(recipientId.length > 0); diff --git a/Signal/src/ViewControllers/ConversationView/MessagesViewController.m b/Signal/src/ViewControllers/ConversationView/MessagesViewController.m index 2d4a97c2a..1930f91f1 100644 --- a/Signal/src/ViewControllers/ConversationView/MessagesViewController.m +++ b/Signal/src/ViewControllers/ConversationView/MessagesViewController.m @@ -169,13 +169,16 @@ typedef enum : NSUInteger { // These two properties must be updated in lockstep. // -// * The first step is to update uiDatabaseConnection using beginLongLivedReadTransaction. -// * The second step is to update messageMappings. -// * We can't do the first step without doing the second step soon afterward. -// * We can't do the second step without doing the first step first. +// * The first (required) step is to update uiDatabaseConnection using beginLongLivedReadTransaction. +// * The second (required) step is to update messageMappings. +// * The third (optional) step is to update the messageMappings range using +// updateMessageMappingRangeOptions. +// * The steps must be done in strict order. +// * If we do any of the steps, we must do all of the required steps. // * We can't use messageMappings in between the first and second steps; e.g. // we can't do any layout, since that uses numberOfItemsInSection: and // interactionAtIndexPath: which use the messageMappings. +// * If we do the third step, we must call resetContentAndLayout afterward. @property (nonatomic) YapDatabaseConnection *uiDatabaseConnection; @property (nonatomic) YapDatabaseViewMappings *messageMappings; @@ -380,10 +383,10 @@ typedef enum : NSUInteger { [[YapDatabaseViewMappings alloc] initWithGroups:@[ thread.uniqueId ] view:TSMessageDatabaseViewExtensionName]; // We need to impose the range restrictions on the mappings immediately to avoid // doing a great deal of unnecessary work and causing a perf hotspot. - [self updateMessageMappingRangeOptions]; [self.uiDatabaseConnection readWithBlock:^(YapDatabaseReadTransaction *transaction) { [self.messageMappings updateWithTransaction:transaction]; }]; + [self updateMessageMappingRangeOptions]; [self updateShouldObserveDBModifications]; self.page = 0; @@ -1466,21 +1469,19 @@ typedef enum : NSUInteger { // We convert large text messages to attachments // which are presented as normal text messages. const NSUInteger kOversizeTextMessageSizeThreshold = 16 * 1024; + BOOL didAddToProfileWhitelist = [ThreadUtil addThreadToProfileWhitelistIfEmptyContactThread:self.thread]; + TSOutgoingMessage *message; if ([text lengthOfBytesUsingEncoding:NSUTF8StringEncoding] >= kOversizeTextMessageSizeThreshold) { SignalAttachment *attachment = [SignalAttachment attachmentWithData:[text dataUsingEncoding:NSUTF8StringEncoding] dataUTI:SignalAttachment.kOversizeTextAttachmentUTI filename:nil]; - [self updateLastVisibleTimestamp:[ThreadUtil sendMessageWithAttachment:attachment - inThread:self.thread - messageSender:self.messageSender] - .timestampForSorting]; + message = + [ThreadUtil sendMessageWithAttachment:attachment inThread:self.thread messageSender:self.messageSender]; } else { - [self updateLastVisibleTimestamp:[ThreadUtil sendMessageWithText:text - inThread:self.thread - messageSender:self.messageSender] - .timestampForSorting]; + message = [ThreadUtil sendMessageWithText:text inThread:self.thread messageSender:self.messageSender]; } + [self updateLastVisibleTimestamp:message.timestampForSorting]; self.lastMessageSentDate = [NSDate new]; [self clearUnreadMessagesIndicator]; @@ -1491,6 +1492,9 @@ typedef enum : NSUInteger { [self clearDraft]; [self finishSendingMessage]; [((OWSMessagesToolbarContentView *)self.inputToolbar.contentView)ensureSubviews]; + if (didAddToProfileWhitelist) { + [self ensureDynamicInteractions]; + } } } @@ -2295,17 +2299,23 @@ typedef enum : NSUInteger { - (void)updateMessageMappingRangeOptions { - // The "page" length may have been increased by loading "prev" pages at the - // top of the window. - NSUInteger pageLength = kYapDatabasePageSize * (self.page + 1); - // The "old" length may have been increased by insertions of new messages + // The "old" range length may have been increased by insertions of new messages // at the bottom of the window. NSUInteger oldLength = [self.messageMappings numberOfItemsInGroup:self.thread.uniqueId]; - NSUInteger newLength = MAX(pageLength, oldLength); + // The "page-based" range length may have been increased by loading "prev" pages at the + // top of the window. + NSUInteger rangeLength; + while (YES) { + rangeLength = kYapDatabasePageSize * (self.page + 1); + if (rangeLength >= oldLength) { + break; + } + self.page = self.page + 1; + } YapDatabaseViewRangeOptions *rangeOptions = - [YapDatabaseViewRangeOptions flexibleRangeWithLength:newLength offset:0 from:YapDatabaseViewEnd]; + [YapDatabaseViewRangeOptions flexibleRangeWithLength:rangeLength offset:0 from:YapDatabaseViewEnd]; - rangeOptions.maxLength = MAX(newLength, kYapDatabaseRangeMaxLength); + rangeOptions.maxLength = MAX(rangeLength, kYapDatabaseRangeMaxLength); rangeOptions.minLength = kYapDatabaseRangeMinLength; [self.messageMappings setRangeOptions:rangeOptions forGroup:self.thread.uniqueId]; @@ -3176,12 +3186,18 @@ typedef enum : NSUInteger { DDLogVerbose(@"Sending attachment. Size in bytes: %lu, contentType: %@", (unsigned long)attachment.data.length, [attachment mimeType]); - [self updateLastVisibleTimestamp:[ThreadUtil sendMessageWithAttachment:attachment - inThread:self.thread - messageSender:self.messageSender] - .timestampForSorting]; + BOOL didAddToProfileWhitelist = [ThreadUtil addThreadToProfileWhitelistIfEmptyContactThread:self.thread]; + TSOutgoingMessage *message = + [ThreadUtil sendMessageWithAttachment:attachment inThread:self.thread messageSender:self.messageSender]; + [self.editingDatabaseConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { + [message saveWithTransaction:transaction]; + }]; + [self updateLastVisibleTimestamp:message.timestampForSorting]; self.lastMessageSentDate = [NSDate new]; [self clearUnreadMessagesIndicator]; + if (didAddToProfileWhitelist) { + [self ensureDynamicInteractions]; + } } - (NSURL *)videoTempFolder @@ -4244,10 +4260,10 @@ typedef enum : NSUInteger { // We need to `beginLongLivedReadTransaction` before we update our // mapping in order to jump to the most recent commit. [self.uiDatabaseConnection beginLongLivedReadTransaction]; - [self updateMessageMappingRangeOptions]; [self.uiDatabaseConnection readWithBlock:^(YapDatabaseReadTransaction *transaction) { [self.messageMappings updateWithTransaction:transaction]; }]; + [self updateMessageMappingRangeOptions]; } self.messageAdapterCache = [[NSCache alloc] init]; diff --git a/Signal/src/ViewControllers/DebugUI/DebugUIMisc.m b/Signal/src/ViewControllers/DebugUI/DebugUIMisc.m index cdea5c066..e69b2a816 100644 --- a/Signal/src/ViewControllers/DebugUI/DebugUIMisc.m +++ b/Signal/src/ViewControllers/DebugUI/DebugUIMisc.m @@ -53,6 +53,16 @@ NS_ASSUME_NONNULL_BEGIN actionBlock:^{ [DebugUIMisc setManualCensorshipCircumventionEnabled:NO]; }]]; +#ifdef DEBUG + [items addObject:[OWSTableItem itemWithTitle:@"Clear Profile Whitelist" + actionBlock:^{ + [OWSProfileManager.sharedManager clearProfileWhitelist]; + }]]; + [items addObject:[OWSTableItem itemWithTitle:@"Log Profile Whitelist" + actionBlock:^{ + [OWSProfileManager.sharedManager logProfileWhitelist]; + }]]; +#endif return [OWSTableSection sectionWithTitle:self.name items:items]; } diff --git a/Signal/src/ViewControllers/SendExternalFileViewController.m b/Signal/src/ViewControllers/SendExternalFileViewController.m index 553aa76fd..393adb455 100644 --- a/Signal/src/ViewControllers/SendExternalFileViewController.m +++ b/Signal/src/ViewControllers/SendExternalFileViewController.m @@ -64,6 +64,7 @@ NS_ASSUME_NONNULL_BEGIN return; } + [ThreadUtil addThreadToProfileWhitelistIfEmptyContactThread:thread]; [ThreadUtil sendMessageWithAttachment:self.attachment inThread:thread messageSender:self.messageSender]; [Environment messageThreadId:thread.uniqueId]; diff --git a/Signal/src/util/ThreadUtil.h b/Signal/src/util/ThreadUtil.h index dafdad510..6987fe81d 100644 --- a/Signal/src/util/ThreadUtil.h +++ b/Signal/src/util/ThreadUtil.h @@ -38,6 +38,8 @@ NS_ASSUME_NONNULL_BEGIN @property (nonatomic, readonly) BOOL hasMoreUnseenMessages; +@property (nonatomic, readonly) BOOL didInsertDynamicInteractions; + - (void)clearUnreadIndicatorState; @end @@ -94,6 +96,13 @@ NS_ASSUME_NONNULL_BEGIN firstUnseenInteractionTimestamp:(nullable NSNumber *)firstUnseenInteractionTimestamp maxRangeSize:(int)maxRangeSize; +// This method should be called right _before_ we send a message to a thread, +// since we want to auto-add contact threads to the profile whitelist if the +// conversation was initiated by the local user. +// +// Returns YES IFF the thread was just added to the profile whitelist. ++ (BOOL)addThreadToProfileWhitelistIfEmptyContactThread:(TSThread *)thread; + @end NS_ASSUME_NONNULL_END diff --git a/Signal/src/util/ThreadUtil.m b/Signal/src/util/ThreadUtil.m index 37ca8e2f6..a19a1b7fc 100644 --- a/Signal/src/util/ThreadUtil.m +++ b/Signal/src/util/ThreadUtil.m @@ -4,6 +4,7 @@ #import "ThreadUtil.h" #import "OWSContactsManager.h" +#import "OWSProfileManager.h" #import "Signal-Swift.h" #import "TSUnreadIndicatorInteraction.h" #import @@ -27,6 +28,8 @@ NS_ASSUME_NONNULL_BEGIN @property (nonatomic) BOOL hasMoreUnseenMessages; +@property (nonatomic) BOOL didInsertDynamicInteractions; + @end #pragma mark - @@ -397,6 +400,10 @@ NS_ASSUME_NONNULL_BEGIN const int kAddToContactsOfferOffset = -2; const int kUnreadIndicatorOfferOffset = -1; + // Ensure dynamic interactions have a non-negative timestamp even if the conversation was empty. + long long startOfConversationTimestamp + = (long long)(firstMessage ? firstMessage.timestampForSorting : 1000); + if (existingBlockOffer && !shouldHaveBlockOffer) { DDLogInfo(@"%@ Removing block offer: %@ (%llu)", self.tag, @@ -409,7 +416,7 @@ NS_ASSUME_NONNULL_BEGIN // We want the block offer to be the first interaction in their // conversation's timeline, so we back-date it to slightly before // the first incoming message (which we know is the first message). - uint64_t blockOfferTimestamp = (uint64_t)((long long)firstMessage.timestampForSorting + kBlockOfferOffset); + uint64_t blockOfferTimestamp = (uint64_t)(startOfConversationTimestamp + kBlockOfferOffset); NSString *recipientId = ((TSContactThread *)thread).contactIdentifier; TSMessage *offerMessage = @@ -417,6 +424,7 @@ NS_ASSUME_NONNULL_BEGIN thread:thread contactId:recipientId]; [offerMessage saveWithTransaction:transaction]; + result.didInsertDynamicInteractions = YES; DDLogInfo(@"%@ Creating block offer: %@ (%llu)", self.tag, @@ -437,14 +445,14 @@ NS_ASSUME_NONNULL_BEGIN // We want the offer to be the first interaction in their // conversation's timeline, so we back-date it to slightly before // the first incoming message (which we know is the first message). - uint64_t offerTimestamp - = (uint64_t)((long long)firstMessage.timestampForSorting + kAddToContactsOfferOffset); + uint64_t offerTimestamp = (uint64_t)(startOfConversationTimestamp + kAddToContactsOfferOffset); NSString *recipientId = ((TSContactThread *)thread).contactIdentifier; TSMessage *offerMessage = [OWSAddToContactsOfferMessage addToContactsOfferMessage:offerTimestamp thread:thread contactId:recipientId]; [offerMessage saveWithTransaction:transaction]; + result.didInsertDynamicInteractions = YES; DDLogInfo(@"%@ Creating 'add to contacts' offer: %@ (%llu)", self.tag, @@ -465,12 +473,12 @@ NS_ASSUME_NONNULL_BEGIN // We want the offer to be the first interaction in their // conversation's timeline, so we back-date it to slightly before // the first incoming message (which we know is the first message). - uint64_t offerTimestamp - = (uint64_t)((long long)firstMessage.timestampForSorting + kAddToProfileWhitelistOfferOffset); + uint64_t offerTimestamp = (uint64_t)(startOfConversationTimestamp + kAddToProfileWhitelistOfferOffset); TSMessage *offerMessage = [OWSAddToProfileWhitelistOfferMessage addToProfileWhitelistOfferMessage:offerTimestamp thread:thread]; [offerMessage saveWithTransaction:transaction]; + result.didInsertDynamicInteractions = YES; DDLogInfo(@"%@ Creating 'add to profile whitelist' offer: %@ (%llu)", self.tag, @@ -511,6 +519,7 @@ NS_ASSUME_NONNULL_BEGIN hasMoreUnseenMessages:result.hasMoreUnseenMessages missingUnseenSafetyNumberChangeCount:missingUnseenSafetyNumberChangeCount]; [indicator saveWithTransaction:transaction]; + result.didInsertDynamicInteractions = YES; DDLogInfo(@"%@ Creating TSUnreadIndicatorInteraction: %@ (%llu)", self.tag, @@ -523,6 +532,24 @@ NS_ASSUME_NONNULL_BEGIN return result; } ++ (BOOL)addThreadToProfileWhitelistIfEmptyContactThread:(TSThread *)thread +{ + OWSAssert(thread); + + if (thread.isGroupThread) { + return NO; + } + if ([OWSProfileManager.sharedManager isThreadInProfileWhitelist:thread]) { + return NO; + } + if (!thread.hasEverHadMessage) { + [OWSProfileManager.sharedManager addThreadToProfileWhitelist:thread]; + return YES; + } else { + return NO; + } +} + #pragma mark - Logging + (NSString *)tag diff --git a/SignalServiceKit/src/Messages/Interactions/TSErrorMessage.m b/SignalServiceKit/src/Messages/Interactions/TSErrorMessage.m index a50327e40..825ba1aa5 100644 --- a/SignalServiceKit/src/Messages/Interactions/TSErrorMessage.m +++ b/SignalServiceKit/src/Messages/Interactions/TSErrorMessage.m @@ -40,6 +40,10 @@ NSUInteger TSErrorMessageSchemaVersion = 1; _errorMessageSchemaVersion = TSErrorMessageSchemaVersion; + if (self.isDynamicInteraction) { + self.read = YES; + } + return self; } @@ -70,6 +74,10 @@ NSUInteger TSErrorMessageSchemaVersion = 1; _recipientId = recipientId; _errorMessageSchemaVersion = TSErrorMessageSchemaVersion; + if (self.isDynamicInteraction) { + self.read = YES; + } + return self; } diff --git a/SignalServiceKit/src/Messages/Interactions/TSInfoMessage.m b/SignalServiceKit/src/Messages/Interactions/TSInfoMessage.m index d43ef15fd..48d43b625 100644 --- a/SignalServiceKit/src/Messages/Interactions/TSInfoMessage.m +++ b/SignalServiceKit/src/Messages/Interactions/TSInfoMessage.m @@ -35,6 +35,10 @@ NSUInteger TSInfoMessageSchemaVersion = 1; _infoMessageSchemaVersion = TSInfoMessageSchemaVersion; + if (self.isDynamicInteraction) { + self.read = YES; + } + return self; } @@ -56,6 +60,10 @@ NSUInteger TSInfoMessageSchemaVersion = 1; _messageType = infoMessage; _infoMessageSchemaVersion = TSInfoMessageSchemaVersion; + if (self.isDynamicInteraction) { + self.read = YES; + } + return self; } diff --git a/SignalServiceKit/src/Storage/TSDatabaseView.m b/SignalServiceKit/src/Storage/TSDatabaseView.m index 35ed412d5..8334be2d5 100644 --- a/SignalServiceKit/src/Storage/TSDatabaseView.m +++ b/SignalServiceKit/src/Storage/TSDatabaseView.m @@ -125,7 +125,7 @@ NSString *const TSSecondaryDevicesDatabaseViewExtensionName = @"TSSecondaryDevic [self registerMessageDatabaseViewWithName:TSUnreadDatabaseViewExtensionName viewGrouping:viewGrouping - version:@"1" + version:@"2" async:NO]; } @@ -144,7 +144,7 @@ NSString *const TSSecondaryDevicesDatabaseViewExtensionName = @"TSSecondaryDevic [self registerMessageDatabaseViewWithName:TSUnseenDatabaseViewExtensionName viewGrouping:viewGrouping - version:@"1" + version:@"2" async:YES]; } diff --git a/SignalServiceKit/src/Storage/TSStorageManager.m b/SignalServiceKit/src/Storage/TSStorageManager.m index f030b36b9..5b3534805 100644 --- a/SignalServiceKit/src/Storage/TSStorageManager.m +++ b/SignalServiceKit/src/Storage/TSStorageManager.m @@ -391,9 +391,7 @@ static NSString *keychainDBPassAccount = @"TSDatabasePass"; #pragma mark - convenience methods - (void)purgeCollection:(NSString *)collection { - [self.dbReadWriteConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { - [transaction removeAllObjectsInCollection:collection]; - }]; + [self.dbReadWriteConnection purgeCollection:collection]; } - (void)setObject:(id)object forKey:(NSString *)key inCollection:(NSString *)collection { diff --git a/SignalServiceKit/src/Storage/YapDatabaseConnection+OWS.h b/SignalServiceKit/src/Storage/YapDatabaseConnection+OWS.h index d54ccb19f..d6aac59cc 100644 --- a/SignalServiceKit/src/Storage/YapDatabaseConnection+OWS.h +++ b/SignalServiceKit/src/Storage/YapDatabaseConnection+OWS.h @@ -24,6 +24,8 @@ NS_ASSUME_NONNULL_BEGIN - (nullable PreKeyRecord *)preKeyRecordForKey:(NSString *)key inCollection:(NSString *)collection; - (nullable SignedPreKeyRecord *)signedPreKeyRecordForKey:(NSString *)key inCollection:(NSString *)collection; +- (NSUInteger)numberOfKeysInCollection:(NSString *)collection; + #pragma mark - - (void)setObject:(id)object forKey:(NSString *)key inCollection:(NSString *)collection; @@ -31,9 +33,10 @@ NS_ASSUME_NONNULL_BEGIN - (void)removeObjectForKey:(NSString *)string inCollection:(NSString *)collection; - (void)setInt:(int)integer forKey:(NSString *)key inCollection:(NSString *)collection; - (void)setDate:(NSDate *)value forKey:(NSString *)key inCollection:(NSString *)collection; -- (void)purgeCollection:(NSString *)collection; - (int)incrementIntForKey:(NSString *)key inCollection:(NSString *)collection; +- (void)purgeCollection:(NSString *)collection; + @end NS_ASSUME_NONNULL_END diff --git a/SignalServiceKit/src/Storage/YapDatabaseConnection+OWS.m b/SignalServiceKit/src/Storage/YapDatabaseConnection+OWS.m index d01d846ee..0a885dc82 100644 --- a/SignalServiceKit/src/Storage/YapDatabaseConnection+OWS.m +++ b/SignalServiceKit/src/Storage/YapDatabaseConnection+OWS.m @@ -94,6 +94,15 @@ NS_ASSUME_NONNULL_BEGIN #pragma mark - +- (NSUInteger)numberOfKeysInCollection:(NSString *)collection +{ + __block NSUInteger result; + [self readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { + result = [transaction numberOfKeysInCollection:collection]; + }]; + return result; +} + - (void)purgeCollection:(NSString *)collection { [self readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { From 622c0c3f5f47628cfe1597f84a5045ceb2f7d9b6 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Wed, 16 Aug 2017 15:33:46 -0400 Subject: [PATCH 3/4] =?UTF-8?q?*=20Add=20debug=20UI=20tools=20for=20cleari?= =?UTF-8?q?ng=20and=20logging=20the=20profile=20whitelist.=20*=20Auto-add?= =?UTF-8?q?=20new=20contact=20threads=20to=20profile=20whitelist=20when=20?= =?UTF-8?q?local=20user=20sends=20first=20message=20to=20that=20thread.=20?= =?UTF-8?q?*=20Ensure=20dynamic=20interactions=20have=20a=20non-negative?= =?UTF-8?q?=20timestamp=20even=20if=20the=20conversation=20was=20empty.=20?= =?UTF-8?q?*=20Only=20call=20updateMessageMappingRangeOptions=20=5Fafter?= =?UTF-8?q?=5F=20beginLongLivedReadTransaction=20and=20updating=20messageM?= =?UTF-8?q?appings.=20*=20Improve=20documentation=20around=20how=20to=20av?= =?UTF-8?q?oid=20corrupt=20mappings=20in=20conversation=20view.=20*=20Fix?= =?UTF-8?q?=20edge=20cases=20around=20large=20initial=20range=20sizes.=20*?= =?UTF-8?q?=20Always=20treat=20dynamic=20interactions=20as=20read.=20*=20R?= =?UTF-8?q?ebuild=20the=20=E2=80=9Cunseen=E2=80=9D=20database=20views=20to?= =?UTF-8?q?=20remove=20dynamic=20interactions=20(see=20above).?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit // FREEBIE --- SignalServiceKit/src/Messages/TSMessagesManager.m | 6 +++--- SignalServiceKit/src/Storage/TSDatabaseView.m | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/SignalServiceKit/src/Messages/TSMessagesManager.m b/SignalServiceKit/src/Messages/TSMessagesManager.m index 272940895..d0a001ac7 100644 --- a/SignalServiceKit/src/Messages/TSMessagesManager.m +++ b/SignalServiceKit/src/Messages/TSMessagesManager.m @@ -1141,9 +1141,9 @@ NS_ASSUME_NONNULL_BEGIN - (NSUInteger)unreadMessagesCountExcept:(TSThread *)thread { __block NSUInteger numberOfItems; [self.dbConnection readWithBlock:^(YapDatabaseReadTransaction *transaction) { - numberOfItems = [[transaction ext:TSUnreadDatabaseViewExtensionName] numberOfItemsInAllGroups]; - numberOfItems = - numberOfItems - [[transaction ext:TSUnreadDatabaseViewExtensionName] numberOfItemsInGroup:thread.uniqueId]; + id databaseView = [transaction ext:TSUnreadDatabaseViewExtensionName]; + OWSAssert(databaseView); + numberOfItems = ([databaseView numberOfItemsInAllGroups] - [databaseView numberOfItemsInGroup:thread.uniqueId]); }]; return numberOfItems; diff --git a/SignalServiceKit/src/Storage/TSDatabaseView.m b/SignalServiceKit/src/Storage/TSDatabaseView.m index 8334be2d5..5add51cf4 100644 --- a/SignalServiceKit/src/Storage/TSDatabaseView.m +++ b/SignalServiceKit/src/Storage/TSDatabaseView.m @@ -125,7 +125,7 @@ NSString *const TSSecondaryDevicesDatabaseViewExtensionName = @"TSSecondaryDevic [self registerMessageDatabaseViewWithName:TSUnreadDatabaseViewExtensionName viewGrouping:viewGrouping - version:@"2" + version:@"1" async:NO]; } From 164bf19b47e70ae0394f82be66b326b1da58d85a Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Thu, 17 Aug 2017 11:21:40 -0400 Subject: [PATCH 4/4] Respond to CR. // FREEBIE --- Signal/src/Profiles/OWSProfileManager.m | 14 +++++++++++++ Signal/src/util/ThreadUtil.h | 2 -- Signal/src/util/ThreadUtil.m | 20 ++++--------------- .../src/Storage/YapDatabaseConnection+OWS.m | 2 +- 4 files changed, 19 insertions(+), 19 deletions(-) diff --git a/Signal/src/Profiles/OWSProfileManager.m b/Signal/src/Profiles/OWSProfileManager.m index 45a35609c..2517d11f1 100644 --- a/Signal/src/Profiles/OWSProfileManager.m +++ b/Signal/src/Profiles/OWSProfileManager.m @@ -598,6 +598,8 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640; #ifdef DEBUG - (void)clearProfileWhitelist { + DDLogWarn(@"%@ Clearing the profile whitelist.", self.tag); + dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{ @synchronized(self) { @@ -621,8 +623,20 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640; 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); + }]; + }]; } }); } diff --git a/Signal/src/util/ThreadUtil.h b/Signal/src/util/ThreadUtil.h index 6987fe81d..6b38fc105 100644 --- a/Signal/src/util/ThreadUtil.h +++ b/Signal/src/util/ThreadUtil.h @@ -38,8 +38,6 @@ NS_ASSUME_NONNULL_BEGIN @property (nonatomic, readonly) BOOL hasMoreUnseenMessages; -@property (nonatomic, readonly) BOOL didInsertDynamicInteractions; - - (void)clearUnreadIndicatorState; @end diff --git a/Signal/src/util/ThreadUtil.m b/Signal/src/util/ThreadUtil.m index a19a1b7fc..5cbf2c319 100644 --- a/Signal/src/util/ThreadUtil.m +++ b/Signal/src/util/ThreadUtil.m @@ -28,8 +28,6 @@ NS_ASSUME_NONNULL_BEGIN @property (nonatomic) BOOL hasMoreUnseenMessages; -@property (nonatomic) BOOL didInsertDynamicInteractions; - @end #pragma mark - @@ -400,7 +398,10 @@ NS_ASSUME_NONNULL_BEGIN const int kAddToContactsOfferOffset = -2; const int kUnreadIndicatorOfferOffset = -1; - // Ensure dynamic interactions have a non-negative timestamp even if the conversation was empty. + // We want the offers to be the first interactions in their + // conversation's timeline, so we back-date them to slightly before + // the first message - or at an aribtrary old timestamp if the + // conversation has no messages. long long startOfConversationTimestamp = (long long)(firstMessage ? firstMessage.timestampForSorting : 1000); @@ -413,9 +414,6 @@ NS_ASSUME_NONNULL_BEGIN } else if (!existingBlockOffer && shouldHaveBlockOffer) { DDLogInfo(@"Creating block offer for unknown contact"); - // We want the block offer to be the first interaction in their - // conversation's timeline, so we back-date it to slightly before - // the first incoming message (which we know is the first message). uint64_t blockOfferTimestamp = (uint64_t)(startOfConversationTimestamp + kBlockOfferOffset); NSString *recipientId = ((TSContactThread *)thread).contactIdentifier; @@ -424,7 +422,6 @@ NS_ASSUME_NONNULL_BEGIN thread:thread contactId:recipientId]; [offerMessage saveWithTransaction:transaction]; - result.didInsertDynamicInteractions = YES; DDLogInfo(@"%@ Creating block offer: %@ (%llu)", self.tag, @@ -442,9 +439,6 @@ NS_ASSUME_NONNULL_BEGIN DDLogInfo(@"%@ Creating 'add to contacts' offer for unknown contact", self.tag); - // We want the offer to be the first interaction in their - // conversation's timeline, so we back-date it to slightly before - // the first incoming message (which we know is the first message). uint64_t offerTimestamp = (uint64_t)(startOfConversationTimestamp + kAddToContactsOfferOffset); NSString *recipientId = ((TSContactThread *)thread).contactIdentifier; @@ -452,7 +446,6 @@ NS_ASSUME_NONNULL_BEGIN thread:thread contactId:recipientId]; [offerMessage saveWithTransaction:transaction]; - result.didInsertDynamicInteractions = YES; DDLogInfo(@"%@ Creating 'add to contacts' offer: %@ (%llu)", self.tag, @@ -470,15 +463,11 @@ NS_ASSUME_NONNULL_BEGIN DDLogInfo(@"%@ Creating 'add to profile whitelist' offer", self.tag); - // We want the offer to be the first interaction in their - // conversation's timeline, so we back-date it to slightly before - // the first incoming message (which we know is the first message). uint64_t offerTimestamp = (uint64_t)(startOfConversationTimestamp + kAddToProfileWhitelistOfferOffset); TSMessage *offerMessage = [OWSAddToProfileWhitelistOfferMessage addToProfileWhitelistOfferMessage:offerTimestamp thread:thread]; [offerMessage saveWithTransaction:transaction]; - result.didInsertDynamicInteractions = YES; DDLogInfo(@"%@ Creating 'add to profile whitelist' offer: %@ (%llu)", self.tag, @@ -519,7 +508,6 @@ NS_ASSUME_NONNULL_BEGIN hasMoreUnseenMessages:result.hasMoreUnseenMessages missingUnseenSafetyNumberChangeCount:missingUnseenSafetyNumberChangeCount]; [indicator saveWithTransaction:transaction]; - result.didInsertDynamicInteractions = YES; DDLogInfo(@"%@ Creating TSUnreadIndicatorInteraction: %@ (%llu)", self.tag, diff --git a/SignalServiceKit/src/Storage/YapDatabaseConnection+OWS.m b/SignalServiceKit/src/Storage/YapDatabaseConnection+OWS.m index 0a885dc82..957d0d4c8 100644 --- a/SignalServiceKit/src/Storage/YapDatabaseConnection+OWS.m +++ b/SignalServiceKit/src/Storage/YapDatabaseConnection+OWS.m @@ -97,7 +97,7 @@ NS_ASSUME_NONNULL_BEGIN - (NSUInteger)numberOfKeysInCollection:(NSString *)collection { __block NSUInteger result; - [self readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { + [self readWithBlock:^(YapDatabaseReadTransaction *transaction) { result = [transaction numberOfKeysInCollection:collection]; }]; return result;