From 40ac0daa9a204ba981a336a1dce4049ba4110ff3 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Tue, 24 Apr 2018 15:49:08 -0400 Subject: [PATCH] Respond to CR. --- .../ConversationView/Cells/OWSMessageCell.m | 4 +-- .../Utils/MessageRecipientStatusUtils.swift | 7 ++-- .../migrations/OWS109OutgoingMessageState.m | 34 +++++++++++-------- .../src/Messages/OWSMessageSender.m | 7 ++-- 4 files changed, 31 insertions(+), 21 deletions(-) diff --git a/Signal/src/ViewControllers/ConversationView/Cells/OWSMessageCell.m b/Signal/src/ViewControllers/ConversationView/Cells/OWSMessageCell.m index 784a50c27..9bc84276c 100644 --- a/Signal/src/ViewControllers/ConversationView/Cells/OWSMessageCell.m +++ b/Signal/src/ViewControllers/ConversationView/Cells/OWSMessageCell.m @@ -505,10 +505,10 @@ NS_ASSUME_NONNULL_BEGIN if (self.viewItem.interaction.interactionType == OWSInteractionType_OutgoingMessage) { TSOutgoingMessage *outgoingMessage = (TSOutgoingMessage *)self.viewItem.interaction; - if (outgoingMessage.messageState == TSOutgoingMessageStateUnsent) { + if (outgoingMessage.messageState == TSOutgoingMessageStateFailed) { [self.delegate didTapFailedOutgoingMessage:outgoingMessage]; return; - } else if (outgoingMessage.messageState == TSOutgoingMessageStateAttemptingOut) { + } else if (outgoingMessage.messageState == TSOutgoingMessageStateSending) { // Ignore taps on outgoing messages being sent. return; } diff --git a/Signal/src/ViewControllers/Utils/MessageRecipientStatusUtils.swift b/Signal/src/ViewControllers/Utils/MessageRecipientStatusUtils.swift index 80ad3b4ce..6ff08c473 100644 --- a/Signal/src/ViewControllers/Utils/MessageRecipientStatusUtils.swift +++ b/Signal/src/ViewControllers/Utils/MessageRecipientStatusUtils.swift @@ -95,6 +95,7 @@ class MessageRecipientStatusUtils: NSObject { // might be misleading. guard let recipientState = outgoingMessage.recipientState(forRecipientId: recipientId) else { + owsFail("\(self.logTag) no message status for recipient: \(recipientId).") let shortStatusMessage = NSLocalizedString("MESSAGE_STATUS_FAILED_SHORT", comment: "status message for failed messages") let longStatusMessage = NSLocalizedString("MESSAGE_STATUS_FAILED", comment: "message footer for failed messages") return (status:.failed, shortStatusMessage:shortStatusMessage, longStatusMessage:longStatusMessage) @@ -143,7 +144,7 @@ class MessageRecipientStatusUtils: NSObject { return (status:.sent, shortStatusMessage:statusMessage, longStatusMessage:statusMessage) case .skipped: let statusMessage = NSLocalizedString("MESSAGE_STATUS_RECIPIENT_SKIPPED", - comment: "message status if message delivery to a recipient is skipped.") + comment: "message status if message delivery to a recipient is skipped. We skip delivering group messages to users who have left the group or deactivated their Signal account.") return (status:.skipped, shortStatusMessage:statusMessage, longStatusMessage:statusMessage) } } @@ -176,7 +177,7 @@ class MessageRecipientStatusUtils: NSObject { return NSLocalizedString("MESSAGE_STATUS_SENT", comment: "message footer for sent messages") default: - owsFail("Message has unexpected status: \(outgoingMessage.messageState).") + owsFail("\(self.logTag) Message has unexpected status: \(outgoingMessage.messageState).") return NSLocalizedString("MESSAGE_STATUS_SENT", comment: "message footer for sent messages") } @@ -204,7 +205,7 @@ class MessageRecipientStatusUtils: NSObject { return .sent default: - owsFail("Message has unexpected status: \(outgoingMessage.messageState).") + owsFail("\(self.logTag) Message has unexpected status: \(outgoingMessage.messageState).") return .sent } diff --git a/SignalMessaging/environment/migrations/OWS109OutgoingMessageState.m b/SignalMessaging/environment/migrations/OWS109OutgoingMessageState.m index 50296180e..aa1994272 100644 --- a/SignalMessaging/environment/migrations/OWS109OutgoingMessageState.m +++ b/SignalMessaging/environment/migrations/OWS109OutgoingMessageState.m @@ -22,23 +22,29 @@ static NSString *const OWS109OutgoingMessageStateMigrationId = @"109"; { OWSAssert(transaction); - NSMutableArray *outgoingMessages = [NSMutableArray new]; - [transaction enumerateKeysAndObjectsInCollection:TSOutgoingMessage.collection - usingBlock:^(NSString *key, id value, BOOL *stop) { - if (![value isKindOfClass:[TSOutgoingMessage class]]) { - return; - } - TSOutgoingMessage *outgoingMessage = (TSOutgoingMessage *)value; - [outgoingMessages addObject:outgoingMessage]; - }]; - - DDLogInfo(@"Saving %zd outgoing messages.", outgoingMessages.count); - // Persist the migration of the outgoing message state. // For performance, we want to upgrade all existing outgoing messages in // a single transaction. - for (TSOutgoingMessage *outgoingMessage in outgoingMessages) { - [outgoingMessage saveWithTransaction:transaction]; + NSMutableArray *messageIds = + [[transaction allKeysInCollection:TSOutgoingMessage.collection] mutableCopy]; + DDLogInfo(@"%@ Migrating %zd outgoing messages.", self.logTag, messageIds.count); + while (messageIds.count > 0) { + const int kBatchSize = 1000; + @autoreleasepool { + for (int i = 0; i < kBatchSize; i++) { + if (messageIds.count == 0) { + break; + } + NSString *messageId = [messageIds lastObject]; + [messageIds removeLastObject]; + id message = [transaction objectForKey:messageId inCollection:TSOutgoingMessage.collection]; + if (![message isKindOfClass:[TSOutgoingMessage class]]) { + return; + } + TSOutgoingMessage *outgoingMessage = (TSOutgoingMessage *)message; + [outgoingMessage saveWithTransaction:transaction]; + } + } } } diff --git a/SignalServiceKit/src/Messages/OWSMessageSender.m b/SignalServiceKit/src/Messages/OWSMessageSender.m index 5f9bb2cf1..413d2953c 100644 --- a/SignalServiceKit/src/Messages/OWSMessageSender.m +++ b/SignalServiceKit/src/Messages/OWSMessageSender.m @@ -476,8 +476,6 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException"; [self.dbConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { for (NSString *recipientId in obsoleteRecipientIds) { // Mark this recipient as "skipped". - // - // TODO: We should also mark as "skipped" group members who no longer have signal accounts. [message updateWithSkippedRecipient:recipientId transaction:transaction]; } }]; @@ -703,6 +701,11 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException"; thread:(TSThread *)thread { [self.dbConnection asyncReadWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { + if (thread.isGroupThread) { + // Mark as "skipped" group members who no longer have signal accounts. + [message updateWithSkippedRecipient:recipient.recipientId transaction:transaction]; + } + [recipient removeWithTransaction:transaction]; [[TSInfoMessage userNotRegisteredMessageInThread:thread] saveWithTransaction:transaction];