Merge branch 'charlesmchen/fixIncrementalBackups'

Signal/src/ViewControllers/DebugUI/DebugUIBackup.m

@@ -73,6 +73,14 @@ NS_ASSUME_NONNULL_BEGIN
                                      actionBlock:^{
                                          [DebugUIBackup clearBackupMetadataCache];
                                      }]];
+    [items addObject:[OWSTableItem itemWithTitle:@"Log Backup Metadata Cache"
+                                     actionBlock:^{
+                                         [DebugUIBackup logBackupMetadataCache];
+                                     }]];
+    [items addObject:[OWSTableItem itemWithTitle:@"Lazy Restore Attachments"
+                                     actionBlock:^{
+                                         [AppEnvironment.shared.backupLazyRestore runIfNecessary];
+                                     }]];
 
     return [OWSTableSection sectionWithTitle:self.name items:items];
 }
@@ -191,14 +199,14 @@ NS_ASSUME_NONNULL_BEGIN
 
 + (void)clearAllCloudKitRecords
 {
-    OWSLogInfo(@"clearAllCloudKitRecords.");
+    OWSLogInfo(@"");
 
     [OWSBackup.sharedManager clearAllCloudKitRecords];
 }
 
 + (void)clearBackupMetadataCache
 {
-    OWSLogInfo(@"ClearBackupMetadataCache.");
+    OWSLogInfo(@"");
 
     [OWSPrimaryStorage.sharedManager.newDatabaseConnection
         readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) {
@@ -206,6 +214,11 @@ NS_ASSUME_NONNULL_BEGIN
         }];
 }
 
++ (void)logBackupMetadataCache
+{
+    [self.backup logBackupMetadataCache:OWSPrimaryStorage.sharedManager.newDatabaseConnection];
+}
+
 @end
 
 NS_ASSUME_NONNULL_END

Signal/src/util/Backup/OWSBackup.h

@@ -32,6 +32,7 @@ NSError *OWSBackupErrorWithDescription(NSString *description);
 @class OWSBackupIO;
 @class TSAttachmentPointer;
 @class TSThread;
+@class YapDatabaseConnection;
 
 @interface OWSBackup : NSObject
 
@@ -91,6 +92,8 @@ NSError *OWSBackupErrorWithDescription(NSString *description);
 - (void)logBackupRecords;
 - (void)clearAllCloudKitRecords;
 
+- (void)logBackupMetadataCache:(YapDatabaseConnection *)dbConnection;
+
 #pragma mark - Lazy Restore
 
 - (NSArray<NSString *> *)attachmentRecordNamesForLazyRestore;

Signal/src/util/Backup/OWSBackup.m

@@ -866,6 +866,27 @@ NSError *OWSBackupErrorWithDescription(NSString *description)
     return [AnyPromise promiseWithValue:@(1)];
 }
 
+- (void)logBackupMetadataCache:(YapDatabaseConnection *)dbConnection
+{
+    OWSLogInfo(@"");
+
+    [dbConnection readWithBlock:^(YapDatabaseReadTransaction *transaction) {
+        [transaction enumerateKeysAndObjectsInCollection:[OWSBackupFragment collection]
+                                              usingBlock:^(NSString *key, OWSBackupFragment *fragment, BOOL *stop) {
+                                                  OWSLogVerbose(@"fragment: %@, %@, %lu, %@, %@, %@, %@",
+                                                      key,
+                                                      fragment.recordName,
+                                                      (unsigned long)fragment.encryptionKey.length,
+                                                      fragment.relativeFilePath,
+                                                      fragment.attachmentId,
+                                                      fragment.downloadFilePath,
+                                                      fragment.uncompressedDataLength);
+                                              }];
+        OWSLogVerbose(@"Number of fragments: %lu",
+            (unsigned long)[transaction numberOfKeysInCollection:[OWSBackupFragment collection]]);
+    }];
+}
+
 #pragma mark - Notifications
 
 - (void)postDidChangeNotification

Signal/src/util/Backup/OWSBackupExportJob.m

