Resolve issues around database conversion.

Signal/test/util/OWSDatabaseConverterTest.m

@@ -6,7 +6,9 @@
 #import "OWSDatabaseConverter.h"
 #import <Curve25519Kit/Randomness.h>
 #import <SignalServiceKit/OWSStorage.h>
+#import <SignalServiceKit/YapDatabaseConnection+OWS.h>
 #import <YapDatabase/YapDatabase.h>
+#import <YapDatabase/YapDatabasePrivate.h>
 
 NS_ASSUME_NONNULL_BEGIN
 
@@ -33,16 +35,25 @@ NS_ASSUME_NONNULL_BEGIN
     return [Randomness generateRandomBytes:30];
 }
 
-- (nullable NSString *)createUnconvertedDatabase:(NSData *)passwordData
+- (void)openYapDatabase:(NSString *)databaseFilePath
+       databasePassword:(NSData *)databasePassword
+          databaseBlock:(void (^_Nonnull)(YapDatabase *))databaseBlock
 {
-    NSString *temporaryDirectory = NSTemporaryDirectory();
+    OWSAssert(databaseFilePath.length > 0);
-    NSString *filename = [NSUUID UUID].UUIDString;
+    OWSAssert(databasePassword.length > 0);
-    NSString *databaseFilePath = [temporaryDirectory stringByAppendingPathComponent:filename];
+    OWSAssert(databaseBlock);
 
+    DDLogVerbose(@"openYapDatabase: %@", databaseFilePath);
+
+    __weak YapDatabase *_Nullable weakDatabase = nil;
+    dispatch_queue_t snapshotQueue;
+    dispatch_queue_t writeQueue;
+
+    @autoreleasepool {
         YapDatabaseOptions *options = [[YapDatabaseOptions alloc] init];
         options.corruptAction = YapDatabaseCorruptAction_Fail;
         options.cipherKeyBlock = ^{
-        return passwordData;
+            return databasePassword;
         };
         options.enableMultiProcessSupport = YES;
 
@@ -57,13 +68,89 @@ NS_ASSUME_NONNULL_BEGIN
                                                      deserializer:[OWSStorage logOnFailureDeserializer]
                                                           options:options];
         OWSAssert(database);
-    return database ? databaseFilePath : nil;
+
+        weakDatabase = database;
+        snapshotQueue = database->snapshotQueue;
+        writeQueue = database->writeQueue;
+
+        databaseBlock(database);
+
+        // Close the database.
+        database = nil;
+    }
+
+    // Flush the database's queues, which may contain lingering
+    // references to the database.
+    dispatch_sync(snapshotQueue,
+        ^{
+        });
+    dispatch_sync(writeQueue,
+        ^{
+        });
+
+    // Wait for notifications from writes to be fired.
+    {
+        XCTestExpectation *expectation = [self expectationWithDescription:@"Database modified notifications"];
+
+        dispatch_async(dispatch_get_main_queue(), ^{
+            // Database modified notifications are fired on the main queue.
+            // Once this block executes, the main queue has been flushed
+            // and we know that all database modified notifications are
+            // complete.
+            [expectation fulfill];
+        });
+
+        [self waitForExpectationsWithTimeout:5.0
+                                     handler:^(NSError *error) {
+                                         if (error) {
+                                             NSLog(@"Timeout Error: %@", error);
+                                         }
+                                     }];
+    }
+
+    YapDatabase *_Nullable strongDatabase = weakDatabase;
+    OWSAssert(!strongDatabase);
+}
+
+- (nullable NSString *)createUnconvertedDatabase:(NSData *)databasePassword
+{
+    NSString *temporaryDirectory = NSTemporaryDirectory();
+    NSString *filename = [NSUUID UUID].UUIDString;
+    NSString *databaseFilePath = [temporaryDirectory stringByAppendingPathComponent:filename];
+
+    [self openYapDatabase:databaseFilePath
+         databasePassword:databasePassword
+            databaseBlock:^(YapDatabase *database) {
+                YapDatabaseConnection *dbConnection = database.newConnection;
+                [dbConnection setObject:@(YES) forKey:@"test_key_name" inCollection:@"test_collection_name"];
+                [dbConnection flushTransactionsWithCompletionQueue:dispatch_get_main_queue() completionBlock:nil];
+            }];
+
+    OWSAssert([[NSFileManager defaultManager] fileExistsAtPath:databaseFilePath]);
+
+    [self openYapDatabase:databaseFilePath
+         databasePassword:databasePassword
+            databaseBlock:^(YapDatabase *database) {
+                YapDatabaseConnection *dbConnection = database.newConnection;
+                id _Nullable value = [dbConnection objectForKey:@"test_key_name" inCollection:@"test_collection_name"];
+                OWSAssert([@(YES) isEqual:value]);
+            }];
+
+    OWSAssert([[NSFileManager defaultManager] fileExistsAtPath:databaseFilePath]);
+
+    NSError *_Nullable error = nil;
+    NSDictionary *fileAttributes =
+        [[NSFileManager defaultManager] attributesOfItemAtPath:databaseFilePath error:&error];
+    OWSAssert(fileAttributes && !error);
+    DDLogVerbose(@"%@ test database file size: %@", self.logTag, fileAttributes[NSFileSize]);
+
+    return databaseFilePath;
 }
 
 - (void)testDoesDatabaseNeedToBeConverted_Unconverted
 {
-    NSData *passwordData = [self randomDatabasePassword];
+    NSData *databasePassword = [self randomDatabasePassword];
-    NSString *_Nullable databaseFilePath = [self createUnconvertedDatabase:passwordData];
+    NSString *_Nullable databaseFilePath = [self createUnconvertedDatabase:databasePassword];
     XCTAssertTrue([OWSDatabaseConverter doesDatabaseNeedToBeConverted:databaseFilePath]);
 }
 
