From 6f8eddc9558262719a68b1012a2bbac0616ff4f3 Mon Sep 17 00:00:00 2001 From: Michael Kirk Date: Mon, 24 Sep 2018 15:22:38 -0600 Subject: [PATCH] unread indicator uses sortId - removed timestamp parameter. This wasn't totally obvious, previously we were tracking two pieces of state 1. `unreadIndicator.firstUnseenTimestamp`: the first unseen timestamp for a conversation that exists in the database 2. `unreadIndicator.timestamp`: the timestamp of the first interaction *after* the unread indicator that fits in the loading window We don't actually need to track `2` because it was only used in a comparison like: viewItem.interaction.timestampForSorting >= unreadIndicator.timestamp But by definition, unreadIndicator.firstUnseenTimestamp is always less than or equal to unreadIndicator.timestamp. Put into terms of the `sortId` corallary, the sortId of the first unseen interaction in the database is always less than or equal to the sortId of the first unseen interaction that fits in the loading window. In other words, there's no situation where viewItem.interaction.sortId >= unreadIndicator.firstUnseenSortId --- .../ConversationViewController.m | 15 ++-- SignalMessaging/utils/OWSUnreadIndicator.h | 16 ++--- SignalMessaging/utils/OWSUnreadIndicator.m | 15 ++-- SignalMessaging/utils/ThreadUtil.m | 68 ++++++++----------- 4 files changed, 48 insertions(+), 66 deletions(-) diff --git a/Signal/src/ViewControllers/ConversationView/ConversationViewController.m b/Signal/src/ViewControllers/ConversationView/ConversationViewController.m index 16f2a647a..ee32cde6b 100644 --- a/Signal/src/ViewControllers/ConversationView/ConversationViewController.m +++ b/Signal/src/ViewControllers/ConversationView/ConversationViewController.m @@ -4958,20 +4958,15 @@ typedef enum : NSUInteger { && !((id)viewItem.interaction).wasRead); if (isItemUnread && !unreadIndicator && !hasPlacedUnreadIndicator && !self.hasClearedUnreadMessagesIndicator) { - unreadIndicator = - [[OWSUnreadIndicator alloc] initUnreadIndicatorWithTimestamp:viewItem.interaction.timestamp - hasMoreUnseenMessages:NO - missingUnseenSafetyNumberChangeCount:0 - unreadIndicatorPosition:0 - firstUnseenInteractionTimestamp:viewItem.interaction.timestamp]; + unreadIndicator = [[OWSUnreadIndicator alloc] initWithFirstUnseenSortId:viewItem.interaction.sortId + hasMoreUnseenMessages:NO + missingUnseenSafetyNumberChangeCount:0 + unreadIndicatorPosition:0]; } // Place the unread indicator onto the first appropriate view item, // if any. - // MJK FIXME - use sortId - // I'm not sure why we need a comparison here, vs. an equality. Maybe if the first unread - // message is deleted, we'd need to be sure everything still works. - if (unreadIndicator && viewItem.interaction.timestampForLegacySorting >= unreadIndicator.timestamp) { + if (unreadIndicator && viewItem.interaction.sortId >= unreadIndicator.firstUnseenSortId) { viewItem.unreadIndicator = unreadIndicator; unreadIndicator = nil; hasPlacedUnreadIndicator = YES; diff --git a/SignalMessaging/utils/OWSUnreadIndicator.h b/SignalMessaging/utils/OWSUnreadIndicator.h index 142d9c28c..ee6f7b37e 100644 --- a/SignalMessaging/utils/OWSUnreadIndicator.h +++ b/SignalMessaging/utils/OWSUnreadIndicator.h @@ -6,14 +6,11 @@ NS_ASSUME_NONNULL_BEGIN @interface OWSUnreadIndicator : NSObject -// MJK FIXME - do we need this timestamp column? -@property (nonatomic, readonly) uint64_t timestamp; - @property (nonatomic, readonly) BOOL hasMoreUnseenMessages; @property (nonatomic, readonly) NSUInteger missingUnseenSafetyNumberChangeCount; -// The timestamp of the oldest unseen message. +// The sortId of the oldest unseen message. // // Once we enter messages view, we mark all messages read, so we need // a snapshot of what the first unread message was when we entered the @@ -21,7 +18,7 @@ NS_ASSUME_NONNULL_BEGIN // repeatedly. The unread indicator should continue to show up until // it has been cleared, at which point hideUnreadMessagesIndicator is // YES in ensureDynamicInteractionsForThread:... -@property (nonatomic, readonly) uint64_t firstUnseenInteractionTimestamp; +@property (nonatomic, readonly) uint64_t firstUnseenSortId; // The index of the unseen indicator, counting from the _end_ of the conversation // history. @@ -33,11 +30,10 @@ NS_ASSUME_NONNULL_BEGIN - (instancetype)init NS_UNAVAILABLE; -- (instancetype)initUnreadIndicatorWithTimestamp:(uint64_t)timestamp - hasMoreUnseenMessages:(BOOL)hasMoreUnseenMessages - missingUnseenSafetyNumberChangeCount:(NSUInteger)missingUnseenSafetyNumberChangeCount - unreadIndicatorPosition:(NSInteger)unreadIndicatorPosition - firstUnseenInteractionTimestamp:(uint64_t)firstUnseenInteractionTimestamp NS_DESIGNATED_INITIALIZER; +- (instancetype)initWithFirstUnseenSortId:(uint64_t)firstUnseenSortId + hasMoreUnseenMessages:(BOOL)hasMoreUnseenMessages + missingUnseenSafetyNumberChangeCount:(NSUInteger)missingUnseenSafetyNumberChangeCount + unreadIndicatorPosition:(NSInteger)unreadIndicatorPosition NS_DESIGNATED_INITIALIZER; @end diff --git a/SignalMessaging/utils/OWSUnreadIndicator.m b/SignalMessaging/utils/OWSUnreadIndicator.m index a62190245..3cc384df3 100644 --- a/SignalMessaging/utils/OWSUnreadIndicator.m +++ b/SignalMessaging/utils/OWSUnreadIndicator.m @@ -8,11 +8,10 @@ NS_ASSUME_NONNULL_BEGIN @implementation OWSUnreadIndicator -- (instancetype)initUnreadIndicatorWithTimestamp:(uint64_t)timestamp - hasMoreUnseenMessages:(BOOL)hasMoreUnseenMessages - missingUnseenSafetyNumberChangeCount:(NSUInteger)missingUnseenSafetyNumberChangeCount - unreadIndicatorPosition:(NSInteger)unreadIndicatorPosition - firstUnseenInteractionTimestamp:(uint64_t)firstUnseenInteractionTimestamp +- (instancetype)initWithFirstUnseenSortId:(uint64_t)firstUnseenSortId + hasMoreUnseenMessages:(BOOL)hasMoreUnseenMessages + missingUnseenSafetyNumberChangeCount:(NSUInteger)missingUnseenSafetyNumberChangeCount + unreadIndicatorPosition:(NSInteger)unreadIndicatorPosition { self = [super init]; @@ -20,11 +19,10 @@ NS_ASSUME_NONNULL_BEGIN return self; } - _timestamp = timestamp; + _firstUnseenSortId = firstUnseenSortId; _hasMoreUnseenMessages = hasMoreUnseenMessages; _missingUnseenSafetyNumberChangeCount = missingUnseenSafetyNumberChangeCount; _unreadIndicatorPosition = unreadIndicatorPosition; - _firstUnseenInteractionTimestamp = firstUnseenInteractionTimestamp; return self; } @@ -40,7 +38,8 @@ NS_ASSUME_NONNULL_BEGIN } OWSUnreadIndicator *other = object; - return (self.timestamp == other.timestamp && self.hasMoreUnseenMessages == other.hasMoreUnseenMessages + return (self.firstUnseenSortId == other.firstUnseenSortId + && self.hasMoreUnseenMessages == other.hasMoreUnseenMessages && self.missingUnseenSafetyNumberChangeCount == other.missingUnseenSafetyNumberChangeCount && self.unreadIndicatorPosition == other.unreadIndicatorPosition); } diff --git a/SignalMessaging/utils/ThreadUtil.m b/SignalMessaging/utils/ThreadUtil.m index 89b6a7cc9..74d0117c8 100644 --- a/SignalMessaging/utils/ThreadUtil.m +++ b/SignalMessaging/utils/ThreadUtil.m @@ -363,15 +363,14 @@ NS_ASSUME_NONNULL_BEGIN // have been marked as read. // // IFF this variable is non-null, there are unseen messages in the thread. - NSNumber *_Nullable firstUnseenInteractionTimestamp = nil; + NSNumber *_Nullable firstUnseenSortId = nil; if (lastUnreadIndicator) { - firstUnseenInteractionTimestamp = @(lastUnreadIndicator.firstUnseenInteractionTimestamp); + firstUnseenSortId = @(lastUnreadIndicator.firstUnseenSortId); } else { TSInteraction *_Nullable firstUnseenInteraction = [[TSDatabaseView unseenDatabaseViewExtension:transaction] firstObjectInGroup:thread.uniqueId]; if (firstUnseenInteraction) { - // MJK FIXME - use sortId - firstUnseenInteractionTimestamp = @(firstUnseenInteraction.timestampForLegacySorting); + firstUnseenSortId = @(firstUnseenInteraction.sortId); } } @@ -525,14 +524,14 @@ NS_ASSUME_NONNULL_BEGIN } [self ensureUnreadIndicator:result - thread:thread - transaction:transaction - shouldHaveContactOffers:shouldHaveContactOffers - maxRangeSize:maxRangeSize - blockingSafetyNumberChanges:blockingSafetyNumberChanges - nonBlockingSafetyNumberChanges:nonBlockingSafetyNumberChanges - hideUnreadMessagesIndicator:hideUnreadMessagesIndicator - firstUnseenInteractionTimestamp:firstUnseenInteractionTimestamp]; + thread:thread + transaction:transaction + shouldHaveContactOffers:shouldHaveContactOffers + maxRangeSize:maxRangeSize + blockingSafetyNumberChanges:blockingSafetyNumberChanges + nonBlockingSafetyNumberChanges:nonBlockingSafetyNumberChanges + hideUnreadMessagesIndicator:hideUnreadMessagesIndicator + firstUnseenSortId:firstUnseenSortId]; // Determine the position of the focus message _after_ performing any mutations // around dynamic interactions. @@ -546,14 +545,14 @@ NS_ASSUME_NONNULL_BEGIN } + (void)ensureUnreadIndicator:(ThreadDynamicInteractions *)dynamicInteractions - thread:(TSThread *)thread - transaction:(YapDatabaseReadWriteTransaction *)transaction - shouldHaveContactOffers:(BOOL)shouldHaveContactOffers - maxRangeSize:(int)maxRangeSize - blockingSafetyNumberChanges:(NSArray *)blockingSafetyNumberChanges - nonBlockingSafetyNumberChanges:(NSArray *)nonBlockingSafetyNumberChanges - hideUnreadMessagesIndicator:(BOOL)hideUnreadMessagesIndicator - firstUnseenInteractionTimestamp:(nullable NSNumber *)firstUnseenInteractionTimestamp + thread:(TSThread *)thread + transaction:(YapDatabaseReadWriteTransaction *)transaction + shouldHaveContactOffers:(BOOL)shouldHaveContactOffers + maxRangeSize:(int)maxRangeSize + blockingSafetyNumberChanges:(NSArray *)blockingSafetyNumberChanges + nonBlockingSafetyNumberChanges:(NSArray *)nonBlockingSafetyNumberChanges + hideUnreadMessagesIndicator:(BOOL)hideUnreadMessagesIndicator + firstUnseenSortId:(nullable NSNumber *)firstUnseenSortId { OWSAssertDebug(dynamicInteractions); OWSAssertDebug(thread); @@ -564,7 +563,7 @@ NS_ASSUME_NONNULL_BEGIN if (hideUnreadMessagesIndicator) { return; } - if (!firstUnseenInteractionTimestamp) { + if (!firstUnseenSortId) { // If there are no unseen interactions, don't show an unread indicator. return; } @@ -600,9 +599,7 @@ NS_ASSUME_NONNULL_BEGIN return; } - // MJK FIXME - use sortId - if (interaction.timestampForLegacySorting - < firstUnseenInteractionTimestamp.unsignedLongLongValue) { + if (interaction.sortId < firstUnseenSortId.unsignedLongLongValue) { // By default we want the unread indicator to appear just before // the first unread message. *stop = YES; @@ -634,15 +631,12 @@ NS_ASSUME_NONNULL_BEGIN if (hasMoreUnseenMessages) { NSMutableSet *missingUnseenSafetyNumberChanges = [NSMutableSet set]; for (TSInvalidIdentityKeyErrorMessage *safetyNumberChange in blockingSafetyNumberChanges) { - // MJK FIXME - use sortId - BOOL isUnseen - = safetyNumberChange.timestampForLegacySorting >= firstUnseenInteractionTimestamp.unsignedLongLongValue; + BOOL isUnseen = safetyNumberChange.sortId >= firstUnseenSortId.unsignedLongLongValue; if (!isUnseen) { continue; } - // MJK FIXME - use sortId - BOOL isMissing = safetyNumberChange.timestampForLegacySorting - < interactionAfterUnreadIndicator.timestampForLegacySorting; + + BOOL isMissing = safetyNumberChange.sortId < interactionAfterUnreadIndicator.sortId; if (!isMissing) { continue; } @@ -667,14 +661,12 @@ NS_ASSUME_NONNULL_BEGIN unreadIndicatorPosition++; } - // MJK FIXME - use sortId, or maybe just timestamp, do we even need timestamp? - dynamicInteractions.unreadIndicator = [[OWSUnreadIndicator alloc] - initUnreadIndicatorWithTimestamp:interactionAfterUnreadIndicator.timestampForLegacySorting - hasMoreUnseenMessages:hasMoreUnseenMessages - missingUnseenSafetyNumberChangeCount:missingUnseenSafetyNumberChangeCount - unreadIndicatorPosition:unreadIndicatorPosition - firstUnseenInteractionTimestamp:firstUnseenInteractionTimestamp.unsignedLongLongValue]; - OWSLogInfo(@"Creating Unread Indicator: %llu", dynamicInteractions.unreadIndicator.timestamp); + dynamicInteractions.unreadIndicator = + [[OWSUnreadIndicator alloc] initWithFirstUnseenSortId:firstUnseenSortId.unsignedLongLongValue + hasMoreUnseenMessages:hasMoreUnseenMessages + missingUnseenSafetyNumberChangeCount:missingUnseenSafetyNumberChangeCount + unreadIndicatorPosition:unreadIndicatorPosition]; + OWSLogInfo(@"Creating Unread Indicator: %llu", dynamicInteractions.unreadIndicator.firstUnseenSortId); } + (nullable NSNumber *)focusMessagePositionForThread:(TSThread *)thread