diff --git a/Signal/src/ViewControllers/MessageMetadataViewController.swift b/Signal/src/ViewControllers/MessageMetadataViewController.swift index cd1c4844d..bb0abeb70 100644 --- a/Signal/src/ViewControllers/MessageMetadataViewController.swift +++ b/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. - // We should. NOTE: This work is in PR. + let recipientDeliveryMap = message.recipientDeliveryMap + 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.") diff --git a/Signal/translations/en.lproj/Localizable.strings b/Signal/translations/en.lproj/Localizable.strings index 349079db7..d9043c9c1 100644 --- a/Signal/translations/en.lproj/Localizable.strings +++ b/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."; diff --git a/SignalServiceKit/src/Messages/Interactions/TSOutgoingMessage.h b/SignalServiceKit/src/Messages/Interactions/TSOutgoingMessage.h index ab1e9c157..a1835a5e5 100644 --- a/SignalServiceKit/src/Messages/Interactions/TSOutgoingMessage.h +++ b/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 *recipientDeliveryMap; + // Map of "recipient id"-to-"read time" of the recipients who have read the message. @property (atomic, readonly) NSDictionary *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; -- (void)updateWithWasDelivered; +// 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)updateWithDeliveredToRecipientId:(NSString *)recipientId + deliveryTimestamp:(NSNumber *_Nullable)deliveryTimestamp + transaction:(YapDatabaseReadWriteTransaction *)transaction; - (void)updateWithWasSentFromLinkedDeviceWithTransaction:(YapDatabaseReadWriteTransaction *)transaction; - (void)updateWithSingleGroupRecipient:(NSString *)singleGroupRecipient transaction:(YapDatabaseReadWriteTransaction *)transaction; diff --git a/SignalServiceKit/src/Messages/Interactions/TSOutgoingMessage.m b/SignalServiceKit/src/Messages/Interactions/TSOutgoingMessage.m index 8b686304a..bfcdf9d90 100644 --- a/SignalServiceKit/src/Messages/Interactions/TSOutgoingMessage.m +++ b/SignalServiceKit/src/Messages/Interactions/TSOutgoingMessage.m @@ -38,6 +38,8 @@ NSString *const kTSOutgoingMessageSentRecipientAll = @"kTSOutgoingMessageSentRec @property (atomic) TSGroupMetaMessage groupMetaMessage; +@property (atomic) NSDictionary *recipientDeliveryMap; + @property (atomic) NSDictionary *recipientReadMap; @end @@ -307,23 +309,29 @@ 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) { + + if (deliveryTimestamp) { + NSMutableDictionary *recipientDeliveryMap + = (message.recipientDeliveryMap + ? [message.recipientDeliveryMap mutableCopy] + : [NSMutableDictionary new]); + recipientDeliveryMap[recipientId] = deliveryTimestamp; + message.recipientDeliveryMap = [recipientDeliveryMap copy]; + } + [message setWasDelivered:YES]; }]; } -- (void)updateWithWasDelivered -{ - [self.dbReadWriteConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { - [self updateWithWasDeliveredWithTransaction:transaction]; - }]; -} - - (void)updateWithWasSentFromLinkedDeviceWithTransaction:(YapDatabaseReadWriteTransaction *)transaction { OWSAssert(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]; }]; } diff --git a/SignalServiceKit/src/Messages/OWSMessageManager.m b/SignalServiceKit/src/Messages/OWSMessageManager.m index 90d7539f0..d1d32d5fa 100644 --- a/SignalServiceKit/src/Messages/OWSMessageManager.m +++ b/SignalServiceKit/src/Messages/OWSMessageManager.m @@ -186,23 +186,51 @@ NS_ASSUME_NONNULL_BEGIN OWSAssert(envelope); OWSAssert(transaction); - NSArray *messages - = (NSArray *)[TSInteraction interactionsWithTimestamp:envelope.timestamp - ofClass:[TSOutgoingMessage class] - withTransaction:transaction]; - if (messages.count < 1) { - // Desktop currently 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); - } else { - if (messages.count > 1) { - DDLogInfo(@"%@ More than one message (%zd) for delivery receipt: %llu", - self.tag, - messages.count, - envelope.timestamp); - } - for (TSOutgoingMessage *outgoingMessage in messages) { - [outgoingMessage updateWithWasDeliveredWithTransaction: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 *)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 *messages + = (NSArray *)[TSInteraction interactionsWithTimestamp:timestamp + ofClass:[TSOutgoingMessage class] + withTransaction:transaction]; + if (messages.count < 1) { + // The service sends delivery receipts for "unpersisted" messages + // like group updates, so these errors are expected to a certain extent. + // + // 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", self.tag, messages.count, timestamp); + } + for (TSOutgoingMessage *outgoingMessage in messages) { + [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 *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); diff --git a/SignalServiceKit/src/Messages/OWSReadReceiptManager.h b/SignalServiceKit/src/Messages/OWSReadReceiptManager.h index ad925c6f1..45bc80291 100644 --- a/SignalServiceKit/src/Messages/OWSReadReceiptManager.h +++ b/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 - envelope:(OWSSignalServiceProtosEnvelope *)envelope; +- (void)processReadReceiptsFromRecipientId:(NSString *)recipientId + sentTimestamps:(NSArray *)sentTimestamps + readTimestamp:(uint64_t)readTimestamp; - (void)applyEarlyReadReceiptsForOutgoingMessageFromLinkedDevice:(TSOutgoingMessage *)message transaction:(YapDatabaseReadWriteTransaction *)transaction; diff --git a/SignalServiceKit/src/Messages/OWSReadReceiptManager.m b/SignalServiceKit/src/Messages/OWSReadReceiptManager.m index bdabd2eac..f25576b46 100644 --- a/SignalServiceKit/src/Messages/OWSReadReceiptManager.m +++ b/SignalServiceKit/src/Messages/OWSReadReceiptManager.m @@ -340,28 +340,22 @@ NSString *const OWSReadReceiptManagerAreReadReceiptsEnabled = @"areReadReceiptsE #pragma mark - Read Receipts From Recipient -- (void)processReadReceiptsFromRecipient:(OWSSignalServiceProtosReceiptMessage *)receiptMessage - envelope:(OWSSignalServiceProtosEnvelope *)envelope +- (void)processReadReceiptsFromRecipientId:(NSString *)recipientId + sentTimestamps:(NSArray *)sentTimestamps + readTimestamp:(uint64_t)readTimestamp { - OWSAssert(receiptMessage); - OWSAssert(envelope); - OWSAssert(receiptMessage.type == OWSSignalServiceProtosReceiptMessageTypeRead); + OWSAssert(recipientId.length > 0); + OWSAssert(sentTimestamps); 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++) { - UInt64 sentTimestamp = [sentTimestamps uint64AtIndex:i]; + for (NSNumber *nsSentTimestamp in sentTimestamps) { + UInt64 sentTimestamp = [nsSentTimestamp unsignedLongLongValue]; NSArray *messages = (NSArray *)[TSInteraction interactionsWithTimestamp:sentTimestamp