Rework the "update with..." methods to avoid re-saving deleted models.

SignalServiceKit/src/Contacts/TSThread.m

@@ -384,7 +384,7 @@ 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
+- (void)applyChangeToSelfAndLatestThread:(YapDatabaseReadWriteTransaction *)transaction
                              changeBlock:(void (^)(TSThread *))changeBlock
 {
     OWSAssert(transaction);
@@ -402,7 +402,7 @@ NS_ASSUME_NONNULL_BEGIN
 - (void)updateWithMutedUntilDate:(NSDate *)mutedUntilDate
 {
     [self.dbReadWriteConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) {
-        [self applyChangeToSelfAndLatestCopy:transaction
+        [self applyChangeToSelfAndLatestThread:transaction
                                    changeBlock:^(TSThread *thread) {
                                        [thread setMutedUntilDate:mutedUntilDate];
                                    }];

SignalServiceKit/src/Messages/Interactions/TSMessage.h

@@ -15,9 +15,9 @@ NS_ASSUME_NONNULL_BEGIN
 @interface TSMessage : TSInteraction
 
 @property (nonatomic, readonly) NSMutableArray<NSString *> *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

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

SignalServiceKit/src/Messages/Interactions/TSOutgoingMessage.m

@@ -242,7 +242,7 @@ 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
+- (void)applyChangeToSelfAndLatestOutgoingMessage:(YapDatabaseReadWriteTransaction *)transaction
                                       changeBlock:(void (^)(TSOutgoingMessage *))changeBlock
 {
     OWSAssert(transaction);
@@ -262,7 +262,7 @@ NSString *const kTSOutgoingMessageSentRecipientAll = @"kTSOutgoingMessageSentRec
     OWSAssert(error);
 
     [self.dbReadWriteConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) {
-        [self applyChangeToSelfAndLatestCopy:transaction
+        [self applyChangeToSelfAndLatestOutgoingMessage:transaction
                                             changeBlock:^(TSOutgoingMessage *message) {
                                                 [message setMessageState:TSOutgoingMessageStateUnsent];
                                                 [message setMostRecentFailureText:error.localizedDescription];
@@ -282,7 +282,7 @@ NSString *const kTSOutgoingMessageSentRecipientAll = @"kTSOutgoingMessageSentRec
 {
     OWSAssert(transaction);
 
-    [self applyChangeToSelfAndLatestCopy:transaction
+    [self applyChangeToSelfAndLatestOutgoingMessage:transaction
                                         changeBlock:^(TSOutgoingMessage *message) {
                                             [message setMessageState:messageState];
                                         }];
@@ -291,7 +291,7 @@ NSString *const kTSOutgoingMessageSentRecipientAll = @"kTSOutgoingMessageSentRec
 - (void)updateWithHasSyncedTranscript:(BOOL)hasSyncedTranscript
                           transaction:(YapDatabaseReadWriteTransaction *)transaction
 {
-    [self applyChangeToSelfAndLatestCopy:transaction
+    [self applyChangeToSelfAndLatestOutgoingMessage:transaction
                                         changeBlock:^(TSOutgoingMessage *message) {
                                             [message setHasSyncedTranscript:hasSyncedTranscript];
                                         }];
@@ -302,7 +302,7 @@ NSString *const kTSOutgoingMessageSentRecipientAll = @"kTSOutgoingMessageSentRec
     OWSAssert(customMessage);
     OWSAssert(transaction);
 
-    [self applyChangeToSelfAndLatestCopy:transaction
+    [self applyChangeToSelfAndLatestOutgoingMessage:transaction
                                         changeBlock:^(TSOutgoingMessage *message) {
                                             [message setCustomMessage:customMessage];
                                         }];
@@ -322,12 +322,13 @@ NSString *const kTSOutgoingMessageSentRecipientAll = @"kTSOutgoingMessageSentRec
     OWSAssert(recipientId.length > 0);
     OWSAssert(transaction);
 
-    [self applyChangeToSelfAndLatestCopy:transaction
+    [self applyChangeToSelfAndLatestOutgoingMessage:transaction
                                         changeBlock:^(TSOutgoingMessage *message) {
 
                                             if (deliveryTimestamp) {
                                                 NSMutableDictionary<NSString *, NSNumber *> *recipientDeliveryMap
-                                         = (message.recipientDeliveryMap ? [message.recipientDeliveryMap mutableCopy]
+                                                    = (message.recipientDeliveryMap
+                                                            ? [message.recipientDeliveryMap mutableCopy]
                                                             : [NSMutableDictionary new]);
                                                 recipientDeliveryMap[recipientId] = deliveryTimestamp;
                                                 message.recipientDeliveryMap = [recipientDeliveryMap copy];
@@ -341,7 +342,7 @@ NSString *const kTSOutgoingMessageSentRecipientAll = @"kTSOutgoingMessageSentRec
 {
     OWSAssert(transaction);
 
-    [self applyChangeToSelfAndLatestCopy:transaction
+    [self applyChangeToSelfAndLatestOutgoingMessage:transaction
                                         changeBlock:^(TSOutgoingMessage *message) {
                                             [message setMessageState:TSOutgoingMessageStateSentToService];
                                             [message setWasDelivered:YES];
@@ -355,7 +356,7 @@ NSString *const kTSOutgoingMessageSentRecipientAll = @"kTSOutgoingMessageSentRec
     OWSAssert(transaction);
     OWSAssert(singleGroupRecipient.length > 0);
 
-    [self applyChangeToSelfAndLatestCopy:transaction
+    [self applyChangeToSelfAndLatestOutgoingMessage:transaction
                                         changeBlock:^(TSOutgoingMessage *message) {
                                             [message setSingleGroupRecipient:singleGroupRecipient];
                                         }];
@@ -410,7 +411,7 @@ NSString *const kTSOutgoingMessageSentRecipientAll = @"kTSOutgoingMessageSentRec
 - (void)updateWithSentRecipient:(NSString *)contactId transaction:(YapDatabaseReadWriteTransaction *)transaction
 {
     OWSAssert(transaction);
-    [self applyChangeToSelfAndLatestCopy:transaction
+    [self applyChangeToSelfAndLatestOutgoingMessage:transaction
                                         changeBlock:^(TSOutgoingMessage *message) {
                                             [message addSentRecipient:contactId];
                                         }];
@@ -423,7 +424,7 @@ NSString *const kTSOutgoingMessageSentRecipientAll = @"kTSOutgoingMessageSentRec
     OWSAssert(recipientId.length > 0);
     OWSAssert(transaction);
 
-    [self applyChangeToSelfAndLatestCopy:transaction
+    [self applyChangeToSelfAndLatestOutgoingMessage:transaction
                                         changeBlock:^(TSOutgoingMessage *message) {
                                             NSMutableDictionary<NSString *, NSNumber *> *recipientReadMap
                                                 = (message.recipientReadMap ? [message.recipientReadMap mutableCopy]

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.