Merge branch 'charlesmchen/readReceiptsVsOlderMessages'

src/Contacts/TSThread.h

@@ -72,7 +72,6 @@ NS_ASSUME_NONNULL_BEGIN
 
 - (BOOL)hasSafetyNumbers;
 
-- (void)markAllAsRead;
 - (void)markAllAsReadWithTransaction:(YapDatabaseReadWriteTransaction *)transaction;
 
 /**
@@ -177,6 +176,9 @@ NS_ASSUME_NONNULL_BEGIN
 // data loss and will resolve all known issues.
 - (void)updateWithMutedUntilDate:(NSDate *)mutedUntilDate;
 
+// Returns YES IFF the interaction should show up in the inbox as the last message.
++ (BOOL)shouldInteractionAppearInInbox:(TSInteraction *)interaction;
+
 @end
 
 NS_ASSUME_NONNULL_END

src/Contacts/TSThread.m

@@ -228,26 +228,28 @@ NS_ASSUME_NONNULL_BEGIN
 - (void)markAllAsReadWithTransaction:(YapDatabaseReadWriteTransaction *)transaction
 {
     for (id<OWSReadTracking> message in [self unseenMessagesWithTransaction:transaction]) {
-        [message markAsReadLocallyWithTransaction:transaction];
+        [message markAsReadWithTransaction:transaction sendReadReceipt:YES updateExpiration:YES];
     }
 
     // Just to be defensive, we'll also check for unread messages.
     OWSAssert([self unseenMessagesWithTransaction:transaction].count < 1);
 }
 
-- (void)markAllAsRead
-{
-    [self.dbConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *_Nonnull transaction) {
-        [self markAllAsReadWithTransaction:transaction];
-    }];
-}
-
 - (TSInteraction *) lastInteraction {
     __block TSInteraction *last;
     [TSStorageManager.sharedManager.dbConnection readWithBlock:^(YapDatabaseReadTransaction *transaction){
         last = [[transaction ext:TSMessageDatabaseViewExtensionName] lastObjectInGroup:self.uniqueId];
     }];
-    return (TSInteraction *)last;
+    return last;
+}
+
+- (TSInteraction *)lastInteractionForInbox
+{
+    __block TSInteraction *last;
+    [TSStorageManager.sharedManager.dbConnection readWithBlock:^(YapDatabaseReadTransaction *transaction) {
+        last = [[transaction ext:TSThreadInboxMessagesDatabaseViewExtensionName] lastObjectInGroup:self.uniqueId];
+    }];
+    return last;
 }
 
 - (NSDate *)lastMessageDate {
@@ -259,49 +261,53 @@ NS_ASSUME_NONNULL_BEGIN
 }
 
 - (NSString *)lastMessageLabel {
-    if (self.lastInteraction == nil) {
+    TSInteraction *interaction = self.lastInteractionForInbox;
+    if (interaction == nil) {
         return @"";
     } else {
-        return [self lastInteraction].description;
+        return interaction.description;
     }
 }
 
-- (void)updateWithLastMessage:(TSInteraction *)lastMessage transaction:(YapDatabaseReadWriteTransaction *)transaction {
-    OWSAssert(lastMessage);
-    OWSAssert(transaction);
++ (BOOL)shouldInteractionAppearInInbox:(TSInteraction *)interaction
+{
+    OWSAssert(interaction);
 
-    if (lastMessage.isDynamicInteraction) {
-        DDLogDebug(@"%@ not updating lastMessage for thread: %@ dynamic interaction: %@",
-            self.tag,
-            self,
-            lastMessage.debugDescription);
-        return;
+    if (interaction.isDynamicInteraction) {
+        DDLogDebug(@"%@ not showing dynamic interaction in inbox: %@", self.tag, interaction.debugDescription);
+        return NO;
     }
 
-    NSDate *lastMessageDate = [lastMessage dateForSorting];
-
-    if ([lastMessage isKindOfClass:[TSErrorMessage class]]) {
-        TSErrorMessage *errorMessage = (TSErrorMessage *)lastMessage;
+    if ([interaction isKindOfClass:[TSErrorMessage class]]) {
+        TSErrorMessage *errorMessage = (TSErrorMessage *)interaction;
         if (errorMessage.errorType == TSErrorMessageNonBlockingIdentityChange) {
             // Otherwise all group threads with the recipient will percolate to the top of the inbox, even though
             // there was no meaningful interaction.
-            DDLogDebug(@"%@ not updating lastMessage for thread: %@ nonblocking identity change: %@",
-                self.tag,
-                self,
-                errorMessage.debugDescription);
-            return;
+            DDLogDebug(
+                @"%@ not showing nonblocking identity change in inbox: %@", self.tag, errorMessage.debugDescription);
+            return NO;
         }
-    } else if ([lastMessage isKindOfClass:[TSInfoMessage class]]) {
-        TSInfoMessage *infoMessage = (TSInfoMessage *)lastMessage;
+    } else if ([interaction isKindOfClass:[TSInfoMessage class]]) {
+        TSInfoMessage *infoMessage = (TSInfoMessage *)interaction;
         if (infoMessage.messageType == TSInfoMessageVerificationStateChange) {
-            DDLogDebug(@"%@ not updating lastMessage for thread: %@ verification state change: %@",
-                self.tag,
-                self,
-                infoMessage.debugDescription);
-            return;
+            DDLogDebug(
+                @"%@ not showing verification state change in inbox: %@", self.tag, infoMessage.debugDescription);
+            return NO;
         }
     }
 
+    return YES;
+}
+
+- (void)updateWithLastMessage:(TSInteraction *)lastMessage transaction:(YapDatabaseReadWriteTransaction *)transaction {
+    OWSAssert(lastMessage);
+    OWSAssert(transaction);
+
+    if (![self.class shouldInteractionAppearInInbox:lastMessage]) {
+        return;
+    }
+
+    NSDate *lastMessageDate = [lastMessage dateForSorting];
     if (!_lastMessageDate || [lastMessageDate timeIntervalSinceDate:self.lastMessageDate] > 0) {
         _lastMessageDate = lastMessageDate;
 

src/Devices/OWSReadReceiptsProcessor.m

@@ -7,7 +7,10 @@
 #import "OWSReadReceipt.h"
 #import "OWSSignalServiceProtos.pb.h"
 #import "TSContactThread.h"
+#import "TSDatabaseView.h"
 #import "TSIncomingMessage.h"
+#import "TSStorageManager.h"
+#import <YapDatabase/YapDatabaseConnection.h>
 
 NS_ASSUME_NONNULL_BEGIN
 
@@ -82,14 +85,52 @@ NSString *const OWSReadReceiptsProcessorMarkedMessageAsReadNotification =
         TSIncomingMessage *message =
             [TSIncomingMessage findMessageWithAuthorId:readReceipt.senderId timestamp:readReceipt.timestamp];
         if (message) {
-            [message markAsReadFromReadReceipt];
-            [OWSDisappearingMessagesJob setExpirationForMessage:message expirationStartedAt:readReceipt.timestamp];
+            OWSAssert(message.thread);
+
+            // Mark all unread messages in this thread that are older than message specified in the read
+            // receipt.
+            NSMutableArray<id<OWSReadTracking>> *interactionsToMarkAsRead = [NSMutableArray new];
+            [self.storageManager.dbConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) {
+                [[transaction ext:TSUnseenDatabaseViewExtensionName]
+                    enumerateRowsInGroup:message.uniqueThreadId
+                              usingBlock:^(NSString *collection,
+                                  NSString *key,
+                                  id object,
+                                  id metadata,
+                                  NSUInteger index,
+                                  BOOL *stop) {
+
+                                  TSInteraction *interaction = object;
+                                  if (interaction.timestampForSorting > message.timestampForSorting) {
+                                      *stop = YES;
+                                      return;
+                                  }
+
+                                  id<OWSReadTracking> possiblyRead = (id<OWSReadTracking>)object;
+                                  OWSAssert(!possiblyRead.read);
+                                  [interactionsToMarkAsRead addObject:possiblyRead];
+                              }];
+
+                for (id<OWSReadTracking> interaction in interactionsToMarkAsRead) {
+                    // * Don't send a read receipt in response to a read receipt.
+                    // * Don't update expiration; we'll do that in the next statement.
+                    [interaction markAsReadWithTransaction:transaction sendReadReceipt:NO updateExpiration:NO];
+
+                    // Update expiration using the timestamp from the readReceipt.
+                    [OWSDisappearingMessagesJob setExpirationForMessage:(TSMessage *)interaction
+                                                    expirationStartedAt:readReceipt.timestamp];
+
+                    // Fire event that will cancel any pending notifications for this message.
+                    dispatch_async(dispatch_get_main_queue(), ^{
+                        [[NSNotificationCenter defaultCenter]
+                            postNotificationName:OWSReadReceiptsProcessorMarkedMessageAsReadNotification
+                                          object:(TSMessage *)interaction];
+                    });
+                }
+            }];
+
             // If it was previously saved, no need to keep it around any longer.
             [readReceipt remove];
-            [[NSNotificationCenter defaultCenter]
-                postNotificationName:OWSReadReceiptsProcessorMarkedMessageAsReadNotification
-                              object:message];
-
         } else {
             DDLogDebug(@"%@ Received read receipt for an unknown message. Saving it for later.", self.tag);
             [readReceipt save];

src/Messages/Interactions/TSErrorMessage.h

@@ -58,8 +58,6 @@ typedef NS_ENUM(int32_t, TSErrorMessageType) {
 @property (nonatomic, readonly) TSErrorMessageType errorType;
 @property (nullable, nonatomic, readonly) NSString *recipientId;
 
-- (void)markAsReadLocallyWithTransaction:(YapDatabaseReadWriteTransaction *)transaction;
-
 @end
 
 NS_ASSUME_NONNULL_END

src/Messages/Interactions/TSErrorMessage.m

@@ -167,19 +167,22 @@ NSUInteger TSErrorMessageSchemaVersion = 1;
     return NO;
 }
 
-- (void)markAsReadLocally
-{
-    [self.dbConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) {
-        [self markAsReadLocallyWithTransaction:transaction];
-    }];
-}
-
-- (void)markAsReadLocallyWithTransaction:(YapDatabaseReadWriteTransaction *)transaction
+- (void)markAsReadWithTransaction:(YapDatabaseReadWriteTransaction *)transaction
+                  sendReadReceipt:(BOOL)sendReadReceipt
+                 updateExpiration:(BOOL)updateExpiration
 {
     OWSAssert(transaction);
+
+    if (_read) {
+        return;
+    }
+
     DDLogInfo(@"%@ marking as read uniqueId: %@ which has timestamp: %llu", self.tag, self.uniqueId, self.timestamp);
     _read = YES;
     [self saveWithTransaction:transaction];
+    [self touchThreadWithTransaction:transaction];
+
+    // Ignore sendReadReceipt and updateExpiration; they don't apply to error messages.
 }
 
 #pragma mark - Logging

src/Messages/Interactions/TSIncomingMessage.h

@@ -108,20 +108,6 @@ extern NSString *const TSIncomingMessageWasReadOnThisDeviceNotification;
 // This will be 0 for messages created before we were tracking sourceDeviceId
 @property (nonatomic, readonly) UInt32 sourceDeviceId;
 
-/*
- * Marks a message as having been read on this device (as opposed to responding to a remote read receipt).
- *
- */
-- (void)markAsReadLocally;
-// TODO possible to remove?
-- (void)markAsReadLocallyWithTransaction:(YapDatabaseReadWriteTransaction *)transaction;
-
-/**
- * Similar to markAsReadWithTransaction, but doesn't send out read receipts.
- * Used for *responding* to a remote read receipt.
- */
-- (void)markAsReadFromReadReceipt;
-
 @end
 
 NS_ASSUME_NONNULL_END

