Merge branch 'charlesmchen/deliveryReceipts'

Signal/src/ViewControllers/MessageMetadataViewController.swift

@@ -293,8 +293,15 @@ class MessageMetadataViewController: OWSViewController {
                           MessageMetadataViewController.dateFormatter.string(from:readDate))
         }
 
-        // TODO: We don't currently track delivery state on a per-recipient basis.
+        let recipientDeliveryMap = message.recipientDeliveryMap
-        //       We should.  NOTE: This work is in PR.
+        if let deliveryTimestamp = recipientDeliveryMap[recipientId] {
+            assert(message.messageState == .sentToService)
+            let deliveryDate = NSDate.ows_date(withMillisecondsSince1970:deliveryTimestamp.uint64Value)
+            return String(format:NSLocalizedString("MESSAGE_STATUS_DELIVERED_WITH_TIMESTAMP_FORMAT",
+                                                   comment: "message status for messages delivered to the recipient. Embeds: {{the date and time the message was delivered}}."),
+                          MessageMetadataViewController.dateFormatter.string(from:deliveryDate))
+        }
+
         if message.wasDelivered {
             return NSLocalizedString("MESSAGE_STATUS_DELIVERED",
                                      comment:"message status for message delivered to their recipient.")

Signal/translations/en.lproj/Localizable.strings

@@ -833,6 +833,9 @@
    message status for message delivered to their recipient. */
 "MESSAGE_STATUS_DELIVERED" = "Delivered";
 
+/* message status for messages delivered to the recipient. Embeds: {{the date and time the message was delivered}}. */
+"MESSAGE_STATUS_DELIVERED_WITH_TIMESTAMP_FORMAT" = "Delivered %@";
+
 /* message footer for failed messages */
 "MESSAGE_STATUS_FAILED" = "Sending failed. Tap for info.";
 

SignalServiceKit/src/Messages/Interactions/TSOutgoingMessage.h

@@ -104,6 +104,9 @@ typedef NS_ENUM(NSInteger, TSGroupMetaMessage) {
 // This property won't be accurate for legacy messages.
 @property (atomic, readonly) BOOL isFromLinkedDevice;
 
+// Map of "recipient id"-to-"delivery time" of the recipients who have received the message.
+@property (atomic, readonly) NSDictionary<NSString *, NSNumber *> *recipientDeliveryMap;
+
 // Map of "recipient id"-to-"read time" of the recipients who have read the message.
 @property (atomic, readonly) NSDictionary<NSString *, NSNumber *> *recipientReadMap;
 
@@ -171,8 +174,13 @@ typedef NS_ENUM(NSInteger, TSGroupMetaMessage) {
 - (void)updateWithHasSyncedTranscript:(BOOL)hasSyncedTranscript;
 - (void)updateWithCustomMessage:(NSString *)customMessage transaction:(YapDatabaseReadWriteTransaction *)transaction;
 - (void)updateWithCustomMessage:(NSString *)customMessage;
-- (void)updateWithWasDeliveredWithTransaction:(YapDatabaseReadWriteTransaction *)transaction;
+// deliveryTimestamp is an optional parameter, since legacy
-- (void)updateWithWasDelivered;
+// delivery receipts don't have a "delivery timestamp".  Those
+// messages repurpose the "timestamp" field to indicate when the
+// corresponding message was originally sent.
+- (void)updateWithDeliveredToRecipientId:(NSString *)recipientId
+                       deliveryTimestamp:(NSNumber *_Nullable)deliveryTimestamp
+                             transaction:(YapDatabaseReadWriteTransaction *)transaction;
 - (void)updateWithWasSentFromLinkedDeviceWithTransaction:(YapDatabaseReadWriteTransaction *)transaction;
 - (void)updateWithSingleGroupRecipient:(NSString *)singleGroupRecipient
                            transaction:(YapDatabaseReadWriteTransaction *)transaction;

SignalServiceKit/src/Messages/Interactions/TSOutgoingMessage.m

@@ -38,6 +38,8 @@ NSString *const kTSOutgoingMessageSentRecipientAll = @"kTSOutgoingMessageSentRec
 
 @property (atomic) TSGroupMetaMessage groupMetaMessage;
 
+@property (atomic) NSDictionary<NSString *, NSNumber *> *recipientDeliveryMap;
+
 @property (atomic) NSDictionary<NSString *, NSNumber *> *recipientReadMap;
 
 @end
@@ -307,20 +309,26 @@ NSString *const kTSOutgoingMessageSentRecipientAll = @"kTSOutgoingMessageSentRec
     }];
 }
 