@@ -74,10 +161,15 @@ NS_ASSUME_NONNULL_BEGIN
 
 - (void)testDatabaseConversion
 {
-    NSData *passwordData = [self randomDatabasePassword];
+    NSData *databasePassword = [self randomDatabasePassword];
-    NSString *_Nullable databaseFilePath = [self createUnconvertedDatabase:passwordData];
+    NSString *_Nullable databaseFilePath = [self createUnconvertedDatabase:databasePassword];
     XCTAssertTrue([OWSDatabaseConverter doesDatabaseNeedToBeConverted:databaseFilePath]);
-    [OWSDatabaseConverter convertDatabaseIfNecessary:databaseFilePath];
+    NSError *_Nullable error =
+        [OWSDatabaseConverter convertDatabaseIfNecessary:databaseFilePath databasePassword:databasePassword];
+    if (error) {
+        DDLogError(@"%s error: %@", __PRETTY_FUNCTION__, error);
+    }
+    XCTAssertNil(error);
     XCTAssertFalse([OWSDatabaseConverter doesDatabaseNeedToBeConverted:databaseFilePath]);
 }
 

SignalMessaging/utils/OWSDatabaseConverter.h

@@ -12,8 +12,9 @@ NS_ASSUME_NONNULL_BEGIN
 
 - (instancetype)init NS_UNAVAILABLE;
 
-+ (void)convertDatabaseIfNecessary;
++ (nullable NSError *)convertDatabaseIfNecessary;
-+ (void)convertDatabaseIfNecessary:(NSString *)databaseFilePath;
++ (nullable NSError *)convertDatabaseIfNecessary:(NSString *)databaseFilePath
+                                databasePassword:(NSData *)databasePassword;
 
 @end
 

SignalMessaging/utils/OWSDatabaseConverter.m

@@ -4,42 +4,64 @@
 
 #import "OWSDatabaseConverter.h"
 #import "sqlite3.h"
+#import <SignalServiceKit/NSData+hexString.h>
 #import <SignalServiceKit/OWSError.h>
 #import <SignalServiceKit/OWSFileSystem.h>
 #import <SignalServiceKit/TSStorageManager.h>
 
 NS_ASSUME_NONNULL_BEGIN
 
-const int kSqliteHeaderLength = 32;
+const NSUInteger kSqliteHeaderLength = 32;
+
+@interface OWSStorage (OWSDatabaseConverter)
+
++ (YapDatabaseDeserializer)logOnFailureDeserializer;
+
+@end
+
+#pragma mark -
 
 @implementation OWSDatabaseConverter
 