src/Messages/Interactions/TSIncomingMessage.m

@@ -3,6 +3,8 @@
 //
 
 #import "TSIncomingMessage.h"
+#import "OWSDisappearingMessagesConfiguration.h"
+#import "OWSDisappearingMessagesJob.h"
 #import "TSContactThread.h"
 #import "TSDatabaseSecondaryIndexes.h"
 #import "TSGroupThread.h"
@@ -115,37 +117,33 @@ NSString *const TSIncomingMessageWasReadOnThisDeviceNotification = @"TSIncomingM
     return YES;
 }
 
-- (void)markAsReadFromReadReceipt
+- (void)markAsReadWithTransaction:(YapDatabaseReadWriteTransaction *)transaction
+                  sendReadReceipt:(BOOL)sendReadReceipt
+                 updateExpiration:(BOOL)updateExpiration
 {
-    [self.dbConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) {
-        [self markAsReadWithoutNotificationWithTransaction:transaction];
-    }];
-}
-
-- (void)markAsReadLocallyWithTransaction:(YapDatabaseReadWriteTransaction *)transaction
-{
-    [self markAsReadWithoutNotificationWithTransaction:transaction];
-    [[NSNotificationCenter defaultCenter] postNotificationName:TSIncomingMessageWasReadOnThisDeviceNotification
-                                                        object:self];
-}
+    OWSAssert(transaction);
 