@@ -315,6 +315,8 @@ NS_ASSUME_NONNULL_BEGIN
 // If we are replacing an existing backup, we use some of its contents for continuity.
 @property (nonatomic, nullable) NSSet<NSString *> *lastValidRecordNames;
 
+@property (nonatomic, nullable) YapDatabaseConnection *dbConnection;
+
 @end
 
 #pragma mark -
@@ -354,6 +356,8 @@ NS_ASSUME_NONNULL_BEGIN
 
     [self updateProgressWithDescription:nil progress:nil];
 
+    self.dbConnection = self.primaryStorage.newDatabaseConnection;
+
     [[self.backup ensureCloudKitAccess]
             .thenInBackground(^{
                 [self updateProgressWithDescription:NSLocalizedString(@"BACKUP_EXPORT_PHASE_CONFIGURATION",
@@ -479,12 +483,6 @@ NS_ASSUME_NONNULL_BEGIN
                                             @"Indicates that the database data is being exported.")
                                progress:nil];
 
-    YapDatabaseConnection *_Nullable dbConnection = self.primaryStorage.newDatabaseConnection;
-    if (!dbConnection) {
-        OWSFailDebug(@"Could not create dbConnection.");
-        return NO;
-    }
-
     OWSDBExportStream *exportStream = [[OWSDBExportStream alloc] initWithBackupIO:self.backupIO];
 
     __block BOOL aborted = NO;
@@ -548,7 +546,7 @@ NS_ASSUME_NONNULL_BEGIN
     __block NSUInteger copiedMigrations = 0;
     __block NSUInteger copiedMisc = 0;
     self.unsavedAttachmentExports = [NSMutableArray new];
-    [dbConnection readWithBlock:^(YapDatabaseReadTransaction *transaction) {
+    [self.dbConnection readWithBlock:^(YapDatabaseReadTransaction *transaction) {
         copiedThreads = exportEntities(transaction,
             [TSThread collection],
             [TSThread class],
@@ -570,10 +568,9 @@ NS_ASSUME_NONNULL_BEGIN
                 }
                 TSAttachmentStream *attachmentStream = object;
                 NSString *_Nullable filePath = attachmentStream.originalFilePath;
-                if (!filePath) {
-                    OWSLogError(@"attachment is missing file.");
+                if (!filePath || ![NSFileManager.defaultManager fileExistsAtPath:filePath]) {
+                    OWSFailDebug(@"attachment is missing file.");
                     return NO;
-                    OWSAssertDebug(attachmentStream.uniqueId.length > 0);
                 }
 
                 // OWSAttachmentExport is used to lazily write an encrypted copy of the
@@ -866,13 +863,15 @@ NS_ASSUME_NONNULL_BEGIN
             [self.savedAttachmentItems addObject:exportItem];
 
             // Immediately save the record metadata to facilitate export resume.
-            OWSBackupFragment *backupFragment = [OWSBackupFragment new];
+            OWSBackupFragment *backupFragment = [[OWSBackupFragment alloc] initWithUniqueId:recordName];
             backupFragment.recordName = recordName;
             backupFragment.encryptionKey = exportItem.encryptedItem.encryptionKey;
             backupFragment.relativeFilePath = attachmentExport.relativeFilePath;
             backupFragment.attachmentId = attachmentExport.attachmentId;
             backupFragment.uncompressedDataLength = exportItem.uncompressedDataLength;
-            [backupFragment save];
+            [self.dbConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) {
+                [backupFragment saveWithTransaction:transaction];
+            }];
 
             OWSLogVerbose(
                 @"saved attachment: %@ as %@", attachmentExport.attachmentFilePath, attachmentExport.relativeFilePath);
@@ -1064,9 +1063,11 @@ NS_ASSUME_NONNULL_BEGIN
     // After every successful backup export, we can (and should) cull metadata
     // for any backup fragment (i.e. CloudKit record) that wasn't involved in
     // the latest backup export.
-    [self.primaryStorage.newDatabaseConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) {
+    [self.dbConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) {
+        NSArray<NSString *> *allRecordNames = [transaction allKeysInCollection:[OWSBackupFragment collection]];
+
         NSMutableSet<NSString *> *obsoleteRecordNames = [NSMutableSet new];
-        [obsoleteRecordNames addObjectsFromArray:[transaction allKeysInCollection:[OWSBackupFragment collection]]];
+        [obsoleteRecordNames addObjectsFromArray:allRecordNames];
         [obsoleteRecordNames minusSet:activeRecordNames];
         
         [transaction removeObjectsForKeys:obsoleteRecordNames.allObjects inCollection:[OWSBackupFragment collection]];

Signal/src/util/Backup/OWSBackupIO.m

@@ -76,6 +76,10 @@ static const compression_algorithm SignalCompressionAlgorithm = COMPRESSION_LZMA
     OWSAssertDebug(encryptionKey.length > 0);
 
     @autoreleasepool {
+        if (![[NSFileManager defaultManager] fileExistsAtPath:srcFilePath]) {
+            OWSFailDebug(@"Missing source file.");
+            return nil;
+        }
 
         // TODO: Encrypt the file without loading it into memory.
         NSData *_Nullable srcData = [NSData dataWithContentsOfFile:srcFilePath];

Signal/src/util/Backup/OWSBackupImportJob.m

@@ -29,6 +29,8 @@ NSString *const kOWSBackup_ImportDatabaseKeySpec = @"kOWSBackup_ImportDatabaseKe
 
 @property (nonatomic) OWSBackupManifestContents *manifest;
 
+@property (nonatomic, nullable) YapDatabaseConnection *dbConnection;
+
 @end
 
 #pragma mark -
@@ -92,6 +94,8 @@ NSString *const kOWSBackup_ImportDatabaseKeySpec = @"kOWSBackup_ImportDatabaseKe
 
     self.backgroundTask = [OWSBackgroundTask backgroundTaskWithLabelStr:__PRETTY_FUNCTION__];
 
+    self.dbConnection = self.primaryStorage.newDatabaseConnection;
+
     [self updateProgressWithDescription:nil progress:nil];
 
     [[self.backup ensureCloudKitAccess]
@@ -150,7 +154,7 @@ NSString *const kOWSBackup_ImportDatabaseKeySpec = @"kOWSBackup_ImportDatabaseKe
     [allItems addObjectsFromArray:self.attachmentsItems];
 
     // Record metadata for all items, so that we can re-use them in incremental backups after the restore.
-    [self.primaryStorage.newDatabaseConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) {
+    [self.dbConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) {
         for (OWSBackupFragment *item in allItems) {
             [item saveWithTransaction:transaction];
         }
@@ -171,7 +175,7 @@ NSString *const kOWSBackup_ImportDatabaseKeySpec = @"kOWSBackup_ImportDatabaseKe
         })
         .then(^{
             // Kick off lazy restore on main thread.
-            [self.backupLazyRestore runIfNecessary];
+            [self.backupLazyRestore clearCompleteAndRunIfNecessary];
 
             [self.profileManager fetchLocalUsersProfile];
 
@@ -324,8 +328,7 @@ NSString *const kOWSBackup_ImportDatabaseKeySpec = @"kOWSBackup_ImportDatabaseKe
     }
 
     __block NSUInteger count = 0;
-    YapDatabaseConnection *dbConnection = self.primaryStorage.newDatabaseConnection;
-    [dbConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) {
+    [self.dbConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) {
         for (OWSBackupFragment *item in self.attachmentsItems) {
             if (self.isComplete) {
                 return;
@@ -379,12 +382,6 @@ NSString *const kOWSBackup_ImportDatabaseKeySpec = @"kOWSBackup_ImportDatabaseKe
         return [AnyPromise promiseWithValue:OWSBackupErrorWithDescription(@"Backup import no longer active.")];
     }
 
-    YapDatabaseConnection *_Nullable dbConnection = self.primaryStorage.newDatabaseConnection;
-    if (!dbConnection) {
-        OWSFailDebug(@"Could not create dbConnection.");
-        return [AnyPromise promiseWithValue:OWSBackupErrorWithDescription(@"Could not create dbConnection.")];
-    }
-
     // Order matters here.
     NSArray<NSString *> *collectionsToRestore = @[
         [TSThread collection],
@@ -397,7 +394,7 @@ NSString *const kOWSBackup_ImportDatabaseKeySpec = @"kOWSBackup_ImportDatabaseKe
     NSMutableDictionary<NSString *, NSNumber *> *restoredEntityCounts = [NSMutableDictionary new];
     __block unsigned long long copiedEntities = 0;
     __block BOOL aborted = NO;
-    [dbConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) {
+    [self.dbConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) {
         for (NSString *collection in collectionsToRestore) {
             if ([collection isEqualToString:[OWSDatabaseMigration collection]]) {
                 // It's okay if there are existing migrations; we'll clear those

Signal/src/util/Backup/OWSBackupJob.m

@@ -300,7 +300,7 @@ NSString *const kOWSBackup_KeychainService = @"kOWSBackup_KeychainService";
             return nil;
         }
 
-        OWSBackupFragment *item = [OWSBackupFragment new];
+        OWSBackupFragment *item = [[OWSBackupFragment alloc] initWithUniqueId:recordName];
         item.recordName = recordName;
         item.encryptionKey = encryptionKey;
         item.relativeFilePath = relativeFilePath;

Signal/src/util/Backup/OWSBackupLazyRestore.swift

@@ -53,6 +53,15 @@ public class BackupLazyRestore: NSObject {
 
     private let backgroundQueue = DispatchQueue.global(qos: .background)
 
+    @objc
+    public func clearCompleteAndRunIfNecessary() {
+        AssertIsOnMainThread()
+
+        isComplete = false
+
+        runIfNecessary()
+    }
+
     @objc
     public func runIfNecessary() {
         AssertIsOnMainThread()

SignalServiceKit/src/Messages/Attachments/OWSAttachmentDownloads.m

@@ -186,7 +186,6 @@ typedef void (^AttachmentDownloadFailure)(NSError *error);
                                 failure:(void (^)(NSError *error))failureHandler
 {
     OWSAssertDebug(attachmentStreamsParam);
-    OWSAssertDebug(attachmentPointers.count > 0);
 
     // To avoid deadlocks, synchronize on self outside of the transaction.
     dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{

SignalServiceKit/src/Messages/Attachments/TSAttachmentPointer.m

@@ -183,7 +183,10 @@ NS_ASSUME_NONNULL_BEGIN
     if (!self.lazyRestoreFragmentId) {
         return nil;
     }
-    return [OWSBackupFragment fetchObjectWithUniqueID:self.lazyRestoreFragmentId];
+    OWSBackupFragment *_Nullable backupFragment =
+        [OWSBackupFragment fetchObjectWithUniqueID:self.lazyRestoreFragmentId];
+    OWSAssertDebug(backupFragment);
+    return backupFragment;
 }
 
 #pragma mark - Update With... Methods

SignalServiceKit/src/Util/OWSBackupFragment.h

@@ -37,6 +37,8 @@ NS_ASSUME_NONNULL_BEGIN
 // This property is only set if the manifest item is compressed.
 @property (nonatomic, nullable) NSNumber *uncompressedDataLength;
 
+- (instancetype)init NS_UNAVAILABLE;
+
 @end
 
 NS_ASSUME_NONNULL_END

SignalServiceKit/src/Util/OWSBackupFragment.m

@@ -8,16 +8,6 @@ NS_ASSUME_NONNULL_BEGIN
 
 @implementation OWSBackupFragment
 
-- (void)saveWithTransaction:(YapDatabaseReadWriteTransaction *)transaction
-{
-    OWSAssertDebug(self.recordName.length > 0);
-
-    if (!self.uniqueId) {
-        self.uniqueId = self.recordName;
-    }
-    [super saveWithTransaction:transaction];
-}
-
 @end
 
 NS_ASSUME_NONNULL_END