-- (void)updateWithWasDeliveredWithTransaction:(YapDatabaseReadWriteTransaction *)transaction
+- (void)updateWithDeliveredToRecipientId:(NSString *)recipientId
+                       deliveryTimestamp:(NSNumber *_Nullable)deliveryTimestamp
+                             transaction:(YapDatabaseReadWriteTransaction *)transaction
 {
+    OWSAssert(recipientId.length > 0);
     OWSAssert(transaction);
 
     [self applyChangeToSelfAndLatestOutgoingMessage:transaction
                                         changeBlock:^(TSOutgoingMessage *message) {
-                                            [message setWasDelivered:YES];
+
-                                        }];
+                                            if (deliveryTimestamp) {
+                                                NSMutableDictionary<NSString *, NSNumber *> *recipientDeliveryMap
+                                                    = (message.recipientDeliveryMap
+                                                            ? [message.recipientDeliveryMap mutableCopy]
+                                                            : [NSMutableDictionary new]);
+                                                recipientDeliveryMap[recipientId] = deliveryTimestamp;
+                                                message.recipientDeliveryMap = [recipientDeliveryMap copy];
                                             }
 
-- (void)updateWithWasDelivered
+                                            [message setWasDelivered:YES];
-{
-    [self.dbReadWriteConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) {
-        [self updateWithWasDeliveredWithTransaction:transaction];
                                         }];
 }
 
@@ -423,7 +431,7 @@ NSString *const kTSOutgoingMessageSentRecipientAll = @"kTSOutgoingMessageSentRec
                                                 = (message.recipientReadMap ? [message.recipientReadMap mutableCopy]
                                                                             : [NSMutableDictionary new]);
                                             recipientReadMap[recipientId] = @(readTimestamp);
-                                            message.recipientReadMap = recipientReadMap;
+                                            message.recipientReadMap = [recipientReadMap copy];
                                         }];
 }
 

SignalServiceKit/src/Messages/OWSMessageManager.m

@@ -186,23 +186,51 @@ NS_ASSUME_NONNULL_BEGIN
     OWSAssert(envelope);
     OWSAssert(transaction);
 
+    // Old-style delivery notices don't include a "delivery timestamp".
+    [self processDeliveryReceiptsFromRecipientId:envelope.source
+                                  sentTimestamps:@[
+                                      @(envelope.timestamp),
+                                  ]
+                               deliveryTimestamp:nil
+                                     transaction:transaction];
+}
+
+// deliveryTimestamp is an optional parameter, since legacy
+// delivery receipts don't have a "delivery timestamp".  Those
+// messages repurpose the "timestamp" field to indicate when the
+// corresponding message was originally sent.
+- (void)processDeliveryReceiptsFromRecipientId:(NSString *)recipientId
+                                sentTimestamps:(NSArray<NSNumber *> *)sentTimestamps
+                             deliveryTimestamp:(NSNumber *_Nullable)deliveryTimestamp
+                                   transaction:(YapDatabaseReadWriteTransaction *)transaction
+{
+    OWSAssert(recipientId);
+    OWSAssert(sentTimestamps);
+    OWSAssert(transaction);
+
+    for (NSNumber *nsTimestamp in sentTimestamps) {
+        uint64_t timestamp = [nsTimestamp unsignedLongLongValue];
+
         NSArray<TSOutgoingMessage *> *messages
-        = (NSArray<TSOutgoingMessage *> *)[TSInteraction interactionsWithTimestamp:envelope.timestamp
+            = (NSArray<TSOutgoingMessage *> *)[TSInteraction interactionsWithTimestamp:timestamp
                                                                                ofClass:[TSOutgoingMessage class]
                                                                        withTransaction:transaction];
         if (messages.count < 1) {
-        // Desktop currently sends delivery receipts for "unpersisted" messages
+            // The service sends delivery receipts for "unpersisted" messages
             // like group updates, so these errors are expected to a certain extent.
-        DDLogInfo(@"%@ Missing message for delivery receipt: %llu", self.tag, envelope.timestamp);
+            //
+            // TODO: persist "early" delivery receipts.
+            DDLogInfo(@"%@ Missing message for delivery receipt: %llu", self.tag, timestamp);
         } else {
             if (messages.count > 1) {
-            DDLogInfo(@"%@ More than one message (%zd) for delivery receipt: %llu",
+                DDLogInfo(
-                self.tag,
+                    @"%@ More than one message (%zd) for delivery receipt: %llu", self.tag, messages.count, timestamp);
-                messages.count,
-                envelope.timestamp);
             }
             for (TSOutgoingMessage *outgoingMessage in messages) {
-            [outgoingMessage updateWithWasDeliveredWithTransaction:transaction];
+                [outgoingMessage updateWithDeliveredToRecipientId:recipientId
+                                                deliveryTimestamp:deliveryTimestamp
+                                                      transaction:transaction];
+            }
         }
     }
 }
@@ -243,7 +271,7 @@ NS_ASSUME_NONNULL_BEGIN
         } else if (content.hasNullMessage) {
             DDLogInfo(@"%@ Received null message.", self.tag);
         } else if (content.hasReceiptMessage) {
-            [self handleIncomingEnvelope:envelope withReceiptMessage:content.receiptMessage];
+            [self handleIncomingEnvelope:envelope withReceiptMessage:content.receiptMessage transaction:transaction];
         } else {
             DDLogWarn(@"%@ Ignoring envelope. Content with no known payload", self.tag);
         }
@@ -340,17 +368,32 @@ NS_ASSUME_NONNULL_BEGIN
 
 - (void)handleIncomingEnvelope:(OWSSignalServiceProtosEnvelope *)envelope
             withReceiptMessage:(OWSSignalServiceProtosReceiptMessage *)receiptMessage