-- (void)markAsReadLocally
-{
-    [self.dbConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) {
-        [self markAsReadWithoutNotificationWithTransaction:transaction];
-    }];
-    // Notification must happen outside of the transaction, else we'll likely crash when the notification receiver
-    // tries to do anything with the DB.
-    [[NSNotificationCenter defaultCenter] postNotificationName:TSIncomingMessageWasReadOnThisDeviceNotification
-                                                        object:self];
-}
+    if (_read) {
+        return;
+    }
 
-- (void)markAsReadWithoutNotificationWithTransaction:(YapDatabaseReadWriteTransaction *)transaction
-{
     DDLogInfo(@"%@ marking as read uniqueId: %@ which has timestamp: %llu", self.tag, self.uniqueId, self.timestamp);
     _read = YES;
     [self saveWithTransaction:transaction];
     [self touchThreadWithTransaction:transaction];
+
+    if (updateExpiration) {
+        [OWSDisappearingMessagesJob setExpirationForMessage:self];
+    }
+
+    if (sendReadReceipt) {
+        // Notification must happen outside of the transaction, else we'll likely crash when the notification receiver
+        // tries to do anything with the DB.
+        dispatch_async(dispatch_get_main_queue(), ^{
+            [[NSNotificationCenter defaultCenter] postNotificationName:TSIncomingMessageWasReadOnThisDeviceNotification
+                                                                object:self];
+        });
+    }
 }
 
 #pragma mark - Logging

