From a60d8eb161e7d7c76de34da06249e8d4b80d98e5 Mon Sep 17 00:00:00 2001 From: Michael Kirk Date: Fri, 24 Aug 2018 16:55:12 -0600 Subject: [PATCH] WIP: migration / autoincrement logic TODO: -[ ] contact offer -[ ] verify all paths that utilized timestampForSorting, e.g. make sure SN appear before the message they affect, etc. -[x] Monotonic ID -[x] New extension which sorts by id -[x] Migration -[ ] batch migration? --- Signal.xcodeproj/project.pbxproj | 4 + .../migrations/OWS110SortIdMigration.swift | 40 ++++++++++ .../migrations/OWSDatabaseMigrationRunner.m | 3 +- SignalMessaging/utils/ThreadUtil.m | 1 + .../src/Messages/Interactions/TSInteraction.h | 4 + .../src/Messages/Interactions/TSInteraction.m | 37 +++++++-- .../src/Storage/IncrementingIdFinder.swift | 21 +++++ .../src/Storage/OWSPrimaryStorage.m | 1 + SignalServiceKit/src/Storage/TSDatabaseView.h | 8 ++ SignalServiceKit/src/Storage/TSDatabaseView.m | 79 ++++++++++++++++++- 10 files changed, 191 insertions(+), 7 deletions(-) create mode 100644 SignalMessaging/environment/migrations/OWS110SortIdMigration.swift create mode 100644 SignalServiceKit/src/Storage/IncrementingIdFinder.swift diff --git a/Signal.xcodeproj/project.pbxproj b/Signal.xcodeproj/project.pbxproj index 5e8c55c58..d5a6e4503 100644 --- a/Signal.xcodeproj/project.pbxproj +++ b/Signal.xcodeproj/project.pbxproj @@ -445,6 +445,7 @@ 4C6F527C20FFE8400097DEEE /* SignalUBSan.supp in Resources */ = {isa = PBXBuildFile; fileRef = 4C6F527B20FFE8400097DEEE /* SignalUBSan.supp */; }; 4C858A52212DC5E1001B45D3 /* UIImage+OWS.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4C858A51212DC5E1001B45D3 /* UIImage+OWS.swift */; }; 4C948FF72146EB4800349F0D /* BlockListCache.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4C948FF62146EB4800349F0D /* BlockListCache.swift */; }; + 4C858A562130CBEC001B45D3 /* OWS110SortIdMigration.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4C858A552130CBEC001B45D3 /* OWS110SortIdMigration.swift */; }; 4CA5F793211E1F06008C2708 /* Toast.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4CA5F792211E1F06008C2708 /* Toast.swift */; }; 4CB5F26720F6E1E2004D1B42 /* MenuActionsViewController.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4CFF4C0920F55BBA005DA313 /* MenuActionsViewController.swift */; }; 4CB5F26920F7D060004D1B42 /* MessageActions.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4CB5F26820F7D060004D1B42 /* MessageActions.swift */; }; @@ -1136,6 +1137,7 @@ 4C6F527B20FFE8400097DEEE /* SignalUBSan.supp */ = {isa = PBXFileReference; lastKnownFileType = text; path = SignalUBSan.supp; sourceTree = ""; }; 4C858A51212DC5E1001B45D3 /* UIImage+OWS.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "UIImage+OWS.swift"; sourceTree = ""; }; 4C948FF62146EB4800349F0D /* BlockListCache.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BlockListCache.swift; sourceTree = ""; }; + 4C858A552130CBEC001B45D3 /* OWS110SortIdMigration.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OWS110SortIdMigration.swift; sourceTree = ""; }; 4CA5F792211E1F06008C2708 /* Toast.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Toast.swift; sourceTree = ""; }; 4CB5F26820F7D060004D1B42 /* MessageActions.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MessageActions.swift; sourceTree = ""; }; 4CC0B59B20EC5F2E00CF6EE0 /* ConversationConfigurationSyncOperation.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ConversationConfigurationSyncOperation.swift; sourceTree = ""; }; @@ -1663,6 +1665,7 @@ 346129E41FD5C0C600532771 /* OWSDatabaseMigrationRunner.m */, 34ABB2C32090C59700C727A6 /* OWSResaveCollectionDBMigration.h */, 34ABB2C22090C59600C727A6 /* OWSResaveCollectionDBMigration.m */, + 4C858A552130CBEC001B45D3 /* OWS110SortIdMigration.swift */, ); path = migrations; sourceTree = ""; @@ -3202,6 +3205,7 @@ 34AC09E6211B39B100997B47 /* SelectRecipientViewController.m in Sources */, 4C858A52212DC5E1001B45D3 /* UIImage+OWS.swift in Sources */, 34480B671FD0AA9400BC14EF /* UIFont+OWS.m in Sources */, + 4C858A562130CBEC001B45D3 /* OWS110SortIdMigration.swift in Sources */, 346129E61FD5C0C600532771 /* OWSDatabaseMigrationRunner.m in Sources */, 34AC0A11211B39EA00997B47 /* OWSLayerView.swift in Sources */, 34AC0A1B211B39EA00997B47 /* GradientView.swift in Sources */, diff --git a/SignalMessaging/environment/migrations/OWS110SortIdMigration.swift b/SignalMessaging/environment/migrations/OWS110SortIdMigration.swift new file mode 100644 index 000000000..e469d1bfd --- /dev/null +++ b/SignalMessaging/environment/migrations/OWS110SortIdMigration.swift @@ -0,0 +1,40 @@ +// +// Copyright (c) 2018 Open Whisper Systems. All rights reserved. +// + +import Foundation + +class OWS110SortIdMigration: OWSDatabaseMigration { + // increment a similar constant for each migration. + @objc + class func migrationId() -> String { + return "110" + } + + override public func runUp(completion: @escaping OWSDatabaseMigrationCompletion) { + Logger.debug("") + + // TODO batch this? + self.dbReadWriteConnection().readWrite { transaction in + guard let legacySorting: YapDatabaseAutoViewTransaction = transaction.extension(TSMessageDatabaseViewExtensionName_Legacy) as? YapDatabaseAutoViewTransaction else { + owsFailDebug("legacySorting was unexpectedly nil") + return + } + + legacySorting.enumerateGroups { group, _ in + legacySorting.enumerateKeysAndObjects(inGroup: group) { (_, _, object, _, _) in + guard let interaction = object as? TSInteraction else { + owsFailDebug("unexpected object: \(type(of: object))") + return + } + + interaction.saveNextSortId(transaction: transaction) + Logger.debug("thread: \(interaction.uniqueThreadId), timestampForSorting:\(interaction.timestampForSorting()), sortId: \(interaction.sortId)") + } + } + } + + completion() + } + +} diff --git a/SignalMessaging/environment/migrations/OWSDatabaseMigrationRunner.m b/SignalMessaging/environment/migrations/OWSDatabaseMigrationRunner.m index f150e17b9..00eb09346 100644 --- a/SignalMessaging/environment/migrations/OWSDatabaseMigrationRunner.m +++ b/SignalMessaging/environment/migrations/OWSDatabaseMigrationRunner.m @@ -44,7 +44,8 @@ NS_ASSUME_NONNULL_BEGIN [[OWS106EnsureProfileComplete alloc] initWithPrimaryStorage:primaryStorage], [[OWS107LegacySounds alloc] initWithPrimaryStorage:primaryStorage], [[OWS108CallLoggingPreference alloc] initWithPrimaryStorage:primaryStorage], - [[OWS109OutgoingMessageState alloc] initWithPrimaryStorage:primaryStorage] + [[OWS109OutgoingMessageState alloc] initWithPrimaryStorage:primaryStorage], + [[OWS110SortIdMigration alloc] initWithPrimaryStorage:primaryStorage] ]; } diff --git a/SignalMessaging/utils/ThreadUtil.m b/SignalMessaging/utils/ThreadUtil.m index b5198df14..1d0d7cf70 100644 --- a/SignalMessaging/utils/ThreadUtil.m +++ b/SignalMessaging/utils/ThreadUtil.m @@ -255,6 +255,7 @@ NS_ASSUME_NONNULL_BEGIN #pragma mark - Dynamic Interactions +// MJK TODO - dynamic interactions + (ThreadDynamicInteractions *)ensureDynamicInteractionsForThread:(TSThread *)thread contactsManager:(OWSContactsManager *)contactsManager blockingManager:(OWSBlockingManager *)blockingManager diff --git a/SignalServiceKit/src/Messages/Interactions/TSInteraction.h b/SignalServiceKit/src/Messages/Interactions/TSInteraction.h index a5b29b147..929dea0a6 100644 --- a/SignalServiceKit/src/Messages/Interactions/TSInteraction.h +++ b/SignalServiceKit/src/Messages/Interactions/TSInteraction.h @@ -33,6 +33,7 @@ NSString *NSStringFromOWSInteractionType(OWSInteractionType value); @property (nonatomic, readonly) NSString *uniqueThreadId; @property (nonatomic, readonly) TSThread *thread; @property (nonatomic, readonly) uint64_t timestamp; +@property (nonatomic, readonly) uint64_t sortId; - (OWSInteractionType)interactionType; @@ -66,6 +67,9 @@ NSString *NSStringFromOWSInteractionType(OWSInteractionType value); // unseen message indicators, etc. - (BOOL)isDynamicInteraction; +- (void)saveNextSortIdWithTransaction:(YapDatabaseReadWriteTransaction *)transaction + NS_SWIFT_NAME(saveNextSortId(transaction:)); + @end NS_ASSUME_NONNULL_END diff --git a/SignalServiceKit/src/Messages/Interactions/TSInteraction.m b/SignalServiceKit/src/Messages/Interactions/TSInteraction.m index d76cfe9e5..a6d46f260 100644 --- a/SignalServiceKit/src/Messages/Interactions/TSInteraction.m +++ b/SignalServiceKit/src/Messages/Interactions/TSInteraction.m @@ -7,6 +7,7 @@ #import "OWSPrimaryStorage+messageIDs.h" #import "TSDatabaseSecondaryIndexes.h" #import "TSThread.h" +#import NS_ASSUME_NONNULL_BEGIN @@ -30,6 +31,12 @@ NSString *NSStringFromOWSInteractionType(OWSInteractionType value) } } +@interface TSInteraction () + +@property (nonatomic) uint64_t sortId; + +@end + @implementation TSInteraction + (NSArray *)interactionsWithTimestamp:(uint64_t)timestamp @@ -124,12 +131,12 @@ NSString *NSStringFromOWSInteractionType(OWSInteractionType value) { OWSAssertDebug(other); - uint64_t timestamp1 = self.timestampForSorting; - uint64_t timestamp2 = other.timestampForSorting; + uint64_t sortId1 = self.sortId; + uint64_t sortId2 = self.sortId; - if (timestamp1 > timestamp2) { + if (sortId1 > sortId2) { return NSOrderedDescending; - } else if (timestamp1 < timestamp2) { + } else if (sortId1 < sortId2) { return NSOrderedAscending; } else { return NSOrderedSame; @@ -151,10 +158,16 @@ NSString *NSStringFromOWSInteractionType(OWSInteractionType value) (unsigned long)self.timestamp]; } -- (void)saveWithTransaction:(YapDatabaseReadWriteTransaction *)transaction { +- (void)saveWithTransaction:(YapDatabaseReadWriteTransaction *)transaction +{ + // MJK can we remove this? We can't trust the legacy order of this id field. Any reason not to use UUID like for + // other objects? if (!self.uniqueId) { self.uniqueId = [OWSPrimaryStorage getAndIncrementMessageIdWithTransaction:transaction]; } + if (self.sortId == 0) { + self.sortId = [SSKIncrementingIdFinder nextIdWithKey:[TSInteraction collection] transaction:transaction]; + } [super saveWithTransaction:transaction]; @@ -175,6 +188,20 @@ NSString *NSStringFromOWSInteractionType(OWSInteractionType value) return NO; } +#pragma mark - sorting migration + +- (void)saveNextSortIdWithTransaction:(YapDatabaseReadWriteTransaction *)transaction +{ + if (self.sortId != 0) { + // This could happen if something else in our startup process saved the interaction + // e.g. another migration ran. + // During the migration, since we're enumerating the interactions in the proper order, + // we want to ignore any previously assigned sortId + self.sortId = 0; + } + [self saveWithTransaction:transaction]; +} + @end NS_ASSUME_NONNULL_END diff --git a/SignalServiceKit/src/Storage/IncrementingIdFinder.swift b/SignalServiceKit/src/Storage/IncrementingIdFinder.swift new file mode 100644 index 000000000..a3936e428 --- /dev/null +++ b/SignalServiceKit/src/Storage/IncrementingIdFinder.swift @@ -0,0 +1,21 @@ +// +// Copyright (c) 2018 Open Whisper Systems. All rights reserved. +// + +import Foundation + +@objc +public class SSKIncrementingIdFinder: NSObject { + + private static let collectionName = "IncrementingIdCollection" + + @objc + public class func nextId(key: String, transaction: YapDatabaseReadWriteTransaction) -> UInt64 { + let previousId: UInt64 = transaction.object(forKey: key, inCollection: collectionName) as? UInt64 ?? 0 + let nextId: UInt64 = previousId + 1 + + transaction.setObject(nextId, forKey: key, inCollection: collectionName) + Logger.debug("key: \(key) nextId: \(nextId)") + return nextId + } +} diff --git a/SignalServiceKit/src/Storage/OWSPrimaryStorage.m b/SignalServiceKit/src/Storage/OWSPrimaryStorage.m index b7000f0f7..5723cf037 100644 --- a/SignalServiceKit/src/Storage/OWSPrimaryStorage.m +++ b/SignalServiceKit/src/Storage/OWSPrimaryStorage.m @@ -190,6 +190,7 @@ void VerifyRegistrationsForPrimaryStorage(OWSStorage *storage) // // All sync registrations must be done before all async registrations, // or the sync registrations will block on the async registrations. + [TSDatabaseView asyncRegisterLegacyThreadInteractionsDatabaseView:self]; [TSDatabaseView asyncRegisterThreadInteractionsDatabaseView:self]; [TSDatabaseView asyncRegisterThreadDatabaseView:self]; [TSDatabaseView asyncRegisterUnreadDatabaseView:self]; diff --git a/SignalServiceKit/src/Storage/TSDatabaseView.h b/SignalServiceKit/src/Storage/TSDatabaseView.h index 3e83d174b..34af97813 100644 --- a/SignalServiceKit/src/Storage/TSDatabaseView.h +++ b/SignalServiceKit/src/Storage/TSDatabaseView.h @@ -13,6 +13,8 @@ extern NSString *const TSSecondaryDevicesGroup; extern NSString *const TSThreadDatabaseViewExtensionName; extern NSString *const TSMessageDatabaseViewExtensionName; +extern NSString *const TSMessageDatabaseViewExtensionName_Legacy; + extern NSString *const TSUnreadDatabaseViewExtensionName; extern NSString *const TSSecondaryDevicesDatabaseViewExtensionName; @@ -30,8 +32,10 @@ extern NSString *const TSLazyRestoreAttachmentsDatabaseViewExtensionName; // otherwise it returns the "unread" database view. + (id)unseenDatabaseViewExtension:(YapDatabaseReadTransaction *)transaction; +// MJK TODO - dynamic interactions + (id)threadOutgoingMessageDatabaseView:(YapDatabaseReadTransaction *)transaction; +// MJK reconsider this? It used to be for SN changes and dynamic interactions. Now maybe only SN changes? + (id)threadSpecialMessagesDatabaseView:(YapDatabaseReadTransaction *)transaction; #pragma mark - Registration @@ -42,6 +46,9 @@ extern NSString *const TSLazyRestoreAttachmentsDatabaseViewExtensionName; + (void)asyncRegisterThreadDatabaseView:(OWSStorage *)storage; + (void)asyncRegisterThreadInteractionsDatabaseView:(OWSStorage *)storage; ++ (void)asyncRegisterLegacyThreadInteractionsDatabaseView:(OWSStorage *)storage; + +// MJK TODO - dynamic interactions + (void)asyncRegisterThreadOutgoingMessagesDatabaseView:(OWSStorage *)storage; // Instances of OWSReadTracking for wasRead is NO and shouldAffectUnreadCounts is YES. @@ -54,6 +61,7 @@ extern NSString *const TSLazyRestoreAttachmentsDatabaseViewExtensionName; // Instances of OWSReadTracking for wasRead is NO. + (void)asyncRegisterUnseenDatabaseView:(OWSStorage *)storage; +// MJK reconsider this? It used to be for SN changes and dynamic interactions. Now maybe only SN changes? + (void)asyncRegisterThreadSpecialMessagesDatabaseView:(OWSStorage *)storage; + (void)asyncRegisterSecondaryDevicesDatabaseView:(OWSStorage *)storage; diff --git a/SignalServiceKit/src/Storage/TSDatabaseView.m b/SignalServiceKit/src/Storage/TSDatabaseView.m index ce24c1087..c1669e512 100644 --- a/SignalServiceKit/src/Storage/TSDatabaseView.m +++ b/SignalServiceKit/src/Storage/TSDatabaseView.m @@ -24,7 +24,18 @@ NSString *const TSSecondaryDevicesGroup = @"TSSecondaryDevicesGroup"; // YAPDB BUG: when changing from non-persistent to persistent view, we had to rename TSThreadDatabaseViewExtensionName // -> TSThreadDatabaseViewExtensionName2 to work around https://github.com/yapstudios/YapDatabase/issues/324 NSString *const TSThreadDatabaseViewExtensionName = @"TSThreadDatabaseViewExtensionName2"; -NSString *const TSMessageDatabaseViewExtensionName = @"TSMessageDatabaseViewExtensionName"; + +// We sort interactions by a monotonically increasing counter. +// +// Previously we sorted the interactions database by local timestamp, which was problematic if the local clock changed. +// We need to maintain the legacy extension for purposes of migration. +// +// The "Legacy" sorting extension name constant has the same value as always, so that it won't need to be rebuilt, while +// the "Modern" sorting extension name constant has the same symbol name as we've always used for sorting interactions, +// so that the callsites won't need to change. +NSString *const TSMessageDatabaseViewExtensionName = @"TSMessageDatabaseViewExtensionName_Monotonic"; +NSString *const TSMessageDatabaseViewExtensionName_Legacy = @"TSMessageDatabaseViewExtensionName"; + NSString *const TSThreadOutgoingMessageDatabaseViewExtensionName = @"TSThreadOutgoingMessageDatabaseViewExtensionName"; NSString *const TSUnreadDatabaseViewExtensionName = @"TSUnreadDatabaseViewExtensionName"; NSString *const TSUnseenDatabaseViewExtensionName = @"TSUnseenDatabaseViewExtensionName"; @@ -151,6 +162,71 @@ NSString *const TSLazyRestoreAttachmentsGroup = @"TSLazyRestoreAttachmentsGroup" storage:storage]; } ++ (void)asyncRegisterLegacyThreadInteractionsDatabaseView:(OWSStorage *)storage +{ + OWSAssertIsOnMainThread(); + OWSAssert(storage); + + YapDatabaseView *existingView = [storage registeredExtension:TSMessageDatabaseViewExtensionName_Legacy]; + if (existingView) { + OWSFailDebug(@"Registered database view twice: %@", TSMessageDatabaseViewExtensionName_Legacy); + return; + } + + YapDatabaseViewGrouping *viewGrouping = [YapDatabaseViewGrouping withObjectBlock:^NSString *( + YapDatabaseReadTransaction *transaction, NSString *collection, NSString *key, id object) { + if (![object isKindOfClass:[TSInteraction class]]) { + OWSFailDebug(@"%@ Unexpected entity %@ in collection: %@", self.logTag, [object class], collection); + return nil; + } + TSInteraction *interaction = (TSInteraction *)object; + + return interaction.uniqueThreadId; + }]; + + YapDatabaseViewSorting *viewSorting = + [YapDatabaseViewSorting withObjectBlock:^NSComparisonResult(YapDatabaseReadTransaction *transaction, + NSString *group, + NSString *collection1, + NSString *key1, + id object1, + NSString *collection2, + NSString *key2, + id object2) { + if (![object1 isKindOfClass:[TSInteraction class]]) { + OWSFailDebug(@"%@ Unexpected entity %@ in collection: %@", self.logTag, [object1 class], collection1); + return NSOrderedSame; + } + if (![object2 isKindOfClass:[TSInteraction class]]) { + OWSFailDebug(@"%@ Unexpected entity %@ in collection: %@", self.logTag, [object2 class], collection2); + return NSOrderedSame; + } + TSInteraction *interaction1 = (TSInteraction *)object1; + TSInteraction *interaction2 = (TSInteraction *)object2; + + uint64_t timestamp1 = interaction1.timestampForSorting; + uint64_t timestamp2 = interaction2.timestampForSorting; + + if (timestamp1 > timestamp2) { + return NSOrderedDescending; + } else if (timestamp1 < timestamp2) { + return NSOrderedAscending; + } else { + return NSOrderedSame; + } + }]; + + YapDatabaseViewOptions *options = [YapDatabaseViewOptions new]; + options.isPersistent = YES; + options.allowedCollections = + [[YapWhitelistBlacklist alloc] initWithWhitelist:[NSSet setWithObject:[TSInteraction collection]]]; + + YapDatabaseView *view = + [[YapDatabaseAutoView alloc] initWithGrouping:viewGrouping sorting:viewSorting versionTag:@"1" options:options]; + + [storage asyncRegisterExtension:view withName:TSMessageDatabaseViewExtensionName_Legacy]; +} + + (void)asyncRegisterThreadInteractionsDatabaseView:(OWSStorage *)storage { YapDatabaseViewGrouping *viewGrouping = [YapDatabaseViewGrouping withObjectBlock:^NSString *( @@ -427,6 +503,7 @@ NSString *const TSLazyRestoreAttachmentsGroup = @"TSLazyRestoreAttachmentsGroup" return result; } +// MJK TODO - dynamic interactions + (id)threadOutgoingMessageDatabaseView:(YapDatabaseReadTransaction *)transaction { OWSAssertDebug(transaction);