+                   transaction:(YapDatabaseReadWriteTransaction *)transaction
 {
     OWSAssert(envelope);
     OWSAssert(receiptMessage);
+    OWSAssert(transaction);
+
+    PBArray *messageTimestamps = receiptMessage.timestamp;
+    NSMutableArray<NSNumber *> *sentTimestamps = [NSMutableArray new];
+    for (int i = 0; i < messageTimestamps.count; i++) {
+        UInt64 timestamp = [messageTimestamps uint64AtIndex:i];
+        [sentTimestamps addObject:@(timestamp)];
+    }
 
     switch (receiptMessage.type) {
         case OWSSignalServiceProtosReceiptMessageTypeDelivery:
-            DDLogInfo(@"%@ Ignoring receipt message with delivery receipt.", self.tag);
+            DDLogVerbose(@"%@ Processing receipt message with delivery receipts.", self.tag);
+            [self processDeliveryReceiptsFromRecipientId:envelope.source
+                                          sentTimestamps:sentTimestamps
+                                       deliveryTimestamp:@(envelope.timestamp)
+                                             transaction:transaction];
             return;
         case OWSSignalServiceProtosReceiptMessageTypeRead:
             DDLogVerbose(@"%@ Processing receipt message with read receipts.", self.tag);
-            [OWSReadReceiptManager.sharedManager processReadReceiptsFromRecipient:receiptMessage envelope:envelope];
+            [OWSReadReceiptManager.sharedManager processReadReceiptsFromRecipientId:envelope.source
+                                                                     sentTimestamps:sentTimestamps
+                                                                      readTimestamp:envelope.timestamp];
             break;
         default:
             DDLogInfo(@"%@ Ignoring receipt message of unknown type: %d.", self.tag, (int)receiptMessage.type);

SignalServiceKit/src/Messages/OWSReadReceiptManager.h

@@ -4,8 +4,6 @@
 
 NS_ASSUME_NONNULL_BEGIN
 
-@class OWSSignalServiceProtosEnvelope;
-@class OWSSignalServiceProtosReceiptMessage;
 @class OWSSignalServiceProtosSyncMessageRead;
 @class TSIncomingMessage;
 @class TSOutgoingMessage;
@@ -41,8 +39,9 @@ extern NSString *const kIncomingMessageMarkedAsReadNotification;
 // from a user to whom we have sent a message.
 //
 // This method can be called from any thread.
-- (void)processReadReceiptsFromRecipient:(OWSSignalServiceProtosReceiptMessage *)receiptMessage
+- (void)processReadReceiptsFromRecipientId:(NSString *)recipientId
-                                envelope:(OWSSignalServiceProtosEnvelope *)envelope;
+                            sentTimestamps:(NSArray<NSNumber *> *)sentTimestamps
+                             readTimestamp:(uint64_t)readTimestamp;
 
 - (void)applyEarlyReadReceiptsForOutgoingMessageFromLinkedDevice:(TSOutgoingMessage *)message
                                                      transaction:(YapDatabaseReadWriteTransaction *)transaction;

SignalServiceKit/src/Messages/OWSReadReceiptManager.m

@@ -340,28 +340,22 @@ NSString *const OWSReadReceiptManagerAreReadReceiptsEnabled = @"areReadReceiptsE
 
 #pragma mark - Read Receipts From Recipient
 
-- (void)processReadReceiptsFromRecipient:(OWSSignalServiceProtosReceiptMessage *)receiptMessage
+- (void)processReadReceiptsFromRecipientId:(NSString *)recipientId
-                                envelope:(OWSSignalServiceProtosEnvelope *)envelope
+                            sentTimestamps:(NSArray<NSNumber *> *)sentTimestamps
+                             readTimestamp:(uint64_t)readTimestamp
 {
-    OWSAssert(receiptMessage);
+    OWSAssert(recipientId.length > 0);
-    OWSAssert(envelope);
+    OWSAssert(sentTimestamps);
-    OWSAssert(receiptMessage.type == OWSSignalServiceProtosReceiptMessageTypeRead);
 
     if (![self areReadReceiptsEnabled]) {
         DDLogInfo(@"%@ Ignoring incoming receipt message as read receipts are disabled.", self.tag);
         return;
     }
 
-    NSString *recipientId = envelope.source;
-    OWSAssert(recipientId.length > 0);
-
-    PBArray *sentTimestamps = receiptMessage.timestamp;
-    UInt64 readTimestamp = envelope.timestamp;
-
     dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{
         [self.dbConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) {
-            for (int i = 0; i < sentTimestamps.count; i++) {
+            for (NSNumber *nsSentTimestamp in sentTimestamps) {
-                UInt64 sentTimestamp = [sentTimestamps uint64AtIndex:i];
+                UInt64 sentTimestamp = [nsSentTimestamp unsignedLongLongValue];
 
                 NSArray<TSOutgoingMessage *> *messages
                     = (NSArray<TSOutgoingMessage *> *)[TSInteraction interactionsWithTimestamp:sentTimestamp