src/Messages/Interactions/TSInfoMessage.h

@@ -44,8 +44,6 @@ typedef NS_ENUM(NSInteger, TSInfoMessageType) {
                  expiresInSeconds:(uint32_t)expiresInSeconds
                   expireStartedAt:(uint64_t)expireStartedAt NS_UNAVAILABLE;
 
-- (void)markAsReadLocallyWithTransaction:(YapDatabaseReadWriteTransaction *)transaction;
-
 @end
 
 NS_ASSUME_NONNULL_END

src/Messages/Interactions/TSInfoMessage.m

@@ -109,19 +109,22 @@ NSUInteger TSInfoMessageSchemaVersion = 1;
     return NO;
 }
 
-- (void)markAsReadLocally
-{
-    [self.dbConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) {
-        [self markAsReadLocallyWithTransaction:transaction];
-    }];
-}
-
-- (void)markAsReadLocallyWithTransaction:(YapDatabaseReadWriteTransaction *)transaction
+- (void)markAsReadWithTransaction:(YapDatabaseReadWriteTransaction *)transaction
+                  sendReadReceipt:(BOOL)sendReadReceipt
+                 updateExpiration:(BOOL)updateExpiration
 {
     OWSAssert(transaction);
+
+    if (_read) {
+        return;
+    }
+
     DDLogInfo(@"%@ marking as read uniqueId: %@ which has timestamp: %llu", self.tag, self.uniqueId, self.timestamp);
     _read = YES;
     [self saveWithTransaction:transaction];
+    [self touchThreadWithTransaction:transaction];
+
+    // Ignore sendReadReceipt and updateExpiration; they don't apply to info messages.
 }
 
 #pragma mark - Logging

