Merge branch 'charlesmchen/dbMigrationsVsStorageReadiness'

Signal/src/AppDelegate.m

@@ -6,8 +6,8 @@
 #import "AppStoreRating.h"
 #import "AppUpdateNag.h"
 #import "CodeVerificationViewController.h"
-#import "HomeViewController.h"
 #import "DebugLogger.h"
+#import "HomeViewController.h"
 #import "MainAppContext.h"
 #import "NotificationsManager.h"
 #import "OWS2FASettingsViewController.h"
@@ -156,11 +156,16 @@ static NSString *const kURLHostVerifyPrefix             = @"verify";
     // This block will be cleared in storageIsReady.
     [DeviceSleepManager.sharedInstance addBlockWithBlockObject:self];
 
-    [AppSetup setupEnvironment:^{
+    [AppSetup setupEnvironmentWithCallMessageHandlerBlock:^{
         return SignalApp.sharedApp.callMessageHandler;
     }
         notificationsProtocolBlock:^{
             return SignalApp.sharedApp.notificationsManager;
+        }
+        migrationCompletion:^{
+            OWSAssertIsOnMainThread();
+
+            [self versionMigrationsDidComplete];
         }];
 
     [UIUtil applySignalAppearence];
@@ -176,14 +181,6 @@ static NSString *const kURLHostVerifyPrefix             = @"verify";
     mainWindow.rootViewController = [self loadingRootViewController];
     [mainWindow makeKeyAndVisible];
 
-    // performUpdateCheck must be invoked after Environment has been initialized because
-    // upgrade process may depend on Environment.
-    [VersionMigrations performUpdateCheckWithCompletion:^{
-        OWSAssertIsOnMainThread();
-
-        [self versionMigrationsDidComplete];
-    }];
-
     // Accept push notification when app is not open
     NSDictionary *remoteNotif = launchOptions[UIApplicationLaunchOptionsRemoteNotificationKey];
     if (remoteNotif) {
@@ -1089,6 +1086,7 @@ static NSString *const kURLHostVerifyPrefix             = @"verify";
     [AppVersion.instance mainAppLaunchDidComplete];
 
     [Environment.current.contactsManager loadSignalAccountsFromCache];
+    [Environment.current.contactsManager startObserving];
 
     // If there were any messages in our local queue which we hadn't yet processed.
     [[OWSMessageReceiver sharedInstance] handleAnyUnprocessedEnvelopesAsync];

SignalMessaging/environment/AppSetup.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
@@ -13,8 +13,9 @@ typedef id<NotificationsProtocol> _Nonnull (^NotificationsManagerBlock)(void);
 // This is _NOT_ a singleton and will be instantiated each time that the SAE is used.
 @interface AppSetup : NSObject
 
-+ (void)setupEnvironment:(CallMessageHandlerBlock)callMessageHandlerBlock
-    notificationsProtocolBlock:(NotificationsManagerBlock)notificationsManagerBlock;
++ (void)setupEnvironmentWithCallMessageHandlerBlock:(CallMessageHandlerBlock)callMessageHandlerBlock
+                         notificationsProtocolBlock:(NotificationsManagerBlock)notificationsManagerBlock
+                                migrationCompletion:(dispatch_block_t)migrationCompletion;
 
 @end
 

SignalMessaging/environment/AppSetup.m

@@ -18,11 +18,16 @@ NS_ASSUME_NONNULL_BEGIN
 
 @implementation AppSetup
 
-+ (void)setupEnvironment:(CallMessageHandlerBlock)callMessageHandlerBlock
-    notificationsProtocolBlock:(NotificationsManagerBlock)notificationsManagerBlock
++ (void)setupEnvironmentWithCallMessageHandlerBlock:(CallMessageHandlerBlock)callMessageHandlerBlock
+                         notificationsProtocolBlock:(NotificationsManagerBlock)notificationsManagerBlock
+                                migrationCompletion:(dispatch_block_t)migrationCompletion
 {
     OWSAssert(callMessageHandlerBlock);
     OWSAssert(notificationsManagerBlock);
+    OWSAssert(migrationCompletion);
+
+    __block OWSBackgroundTask *_Nullable backgroundTask =
+        [OWSBackgroundTask backgroundTaskWithLabelStr:__PRETTY_FUNCTION__];
 
     static dispatch_once_t onceToken;
     dispatch_once(&onceToken, ^{
@@ -46,8 +51,16 @@ NS_ASSUME_NONNULL_BEGIN
         [NSKeyedUnarchiver setClass:[OWSUserProfile class] forClassName:[OWSUserProfile collection]];
         [NSKeyedUnarchiver setClass:[OWSDatabaseMigration class] forClassName:[OWSDatabaseMigration collection]];
 
-        [OWSStorage setupStorage];
-        [[Environment current].contactsManager startObserving];
+        [OWSStorage registerExtensionsWithMigrationBlock:^() {
+            // Don't start database migrations until storage is ready.
+            [VersionMigrations performUpdateCheckWithCompletion:^() {
+                OWSAssertIsOnMainThread();
+
+                migrationCompletion();
+
+                backgroundTask = nil;
+            }];
+        }];
     });
 }
 

SignalMessaging/environment/VersionMigrations.m

@@ -33,6 +33,8 @@ NS_ASSUME_NONNULL_BEGIN
 
 + (void)performUpdateCheckWithCompletion:(VersionMigrationCompletion)completion
 {
+    DDLogInfo(@"%@ %s", self.logTag, __PRETTY_FUNCTION__);
+
     // performUpdateCheck must be invoked after Environment has been initialized because
     // upgrade process may depend on Environment.
     OWSAssert([Environment current]);
@@ -86,8 +88,10 @@ NS_ASSUME_NONNULL_BEGIN
         [self clearBloomFilterCache];
     }
 
-    [[[OWSDatabaseMigrationRunner alloc] initWithPrimaryStorage:[OWSPrimaryStorage sharedManager]]
-        runAllOutstandingWithCompletion:completion];
+    dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{
+        [[[OWSDatabaseMigrationRunner alloc] initWithPrimaryStorage:[OWSPrimaryStorage sharedManager]]
+            runAllOutstandingWithCompletion:completion];
+    });
 }
 
 + (BOOL)isVersion:(NSString *)thisVersionString

SignalMessaging/environment/migrations/OWSDatabaseMigration.m

@@ -58,16 +58,6 @@ NS_ASSUME_NONNULL_BEGIN
     OWSAssert(completion);
 
     OWSDatabaseConnection *dbConnection = (OWSDatabaseConnection *)self.primaryStorage.newDatabaseConnection;
-    // These migrations won't be run until storage registrations are enqueued,
-    // but this transaction might begin before all registrations are marked as
-    // complete, so disable this checking.
-    //
-    // TODO: Once we move "app readiness" into AppSetup, we should explicitly
-    // not start these migrations until storage is ready.  We can then remove
-    // this statement which disables checking.
-#ifdef DEBUG
-    dbConnection.canWriteBeforeStorageReady = YES;
-#endif
 
     [dbConnection asyncReadWriteWithBlock:^(YapDatabaseReadWriteTransaction *_Nonnull transaction) {
         [self runUpWithTransaction:transaction];
@@ -88,18 +78,12 @@ NS_ASSUME_NONNULL_BEGIN
     return self.dbReadWriteConnection;
 }
 
-// Database migrations need to occur _before_ storage is ready (by definition),
-// so we need to use a connection with canWriteBeforeStorageReady set in
-// debug builds.
 + (YapDatabaseConnection *)dbReadWriteConnection
 {
     static dispatch_once_t onceToken;
     static YapDatabaseConnection *sharedDBConnection;
     dispatch_once(&onceToken, ^{
         sharedDBConnection = [OWSPrimaryStorage sharedManager].newDatabaseConnection;
-
-        OWSAssert([sharedDBConnection isKindOfClass:[OWSDatabaseConnection class]]);
-        ((OWSDatabaseConnection *)sharedDBConnection).canWriteBeforeStorageReady = YES;
     });
 
     return sharedDBConnection;

SignalServiceKit/src/Storage/OWSStorage.h

@@ -22,10 +22,6 @@ extern NSString *const StorageIsReadyNotification;
 
 @property (atomic, weak) id<OWSDatabaseConnectionDelegate> delegate;
 
-#ifdef DEBUG
-@property (atomic) BOOL canWriteBeforeStorageReady;
-#endif
-
 - (instancetype)init NS_UNAVAILABLE;
 - (instancetype)initWithDatabase:(YapDatabase *)database
                         delegate:(id<OWSDatabaseConnectionDelegate>)delegate NS_DESIGNATED_INITIALIZER;
@@ -48,6 +44,8 @@ extern NSString *const StorageIsReadyNotification;
 
 #pragma mark -
 
+typedef void (^OWSStorageMigrationBlock)(void);
+
 @interface OWSStorage : NSObject
 
 - (instancetype)init NS_UNAVAILABLE;
@@ -60,7 +58,7 @@ extern NSString *const StorageIsReadyNotification;
 // This object can be used to filter database notifications.
 @property (nonatomic, readonly, nullable) id dbNotificationObject;
 
-+ (void)setupStorage;
++ (void)registerExtensionsWithMigrationBlock:(OWSStorageMigrationBlock)migrationBlock;
 
 + (void)resetAllStorage;
 

SignalServiceKit/src/Storage/OWSStorage.m

@@ -74,7 +74,7 @@ typedef NSData *_Nullable (^CreateDatabaseMetadataBlock)(void);
 {
     id<OWSDatabaseConnectionDelegate> delegate = self.delegate;
     OWSAssert(delegate);
-    OWSAssert(delegate.areAllRegistrationsComplete || self.canWriteBeforeStorageReady);
+    OWSAssert(delegate.areAllRegistrationsComplete);
 
     OWSBackgroundTask *_Nullable backgroundTask = nil;
     if (CurrentAppContext().isMainApp) {
@@ -101,7 +101,7 @@ typedef NSData *_Nullable (^CreateDatabaseMetadataBlock)(void);
 {
     id<OWSDatabaseConnectionDelegate> delegate = self.delegate;
     OWSAssert(delegate);
-    OWSAssert(delegate.areAllRegistrationsComplete || self.canWriteBeforeStorageReady);
+    OWSAssert(delegate.areAllRegistrationsComplete);
 
     __block OWSBackgroundTask *_Nullable backgroundTask = nil;
     if (CurrentAppContext().isMainApp) {
@@ -176,13 +176,6 @@ typedef NSData *_Nullable (^CreateDatabaseMetadataBlock)(void);
 - (YapDatabaseConnection *)registrationConnection
 {
     YapDatabaseConnection *connection = [super registrationConnection];
-
-#ifdef DEBUG
-    // Flag the registration connection as such.
-    OWSAssert([connection isKindOfClass:[OWSDatabaseConnection class]]);
-    ((OWSDatabaseConnection *)connection).canWriteBeforeStorageReady = YES;
-#endif
-
     return connection;
 }
 
@@ -332,29 +325,24 @@ typedef NSData *_Nullable (^CreateDatabaseMetadataBlock)(void);
     OWS_ABSTRACT_METHOD();
 }
 
-+ (NSArray<OWSStorage *> *)allStorages
++ (void)registerExtensionsWithMigrationBlock:(OWSStorageMigrationBlock)migrationBlock
 {
-    return @[
-        OWSPrimaryStorage.sharedManager,
-    ];
-}
+    OWSAssert(migrationBlock);
 
-+ (void)setupStorage
-{
     __block OWSBackgroundTask *_Nullable backgroundTask =
         [OWSBackgroundTask backgroundTaskWithLabelStr:__PRETTY_FUNCTION__];
 
-    for (OWSStorage *storage in self.allStorages) {
-        [storage runSyncRegistrations];
-    }
+    [OWSPrimaryStorage.sharedManager runSyncRegistrations];
 
-    for (OWSStorage *storage in self.allStorages) {
-        [storage runAsyncRegistrationsWithCompletion:^{
-            if ([self postRegistrationCompleteNotificationIfPossible]) {
-                backgroundTask = nil;
-            }
-        }];
-    }
+    [OWSPrimaryStorage.sharedManager runAsyncRegistrationsWithCompletion:^{
+        OWSAssert(self.isStorageReady);
+
+        [self postRegistrationCompleteNotification];
+
+        migrationBlock();
+
+        backgroundTask = nil;
+    }];
 }
 
 - (YapDatabaseConnection *)registrationConnection
@@ -363,11 +351,11 @@ typedef NSData *_Nullable (^CreateDatabaseMetadataBlock)(void);
 }
 
 // Returns YES IFF all registrations are complete.
-+ (BOOL)postRegistrationCompleteNotificationIfPossible
++ (void)postRegistrationCompleteNotification
 {
-    if (!self.isStorageReady) {
-        return NO;
-    }
+    OWSAssert(self.isStorageReady);
+
+    DDLogInfo(@"%@ %s", self.logTag, __PRETTY_FUNCTION__);
 
     static dispatch_once_t onceToken;
     dispatch_once(&onceToken, ^{
@@ -375,18 +363,11 @@ typedef NSData *_Nullable (^CreateDatabaseMetadataBlock)(void);
                                                                  object:nil
                                                                userInfo:nil];
     });
-
-    return YES;
 }
 
 + (BOOL)isStorageReady
 {
-    for (OWSStorage *storage in self.allStorages) {
-        if (!storage.areAllRegistrationsComplete) {
-            return NO;
-        }
-    }
-    return YES;
+    return OWSPrimaryStorage.sharedManager.areAllRegistrationsComplete;
 }
 
 - (BOOL)tryToLoadDatabase

SignalShareExtension/ShareViewController.swift

@@ -88,20 +88,20 @@ public class ShareViewController: UIViewController, ShareViewDelegate, SAEFailed
         }.retainUntilComplete()
 
         // We shouldn't set up our environment until after we've consulted isReadyForAppExtensions.
-        AppSetup.setupEnvironment({
+        AppSetup.setupEnvironment(callMessageHandlerBlock: {
             return NoopCallMessageHandler()
-        }) {
+        },
+                                  notificationsProtocolBlock: {
             return NoopNotificationsManager()
-        }
+        },
+                                  migrationCompletion: { [weak self] in
+                                    SwiftAssertIsOnMainThread(#function)
 
-        // performUpdateCheck must be invoked after Environment has been initialized because
-        // upgrade process may depend on Environment.
-        VersionMigrations.performUpdateCheck(completion: { [weak self] in
-            SwiftAssertIsOnMainThread(#function)
-
-            guard let strongSelf = self else { return }
+                                    guard let strongSelf = self else { return }
 
-            strongSelf.versionMigrationsDidComplete()
+                                    // performUpdateCheck must be invoked after Environment has been initialized because
+                                    // upgrade process may depend on Environment.
+                                    strongSelf.versionMigrationsDidComplete()
         })
 
         // We don't need to use "screen protection" in the SAE.
@@ -268,6 +268,7 @@ public class ShareViewController: UIViewController, ShareViewDelegate, SAEFailed
         AppVersion.instance().saeLaunchDidComplete()
 
         Environment.current().contactsManager.loadSignalAccountsFromCache()
+        Environment.current().contactsManager.startObserving()
 
         ensureRootViewController()