From fce52841f95acb4d2caa8948f4bdfff71831f0e3 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Tue, 14 Nov 2017 13:41:01 -0500 Subject: [PATCH 1/7] Don't resurrect zombies. --- .../Messages/Interactions/TSOutgoingMessage.h | 4 +- .../Messages/Interactions/TSOutgoingMessage.m | 18 ++---- .../src/Messages/OWSMessageSender.m | 57 +++++++++++++++++-- SignalServiceKit/src/Util/OWSError.h | 1 + 4 files changed, 61 insertions(+), 19 deletions(-) diff --git a/SignalServiceKit/src/Messages/Interactions/TSOutgoingMessage.h b/SignalServiceKit/src/Messages/Interactions/TSOutgoingMessage.h index e8920ad21..f53161dbe 100644 --- a/SignalServiceKit/src/Messages/Interactions/TSOutgoingMessage.h +++ b/SignalServiceKit/src/Messages/Interactions/TSOutgoingMessage.h @@ -173,7 +173,8 @@ typedef NS_ENUM(NSInteger, TSGroupMetaMessage) { - (void)updateWithMessageState:(TSOutgoingMessageState)messageState transaction:(YapDatabaseReadWriteTransaction *)transaction; - (void)updateWithSendingError:(NSError *)error; -- (void)updateWithHasSyncedTranscript:(BOOL)hasSyncedTranscript; +- (void)updateWithHasSyncedTranscript:(BOOL)hasSyncedTranscript + transaction:(YapDatabaseReadWriteTransaction *)transaction; - (void)updateWithCustomMessage:(NSString *)customMessage transaction:(YapDatabaseReadWriteTransaction *)transaction; - (void)updateWithCustomMessage:(NSString *)customMessage; // deliveryTimestamp is an optional parameter, since legacy @@ -196,7 +197,6 @@ typedef NS_ENUM(NSInteger, TSGroupMetaMessage) { - (NSUInteger)sentRecipientsCount; - (BOOL)wasSentToRecipient:(NSString *)contactId; - (void)updateWithSentRecipient:(NSString *)contactId transaction:(YapDatabaseReadWriteTransaction *)transaction; -- (void)updateWithSentRecipient:(NSString *)contactId; @end diff --git a/SignalServiceKit/src/Messages/Interactions/TSOutgoingMessage.m b/SignalServiceKit/src/Messages/Interactions/TSOutgoingMessage.m index 6fc0ca951..41e7c82a3 100644 --- a/SignalServiceKit/src/Messages/Interactions/TSOutgoingMessage.m +++ b/SignalServiceKit/src/Messages/Interactions/TSOutgoingMessage.m @@ -292,13 +292,12 @@ NSString *const kTSOutgoingMessageSentRecipientAll = @"kTSOutgoingMessageSentRec } - (void)updateWithHasSyncedTranscript:(BOOL)hasSyncedTranscript + transaction:(YapDatabaseReadWriteTransaction *)transaction { - [self.dbReadWriteConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { - [self applyChangeToSelfAndLatestOutgoingMessage:transaction - changeBlock:^(TSOutgoingMessage *message) { - [message setHasSyncedTranscript:hasSyncedTranscript]; - }]; - }]; + [self applyChangeToSelfAndLatestOutgoingMessage:transaction + changeBlock:^(TSOutgoingMessage *message) { + [message setHasSyncedTranscript:hasSyncedTranscript]; + }]; } - (void)updateWithCustomMessage:(NSString *)customMessage transaction:(YapDatabaseReadWriteTransaction *)transaction @@ -421,13 +420,6 @@ NSString *const kTSOutgoingMessageSentRecipientAll = @"kTSOutgoingMessageSentRec }]; } -- (void)updateWithSentRecipient:(NSString *)contactId -{ - [self.dbReadWriteConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { - [self updateWithSentRecipient:contactId transaction:transaction]; - }]; -} - - (void)updateWithReadRecipientId:(NSString *)recipientId readTimestamp:(uint64_t)readTimestamp transaction:(YapDatabaseReadWriteTransaction *)transaction diff --git a/SignalServiceKit/src/Messages/OWSMessageSender.m b/SignalServiceKit/src/Messages/OWSMessageSender.m index 6ce1a2c79..d45f7cc03 100644 --- a/SignalServiceKit/src/Messages/OWSMessageSender.m +++ b/SignalServiceKit/src/Messages/OWSMessageSender.m @@ -190,7 +190,12 @@ NSUInteger const OWSSendMessageOperationMaxRetries = 4; return; } - [message updateWithMessageState:TSOutgoingMessageStateSentToService]; + // Update the message unless it has been deleted. + if ([TSOutgoingMessage fetchObjectWithUniqueID:message.uniqueId]) { + [message updateWithMessageState:TSOutgoingMessageStateSentToService]; + } else { + DDLogInfo(@"%@ not marking message sent; message deleted.", strongSelf.logTag); + } aSuccessHandler(); @@ -204,7 +209,12 @@ NSUInteger const OWSSendMessageOperationMaxRetries = 4; return; } - [strongSelf.message updateWithSendingError:error]; + // Update the message unless it has been deleted. + if ([TSOutgoingMessage fetchObjectWithUniqueID:message.uniqueId]) { + [strongSelf.message updateWithSendingError:error]; + } else { + DDLogInfo(@"%@ not marking message failed; message deleted.", strongSelf.logTag); + } DDLogDebug(@"%@ failed with error: %@", strongSelf.logTag, error); aFailureHandler(error); @@ -280,6 +290,15 @@ NSUInteger const OWSSendMessageOperationMaxRetries = 4; - (void)tryWithRemainingRetries:(NSUInteger)remainingRetries { + // If the message has been deleted, abort send. + if (![TSOutgoingMessage fetchObjectWithUniqueID:self.message.uniqueId]) { + DDLogInfo(@"%@ aborting message send; message deleted.", self.logTag); + NSError *error = OWSErrorWithCodeDescription( + OWSErrorCodeMessageDeletedBeforeSent, @"Message was deleted before it could be sent."); + self.failureHandler(error); + return; + } + // Use this flag to ensure a given operation only succeeds or fails once. __block BOOL onceFlag = NO; RetryableFailureHandler retryableFailureHandler = ^(NSError *_Nonnull error) { @@ -429,6 +448,22 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException"; // // So we're using YDB behavior to ensure this invariant, which is a bit // unorthodox. + // Update the message unless it has been deleted. + // __block BOOL isMessageDeleted = NO; + // [self.dbConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { + // if ([TSOutgoingMessage fetchObjectWithUniqueID:message.uniqueId + // transaction:transaction]) { + // [message updateWithMessageState:TSOutgoingMessageStateAttemptingOut transaction:transaction]; + // } else { + // DDLogInfo(@"%@ not marking message as sending; message deleted.", self.logTag); + // isMessageDeleted = YES; + // } + // }]; + // if (isMessageDeleted) { + // NSError *error = OWSErrorWithCodeDescription(OWSErrorCodeMessageDeletedBeforeSent, @"Message was + // deleted before it could be sent."); failureHandler(error); return; + // } + DDLogError(@"------ TSOutgoingMessageStateAttemptingOut: %@ %@", message.debugDescription, message.description); [message updateWithMessageState:TSOutgoingMessageStateAttemptingOut]; OWSSendMessageOperation *sendMessageOperation = @@ -704,7 +739,14 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException"; attempts:OWSMessageSenderRetryAttempts success:^{ DDLogInfo(@"%@ Marking group message as sent to recipient: %@", self.logTag, recipient.uniqueId); - [message updateWithSentRecipient:recipient.uniqueId]; + [self.dbConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { + // Update the message unless it has been deleted. + if ([TSOutgoingMessage fetchObjectWithUniqueID:message.uniqueId]) { + [message updateWithSentRecipient:recipient.uniqueId transaction:transaction]; + } else { + DDLogInfo(@"%@ not marking message as sent to recipient; message deleted.", self.logTag); + } + }]; [futureSource trySetResult:@1]; } failure:^(NSError *error) { @@ -1146,7 +1188,14 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException"; if (message.shouldSyncTranscript) { // TODO: I suspect we shouldn't optimistically set hasSyncedTranscript. // We could set this in a success handler for [sendSyncTranscriptForMessage:]. - [message updateWithHasSyncedTranscript:YES]; + [self.dbConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { + // Update the message unless it has been deleted. + if ([TSOutgoingMessage fetchObjectWithUniqueID:message.uniqueId]) { + [message updateWithHasSyncedTranscript:YES transaction:transaction]; + } else { + DDLogInfo(@"%@ not marking message as having synced transcript; message deleted.", self.logTag); + } + }]; [self sendSyncTranscriptForMessage:message]; } diff --git a/SignalServiceKit/src/Util/OWSError.h b/SignalServiceKit/src/Util/OWSError.h index 3d70ba7b9..834497c88 100644 --- a/SignalServiceKit/src/Util/OWSError.h +++ b/SignalServiceKit/src/Util/OWSError.h @@ -26,6 +26,7 @@ typedef NS_ENUM(NSInteger, OWSErrorCode) { OWSErrorCodeMessageSendNoValidRecipients = 777407, OWSErrorCodeContactsUpdaterRateLimit = 777408, OWSErrorCodeCouldNotWriteAttachmentData = 777409, + OWSErrorCodeMessageDeletedBeforeSent = 777410, }; extern NSError *OWSErrorWithCodeDescription(OWSErrorCode code, NSString *description); From 69fa80b890ff889df6162ba8f54ac15aa2e41d16 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Tue, 14 Nov 2017 13:43:50 -0500 Subject: [PATCH 2/7] Don't resurrect zombies. --- SignalServiceKit/src/Messages/OWSMessageSender.m | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/SignalServiceKit/src/Messages/OWSMessageSender.m b/SignalServiceKit/src/Messages/OWSMessageSender.m index d45f7cc03..0241cdb5e 100644 --- a/SignalServiceKit/src/Messages/OWSMessageSender.m +++ b/SignalServiceKit/src/Messages/OWSMessageSender.m @@ -448,22 +448,6 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException"; // // So we're using YDB behavior to ensure this invariant, which is a bit // unorthodox. - // Update the message unless it has been deleted. - // __block BOOL isMessageDeleted = NO; - // [self.dbConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { - // if ([TSOutgoingMessage fetchObjectWithUniqueID:message.uniqueId - // transaction:transaction]) { - // [message updateWithMessageState:TSOutgoingMessageStateAttemptingOut transaction:transaction]; - // } else { - // DDLogInfo(@"%@ not marking message as sending; message deleted.", self.logTag); - // isMessageDeleted = YES; - // } - // }]; - // if (isMessageDeleted) { - // NSError *error = OWSErrorWithCodeDescription(OWSErrorCodeMessageDeletedBeforeSent, @"Message was - // deleted before it could be sent."); failureHandler(error); return; - // } - DDLogError(@"------ TSOutgoingMessageStateAttemptingOut: %@ %@", message.debugDescription, message.description); [message updateWithMessageState:TSOutgoingMessageStateAttemptingOut]; OWSSendMessageOperation *sendMessageOperation = From c6160a5a1e3bdf0c5de604394b87076299527ca0 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Wed, 15 Nov 2017 06:39:10 -0500 Subject: [PATCH 3/7] Rework the "update with..." methods to avoid re-saving deleted models. --- Signal/src/Jobs/SessionResetJob.swift | 4 +- Signal/src/MessageSender+Promise.swift | 2 +- .../src/MultiDeviceProfileKeyUpdateJob.swift | 4 +- Signal/src/Profiles/OWSProfileManager.m | 2 +- .../ConversationViewController.m | 6 +- .../ViewControllers/DebugUI/DebugUIMessages.m | 13 +- .../ViewControllers/DebugUI/DebugUIStress.m | 2 +- .../src/ViewControllers/HomeViewController.m | 2 +- .../ViewControllers/NewGroupViewController.m | 17 +-- .../OWSConversationSettingsViewController.m | 2 +- Signal/src/util/OWSContactsSyncing.m | 2 +- Signal/src/util/ThreadUtil.m | 4 +- SignalServiceKit/src/Contacts/TSThread.h | 13 +- SignalServiceKit/src/Contacts/TSThread.m | 15 +- .../src/Devices/OWSRecordTranscriptJob.m | 2 + .../Messages/Interactions/TSOutgoingMessage.h | 15 +- .../Messages/Interactions/TSOutgoingMessage.m | 108 +++++++------- .../src/Messages/OWSBlockingManager.m | 2 +- .../src/Messages/OWSIdentityManager.m | 4 +- .../src/Messages/OWSMessageManager.m | 12 +- .../src/Messages/OWSMessageSender.h | 32 ++--- .../src/Messages/OWSMessageSender.m | 135 ++++++++---------- ...oteOfUpdatedDisappearingConfigurationJob.m | 2 +- .../src/Messages/OWSReadReceiptManager.m | 6 +- 24 files changed, 196 insertions(+), 210 deletions(-) diff --git a/Signal/src/Jobs/SessionResetJob.swift b/Signal/src/Jobs/SessionResetJob.swift index 3ccd9fad8..935bd6e63 100644 --- a/Signal/src/Jobs/SessionResetJob.swift +++ b/Signal/src/Jobs/SessionResetJob.swift @@ -30,9 +30,9 @@ class SessionResetJob: NSObject { self.storageManager.deleteAllSessions(forContact: self.recipientId) DispatchQueue.main.async { - let endSessionMessage = EndSessionMessage(timestamp:NSDate.ows_millisecondTimeStamp(), in: self.thread) + let endSessionMessage = EndSessionMessage(timestamp: NSDate.ows_millisecondTimeStamp(), in: self.thread) - self.messageSender.send(endSessionMessage, success: { + self.messageSender.enqueue(endSessionMessage, success: { Logger.info("\(self.TAG) successfully sent EndSession Promise { let promise: Promise = Promise { fulfill, reject in - self.send(message, success: fulfill, failure: reject) + self.enqueue(message, success: fulfill, failure: reject) } // Ensure sends complete before they're GC'd. diff --git a/Signal/src/MultiDeviceProfileKeyUpdateJob.swift b/Signal/src/MultiDeviceProfileKeyUpdateJob.swift index 79a2f9221..a48ff8a36 100644 --- a/Signal/src/MultiDeviceProfileKeyUpdateJob.swift +++ b/Signal/src/MultiDeviceProfileKeyUpdateJob.swift @@ -43,8 +43,8 @@ import PromiseKit identityManager: self.identityManager, profileManager: self.profileManager) - let dataSource = DataSourceValue.dataSource(withSyncMessage:syncContactsMessage.buildPlainTextAttachmentData()) - self.messageSender.sendTemporaryAttachmentData(dataSource, + let dataSource = DataSourceValue.dataSource(withSyncMessage: syncContactsMessage.buildPlainTextAttachmentData()) + self.messageSender.enqueueOutgoingTemporaryAttachment(dataSource, contentType: OWSMimeTypeApplicationOctetStream, in: syncContactsMessage, success: { diff --git a/Signal/src/Profiles/OWSProfileManager.m b/Signal/src/Profiles/OWSProfileManager.m index 311cc471e..c2aa31baa 100644 --- a/Signal/src/Profiles/OWSProfileManager.m +++ b/Signal/src/Profiles/OWSProfileManager.m @@ -1465,7 +1465,7 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640; return; } - [self.messageSender sendMessage:message + [self.messageSender enqueueOutgoingMessage:message success:^{ DDLogInfo(@"%@ Successfully sent profile key message to thread: %@", self.logTag, thread); [OWSProfileManager.sharedManager addThreadToProfileWhitelist:thread]; diff --git a/Signal/src/ViewControllers/ConversationView/ConversationViewController.m b/Signal/src/ViewControllers/ConversationView/ConversationViewController.m index 42996a9ef..d1425ec6c 100644 --- a/Signal/src/ViewControllers/ConversationView/ConversationViewController.m +++ b/Signal/src/ViewControllers/ConversationView/ConversationViewController.m @@ -1654,7 +1654,7 @@ typedef NS_ENUM(NSInteger, MessagesRangeSizeMode) { [UIAlertAction actionWithTitle:NSLocalizedString(@"SEND_AGAIN_BUTTON", @"") style:UIAlertActionStyleDefault handler:^(UIAlertAction *_Nonnull action) { - [self.messageSender sendMessage:message + [self.messageSender enqueueOutgoingMessage:message success:^{ DDLogInfo(@"%@ Successfully resent failed message.", self.logTag); } @@ -3391,7 +3391,7 @@ typedef NS_ENUM(NSInteger, MessagesRangeSizeMode) { if (newGroupModel.groupImage) { NSData *data = UIImagePNGRepresentation(newGroupModel.groupImage); DataSource *_Nullable dataSource = [DataSourceValue dataSourceWithData:data fileExtension:@"png"]; - [self.messageSender sendAttachmentData:dataSource + [self.messageSender enqueueOutgoingAttachment:dataSource contentType:OWSMimeTypeImagePng sourceFilename:nil inMessage:message @@ -3405,7 +3405,7 @@ typedef NS_ENUM(NSInteger, MessagesRangeSizeMode) { DDLogError(@"%@ Failed to send group avatar update with error: %@", self.logTag, error); }]; } else { - [self.messageSender sendMessage:message + [self.messageSender enqueueOutgoingMessage:message success:^{ DDLogDebug(@"%@ Successfully sent group update", self.logTag); if (successCompletion) { diff --git a/Signal/src/ViewControllers/DebugUI/DebugUIMessages.m b/Signal/src/ViewControllers/DebugUI/DebugUIMessages.m index 296acb861..b70a2278e 100644 --- a/Signal/src/ViewControllers/DebugUI/DebugUIMessages.m +++ b/Signal/src/ViewControllers/DebugUI/DebugUIMessages.m @@ -219,7 +219,7 @@ NS_ASSUME_NONNULL_BEGIN OWSSyncGroupsRequestMessage *syncGroupsRequestMessage = [[OWSSyncGroupsRequestMessage alloc] initWithThread:thread groupId:[Randomness generateRandomBytes:16]]; - [[Environment getCurrent].messageSender sendMessage:syncGroupsRequestMessage + [[Environment getCurrent].messageSender enqueueOutgoingMessage:syncGroupsRequestMessage success:^{ DDLogWarn(@"%@ Successfully sent Request Group Info message.", self.logTag); } @@ -1055,7 +1055,6 @@ NS_ASSUME_NONNULL_BEGIN TSOutgoingMessage *message = [[TSOutgoingMessage alloc] initWithTimestamp:[NSDate ows_millisecondTimeStamp] inThread:thread groupMetaMessage:TSGroupMessageNew]; - // This will save the message. [message updateWithCustomMessage:NSLocalizedString(@"GROUP_CREATED", nil)]; OWSMessageSender *messageSender = [Environment getCurrent].messageSender; @@ -1067,11 +1066,11 @@ NS_ASSUME_NONNULL_BEGIN }); }); }; - [messageSender sendMessage:message - success:completion - failure:^(NSError *error) { - completion(); - }]; + [messageSender enqueueOutgoingMessage:message + success:completion + failure:^(NSError *error) { + completion(); + }]; } + (void)injectFakeIncomingMessages:(int)counter thread:(TSThread *)thread diff --git a/Signal/src/ViewControllers/DebugUI/DebugUIStress.m b/Signal/src/ViewControllers/DebugUI/DebugUIStress.m index 006b49dec..68284d7ca 100644 --- a/Signal/src/ViewControllers/DebugUI/DebugUIStress.m +++ b/Signal/src/ViewControllers/DebugUI/DebugUIStress.m @@ -437,7 +437,7 @@ NS_ASSUME_NONNULL_BEGIN OWSAssert(message); OWSMessageSender *messageSender = [Environment getCurrent].messageSender; - [messageSender sendMessage:message + [messageSender enqueueOutgoingMessage:message success:^{ DDLogInfo(@"%@ Successfully sent message.", self.logTag); } diff --git a/Signal/src/ViewControllers/HomeViewController.m b/Signal/src/ViewControllers/HomeViewController.m index ff1a66587..a05ccabaf 100644 --- a/Signal/src/ViewControllers/HomeViewController.m +++ b/Signal/src/ViewControllers/HomeViewController.m @@ -679,7 +679,7 @@ typedef NS_ENUM(NSInteger, CellState) { kArchiveState, kInboxState }; TSOutgoingMessage *message = [[TSOutgoingMessage alloc] initWithTimestamp:[NSDate ows_millisecondTimeStamp] inThread:thread groupMetaMessage:TSGroupMessageQuit]; - [self.messageSender sendMessage:message + [self.messageSender enqueueOutgoingMessage:message success:^{ [self dismissViewControllerAnimated:YES completion:^{ diff --git a/Signal/src/ViewControllers/NewGroupViewController.m b/Signal/src/ViewControllers/NewGroupViewController.m index 91af1da90..016861e6c 100644 --- a/Signal/src/ViewControllers/NewGroupViewController.m +++ b/Signal/src/ViewControllers/NewGroupViewController.m @@ -493,7 +493,6 @@ const NSUInteger kNewGroupViewControllerAvatarWidth = 68; inThread:thread groupMetaMessage:TSGroupMessageNew]; - // This will save the message. [message updateWithCustomMessage:NSLocalizedString(@"GROUP_CREATED", nil)]; dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{ @@ -501,14 +500,16 @@ const NSUInteger kNewGroupViewControllerAvatarWidth = 68; NSData *data = UIImagePNGRepresentation(model.groupImage); DataSource *_Nullable dataSource = [DataSourceValue dataSourceWithData:data fileExtension:@"png"]; - [self.messageSender sendAttachmentData:dataSource - contentType:OWSMimeTypeImagePng - sourceFilename:nil - inMessage:message - success:successHandler - failure:failureHandler]; + [self.messageSender enqueueOutgoingAttachment:dataSource + contentType:OWSMimeTypeImagePng + sourceFilename:nil + inMessage:message + success:successHandler + failure:failureHandler]; } else { - [self.messageSender sendMessage:message success:successHandler failure:failureHandler]; + [self.messageSender enqueueOutgoingMessage:message + success:successHandler + failure:failureHandler]; } }); }]; diff --git a/Signal/src/ViewControllers/OWSConversationSettingsViewController.m b/Signal/src/ViewControllers/OWSConversationSettingsViewController.m index 7188e8a29..29e732a43 100644 --- a/Signal/src/ViewControllers/OWSConversationSettingsViewController.m +++ b/Signal/src/ViewControllers/OWSConversationSettingsViewController.m @@ -885,7 +885,7 @@ NS_ASSUME_NONNULL_BEGIN TSOutgoingMessage *message = [[TSOutgoingMessage alloc] initWithTimestamp:[NSDate ows_millisecondTimeStamp] inThread:gThread groupMetaMessage:TSGroupMessageQuit]; - [self.messageSender sendMessage:message + [self.messageSender enqueueOutgoingMessage:message success:^{ DDLogInfo(@"%@ Successfully left group.", self.logTag); } diff --git a/Signal/src/util/OWSContactsSyncing.m b/Signal/src/util/OWSContactsSyncing.m index c3a1448f5..80dce55ff 100644 --- a/Signal/src/util/OWSContactsSyncing.m +++ b/Signal/src/util/OWSContactsSyncing.m @@ -115,7 +115,7 @@ NSString *const kTSStorageManagerOWSContactsSyncingLastMessageKey = DataSource *dataSource = [DataSourceValue dataSourceWithSyncMessage:[syncContactsMessage buildPlainTextAttachmentData]]; - [self.messageSender sendTemporaryAttachmentData:dataSource + [self.messageSender enqueueOutgoingTemporaryAttachment:dataSource contentType:OWSMimeTypeApplicationOctetStream inMessage:syncContactsMessage success:^{ diff --git a/Signal/src/util/ThreadUtil.m b/Signal/src/util/ThreadUtil.m index 16656e121..8cd63f65f 100644 --- a/Signal/src/util/ThreadUtil.m +++ b/Signal/src/util/ThreadUtil.m @@ -86,7 +86,7 @@ NS_ASSUME_NONNULL_BEGIN attachmentIds:[NSMutableArray new] expiresInSeconds:(configuration.isEnabled ? configuration.durationSeconds : 0)]; - [messageSender sendMessage:message success:successHandler failure:failureHandler]; + [messageSender enqueueOutgoingMessage:message success:successHandler failure:failureHandler]; return message; } @@ -117,7 +117,7 @@ NS_ASSUME_NONNULL_BEGIN inThread:thread isVoiceMessage:[attachment isVoiceMessage] expiresInSeconds:(configuration.isEnabled ? configuration.durationSeconds : 0)]; - [messageSender sendAttachmentData:attachment.dataSource + [messageSender enqueueOutgoingAttachment:attachment.dataSource contentType:attachment.mimeType sourceFilename:attachment.filenameOrDefault inMessage:message diff --git a/SignalServiceKit/src/Contacts/TSThread.h b/SignalServiceKit/src/Contacts/TSThread.h index fd9bcf55e..4ed06aa29 100644 --- a/SignalServiceKit/src/Contacts/TSThread.h +++ b/SignalServiceKit/src/Contacts/TSThread.h @@ -157,18 +157,19 @@ NS_ASSUME_NONNULL_BEGIN @property (atomic, readonly, nullable) NSDate *mutedUntilDate; // This model may be updated from many threads. We don't want to save -// our local copy (this instance) since it may be out of date. Instead, we +// our local copy (this instance) since it may be out of date. We also +// want to avoid re-saving a model that has been deleted. Therefore, we // use these "updateWith..." methods to: // // a) Update a property of this instance. -// b) Load an up-to-date instance of this model from from the data store. -// c) Update and save that fresh instance. -// d) If this instance hasn't yet been saved, save this local instance. +// b) If a copy of this model exists in the database, load an up-to-date copy, +// and update and save that copy. +// b) If a copy of this model _DOES NOT_ exist in the database, do _NOT_ save +// this local instance. // // After "updateWith...": // -// a) An updated copy of this instance will always have been saved in the -// data store. +// a) An copy of this model in the database will have been updated. // b) The local property on this instance will always have been updated. // c) Other properties on this instance may be out of date. // diff --git a/SignalServiceKit/src/Contacts/TSThread.m b/SignalServiceKit/src/Contacts/TSThread.m index 0364b952b..fe7d2ab22 100644 --- a/SignalServiceKit/src/Contacts/TSThread.m +++ b/SignalServiceKit/src/Contacts/TSThread.m @@ -384,8 +384,8 @@ NS_ASSUME_NONNULL_BEGIN // This method does the work for the "updateWith..." methods. Please see // the header for a discussion of those methods. -- (void)applyChangeToSelfAndLatestThread:(YapDatabaseReadWriteTransaction *)transaction - changeBlock:(void (^)(TSThread *))changeBlock +- (void)applyChangeToSelfAndLatestCopy:(YapDatabaseReadWriteTransaction *)transaction + changeBlock:(void (^)(TSThread *))changeBlock { OWSAssert(transaction); @@ -396,19 +396,16 @@ NS_ASSUME_NONNULL_BEGIN if (latestInstance) { changeBlock(latestInstance); [latestInstance saveWithTransaction:transaction]; - } else { - // This message has not yet been saved. - [self saveWithTransaction:transaction]; } } - (void)updateWithMutedUntilDate:(NSDate *)mutedUntilDate { [self.dbReadWriteConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { - [self applyChangeToSelfAndLatestThread:transaction - changeBlock:^(TSThread *thread) { - [thread setMutedUntilDate:mutedUntilDate]; - }]; + [self applyChangeToSelfAndLatestCopy:transaction + changeBlock:^(TSThread *thread) { + [thread setMutedUntilDate:mutedUntilDate]; + }]; }]; } diff --git a/SignalServiceKit/src/Devices/OWSRecordTranscriptJob.m b/SignalServiceKit/src/Devices/OWSRecordTranscriptJob.m index 7bbde72e4..edfe82342 100644 --- a/SignalServiceKit/src/Devices/OWSRecordTranscriptJob.m +++ b/SignalServiceKit/src/Devices/OWSRecordTranscriptJob.m @@ -109,6 +109,7 @@ NS_ASSUME_NONNULL_BEGIN return; } + [outgoingMessage saveWithTransaction:transaction]; [outgoingMessage updateWithWasSentFromLinkedDeviceWithTransaction:transaction]; [OWSDisappearingMessagesJob becomeConsistentWithConfigurationForMessage:outgoingMessage contactsManager:self.contactsManager]; @@ -138,6 +139,7 @@ NS_ASSUME_NONNULL_BEGIN expiresInSeconds:transcript.expirationDuration expireStartedAt:transcript.expirationStartedAt]; // Since textMessage is a new message, updateWithWasSentAndDelivered will save it. + [textMessage saveWithTransaction:transaction]; [textMessage updateWithWasSentFromLinkedDeviceWithTransaction:transaction]; } } diff --git a/SignalServiceKit/src/Messages/Interactions/TSOutgoingMessage.h b/SignalServiceKit/src/Messages/Interactions/TSOutgoingMessage.h index f53161dbe..abb39aa49 100644 --- a/SignalServiceKit/src/Messages/Interactions/TSOutgoingMessage.h +++ b/SignalServiceKit/src/Messages/Interactions/TSOutgoingMessage.h @@ -148,19 +148,20 @@ typedef NS_ENUM(NSInteger, TSGroupMetaMessage) { - (OWSSignalServiceProtosAttachmentPointer *)buildAttachmentProtoForAttachmentId:(NSString *)attachmentId filename:(nullable NSString *)filename; -// TSOutgoingMessage are updated from many threads. We don't want to save -// our local copy (this instance) since it may be out of date. Instead, we +// This model may be updated from many threads. We don't want to save +// our local copy (this instance) since it may be out of date. We also +// want to avoid re-saving a model that has been deleted. Therefore, we // use these "updateWith..." methods to: // // a) Update a property of this instance. -// b) Load an up-to-date instance of this model from from the data store. -// c) Update and save that fresh instance. -// d) If this message hasn't yet been saved, save this local instance. +// b) If a copy of this model exists in the database, load an up-to-date copy, +// and update and save that copy. +// b) If a copy of this model _DOES NOT_ exist in the database, do _NOT_ save +// this local instance. // // After "updateWith...": // -// a) An updated copy of this message will always have been saved in the -// data store. +// a) An copy of this model in the database will have been updated. // b) The local property on this instance will always have been updated. // c) Other properties on this instance may be out of date. // diff --git a/SignalServiceKit/src/Messages/Interactions/TSOutgoingMessage.m b/SignalServiceKit/src/Messages/Interactions/TSOutgoingMessage.m index 41e7c82a3..79c0aea67 100644 --- a/SignalServiceKit/src/Messages/Interactions/TSOutgoingMessage.m +++ b/SignalServiceKit/src/Messages/Interactions/TSOutgoingMessage.m @@ -242,8 +242,8 @@ NSString *const kTSOutgoingMessageSentRecipientAll = @"kTSOutgoingMessageSentRec // This method does the work for the "updateWith..." methods. Please see // the header for a discussion of those methods. -- (void)applyChangeToSelfAndLatestOutgoingMessage:(YapDatabaseReadWriteTransaction *)transaction - changeBlock:(void (^)(TSOutgoingMessage *))changeBlock +- (void)applyChangeToSelfAndLatestCopy:(YapDatabaseReadWriteTransaction *)transaction + changeBlock:(void (^)(TSOutgoingMessage *))changeBlock { OWSAssert(transaction); @@ -254,9 +254,6 @@ NSString *const kTSOutgoingMessageSentRecipientAll = @"kTSOutgoingMessageSentRec if (latestMessage) { changeBlock(latestMessage); [latestMessage saveWithTransaction:transaction]; - } else { - // This message has not yet been saved. - [self saveWithTransaction:transaction]; } } @@ -265,11 +262,11 @@ NSString *const kTSOutgoingMessageSentRecipientAll = @"kTSOutgoingMessageSentRec OWSAssert(error); [self.dbReadWriteConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { - [self applyChangeToSelfAndLatestOutgoingMessage:transaction - changeBlock:^(TSOutgoingMessage *message) { - [message setMessageState:TSOutgoingMessageStateUnsent]; - [message setMostRecentFailureText:error.localizedDescription]; - }]; + [self applyChangeToSelfAndLatestCopy:transaction + changeBlock:^(TSOutgoingMessage *message) { + [message setMessageState:TSOutgoingMessageStateUnsent]; + [message setMostRecentFailureText:error.localizedDescription]; + }]; }]; } @@ -285,19 +282,19 @@ NSString *const kTSOutgoingMessageSentRecipientAll = @"kTSOutgoingMessageSentRec { OWSAssert(transaction); - [self applyChangeToSelfAndLatestOutgoingMessage:transaction - changeBlock:^(TSOutgoingMessage *message) { - [message setMessageState:messageState]; - }]; + [self applyChangeToSelfAndLatestCopy:transaction + changeBlock:^(TSOutgoingMessage *message) { + [message setMessageState:messageState]; + }]; } - (void)updateWithHasSyncedTranscript:(BOOL)hasSyncedTranscript transaction:(YapDatabaseReadWriteTransaction *)transaction { - [self applyChangeToSelfAndLatestOutgoingMessage:transaction - changeBlock:^(TSOutgoingMessage *message) { - [message setHasSyncedTranscript:hasSyncedTranscript]; - }]; + [self applyChangeToSelfAndLatestCopy:transaction + changeBlock:^(TSOutgoingMessage *message) { + [message setHasSyncedTranscript:hasSyncedTranscript]; + }]; } - (void)updateWithCustomMessage:(NSString *)customMessage transaction:(YapDatabaseReadWriteTransaction *)transaction @@ -305,10 +302,10 @@ NSString *const kTSOutgoingMessageSentRecipientAll = @"kTSOutgoingMessageSentRec OWSAssert(customMessage); OWSAssert(transaction); - [self applyChangeToSelfAndLatestOutgoingMessage:transaction - changeBlock:^(TSOutgoingMessage *message) { - [message setCustomMessage:customMessage]; - }]; + [self applyChangeToSelfAndLatestCopy:transaction + changeBlock:^(TSOutgoingMessage *message) { + [message setCustomMessage:customMessage]; + }]; } - (void)updateWithCustomMessage:(NSString *)customMessage @@ -325,32 +322,31 @@ NSString *const kTSOutgoingMessageSentRecipientAll = @"kTSOutgoingMessageSentRec OWSAssert(recipientId.length > 0); OWSAssert(transaction); - [self applyChangeToSelfAndLatestOutgoingMessage:transaction - changeBlock:^(TSOutgoingMessage *message) { + [self applyChangeToSelfAndLatestCopy:transaction + changeBlock:^(TSOutgoingMessage *message) { - if (deliveryTimestamp) { - NSMutableDictionary *recipientDeliveryMap - = (message.recipientDeliveryMap - ? [message.recipientDeliveryMap mutableCopy] - : [NSMutableDictionary new]); - recipientDeliveryMap[recipientId] = deliveryTimestamp; - message.recipientDeliveryMap = [recipientDeliveryMap copy]; - } + if (deliveryTimestamp) { + NSMutableDictionary *recipientDeliveryMap + = (message.recipientDeliveryMap ? [message.recipientDeliveryMap mutableCopy] + : [NSMutableDictionary new]); + recipientDeliveryMap[recipientId] = deliveryTimestamp; + message.recipientDeliveryMap = [recipientDeliveryMap copy]; + } - [message setWasDelivered:YES]; - }]; + [message setWasDelivered:YES]; + }]; } - (void)updateWithWasSentFromLinkedDeviceWithTransaction:(YapDatabaseReadWriteTransaction *)transaction { OWSAssert(transaction); - [self applyChangeToSelfAndLatestOutgoingMessage:transaction - changeBlock:^(TSOutgoingMessage *message) { - [message setMessageState:TSOutgoingMessageStateSentToService]; - [message setWasDelivered:YES]; - [message setIsFromLinkedDevice:YES]; - }]; + [self applyChangeToSelfAndLatestCopy:transaction + changeBlock:^(TSOutgoingMessage *message) { + [message setMessageState:TSOutgoingMessageStateSentToService]; + [message setWasDelivered:YES]; + [message setIsFromLinkedDevice:YES]; + }]; } - (void)updateWithSingleGroupRecipient:(NSString *)singleGroupRecipient @@ -359,10 +355,10 @@ NSString *const kTSOutgoingMessageSentRecipientAll = @"kTSOutgoingMessageSentRec OWSAssert(transaction); OWSAssert(singleGroupRecipient.length > 0); - [self applyChangeToSelfAndLatestOutgoingMessage:transaction - changeBlock:^(TSOutgoingMessage *message) { - [message setSingleGroupRecipient:singleGroupRecipient]; - }]; + [self applyChangeToSelfAndLatestCopy:transaction + changeBlock:^(TSOutgoingMessage *message) { + [message setSingleGroupRecipient:singleGroupRecipient]; + }]; } #pragma mark - Sent Recipients @@ -414,10 +410,10 @@ NSString *const kTSOutgoingMessageSentRecipientAll = @"kTSOutgoingMessageSentRec - (void)updateWithSentRecipient:(NSString *)contactId transaction:(YapDatabaseReadWriteTransaction *)transaction { OWSAssert(transaction); - [self applyChangeToSelfAndLatestOutgoingMessage:transaction - changeBlock:^(TSOutgoingMessage *message) { - [message addSentRecipient:contactId]; - }]; + [self applyChangeToSelfAndLatestCopy:transaction + changeBlock:^(TSOutgoingMessage *message) { + [message addSentRecipient:contactId]; + }]; } - (void)updateWithReadRecipientId:(NSString *)recipientId @@ -427,14 +423,14 @@ NSString *const kTSOutgoingMessageSentRecipientAll = @"kTSOutgoingMessageSentRec OWSAssert(recipientId.length > 0); OWSAssert(transaction); - [self applyChangeToSelfAndLatestOutgoingMessage:transaction - changeBlock:^(TSOutgoingMessage *message) { - NSMutableDictionary *recipientReadMap - = (message.recipientReadMap ? [message.recipientReadMap mutableCopy] - : [NSMutableDictionary new]); - recipientReadMap[recipientId] = @(readTimestamp); - message.recipientReadMap = [recipientReadMap copy]; - }]; + [self applyChangeToSelfAndLatestCopy:transaction + changeBlock:^(TSOutgoingMessage *message) { + NSMutableDictionary *recipientReadMap + = (message.recipientReadMap ? [message.recipientReadMap mutableCopy] + : [NSMutableDictionary new]); + recipientReadMap[recipientId] = @(readTimestamp); + message.recipientReadMap = [recipientReadMap copy]; + }]; } - (nullable NSNumber *)firstRecipientReadTimestamp diff --git a/SignalServiceKit/src/Messages/OWSBlockingManager.m b/SignalServiceKit/src/Messages/OWSBlockingManager.m index bb419ed70..c09bb10b5 100644 --- a/SignalServiceKit/src/Messages/OWSBlockingManager.m +++ b/SignalServiceKit/src/Messages/OWSBlockingManager.m @@ -258,7 +258,7 @@ NSString *const kOWSBlockingManager_SyncedBlockedPhoneNumbersKey = @"kOWSBlockin OWSBlockedPhoneNumbersMessage *message = [[OWSBlockedPhoneNumbersMessage alloc] initWithPhoneNumbers:blockedPhoneNumbers]; - [self.messageSender sendMessage:message + [self.messageSender enqueueOutgoingMessage:message success:^{ DDLogInfo(@"%@ Successfully sent blocked phone numbers sync message", self.logTag); diff --git a/SignalServiceKit/src/Messages/OWSIdentityManager.m b/SignalServiceKit/src/Messages/OWSIdentityManager.m index 0bf98e61e..1ead01406 100644 --- a/SignalServiceKit/src/Messages/OWSIdentityManager.m +++ b/SignalServiceKit/src/Messages/OWSIdentityManager.m @@ -517,10 +517,10 @@ NSString *const kNSNotificationName_IdentityStateDidChange = @"kNSNotificationNa // subsequently OWSOutgoingNullMessage *nullMessage = [[OWSOutgoingNullMessage alloc] initWithContactThread:contactThread verificationStateSyncMessage:message]; - [self.messageSender sendMessage:nullMessage + [self.messageSender enqueueOutgoingMessage:nullMessage success:^{ DDLogInfo(@"%@ Successfully sent verification state NullMessage", self.logTag); - [self.messageSender sendMessage:message + [self.messageSender enqueueOutgoingMessage:message success:^{ DDLogInfo(@"%@ Successfully sent verification state sync message", self.logTag); diff --git a/SignalServiceKit/src/Messages/OWSMessageManager.m b/SignalServiceKit/src/Messages/OWSMessageManager.m index 64506c558..1ca4b9366 100644 --- a/SignalServiceKit/src/Messages/OWSMessageManager.m +++ b/SignalServiceKit/src/Messages/OWSMessageManager.m @@ -366,7 +366,7 @@ NS_ASSUME_NONNULL_BEGIN OWSSyncGroupsRequestMessage *syncGroupsRequestMessage = [[OWSSyncGroupsRequestMessage alloc] initWithThread:thread groupId:groupId]; - [self.messageSender sendMessage:syncGroupsRequestMessage + [self.messageSender enqueueOutgoingMessage:syncGroupsRequestMessage success:^{ DDLogWarn(@"%@ Successfully sent Request Group Info message.", self.logTag); } @@ -609,7 +609,7 @@ NS_ASSUME_NONNULL_BEGIN profileManager:self.profileManager]; DataSource *dataSource = [DataSourceValue dataSourceWithSyncMessage:[syncContactsMessage buildPlainTextAttachmentData]]; - [self.messageSender sendTemporaryAttachmentData:dataSource + [self.messageSender enqueueOutgoingTemporaryAttachment:dataSource contentType:OWSMimeTypeApplicationOctetStream inMessage:syncContactsMessage success:^{ @@ -622,7 +622,7 @@ NS_ASSUME_NONNULL_BEGIN OWSSyncGroupsMessage *syncGroupsMessage = [[OWSSyncGroupsMessage alloc] init]; DataSource *dataSource = [DataSourceValue dataSourceWithSyncMessage:[syncGroupsMessage buildPlainTextAttachmentDataWithTransaction:transaction]]; - [self.messageSender sendTemporaryAttachmentData:dataSource + [self.messageSender enqueueOutgoingTemporaryAttachment:dataSource contentType:OWSMimeTypeApplicationOctetStream inMessage:syncGroupsMessage success:^{ @@ -639,7 +639,7 @@ NS_ASSUME_NONNULL_BEGIN [[OWSReadReceiptManager sharedManager] areReadReceiptsEnabledWithTransaction:transaction]; OWSSyncConfigurationMessage *syncConfigurationMessage = [[OWSSyncConfigurationMessage alloc] initWithReadReceiptsEnabled:areReadReceiptsEnabled]; - [self.messageSender sendMessage:syncConfigurationMessage + [self.messageSender enqueueOutgoingMessage:syncConfigurationMessage success:^{ DDLogInfo(@"%@ Successfully sent Configuration response syncMessage.", self.logTag); } @@ -774,7 +774,7 @@ NS_ASSUME_NONNULL_BEGIN if (gThread.groupModel.groupImage) { NSData *data = UIImagePNGRepresentation(gThread.groupModel.groupImage); DataSource *_Nullable dataSource = [DataSourceValue dataSourceWithData:data fileExtension:@"png"]; - [self.messageSender sendAttachmentData:dataSource + [self.messageSender enqueueOutgoingAttachment:dataSource contentType:OWSMimeTypeImagePng sourceFilename:nil inMessage:message @@ -785,7 +785,7 @@ NS_ASSUME_NONNULL_BEGIN DDLogError(@"%@ Failed to send group avatar update with error: %@", self.logTag, error); }]; } else { - [self.messageSender sendMessage:message + [self.messageSender enqueueOutgoingMessage:message success:^{ DDLogDebug(@"%@ Successfully sent group update", self.logTag); } diff --git a/SignalServiceKit/src/Messages/OWSMessageSender.h b/SignalServiceKit/src/Messages/OWSMessageSender.h index 995490d6c..df17fa883 100644 --- a/SignalServiceKit/src/Messages/OWSMessageSender.h +++ b/SignalServiceKit/src/Messages/OWSMessageSender.h @@ -63,33 +63,33 @@ NS_SWIFT_NAME(MessageSender) /** * Send and resend text messages or resend messages with existing attachments. - * If you haven't yet created the attachment, see the `sendAttachmentData:` variants. + * If you haven't yet created the attachment, see the ` enqueueOutgoingAttachment:` variants. */ // TODO: make transaction nonnull and remove `sendMessage:success:failure` -- (void)sendMessage:(TSOutgoingMessage *)message - success:(void (^)(void))successHandler - failure:(void (^)(NSError *error))failureHandler; +- (void)enqueueOutgoingMessage:(TSOutgoingMessage *)message + success:(void (^)(void))successHandler + failure:(void (^)(NSError *error))failureHandler; /** * Takes care of allocating and uploading the attachment, then sends the message. * Only necessary to call once. If sending fails, retry with `sendMessage:`. */ -- (void)sendAttachmentData:(DataSource *)dataSource - contentType:(NSString *)contentType - sourceFilename:(nullable NSString *)sourceFilename - inMessage:(TSOutgoingMessage *)outgoingMessage - success:(void (^)(void))successHandler - failure:(void (^)(NSError *error))failureHandler; +- (void)enqueueOutgoingAttachment:(DataSource *)dataSource + contentType:(NSString *)contentType + sourceFilename:(nullable NSString *)sourceFilename + inMessage:(TSOutgoingMessage *)outgoingMessage + success:(void (^)(void))successHandler + failure:(void (^)(NSError *error))failureHandler; /** - * Same as `sendAttachmentData:`, but deletes the local copy of the attachment after sending. + * Same as ` enqueueOutgoingAttachment:`, but deletes the local copy of the attachment after sending. * Used for sending sync request data, not for user visible attachments. */ -- (void)sendTemporaryAttachmentData:(DataSource *)dataSource - contentType:(NSString *)contentType - inMessage:(TSOutgoingMessage *)outgoingMessage - success:(void (^)(void))successHandler - failure:(void (^)(NSError *error))failureHandler; +- (void)enqueueOutgoingTemporaryAttachment:(DataSource *)dataSource + contentType:(NSString *)contentType + inMessage:(TSOutgoingMessage *)outgoingMessage + success:(void (^)(void))successHandler + failure:(void (^)(NSError *error))failureHandler; /** * Set local configuration to match that of the of `outgoingMessage`'s sender diff --git a/SignalServiceKit/src/Messages/OWSMessageSender.m b/SignalServiceKit/src/Messages/OWSMessageSender.m index 0241cdb5e..82ffe293c 100644 --- a/SignalServiceKit/src/Messages/OWSMessageSender.m +++ b/SignalServiceKit/src/Messages/OWSMessageSender.m @@ -155,7 +155,7 @@ NSUInteger const OWSSendMessageOperationMaxRetries = 4; @property (nonatomic, readonly) TSOutgoingMessage *message; @property (nonatomic, readonly) OWSMessageSender *messageSender; -@property (nonatomic, readonly) void (^successHandler)(); +@property (nonatomic, readonly) void (^successHandler)(void); @property (nonatomic, readonly) void (^failureHandler)(NSError *_Nonnull error); @property (nonatomic) OWSSendMessageOperationState operationState; @property (nonatomic) UIBackgroundTaskIdentifier backgroundTaskIdentifier; @@ -190,12 +190,7 @@ NSUInteger const OWSSendMessageOperationMaxRetries = 4; return; } - // Update the message unless it has been deleted. - if ([TSOutgoingMessage fetchObjectWithUniqueID:message.uniqueId]) { - [message updateWithMessageState:TSOutgoingMessageStateSentToService]; - } else { - DDLogInfo(@"%@ not marking message sent; message deleted.", strongSelf.logTag); - } + [message updateWithMessageState:TSOutgoingMessageStateSentToService]; aSuccessHandler(); @@ -209,12 +204,7 @@ NSUInteger const OWSSendMessageOperationMaxRetries = 4; return; } - // Update the message unless it has been deleted. - if ([TSOutgoingMessage fetchObjectWithUniqueID:message.uniqueId]) { - [strongSelf.message updateWithSendingError:error]; - } else { - DDLogInfo(@"%@ not marking message failed; message deleted.", strongSelf.logTag); - } + [strongSelf.message updateWithSendingError:error]; DDLogDebug(@"%@ failed with error: %@", strongSelf.logTag, error); aFailureHandler(error); @@ -429,9 +419,9 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException"; } } -- (void)sendMessage:(TSOutgoingMessage *)message - success:(void (^)(void))successHandler - failure:(void (^)(NSError *error))failureHandler +- (void)enqueueOutgoingMessage:(TSOutgoingMessage *)message + success:(void (^)(void))successHandler + failure:(void (^)(NSError *error))failureHandler { OWSAssert(message); @@ -448,7 +438,11 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException"; // // So we're using YDB behavior to ensure this invariant, which is a bit // unorthodox. - [message updateWithMessageState:TSOutgoingMessageStateAttemptingOut]; + [self.dbConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { + // All outgoing messages should be saved at the time they are enqueued. + [message saveWithTransaction:transaction]; + [message updateWithMessageState:TSOutgoingMessageStateAttemptingOut transaction:transaction]; + }]; OWSSendMessageOperation *sendMessageOperation = [[OWSSendMessageOperation alloc] initWithMessage:message @@ -476,12 +470,13 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException"; { [self ensureAnyAttachmentsUploaded:message success:^() { - [self deliverMessage:message - success:successHandler - failure:^(NSError *error) { - DDLogDebug(@"%@ Message send attempt failed: %@", self.logTag, message.debugDescription); - failureHandler(error); - }]; + [self sendMessageToService:message + success:successHandler + failure:^(NSError *error) { + DDLogDebug( + @"%@ Message send attempt failed: %@", self.logTag, message.debugDescription); + failureHandler(error); + }]; } failure:^(NSError *error) { DDLogDebug(@"%@ Attachment upload attempt failed: %@", self.logTag, message.debugDescription); @@ -514,15 +509,15 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException"; failure:failureHandler]; } -- (void)sendTemporaryAttachmentData:(DataSource *)dataSource - contentType:(NSString *)contentType - inMessage:(TSOutgoingMessage *)message - success:(void (^)(void))successHandler - failure:(void (^)(NSError *error))failureHandler +- (void)enqueueOutgoingTemporaryAttachment:(DataSource *)dataSource + contentType:(NSString *)contentType + inMessage:(TSOutgoingMessage *)message + success:(void (^)(void))successHandler + failure:(void (^)(NSError *error))failureHandler { OWSAssert(dataSource); - void (^successWithDeleteHandler)() = ^() { + void (^successWithDeleteHandler)(void) = ^() { successHandler(); DDLogDebug(@"Removing temporary attachment message."); @@ -536,20 +531,20 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException"; [message remove]; }; - [self sendAttachmentData:dataSource - contentType:contentType - sourceFilename:nil - inMessage:message - success:successWithDeleteHandler - failure:failureWithDeleteHandler]; + [self enqueueOutgoingAttachment:dataSource + contentType:contentType + sourceFilename:nil + inMessage:message + success:successWithDeleteHandler + failure:failureWithDeleteHandler]; } -- (void)sendAttachmentData:(DataSource *)dataSource - contentType:(NSString *)contentType - sourceFilename:(nullable NSString *)sourceFilename - inMessage:(TSOutgoingMessage *)message - success:(void (^)(void))successHandler - failure:(void (^)(NSError *error))failureHandler +- (void)enqueueOutgoingAttachment:(DataSource *)dataSource + contentType:(NSString *)contentType + sourceFilename:(nullable NSString *)sourceFilename + inMessage:(TSOutgoingMessage *)message + success:(void (^)(void))successHandler + failure:(void (^)(NSError *error))failureHandler { OWSAssert(dataSource); @@ -573,9 +568,8 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException"; if (sourceFilename) { message.attachmentFilenameMap[attachmentStream.uniqueId] = sourceFilename; } - [message save]; - [self sendMessage:message success:successHandler failure:failureHandler]; + [self enqueueOutgoingMessage:message success:successHandler failure:failureHandler]; }); } @@ -605,9 +599,9 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException"; return [recipients copy]; } -- (void)deliverMessage:(TSOutgoingMessage *)message - success:(void (^)(void))successHandler - failure:(RetryableFailureHandler)failureHandler +- (void)sendMessageToService:(TSOutgoingMessage *)message + success:(void (^)(void))successHandler + failure:(RetryableFailureHandler)failureHandler { dispatch_async([OWSDispatch sendingQueue], ^{ TSThread *thread = message.thread; @@ -693,12 +687,12 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException"; return; } - [self sendMessage:message - recipient:recipient - thread:thread - attempts:OWSMessageSenderRetryAttempts - success:successHandler - failure:failureHandler]; + [self sendMessageToService:message + recipient:recipient + thread:thread + attempts:OWSMessageSenderRetryAttempts + success:successHandler + failure:failureHandler]; } else { // Neither a group nor contact thread? This should never happen. OWSFail(@"%@ Unknown message type: %@", self.logTag, NSStringFromClass([message class])); @@ -717,7 +711,7 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException"; { TOCFutureSource *futureSource = [[TOCFutureSource alloc] init]; - [self sendMessage:message + [self sendMessageToService:message recipient:recipient thread:thread attempts:OWSMessageSenderRetryAttempts @@ -852,12 +846,12 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException"; }]; } -- (void)sendMessage:(TSOutgoingMessage *)message - recipient:(SignalRecipient *)recipient - thread:(TSThread *)thread - attempts:(int)remainingAttempts - success:(void (^)(void))successHandler - failure:(RetryableFailureHandler)failureHandler +- (void)sendMessageToService:(TSOutgoingMessage *)message + recipient:(SignalRecipient *)recipient + thread:(TSThread *)thread + attempts:(int)remainingAttempts + success:(void (^)(void))successHandler + failure:(RetryableFailureHandler)failureHandler { DDLogInfo(@"%@ attempting to send message: %@, timestamp: %llu, recipient: %@", self.logTag, @@ -1048,7 +1042,7 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException"; long statuscode = response.statusCode; NSData *responseData = error.userInfo[AFNetworkingOperationFailingURLResponseDataErrorKey]; - void (^retrySend)() = ^void() { + void (^retrySend)(void) = ^void() { if (remainingAttempts <= 0) { // Since we've already repeatedly failed to send to the messaging API, // it's unlikely that repeating the whole process will succeed. @@ -1058,12 +1052,12 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException"; dispatch_async([OWSDispatch sendingQueue], ^{ DDLogDebug(@"%@ Retrying: %@", self.logTag, message.debugDescription); - [self sendMessage:message - recipient:recipient - thread:thread - attempts:remainingAttempts - success:successHandler - failure:failureHandler]; + [self sendMessageToService:message + recipient:recipient + thread:thread + attempts:remainingAttempts + success:successHandler + failure:failureHandler]; }); }; @@ -1173,12 +1167,7 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException"; // TODO: I suspect we shouldn't optimistically set hasSyncedTranscript. // We could set this in a success handler for [sendSyncTranscriptForMessage:]. [self.dbConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { - // Update the message unless it has been deleted. - if ([TSOutgoingMessage fetchObjectWithUniqueID:message.uniqueId]) { - [message updateWithHasSyncedTranscript:YES transaction:transaction]; - } else { - DDLogInfo(@"%@ not marking message as having synced transcript; message deleted.", self.logTag); - } + [message updateWithHasSyncedTranscript:YES transaction:transaction]; }]; [self sendSyncTranscriptForMessage:message]; } @@ -1259,7 +1248,7 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException"; OWSOutgoingSentMessageTranscript *sentMessageTranscript = [[OWSOutgoingSentMessageTranscript alloc] initWithOutgoingMessage:message]; - [self sendMessage:sentMessageTranscript + [self sendMessageToService:sentMessageTranscript recipient:[SignalRecipient selfRecipient] thread:message.thread attempts:OWSMessageSenderRetryAttempts diff --git a/SignalServiceKit/src/Messages/OWSNotifyRemoteOfUpdatedDisappearingConfigurationJob.m b/SignalServiceKit/src/Messages/OWSNotifyRemoteOfUpdatedDisappearingConfigurationJob.m index 2a4138cdc..ca9e0edf6 100644 --- a/SignalServiceKit/src/Messages/OWSNotifyRemoteOfUpdatedDisappearingConfigurationJob.m +++ b/SignalServiceKit/src/Messages/OWSNotifyRemoteOfUpdatedDisappearingConfigurationJob.m @@ -49,7 +49,7 @@ NS_ASSUME_NONNULL_BEGIN [[OWSDisappearingMessagesConfigurationMessage alloc] initWithConfiguration:self.configuration thread:self.thread]; - [self.messageSender sendMessage:message + [self.messageSender enqueueOutgoingMessage:message success:^{ DDLogDebug( @"%@ Successfully notified %@ of new disappearing messages configuration", self.logTag, self.thread); diff --git a/SignalServiceKit/src/Messages/OWSReadReceiptManager.m b/SignalServiceKit/src/Messages/OWSReadReceiptManager.m index d8db1435f..5f95d2997 100644 --- a/SignalServiceKit/src/Messages/OWSReadReceiptManager.m +++ b/SignalServiceKit/src/Messages/OWSReadReceiptManager.m @@ -229,7 +229,7 @@ NSString *const OWSReadReceiptManagerAreReadReceiptsEnabled = @"areReadReceiptsE OWSReadReceiptsForLinkedDevicesMessage *message = [[OWSReadReceiptsForLinkedDevicesMessage alloc] initWithReadReceipts:readReceiptsForLinkedDevices]; - [self.messageSender sendMessage:message + [self.messageSender enqueueOutgoingMessage:message success:^{ DDLogInfo(@"%@ Successfully sent %zd read receipt to linked devices.", self.logTag, @@ -253,7 +253,7 @@ NSString *const OWSReadReceiptManagerAreReadReceiptsEnabled = @"areReadReceiptsE [[OWSReadReceiptsForSenderMessage alloc] initWithThread:thread messageTimestamps:timestamps.allObjects]; - [self.messageSender sendMessage:message + [self.messageSender enqueueOutgoingMessage:message success:^{ DDLogInfo(@"%@ Successfully sent %zd read receipts to sender.", self.logTag, timestamps.count); } @@ -590,7 +590,7 @@ NSString *const OWSReadReceiptManagerAreReadReceiptsEnabled = @"areReadReceiptsE OWSSyncConfigurationMessage *syncConfigurationMessage = [[OWSSyncConfigurationMessage alloc] initWithReadReceiptsEnabled:value]; - [self.messageSender sendMessage:syncConfigurationMessage + [self.messageSender enqueueOutgoingMessage:syncConfigurationMessage success:^{ DDLogInfo(@"%@ Successfully sent Configuration syncMessage.", self.logTag); } From 94b59c326ebd568afcfdaf97e9175fc38c02a9f2 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Wed, 15 Nov 2017 06:53:05 -0500 Subject: [PATCH 4/7] Rework the "update with..." methods to avoid re-saving deleted models. --- SignalServiceKit/src/Messages/OWSMessageSender.m | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/SignalServiceKit/src/Messages/OWSMessageSender.m b/SignalServiceKit/src/Messages/OWSMessageSender.m index 82ffe293c..84cba896f 100644 --- a/SignalServiceKit/src/Messages/OWSMessageSender.m +++ b/SignalServiceKit/src/Messages/OWSMessageSender.m @@ -718,12 +718,7 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException"; success:^{ DDLogInfo(@"%@ Marking group message as sent to recipient: %@", self.logTag, recipient.uniqueId); [self.dbConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { - // Update the message unless it has been deleted. - if ([TSOutgoingMessage fetchObjectWithUniqueID:message.uniqueId]) { - [message updateWithSentRecipient:recipient.uniqueId transaction:transaction]; - } else { - DDLogInfo(@"%@ not marking message as sent to recipient; message deleted.", self.logTag); - } + [message updateWithSentRecipient:recipient.uniqueId transaction:transaction]; }]; [futureSource trySetResult:@1]; } From 5eea0347b50ea130f23d5494e2c95ec9dc7db3f5 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Wed, 15 Nov 2017 07:11:49 -0500 Subject: [PATCH 5/7] Rework the "update with..." methods to avoid re-saving deleted models. --- SignalServiceKit/src/Contacts/TSThread.m | 12 +- .../src/Messages/Interactions/TSMessage.h | 32 +++++- .../src/Messages/Interactions/TSMessage.m | 33 ++++++ .../Messages/Interactions/TSOutgoingMessage.m | 105 +++++++++--------- .../src/Messages/OWSDisappearingMessagesJob.m | 3 +- 5 files changed, 122 insertions(+), 63 deletions(-) diff --git a/SignalServiceKit/src/Contacts/TSThread.m b/SignalServiceKit/src/Contacts/TSThread.m index fe7d2ab22..292a570bc 100644 --- a/SignalServiceKit/src/Contacts/TSThread.m +++ b/SignalServiceKit/src/Contacts/TSThread.m @@ -384,8 +384,8 @@ NS_ASSUME_NONNULL_BEGIN // This method does the work for the "updateWith..." methods. Please see // the header for a discussion of those methods. -- (void)applyChangeToSelfAndLatestCopy:(YapDatabaseReadWriteTransaction *)transaction - changeBlock:(void (^)(TSThread *))changeBlock +- (void)applyChangeToSelfAndLatestThread:(YapDatabaseReadWriteTransaction *)transaction + changeBlock:(void (^)(TSThread *))changeBlock { OWSAssert(transaction); @@ -402,10 +402,10 @@ NS_ASSUME_NONNULL_BEGIN - (void)updateWithMutedUntilDate:(NSDate *)mutedUntilDate { [self.dbReadWriteConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { - [self applyChangeToSelfAndLatestCopy:transaction - changeBlock:^(TSThread *thread) { - [thread setMutedUntilDate:mutedUntilDate]; - }]; + [self applyChangeToSelfAndLatestThread:transaction + changeBlock:^(TSThread *thread) { + [thread setMutedUntilDate:mutedUntilDate]; + }]; }]; } diff --git a/SignalServiceKit/src/Messages/Interactions/TSMessage.h b/SignalServiceKit/src/Messages/Interactions/TSMessage.h index 68b88b19d..068f9c28a 100644 --- a/SignalServiceKit/src/Messages/Interactions/TSMessage.h +++ b/SignalServiceKit/src/Messages/Interactions/TSMessage.h @@ -15,9 +15,9 @@ NS_ASSUME_NONNULL_BEGIN @interface TSMessage : TSInteraction @property (nonatomic, readonly) NSMutableArray *attachmentIds; -@property (nullable, nonatomic) NSString *body; -@property (nonatomic) uint32_t expiresInSeconds; -@property (nonatomic) uint64_t expireStartedAt; +@property (nonatomic, readonly, nullable) NSString *body; +@property (nonatomic, readonly) uint32_t expiresInSeconds; +@property (nonatomic, readonly) uint64_t expireStartedAt; @property (nonatomic, readonly) uint64_t expiresAt; @property (nonatomic, readonly) BOOL isExpiringMessage; @@ -56,6 +56,32 @@ NS_ASSUME_NONNULL_BEGIN - (BOOL)shouldStartExpireTimer; - (BOOL)shouldStartExpireTimer:(YapDatabaseReadTransaction *)transaction; +#pragma mark - Update Methods + +// This model may be updated from many threads. We don't want to save +// our local copy (this instance) since it may be out of date. We also +// want to avoid re-saving a model that has been deleted. Therefore, we +// use these "updateWith..." methods to: +// +// a) Update a property of this instance. +// b) If a copy of this model exists in the database, load an up-to-date copy, +// and update and save that copy. +// b) If a copy of this model _DOES NOT_ exist in the database, do _NOT_ save +// this local instance. +// +// After "updateWith...": +// +// a) An copy of this model in the database will have been updated. +// b) The local property on this instance will always have been updated. +// c) Other properties on this instance may be out of date. +// +// All mutable properties of this class have been made read-only to +// prevent accidentally modifying them directly. +// +// This isn't a perfect arrangement, but in practice this will prevent +// data loss and will resolve all known issues. +- (void)updateWithExpireStartedAt:(uint64_t)expireStartedAt transaction:(YapDatabaseReadWriteTransaction *)transaction; + @end NS_ASSUME_NONNULL_END diff --git a/SignalServiceKit/src/Messages/Interactions/TSMessage.m b/SignalServiceKit/src/Messages/Interactions/TSMessage.m index ce44fc1f8..74a8b7282 100644 --- a/SignalServiceKit/src/Messages/Interactions/TSMessage.m +++ b/SignalServiceKit/src/Messages/Interactions/TSMessage.m @@ -16,6 +16,10 @@ static const NSUInteger OWSMessageSchemaVersion = 3; @interface TSMessage () +@property (nonatomic, nullable) NSString *body; +@property (nonatomic) uint32_t expiresInSeconds; +@property (nonatomic) uint64_t expireStartedAt; + /** * The version of the model class's schema last used to serialize this model. Use this to manage data migrations during * object de/serialization. @@ -277,6 +281,35 @@ static const NSUInteger OWSMessageSchemaVersion = 3; return YES; } +#pragma mark - Update Methods + +// This method does the work for the "updateWith..." methods. Please see +// the header for a discussion of those methods. +- (void)applyChangeToSelfAndLatestMessage:(YapDatabaseReadWriteTransaction *)transaction + changeBlock:(void (^)(TSMessage *))changeBlock +{ + OWSAssert(transaction); + + changeBlock(self); + + NSString *collection = [[self class] collection]; + TSMessage *latestMessage = [transaction objectForKey:self.uniqueId inCollection:collection]; + if (latestMessage) { + changeBlock(latestMessage); + [latestMessage saveWithTransaction:transaction]; + } +} + +- (void)updateWithExpireStartedAt:(uint64_t)expireStartedAt transaction:(YapDatabaseReadWriteTransaction *)transaction +{ + OWSAssert(expireStartedAt > 0); + + [self applyChangeToSelfAndLatestMessage:transaction + changeBlock:^(TSMessage *message) { + [message setExpireStartedAt:expireStartedAt]; + }]; +} + @end NS_ASSUME_NONNULL_END diff --git a/SignalServiceKit/src/Messages/Interactions/TSOutgoingMessage.m b/SignalServiceKit/src/Messages/Interactions/TSOutgoingMessage.m index 79c0aea67..c130ee9e2 100644 --- a/SignalServiceKit/src/Messages/Interactions/TSOutgoingMessage.m +++ b/SignalServiceKit/src/Messages/Interactions/TSOutgoingMessage.m @@ -242,8 +242,8 @@ NSString *const kTSOutgoingMessageSentRecipientAll = @"kTSOutgoingMessageSentRec // This method does the work for the "updateWith..." methods. Please see // the header for a discussion of those methods. -- (void)applyChangeToSelfAndLatestCopy:(YapDatabaseReadWriteTransaction *)transaction - changeBlock:(void (^)(TSOutgoingMessage *))changeBlock +- (void)applyChangeToSelfAndLatestOutgoingMessage:(YapDatabaseReadWriteTransaction *)transaction + changeBlock:(void (^)(TSOutgoingMessage *))changeBlock { OWSAssert(transaction); @@ -262,11 +262,11 @@ NSString *const kTSOutgoingMessageSentRecipientAll = @"kTSOutgoingMessageSentRec OWSAssert(error); [self.dbReadWriteConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { - [self applyChangeToSelfAndLatestCopy:transaction - changeBlock:^(TSOutgoingMessage *message) { - [message setMessageState:TSOutgoingMessageStateUnsent]; - [message setMostRecentFailureText:error.localizedDescription]; - }]; + [self applyChangeToSelfAndLatestOutgoingMessage:transaction + changeBlock:^(TSOutgoingMessage *message) { + [message setMessageState:TSOutgoingMessageStateUnsent]; + [message setMostRecentFailureText:error.localizedDescription]; + }]; }]; } @@ -282,19 +282,19 @@ NSString *const kTSOutgoingMessageSentRecipientAll = @"kTSOutgoingMessageSentRec { OWSAssert(transaction); - [self applyChangeToSelfAndLatestCopy:transaction - changeBlock:^(TSOutgoingMessage *message) { - [message setMessageState:messageState]; - }]; + [self applyChangeToSelfAndLatestOutgoingMessage:transaction + changeBlock:^(TSOutgoingMessage *message) { + [message setMessageState:messageState]; + }]; } - (void)updateWithHasSyncedTranscript:(BOOL)hasSyncedTranscript transaction:(YapDatabaseReadWriteTransaction *)transaction { - [self applyChangeToSelfAndLatestCopy:transaction - changeBlock:^(TSOutgoingMessage *message) { - [message setHasSyncedTranscript:hasSyncedTranscript]; - }]; + [self applyChangeToSelfAndLatestOutgoingMessage:transaction + changeBlock:^(TSOutgoingMessage *message) { + [message setHasSyncedTranscript:hasSyncedTranscript]; + }]; } - (void)updateWithCustomMessage:(NSString *)customMessage transaction:(YapDatabaseReadWriteTransaction *)transaction @@ -302,10 +302,10 @@ NSString *const kTSOutgoingMessageSentRecipientAll = @"kTSOutgoingMessageSentRec OWSAssert(customMessage); OWSAssert(transaction); - [self applyChangeToSelfAndLatestCopy:transaction - changeBlock:^(TSOutgoingMessage *message) { - [message setCustomMessage:customMessage]; - }]; + [self applyChangeToSelfAndLatestOutgoingMessage:transaction + changeBlock:^(TSOutgoingMessage *message) { + [message setCustomMessage:customMessage]; + }]; } - (void)updateWithCustomMessage:(NSString *)customMessage @@ -322,31 +322,32 @@ NSString *const kTSOutgoingMessageSentRecipientAll = @"kTSOutgoingMessageSentRec OWSAssert(recipientId.length > 0); OWSAssert(transaction); - [self applyChangeToSelfAndLatestCopy:transaction - changeBlock:^(TSOutgoingMessage *message) { + [self applyChangeToSelfAndLatestOutgoingMessage:transaction + changeBlock:^(TSOutgoingMessage *message) { - if (deliveryTimestamp) { - NSMutableDictionary *recipientDeliveryMap - = (message.recipientDeliveryMap ? [message.recipientDeliveryMap mutableCopy] - : [NSMutableDictionary new]); - recipientDeliveryMap[recipientId] = deliveryTimestamp; - message.recipientDeliveryMap = [recipientDeliveryMap copy]; - } + if (deliveryTimestamp) { + NSMutableDictionary *recipientDeliveryMap + = (message.recipientDeliveryMap + ? [message.recipientDeliveryMap mutableCopy] + : [NSMutableDictionary new]); + recipientDeliveryMap[recipientId] = deliveryTimestamp; + message.recipientDeliveryMap = [recipientDeliveryMap copy]; + } - [message setWasDelivered:YES]; - }]; + [message setWasDelivered:YES]; + }]; } - (void)updateWithWasSentFromLinkedDeviceWithTransaction:(YapDatabaseReadWriteTransaction *)transaction { OWSAssert(transaction); - [self applyChangeToSelfAndLatestCopy:transaction - changeBlock:^(TSOutgoingMessage *message) { - [message setMessageState:TSOutgoingMessageStateSentToService]; - [message setWasDelivered:YES]; - [message setIsFromLinkedDevice:YES]; - }]; + [self applyChangeToSelfAndLatestOutgoingMessage:transaction + changeBlock:^(TSOutgoingMessage *message) { + [message setMessageState:TSOutgoingMessageStateSentToService]; + [message setWasDelivered:YES]; + [message setIsFromLinkedDevice:YES]; + }]; } - (void)updateWithSingleGroupRecipient:(NSString *)singleGroupRecipient @@ -355,10 +356,10 @@ NSString *const kTSOutgoingMessageSentRecipientAll = @"kTSOutgoingMessageSentRec OWSAssert(transaction); OWSAssert(singleGroupRecipient.length > 0); - [self applyChangeToSelfAndLatestCopy:transaction - changeBlock:^(TSOutgoingMessage *message) { - [message setSingleGroupRecipient:singleGroupRecipient]; - }]; + [self applyChangeToSelfAndLatestOutgoingMessage:transaction + changeBlock:^(TSOutgoingMessage *message) { + [message setSingleGroupRecipient:singleGroupRecipient]; + }]; } #pragma mark - Sent Recipients @@ -410,10 +411,10 @@ NSString *const kTSOutgoingMessageSentRecipientAll = @"kTSOutgoingMessageSentRec - (void)updateWithSentRecipient:(NSString *)contactId transaction:(YapDatabaseReadWriteTransaction *)transaction { OWSAssert(transaction); - [self applyChangeToSelfAndLatestCopy:transaction - changeBlock:^(TSOutgoingMessage *message) { - [message addSentRecipient:contactId]; - }]; + [self applyChangeToSelfAndLatestOutgoingMessage:transaction + changeBlock:^(TSOutgoingMessage *message) { + [message addSentRecipient:contactId]; + }]; } - (void)updateWithReadRecipientId:(NSString *)recipientId @@ -423,14 +424,14 @@ NSString *const kTSOutgoingMessageSentRecipientAll = @"kTSOutgoingMessageSentRec OWSAssert(recipientId.length > 0); OWSAssert(transaction); - [self applyChangeToSelfAndLatestCopy:transaction - changeBlock:^(TSOutgoingMessage *message) { - NSMutableDictionary *recipientReadMap - = (message.recipientReadMap ? [message.recipientReadMap mutableCopy] - : [NSMutableDictionary new]); - recipientReadMap[recipientId] = @(readTimestamp); - message.recipientReadMap = [recipientReadMap copy]; - }]; + [self applyChangeToSelfAndLatestOutgoingMessage:transaction + changeBlock:^(TSOutgoingMessage *message) { + NSMutableDictionary *recipientReadMap + = (message.recipientReadMap ? [message.recipientReadMap mutableCopy] + : [NSMutableDictionary new]); + recipientReadMap[recipientId] = @(readTimestamp); + message.recipientReadMap = [recipientReadMap copy]; + }]; } - (nullable NSNumber *)firstRecipientReadTimestamp diff --git a/SignalServiceKit/src/Messages/OWSDisappearingMessagesJob.m b/SignalServiceKit/src/Messages/OWSDisappearingMessagesJob.m index a7eb710e4..53ad1c851 100644 --- a/SignalServiceKit/src/Messages/OWSDisappearingMessagesJob.m +++ b/SignalServiceKit/src/Messages/OWSDisappearingMessagesJob.m @@ -184,8 +184,7 @@ NS_ASSUME_NONNULL_BEGIN // Don't clobber if multiple actions simultaneously triggered expiration. if (message.expireStartedAt == 0 || message.expireStartedAt > expirationStartedAt) { - message.expireStartedAt = expirationStartedAt; - [message saveWithTransaction:transaction]; + [message updateWithExpireStartedAt:expirationStartedAt transaction:transaction]; } // Necessary that the async expiration run happens *after* the message is saved with expiration configuration. From 00feb14b10353191f0a76fd8590456516eb3277b Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Wed, 15 Nov 2017 13:15:48 -0500 Subject: [PATCH 6/7] Respond to CR. // FREEBIE --- ...ExperienceUpgradesPageViewController.swift | 6 +- .../MessageDetailViewController.swift | 12 +- .../environment/ExperienceUpgradeFinder.swift | 2 +- .../ExperienceUpgrade.swift | 2 +- .../OWSDisappearingMessagesConfiguration.m | 2 +- SignalServiceKit/src/Contacts/TSThread.h | 24 +--- SignalServiceKit/src/Contacts/TSThread.m | 28 +--- .../src/Messages/Interactions/TSMessage.h | 24 +--- .../src/Messages/Interactions/TSMessage.m | 27 +--- .../Messages/Interactions/TSOutgoingMessage.h | 24 +--- .../Messages/Interactions/TSOutgoingMessage.m | 120 ++++++++---------- .../src/Messages/OWSBatchMessageProcessor.m | 2 +- .../src/Messages/OWSMessageReceiver.m | 2 +- .../src/Security/OWSRecipientIdentity.h | 2 +- .../src/Storage/TSYapDatabaseObject.h | 46 ++++++- .../src/Storage/TSYapDatabaseObject.m | 37 +++++- 16 files changed, 163 insertions(+), 197 deletions(-) diff --git a/Signal/src/ViewControllers/ExperienceUpgradesPageViewController.swift b/Signal/src/ViewControllers/ExperienceUpgradesPageViewController.swift index e41f8f8aa..dc9d2c1d5 100644 --- a/Signal/src/ViewControllers/ExperienceUpgradesPageViewController.swift +++ b/Signal/src/ViewControllers/ExperienceUpgradesPageViewController.swift @@ -526,7 +526,11 @@ class ExperienceUpgradesPageViewController: OWSViewController, UIPageViewControl } public func addViewController(experienceUpgrade: ExperienceUpgrade) { - guard let identifier = ExperienceUpgradeId(rawValue: experienceUpgrade.uniqueId) else { + guard let uniqueId = experienceUpgrade.uniqueId else { + Logger.error("\(self.TAG) experienceUpgrade is missing uniqueId.") + return + } + guard let identifier = ExperienceUpgradeId(rawValue: uniqueId) else { owsFail("\(TAG) unknown experience upgrade. skipping") return } diff --git a/Signal/src/ViewControllers/MessageDetailViewController.swift b/Signal/src/ViewControllers/MessageDetailViewController.swift index 455e637a3..bac831bcd 100644 --- a/Signal/src/ViewControllers/MessageDetailViewController.swift +++ b/Signal/src/ViewControllers/MessageDetailViewController.swift @@ -551,7 +551,11 @@ class MessageDetailViewController: OWSViewController, UIScrollViewDelegate { AssertIsOnMainThread() self.databaseConnection.read { transaction in - guard let newMessage = TSInteraction.fetch(uniqueId: self.message.uniqueId, transaction: transaction) as? TSMessage else { + guard let uniqueId = self.message.uniqueId else { + Logger.error("\(self.TAG) Message is missing uniqueId.") + return + } + guard let newMessage = TSInteraction.fetch(uniqueId: uniqueId, transaction: transaction) as? TSMessage else { Logger.error("\(self.TAG) Couldn't reload message.") return } @@ -564,7 +568,11 @@ class MessageDetailViewController: OWSViewController, UIScrollViewDelegate { let notifications = self.databaseConnection.beginLongLivedReadTransaction() - guard self.databaseConnection.hasChange(forKey: message.uniqueId, + guard let uniqueId = self.message.uniqueId else { + Logger.error("\(self.TAG) Message is missing uniqueId.") + return + } + guard self.databaseConnection.hasChange(forKey: uniqueId, inCollection: TSInteraction.collection(), in: notifications) else { Logger.debug("\(TAG) No relevant changes.") diff --git a/Signal/src/environment/ExperienceUpgradeFinder.swift b/Signal/src/environment/ExperienceUpgradeFinder.swift index 537ca2844..0641b960e 100644 --- a/Signal/src/environment/ExperienceUpgradeFinder.swift +++ b/Signal/src/environment/ExperienceUpgradeFinder.swift @@ -59,7 +59,7 @@ class ExperienceUpgradeFinder: NSObject { // MARK: - Instance Methods public func allUnseen(transaction: YapDatabaseReadTransaction) -> [ExperienceUpgrade] { - return allExperienceUpgrades.filter { ExperienceUpgrade.fetch(uniqueId: $0.uniqueId, transaction: transaction) == nil } + return allExperienceUpgrades.filter { ExperienceUpgrade.fetch(uniqueId: $0.uniqueId!, transaction: transaction) == nil } } public func markAllAsSeen(transaction: YapDatabaseReadWriteTransaction) { diff --git a/Signal/src/environment/ExperienceUpgrades/ExperienceUpgrade.swift b/Signal/src/environment/ExperienceUpgrades/ExperienceUpgrade.swift index 699a4e225..2e86facd7 100644 --- a/Signal/src/environment/ExperienceUpgrades/ExperienceUpgrade.swift +++ b/Signal/src/environment/ExperienceUpgrades/ExperienceUpgrade.swift @@ -16,7 +16,7 @@ class ExperienceUpgrade: TSYapDatabaseObject { super.init(uniqueId: uniqueId) } - override required init(uniqueId: String) { + override required init(uniqueId: String?) { // This is the unfortunate seam between strict swift and fast-and-loose objc // we can't leave these properties nil, since we really "don't know" that the superclass // will assign them. diff --git a/SignalServiceKit/src/Contacts/OWSDisappearingMessagesConfiguration.m b/SignalServiceKit/src/Contacts/OWSDisappearingMessagesConfiguration.m index 484420ea3..f3242de2f 100644 --- a/SignalServiceKit/src/Contacts/OWSDisappearingMessagesConfiguration.m +++ b/SignalServiceKit/src/Contacts/OWSDisappearingMessagesConfiguration.m @@ -27,7 +27,7 @@ const uint32_t OWSDisappearingMessagesConfigurationDefaultExpirationDuration = k durationSeconds:OWSDisappearingMessagesConfigurationDefaultExpirationDuration]; } -- (instancetype)initWithCoder:(NSCoder *)coder +- (nullable instancetype)initWithCoder:(NSCoder *)coder { self = [super initWithCoder:coder]; diff --git a/SignalServiceKit/src/Contacts/TSThread.h b/SignalServiceKit/src/Contacts/TSThread.h index 4ed06aa29..e0308edae 100644 --- a/SignalServiceKit/src/Contacts/TSThread.h +++ b/SignalServiceKit/src/Contacts/TSThread.h @@ -156,28 +156,8 @@ NS_ASSUME_NONNULL_BEGIN @property (atomic, readonly) BOOL isMuted; @property (atomic, readonly, nullable) NSDate *mutedUntilDate; -// This model may be updated from many threads. We don't want to save -// our local copy (this instance) since it may be out of date. We also -// want to avoid re-saving a model that has been deleted. Therefore, we -// use these "updateWith..." methods to: -// -// a) Update a property of this instance. -// b) If a copy of this model exists in the database, load an up-to-date copy, -// and update and save that copy. -// b) If a copy of this model _DOES NOT_ exist in the database, do _NOT_ save -// this local instance. -// -// After "updateWith...": -// -// a) An copy of this model in the database will have been updated. -// b) The local property on this instance will always have been updated. -// c) Other properties on this instance may be out of date. -// -// All mutable properties of this class have been made read-only to -// prevent accidentally modifying them directly. -// -// This isn't a perfect arrangement, but in practice this will prevent -// data loss and will resolve all known issues. +#pragma mark - Update With... Methods + - (void)updateWithMutedUntilDate:(NSDate *)mutedUntilDate; @end diff --git a/SignalServiceKit/src/Contacts/TSThread.m b/SignalServiceKit/src/Contacts/TSThread.m index 292a570bc..4fa1b9b8f 100644 --- a/SignalServiceKit/src/Contacts/TSThread.m +++ b/SignalServiceKit/src/Contacts/TSThread.m @@ -34,7 +34,8 @@ NS_ASSUME_NONNULL_BEGIN return @"TSThread"; } -- (instancetype)initWithUniqueId:(NSString *)uniqueId { +- (instancetype)initWithUniqueId:(NSString *_Nullable)uniqueId +{ self = [super initWithUniqueId:uniqueId]; if (self) { @@ -382,30 +383,15 @@ NS_ASSUME_NONNULL_BEGIN [mutedUntilDate timeIntervalSinceDate:now] > 0); } -// This method does the work for the "updateWith..." methods. Please see -// the header for a discussion of those methods. -- (void)applyChangeToSelfAndLatestThread:(YapDatabaseReadWriteTransaction *)transaction - changeBlock:(void (^)(TSThread *))changeBlock -{ - OWSAssert(transaction); - - changeBlock(self); - - NSString *collection = [[self class] collection]; - TSThread *latestInstance = [transaction objectForKey:self.uniqueId inCollection:collection]; - if (latestInstance) { - changeBlock(latestInstance); - [latestInstance saveWithTransaction:transaction]; - } -} +#pragma mark - Update With... Methods - (void)updateWithMutedUntilDate:(NSDate *)mutedUntilDate { [self.dbReadWriteConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { - [self applyChangeToSelfAndLatestThread:transaction - changeBlock:^(TSThread *thread) { - [thread setMutedUntilDate:mutedUntilDate]; - }]; + [self applyChangeToSelfAndLatestCopy:transaction + changeBlock:^(TSThread *thread) { + [thread setMutedUntilDate:mutedUntilDate]; + }]; }]; } diff --git a/SignalServiceKit/src/Messages/Interactions/TSMessage.h b/SignalServiceKit/src/Messages/Interactions/TSMessage.h index 068f9c28a..e00caa19b 100644 --- a/SignalServiceKit/src/Messages/Interactions/TSMessage.h +++ b/SignalServiceKit/src/Messages/Interactions/TSMessage.h @@ -56,30 +56,8 @@ NS_ASSUME_NONNULL_BEGIN - (BOOL)shouldStartExpireTimer; - (BOOL)shouldStartExpireTimer:(YapDatabaseReadTransaction *)transaction; -#pragma mark - Update Methods +#pragma mark - Update With... Methods -// This model may be updated from many threads. We don't want to save -// our local copy (this instance) since it may be out of date. We also -// want to avoid re-saving a model that has been deleted. Therefore, we -// use these "updateWith..." methods to: -// -// a) Update a property of this instance. -// b) If a copy of this model exists in the database, load an up-to-date copy, -// and update and save that copy. -// b) If a copy of this model _DOES NOT_ exist in the database, do _NOT_ save -// this local instance. -// -// After "updateWith...": -// -// a) An copy of this model in the database will have been updated. -// b) The local property on this instance will always have been updated. -// c) Other properties on this instance may be out of date. -// -// All mutable properties of this class have been made read-only to -// prevent accidentally modifying them directly. -// -// This isn't a perfect arrangement, but in practice this will prevent -// data loss and will resolve all known issues. - (void)updateWithExpireStartedAt:(uint64_t)expireStartedAt transaction:(YapDatabaseReadWriteTransaction *)transaction; @end diff --git a/SignalServiceKit/src/Messages/Interactions/TSMessage.m b/SignalServiceKit/src/Messages/Interactions/TSMessage.m index 74a8b7282..8207da26b 100644 --- a/SignalServiceKit/src/Messages/Interactions/TSMessage.m +++ b/SignalServiceKit/src/Messages/Interactions/TSMessage.m @@ -281,33 +281,16 @@ static const NSUInteger OWSMessageSchemaVersion = 3; return YES; } -#pragma mark - Update Methods - -// This method does the work for the "updateWith..." methods. Please see -// the header for a discussion of those methods. -- (void)applyChangeToSelfAndLatestMessage:(YapDatabaseReadWriteTransaction *)transaction - changeBlock:(void (^)(TSMessage *))changeBlock -{ - OWSAssert(transaction); - - changeBlock(self); - - NSString *collection = [[self class] collection]; - TSMessage *latestMessage = [transaction objectForKey:self.uniqueId inCollection:collection]; - if (latestMessage) { - changeBlock(latestMessage); - [latestMessage saveWithTransaction:transaction]; - } -} +#pragma mark - Update With... Methods - (void)updateWithExpireStartedAt:(uint64_t)expireStartedAt transaction:(YapDatabaseReadWriteTransaction *)transaction { OWSAssert(expireStartedAt > 0); - [self applyChangeToSelfAndLatestMessage:transaction - changeBlock:^(TSMessage *message) { - [message setExpireStartedAt:expireStartedAt]; - }]; + [self applyChangeToSelfAndLatestCopy:transaction + changeBlock:^(TSMessage *message) { + [message setExpireStartedAt:expireStartedAt]; + }]; } @end diff --git a/SignalServiceKit/src/Messages/Interactions/TSOutgoingMessage.h b/SignalServiceKit/src/Messages/Interactions/TSOutgoingMessage.h index abb39aa49..842bf9574 100644 --- a/SignalServiceKit/src/Messages/Interactions/TSOutgoingMessage.h +++ b/SignalServiceKit/src/Messages/Interactions/TSOutgoingMessage.h @@ -148,28 +148,8 @@ typedef NS_ENUM(NSInteger, TSGroupMetaMessage) { - (OWSSignalServiceProtosAttachmentPointer *)buildAttachmentProtoForAttachmentId:(NSString *)attachmentId filename:(nullable NSString *)filename; -// This model may be updated from many threads. We don't want to save -// our local copy (this instance) since it may be out of date. We also -// want to avoid re-saving a model that has been deleted. Therefore, we -// use these "updateWith..." methods to: -// -// a) Update a property of this instance. -// b) If a copy of this model exists in the database, load an up-to-date copy, -// and update and save that copy. -// b) If a copy of this model _DOES NOT_ exist in the database, do _NOT_ save -// this local instance. -// -// After "updateWith...": -// -// a) An copy of this model in the database will have been updated. -// b) The local property on this instance will always have been updated. -// c) Other properties on this instance may be out of date. -// -// All mutable properties of this class have been made read-only to -// prevent accidentally modifying them directly. -// -// This isn't a perfect arrangement, but in practice this will prevent -// data loss and will resolve all known issues. +#pragma mark - Update With... Methods + - (void)updateWithMessageState:(TSOutgoingMessageState)messageState; - (void)updateWithMessageState:(TSOutgoingMessageState)messageState transaction:(YapDatabaseReadWriteTransaction *)transaction; diff --git a/SignalServiceKit/src/Messages/Interactions/TSOutgoingMessage.m b/SignalServiceKit/src/Messages/Interactions/TSOutgoingMessage.m index c130ee9e2..04a0fc70e 100644 --- a/SignalServiceKit/src/Messages/Interactions/TSOutgoingMessage.m +++ b/SignalServiceKit/src/Messages/Interactions/TSOutgoingMessage.m @@ -238,35 +238,18 @@ NSString *const kTSOutgoingMessageSentRecipientAll = @"kTSOutgoingMessageSentRec return OWSInteractionType_OutgoingMessage; } -#pragma mark - Update Methods - -// This method does the work for the "updateWith..." methods. Please see -// the header for a discussion of those methods. -- (void)applyChangeToSelfAndLatestOutgoingMessage:(YapDatabaseReadWriteTransaction *)transaction - changeBlock:(void (^)(TSOutgoingMessage *))changeBlock -{ - OWSAssert(transaction); - - changeBlock(self); - - NSString *collection = [[self class] collection]; - TSOutgoingMessage *latestMessage = [transaction objectForKey:self.uniqueId inCollection:collection]; - if (latestMessage) { - changeBlock(latestMessage); - [latestMessage saveWithTransaction:transaction]; - } -} +#pragma mark - Update With... Methods - (void)updateWithSendingError:(NSError *)error { OWSAssert(error); [self.dbReadWriteConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { - [self applyChangeToSelfAndLatestOutgoingMessage:transaction - changeBlock:^(TSOutgoingMessage *message) { - [message setMessageState:TSOutgoingMessageStateUnsent]; - [message setMostRecentFailureText:error.localizedDescription]; - }]; + [self applyChangeToSelfAndLatestCopy:transaction + changeBlock:^(TSOutgoingMessage *message) { + [message setMessageState:TSOutgoingMessageStateUnsent]; + [message setMostRecentFailureText:error.localizedDescription]; + }]; }]; } @@ -282,19 +265,19 @@ NSString *const kTSOutgoingMessageSentRecipientAll = @"kTSOutgoingMessageSentRec { OWSAssert(transaction); - [self applyChangeToSelfAndLatestOutgoingMessage:transaction - changeBlock:^(TSOutgoingMessage *message) { - [message setMessageState:messageState]; - }]; + [self applyChangeToSelfAndLatestCopy:transaction + changeBlock:^(TSOutgoingMessage *message) { + [message setMessageState:messageState]; + }]; } - (void)updateWithHasSyncedTranscript:(BOOL)hasSyncedTranscript transaction:(YapDatabaseReadWriteTransaction *)transaction { - [self applyChangeToSelfAndLatestOutgoingMessage:transaction - changeBlock:^(TSOutgoingMessage *message) { - [message setHasSyncedTranscript:hasSyncedTranscript]; - }]; + [self applyChangeToSelfAndLatestCopy:transaction + changeBlock:^(TSOutgoingMessage *message) { + [message setHasSyncedTranscript:hasSyncedTranscript]; + }]; } - (void)updateWithCustomMessage:(NSString *)customMessage transaction:(YapDatabaseReadWriteTransaction *)transaction @@ -302,10 +285,10 @@ NSString *const kTSOutgoingMessageSentRecipientAll = @"kTSOutgoingMessageSentRec OWSAssert(customMessage); OWSAssert(transaction); - [self applyChangeToSelfAndLatestOutgoingMessage:transaction - changeBlock:^(TSOutgoingMessage *message) { - [message setCustomMessage:customMessage]; - }]; + [self applyChangeToSelfAndLatestCopy:transaction + changeBlock:^(TSOutgoingMessage *message) { + [message setCustomMessage:customMessage]; + }]; } - (void)updateWithCustomMessage:(NSString *)customMessage @@ -322,32 +305,31 @@ NSString *const kTSOutgoingMessageSentRecipientAll = @"kTSOutgoingMessageSentRec OWSAssert(recipientId.length > 0); OWSAssert(transaction); - [self applyChangeToSelfAndLatestOutgoingMessage:transaction - changeBlock:^(TSOutgoingMessage *message) { + [self applyChangeToSelfAndLatestCopy:transaction + changeBlock:^(TSOutgoingMessage *message) { - if (deliveryTimestamp) { - NSMutableDictionary *recipientDeliveryMap - = (message.recipientDeliveryMap - ? [message.recipientDeliveryMap mutableCopy] - : [NSMutableDictionary new]); - recipientDeliveryMap[recipientId] = deliveryTimestamp; - message.recipientDeliveryMap = [recipientDeliveryMap copy]; - } + if (deliveryTimestamp) { + NSMutableDictionary *recipientDeliveryMap + = (message.recipientDeliveryMap ? [message.recipientDeliveryMap mutableCopy] + : [NSMutableDictionary new]); + recipientDeliveryMap[recipientId] = deliveryTimestamp; + message.recipientDeliveryMap = [recipientDeliveryMap copy]; + } - [message setWasDelivered:YES]; - }]; + [message setWasDelivered:YES]; + }]; } - (void)updateWithWasSentFromLinkedDeviceWithTransaction:(YapDatabaseReadWriteTransaction *)transaction { OWSAssert(transaction); - [self applyChangeToSelfAndLatestOutgoingMessage:transaction - changeBlock:^(TSOutgoingMessage *message) { - [message setMessageState:TSOutgoingMessageStateSentToService]; - [message setWasDelivered:YES]; - [message setIsFromLinkedDevice:YES]; - }]; + [self applyChangeToSelfAndLatestCopy:transaction + changeBlock:^(TSOutgoingMessage *message) { + [message setMessageState:TSOutgoingMessageStateSentToService]; + [message setWasDelivered:YES]; + [message setIsFromLinkedDevice:YES]; + }]; } - (void)updateWithSingleGroupRecipient:(NSString *)singleGroupRecipient @@ -356,10 +338,10 @@ NSString *const kTSOutgoingMessageSentRecipientAll = @"kTSOutgoingMessageSentRec OWSAssert(transaction); OWSAssert(singleGroupRecipient.length > 0); - [self applyChangeToSelfAndLatestOutgoingMessage:transaction - changeBlock:^(TSOutgoingMessage *message) { - [message setSingleGroupRecipient:singleGroupRecipient]; - }]; + [self applyChangeToSelfAndLatestCopy:transaction + changeBlock:^(TSOutgoingMessage *message) { + [message setSingleGroupRecipient:singleGroupRecipient]; + }]; } #pragma mark - Sent Recipients @@ -411,10 +393,10 @@ NSString *const kTSOutgoingMessageSentRecipientAll = @"kTSOutgoingMessageSentRec - (void)updateWithSentRecipient:(NSString *)contactId transaction:(YapDatabaseReadWriteTransaction *)transaction { OWSAssert(transaction); - [self applyChangeToSelfAndLatestOutgoingMessage:transaction - changeBlock:^(TSOutgoingMessage *message) { - [message addSentRecipient:contactId]; - }]; + [self applyChangeToSelfAndLatestCopy:transaction + changeBlock:^(TSOutgoingMessage *message) { + [message addSentRecipient:contactId]; + }]; } - (void)updateWithReadRecipientId:(NSString *)recipientId @@ -424,14 +406,14 @@ NSString *const kTSOutgoingMessageSentRecipientAll = @"kTSOutgoingMessageSentRec OWSAssert(recipientId.length > 0); OWSAssert(transaction); - [self applyChangeToSelfAndLatestOutgoingMessage:transaction - changeBlock:^(TSOutgoingMessage *message) { - NSMutableDictionary *recipientReadMap - = (message.recipientReadMap ? [message.recipientReadMap mutableCopy] - : [NSMutableDictionary new]); - recipientReadMap[recipientId] = @(readTimestamp); - message.recipientReadMap = [recipientReadMap copy]; - }]; + [self applyChangeToSelfAndLatestCopy:transaction + changeBlock:^(TSOutgoingMessage *message) { + NSMutableDictionary *recipientReadMap + = (message.recipientReadMap ? [message.recipientReadMap mutableCopy] + : [NSMutableDictionary new]); + recipientReadMap[recipientId] = @(readTimestamp); + message.recipientReadMap = [recipientReadMap copy]; + }]; } - (nullable NSNumber *)firstRecipientReadTimestamp diff --git a/SignalServiceKit/src/Messages/OWSBatchMessageProcessor.m b/SignalServiceKit/src/Messages/OWSBatchMessageProcessor.m index ccef73244..f894f83d1 100644 --- a/SignalServiceKit/src/Messages/OWSBatchMessageProcessor.m +++ b/SignalServiceKit/src/Messages/OWSBatchMessageProcessor.m @@ -30,7 +30,7 @@ NS_ASSUME_NONNULL_BEGIN - (instancetype)initWithEnvelopeData:(NSData *)envelopeData plaintextData:(NSData *_Nullable)plaintextData NS_DESIGNATED_INITIALIZER; - (nullable instancetype)initWithCoder:(NSCoder *)coder NS_DESIGNATED_INITIALIZER; -- (instancetype)initWithUniqueId:(NSString *)uniqueId NS_UNAVAILABLE; +- (instancetype)initWithUniqueId:(NSString *_Nullable)uniqueId NS_UNAVAILABLE; - (OWSSignalServiceProtosEnvelope *)envelopeProto; @end diff --git a/SignalServiceKit/src/Messages/OWSMessageReceiver.m b/SignalServiceKit/src/Messages/OWSMessageReceiver.m index 33197ef31..6831f7da3 100644 --- a/SignalServiceKit/src/Messages/OWSMessageReceiver.m +++ b/SignalServiceKit/src/Messages/OWSMessageReceiver.m @@ -25,7 +25,7 @@ NS_ASSUME_NONNULL_BEGIN - (instancetype)initWithEnvelope:(OWSSignalServiceProtosEnvelope *)envelope NS_DESIGNATED_INITIALIZER; - (nullable instancetype)initWithCoder:(NSCoder *)coder NS_DESIGNATED_INITIALIZER; -- (instancetype)initWithUniqueId:(NSString *)uniqueId NS_UNAVAILABLE; +- (instancetype)initWithUniqueId:(NSString *_Nullable)uniqueId NS_UNAVAILABLE; - (OWSSignalServiceProtosEnvelope *)envelopeProto; @end diff --git a/SignalServiceKit/src/Security/OWSRecipientIdentity.h b/SignalServiceKit/src/Security/OWSRecipientIdentity.h index 7b5c82608..667f2f9e8 100644 --- a/SignalServiceKit/src/Security/OWSRecipientIdentity.h +++ b/SignalServiceKit/src/Security/OWSRecipientIdentity.h @@ -31,7 +31,7 @@ OWSSignalServiceProtosVerifiedState OWSVerificationStateToProtoState(OWSVerifica #pragma mark - Initializers -- (instancetype)initWithUniqueId:(NSString *)uniqueId NS_UNAVAILABLE; +- (instancetype)initWithUniqueId:(NSString *_Nullable)uniqueId NS_UNAVAILABLE; - (instancetype)initWithCoder:(NSCoder *)coder NS_DESIGNATED_INITIALIZER; diff --git a/SignalServiceKit/src/Storage/TSYapDatabaseObject.h b/SignalServiceKit/src/Storage/TSYapDatabaseObject.h index cb68f9496..1e47a8ca7 100644 --- a/SignalServiceKit/src/Storage/TSYapDatabaseObject.h +++ b/SignalServiceKit/src/Storage/TSYapDatabaseObject.h @@ -4,6 +4,8 @@ #import +NS_ASSUME_NONNULL_BEGIN + @class TSStorageManager; @class YapDatabaseConnection; @class YapDatabaseReadTransaction; @@ -18,8 +20,8 @@ * * @return Initialized object */ -- (instancetype)initWithUniqueId:(NSString *)uniqueId NS_DESIGNATED_INITIALIZER; -- (instancetype)initWithCoder:(NSCoder *)coder NS_DESIGNATED_INITIALIZER; +- (instancetype)initWithUniqueId:(NSString *_Nullable)uniqueId NS_DESIGNATED_INITIALIZER; +- (nullable instancetype)initWithCoder:(NSCoder *)coder NS_DESIGNATED_INITIALIZER; /** * Returns the collection to which the object belongs. @@ -77,8 +79,11 @@ * * @return Instance of the object or nil if non-existent */ -+ (instancetype)fetchObjectWithUniqueID:(NSString *)uniqueID transaction:(YapDatabaseReadTransaction *)transaction NS_SWIFT_NAME(fetch(uniqueId:transaction:)); -+ (instancetype)fetchObjectWithUniqueID:(NSString *)uniqueID NS_SWIFT_NAME(fetch(uniqueId:)); ++ (nullable instancetype)fetchObjectWithUniqueID:(NSString *)uniqueID + transaction:(YapDatabaseReadTransaction *)transaction + NS_SWIFT_NAME(fetch(uniqueId:transaction +:)); ++ (nullable instancetype)fetchObjectWithUniqueID:(NSString *)uniqueID NS_SWIFT_NAME(fetch(uniqueId:)); /** * Saves the object with the shared readWrite connection. @@ -113,9 +118,40 @@ /** * The unique identifier of the stored object */ -@property (nonatomic) NSString *uniqueId; +@property (nonatomic, nullable) NSString *uniqueId; - (void)removeWithTransaction:(YapDatabaseReadWriteTransaction *)transaction; - (void)remove; +#pragma mark Update With... + +// This method is used by "updateWith..." methods. +// +// This model may be updated from many threads. We don't want to save +// our local copy (this instance) since it may be out of date. We also +// want to avoid re-saving a model that has been deleted. Therefore, we +// use "updateWith..." methods to: +// +// a) Update a property of this instance. +// b) If a copy of this model exists in the database, load an up-to-date copy, +// and update and save that copy. +// b) If a copy of this model _DOES NOT_ exist in the database, do _NOT_ save +// this local instance. +// +// After "updateWith...": +// +// a) Any copy of this model in the database will have been updated. +// b) The local property on this instance will always have been updated. +// c) Other properties on this instance may be out of date. +// +// All mutable properties of this class have been made read-only to +// prevent accidentally modifying them directly. +// +// This isn't a perfect arrangement, but in practice this will prevent +// data loss and will resolve all known issues. +- (void)applyChangeToSelfAndLatestCopy:(YapDatabaseReadWriteTransaction *)transaction + changeBlock:(void (^)(id))changeBlock; + @end + +NS_ASSUME_NONNULL_END diff --git a/SignalServiceKit/src/Storage/TSYapDatabaseObject.m b/SignalServiceKit/src/Storage/TSYapDatabaseObject.m index ec392ccde..22c637e08 100644 --- a/SignalServiceKit/src/Storage/TSYapDatabaseObject.m +++ b/SignalServiceKit/src/Storage/TSYapDatabaseObject.m @@ -6,6 +6,8 @@ #import "TSStorageManager.h" #import +NS_ASSUME_NONNULL_BEGIN + @implementation TSYapDatabaseObject - (instancetype)init @@ -13,7 +15,7 @@ return [self initWithUniqueId:[[NSUUID UUID] UUIDString]]; } -- (instancetype)initWithUniqueId:(NSString *)aUniqueId +- (instancetype)initWithUniqueId:(NSString *_Nullable)aUniqueId { self = [super init]; if (!self) { @@ -25,6 +27,11 @@ return self; } +- (nullable instancetype)initWithCoder:(NSCoder *)coder +{ + return [super initWithCoder:coder]; +} + - (void)saveWithTransaction:(YapDatabaseReadWriteTransaction *)transaction { [transaction setObject:self forKey:self.uniqueId inCollection:[[self class] collection]]; @@ -176,18 +183,40 @@ }]; } -+ (instancetype)fetchObjectWithUniqueID:(NSString *)uniqueID transaction:(YapDatabaseReadTransaction *)transaction ++ (nullable instancetype)fetchObjectWithUniqueID:(NSString *)uniqueID + transaction:(YapDatabaseReadTransaction *)transaction { return [transaction objectForKey:uniqueID inCollection:[self collection]]; } -+ (instancetype)fetchObjectWithUniqueID:(NSString *)uniqueID ++ (nullable instancetype)fetchObjectWithUniqueID:(NSString *)uniqueID { - __block id object; + __block id _Nullable object = nil; [[self dbReadConnection] readWithBlock:^(YapDatabaseReadTransaction *transaction) { object = [transaction objectForKey:uniqueID inCollection:[self collection]]; }]; return object; } +#pragma mark Update With... + +// This method does the work for the "updateWith..." methods. Please see +// the header for a discussion of those methods. +- (void)applyChangeToSelfAndLatestCopy:(YapDatabaseReadWriteTransaction *)transaction + changeBlock:(void (^)(id))changeBlock +{ + OWSAssert(transaction); + + changeBlock(self); + + NSString *collection = [[self class] collection]; + id latestInstance = [transaction objectForKey:self.uniqueId inCollection:collection]; + if (latestInstance) { + changeBlock(latestInstance); + [latestInstance saveWithTransaction:transaction]; + } +} + @end + +NS_ASSUME_NONNULL_END From 19ba564f80c478d0808eedd54a787bc75a178a8a Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Wed, 15 Nov 2017 13:21:31 -0500 Subject: [PATCH 7/7] Respond to CR. // FREEBIE --- .../src/MultiDeviceProfileKeyUpdateJob.swift | 2 +- Signal/src/Profiles/OWSProfileManager.m | 2 +- .../ConversationViewController.m | 6 +-- .../ViewControllers/DebugUI/DebugUIMessages.m | 12 +++--- .../ViewControllers/DebugUI/DebugUIStress.m | 2 +- .../src/ViewControllers/HomeViewController.m | 2 +- .../ViewControllers/NewGroupViewController.m | 16 ++++--- .../OWSConversationSettingsViewController.m | 2 +- Signal/src/util/OWSContactsSyncing.m | 2 +- Signal/src/util/ThreadUtil.m | 4 +- .../src/Messages/OWSBlockingManager.m | 2 +- .../src/Messages/OWSIdentityManager.m | 4 +- .../src/Messages/OWSMessageManager.m | 12 +++--- .../src/Messages/OWSMessageSender.h | 32 +++++++------- .../src/Messages/OWSMessageSender.m | 42 +++++++++---------- ...oteOfUpdatedDisappearingConfigurationJob.m | 2 +- .../src/Messages/OWSReadReceiptManager.m | 6 +-- .../src/Storage/TSYapDatabaseObject.h | 3 +- 18 files changed, 75 insertions(+), 78 deletions(-) diff --git a/Signal/src/MultiDeviceProfileKeyUpdateJob.swift b/Signal/src/MultiDeviceProfileKeyUpdateJob.swift index a48ff8a36..406e662a4 100644 --- a/Signal/src/MultiDeviceProfileKeyUpdateJob.swift +++ b/Signal/src/MultiDeviceProfileKeyUpdateJob.swift @@ -44,7 +44,7 @@ import PromiseKit profileManager: self.profileManager) let dataSource = DataSourceValue.dataSource(withSyncMessage: syncContactsMessage.buildPlainTextAttachmentData()) - self.messageSender.enqueueOutgoingTemporaryAttachment(dataSource, + self.messageSender.enqueueTemporaryAttachment(dataSource, contentType: OWSMimeTypeApplicationOctetStream, in: syncContactsMessage, success: { diff --git a/Signal/src/Profiles/OWSProfileManager.m b/Signal/src/Profiles/OWSProfileManager.m index c2aa31baa..57bdbf215 100644 --- a/Signal/src/Profiles/OWSProfileManager.m +++ b/Signal/src/Profiles/OWSProfileManager.m @@ -1465,7 +1465,7 @@ const NSUInteger kOWSProfileManager_MaxAvatarDiameter = 640; return; } - [self.messageSender enqueueOutgoingMessage:message + [self.messageSender enqueueMessage:message success:^{ DDLogInfo(@"%@ Successfully sent profile key message to thread: %@", self.logTag, thread); [OWSProfileManager.sharedManager addThreadToProfileWhitelist:thread]; diff --git a/Signal/src/ViewControllers/ConversationView/ConversationViewController.m b/Signal/src/ViewControllers/ConversationView/ConversationViewController.m index d1425ec6c..d735a6a04 100644 --- a/Signal/src/ViewControllers/ConversationView/ConversationViewController.m +++ b/Signal/src/ViewControllers/ConversationView/ConversationViewController.m @@ -1654,7 +1654,7 @@ typedef NS_ENUM(NSInteger, MessagesRangeSizeMode) { [UIAlertAction actionWithTitle:NSLocalizedString(@"SEND_AGAIN_BUTTON", @"") style:UIAlertActionStyleDefault handler:^(UIAlertAction *_Nonnull action) { - [self.messageSender enqueueOutgoingMessage:message + [self.messageSender enqueueMessage:message success:^{ DDLogInfo(@"%@ Successfully resent failed message.", self.logTag); } @@ -3391,7 +3391,7 @@ typedef NS_ENUM(NSInteger, MessagesRangeSizeMode) { if (newGroupModel.groupImage) { NSData *data = UIImagePNGRepresentation(newGroupModel.groupImage); DataSource *_Nullable dataSource = [DataSourceValue dataSourceWithData:data fileExtension:@"png"]; - [self.messageSender enqueueOutgoingAttachment:dataSource + [self.messageSender enqueueAttachment:dataSource contentType:OWSMimeTypeImagePng sourceFilename:nil inMessage:message @@ -3405,7 +3405,7 @@ typedef NS_ENUM(NSInteger, MessagesRangeSizeMode) { DDLogError(@"%@ Failed to send group avatar update with error: %@", self.logTag, error); }]; } else { - [self.messageSender enqueueOutgoingMessage:message + [self.messageSender enqueueMessage:message success:^{ DDLogDebug(@"%@ Successfully sent group update", self.logTag); if (successCompletion) { diff --git a/Signal/src/ViewControllers/DebugUI/DebugUIMessages.m b/Signal/src/ViewControllers/DebugUI/DebugUIMessages.m index b70a2278e..ca9f92063 100644 --- a/Signal/src/ViewControllers/DebugUI/DebugUIMessages.m +++ b/Signal/src/ViewControllers/DebugUI/DebugUIMessages.m @@ -219,7 +219,7 @@ NS_ASSUME_NONNULL_BEGIN OWSSyncGroupsRequestMessage *syncGroupsRequestMessage = [[OWSSyncGroupsRequestMessage alloc] initWithThread:thread groupId:[Randomness generateRandomBytes:16]]; - [[Environment getCurrent].messageSender enqueueOutgoingMessage:syncGroupsRequestMessage + [[Environment getCurrent].messageSender enqueueMessage:syncGroupsRequestMessage success:^{ DDLogWarn(@"%@ Successfully sent Request Group Info message.", self.logTag); } @@ -1066,11 +1066,11 @@ NS_ASSUME_NONNULL_BEGIN }); }); }; - [messageSender enqueueOutgoingMessage:message - success:completion - failure:^(NSError *error) { - completion(); - }]; + [messageSender enqueueMessage:message + success:completion + failure:^(NSError *error) { + completion(); + }]; } + (void)injectFakeIncomingMessages:(int)counter thread:(TSThread *)thread diff --git a/Signal/src/ViewControllers/DebugUI/DebugUIStress.m b/Signal/src/ViewControllers/DebugUI/DebugUIStress.m index 68284d7ca..bf318a8cf 100644 --- a/Signal/src/ViewControllers/DebugUI/DebugUIStress.m +++ b/Signal/src/ViewControllers/DebugUI/DebugUIStress.m @@ -437,7 +437,7 @@ NS_ASSUME_NONNULL_BEGIN OWSAssert(message); OWSMessageSender *messageSender = [Environment getCurrent].messageSender; - [messageSender enqueueOutgoingMessage:message + [messageSender enqueueMessage:message success:^{ DDLogInfo(@"%@ Successfully sent message.", self.logTag); } diff --git a/Signal/src/ViewControllers/HomeViewController.m b/Signal/src/ViewControllers/HomeViewController.m index a05ccabaf..182caeb6f 100644 --- a/Signal/src/ViewControllers/HomeViewController.m +++ b/Signal/src/ViewControllers/HomeViewController.m @@ -679,7 +679,7 @@ typedef NS_ENUM(NSInteger, CellState) { kArchiveState, kInboxState }; TSOutgoingMessage *message = [[TSOutgoingMessage alloc] initWithTimestamp:[NSDate ows_millisecondTimeStamp] inThread:thread groupMetaMessage:TSGroupMessageQuit]; - [self.messageSender enqueueOutgoingMessage:message + [self.messageSender enqueueMessage:message success:^{ [self dismissViewControllerAnimated:YES completion:^{ diff --git a/Signal/src/ViewControllers/NewGroupViewController.m b/Signal/src/ViewControllers/NewGroupViewController.m index 016861e6c..87c4b3544 100644 --- a/Signal/src/ViewControllers/NewGroupViewController.m +++ b/Signal/src/ViewControllers/NewGroupViewController.m @@ -500,16 +500,14 @@ const NSUInteger kNewGroupViewControllerAvatarWidth = 68; NSData *data = UIImagePNGRepresentation(model.groupImage); DataSource *_Nullable dataSource = [DataSourceValue dataSourceWithData:data fileExtension:@"png"]; - [self.messageSender enqueueOutgoingAttachment:dataSource - contentType:OWSMimeTypeImagePng - sourceFilename:nil - inMessage:message - success:successHandler - failure:failureHandler]; + [self.messageSender enqueueAttachment:dataSource + contentType:OWSMimeTypeImagePng + sourceFilename:nil + inMessage:message + success:successHandler + failure:failureHandler]; } else { - [self.messageSender enqueueOutgoingMessage:message - success:successHandler - failure:failureHandler]; + [self.messageSender enqueueMessage:message success:successHandler failure:failureHandler]; } }); }]; diff --git a/Signal/src/ViewControllers/OWSConversationSettingsViewController.m b/Signal/src/ViewControllers/OWSConversationSettingsViewController.m index 29e732a43..73e7b1ee7 100644 --- a/Signal/src/ViewControllers/OWSConversationSettingsViewController.m +++ b/Signal/src/ViewControllers/OWSConversationSettingsViewController.m @@ -885,7 +885,7 @@ NS_ASSUME_NONNULL_BEGIN TSOutgoingMessage *message = [[TSOutgoingMessage alloc] initWithTimestamp:[NSDate ows_millisecondTimeStamp] inThread:gThread groupMetaMessage:TSGroupMessageQuit]; - [self.messageSender enqueueOutgoingMessage:message + [self.messageSender enqueueMessage:message success:^{ DDLogInfo(@"%@ Successfully left group.", self.logTag); } diff --git a/Signal/src/util/OWSContactsSyncing.m b/Signal/src/util/OWSContactsSyncing.m index 80dce55ff..4f778ccd5 100644 --- a/Signal/src/util/OWSContactsSyncing.m +++ b/Signal/src/util/OWSContactsSyncing.m @@ -115,7 +115,7 @@ NSString *const kTSStorageManagerOWSContactsSyncingLastMessageKey = DataSource *dataSource = [DataSourceValue dataSourceWithSyncMessage:[syncContactsMessage buildPlainTextAttachmentData]]; - [self.messageSender enqueueOutgoingTemporaryAttachment:dataSource + [self.messageSender enqueueTemporaryAttachment:dataSource contentType:OWSMimeTypeApplicationOctetStream inMessage:syncContactsMessage success:^{ diff --git a/Signal/src/util/ThreadUtil.m b/Signal/src/util/ThreadUtil.m index 8cd63f65f..00b6638ee 100644 --- a/Signal/src/util/ThreadUtil.m +++ b/Signal/src/util/ThreadUtil.m @@ -86,7 +86,7 @@ NS_ASSUME_NONNULL_BEGIN attachmentIds:[NSMutableArray new] expiresInSeconds:(configuration.isEnabled ? configuration.durationSeconds : 0)]; - [messageSender enqueueOutgoingMessage:message success:successHandler failure:failureHandler]; + [messageSender enqueueMessage:message success:successHandler failure:failureHandler]; return message; } @@ -117,7 +117,7 @@ NS_ASSUME_NONNULL_BEGIN inThread:thread isVoiceMessage:[attachment isVoiceMessage] expiresInSeconds:(configuration.isEnabled ? configuration.durationSeconds : 0)]; - [messageSender enqueueOutgoingAttachment:attachment.dataSource + [messageSender enqueueAttachment:attachment.dataSource contentType:attachment.mimeType sourceFilename:attachment.filenameOrDefault inMessage:message diff --git a/SignalServiceKit/src/Messages/OWSBlockingManager.m b/SignalServiceKit/src/Messages/OWSBlockingManager.m index c09bb10b5..050f75763 100644 --- a/SignalServiceKit/src/Messages/OWSBlockingManager.m +++ b/SignalServiceKit/src/Messages/OWSBlockingManager.m @@ -258,7 +258,7 @@ NSString *const kOWSBlockingManager_SyncedBlockedPhoneNumbersKey = @"kOWSBlockin OWSBlockedPhoneNumbersMessage *message = [[OWSBlockedPhoneNumbersMessage alloc] initWithPhoneNumbers:blockedPhoneNumbers]; - [self.messageSender enqueueOutgoingMessage:message + [self.messageSender enqueueMessage:message success:^{ DDLogInfo(@"%@ Successfully sent blocked phone numbers sync message", self.logTag); diff --git a/SignalServiceKit/src/Messages/OWSIdentityManager.m b/SignalServiceKit/src/Messages/OWSIdentityManager.m index 1ead01406..7f3abe158 100644 --- a/SignalServiceKit/src/Messages/OWSIdentityManager.m +++ b/SignalServiceKit/src/Messages/OWSIdentityManager.m @@ -517,10 +517,10 @@ NSString *const kNSNotificationName_IdentityStateDidChange = @"kNSNotificationNa // subsequently OWSOutgoingNullMessage *nullMessage = [[OWSOutgoingNullMessage alloc] initWithContactThread:contactThread verificationStateSyncMessage:message]; - [self.messageSender enqueueOutgoingMessage:nullMessage + [self.messageSender enqueueMessage:nullMessage success:^{ DDLogInfo(@"%@ Successfully sent verification state NullMessage", self.logTag); - [self.messageSender enqueueOutgoingMessage:message + [self.messageSender enqueueMessage:message success:^{ DDLogInfo(@"%@ Successfully sent verification state sync message", self.logTag); diff --git a/SignalServiceKit/src/Messages/OWSMessageManager.m b/SignalServiceKit/src/Messages/OWSMessageManager.m index 1ca4b9366..e74e898ab 100644 --- a/SignalServiceKit/src/Messages/OWSMessageManager.m +++ b/SignalServiceKit/src/Messages/OWSMessageManager.m @@ -366,7 +366,7 @@ NS_ASSUME_NONNULL_BEGIN OWSSyncGroupsRequestMessage *syncGroupsRequestMessage = [[OWSSyncGroupsRequestMessage alloc] initWithThread:thread groupId:groupId]; - [self.messageSender enqueueOutgoingMessage:syncGroupsRequestMessage + [self.messageSender enqueueMessage:syncGroupsRequestMessage success:^{ DDLogWarn(@"%@ Successfully sent Request Group Info message.", self.logTag); } @@ -609,7 +609,7 @@ NS_ASSUME_NONNULL_BEGIN profileManager:self.profileManager]; DataSource *dataSource = [DataSourceValue dataSourceWithSyncMessage:[syncContactsMessage buildPlainTextAttachmentData]]; - [self.messageSender enqueueOutgoingTemporaryAttachment:dataSource + [self.messageSender enqueueTemporaryAttachment:dataSource contentType:OWSMimeTypeApplicationOctetStream inMessage:syncContactsMessage success:^{ @@ -622,7 +622,7 @@ NS_ASSUME_NONNULL_BEGIN OWSSyncGroupsMessage *syncGroupsMessage = [[OWSSyncGroupsMessage alloc] init]; DataSource *dataSource = [DataSourceValue dataSourceWithSyncMessage:[syncGroupsMessage buildPlainTextAttachmentDataWithTransaction:transaction]]; - [self.messageSender enqueueOutgoingTemporaryAttachment:dataSource + [self.messageSender enqueueTemporaryAttachment:dataSource contentType:OWSMimeTypeApplicationOctetStream inMessage:syncGroupsMessage success:^{ @@ -639,7 +639,7 @@ NS_ASSUME_NONNULL_BEGIN [[OWSReadReceiptManager sharedManager] areReadReceiptsEnabledWithTransaction:transaction]; OWSSyncConfigurationMessage *syncConfigurationMessage = [[OWSSyncConfigurationMessage alloc] initWithReadReceiptsEnabled:areReadReceiptsEnabled]; - [self.messageSender enqueueOutgoingMessage:syncConfigurationMessage + [self.messageSender enqueueMessage:syncConfigurationMessage success:^{ DDLogInfo(@"%@ Successfully sent Configuration response syncMessage.", self.logTag); } @@ -774,7 +774,7 @@ NS_ASSUME_NONNULL_BEGIN if (gThread.groupModel.groupImage) { NSData *data = UIImagePNGRepresentation(gThread.groupModel.groupImage); DataSource *_Nullable dataSource = [DataSourceValue dataSourceWithData:data fileExtension:@"png"]; - [self.messageSender enqueueOutgoingAttachment:dataSource + [self.messageSender enqueueAttachment:dataSource contentType:OWSMimeTypeImagePng sourceFilename:nil inMessage:message @@ -785,7 +785,7 @@ NS_ASSUME_NONNULL_BEGIN DDLogError(@"%@ Failed to send group avatar update with error: %@", self.logTag, error); }]; } else { - [self.messageSender enqueueOutgoingMessage:message + [self.messageSender enqueueMessage:message success:^{ DDLogDebug(@"%@ Successfully sent group update", self.logTag); } diff --git a/SignalServiceKit/src/Messages/OWSMessageSender.h b/SignalServiceKit/src/Messages/OWSMessageSender.h index df17fa883..601a5828b 100644 --- a/SignalServiceKit/src/Messages/OWSMessageSender.h +++ b/SignalServiceKit/src/Messages/OWSMessageSender.h @@ -63,33 +63,33 @@ NS_SWIFT_NAME(MessageSender) /** * Send and resend text messages or resend messages with existing attachments. - * If you haven't yet created the attachment, see the ` enqueueOutgoingAttachment:` variants. + * If you haven't yet created the attachment, see the ` enqueueAttachment:` variants. */ // TODO: make transaction nonnull and remove `sendMessage:success:failure` -- (void)enqueueOutgoingMessage:(TSOutgoingMessage *)message - success:(void (^)(void))successHandler - failure:(void (^)(NSError *error))failureHandler; +- (void)enqueueMessage:(TSOutgoingMessage *)message + success:(void (^)(void))successHandler + failure:(void (^)(NSError *error))failureHandler; /** * Takes care of allocating and uploading the attachment, then sends the message. * Only necessary to call once. If sending fails, retry with `sendMessage:`. */ -- (void)enqueueOutgoingAttachment:(DataSource *)dataSource - contentType:(NSString *)contentType - sourceFilename:(nullable NSString *)sourceFilename - inMessage:(TSOutgoingMessage *)outgoingMessage - success:(void (^)(void))successHandler - failure:(void (^)(NSError *error))failureHandler; +- (void)enqueueAttachment:(DataSource *)dataSource + contentType:(NSString *)contentType + sourceFilename:(nullable NSString *)sourceFilename + inMessage:(TSOutgoingMessage *)outgoingMessage + success:(void (^)(void))successHandler + failure:(void (^)(NSError *error))failureHandler; /** - * Same as ` enqueueOutgoingAttachment:`, but deletes the local copy of the attachment after sending. + * Same as ` enqueueAttachment:`, but deletes the local copy of the attachment after sending. * Used for sending sync request data, not for user visible attachments. */ -- (void)enqueueOutgoingTemporaryAttachment:(DataSource *)dataSource - contentType:(NSString *)contentType - inMessage:(TSOutgoingMessage *)outgoingMessage - success:(void (^)(void))successHandler - failure:(void (^)(NSError *error))failureHandler; +- (void)enqueueTemporaryAttachment:(DataSource *)dataSource + contentType:(NSString *)contentType + inMessage:(TSOutgoingMessage *)outgoingMessage + success:(void (^)(void))successHandler + failure:(void (^)(NSError *error))failureHandler; /** * Set local configuration to match that of the of `outgoingMessage`'s sender diff --git a/SignalServiceKit/src/Messages/OWSMessageSender.m b/SignalServiceKit/src/Messages/OWSMessageSender.m index 84cba896f..f26bfa33e 100644 --- a/SignalServiceKit/src/Messages/OWSMessageSender.m +++ b/SignalServiceKit/src/Messages/OWSMessageSender.m @@ -419,9 +419,9 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException"; } } -- (void)enqueueOutgoingMessage:(TSOutgoingMessage *)message - success:(void (^)(void))successHandler - failure:(void (^)(NSError *error))failureHandler +- (void)enqueueMessage:(TSOutgoingMessage *)message + success:(void (^)(void))successHandler + failure:(void (^)(NSError *error))failureHandler { OWSAssert(message); @@ -509,11 +509,11 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException"; failure:failureHandler]; } -- (void)enqueueOutgoingTemporaryAttachment:(DataSource *)dataSource - contentType:(NSString *)contentType - inMessage:(TSOutgoingMessage *)message - success:(void (^)(void))successHandler - failure:(void (^)(NSError *error))failureHandler +- (void)enqueueTemporaryAttachment:(DataSource *)dataSource + contentType:(NSString *)contentType + inMessage:(TSOutgoingMessage *)message + success:(void (^)(void))successHandler + failure:(void (^)(NSError *error))failureHandler { OWSAssert(dataSource); @@ -531,20 +531,20 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException"; [message remove]; }; - [self enqueueOutgoingAttachment:dataSource - contentType:contentType - sourceFilename:nil - inMessage:message - success:successWithDeleteHandler - failure:failureWithDeleteHandler]; + [self enqueueAttachment:dataSource + contentType:contentType + sourceFilename:nil + inMessage:message + success:successWithDeleteHandler + failure:failureWithDeleteHandler]; } -- (void)enqueueOutgoingAttachment:(DataSource *)dataSource - contentType:(NSString *)contentType - sourceFilename:(nullable NSString *)sourceFilename - inMessage:(TSOutgoingMessage *)message - success:(void (^)(void))successHandler - failure:(void (^)(NSError *error))failureHandler +- (void)enqueueAttachment:(DataSource *)dataSource + contentType:(NSString *)contentType + sourceFilename:(nullable NSString *)sourceFilename + inMessage:(TSOutgoingMessage *)message + success:(void (^)(void))successHandler + failure:(void (^)(NSError *error))failureHandler { OWSAssert(dataSource); @@ -569,7 +569,7 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException"; message.attachmentFilenameMap[attachmentStream.uniqueId] = sourceFilename; } - [self enqueueOutgoingMessage:message success:successHandler failure:failureHandler]; + [self enqueueMessage:message success:successHandler failure:failureHandler]; }); } diff --git a/SignalServiceKit/src/Messages/OWSNotifyRemoteOfUpdatedDisappearingConfigurationJob.m b/SignalServiceKit/src/Messages/OWSNotifyRemoteOfUpdatedDisappearingConfigurationJob.m index ca9e0edf6..152deba75 100644 --- a/SignalServiceKit/src/Messages/OWSNotifyRemoteOfUpdatedDisappearingConfigurationJob.m +++ b/SignalServiceKit/src/Messages/OWSNotifyRemoteOfUpdatedDisappearingConfigurationJob.m @@ -49,7 +49,7 @@ NS_ASSUME_NONNULL_BEGIN [[OWSDisappearingMessagesConfigurationMessage alloc] initWithConfiguration:self.configuration thread:self.thread]; - [self.messageSender enqueueOutgoingMessage:message + [self.messageSender enqueueMessage:message success:^{ DDLogDebug( @"%@ Successfully notified %@ of new disappearing messages configuration", self.logTag, self.thread); diff --git a/SignalServiceKit/src/Messages/OWSReadReceiptManager.m b/SignalServiceKit/src/Messages/OWSReadReceiptManager.m index 5f95d2997..3b94df19e 100644 --- a/SignalServiceKit/src/Messages/OWSReadReceiptManager.m +++ b/SignalServiceKit/src/Messages/OWSReadReceiptManager.m @@ -229,7 +229,7 @@ NSString *const OWSReadReceiptManagerAreReadReceiptsEnabled = @"areReadReceiptsE OWSReadReceiptsForLinkedDevicesMessage *message = [[OWSReadReceiptsForLinkedDevicesMessage alloc] initWithReadReceipts:readReceiptsForLinkedDevices]; - [self.messageSender enqueueOutgoingMessage:message + [self.messageSender enqueueMessage:message success:^{ DDLogInfo(@"%@ Successfully sent %zd read receipt to linked devices.", self.logTag, @@ -253,7 +253,7 @@ NSString *const OWSReadReceiptManagerAreReadReceiptsEnabled = @"areReadReceiptsE [[OWSReadReceiptsForSenderMessage alloc] initWithThread:thread messageTimestamps:timestamps.allObjects]; - [self.messageSender enqueueOutgoingMessage:message + [self.messageSender enqueueMessage:message success:^{ DDLogInfo(@"%@ Successfully sent %zd read receipts to sender.", self.logTag, timestamps.count); } @@ -590,7 +590,7 @@ NSString *const OWSReadReceiptManagerAreReadReceiptsEnabled = @"areReadReceiptsE OWSSyncConfigurationMessage *syncConfigurationMessage = [[OWSSyncConfigurationMessage alloc] initWithReadReceiptsEnabled:value]; - [self.messageSender enqueueOutgoingMessage:syncConfigurationMessage + [self.messageSender enqueueMessage:syncConfigurationMessage success:^{ DDLogInfo(@"%@ Successfully sent Configuration syncMessage.", self.logTag); } diff --git a/SignalServiceKit/src/Storage/TSYapDatabaseObject.h b/SignalServiceKit/src/Storage/TSYapDatabaseObject.h index 1e47a8ca7..03f8f115c 100644 --- a/SignalServiceKit/src/Storage/TSYapDatabaseObject.h +++ b/SignalServiceKit/src/Storage/TSYapDatabaseObject.h @@ -81,8 +81,7 @@ NS_ASSUME_NONNULL_BEGIN */ + (nullable instancetype)fetchObjectWithUniqueID:(NSString *)uniqueID transaction:(YapDatabaseReadTransaction *)transaction - NS_SWIFT_NAME(fetch(uniqueId:transaction -:)); + NS_SWIFT_NAME(fetch(uniqueId:transaction:)); + (nullable instancetype)fetchObjectWithUniqueID:(NSString *)uniqueID NS_SWIFT_NAME(fetch(uniqueId:)); /**