src/Messages/OWSIncomingMessageReadObserver.m

@@ -4,8 +4,6 @@
 
 #import "OWSIncomingMessageReadObserver.h"
 #import "NSDate+millisecondTimeStamp.h"
-#import "OWSDisappearingMessagesConfiguration.h"
-#import "OWSDisappearingMessagesJob.h"
 #import "OWSSendReadReceiptsJob.h"
 #import "TSIncomingMessage.h"
 
@@ -13,7 +11,7 @@ NS_ASSUME_NONNULL_BEGIN
 
 @interface OWSIncomingMessageReadObserver ()
 
-@property BOOL isObserving;
+@property (nonatomic) BOOL isObserving;
 @property (nonatomic, readonly) OWSSendReadReceiptsJob *sendReadReceiptsJob;
 
 @end
@@ -41,6 +39,8 @@ NS_ASSUME_NONNULL_BEGIN
 
 - (void)startObserving
 {
+    OWSAssert([NSThread isMainThread]);
+
     if (self.isObserving) {
         return;
     }
@@ -60,7 +60,6 @@ NS_ASSUME_NONNULL_BEGIN
     }
 
     TSIncomingMessage *message = (TSIncomingMessage *)notification.object;
-    [OWSDisappearingMessagesJob setExpirationForMessage:message];
     [self.sendReadReceiptsJob runWith:message];
 }
 

src/Messages/OWSReadTracking.h

@@ -20,10 +20,10 @@
 - (BOOL)shouldAffectUnreadCounts;
 
 /**
- * Call when the user viewed the message/call on this device. "locally" as opposed to being notified via a read receipt
- * sync message of a remote read.
+ * Used for *responding* to a remote read receipt or in response to user activity.
  */
-- (void)markAsReadLocally;
-- (void)markAsReadLocallyWithTransaction:(YapDatabaseReadWriteTransaction *)transaction;
+- (void)markAsReadWithTransaction:(YapDatabaseReadWriteTransaction *)transaction
+                  sendReadReceipt:(BOOL)sendReadReceipt
+                 updateExpiration:(BOOL)updateExpiration;
 
 @end

src/Messages/TSCall.m

@@ -86,22 +86,24 @@ NSUInteger TSCallCurrentSchemaVersion = 1;
     return YES;
 }
 
-- (void)markAsReadLocallyWithTransaction:(YapDatabaseReadWriteTransaction *)transaction
+- (void)markAsReadWithTransaction:(YapDatabaseReadWriteTransaction *)transaction
+                  sendReadReceipt:(BOOL)sendReadReceipt
+                 updateExpiration:(BOOL)updateExpiration
 {
-    DDLogInfo(@"%@ marking as read uniqueId: %@ which has timestamp: %llu", self.tag, self.uniqueId, self.timestamp);
+    OWSAssert(transaction);
+
+    if (_read) {
+        return;
+    }
 
+    DDLogInfo(@"%@ marking as read uniqueId: %@ which has timestamp: %llu", self.tag, self.uniqueId, self.timestamp);
     _read = YES;
     [self saveWithTransaction:transaction];
-
-    // redraw any thread-related unread count UI.
     [self touchThreadWithTransaction:transaction];
-}
 
-- (void)markAsReadLocally
-{
-    [self.dbConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *_Nonnull transaction) {
-        [self markAsReadLocallyWithTransaction:transaction];
-    }];
+    // Ignore sendReadReceipt and updateExpiration; they don't apply to calls.
+    //
+    // TODO: Should we update expiration of calls?
 }
 
 #pragma mark - Methods

src/Messages/TSMessagesManager.m

@@ -924,7 +924,8 @@ NS_ASSUME_NONNULL_BEGIN
           // automatically marked as read.
           BOOL shouldMarkMessageAsRead = [envelope.source isEqualToString:localNumber];
           if (shouldMarkMessageAsRead) {
-              [incomingMessage markAsReadLocallyWithTransaction:transaction];
+              // Don't send a read receipt for messages sent by ourselves.
+              [incomingMessage markAsReadWithTransaction:transaction sendReadReceipt:NO updateExpiration:YES];
           }
 
           // Other clients allow attachments to be sent along with body, we want the text displayed as a separate

