From 57ae2b173fc1396e485f39d72517ffdf838966b2 Mon Sep 17 00:00:00 2001 From: Michael Kirk Date: Fri, 13 Apr 2018 15:59:16 -0400 Subject: [PATCH 1/8] Clarify existing functionality, but no change in behavior rename vars use clearer date comparison method // FREEBIE --- .../src/Messages/OWSDisappearingMessagesJob.m | 67 +++++++++++-------- 1 file changed, 38 insertions(+), 29 deletions(-) diff --git a/SignalServiceKit/src/Messages/OWSDisappearingMessagesJob.m b/SignalServiceKit/src/Messages/OWSDisappearingMessagesJob.m index 3f7e83eb5..feaa44b7e 100644 --- a/SignalServiceKit/src/Messages/OWSDisappearingMessagesJob.m +++ b/SignalServiceKit/src/Messages/OWSDisappearingMessagesJob.m @@ -96,12 +96,12 @@ NS_ASSUME_NONNULL_BEGIN [self.disappearingMessagesFinder enumerateExpiredMessagesWithBlock:^(TSMessage *message) { // sanity check if (message.expiresAt > now) { - DDLogError( + OWSFail( @"%@ Refusing to remove message which doesn't expire until: %lld", self.logTag, message.expiresAt); return; } - DDLogDebug(@"%@ Removing message which expired at: %lld", self.logTag, message.expiresAt); + DDLogInfo(@"%@ Removing message which expired at: %lld", self.logTag, message.expiresAt); [message removeWithTransaction:transaction]; expirationCount++; } @@ -130,12 +130,12 @@ NS_ASSUME_NONNULL_BEGIN // In theory we could kill the loop here. It should resume when the next expiring message is saved, // But this is a safeguard for any race conditions that exist while running the job as a new message is saved. DDLogDebug(@"%@ No more expiring messages.", self.logTag); - [self runLater]; + [self scheduleRunLater]; return; } uint64_t nextExpirationAt = [nextExpirationTimestampNumber unsignedLongLongValue]; - [self runByDate:[NSDate ows_dateWithMillisecondsSince1970:MAX(nextExpirationAt, now)]]; + [self scheduleRunByDate:[NSDate ows_dateWithMillisecondsSince1970:MAX(nextExpirationAt, now)]]; } + (void)setExpirationForMessage:(TSMessage *)message @@ -195,7 +195,7 @@ NS_ASSUME_NONNULL_BEGIN } // Necessary that the async expiration run happens *after* the message is saved with expiration configuration. - [self runByDate:[NSDate ows_dateWithMillisecondsSince1970:message.expiresAt]]; + [self scheduleRunByDate:[NSDate ows_dateWithMillisecondsSince1970:message.expiresAt]]; } + (void)setExpirationsForThread:(TSThread *)thread @@ -304,13 +304,13 @@ NS_ASSUME_NONNULL_BEGIN } self.hasStarted = YES; - [self runNow]; + [self scheduleRunNow]; }); } -- (void)runNow +- (void)scheduleRunNow { - [self runByDate:[NSDate new] ignoreMinDelay:YES]; + [self scheduleRunByDate:[NSDate new] ignoreMinDelay:YES]; } - (NSTimeInterval)maxDelaySeconds @@ -320,17 +320,31 @@ NS_ASSUME_NONNULL_BEGIN } // Waits the maximum amount of time to run again. -- (void)runLater +- (void)scheduleRunLater { - [self runByDate:[NSDate dateWithTimeIntervalSinceNow:self.maxDelaySeconds] ignoreMinDelay:YES]; + [self scheduleRunByDate:[NSDate dateWithTimeIntervalSinceNow:self.maxDelaySeconds] ignoreMinDelay:YES]; } -- (void)runByDate:(NSDate *)date +- (void)scheduleRunByDate:(NSDate *)date { - [self runByDate:date ignoreMinDelay:NO]; + [self scheduleRunByDate:date ignoreMinDelay:NO]; } -- (void)runByDate:(NSDate *)date ignoreMinDelay:(BOOL)ignoreMinDelay +- (NSDateFormatter *)dateFormatter +{ + static NSDateFormatter *dateFormatter; + static dispatch_once_t onceToken; + dispatch_once(&onceToken, ^{ + dateFormatter = [NSDateFormatter new]; + dateFormatter.dateStyle = NSDateFormatterNoStyle; + dateFormatter.timeStyle = kCFDateFormatterMediumStyle; + dateFormatter.locale = [NSLocale systemLocale]; + }); + + return dateFormatter; +} + +- (void)scheduleRunByDate:(NSDate *)date ignoreMinDelay:(BOOL)ignoreMinDelay { OWSAssert(date); @@ -340,22 +354,16 @@ NS_ASSUME_NONNULL_BEGIN return; } - NSDateFormatter *dateFormatter = [NSDateFormatter new]; - dateFormatter.dateStyle = NSDateFormatterNoStyle; - dateFormatter.timeStyle = kCFDateFormatterMediumStyle; - dateFormatter.locale = [NSLocale systemLocale]; - // Don't run more often than once per second. const NSTimeInterval kMinDelaySeconds = ignoreMinDelay ? 0.f : 1.f; - NSTimeInterval delaySeconds - = MAX(kMinDelaySeconds, MIN(self.maxDelaySeconds, [date timeIntervalSinceDate:[NSDate new]])); - NSDate *timerScheduleDate = [NSDate dateWithTimeIntervalSinceNow:delaySeconds]; - if (self.timerScheduleDate && [timerScheduleDate timeIntervalSinceDate:self.timerScheduleDate] > 0) { - DDLogVerbose(@"%@ Request to run at %@ (%d sec.) ignored due to scheduled run at %@ (%d sec.)", + NSTimeInterval delaySeconds = MAX(kMinDelaySeconds, MIN(self.maxDelaySeconds, date.timeIntervalSinceNow)); + NSDate *newTimerScheduleDate = [NSDate dateWithTimeIntervalSinceNow:delaySeconds]; + if (self.timerScheduleDate && [self.timerScheduleDate isBeforeDate:newTimerScheduleDate]) { + DDLogVerbose(@"%@ Request to run at %@ (%d sec.) ignored due to earlier scheduled run at %@ (%d sec.)", self.logTag, - [dateFormatter stringFromDate:date], + [self.dateFormatter stringFromDate:date], (int)round(MAX(0, [date timeIntervalSinceDate:[NSDate new]])), - [dateFormatter stringFromDate:self.timerScheduleDate], + [self.dateFormatter stringFromDate:self.timerScheduleDate], (int)round(MAX(0, [self.timerScheduleDate timeIntervalSinceDate:[NSDate new]]))); return; } @@ -363,10 +371,10 @@ NS_ASSUME_NONNULL_BEGIN // Update Schedule DDLogVerbose(@"%@ Scheduled run at %@ (%d sec.)", self.logTag, - [dateFormatter stringFromDate:timerScheduleDate], - (int)round(MAX(0, [timerScheduleDate timeIntervalSinceDate:[NSDate new]]))); + [self.dateFormatter stringFromDate:newTimerScheduleDate], + (int)round(MAX(0, [newTimerScheduleDate timeIntervalSinceDate:[NSDate new]]))); [self resetTimer]; - self.timerScheduleDate = timerScheduleDate; + self.timerScheduleDate = newTimerScheduleDate; self.timer = [NSTimer weakScheduledTimerWithTimeInterval:delaySeconds target:self selector:@selector(timerDidFire) @@ -378,6 +386,7 @@ NS_ASSUME_NONNULL_BEGIN - (void)timerDidFire { OWSAssertIsOnMainThread(); + DDLogDebug(@"%@ in %s", self.logTag, __PRETTY_FUNCTION__); if (!CurrentAppContext().isMainAppAndActive) { // Don't schedule run when inactive or not in main app. @@ -408,7 +417,7 @@ NS_ASSUME_NONNULL_BEGIN OWSAssertIsOnMainThread(); [AppReadiness runNowOrWhenAppIsReady:^{ - [self runNow]; + [self scheduleRunNow]; }]; } From b7625689cb11510960e8175b2d717807a02364dc Mon Sep 17 00:00:00 2001 From: Michael Kirk Date: Mon, 16 Apr 2018 11:18:39 -0400 Subject: [PATCH 2/8] Simplify reasoning around disappearing messages 1. Max duration between runs is now a separate timer we set up once and don't touch, so we can separate any potential bugs in scheduling logic. 2. When we want to "run now" we just run now, rather than going through the scheduler. 3. Detect if messages aren't being deleted in a timely way. // FREEBIE --- .../src/Messages/OWSDisappearingMessagesJob.m | 155 +++++++++++------- 1 file changed, 94 insertions(+), 61 deletions(-) diff --git a/SignalServiceKit/src/Messages/OWSDisappearingMessagesJob.m b/SignalServiceKit/src/Messages/OWSDisappearingMessagesJob.m index feaa44b7e..81c53c212 100644 --- a/SignalServiceKit/src/Messages/OWSDisappearingMessagesJob.m +++ b/SignalServiceKit/src/Messages/OWSDisappearingMessagesJob.m @@ -17,6 +17,7 @@ #import "TSMessage.h" NS_ASSUME_NONNULL_BEGIN + // Can we move to Signal-iOS? @interface OWSDisappearingMessagesJob () @@ -24,13 +25,25 @@ NS_ASSUME_NONNULL_BEGIN @property (nonatomic, readonly) OWSDisappearingMessagesFinder *disappearingMessagesFinder; ++ (dispatch_queue_t)serialQueue; + // These three properties should only be accessed on the main thread. @property (nonatomic) BOOL hasStarted; -@property (nonatomic, nullable) NSTimer *timer; -@property (nonatomic, nullable) NSDate *timerScheduleDate; +@property (nonatomic, nullable) NSTimer *nextDisappearanceTimer; +@property (nonatomic, nullable) NSDate *nextDisappearanceDate; +@property (nonatomic, nullable) NSTimer *fallbackTimer; @end +void AssertIsOnDisappearingMessagesQueue() +{ +#ifdef DEBUG + if (@available(iOS 10.0, *)) { + dispatch_assert_queue(OWSDisappearingMessagesJob.serialQueue); + } +#endif +} + #pragma mark - @implementation OWSDisappearingMessagesJob @@ -55,6 +68,18 @@ NS_ASSUME_NONNULL_BEGIN _databaseConnection = primaryStorage.newDatabaseConnection; _disappearingMessagesFinder = [OWSDisappearingMessagesFinder new]; + // suspenders in case a deletion schedule is missed. + NSTimeInterval kFallBackTimerInterval = 5 * kMinuteInterval; + [AppReadiness runNowOrWhenAppIsReady:^{ + if (CurrentAppContext().isMainApp) { + self->_fallbackTimer = [NSTimer weakScheduledTimerWithTimeInterval:kFallBackTimerInterval + target:self + selector:@selector(fallbackTimerDidFire) + userInfo:nil + repeats:YES]; + } + }]; + OWSSingletonAssert(); [[NSNotificationCenter defaultCenter] addObserver:self @@ -84,14 +109,15 @@ NS_ASSUME_NONNULL_BEGIN return queue; } -// This method should only be called on the serialQueue. -- (void)run +- (NSUInteger)deleteExpiredMessages { + AssertIsOnDisappearingMessagesQueue(); + uint64_t now = [NSDate ows_millisecondTimeStamp]; OWSBackgroundTask *_Nullable backgroundTask = [OWSBackgroundTask backgroundTaskWithLabelStr:__PRETTY_FUNCTION__]; - __block uint expirationCount = 0; + __block NSUInteger expirationCount = 0; [self.databaseConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *_Nonnull transaction) { [self.disappearingMessagesFinder enumerateExpiredMessagesWithBlock:^(TSMessage *message) { // sanity check @@ -108,34 +134,36 @@ NS_ASSUME_NONNULL_BEGIN transaction:transaction]; }]; - DDLogDebug(@"%@ Removed %u expired messages", self.logTag, expirationCount); + DDLogDebug(@"%@ Removed %tu expired messages", self.logTag, expirationCount); backgroundTask = nil; + return expirationCount; } -// This method should only be called on the serialQueue. -- (void)runLoop +// deletes any expired messages and schedules the next run. +- (NSUInteger)runLoop { - DDLogVerbose(@"%@ Run", self.logTag); + DDLogVerbose(@"%@ in runLoop", self.logTag); + AssertIsOnDisappearingMessagesQueue(); - [self run]; + NSUInteger deletedCount = [self deleteExpiredMessages]; - uint64_t now = [NSDate ows_millisecondTimeStamp]; __block NSNumber *nextExpirationTimestampNumber; [self.databaseConnection readWithBlock:^(YapDatabaseReadTransaction *_Nonnull transaction) { nextExpirationTimestampNumber = [self.disappearingMessagesFinder nextExpirationTimestampWithTransaction:transaction]; }]; + if (!nextExpirationTimestampNumber) { - // In theory we could kill the loop here. It should resume when the next expiring message is saved, - // But this is a safeguard for any race conditions that exist while running the job as a new message is saved. DDLogDebug(@"%@ No more expiring messages.", self.logTag); - [self scheduleRunLater]; - return; + return deletedCount; } - uint64_t nextExpirationAt = [nextExpirationTimestampNumber unsignedLongLongValue]; - [self scheduleRunByDate:[NSDate ows_dateWithMillisecondsSince1970:MAX(nextExpirationAt, now)]]; + uint64_t nextExpirationAt = nextExpirationTimestampNumber.unsignedLongLongValue; + NSDate *nextEpirationDate = [NSDate ows_dateWithMillisecondsSince1970:nextExpirationAt]; + [self scheduleRunByDate:nextEpirationDate]; + + return deletedCount; } + (void)setExpirationForMessage:(TSMessage *)message @@ -304,32 +332,12 @@ NS_ASSUME_NONNULL_BEGIN } self.hasStarted = YES; - [self scheduleRunNow]; + dispatch_async(OWSDisappearingMessagesJob.serialQueue, ^{ + [self runLoop]; + }); }); } -- (void)scheduleRunNow -{ - [self scheduleRunByDate:[NSDate new] ignoreMinDelay:YES]; -} - -- (NSTimeInterval)maxDelaySeconds -{ - // Don't run less often than once per N minutes. - return 5 * kMinuteInterval; -} - -// Waits the maximum amount of time to run again. -- (void)scheduleRunLater -{ - [self scheduleRunByDate:[NSDate dateWithTimeIntervalSinceNow:self.maxDelaySeconds] ignoreMinDelay:YES]; -} - -- (void)scheduleRunByDate:(NSDate *)date -{ - [self scheduleRunByDate:date ignoreMinDelay:NO]; -} - - (NSDateFormatter *)dateFormatter { static NSDateFormatter *dateFormatter; @@ -344,7 +352,7 @@ NS_ASSUME_NONNULL_BEGIN return dateFormatter; } -- (void)scheduleRunByDate:(NSDate *)date ignoreMinDelay:(BOOL)ignoreMinDelay +- (void)scheduleRunByDate:(NSDate *)date { OWSAssert(date); @@ -355,16 +363,16 @@ NS_ASSUME_NONNULL_BEGIN } // Don't run more often than once per second. - const NSTimeInterval kMinDelaySeconds = ignoreMinDelay ? 0.f : 1.f; - NSTimeInterval delaySeconds = MAX(kMinDelaySeconds, MIN(self.maxDelaySeconds, date.timeIntervalSinceNow)); + const NSTimeInterval kMinDelaySeconds = 1.0; + NSTimeInterval delaySeconds = MAX(kMinDelaySeconds, date.timeIntervalSinceNow); NSDate *newTimerScheduleDate = [NSDate dateWithTimeIntervalSinceNow:delaySeconds]; - if (self.timerScheduleDate && [self.timerScheduleDate isBeforeDate:newTimerScheduleDate]) { + if (self.nextDisappearanceDate && [self.nextDisappearanceDate isBeforeDate:newTimerScheduleDate]) { DDLogVerbose(@"%@ Request to run at %@ (%d sec.) ignored due to earlier scheduled run at %@ (%d sec.)", self.logTag, [self.dateFormatter stringFromDate:date], (int)round(MAX(0, [date timeIntervalSinceDate:[NSDate new]])), - [self.dateFormatter stringFromDate:self.timerScheduleDate], - (int)round(MAX(0, [self.timerScheduleDate timeIntervalSinceDate:[NSDate new]]))); + [self.dateFormatter stringFromDate:self.nextDisappearanceDate], + (int)round(MAX(0, [self.nextDisappearanceDate timeIntervalSinceDate:[NSDate new]]))); return; } @@ -373,17 +381,17 @@ NS_ASSUME_NONNULL_BEGIN self.logTag, [self.dateFormatter stringFromDate:newTimerScheduleDate], (int)round(MAX(0, [newTimerScheduleDate timeIntervalSinceDate:[NSDate new]]))); - [self resetTimer]; - self.timerScheduleDate = newTimerScheduleDate; - self.timer = [NSTimer weakScheduledTimerWithTimeInterval:delaySeconds - target:self - selector:@selector(timerDidFire) - userInfo:nil - repeats:NO]; + [self resetNextDisappearanceTimer]; + self.nextDisappearanceDate = newTimerScheduleDate; + self.nextDisappearanceTimer = [NSTimer weakScheduledTimerWithTimeInterval:delaySeconds + target:self + selector:@selector(disappearanceTimerDidFire) + userInfo:nil + repeats:NO]; }); } -- (void)timerDidFire +- (void)disappearanceTimerDidFire { OWSAssertIsOnMainThread(); DDLogDebug(@"%@ in %s", self.logTag, __PRETTY_FUNCTION__); @@ -394,20 +402,43 @@ NS_ASSUME_NONNULL_BEGIN return; } - [self resetTimer]; + [self resetNextDisappearanceTimer]; dispatch_async(OWSDisappearingMessagesJob.serialQueue, ^{ [self runLoop]; }); } -- (void)resetTimer +- (void)fallbackTimerDidFire +{ + OWSAssertIsOnMainThread(); + DDLogDebug(@"%@ in %s", self.logTag, __PRETTY_FUNCTION__); + + BOOL recentlyScheduledDisappearanceTimer = NO; + if (fabs(self.nextDisappearanceDate.timeIntervalSinceNow) < 1.0) { + recentlyScheduledDisappearanceTimer = YES; + } + + dispatch_async(OWSDisappearingMessagesJob.serialQueue, ^{ + NSUInteger deletedCount = [self runLoop]; + + // Normally deletions should happen via the disappearanceTimer, to make sure that they're timely. + // So, if we're deleting something via the fallback timer, something may have gone wrong. The + // exception is if we're in close proximity to the disappearanceTimer, in which case a race condition + // is inevitable. + if (!recentlyScheduledDisappearanceTimer && deletedCount > 0) { + OWSProdLogAndFail(@"%@ unexpectedly deleted disappearing messages via fallback timer."); + } + }); +} + +- (void)resetNextDisappearanceTimer { OWSAssertIsOnMainThread(); - [self.timer invalidate]; - self.timer = nil; - self.timerScheduleDate = nil; + [self.nextDisappearanceTimer invalidate]; + self.nextDisappearanceTimer = nil; + self.nextDisappearanceDate = nil; } #pragma mark - Notifications @@ -417,7 +448,9 @@ NS_ASSUME_NONNULL_BEGIN OWSAssertIsOnMainThread(); [AppReadiness runNowOrWhenAppIsReady:^{ - [self scheduleRunNow]; + dispatch_async(OWSDisappearingMessagesJob.serialQueue, ^{ + [self runLoop]; + }); }]; } @@ -425,7 +458,7 @@ NS_ASSUME_NONNULL_BEGIN { OWSAssertIsOnMainThread(); - [self resetTimer]; + [self resetNextDisappearanceTimer]; } @end From dfb2a034af81834fa2fea41beeb03e8e92868550 Mon Sep 17 00:00:00 2001 From: Michael Kirk Date: Mon, 16 Apr 2018 18:38:29 -0400 Subject: [PATCH 3/8] Use explicit transactions. - Start expiration within scope of existing transaction when we're already in a transaction // FREEBIE --- .../ViewControllers/DebugUI/DebugUIMessages.m | 8 +- .../OWSDisappearingMessagesConfiguration.h | 5 +- .../OWSDisappearingMessagesConfiguration.m | 4 +- SignalServiceKit/src/Contacts/TSThread.m | 2 +- .../src/Devices/OWSRecordTranscriptJob.m | 18 +- .../Messages/Interactions/TSErrorMessage.m | 3 +- .../Messages/Interactions/TSIncomingMessage.m | 9 +- .../src/Messages/Interactions/TSInfoMessage.m | 3 +- .../src/Messages/OWSDisappearingMessagesJob.h | 16 +- .../src/Messages/OWSDisappearingMessagesJob.m | 180 +++++++----------- .../src/Messages/OWSMessageManager.m | 7 +- .../src/Messages/OWSMessageSender.h | 8 - .../src/Messages/OWSMessageSender.m | 11 +- .../src/Messages/OWSReadReceiptManager.m | 21 +- .../src/Messages/OWSReadTracking.h | 7 +- SignalServiceKit/src/Messages/TSCall.m | 3 +- 16 files changed, 142 insertions(+), 163 deletions(-) diff --git a/Signal/src/ViewControllers/DebugUI/DebugUIMessages.m b/Signal/src/ViewControllers/DebugUI/DebugUIMessages.m index e38bafbf9..b0452a11f 100644 --- a/Signal/src/ViewControllers/DebugUI/DebugUIMessages.m +++ b/Signal/src/ViewControllers/DebugUI/DebugUIMessages.m @@ -3278,7 +3278,7 @@ NS_ASSUME_NONNULL_BEGIN attachmentIds:@[] expiresInSeconds:0 quotedMessage:nil]; - [message markAsReadWithTransaction:transaction sendReadReceipt:NO updateExpiration:NO]; + [message markAsReadWithTransaction:transaction sendReadReceipt:NO]; break; } case 1: { @@ -3316,7 +3316,7 @@ NS_ASSUME_NONNULL_BEGIN ] expiresInSeconds:0 quotedMessage:nil]; - [message markAsReadWithTransaction:transaction sendReadReceipt:NO updateExpiration:NO]; + [message markAsReadWithTransaction:transaction sendReadReceipt:NO]; break; } case 3: { @@ -3767,7 +3767,7 @@ NS_ASSUME_NONNULL_BEGIN attachmentIds:[NSMutableArray new] expiresInSeconds:0 quotedMessage:nil]; - [message markAsReadWithTransaction:transaction sendReadReceipt:NO updateExpiration:NO]; + [message markAsReadWithTransaction:transaction sendReadReceipt:NO]; } { TSOutgoingMessage *message = @@ -4105,7 +4105,7 @@ NS_ASSUME_NONNULL_BEGIN attachmentIds:attachmentIds expiresInSeconds:0 quotedMessage:quotedMessage]; - [message markAsReadWithTransaction:transaction sendReadReceipt:NO updateExpiration:NO]; + [message markAsReadWithTransaction:transaction sendReadReceipt:NO]; return message; } diff --git a/SignalServiceKit/src/Contacts/OWSDisappearingMessagesConfiguration.h b/SignalServiceKit/src/Contacts/OWSDisappearingMessagesConfiguration.h index dc3bdbf35..efd28287f 100644 --- a/SignalServiceKit/src/Contacts/OWSDisappearingMessagesConfiguration.h +++ b/SignalServiceKit/src/Contacts/OWSDisappearingMessagesConfiguration.h @@ -8,6 +8,8 @@ NS_ASSUME_NONNULL_BEGIN #define OWSDisappearingMessagesConfigurationDefaultExpirationDuration kDayInterval +@class YapDatabaseReadTransaction; + @interface OWSDisappearingMessagesConfiguration : TSYapDatabaseObject - (instancetype)initDefaultWithThreadId:(NSString *)threadId; @@ -21,7 +23,8 @@ NS_ASSUME_NONNULL_BEGIN @property (nonatomic, readonly) BOOL dictionaryValueDidChange; @property (readonly, getter=isNewRecord) BOOL newRecord; -+ (instancetype)fetchOrCreateDefaultWithThreadId:(NSString *)threadId; ++ (instancetype)fetchOrCreateDefaultWithThreadId:(NSString *)threadId + transaction:(YapDatabaseReadTransaction *)transaction; + (NSArray *)validDurationsSeconds; diff --git a/SignalServiceKit/src/Contacts/OWSDisappearingMessagesConfiguration.m b/SignalServiceKit/src/Contacts/OWSDisappearingMessagesConfiguration.m index 0ca190426..4fa0f343e 100644 --- a/SignalServiceKit/src/Contacts/OWSDisappearingMessagesConfiguration.m +++ b/SignalServiceKit/src/Contacts/OWSDisappearingMessagesConfiguration.m @@ -51,8 +51,10 @@ NS_ASSUME_NONNULL_BEGIN } + (instancetype)fetchOrCreateDefaultWithThreadId:(NSString *)threadId + transaction:(YapDatabaseReadTransaction *)transaction { - OWSDisappearingMessagesConfiguration *savedConfiguration = [self fetchObjectWithUniqueID:threadId]; + OWSDisappearingMessagesConfiguration *savedConfiguration = + [self fetchObjectWithUniqueID:threadId transaction:transaction]; if (savedConfiguration) { return savedConfiguration; } else { diff --git a/SignalServiceKit/src/Contacts/TSThread.m b/SignalServiceKit/src/Contacts/TSThread.m index b4798cdfa..78bfffeb9 100644 --- a/SignalServiceKit/src/Contacts/TSThread.m +++ b/SignalServiceKit/src/Contacts/TSThread.m @@ -230,7 +230,7 @@ NS_ASSUME_NONNULL_BEGIN - (void)markAllAsReadWithTransaction:(YapDatabaseReadWriteTransaction *)transaction { for (id message in [self unseenMessagesWithTransaction:transaction]) { - [message markAsReadWithTransaction:transaction sendReadReceipt:YES updateExpiration:YES]; + [message markAsReadWithTransaction:transaction sendReadReceipt:YES]; } // Just to be defensive, we'll also check for unread messages. diff --git a/SignalServiceKit/src/Devices/OWSRecordTranscriptJob.m b/SignalServiceKit/src/Devices/OWSRecordTranscriptJob.m index 72a2177dc..999b4d508 100644 --- a/SignalServiceKit/src/Devices/OWSRecordTranscriptJob.m +++ b/SignalServiceKit/src/Devices/OWSRecordTranscriptJob.m @@ -134,9 +134,13 @@ NS_ASSUME_NONNULL_BEGIN } if (transcript.isExpirationTimerUpdate) { - [OWSDisappearingMessagesJob becomeConsistentWithConfigurationForMessage:outgoingMessage - contactsManager:self.contactsManager]; + [[OWSDisappearingMessagesJob sharedJob] becomeConsistentWithConfigurationForMessage:outgoingMessage + contactsManager:self.contactsManager + transaction:transaction]; // early return to avoid saving an empty incoming message. + OWSAssert(transcript.body.length == 0); + OWSAssert(outgoingMessage.attachmentIds.count == 0); + return; } @@ -147,10 +151,12 @@ NS_ASSUME_NONNULL_BEGIN [outgoingMessage saveWithTransaction:transaction]; [outgoingMessage updateWithWasSentFromLinkedDeviceWithTransaction:transaction]; - [OWSDisappearingMessagesJob becomeConsistentWithConfigurationForMessage:outgoingMessage - contactsManager:self.contactsManager]; - [OWSDisappearingMessagesJob setExpirationForMessage:outgoingMessage - expirationStartedAt:transcript.expirationStartedAt]; + [[OWSDisappearingMessagesJob sharedJob] becomeConsistentWithConfigurationForMessage:outgoingMessage + contactsManager:self.contactsManager + transaction:transaction]; + [[OWSDisappearingMessagesJob sharedJob] setExpirationForMessage:outgoingMessage + expirationStartedAt:transcript.expirationStartedAt + transaction:transaction]; [self.readReceiptManager applyEarlyReadReceiptsForOutgoingMessageFromLinkedDevice:outgoingMessage transaction:transaction]; diff --git a/SignalServiceKit/src/Messages/Interactions/TSErrorMessage.m b/SignalServiceKit/src/Messages/Interactions/TSErrorMessage.m index 497aca868..97693e429 100644 --- a/SignalServiceKit/src/Messages/Interactions/TSErrorMessage.m +++ b/SignalServiceKit/src/Messages/Interactions/TSErrorMessage.m @@ -186,7 +186,6 @@ NSUInteger TSErrorMessageSchemaVersion = 1; - (void)markAsReadWithTransaction:(YapDatabaseReadWriteTransaction *)transaction sendReadReceipt:(BOOL)sendReadReceipt - updateExpiration:(BOOL)updateExpiration { OWSAssert(transaction); @@ -200,7 +199,7 @@ NSUInteger TSErrorMessageSchemaVersion = 1; [self saveWithTransaction:transaction]; [self touchThreadWithTransaction:transaction]; - // Ignore sendReadReceipt and updateExpiration; they don't apply to error messages. + // Ignore sendReadReceipt - it doesn't apply to error messages. } @end diff --git a/SignalServiceKit/src/Messages/Interactions/TSIncomingMessage.m b/SignalServiceKit/src/Messages/Interactions/TSIncomingMessage.m index 87209f0a1..e106cb28d 100644 --- a/SignalServiceKit/src/Messages/Interactions/TSIncomingMessage.m +++ b/SignalServiceKit/src/Messages/Interactions/TSIncomingMessage.m @@ -136,7 +136,6 @@ NS_ASSUME_NONNULL_BEGIN - (void)markAsReadWithTransaction:(YapDatabaseReadWriteTransaction *)transaction sendReadReceipt:(BOOL)sendReadReceipt - updateExpiration:(BOOL)updateExpiration { OWSAssert(transaction); @@ -149,10 +148,10 @@ NS_ASSUME_NONNULL_BEGIN _read = YES; [self saveWithTransaction:transaction]; [self touchThreadWithTransaction:transaction]; - - if (updateExpiration) { - [OWSDisappearingMessagesJob setExpirationForMessage:self]; - } + + [[OWSDisappearingMessagesJob sharedJob] startExpirationForMessage:self + transaction:transaction]; + if (sendReadReceipt) { [OWSReadReceiptManager.sharedManager messageWasReadLocally:self]; diff --git a/SignalServiceKit/src/Messages/Interactions/TSInfoMessage.m b/SignalServiceKit/src/Messages/Interactions/TSInfoMessage.m index 4c53c8cc5..11bb4075b 100644 --- a/SignalServiceKit/src/Messages/Interactions/TSInfoMessage.m +++ b/SignalServiceKit/src/Messages/Interactions/TSInfoMessage.m @@ -131,7 +131,6 @@ NSUInteger TSInfoMessageSchemaVersion = 1; - (void)markAsReadWithTransaction:(YapDatabaseReadWriteTransaction *)transaction sendReadReceipt:(BOOL)sendReadReceipt - updateExpiration:(BOOL)updateExpiration { OWSAssert(transaction); @@ -145,7 +144,7 @@ NSUInteger TSInfoMessageSchemaVersion = 1; [self saveWithTransaction:transaction]; [self touchThreadWithTransaction:transaction]; - // Ignore sendReadReceipt and updateExpiration; they don't apply to info messages. + // Ignore sendReadReceipt, it doesn't apply to info messages. } @end diff --git a/SignalServiceKit/src/Messages/OWSDisappearingMessagesJob.h b/SignalServiceKit/src/Messages/OWSDisappearingMessagesJob.h index 9b6820a89..5f80529c3 100644 --- a/SignalServiceKit/src/Messages/OWSDisappearingMessagesJob.h +++ b/SignalServiceKit/src/Messages/OWSDisappearingMessagesJob.h @@ -7,6 +7,7 @@ NS_ASSUME_NONNULL_BEGIN @class OWSPrimaryStorage; @class TSMessage; @class TSThread; +@class YapDatabaseReadWriteTransaction; @protocol ContactsManagerProtocol; @@ -16,9 +17,13 @@ NS_ASSUME_NONNULL_BEGIN - (instancetype)init NS_UNAVAILABLE; -+ (void)setExpirationsForThread:(TSThread *)thread; -+ (void)setExpirationForMessage:(TSMessage *)message; -+ (void)setExpirationForMessage:(TSMessage *)message expirationStartedAt:(uint64_t)expirationStartedAt; +//+ (void)setExpirationsForThread:(TSThread *)thread; +- (void)startExpirationForMessage:(TSMessage *)message + transaction:(YapDatabaseReadWriteTransaction *_Nonnull)transaction; + +- (void)setExpirationForMessage:(TSMessage *)message + expirationStartedAt:(uint64_t)expirationStartedAt + transaction:(YapDatabaseReadWriteTransaction *_Nonnull)transaction; /** * Synchronize our disappearing messages settings with that of the given message. Useful so we can @@ -31,8 +36,9 @@ NS_ASSUME_NONNULL_BEGIN * @param contactsManager * Provides the contact name responsible for any configuration changes in an info message. */ -+ (void)becomeConsistentWithConfigurationForMessage:(TSMessage *)message - contactsManager:(id)contactsManager; +- (void)becomeConsistentWithConfigurationForMessage:(TSMessage *)message + contactsManager:(id)contactsManager + transaction:(YapDatabaseReadWriteTransaction *)transaction; // Clean up any messages that expired since last launch immediately // and continue cleaning in the background. diff --git a/SignalServiceKit/src/Messages/OWSDisappearingMessagesJob.m b/SignalServiceKit/src/Messages/OWSDisappearingMessagesJob.m index 81c53c212..84521a033 100644 --- a/SignalServiceKit/src/Messages/OWSDisappearingMessagesJob.m +++ b/SignalServiceKit/src/Messages/OWSDisappearingMessagesJob.m @@ -166,42 +166,23 @@ void AssertIsOnDisappearingMessagesQueue() return deletedCount; } -+ (void)setExpirationForMessage:(TSMessage *)message -{ - dispatch_async(self.serialQueue, ^{ - [[self sharedJob] setExpirationForMessage:message]; - }); -} - -- (void)setExpirationForMessage:(TSMessage *)message +- (void)startExpirationForMessage:(TSMessage *)message + transaction:(YapDatabaseReadWriteTransaction *_Nonnull)transaction { if (!message.isExpiringMessage) { return; } OWSDisappearingMessagesConfiguration *disappearingConfig = - [OWSDisappearingMessagesConfiguration fetchObjectWithUniqueID:message.uniqueThreadId]; + [OWSDisappearingMessagesConfiguration fetchObjectWithUniqueID:message.uniqueThreadId transaction:transaction]; if (!disappearingConfig.isEnabled) { return; } - [self setExpirationForMessage:message expirationStartedAt:[NSDate ows_millisecondTimeStamp]]; -} - -+ (void)setExpirationForMessage:(TSMessage *)message expirationStartedAt:(uint64_t)expirationStartedAt -{ - dispatch_async(self.serialQueue, ^{ - [[self sharedJob] setExpirationForMessage:message expirationStartedAt:expirationStartedAt]; - }); -} - -// This method should only be called on the serialQueue. -- (void)setExpirationForMessage:(TSMessage *)message expirationStartedAt:(uint64_t)expirationStartedAt -{ - [self.databaseConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *_Nonnull transaction) { - [self setExpirationForMessage:message expirationStartedAt:expirationStartedAt transaction:transaction]; - }]; + [self setExpirationForMessage:message + expirationStartedAt:[NSDate ows_millisecondTimeStamp] + transaction:transaction]; } - (void)setExpirationForMessage:(TSMessage *)message @@ -222,106 +203,93 @@ void AssertIsOnDisappearingMessagesQueue() [message updateWithExpireStartedAt:expirationStartedAt transaction:transaction]; } - // Necessary that the async expiration run happens *after* the message is saved with expiration configuration. - [self scheduleRunByDate:[NSDate ows_dateWithMillisecondsSince1970:message.expiresAt]]; -} - -+ (void)setExpirationsForThread:(TSThread *)thread -{ - dispatch_async(self.serialQueue, ^{ - [[self sharedJob] setExpirationsForThread:thread]; - }); + [transaction addCompletionQueue:nil + completionBlock:^{ + // Necessary that the async expiration run happens *after* the message is saved with it's new + // expiration configuration. + [self scheduleRunByDate:[NSDate ows_dateWithMillisecondsSince1970:message.expiresAt]]; + }]; } -// This method should only be called on the serialQueue. -- (void)setExpirationsForThread:(TSThread *)thread +- (void)setExpirationsForThread:(TSThread *)thread transaction:(YapDatabaseReadWriteTransaction *)transaction { OWSBackgroundTask *_Nullable backgroundTask = [OWSBackgroundTask backgroundTaskWithLabelStr:__PRETTY_FUNCTION__]; uint64_t now = [NSDate ows_millisecondTimeStamp]; - [self.databaseConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *_Nonnull transaction) { - [self.disappearingMessagesFinder - enumerateUnstartedExpiringMessagesInThread:thread - block:^(TSMessage *_Nonnull message) { - DDLogWarn( - @"%@ Starting expiring message which should have already " - @"been started.", - self.logTag); - // specify "now" in case D.M. have since been disabled, but we have - // existing unstarted expiring messages that still need to expire. - [self setExpirationForMessage:message - expirationStartedAt:now - transaction:transaction]; - } - transaction:transaction]; - }]; + [self.disappearingMessagesFinder + enumerateUnstartedExpiringMessagesInThread:thread + block:^(TSMessage *_Nonnull message) { + DDLogWarn(@"%@ Starting expiring message which should have already " + @"been started.", + self.logTag); + // specify "now" in case D.M. have since been disabled, but we have + // existing unstarted expiring messages that still need to expire. + [self setExpirationForMessage:message + expirationStartedAt:now + transaction:transaction]; + } + transaction:transaction]; backgroundTask = nil; } -+ (void)becomeConsistentWithConfigurationForMessage:(TSMessage *)message - contactsManager:(id)contactsManager -{ - [[self sharedJob] becomeConsistentWithConfigurationForMessage:message contactsManager:contactsManager]; -} - - (void)becomeConsistentWithConfigurationForMessage:(TSMessage *)message contactsManager:(id)contactsManager + transaction:(YapDatabaseReadWriteTransaction *)transaction { OWSAssert(message); OWSAssert(contactsManager); - + __block OWSBackgroundTask *_Nullable backgroundTask = [OWSBackgroundTask backgroundTaskWithLabelStr:__PRETTY_FUNCTION__]; - - dispatch_async(OWSDisappearingMessagesJob.serialQueue, ^{ - // Become eventually consistent in the case that the remote changed their settings at the same time. - // Also in case remote doesn't support expiring messages - OWSDisappearingMessagesConfiguration *disappearingMessagesConfiguration = - [OWSDisappearingMessagesConfiguration fetchOrCreateDefaultWithThreadId:message.uniqueThreadId]; - - BOOL changed = NO; - if (message.expiresInSeconds == 0) { - if (disappearingMessagesConfiguration.isEnabled) { - changed = YES; - DDLogWarn(@"%@ Received remote message which had no expiration set, disabling our expiration to become " - @"consistent.", - self.logTag); - disappearingMessagesConfiguration.enabled = NO; - [disappearingMessagesConfiguration save]; - } - } else if (message.expiresInSeconds != disappearingMessagesConfiguration.durationSeconds) { + + // Become eventually consistent in the case that the remote changed their settings at the same time. + // Also in case remote doesn't support expiring messages + OWSDisappearingMessagesConfiguration *disappearingMessagesConfiguration = + [OWSDisappearingMessagesConfiguration fetchOrCreateDefaultWithThreadId:message.uniqueThreadId + transaction:transaction]; + + BOOL changed = NO; + if (message.expiresInSeconds == 0) { + if (disappearingMessagesConfiguration.isEnabled) { changed = YES; - DDLogInfo(@"%@ Received remote message with different expiration set, updating our expiration to become " + DDLogWarn(@"%@ Received remote message which had no expiration set, disabling our expiration to become " @"consistent.", - self.logTag); - disappearingMessagesConfiguration.enabled = YES; - disappearingMessagesConfiguration.durationSeconds = message.expiresInSeconds; - [disappearingMessagesConfiguration save]; - } - - if (!changed) { - return; - } - - if ([message isKindOfClass:[TSIncomingMessage class]]) { - TSIncomingMessage *incomingMessage = (TSIncomingMessage *)message; - NSString *contactName = [contactsManager displayNameForPhoneIdentifier:incomingMessage.messageAuthorId]; - - // We want the info message to appear _before_ the message. - [[[OWSDisappearingConfigurationUpdateInfoMessage alloc] initWithTimestamp:message.timestamp - 1 - thread:message.thread - configuration:disappearingMessagesConfiguration - createdByRemoteName:contactName] save]; - } else { - // We want the info message to appear _before_ the message. - [[[OWSDisappearingConfigurationUpdateInfoMessage alloc] initWithTimestamp:message.timestamp - 1 - thread:message.thread - configuration:disappearingMessagesConfiguration] - save]; + self.logTag); + disappearingMessagesConfiguration.enabled = NO; + [disappearingMessagesConfiguration saveWithTransaction:transaction]; } - - backgroundTask = nil; - }); + } else if (message.expiresInSeconds != disappearingMessagesConfiguration.durationSeconds) { + changed = YES; + DDLogInfo(@"%@ Received remote message with different expiration set, updating our expiration to become " + @"consistent.", + self.logTag); + disappearingMessagesConfiguration.enabled = YES; + disappearingMessagesConfiguration.durationSeconds = message.expiresInSeconds; + [disappearingMessagesConfiguration saveWithTransaction:transaction]; + } + + if (!changed) { + return; + } + + if ([message isKindOfClass:[TSIncomingMessage class]]) { + TSIncomingMessage *incomingMessage = (TSIncomingMessage *)message; + NSString *contactName = [contactsManager displayNameForPhoneIdentifier:incomingMessage.messageAuthorId]; + + // We want the info message to appear _before_ the message. + [[[OWSDisappearingConfigurationUpdateInfoMessage alloc] initWithTimestamp:message.timestamp - 1 + thread:message.thread + configuration:disappearingMessagesConfiguration + createdByRemoteName:contactName] saveWithTransaction:transaction]; + } else { + // We want the info message to appear _before_ the message. + [[[OWSDisappearingConfigurationUpdateInfoMessage alloc] initWithTimestamp:message.timestamp - 1 + thread:message.thread + configuration:disappearingMessagesConfiguration] + saveWithTransaction:transaction]; + } + + backgroundTask = nil; } - (void)startIfNecessary diff --git a/SignalServiceKit/src/Messages/OWSMessageManager.m b/SignalServiceKit/src/Messages/OWSMessageManager.m index 1c64c563b..1096823b6 100644 --- a/SignalServiceKit/src/Messages/OWSMessageManager.m +++ b/SignalServiceKit/src/Messages/OWSMessageManager.m @@ -1092,7 +1092,7 @@ NS_ASSUME_NONNULL_BEGIN BOOL shouldMarkMessageAsRead = [envelope.source isEqualToString:localNumber]; if (shouldMarkMessageAsRead) { // Don't send a read receipt for messages sent by ourselves. - [incomingMessage markAsReadWithTransaction:transaction sendReadReceipt:NO updateExpiration:YES]; + [incomingMessage markAsReadWithTransaction:transaction sendReadReceipt:NO]; } TSQuotedMessage *_Nullable quotedMessage = incomingMessage.quotedMessage; @@ -1130,8 +1130,9 @@ NS_ASSUME_NONNULL_BEGIN [OWSReadReceiptManager.sharedManager applyEarlyReadReceiptsForIncomingMessage:incomingMessage transaction:transaction]; - [OWSDisappearingMessagesJob becomeConsistentWithConfigurationForMessage:incomingMessage - contactsManager:self.contactsManager]; + [[OWSDisappearingMessagesJob sharedJob] becomeConsistentWithConfigurationForMessage:incomingMessage + contactsManager:self.contactsManager + transaction:transaction]; // Update thread preview in inbox [thread touchWithTransaction:transaction]; diff --git a/SignalServiceKit/src/Messages/OWSMessageSender.h b/SignalServiceKit/src/Messages/OWSMessageSender.h index 4539b9ff0..6b65f8f9b 100644 --- a/SignalServiceKit/src/Messages/OWSMessageSender.h +++ b/SignalServiceKit/src/Messages/OWSMessageSender.h @@ -84,14 +84,6 @@ NS_SWIFT_NAME(MessageSender) success:(void (^)(void))successHandler failure:(void (^)(NSError *error))failureHandler; -/** - * Set local configuration to match that of the of `outgoingMessage`'s sender - * - * We do this because messages and async message latency make it possible for thread participants disappearing messags - * configuration to get out of sync. - */ -- (void)becomeConsistentWithDisappearingConfigurationForMessage:(TSOutgoingMessage *)outgoingMessage; - @end NS_ASSUME_NONNULL_END diff --git a/SignalServiceKit/src/Messages/OWSMessageSender.m b/SignalServiceKit/src/Messages/OWSMessageSender.m index 266419d65..3040f0e59 100644 --- a/SignalServiceKit/src/Messages/OWSMessageSender.m +++ b/SignalServiceKit/src/Messages/OWSMessageSender.m @@ -1052,13 +1052,10 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException"; [self sendSyncTranscriptForMessage:message]; } - [OWSDisappearingMessagesJob setExpirationForMessage:message]; -} - -- (void)becomeConsistentWithDisappearingConfigurationForMessage:(TSOutgoingMessage *)outgoingMessage -{ - [OWSDisappearingMessagesJob becomeConsistentWithConfigurationForMessage:outgoingMessage - contactsManager:self.contactsManager]; + [self.dbConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { + [[OWSDisappearingMessagesJob sharedJob] startExpirationForMessage:message + transaction:transaction]; + }]; } - (void)handleSendToMyself:(TSOutgoingMessage *)outgoingMessage diff --git a/SignalServiceKit/src/Messages/OWSReadReceiptManager.m b/SignalServiceKit/src/Messages/OWSReadReceiptManager.m index 25587a5b5..a7f7392a4 100644 --- a/SignalServiceKit/src/Messages/OWSReadReceiptManager.m +++ b/SignalServiceKit/src/Messages/OWSReadReceiptManager.m @@ -431,11 +431,15 @@ NSString *const OWSReadReceiptManagerAreReadReceiptsEnabled = @"areReadReceiptsE if (!readReceipt) { return; } - [message markAsReadWithTransaction:transaction sendReadReceipt:NO updateExpiration:YES]; + [message markAsReadWithTransaction:transaction sendReadReceipt:NO]; [readReceipt removeWithTransaction:transaction]; - [[NSNotificationCenter defaultCenter] postNotificationNameAsync:kIncomingMessageMarkedAsReadNotification - object:message]; + [transaction addCompletionQueue:nil + completionBlock:^{ + [[NSNotificationCenter defaultCenter] + postNotificationNameAsync:kIncomingMessageMarkedAsReadNotification + object:message]; + }]; } - (void)processReadReceiptsFromLinkedDevice:(NSArray *)readReceiptProtos @@ -538,13 +542,18 @@ NSString *const OWSReadReceiptManagerAreReadReceiptsEnabled = @"areReadReceiptsE } else { DDLogError(@"Marking %zd messages as read by linked device.", interactions.count); } + for (id possiblyRead in interactions) { - [possiblyRead markAsReadWithTransaction:transaction sendReadReceipt:wasLocal updateExpiration:YES]; + [possiblyRead markAsReadWithTransaction:transaction sendReadReceipt:wasLocal]; if ([possiblyRead isKindOfClass:[TSIncomingMessage class]]) { TSIncomingMessage *incomingMessage = (TSIncomingMessage *)possiblyRead; - [[NSNotificationCenter defaultCenter] postNotificationNameAsync:kIncomingMessageMarkedAsReadNotification - object:incomingMessage]; + [transaction addCompletionQueue:nil + completionBlock:^{ + [[NSNotificationCenter defaultCenter] + postNotificationNameAsync:kIncomingMessageMarkedAsReadNotification + object:incomingMessage]; + }]; } } } diff --git a/SignalServiceKit/src/Messages/OWSReadTracking.h b/SignalServiceKit/src/Messages/OWSReadTracking.h index ed3dd27be..89122e8c5 100644 --- a/SignalServiceKit/src/Messages/OWSReadTracking.h +++ b/SignalServiceKit/src/Messages/OWSReadTracking.h @@ -1,5 +1,5 @@ // -// Copyright (c) 2017 Open Whisper Systems. All rights reserved. +// Copyright (c) 2018 Open Whisper Systems. All rights reserved. // @class YapDatabaseReadWriteTransaction; @@ -21,10 +21,9 @@ - (BOOL)shouldAffectUnreadCounts; /** - * Used for *responding* to a remote read receipt or in response to user activity. + * Used both for *responding* to a remote read receipt and in response to the local user's activity. */ - (void)markAsReadWithTransaction:(YapDatabaseReadWriteTransaction *)transaction - sendReadReceipt:(BOOL)sendReadReceipt - updateExpiration:(BOOL)updateExpiration; + sendReadReceipt:(BOOL)sendReadReceipt; @end diff --git a/SignalServiceKit/src/Messages/TSCall.m b/SignalServiceKit/src/Messages/TSCall.m index a0b05c891..28e7e38d0 100644 --- a/SignalServiceKit/src/Messages/TSCall.m +++ b/SignalServiceKit/src/Messages/TSCall.m @@ -96,7 +96,6 @@ NSUInteger TSCallCurrentSchemaVersion = 1; - (void)markAsReadWithTransaction:(YapDatabaseReadWriteTransaction *)transaction sendReadReceipt:(BOOL)sendReadReceipt - updateExpiration:(BOOL)updateExpiration { OWSAssert(transaction); @@ -110,7 +109,7 @@ NSUInteger TSCallCurrentSchemaVersion = 1; [self saveWithTransaction:transaction]; [self touchThreadWithTransaction:transaction]; - // Ignore sendReadReceipt and updateExpiration; they don't apply to calls. + // Ignore sendReadReceipt - it doesn't apply to calls. } #pragma mark - Methods From 754549adf18a9f0a60911ae104db4fc6dd1b3a6e Mon Sep 17 00:00:00 2001 From: Michael Kirk Date: Tue, 17 Apr 2018 20:53:27 -0400 Subject: [PATCH 4/8] Start timer for expiring message based on when read receipt was sent // FREEBIE --- .../ViewControllers/DebugUI/DebugUIMessages.m | 8 +- SignalMessaging/utils/ThreadUtil.h | 2 +- SignalServiceKit/src/Contacts/TSThread.m | 3 +- .../src/Devices/OWSLinkedDeviceReadReceipt.h | 11 ++- .../src/Devices/OWSLinkedDeviceReadReceipt.m | 58 +++++++++++--- .../OWSReadReceiptsForLinkedDevicesMessage.m | 4 +- .../Messages/Interactions/TSErrorMessage.m | 10 ++- .../Messages/Interactions/TSIncomingMessage.h | 4 + .../Messages/Interactions/TSIncomingMessage.m | 32 +++++--- .../src/Messages/Interactions/TSInfoMessage.m | 10 ++- .../src/Messages/OWSDisappearingMessagesJob.h | 5 +- .../src/Messages/OWSDisappearingMessagesJob.m | 55 +++++++------ .../src/Messages/OWSMessageManager.m | 11 +-- .../src/Messages/OWSMessageSender.m | 6 +- .../src/Messages/OWSReadReceiptManager.h | 5 +- .../src/Messages/OWSReadReceiptManager.m | 78 ++++++++++++------- .../src/Messages/OWSReadTracking.h | 7 +- SignalServiceKit/src/Messages/TSCall.m | 13 +++- 18 files changed, 211 insertions(+), 111 deletions(-) diff --git a/Signal/src/ViewControllers/DebugUI/DebugUIMessages.m b/Signal/src/ViewControllers/DebugUI/DebugUIMessages.m index b0452a11f..9e5aaad63 100644 --- a/Signal/src/ViewControllers/DebugUI/DebugUIMessages.m +++ b/Signal/src/ViewControllers/DebugUI/DebugUIMessages.m @@ -3278,7 +3278,7 @@ NS_ASSUME_NONNULL_BEGIN attachmentIds:@[] expiresInSeconds:0 quotedMessage:nil]; - [message markAsReadWithTransaction:transaction sendReadReceipt:NO]; + [message markAsReadNowWithSendReadReceipt:NO transaction:transaction]; break; } case 1: { @@ -3316,7 +3316,7 @@ NS_ASSUME_NONNULL_BEGIN ] expiresInSeconds:0 quotedMessage:nil]; - [message markAsReadWithTransaction:transaction sendReadReceipt:NO]; + [message markAsReadNowWithSendReadReceipt:NO transaction:transaction]; break; } case 3: { @@ -3767,7 +3767,7 @@ NS_ASSUME_NONNULL_BEGIN attachmentIds:[NSMutableArray new] expiresInSeconds:0 quotedMessage:nil]; - [message markAsReadWithTransaction:transaction sendReadReceipt:NO]; + [message markAsReadNowWithSendReadReceipt:NO transaction:transaction]; } { TSOutgoingMessage *message = @@ -4105,7 +4105,7 @@ NS_ASSUME_NONNULL_BEGIN attachmentIds:attachmentIds expiresInSeconds:0 quotedMessage:quotedMessage]; - [message markAsReadWithTransaction:transaction sendReadReceipt:NO]; + [message markAsReadNowWithSendReadReceipt:NO transaction:transaction]; return message; } diff --git a/SignalMessaging/utils/ThreadUtil.h b/SignalMessaging/utils/ThreadUtil.h index e6cefa0d6..130fbf492 100644 --- a/SignalMessaging/utils/ThreadUtil.h +++ b/SignalMessaging/utils/ThreadUtil.h @@ -26,7 +26,7 @@ NS_ASSUME_NONNULL_BEGIN @property (nonatomic, nullable, readonly) NSNumber *unreadIndicatorPosition; // If there are unseen messages in the thread, this is the timestamp -// of the oldest unseen messaage. +// of the oldest unseen message. // // Once we enter messages view, we mark all messages read, so we need // a snapshot of what the first unread message was when we entered the diff --git a/SignalServiceKit/src/Contacts/TSThread.m b/SignalServiceKit/src/Contacts/TSThread.m index 78bfffeb9..f9cc5fe4c 100644 --- a/SignalServiceKit/src/Contacts/TSThread.m +++ b/SignalServiceKit/src/Contacts/TSThread.m @@ -3,6 +3,7 @@ // #import "TSThread.h" +#import "NSDate+OWS.h" #import "OWSPrimaryStorage.h" #import "OWSReadTracking.h" #import "TSDatabaseView.h" @@ -230,7 +231,7 @@ NS_ASSUME_NONNULL_BEGIN - (void)markAllAsReadWithTransaction:(YapDatabaseReadWriteTransaction *)transaction { for (id message in [self unseenMessagesWithTransaction:transaction]) { - [message markAsReadWithTransaction:transaction sendReadReceipt:YES]; + [message markAsReadAtTimestamp:[NSDate ows_millisecondTimeStamp] sendReadReceipt:YES transaction:transaction]; } // Just to be defensive, we'll also check for unread messages. diff --git a/SignalServiceKit/src/Devices/OWSLinkedDeviceReadReceipt.h b/SignalServiceKit/src/Devices/OWSLinkedDeviceReadReceipt.h index 8a4728c56..eb5d5f79a 100644 --- a/SignalServiceKit/src/Devices/OWSLinkedDeviceReadReceipt.h +++ b/SignalServiceKit/src/Devices/OWSLinkedDeviceReadReceipt.h @@ -1,5 +1,5 @@ // -// Copyright (c) 2017 Open Whisper Systems. All rights reserved. +// Copyright (c) 2018 Open Whisper Systems. All rights reserved. // #import "TSYapDatabaseObject.h" @@ -9,12 +9,15 @@ NS_ASSUME_NONNULL_BEGIN @interface OWSLinkedDeviceReadReceipt : TSYapDatabaseObject @property (nonatomic, readonly) NSString *senderId; -@property (nonatomic, readonly) uint64_t timestamp; +@property (nonatomic, readonly) uint64_t messageIdTimestamp; +@property (nonatomic, readonly) uint64_t readTimestamp; -- (instancetype)initWithSenderId:(NSString *)senderId timestamp:(uint64_t)timestamp; +- (instancetype)initWithSenderId:(NSString *)senderId + messageIdTimestamp:(uint64_t)messageIdtimestamp + readTimestamp:(uint64_t)readTimestamp; + (nullable OWSLinkedDeviceReadReceipt *)findLinkedDeviceReadReceiptWithSenderId:(NSString *)senderId - timestamp:(uint64_t)timestamp + messageIdTimestamp:(uint64_t)messageIdTimestamp transaction: (YapDatabaseReadTransaction *)transaction; diff --git a/SignalServiceKit/src/Devices/OWSLinkedDeviceReadReceipt.m b/SignalServiceKit/src/Devices/OWSLinkedDeviceReadReceipt.m index 8a402067a..c7dacaa9b 100644 --- a/SignalServiceKit/src/Devices/OWSLinkedDeviceReadReceipt.m +++ b/SignalServiceKit/src/Devices/OWSLinkedDeviceReadReceipt.m @@ -6,39 +6,75 @@ NS_ASSUME_NONNULL_BEGIN +@interface OWSLinkedDeviceReadReceipt () + +// FIXME remove this `timestamp` property and migrate in initWithCoder. +@property (nonatomic, readonly) uint64_t timestamp; + +@end + @implementation OWSLinkedDeviceReadReceipt -- (instancetype)initWithSenderId:(NSString *)senderId timestamp:(uint64_t)timestamp +- (instancetype)initWithSenderId:(NSString *)senderId + messageIdTimestamp:(uint64_t)messageIdTimestamp + readTimestamp:(uint64_t)readTimestamp { - OWSAssert(senderId.length > 0 && timestamp > 0); + OWSAssert(senderId.length > 0 && messageIdTimestamp > 0); - self = [super initWithUniqueId:[OWSLinkedDeviceReadReceipt uniqueIdForSenderId:senderId timestamp:timestamp]]; + NSString *receiptId = + [OWSLinkedDeviceReadReceipt uniqueIdForSenderId:senderId messageIdTimestamp:messageIdTimestamp]; + self = [super initWithUniqueId:receiptId]; if (!self) { return self; } _senderId = senderId; - _timestamp = timestamp; + _messageIdTimestamp = messageIdTimestamp; + _readTimestamp = readTimestamp; return self; } -+ (NSString *)uniqueIdForSenderId:(NSString *)senderId timestamp:(uint64_t)timestamp +- (nullable instancetype)initWithCoder:(NSCoder *)coder { - OWSAssert(senderId.length > 0 && timestamp > 0); + self = [super initWithCoder:coder]; + if (!self) { + return self; + } + + if (!_messageIdTimestamp) { + // FIXME to remove this legacy `timestamp` property, we need to figure out exactly how MTL encodes uint64_t. + // e.g. can we just do something like: `((NSNumber *)[coder decodeObjectForKey:@"timestamp"]).unsignedLongLong` + _messageIdTimestamp = _timestamp; + } - return [NSString stringWithFormat:@"%@-%llu", senderId, timestamp]; + // For legacy early LinkedDeviceReadReceipts, before we were tracking read time, we assume the message was read as + // soon as it was sent. This is always going to be at least a little earlier than it was actually read, but we have + // nothing better to choose, and by the very fact that we're receiving a read receipt, we have good reason to + // believe they read the message on the other device. + if (_readTimestamp == 0) { + _readTimestamp = _timestamp; + } + + return self; +} + ++ (NSString *)uniqueIdForSenderId:(NSString *)senderId messageIdTimestamp:(uint64_t)messageIdTimestamp +{ + OWSAssert(senderId.length > 0 && messageIdTimestamp > 0); + + return [NSString stringWithFormat:@"%@-%llu", senderId, messageIdTimestamp]; } + (nullable OWSLinkedDeviceReadReceipt *)findLinkedDeviceReadReceiptWithSenderId:(NSString *)senderId - timestamp:(uint64_t)timestamp + messageIdTimestamp:(uint64_t)messageIdTimestamp transaction: (YapDatabaseReadTransaction *)transaction { OWSAssert(transaction); - - return [OWSLinkedDeviceReadReceipt fetchObjectWithUniqueID:[self uniqueIdForSenderId:senderId timestamp:timestamp] - transaction:transaction]; + NSString *receiptId = + [OWSLinkedDeviceReadReceipt uniqueIdForSenderId:senderId messageIdTimestamp:messageIdTimestamp]; + return [OWSLinkedDeviceReadReceipt fetchObjectWithUniqueID:receiptId transaction:transaction]; } @end diff --git a/SignalServiceKit/src/Devices/OWSReadReceiptsForLinkedDevicesMessage.m b/SignalServiceKit/src/Devices/OWSReadReceiptsForLinkedDevicesMessage.m index a72f10257..8ffde41e1 100644 --- a/SignalServiceKit/src/Devices/OWSReadReceiptsForLinkedDevicesMessage.m +++ b/SignalServiceKit/src/Devices/OWSReadReceiptsForLinkedDevicesMessage.m @@ -1,5 +1,5 @@ // -// Copyright (c) 2017 Open Whisper Systems. All rights reserved. +// Copyright (c) 2018 Open Whisper Systems. All rights reserved. // #import "OWSReadReceiptsForLinkedDevicesMessage.h" @@ -35,7 +35,7 @@ NS_ASSUME_NONNULL_BEGIN OWSSignalServiceProtosSyncMessageReadBuilder *readProtoBuilder = [OWSSignalServiceProtosSyncMessageReadBuilder new]; [readProtoBuilder setSender:readReceipt.senderId]; - [readProtoBuilder setTimestamp:readReceipt.timestamp]; + [readProtoBuilder setTimestamp:readReceipt.messageIdTimestamp]; [syncMessageBuilder addRead:[readProtoBuilder build]]; } diff --git a/SignalServiceKit/src/Messages/Interactions/TSErrorMessage.m b/SignalServiceKit/src/Messages/Interactions/TSErrorMessage.m index 97693e429..363605a73 100644 --- a/SignalServiceKit/src/Messages/Interactions/TSErrorMessage.m +++ b/SignalServiceKit/src/Messages/Interactions/TSErrorMessage.m @@ -179,13 +179,19 @@ NSUInteger TSErrorMessageSchemaVersion = 1; #pragma mark - OWSReadTracking +- (uint64_t)expireStartedAt +{ + return 0; +} + - (BOOL)shouldAffectUnreadCounts { return NO; } -- (void)markAsReadWithTransaction:(YapDatabaseReadWriteTransaction *)transaction - sendReadReceipt:(BOOL)sendReadReceipt +- (void)markAsReadAtTimestamp:(uint64_t)readTimestamp + sendReadReceipt:(BOOL)sendReadReceipt + transaction:(YapDatabaseReadWriteTransaction *)transaction { OWSAssert(transaction); diff --git a/SignalServiceKit/src/Messages/Interactions/TSIncomingMessage.h b/SignalServiceKit/src/Messages/Interactions/TSIncomingMessage.h index 823b22515..3e5f49fdc 100644 --- a/SignalServiceKit/src/Messages/Interactions/TSIncomingMessage.h +++ b/SignalServiceKit/src/Messages/Interactions/TSIncomingMessage.h @@ -74,6 +74,10 @@ NS_ASSUME_NONNULL_BEGIN - (NSString *)messageAuthorId; +// convenience method for expiring a message which was just read +- (void)markAsReadNowWithSendReadReceipt:(BOOL)sendReadReceipt + transaction:(YapDatabaseReadWriteTransaction *)transaction; + @end NS_ASSUME_NONNULL_END diff --git a/SignalServiceKit/src/Messages/Interactions/TSIncomingMessage.m b/SignalServiceKit/src/Messages/Interactions/TSIncomingMessage.m index e106cb28d..97944bc4f 100644 --- a/SignalServiceKit/src/Messages/Interactions/TSIncomingMessage.m +++ b/SignalServiceKit/src/Messages/Interactions/TSIncomingMessage.m @@ -3,6 +3,7 @@ // #import "TSIncomingMessage.h" +#import "NSDate+OWS.h" #import "NSNotificationCenter+OWS.h" #import "OWSDisappearingMessagesConfiguration.h" #import "OWSDisappearingMessagesJob.h" @@ -134,24 +135,37 @@ NS_ASSUME_NONNULL_BEGIN return YES; } -- (void)markAsReadWithTransaction:(YapDatabaseReadWriteTransaction *)transaction - sendReadReceipt:(BOOL)sendReadReceipt +- (void)markAsReadNowWithSendReadReceipt:(BOOL)sendReadReceipt + transaction:(YapDatabaseReadWriteTransaction *)transaction; +{ + [self markAsReadAtTimestamp:[NSDate ows_millisecondTimeStamp] + sendReadReceipt:sendReadReceipt + transaction:transaction]; +} + +- (void)markAsReadAtTimestamp:(uint64_t)readTimestamp + sendReadReceipt:(BOOL)sendReadReceipt + transaction:(YapDatabaseReadWriteTransaction *)transaction; { OWSAssert(transaction); - if (_read) { + if (_read && readTimestamp <= self.expireStartedAt) { return; } - DDLogDebug( - @"%@ marking as read uniqueId: %@ which has timestamp: %llu", self.logTag, self.uniqueId, self.timestamp); + NSTimeInterval secondsAgoRead = ([NSDate ows_millisecondTimeStamp] - readTimestamp) / 1000; + DDLogDebug(@"%@ marking uniqueId: %@ which has timestamp: %llu as read: %f seconds ago", + self.logTag, + self.uniqueId, + self.timestamp, + secondsAgoRead); _read = YES; [self saveWithTransaction:transaction]; [self touchThreadWithTransaction:transaction]; - - [[OWSDisappearingMessagesJob sharedJob] startExpirationForMessage:self - transaction:transaction]; - + + [[OWSDisappearingMessagesJob sharedJob] startAnyExpirationForMessage:self + expirationStartedAt:readTimestamp + transaction:transaction]; if (sendReadReceipt) { [OWSReadReceiptManager.sharedManager messageWasReadLocally:self]; diff --git a/SignalServiceKit/src/Messages/Interactions/TSInfoMessage.m b/SignalServiceKit/src/Messages/Interactions/TSInfoMessage.m index 11bb4075b..10abeb18b 100644 --- a/SignalServiceKit/src/Messages/Interactions/TSInfoMessage.m +++ b/SignalServiceKit/src/Messages/Interactions/TSInfoMessage.m @@ -129,8 +129,14 @@ NSUInteger TSInfoMessageSchemaVersion = 1; return NO; } -- (void)markAsReadWithTransaction:(YapDatabaseReadWriteTransaction *)transaction - sendReadReceipt:(BOOL)sendReadReceipt +- (uint64_t)expireStartedAt +{ + return 0; +} + +- (void)markAsReadAtTimestamp:(uint64_t)readTimestamp + sendReadReceipt:(BOOL)sendReadReceipt + transaction:(YapDatabaseReadWriteTransaction *)transaction { OWSAssert(transaction); diff --git a/SignalServiceKit/src/Messages/OWSDisappearingMessagesJob.h b/SignalServiceKit/src/Messages/OWSDisappearingMessagesJob.h index 5f80529c3..dd7ef9ec4 100644 --- a/SignalServiceKit/src/Messages/OWSDisappearingMessagesJob.h +++ b/SignalServiceKit/src/Messages/OWSDisappearingMessagesJob.h @@ -18,8 +18,9 @@ NS_ASSUME_NONNULL_BEGIN - (instancetype)init NS_UNAVAILABLE; //+ (void)setExpirationsForThread:(TSThread *)thread; -- (void)startExpirationForMessage:(TSMessage *)message - transaction:(YapDatabaseReadWriteTransaction *_Nonnull)transaction; +- (void)startAnyExpirationForMessage:(TSMessage *)message + expirationStartedAt:(uint64_t)expirationStartedAt + transaction:(YapDatabaseReadWriteTransaction *_Nonnull)transaction; - (void)setExpirationForMessage:(TSMessage *)message expirationStartedAt:(uint64_t)expirationStartedAt diff --git a/SignalServiceKit/src/Messages/OWSDisappearingMessagesJob.m b/SignalServiceKit/src/Messages/OWSDisappearingMessagesJob.m index 84521a033..4c92811b3 100644 --- a/SignalServiceKit/src/Messages/OWSDisappearingMessagesJob.m +++ b/SignalServiceKit/src/Messages/OWSDisappearingMessagesJob.m @@ -166,8 +166,9 @@ void AssertIsOnDisappearingMessagesQueue() return deletedCount; } -- (void)startExpirationForMessage:(TSMessage *)message - transaction:(YapDatabaseReadWriteTransaction *_Nonnull)transaction +- (void)startAnyExpirationForMessage:(TSMessage *)message + expirationStartedAt:(uint64_t)expirationStartedAt + transaction:(YapDatabaseReadWriteTransaction *_Nonnull)transaction { if (!message.isExpiringMessage) { return; @@ -180,9 +181,7 @@ void AssertIsOnDisappearingMessagesQueue() return; } - [self setExpirationForMessage:message - expirationStartedAt:[NSDate ows_millisecondTimeStamp] - transaction:transaction]; + [self setExpirationForMessage:message expirationStartedAt:expirationStartedAt transaction:transaction]; } - (void)setExpirationForMessage:(TSMessage *)message @@ -195,8 +194,8 @@ void AssertIsOnDisappearingMessagesQueue() return; } - int startedSecondsAgo = [NSDate new].timeIntervalSince1970 - expirationStartedAt / 1000.0; - DDLogDebug(@"%@ Starting expiration for message read %d seconds ago", self.logTag, startedSecondsAgo); + NSTimeInterval startedSecondsAgo = ([NSDate ows_millisecondTimeStamp] - expirationStartedAt) / 1000.0; + DDLogDebug(@"%@ Starting expiration for message read %f seconds ago", self.logTag, startedSecondsAgo); // Don't clobber if multiple actions simultaneously triggered expiration. if (message.expireStartedAt == 0 || message.expireStartedAt > expirationStartedAt) { @@ -211,27 +210,27 @@ void AssertIsOnDisappearingMessagesQueue() }]; } -- (void)setExpirationsForThread:(TSThread *)thread transaction:(YapDatabaseReadWriteTransaction *)transaction -{ - OWSBackgroundTask *_Nullable backgroundTask = [OWSBackgroundTask backgroundTaskWithLabelStr:__PRETTY_FUNCTION__]; - - uint64_t now = [NSDate ows_millisecondTimeStamp]; - [self.disappearingMessagesFinder - enumerateUnstartedExpiringMessagesInThread:thread - block:^(TSMessage *_Nonnull message) { - DDLogWarn(@"%@ Starting expiring message which should have already " - @"been started.", - self.logTag); - // specify "now" in case D.M. have since been disabled, but we have - // existing unstarted expiring messages that still need to expire. - [self setExpirationForMessage:message - expirationStartedAt:now - transaction:transaction]; - } - transaction:transaction]; - - backgroundTask = nil; -} +//- (void)setExpirationsForThread:(TSThread *)thread transaction:(YapDatabaseReadWriteTransaction *)transaction +//{ +// OWSBackgroundTask *_Nullable backgroundTask = [OWSBackgroundTask backgroundTaskWithLabelStr:__PRETTY_FUNCTION__]; +// +// uint64_t now = [NSDate ows_millisecondTimeStamp]; +// [self.disappearingMessagesFinder +// enumerateUnstartedExpiringMessagesInThread:thread +// block:^(TSMessage *_Nonnull message) { +// DDLogWarn(@"%@ Starting expiring message which should have already " +// @"been started.", +// self.logTag); +// // specify "now" in case D.M. have since been disabled, but we have +// // existing unstarted expiring messages that still need to expire. +// [self setExpirationForMessage:message +// expirationStartedAt:now +// transaction:transaction]; +// } +// transaction:transaction]; +// +// backgroundTask = nil; +//} - (void)becomeConsistentWithConfigurationForMessage:(TSMessage *)message contactsManager:(id)contactsManager diff --git a/SignalServiceKit/src/Messages/OWSMessageManager.m b/SignalServiceKit/src/Messages/OWSMessageManager.m index 1096823b6..901f929b5 100644 --- a/SignalServiceKit/src/Messages/OWSMessageManager.m +++ b/SignalServiceKit/src/Messages/OWSMessageManager.m @@ -708,8 +708,8 @@ NS_ASSUME_NONNULL_BEGIN }); } else if (syncMessage.read.count > 0) { DDLogInfo(@"%@ Received %ld read receipt(s)", self.logTag, (u_long)syncMessage.read.count); - [OWSReadReceiptManager.sharedManager processReadReceiptsFromLinkedDevice:syncMessage.read + readTimestamp:envelope.timestamp transaction:transaction]; } else if (syncMessage.hasVerified) { DDLogInfo(@"%@ Received verification state for %@", self.logTag, syncMessage.verified.destination); @@ -1074,7 +1074,6 @@ NS_ASSUME_NONNULL_BEGIN OWSAssert(transaction); OWSAssert([TSAccountManager isRegistered]); - NSString *localNumber = [TSAccountManager localNumber]; if (!thread) { OWSFail(@"%@ Can't finalize without thread", self.logTag); @@ -1087,12 +1086,10 @@ NS_ASSUME_NONNULL_BEGIN [incomingMessage saveWithTransaction:transaction]; - // Any messages sent from the current user - from this device or another - should be - // automatically marked as read. - BOOL shouldMarkMessageAsRead = [envelope.source isEqualToString:localNumber]; - if (shouldMarkMessageAsRead) { + // Any messages sent from the current user - from this device or another - should be automatically marked as read. + if ([envelope.source isEqualToString:TSAccountManager.localNumber]) { // Don't send a read receipt for messages sent by ourselves. - [incomingMessage markAsReadWithTransaction:transaction sendReadReceipt:NO]; + [incomingMessage markAsReadAtTimestamp:envelope.timestamp sendReadReceipt:NO transaction:transaction]; } TSQuotedMessage *_Nullable quotedMessage = incomingMessage.quotedMessage; diff --git a/SignalServiceKit/src/Messages/OWSMessageSender.m b/SignalServiceKit/src/Messages/OWSMessageSender.m index 3040f0e59..bc0501905 100644 --- a/SignalServiceKit/src/Messages/OWSMessageSender.m +++ b/SignalServiceKit/src/Messages/OWSMessageSender.m @@ -7,6 +7,7 @@ #import "ContactsUpdater.h" #import "NSData+keyVersionByte.h" #import "NSData+messagePadding.h" +#import "NSDate+OWS.h" #import "NSError+MessageSending.h" #import "OWSBackgroundTask.h" #import "OWSBlockingManager.h" @@ -1053,8 +1054,9 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException"; } [self.dbConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { - [[OWSDisappearingMessagesJob sharedJob] startExpirationForMessage:message - transaction:transaction]; + [[OWSDisappearingMessagesJob sharedJob] startAnyExpirationForMessage:message + expirationStartedAt:[NSDate ows_millisecondTimeStamp] + transaction:transaction]; }]; } diff --git a/SignalServiceKit/src/Messages/OWSReadReceiptManager.h b/SignalServiceKit/src/Messages/OWSReadReceiptManager.h index 0201cb0cb..36a2a14ad 100644 --- a/SignalServiceKit/src/Messages/OWSReadReceiptManager.h +++ b/SignalServiceKit/src/Messages/OWSReadReceiptManager.h @@ -1,5 +1,5 @@ // -// Copyright (c) 2017 Open Whisper Systems. All rights reserved. +// Copyright (c) 2018 Open Whisper Systems. All rights reserved. // NS_ASSUME_NONNULL_BEGIN @@ -8,8 +8,8 @@ NS_ASSUME_NONNULL_BEGIN @class TSIncomingMessage; @class TSOutgoingMessage; @class TSThread; -@class YapDatabaseReadWriteTransaction; @class YapDatabaseReadTransaction; +@class YapDatabaseReadWriteTransaction; extern NSString *const kIncomingMessageMarkedAsReadNotification; @@ -50,6 +50,7 @@ extern NSString *const kIncomingMessageMarkedAsReadNotification; #pragma mark - Linked Device Read Receipts - (void)processReadReceiptsFromLinkedDevice:(NSArray *)readReceiptProtos + readTimestamp:(uint64_t)readTimestamp transaction:(YapDatabaseReadWriteTransaction *)transaction; - (void)applyEarlyReadReceiptsForIncomingMessage:(TSIncomingMessage *)message diff --git a/SignalServiceKit/src/Messages/OWSReadReceiptManager.m b/SignalServiceKit/src/Messages/OWSReadReceiptManager.m index a7f7392a4..fda03a382 100644 --- a/SignalServiceKit/src/Messages/OWSReadReceiptManager.m +++ b/SignalServiceKit/src/Messages/OWSReadReceiptManager.m @@ -4,6 +4,7 @@ #import "OWSReadReceiptManager.h" #import "AppReadiness.h" +#import "NSDate+OWS.h" #import "NSNotificationCenter+OWS.h" #import "OWSLinkedDeviceReadReceipt.h" #import "OWSMessageSender.h" @@ -289,6 +290,7 @@ NSString *const OWSReadReceiptManagerAreReadReceiptsEnabled = @"areReadReceiptsE [self.dbConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { [self markAsReadBeforeTimestamp:timestamp thread:thread + readTimestamp:[NSDate ows_millisecondTimeStamp] wasLocal:YES transaction:transaction]; }]; @@ -307,10 +309,12 @@ NSString *const OWSReadReceiptManagerAreReadReceiptsEnabled = @"areReadReceiptsE OWSAssert(messageAuthorId.length > 0); OWSLinkedDeviceReadReceipt *newReadReceipt = - [[OWSLinkedDeviceReadReceipt alloc] initWithSenderId:messageAuthorId timestamp:message.timestamp]; + [[OWSLinkedDeviceReadReceipt alloc] initWithSenderId:messageAuthorId + messageIdTimestamp:message.timestamp + readTimestamp:[NSDate ows_millisecondTimeStamp]]; OWSLinkedDeviceReadReceipt *_Nullable oldReadReceipt = self.toLinkedDevicesReadReceiptMap[threadUniqueId]; - if (oldReadReceipt && oldReadReceipt.timestamp > newReadReceipt.timestamp) { + if (oldReadReceipt && oldReadReceipt.messageIdTimestamp > newReadReceipt.messageIdTimestamp) { // If there's an existing "linked device" read receipt for the same thread with // a newer timestamp, discard this "linked device" read receipt. DDLogVerbose(@"%@ Ignoring redundant read receipt for linked devices.", self.logTag); @@ -426,12 +430,13 @@ NSString *const OWSReadReceiptManagerAreReadReceiptsEnabled = @"areReadReceiptsE OWSLinkedDeviceReadReceipt *_Nullable readReceipt = [OWSLinkedDeviceReadReceipt findLinkedDeviceReadReceiptWithSenderId:senderId - timestamp:timestamp + messageIdTimestamp:timestamp transaction:transaction]; if (!readReceipt) { return; } - [message markAsReadWithTransaction:transaction sendReadReceipt:NO]; + + [message markAsReadAtTimestamp:readReceipt.readTimestamp sendReadReceipt:NO transaction:transaction]; [readReceipt removeWithTransaction:transaction]; [transaction addCompletionQueue:nil @@ -443,6 +448,7 @@ NSString *const OWSReadReceiptManagerAreReadReceiptsEnabled = @"areReadReceiptsE } - (void)processReadReceiptsFromLinkedDevice:(NSArray *)readReceiptProtos + readTimestamp:(uint64_t)readTimestamp transaction:(YapDatabaseReadWriteTransaction *)transaction { OWSAssert(readReceiptProtos); @@ -450,41 +456,53 @@ NSString *const OWSReadReceiptManagerAreReadReceiptsEnabled = @"areReadReceiptsE for (OWSSignalServiceProtosSyncMessageRead *readReceiptProto in readReceiptProtos) { NSString *_Nullable senderId = readReceiptProto.sender; - uint64_t timestamp = readReceiptProto.timestamp; - BOOL isValid = senderId.length > 0 && timestamp > 0; - if (!isValid) { + uint64_t messageIdTimestamp = readReceiptProto.timestamp; + + if (senderId.length == 0) { + OWSProdLogAndFail(@"%@ in %s senderId was unexpectedly nil", self.logTag, __PRETTY_FUNCTION__); + continue; + } + + if (messageIdTimestamp == 0) { + OWSProdLogAndFail(@"%@ in %s messageIdTimstamp was unexpectedly 0", self.logTag, __PRETTY_FUNCTION__); continue; } - - NSArray *messages = (NSArray *) [TSInteraction interactionsWithTimestamp:timestamp - ofClass:[TSIncomingMessage class] - withTransaction:transaction]; + + NSArray *messages + = (NSArray *)[TSInteraction interactionsWithTimestamp:messageIdTimestamp + ofClass:[TSIncomingMessage class] + withTransaction:transaction]; if (messages.count > 0) { for (TSIncomingMessage *message in messages) { + NSTimeInterval secondsSinceRead = [NSDate new].timeIntervalSince1970 - readTimestamp / 1000; OWSAssert([message isKindOfClass:[TSIncomingMessage class]]); - - [self markAsReadOnLinkedDevice:message - transaction:transaction]; + DDLogDebug(@"%@ read on linked device %f seconds ago", self.logTag, secondsSinceRead); + [self markAsReadOnLinkedDevice:message readTimestamp:readTimestamp transaction:transaction]; } } else { // Received read receipt for unknown incoming message. // Persist in case we receive the incoming message later. - OWSLinkedDeviceReadReceipt *readReceipt = [[OWSLinkedDeviceReadReceipt alloc] initWithSenderId:senderId timestamp:timestamp]; + OWSLinkedDeviceReadReceipt *readReceipt = + [[OWSLinkedDeviceReadReceipt alloc] initWithSenderId:senderId + messageIdTimestamp:messageIdTimestamp + readTimestamp:readTimestamp]; [readReceipt saveWithTransaction:transaction]; } } } - (void)markAsReadOnLinkedDevice:(TSIncomingMessage *)message + readTimestamp:(uint64_t)readTimestamp transaction:(YapDatabaseReadWriteTransaction *)transaction { OWSAssert(message); OWSAssert(transaction); - // Use timestampForSorting which reflects local sort order, rather than timestamp + // Use `timestampForSorting` which reflects local received order, rather than `timestamp` // which reflect sender time. [self markAsReadBeforeTimestamp:message.timestampForSorting thread:[message threadWithTransaction:transaction] + readTimestamp:readTimestamp wasLocal:NO transaction:transaction]; } @@ -493,15 +511,16 @@ NSString *const OWSReadReceiptManagerAreReadReceiptsEnabled = @"areReadReceiptsE - (void)markAsReadBeforeTimestamp:(uint64_t)timestamp thread:(TSThread *)thread + readTimestamp:(uint64_t)readTimestamp wasLocal:(BOOL)wasLocal transaction:(YapDatabaseReadWriteTransaction *)transaction { OWSAssert(timestamp > 0); OWSAssert(thread); OWSAssert(transaction); - - NSMutableArray> *interactions = [NSMutableArray new]; - + + NSMutableArray> *newlyReadList = [NSMutableArray new]; + [[TSDatabaseView unseenDatabaseViewExtension:transaction] enumerateRowsInGroup:thread.uniqueId usingBlock:^(NSString *collection, @@ -529,25 +548,26 @@ NSString *const OWSReadReceiptManagerAreReadReceiptsEnabled = @"areReadReceiptsE OWSAssert(!possiblyRead.read); if (!possiblyRead.read) { - [interactions addObject:possiblyRead]; + [newlyReadList addObject:possiblyRead]; + } else if (readTimestamp < possiblyRead.expireStartedAt) { + [newlyReadList addObject:possiblyRead]; } }]; - - if (interactions.count < 1) { + + if (newlyReadList.count < 1) { return; } if (wasLocal) { - DDLogError(@"Marking %zd messages as read locally.", interactions.count); + DDLogError(@"Marking %zu messages as read locally.", newlyReadList.count); } else { - DDLogError(@"Marking %zd messages as read by linked device.", interactions.count); + DDLogError(@"Marking %zu messages as read by linked device.", newlyReadList.count); } + for (id readItem in newlyReadList) { + [readItem markAsReadAtTimestamp:readTimestamp sendReadReceipt:wasLocal transaction:transaction]; - for (id possiblyRead in interactions) { - [possiblyRead markAsReadWithTransaction:transaction sendReadReceipt:wasLocal]; - - if ([possiblyRead isKindOfClass:[TSIncomingMessage class]]) { - TSIncomingMessage *incomingMessage = (TSIncomingMessage *)possiblyRead; + if ([readItem isKindOfClass:[TSIncomingMessage class]]) { + TSIncomingMessage *incomingMessage = (TSIncomingMessage *)readItem; [transaction addCompletionQueue:nil completionBlock:^{ [[NSNotificationCenter defaultCenter] diff --git a/SignalServiceKit/src/Messages/OWSReadTracking.h b/SignalServiceKit/src/Messages/OWSReadTracking.h index 89122e8c5..5686e744f 100644 --- a/SignalServiceKit/src/Messages/OWSReadTracking.h +++ b/SignalServiceKit/src/Messages/OWSReadTracking.h @@ -15,15 +15,18 @@ */ @property (nonatomic, readonly, getter=wasRead) BOOL read; +@property (nonatomic, readonly) uint64_t expireStartedAt; @property (nonatomic, readonly) uint64_t timestampForSorting; @property (nonatomic, readonly) NSString *uniqueThreadId; + - (BOOL)shouldAffectUnreadCounts; /** * Used both for *responding* to a remote read receipt and in response to the local user's activity. */ -- (void)markAsReadWithTransaction:(YapDatabaseReadWriteTransaction *)transaction - sendReadReceipt:(BOOL)sendReadReceipt; +- (void)markAsReadAtTimestamp:(uint64_t)readTimestamp + sendReadReceipt:(BOOL)sendReadReceipt + transaction:(YapDatabaseReadWriteTransaction *)transaction; @end diff --git a/SignalServiceKit/src/Messages/TSCall.m b/SignalServiceKit/src/Messages/TSCall.m index 28e7e38d0..7b37efb3a 100644 --- a/SignalServiceKit/src/Messages/TSCall.m +++ b/SignalServiceKit/src/Messages/TSCall.m @@ -89,14 +89,21 @@ NSUInteger TSCallCurrentSchemaVersion = 1; #pragma mark - OWSReadTracking +- (uint64_t)expireStartedAt +{ + return 0; +} + - (BOOL)shouldAffectUnreadCounts { return YES; } -- (void)markAsReadWithTransaction:(YapDatabaseReadWriteTransaction *)transaction - sendReadReceipt:(BOOL)sendReadReceipt +- (void)markAsReadAtTimestamp:(uint64_t)readTimestamp + sendReadReceipt:(BOOL)sendReadReceipt + transaction:(YapDatabaseReadWriteTransaction *)transaction { + OWSAssert(transaction); if (_read) { @@ -126,7 +133,7 @@ NSUInteger TSCallCurrentSchemaVersion = 1; [self.dbReadWriteConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *_Nonnull transaction) { [self saveWithTransaction:transaction]; - + // redraw any thread-related unread count UI. [self touchThreadWithTransaction:transaction]; }]; From b039fdd276ba43076d87d4df832cb9a41e8e26d8 Mon Sep 17 00:00:00 2001 From: Michael Kirk Date: Wed, 18 Apr 2018 10:01:44 -0400 Subject: [PATCH 5/8] UI Fix: start with full hourglass on short timer durations We were positioning relative to "blink" time (2s), rather than delete time, which means that for 10s timers we were starting as though only 8s remained. // FREEBIE --- .../ConversationView/Cells/OWSExpirationTimerView.m | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Signal/src/ViewControllers/ConversationView/Cells/OWSExpirationTimerView.m b/Signal/src/ViewControllers/ConversationView/Cells/OWSExpirationTimerView.m index 15532f8da..4cd15ca41 100644 --- a/Signal/src/ViewControllers/ConversationView/Cells/OWSExpirationTimerView.m +++ b/Signal/src/ViewControllers/ConversationView/Cells/OWSExpirationTimerView.m @@ -159,8 +159,8 @@ const CGFloat kExpirationTimerViewSize = 16.f; OWSFail(@"initialDurationSeconds was unexpectedly 0"); return; } - - CGFloat ratioRemaining = (CGFloat)timeUntilFlashing / (CGFloat)self.initialDurationSeconds; + + CGFloat ratioRemaining = (CGFloat)secondsLeft / (CGFloat)self.initialDurationSeconds; CGFloat ratioComplete = CGFloatClamp((CGFloat)1.0 - ratioRemaining, 0, 1.0); CGPoint startPosition = CGPointMake(0, self.fullHourglassImageView.height * ratioComplete); From a9e7c5e8798a9cdb6a653934ef5f74668a03880b Mon Sep 17 00:00:00 2001 From: Michael Kirk Date: Wed, 18 Apr 2018 11:21:16 -0400 Subject: [PATCH 6/8] Cleanup: simplify migration, remove unused code // FREEBIE --- .../src/Devices/OWSLinkedDeviceReadReceipt.m | 25 ++++++++----------- .../src/Messages/OWSDisappearingMessagesJob.h | 1 - .../src/Messages/OWSDisappearingMessagesJob.m | 22 ---------------- 3 files changed, 10 insertions(+), 38 deletions(-) diff --git a/SignalServiceKit/src/Devices/OWSLinkedDeviceReadReceipt.m b/SignalServiceKit/src/Devices/OWSLinkedDeviceReadReceipt.m index c7dacaa9b..7178a38d4 100644 --- a/SignalServiceKit/src/Devices/OWSLinkedDeviceReadReceipt.m +++ b/SignalServiceKit/src/Devices/OWSLinkedDeviceReadReceipt.m @@ -6,13 +6,6 @@ NS_ASSUME_NONNULL_BEGIN -@interface OWSLinkedDeviceReadReceipt () - -// FIXME remove this `timestamp` property and migrate in initWithCoder. -@property (nonatomic, readonly) uint64_t timestamp; - -@end - @implementation OWSLinkedDeviceReadReceipt - (instancetype)initWithSenderId:(NSString *)senderId @@ -42,18 +35,20 @@ NS_ASSUME_NONNULL_BEGIN return self; } + // renamed timestamp -> messageIdTimestamp if (!_messageIdTimestamp) { - // FIXME to remove this legacy `timestamp` property, we need to figure out exactly how MTL encodes uint64_t. - // e.g. can we just do something like: `((NSNumber *)[coder decodeObjectForKey:@"timestamp"]).unsignedLongLong` - _messageIdTimestamp = _timestamp; + NSNumber *_Nullable legacyTimestamp = (NSNumber *)[coder decodeObjectForKey:@"timestamp"]; + OWSAssert(legacyTimestamp.unsignedLongLongValue > 0); + _messageIdTimestamp = legacyTimestamp.unsignedLongLongValue; } - // For legacy early LinkedDeviceReadReceipts, before we were tracking read time, we assume the message was read as - // soon as it was sent. This is always going to be at least a little earlier than it was actually read, but we have - // nothing better to choose, and by the very fact that we're receiving a read receipt, we have good reason to - // believe they read the message on the other device. + // For legacy objects, before we were tracking read time, use the original messages "sent" timestamp + // as the local read time. This will always be at least a little bit earlier than the message was + // actually read, but it's the safer assumption. At worst we'll delete the message from this device + // earlier than the user expects, but this shouldn't be terrible because we know they've read the + // message on the other device. Keep in mind this *only* affects "early" read receipts. if (_readTimestamp == 0) { - _readTimestamp = _timestamp; + _readTimestamp = _messageIdTimestamp; } return self; diff --git a/SignalServiceKit/src/Messages/OWSDisappearingMessagesJob.h b/SignalServiceKit/src/Messages/OWSDisappearingMessagesJob.h index dd7ef9ec4..032c61add 100644 --- a/SignalServiceKit/src/Messages/OWSDisappearingMessagesJob.h +++ b/SignalServiceKit/src/Messages/OWSDisappearingMessagesJob.h @@ -17,7 +17,6 @@ NS_ASSUME_NONNULL_BEGIN - (instancetype)init NS_UNAVAILABLE; -//+ (void)setExpirationsForThread:(TSThread *)thread; - (void)startAnyExpirationForMessage:(TSMessage *)message expirationStartedAt:(uint64_t)expirationStartedAt transaction:(YapDatabaseReadWriteTransaction *_Nonnull)transaction; diff --git a/SignalServiceKit/src/Messages/OWSDisappearingMessagesJob.m b/SignalServiceKit/src/Messages/OWSDisappearingMessagesJob.m index 4c92811b3..e015900cb 100644 --- a/SignalServiceKit/src/Messages/OWSDisappearingMessagesJob.m +++ b/SignalServiceKit/src/Messages/OWSDisappearingMessagesJob.m @@ -210,28 +210,6 @@ void AssertIsOnDisappearingMessagesQueue() }]; } -//- (void)setExpirationsForThread:(TSThread *)thread transaction:(YapDatabaseReadWriteTransaction *)transaction -//{ -// OWSBackgroundTask *_Nullable backgroundTask = [OWSBackgroundTask backgroundTaskWithLabelStr:__PRETTY_FUNCTION__]; -// -// uint64_t now = [NSDate ows_millisecondTimeStamp]; -// [self.disappearingMessagesFinder -// enumerateUnstartedExpiringMessagesInThread:thread -// block:^(TSMessage *_Nonnull message) { -// DDLogWarn(@"%@ Starting expiring message which should have already " -// @"been started.", -// self.logTag); -// // specify "now" in case D.M. have since been disabled, but we have -// // existing unstarted expiring messages that still need to expire. -// [self setExpirationForMessage:message -// expirationStartedAt:now -// transaction:transaction]; -// } -// transaction:transaction]; -// -// backgroundTask = nil; -//} - - (void)becomeConsistentWithConfigurationForMessage:(TSMessage *)message contactsManager:(id)contactsManager transaction:(YapDatabaseReadWriteTransaction *)transaction From eb140a6839800a1a84bc050d6683095cfb88a3e2 Mon Sep 17 00:00:00 2001 From: Michael Kirk Date: Wed, 18 Apr 2018 11:45:55 -0400 Subject: [PATCH 7/8] Timer info messages *before* the message which changed the timer // FREEBIE --- .../OWSDisappearingConfigurationUpdateInfoMessage.m | 7 +++++++ .../src/Messages/OWSDisappearingMessagesJob.m | 9 +++++---- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/SignalServiceKit/src/Messages/Interactions/OWSDisappearingConfigurationUpdateInfoMessage.m b/SignalServiceKit/src/Messages/Interactions/OWSDisappearingConfigurationUpdateInfoMessage.m index 12b7006d2..1391aa271 100644 --- a/SignalServiceKit/src/Messages/Interactions/OWSDisappearingConfigurationUpdateInfoMessage.m +++ b/SignalServiceKit/src/Messages/Interactions/OWSDisappearingConfigurationUpdateInfoMessage.m @@ -49,6 +49,13 @@ NS_ASSUME_NONNULL_BEGIN return self; } +- (BOOL)shouldUseReceiptDateForSorting +{ + // Use the timestamp, not the "received at" timestamp to sort, + // since we're creating these interactions after the fact and back-dating them. + return NO; +} + - (NSString *)description { if (self.createdByRemoteName) { diff --git a/SignalServiceKit/src/Messages/OWSDisappearingMessagesJob.m b/SignalServiceKit/src/Messages/OWSDisappearingMessagesJob.m index e015900cb..a6034a058 100644 --- a/SignalServiceKit/src/Messages/OWSDisappearingMessagesJob.m +++ b/SignalServiceKit/src/Messages/OWSDisappearingMessagesJob.m @@ -254,16 +254,17 @@ void AssertIsOnDisappearingMessagesQueue() NSString *contactName = [contactsManager displayNameForPhoneIdentifier:incomingMessage.messageAuthorId]; // We want the info message to appear _before_ the message. - [[[OWSDisappearingConfigurationUpdateInfoMessage alloc] initWithTimestamp:message.timestamp - 1 + [[[OWSDisappearingConfigurationUpdateInfoMessage alloc] initWithTimestamp:message.timestampForSorting - 1 thread:message.thread configuration:disappearingMessagesConfiguration - createdByRemoteName:contactName] saveWithTransaction:transaction]; + createdByRemoteName:contactName] + saveWithTransaction:transaction]; } else { // We want the info message to appear _before_ the message. - [[[OWSDisappearingConfigurationUpdateInfoMessage alloc] initWithTimestamp:message.timestamp - 1 + [[[OWSDisappearingConfigurationUpdateInfoMessage alloc] initWithTimestamp:message.timestampForSorting - 1 thread:message.thread configuration:disappearingMessagesConfiguration] - saveWithTransaction:transaction]; + saveWithTransaction:transaction]; } backgroundTask = nil; From c88ce07f660308bdda286b035bd373f1e109feda Mon Sep 17 00:00:00 2001 From: Michael Kirk Date: Wed, 18 Apr 2018 15:27:36 -0400 Subject: [PATCH 8/8] CR: Clean up comments, use property setter instead of ivar // FREEBIE --- .../src/Devices/OWSLinkedDeviceReadReceipt.m | 5 ++--- .../src/Messages/OWSDisappearingMessagesJob.m | 10 +++++----- SignalServiceKit/src/Messages/OWSReadReceiptManager.m | 2 +- 3 files changed, 8 insertions(+), 9 deletions(-) diff --git a/SignalServiceKit/src/Devices/OWSLinkedDeviceReadReceipt.m b/SignalServiceKit/src/Devices/OWSLinkedDeviceReadReceipt.m index 7178a38d4..7dfa06420 100644 --- a/SignalServiceKit/src/Devices/OWSLinkedDeviceReadReceipt.m +++ b/SignalServiceKit/src/Devices/OWSLinkedDeviceReadReceipt.m @@ -44,9 +44,8 @@ NS_ASSUME_NONNULL_BEGIN // For legacy objects, before we were tracking read time, use the original messages "sent" timestamp // as the local read time. This will always be at least a little bit earlier than the message was - // actually read, but it's the safer assumption. At worst we'll delete the message from this device - // earlier than the user expects, but this shouldn't be terrible because we know they've read the - // message on the other device. Keep in mind this *only* affects "early" read receipts. + // actually read, which isn't ideal, but safer than persisting a disappearing message too long, especially + // since we know they read it on their linked desktop. if (_readTimestamp == 0) { _readTimestamp = _messageIdTimestamp; } diff --git a/SignalServiceKit/src/Messages/OWSDisappearingMessagesJob.m b/SignalServiceKit/src/Messages/OWSDisappearingMessagesJob.m index a6034a058..a68437b23 100644 --- a/SignalServiceKit/src/Messages/OWSDisappearingMessagesJob.m +++ b/SignalServiceKit/src/Messages/OWSDisappearingMessagesJob.m @@ -72,11 +72,11 @@ void AssertIsOnDisappearingMessagesQueue() NSTimeInterval kFallBackTimerInterval = 5 * kMinuteInterval; [AppReadiness runNowOrWhenAppIsReady:^{ if (CurrentAppContext().isMainApp) { - self->_fallbackTimer = [NSTimer weakScheduledTimerWithTimeInterval:kFallBackTimerInterval - target:self - selector:@selector(fallbackTimerDidFire) - userInfo:nil - repeats:YES]; + self.fallbackTimer = [NSTimer weakScheduledTimerWithTimeInterval:kFallBackTimerInterval + target:self + selector:@selector(fallbackTimerDidFire) + userInfo:nil + repeats:YES]; } }]; diff --git a/SignalServiceKit/src/Messages/OWSReadReceiptManager.m b/SignalServiceKit/src/Messages/OWSReadReceiptManager.m index fda03a382..ca54eae1d 100644 --- a/SignalServiceKit/src/Messages/OWSReadReceiptManager.m +++ b/SignalServiceKit/src/Messages/OWSReadReceiptManager.m @@ -464,7 +464,7 @@ NSString *const OWSReadReceiptManagerAreReadReceiptsEnabled = @"areReadReceiptsE } if (messageIdTimestamp == 0) { - OWSProdLogAndFail(@"%@ in %s messageIdTimstamp was unexpectedly 0", self.logTag, __PRETTY_FUNCTION__); + OWSProdLogAndFail(@"%@ in %s messageIdTimestamp was unexpectedly 0", self.logTag, __PRETTY_FUNCTION__); continue; }