-+ (BOOL)doesDatabaseNeedToBeConverted:(NSString *)databaseFilePath
++ (NSData *)readFirstNBytesOfDatabaseFile:(NSString *)filePath byteCount:(NSUInteger)byteCount
 {
-    OWSAssert(databaseFilePath.length > 0);
+    OWSAssert(filePath.length > 0);
 
-    if (![[NSFileManager defaultManager] fileExistsAtPath:databaseFilePath]) {
+    @autoreleasepool {
-        DDLogVerbose(@"%@ Skipping database conversion; no legacy database found.", self.logTag);
-        return NO;
-    }
         NSError *error;
         // We use NSDataReadingMappedAlways instead of NSDataReadingMappedIfSafe because
         // we know the database will always exist for the duration of this instance of NSData.
-    NSData *_Nullable data = [NSData dataWithContentsOfURL:[NSURL fileURLWithPath:databaseFilePath]
+        NSData *_Nullable data = [NSData dataWithContentsOfURL:[NSURL fileURLWithPath:filePath]
                                                        options:NSDataReadingMappedAlways
                                                          error:&error];
         if (!data || error) {
-        DDLogError(@"%@ Couldn't read legacy database file header.", self.logTag);
+            DDLogError(@"%@ Couldn't read database file header.", self.logTag);
             // TODO: Make a convenience method (on a category of NSException?) that
             // flushes DDLog before raising a terminal exception.
-        [NSException raise:@"Couldn't read legacy database file header" format:@""];
+            [NSException raise:@"Couldn't read database file header" format:@""];
         }
         // Pull this constant out so that we can use it in our YapDatabase fork.
-    NSData *_Nullable headerData = [data subdataWithRange:NSMakeRange(0, kSqliteHeaderLength)];
+        NSData *_Nullable headerData = [data subdataWithRange:NSMakeRange(0, byteCount)];
-    if (!headerData || headerData.length != kSqliteHeaderLength) {
+        if (!headerData || headerData.length != byteCount) {
-        [NSException raise:@"Database database file header has unexpected length"
+            [NSException raise:@"Database file header has unexpected length"
-                    format:@"Database database file header has unexpected length: %zd", headerData.length];
+                        format:@"Database file header has unexpected length: %zd", headerData.length];
+        }
+        return [headerData copy];
     }
+}
+
++ (BOOL)doesDatabaseNeedToBeConverted:(NSString *)databaseFilePath
+{
+    OWSAssert(databaseFilePath.length > 0);
+
+    if (![[NSFileManager defaultManager] fileExistsAtPath:databaseFilePath]) {
+        DDLogVerbose(@"%@ database file not found.", self.logTag);
+        return nil;
+    }
+
+    NSData *headerData = [self readFirstNBytesOfDatabaseFile:databaseFilePath byteCount:kSqliteHeaderLength];
+    OWSAssert(headerData);
+
     NSString *kUnencryptedHeader = @"SQLite format 3\0";
     NSData *unencryptedHeaderData = [kUnencryptedHeader dataUsingEncoding:NSUTF8StringEncoding];
     BOOL isUnencrypted = [unencryptedHeaderData
@@ -52,36 +74,46 @@ const int kSqliteHeaderLength = 32;
     return YES;
 }
 
-+ (void)convertDatabaseIfNecessary
++ (nullable NSError *)convertDatabaseIfNecessary
 {
     NSString *databaseFilePath = [TSStorageManager legacyDatabaseFilePath];
-    [self convertDatabaseIfNecessary:databaseFilePath];
+
+    NSError *error;
+    NSData *_Nullable databasePassword = [OWSStorage tryToLoadDatabasePassword:&error];
+    if (!databasePassword || error) {
+        return (error
+                ?: OWSErrorWithCodeDescription(
+                       OWSErrorCodeDatabaseConversionFatalError, @"Failed to load database password"));
+    }
+
+    return [self convertDatabaseIfNecessary:databaseFilePath databasePassword:databasePassword];
 }
 
 // TODO upon failure show user error UI
 // TODO upon failure anything we need to do "back out" partial migration
-+ (void)convertDatabaseIfNecessary:(NSString *)databaseFilePath
++ (nullable NSError *)convertDatabaseIfNecessary:(NSString *)databaseFilePath
+                                databasePassword:(NSData *)databasePassword
 {
     if (![self doesDatabaseNeedToBeConverted:databaseFilePath]) {
-        return;
+        return nil;
     }
 
-    [self convertDatabase:(NSString *)databaseFilePath];
+    return [self convertDatabase:(NSString *)databaseFilePath databasePassword:databasePassword];
 }
 
-+ (nullable NSError *)convertDatabase:(NSString *)databaseFilePath
++ (nullable NSError *)convertDatabase:(NSString *)databaseFilePath databasePassword:(NSData *)databasePassword
 {
     OWSAssert(databaseFilePath.length > 0);
+    OWSAssert(databasePassword.length > 0);
 
-    NSError *error;
+    NSData *headerData = [self readFirstNBytesOfDatabaseFile:databaseFilePath byteCount:kSqliteHeaderLength];
-    NSData *_Nullable databasePassword = [OWSStorage tryToLoadDatabasePassword:&error];
+    OWSAssert(headerData);
-    if (!databasePassword || error) {
-        return (error
-                ?: OWSErrorWithCodeDescription(
-                       OWSErrorCodeDatabaseConversionFatalError, @"Failed to load database password"));
-    }
 
-    // TODO:
+    const NSUInteger kSQLCipherSaltLength = 16;
+    OWSAssert(headerData.length >= kSQLCipherSaltLength);
+    NSData *sqlCipherSaltData = [headerData subdataWithRange:NSMakeRange(0, kSQLCipherSaltLength)];
+
+    // TODO: Write salt to keychain.
 
     //    Hello Matthew,
     //
@@ -181,13 +213,10 @@ const int kSqliteHeaderLength = 32;
         return OWSErrorWithCodeDescription(OWSErrorCodeDatabaseConversionFatalError, @"Failed to set SQLCipher key");
     }
 
-    // TODO set plaintext pragma
-    // TODO modify first page
-    // TODO force checkpoint
-
     // -----------------------------------------------------------
     //
     // This block was derived from [Yapdatabase configureDatabase].
+    {
         //
         //    {
         //        int status;
@@ -287,33 +316,53 @@ const int kSqliteHeaderLength = 32;
 
         // END DB setup copied from YapDatabase
         // BEGIN SQLCipher migration
+    }
 
-    NSString *setPlainTextHeaderPragma =
-        [NSString stringWithFormat:@"PRAGMA cipher_plaintext_header_size = %d;", kSqliteHeaderLength];
 
-    status = sqlite3_exec(db, [setPlainTextHeaderPragma UTF8String], NULL, NULL, NULL);
+    // -----------------------------------------------------------
-    if (status != SQLITE_OK) {
+    //
-        DDLogError(@"Error setting PRAGMA cipher_plaintext_header_size = %d: status: %d, error: %s",
+    // SQLCipher migration
-            kSqliteHeaderLength,
-            status,
-            sqlite3_errmsg(db));
-        return OWSErrorWithCodeDescription(
-            OWSErrorCodeDatabaseConversionFatalError, @"Failed to set PRAGMA cipher_plaintext_header_size");
-    }
 
-    // Modify the first page, so that SQLCipher will overwrite, respecting our new cipher_plaintext_header_size
+    //    if (NO)
-    NSString *tableName = [NSString stringWithFormat:@"signal-migration-%@", [NSUUID new].UUIDString];
+    //    {
-    NSString *modificationSQL =
+    //
-        [NSString stringWithFormat:@"CREATE TABLE %@(int a); INSERT INTO %@(a) VALUES (1);", tableName, tableName];
+    //    NSString *setPlainTextHeaderPragma =
-    status = sqlite3_exec(db, [modificationSQL UTF8String], NULL, NULL, NULL);
+    //    [NSString stringWithFormat:@"PRAGMA cipher_plaintext_header_size = %zd;", kSqliteHeaderLength];
-    if (status != SQLITE_OK) {
+    //
-        DDLogError(@"%@ Error modifying first page: %d, error: %s", self.logTag, status, sqlite3_errmsg(db));
+    //    status = sqlite3_exec(db, [setPlainTextHeaderPragma UTF8String], NULL, NULL, NULL);
-        return OWSErrorWithCodeDescription(OWSErrorCodeDatabaseConversionFatalError, @"Error modifying first page");
+    //    if (status != SQLITE_OK) {
-    }
+    //        DDLogError(@"Error setting PRAGMA cipher_plaintext_header_size = %zd: status: %d, error: %s",
+    //                   kSqliteHeaderLength,
+    //                   status,
+    //                   sqlite3_errmsg(db));
+    //        return OWSErrorWithCodeDescription(
+    //                                           OWSErrorCodeDatabaseConversionFatalError, @"Failed to set PRAGMA
+    //                                           cipher_plaintext_header_size");
+    //    }
+    //
+    //    // Modify the first page, so that SQLCipher will overwrite, respecting our new cipher_plaintext_header_size
+    //    NSString *tableName = [NSString stringWithFormat:@"signal-migration-%@", [NSUUID new].UUIDString];
+    //    NSString *modificationSQL =
+    //    [NSString stringWithFormat:@"CREATE TABLE %@(int a); INSERT INTO %@(a) VALUES (1);", tableName, tableName];
+    //    status = sqlite3_exec(db, [modificationSQL UTF8String], NULL, NULL, NULL);
+    //    if (status != SQLITE_OK) {
+    //        DDLogError(@"%@ Error modifying first page: %d, error: %s", self.logTag, status, sqlite3_errmsg(db));
+    //        return OWSErrorWithCodeDescription(OWSErrorCodeDatabaseConversionFatalError, @"Error modifying first
+    //        page");
+    //    }
+    //
+    //    // Force a checkpoint so that the plaintext is written to the actual DB file, not just living in the WAL.
+    //    // TODO do we need/want the earlier checkpoint if we're checkpointing here?
+    //    sqlite3_wal_autocheckpoint(db, 0);
+    //
+    //
+    //    sqlite3_close(db);
+    //    return nil;
+    //    }
 
-    // Force a checkpoint so that the plaintext is written to the actual DB file, not just living in the WAL.
+    // TODO set plaintext pragma
-    // TODO do we need/want the earlier checkpoint if we're checkpointing here?
+    // TODO modify first page
-    sqlite3_wal_autocheckpoint(db, 0);
+    // TODO force checkpoint
 
     return nil;
 }