src/Storage/TSDatabaseView.h

@@ -14,6 +14,7 @@ extern NSString *TSSecondaryDevicesGroup;
 
 extern NSString *TSThreadDatabaseViewExtensionName;
 extern NSString *TSMessageDatabaseViewExtensionName;
+extern NSString *TSThreadInboxMessagesDatabaseViewExtensionName;
 extern NSString *TSThreadIncomingMessageDatabaseViewExtensionName;
 extern NSString *TSThreadOutgoingMessageDatabaseViewExtensionName;
 extern NSString *TSUnreadDatabaseViewExtensionName;
@@ -24,6 +25,7 @@ extern NSString *TSSecondaryDevicesDatabaseViewExtensionName;
 
 + (BOOL)registerThreadDatabaseView;
 + (BOOL)registerThreadInteractionsDatabaseView;
++ (BOOL)registerThreadInboxMessageDatabaseView;
 + (BOOL)registerThreadIncomingMessagesDatabaseView;
 + (BOOL)registerThreadOutgoingMessagesDatabaseView;
 

src/Storage/TSDatabaseView.m

@@ -20,6 +20,7 @@ NSString *TSSecondaryDevicesGroup = @"TSSecondaryDevicesGroup";
 
 NSString *TSThreadDatabaseViewExtensionName  = @"TSThreadDatabaseViewExtensionName";
 NSString *TSMessageDatabaseViewExtensionName = @"TSMessageDatabaseViewExtensionName";
+NSString *TSThreadInboxMessagesDatabaseViewExtensionName = @"TSThreadInboxMessagesDatabaseViewExtensionName";
 NSString *TSThreadIncomingMessageDatabaseViewExtensionName = @"TSThreadOutgoingMessageDatabaseViewExtensionName";
 NSString *TSThreadOutgoingMessageDatabaseViewExtensionName = @"TSThreadOutgoingMessageDatabaseViewExtensionName";
 NSString *TSUnreadDatabaseViewExtensionName  = @"TSUnreadDatabaseViewExtensionName";
@@ -147,6 +148,27 @@ NSString *TSSecondaryDevicesDatabaseViewExtensionName = @"TSSecondaryDevicesData
                                              version:@"1"];
 }
 
++ (BOOL)registerThreadInboxMessageDatabaseView
+{
+    YapDatabaseViewGrouping *viewGrouping = [YapDatabaseViewGrouping withObjectBlock:^NSString *(
+        YapDatabaseReadTransaction *transaction, NSString *collection, NSString *key, id object) {
+        if ([object isKindOfClass:[TSInteraction class]]) {
+            TSInteraction *interaction = (TSInteraction *)object;
+            if (![TSThread shouldInteractionAppearInInbox:interaction]) {
+                return nil;
+            }
+            return interaction.uniqueThreadId;
+        } else {
+            OWSAssert(0);
+        }
+        return nil;
+    }];
+
+    return [self registerMessageDatabaseViewWithName:TSThreadInboxMessagesDatabaseViewExtensionName
+                                        viewGrouping:viewGrouping
+                                             version:@"1"];
+}
+
 + (BOOL)registerThreadIncomingMessagesDatabaseView
 {
     YapDatabaseViewGrouping *viewGrouping = [YapDatabaseViewGrouping withObjectBlock:^NSString *(

src/Storage/TSStorageManager.m

@@ -196,6 +196,7 @@ static NSString *keychainDBPassAccount    = @"TSDatabasePass";
     // Register extensions which are essential for rendering threads synchronously
     [TSDatabaseView registerThreadDatabaseView];
     [TSDatabaseView registerThreadInteractionsDatabaseView];
+    [TSDatabaseView registerThreadInboxMessageDatabaseView];
     [TSDatabaseView registerThreadIncomingMessagesDatabaseView];
     [TSDatabaseView registerThreadOutgoingMessagesDatabaseView];
     [TSDatabaseView registerUnreadDatabaseView];