From 86aae78f1bddf76dcbd5e4cd21911bfc10fbfcbe Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Tue, 13 Mar 2018 09:44:49 -0300 Subject: [PATCH 01/19] Include migrations in backup. --- Signal/src/util/OWSBackupExportJob.m | 22 ++++++++++++ Signal/src/util/OWSBackupImportJob.m | 53 +++++++++++++++++++++++++++- 2 files changed, 74 insertions(+), 1 deletion(-) diff --git a/Signal/src/util/OWSBackupExportJob.m b/Signal/src/util/OWSBackupExportJob.m index 1f7c2d67a..e38959158 100644 --- a/Signal/src/util/OWSBackupExportJob.m +++ b/Signal/src/util/OWSBackupExportJob.m @@ -3,6 +3,7 @@ // #import "OWSBackupExportJob.h" +#import "OWSDatabaseMigration.h" #import "Signal-Swift.h" #import #import @@ -240,6 +241,7 @@ NSString *const kOWSBackup_ExportDatabaseKeySpec = @"kOWSBackup_ExportDatabaseKe __block unsigned long long copiedInteractions = 0; __block unsigned long long copiedEntities = 0; __block unsigned long long copiedAttachments = 0; + __block unsigned long long copiedMigrations = 0; self.attachmentFilePathMap = [NSMutableDictionary new]; @@ -322,6 +324,25 @@ NSString *const kOWSBackup_ExportDatabaseKeySpec = @"kOWSBackup_ExportDatabaseKe copiedInteractions++; copiedEntities++; }]; + + // Copy migrations. + [srcTransaction + enumerateKeysAndObjectsInCollection:[OWSDatabaseMigration collection] + usingBlock:^(NSString *key, id object, BOOL *stop) { + if (self.isComplete) { + *stop = YES; + return; + } + if (![object isKindOfClass:[OWSDatabaseMigration class]]) { + OWSProdLogAndFail( + @"%@ unexpected class: %@", self.logTag, [object class]); + return; + } + OWSDatabaseMigration *migration = object; + [migration saveWithTransaction:dstTransaction]; + copiedMigrations++; + copiedEntities++; + }]; }]; }]; @@ -331,6 +352,7 @@ NSString *const kOWSBackup_ExportDatabaseKeySpec = @"kOWSBackup_ExportDatabaseKe DDLogInfo(@"%@ copiedMessages: %llu", self.logTag, copiedInteractions); DDLogInfo(@"%@ copiedEntities: %llu", self.logTag, copiedEntities); DDLogInfo(@"%@ copiedAttachments: %llu", self.logTag, copiedAttachments); + DDLogInfo(@"%@ copiedMigrations: %llu", self.logTag, copiedMigrations); [self.backupStorage logFileSizes]; diff --git a/Signal/src/util/OWSBackupImportJob.m b/Signal/src/util/OWSBackupImportJob.m index c4c431091..387e26f64 100644 --- a/Signal/src/util/OWSBackupImportJob.m +++ b/Signal/src/util/OWSBackupImportJob.m @@ -3,6 +3,8 @@ // #import "OWSBackupImportJob.h" +#import "OWSDatabaseMigration.h" +#import "OWSDatabaseMigrationRunner.h" #import "Signal-Swift.h" #import #import @@ -124,7 +126,21 @@ NSString *const kOWSBackup_ImportDatabaseKeySpec = @"kOWSBackup_ImportDatabaseKe return; } - [weakSelf succeed]; + [weakSelf ensureMigrations:^(BOOL ensureMigrationsSuccess) { + if (!ensureMigrationsSuccess) { + [weakSelf failWithErrorDescription:NSLocalizedString( + @"BACKUP_IMPORT_ERROR_COULD_NOT_IMPORT", + @"Error indicating the a backup import " + @"could not import the user's data.")]; + return; + } + + if (weakSelf.isComplete) { + return; + } + + [weakSelf succeed]; + }]; }]; }]; }]; @@ -375,6 +391,7 @@ NSString *const kOWSBackup_ImportDatabaseKeySpec = @"kOWSBackup_ImportDatabaseKe __block unsigned long long copiedInteractions = 0; __block unsigned long long copiedEntities = 0; __block unsigned long long copiedAttachments = 0; + __block unsigned long long copiedMigrations = 0; [tempDBConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *srcTransaction) { [primaryDBConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *dstTransaction) { @@ -447,6 +464,28 @@ NSString *const kOWSBackup_ImportDatabaseKeySpec = @"kOWSBackup_ImportDatabaseKe copiedInteractions++; copiedEntities++; }]; + + // Clear existing migrations. + [dstTransaction removeAllObjectsInCollection:[OWSDatabaseMigration collection]]; + + // Copy migrations. + [srcTransaction + enumerateKeysAndObjectsInCollection:[OWSDatabaseMigration collection] + usingBlock:^(NSString *key, id object, BOOL *stop) { + if (self.isComplete) { + *stop = YES; + return; + } + if (![object isKindOfClass:[OWSDatabaseMigration class]]) { + OWSProdLogAndFail( + @"%@ unexpected class: %@", self.logTag, [object class]); + return; + } + OWSDatabaseMigration *migration = object; + [migration saveWithTransaction:dstTransaction]; + copiedMigrations++; + copiedEntities++; + }]; }]; }]; @@ -454,6 +493,7 @@ NSString *const kOWSBackup_ImportDatabaseKeySpec = @"kOWSBackup_ImportDatabaseKe DDLogInfo(@"%@ copiedMessages: %llu", self.logTag, copiedInteractions); DDLogInfo(@"%@ copiedEntities: %llu", self.logTag, copiedEntities); DDLogInfo(@"%@ copiedAttachments: %llu", self.logTag, copiedAttachments); + DDLogInfo(@"%@ copiedMigrations: %llu", self.logTag, copiedMigrations); [backupStorage logFileSizes]; @@ -464,6 +504,17 @@ NSString *const kOWSBackup_ImportDatabaseKeySpec = @"kOWSBackup_ImportDatabaseKe return completion(YES); } +- (void)ensureMigrations:(OWSBackupJobBoolCompletion)completion +{ + OWSAssert(completion); + + DDLogVerbose(@"%@ %s", self.logTag, __PRETTY_FUNCTION__); + + [[[OWSDatabaseMigrationRunner alloc] initWithPrimaryStorage:self.primaryStorage] runAllOutstandingWithCompletion:^{ + completion(YES); + }]; +} + - (BOOL)restoreFileWithRecordName:(NSString *)recordName dstRelativePath:(NSString *)dstRelativePath dstDirPath:(NSString *)dstDirPath From d2f2dd273ae3cac63d508607b14897c5a8668166 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Tue, 13 Mar 2018 10:51:57 -0300 Subject: [PATCH 02/19] Fix edge cases in migrations. --- .../ViewControllers/DebugUI/DebugUIBackup.m | 21 ++- Signal/src/util/OWSBackupExportJob.m | 3 +- Signal/src/util/OWSBackupImportJob.m | 177 ++++++++---------- Signal/src/util/OWSBackupJob.m | 7 + .../OWS106EnsureProfileComplete.swift | 5 - .../migrations/OWSDatabaseMigrationRunner.m | 54 +++--- SignalServiceKit/src/Storage/OWSStorage.m | 2 +- 7 files changed, 132 insertions(+), 137 deletions(-) diff --git a/Signal/src/ViewControllers/DebugUI/DebugUIBackup.m b/Signal/src/ViewControllers/DebugUI/DebugUIBackup.m index e55826d4e..bad3604d4 100644 --- a/Signal/src/ViewControllers/DebugUI/DebugUIBackup.m +++ b/Signal/src/ViewControllers/DebugUI/DebugUIBackup.m @@ -90,7 +90,26 @@ NS_ASSUME_NONNULL_BEGIN { DDLogInfo(@"%@ tryToImportBackup.", self.logTag); - [OWSBackup.sharedManager tryToImportBackup]; + UIAlertController *controller = + [UIAlertController alertControllerWithTitle:@"Restore CloudKit Backup" + message:@"This will delete all of your database contents." + preferredStyle:UIAlertControllerStyleAlert]; + + [controller addAction:[UIAlertAction + actionWithTitle:@"Restore" + style:UIAlertActionStyleDefault + handler:^(UIAlertAction *_Nonnull action) { + [[OWSPrimaryStorage.sharedManager newDatabaseConnection] + readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { + [transaction removeAllObjectsInCollection:[TSThread collection]]; + [transaction removeAllObjectsInCollection:[TSInteraction collection]]; + [transaction removeAllObjectsInCollection:[TSAttachment collection]]; + }]; + [OWSBackup.sharedManager tryToImportBackup]; + }]]; + [controller addAction:[OWSAlerts cancelAction]]; + UIViewController *fromViewController = [[UIApplication sharedApplication] frontmostViewController]; + [fromViewController presentViewController:controller animated:YES completion:nil]; } @end diff --git a/Signal/src/util/OWSBackupExportJob.m b/Signal/src/util/OWSBackupExportJob.m index e38959158..898f7daab 100644 --- a/Signal/src/util/OWSBackupExportJob.m +++ b/Signal/src/util/OWSBackupExportJob.m @@ -11,6 +11,7 @@ #import #import #import +#import #import #import #import @@ -268,7 +269,7 @@ NSString *const kOWSBackup_ExportDatabaseKeySpec = @"kOWSBackup_ExportDatabaseKe // Copy attachments. [srcTransaction - enumerateKeysAndObjectsInCollection:[TSAttachmentStream collection] + enumerateKeysAndObjectsInCollection:[TSAttachment collection] usingBlock:^(NSString *key, id object, BOOL *stop) { if (self.isComplete) { *stop = YES; diff --git a/Signal/src/util/OWSBackupImportJob.m b/Signal/src/util/OWSBackupImportJob.m index 387e26f64..8e8be2524 100644 --- a/Signal/src/util/OWSBackupImportJob.m +++ b/Signal/src/util/OWSBackupImportJob.m @@ -359,7 +359,6 @@ NSString *const kOWSBackup_ImportDatabaseKeySpec = @"kOWSBackup_ImportDatabaseKe [backupStorage runAsyncRegistrationsWithCompletion:^{ dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{ [weakSelf restoreDatabaseContents:backupStorage completion:completion]; - completion(YES); }); }]; }); @@ -387,113 +386,85 @@ NSString *const kOWSBackup_ImportDatabaseKeySpec = @"kOWSBackup_ImportDatabaseKe return completion(NO); } - __block unsigned long long copiedThreads = 0; - __block unsigned long long copiedInteractions = 0; + NSDictionary *collectionTypeMap = @{ + [TSThread collection] : [TSThread class], + [TSAttachment collection] : [TSAttachment class], + [TSInteraction collection] : [TSInteraction class], + [OWSDatabaseMigration collection] : [OWSDatabaseMigration class], + }; + // Order matters here. + NSArray *collectionsToRestore = @[ + [TSThread collection], + [TSAttachment collection], + // Interactions refer to threads and attachments, + // so copy them afterward. + [TSInteraction collection], + [OWSDatabaseMigration collection], + ]; + NSMutableDictionary *restoredEntityCounts = [NSMutableDictionary new]; __block unsigned long long copiedEntities = 0; - __block unsigned long long copiedAttachments = 0; - __block unsigned long long copiedMigrations = 0; - + __block BOOL aborted = NO; [tempDBConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *srcTransaction) { [primaryDBConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *dstTransaction) { - // Copy threads. - [srcTransaction - enumerateKeysAndObjectsInCollection:[TSThread collection] - usingBlock:^(NSString *key, id object, BOOL *stop) { - if (self.isComplete) { - *stop = YES; - return; - } - if (![object isKindOfClass:[TSThread class]]) { - OWSProdLogAndFail( - @"%@ unexpected class: %@", self.logTag, [object class]); - return; - } - TSThread *thread = object; - [thread saveWithTransaction:dstTransaction]; - copiedThreads++; - copiedEntities++; - }]; - - // Copy attachments. - [srcTransaction - enumerateKeysAndObjectsInCollection:[TSAttachmentStream collection] - usingBlock:^(NSString *key, id object, BOOL *stop) { - if (self.isComplete) { - *stop = YES; - return; - } - if (![object isKindOfClass:[TSAttachment class]]) { - OWSProdLogAndFail( - @"%@ unexpected class: %@", self.logTag, [object class]); - return; - } - TSAttachment *attachment = object; - [attachment saveWithTransaction:dstTransaction]; - copiedAttachments++; - copiedEntities++; - }]; - - // Copy interactions. - // - // Interactions refer to threads and attachments, so copy the last. - [srcTransaction - enumerateKeysAndObjectsInCollection:[TSInteraction collection] - usingBlock:^(NSString *key, id object, BOOL *stop) { - if (self.isComplete) { - *stop = YES; - return; - } - if (![object isKindOfClass:[TSInteraction class]]) { - OWSProdLogAndFail( - @"%@ unexpected class: %@", self.logTag, [object class]); - return; - } - // Ignore disappearing messages. - if ([object isKindOfClass:[TSMessage class]]) { - TSMessage *message = object; - if (message.isExpiringMessage) { - return; - } - } - TSInteraction *interaction = object; - // Ignore dynamic interactions. - if (interaction.isDynamicInteraction) { - return; - } - [interaction saveWithTransaction:dstTransaction]; - copiedInteractions++; - copiedEntities++; - }]; + for (NSString *collection in collectionsToRestore) { + if ([collection isEqualToString:[OWSDatabaseMigration collection]]) { + // It's okay if there are existing migrations; we'll clear those + // before restoring. + continue; + } + if ([dstTransaction numberOfKeysInCollection:collection] > 0) { + DDLogError(@"%@ cannot restore into non-empty database (%@).", self.logTag, collection); + aborted = YES; + return completion(NO); + } + } // Clear existing migrations. + // + // This is safe since we only ever import into an empty database. + // Non-database migrations should be idempotent. [dstTransaction removeAllObjectsInCollection:[OWSDatabaseMigration collection]]; - // Copy migrations. - [srcTransaction - enumerateKeysAndObjectsInCollection:[OWSDatabaseMigration collection] - usingBlock:^(NSString *key, id object, BOOL *stop) { - if (self.isComplete) { - *stop = YES; - return; - } - if (![object isKindOfClass:[OWSDatabaseMigration class]]) { - OWSProdLogAndFail( - @"%@ unexpected class: %@", self.logTag, [object class]); - return; - } - OWSDatabaseMigration *migration = object; - [migration saveWithTransaction:dstTransaction]; - copiedMigrations++; - copiedEntities++; - }]; + // Copy database entities. + for (NSString *collection in collectionsToRestore) { + [srcTransaction enumerateKeysAndObjectsInCollection:collection + usingBlock:^(NSString *key, id object, BOOL *stop) { + if (self.isComplete) { + *stop = YES; + aborted = YES; + return; + } + Class expectedType = collectionTypeMap[collection]; + OWSAssert(expectedType); + if (![object isKindOfClass:expectedType]) { + OWSProdLogAndFail(@"%@ unexpected class: %@ != %@", + self.logTag, + [object class], + expectedType); + return; + } + TSYapDatabaseObject *databaseObject = object; + [databaseObject saveWithTransaction:dstTransaction]; + + NSUInteger count + = restoredEntityCounts[collection].unsignedIntValue; + restoredEntityCounts[collection] = @(count + 1); + copiedEntities++; + }]; + } }]; }]; - DDLogInfo(@"%@ copiedThreads: %llu", self.logTag, copiedThreads); - DDLogInfo(@"%@ copiedMessages: %llu", self.logTag, copiedInteractions); + if (aborted) { + return; + } + + for (NSString *collection in collectionsToRestore) { + Class expectedType = collectionTypeMap[collection]; + OWSAssert(expectedType); + DDLogInfo(@"%@ copied %@ (%@): %@", self.logTag, expectedType, collection, restoredEntityCounts[collection]); + } DDLogInfo(@"%@ copiedEntities: %llu", self.logTag, copiedEntities); - DDLogInfo(@"%@ copiedAttachments: %llu", self.logTag, copiedAttachments); - DDLogInfo(@"%@ copiedMigrations: %llu", self.logTag, copiedMigrations); [backupStorage logFileSizes]; @@ -510,9 +481,15 @@ NSString *const kOWSBackup_ImportDatabaseKeySpec = @"kOWSBackup_ImportDatabaseKe DDLogVerbose(@"%@ %s", self.logTag, __PRETTY_FUNCTION__); - [[[OWSDatabaseMigrationRunner alloc] initWithPrimaryStorage:self.primaryStorage] runAllOutstandingWithCompletion:^{ - completion(YES); - }]; + // It's okay that we do this in a separate transaction from the + // restoration of backup contents. If some of migrations don't + // complete, they'll be run the next time the app launches. + dispatch_async(dispatch_get_main_queue(), ^{ + [[[OWSDatabaseMigrationRunner alloc] initWithPrimaryStorage:self.primaryStorage] + runAllOutstandingWithCompletion:^{ + completion(YES); + }]; + }); } - (BOOL)restoreFileWithRecordName:(NSString *)recordName diff --git a/Signal/src/util/OWSBackupJob.m b/Signal/src/util/OWSBackupJob.m index 258e3db49..6c1d7697f 100644 --- a/Signal/src/util/OWSBackupJob.m +++ b/Signal/src/util/OWSBackupJob.m @@ -21,6 +21,7 @@ NSString *const kOWSBackup_KeychainService = @"kOWSBackup_KeychainService"; @property (nonatomic, weak) id delegate; @property (atomic) BOOL isComplete; +@property (atomic) BOOL hasSucceeded; @property (nonatomic) OWSPrimaryStorage *primaryStorage; @@ -96,6 +97,12 @@ NSString *const kOWSBackup_KeychainService = @"kOWSBackup_KeychainService"; return; } self.isComplete = YES; + + // There's a lot of asynchrony in these backup jobs; + // ensure we only end up finishing these jobs once. + OWSAssert(!self.hasSucceeded); + self.hasSucceeded = YES; + [self.delegate backupJobDidSucceed:self]; }); } diff --git a/SignalMessaging/environment/migrations/OWS106EnsureProfileComplete.swift b/SignalMessaging/environment/migrations/OWS106EnsureProfileComplete.swift index dbeac5ae7..d01092f12 100644 --- a/SignalMessaging/environment/migrations/OWS106EnsureProfileComplete.swift +++ b/SignalMessaging/environment/migrations/OWS106EnsureProfileComplete.swift @@ -21,11 +21,6 @@ public class OWS106EnsureProfileComplete: OWSDatabaseMigration { // Overriding runUp since we have some specific completion criteria which // is more likely to fail since it involves network requests. override public func runUp(completion:@escaping ((Void)) -> Void) { - guard type(of: self).sharedCompleteRegistrationFixerJob == nil else { - owsFail("\(self.TAG) should only be called once.") - return - } - let job = CompleteRegistrationFixerJob(completionHandler: { Logger.info("\(self.TAG) Completed. Saving.") self.save() diff --git a/SignalMessaging/environment/migrations/OWSDatabaseMigrationRunner.m b/SignalMessaging/environment/migrations/OWSDatabaseMigrationRunner.m index 1456c0d2e..4a0d3e7a9 100644 --- a/SignalMessaging/environment/migrations/OWSDatabaseMigrationRunner.m +++ b/SignalMessaging/environment/migrations/OWSDatabaseMigrationRunner.m @@ -56,52 +56,48 @@ NS_ASSUME_NONNULL_BEGIN - (void)runAllOutstandingWithCompletion:(OWSDatabaseMigrationCompletion)completion { - [self runMigrations:self.allMigrations completion:completion]; + [self runMigrations:[self.allMigrations mutableCopy] completion:completion]; } -- (void)runMigrations:(NSArray *)migrations +// Run migrations serially to: +// +// * Ensure predictable ordering. +// * Prevent them from interfering with each other (e.g. deadlock). +- (void)runMigrations:(NSMutableArray *)migrations completion:(OWSDatabaseMigrationCompletion)completion { OWSAssert(migrations); OWSAssert(completion); - NSMutableArray *migrationsToRun = [NSMutableArray new]; + // TODO: Remove. + DDLogInfo(@"%@ Considering migrations: %zd", self.logTag, migrations.count); for (OWSDatabaseMigration *migration in migrations) { - if ([OWSDatabaseMigration fetchObjectWithUniqueID:migration.uniqueId] == nil) { - [migrationsToRun addObject:migration]; - } + DDLogInfo(@"%@ Considering migrations: %@", self.logTag, migration.class); } - if (migrationsToRun.count < 1) { + // If there are no more migrations to run, complete. + if (migrations.count < 1) { dispatch_async(dispatch_get_main_queue(), ^{ completion(); }); return; } - NSUInteger totalMigrationCount = migrationsToRun.count; - __block NSUInteger completedMigrationCount = 0; - // Call the completion exactly once, when the last migration completes. - void (^checkMigrationCompletion)(void) = ^{ - @synchronized(self) - { - completedMigrationCount++; - if (completedMigrationCount == totalMigrationCount) { - dispatch_async(dispatch_get_main_queue(), ^{ - completion(); - }); - } - } - }; - - for (OWSDatabaseMigration *migration in migrationsToRun) { - if ([OWSDatabaseMigration fetchObjectWithUniqueID:migration.uniqueId]) { - DDLogDebug(@"%@ Skipping previously run migration: %@", self.logTag, migration); - } else { - DDLogWarn(@"%@ Running migration: %@", self.logTag, migration); - [migration runUpWithCompletion:checkMigrationCompletion]; - } + // Pop next migration from front of queue. + OWSDatabaseMigration *migration = migrations.firstObject; + [migrations removeObjectAtIndex:0]; + + // If migration has already been run, skip it. + if ([OWSDatabaseMigration fetchObjectWithUniqueID:migration.uniqueId] != nil) { + [self runMigrations:migrations completion:completion]; + return; } + + DDLogInfo(@"%@ Running migration: %@", self.logTag, migration); + [migration runUpWithCompletion:^{ + DDLogInfo(@"%@ Migration complete: %@", self.logTag, migration); + [self runMigrations:migrations completion:completion]; + }]; } @end diff --git a/SignalServiceKit/src/Storage/OWSStorage.m b/SignalServiceKit/src/Storage/OWSStorage.m index 0b84a0e78..de7ae50c3 100644 --- a/SignalServiceKit/src/Storage/OWSStorage.m +++ b/SignalServiceKit/src/Storage/OWSStorage.m @@ -74,7 +74,7 @@ typedef NSData *_Nullable (^CreateDatabaseMetadataBlock)(void); { id delegate = self.delegate; OWSAssert(delegate); - OWSAssert(delegate.areAllRegistrationsComplete || self.canWriteBeforeStorageReady); + // OWSAssert(delegate.areAllRegistrationsComplete || self.canWriteBeforeStorageReady); OWSBackgroundTask *_Nullable backgroundTask = nil; if (CurrentAppContext().isMainApp) { From 565743b66d6be5ad161af4c63171c96d2dee0cfb Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Tue, 13 Mar 2018 11:00:08 -0300 Subject: [PATCH 03/19] Fix edge cases in migrations. --- .../environment/migrations/OWSDatabaseMigrationRunner.m | 6 ------ 1 file changed, 6 deletions(-) diff --git a/SignalMessaging/environment/migrations/OWSDatabaseMigrationRunner.m b/SignalMessaging/environment/migrations/OWSDatabaseMigrationRunner.m index 4a0d3e7a9..d5537cb1c 100644 --- a/SignalMessaging/environment/migrations/OWSDatabaseMigrationRunner.m +++ b/SignalMessaging/environment/migrations/OWSDatabaseMigrationRunner.m @@ -69,12 +69,6 @@ NS_ASSUME_NONNULL_BEGIN OWSAssert(migrations); OWSAssert(completion); - // TODO: Remove. - DDLogInfo(@"%@ Considering migrations: %zd", self.logTag, migrations.count); - for (OWSDatabaseMigration *migration in migrations) { - DDLogInfo(@"%@ Considering migrations: %@", self.logTag, migration.class); - } - // If there are no more migrations to run, complete. if (migrations.count < 1) { dispatch_async(dispatch_get_main_queue(), ^{ From 669a3610ab505b5d1da524506d1966bbd720cbcd Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Tue, 13 Mar 2018 11:37:39 -0300 Subject: [PATCH 04/19] Fix attachments. --- .../AppSettings/AppSettingsViewController.m | 4 ++++ Signal/src/ViewControllers/HomeViewController.m | 4 ++++ Signal/src/util/OWSBackupExportJob.m | 6 ++++++ Signal/src/util/OWSBackupImportJob.m | 10 +++++++--- 4 files changed, 21 insertions(+), 3 deletions(-) diff --git a/Signal/src/ViewControllers/AppSettings/AppSettingsViewController.m b/Signal/src/ViewControllers/AppSettings/AppSettingsViewController.m index 26f94e5e9..e60226c28 100644 --- a/Signal/src/ViewControllers/AppSettings/AppSettingsViewController.m +++ b/Signal/src/ViewControllers/AppSettings/AppSettingsViewController.m @@ -90,6 +90,10 @@ self.title = NSLocalizedString(@"SETTINGS_NAV_BAR_TITLE", @"Title for settings activity"); [self updateTableContents]; + + dispatch_async(dispatch_get_main_queue(), ^{ + [self showBackup]; + }); } - (void)viewWillAppear:(BOOL)animated diff --git a/Signal/src/ViewControllers/HomeViewController.m b/Signal/src/ViewControllers/HomeViewController.m index dccd63e15..497a898c1 100644 --- a/Signal/src/ViewControllers/HomeViewController.m +++ b/Signal/src/ViewControllers/HomeViewController.m @@ -284,6 +284,10 @@ typedef NS_ENUM(NSInteger, CellState) { kArchiveState, kInboxState }; } [self updateBarButtonItems]; + + dispatch_async(dispatch_get_main_queue(), ^{ + [self settingsButtonPressed:nil]; + }); } - (void)viewDidAppear:(BOOL)animated diff --git a/Signal/src/util/OWSBackupExportJob.m b/Signal/src/util/OWSBackupExportJob.m index 898f7daab..f49039f32 100644 --- a/Signal/src/util/OWSBackupExportJob.m +++ b/Signal/src/util/OWSBackupExportJob.m @@ -483,6 +483,10 @@ NSString *const kOWSBackup_ExportDatabaseKeySpec = @"kOWSBackup_ExportDatabaseKe return; } strongSelf.attachmentRecordMap[recordName] = attachmentExport.relativeFilePath; + DDLogVerbose(@"%@ exported attachment: %@ as %@", + self.logTag, + attachmentFilePath, + attachmentExport.relativeFilePath); [strongSelf saveNextFileToCloud:completion]; }); } @@ -550,6 +554,8 @@ NSString *const kOWSBackup_ExportDatabaseKeySpec = @"kOWSBackup_ExportDatabaseKe kOWSBackup_ManifestKey_DatabaseKeySpec : databaseKeySpec.base64EncodedString, }; + DDLogVerbose(@"%@ json: %@", self.logTag, json); + NSError *error; NSData *_Nullable jsonData = [NSJSONSerialization dataWithJSONObject:json options:NSJSONWritingPrettyPrinted error:&error]; diff --git a/Signal/src/util/OWSBackupImportJob.m b/Signal/src/util/OWSBackupImportJob.m index 8e8be2524..7bf192ac3 100644 --- a/Signal/src/util/OWSBackupImportJob.m +++ b/Signal/src/util/OWSBackupImportJob.m @@ -203,6 +203,8 @@ NSString *const kOWSBackup_ImportDatabaseKeySpec = @"kOWSBackup_ImportDatabaseKe return completion(NO); } + DDLogVerbose(@"%@ json: %@", self.logTag, json); + NSDictionary *_Nullable databaseRecordMap = json[kOWSBackup_ManifestKey_DatabaseFiles]; NSDictionary *_Nullable attachmentRecordMap = json[kOWSBackup_ManifestKey_AttachmentFiles]; NSString *_Nullable databaseKeySpecBase64 = json[kOWSBackup_ManifestKey_DatabaseKeySpec]; @@ -294,7 +296,7 @@ NSString *const kOWSBackup_ImportDatabaseKeySpec = @"kOWSBackup_ImportDatabaseKe - (void)restoreAttachmentFiles { - DDLogVerbose(@"%@ %s", self.logTag, __PRETTY_FUNCTION__); + DDLogVerbose(@"%@ %s: %zd", self.logTag, __PRETTY_FUNCTION__, self.attachmentRecordMap.count); NSString *attachmentsDirPath = [TSAttachmentStream attachmentsFolder]; @@ -511,8 +513,8 @@ NSString *const kOWSBackup_ImportDatabaseKeySpec = @"kOWSBackup_ImportDatabaseKe } NSString *dstFilePath = [dstDirPath stringByAppendingPathComponent:dstRelativePath]; if ([NSFileManager.defaultManager fileExistsAtPath:dstFilePath]) { - DDLogError(@"%@ skipping redundant file restore.", self.logTag); - return NO; + DDLogError(@"%@ skipping redundant file restore: %@.", self.logTag, dstFilePath); + return YES; } NSString *downloadedFilePath = self.downloadedFileMap[recordName]; if (![NSFileManager.defaultManager fileExistsAtPath:downloadedFilePath]) { @@ -526,6 +528,8 @@ NSString *const kOWSBackup_ImportDatabaseKeySpec = @"kOWSBackup_ImportDatabaseKe return NO; } + DDLogError(@"%@ restored file: %@ (%@).", self.logTag, dstFilePath, dstRelativePath); + return YES; } From 91bf0bdb9fd5baf42fc21ebc3410d3942d4f80d3 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Tue, 13 Mar 2018 12:18:41 -0300 Subject: [PATCH 05/19] Sketch out backup export UI. --- .../OWSBackupSettingsViewController.m | 78 +++++++++++++++++++ Signal/src/util/OWSBackup.h | 5 ++ Signal/src/util/OWSBackup.m | 49 +++++++++++- .../translations/en.lproj/Localizable.strings | 30 +++++++ 4 files changed, 161 insertions(+), 1 deletion(-) diff --git a/Signal/src/ViewControllers/AppSettings/OWSBackupSettingsViewController.m b/Signal/src/ViewControllers/AppSettings/OWSBackupSettingsViewController.m index afe7a25e4..9651a72e8 100644 --- a/Signal/src/ViewControllers/AppSettings/OWSBackupSettingsViewController.m +++ b/Signal/src/ViewControllers/AppSettings/OWSBackupSettingsViewController.m @@ -70,9 +70,87 @@ NS_ASSUME_NONNULL_BEGIN selector:@selector(isBackupEnabledDidChange:)]]; [contents addSection:enableSection]; + if (isBackupEnabled) { + // TODO: This UI is temporary. + // Enabling backup will involve entering and registering a PIN. + OWSTableSection *progressSection = [OWSTableSection new]; + [progressSection + addItem:[OWSTableItem labelItemWithText:NSLocalizedString(@"SETTINGS_BACKUP_STATUS", + @"Label for status row in the in the backup settings view.") + accessoryText:[self backupExportStateLocalizedDescription]]]; + if (OWSBackup.sharedManager.backupExportState == OWSBackupState_InProgress) { + if (OWSBackup.sharedManager.backupExportDescription) { + [progressSection + addItem:[OWSTableItem + labelItemWithText:NSLocalizedString(@"SETTINGS_BACKUP_PHASE", + @"Label for phase row in the in the backup settings view.") + accessoryText:OWSBackup.sharedManager.backupExportDescription]]; + if (OWSBackup.sharedManager.backupExportProgress) { + NSUInteger progressPercent + = (NSUInteger)round(OWSBackup.sharedManager.backupExportProgress.floatValue * 100); + DDLogVerbose(@"%@ '%@', '%@'", + self.logTag, + @(progressPercent).stringValue, + [NSString + stringWithFormat:NSLocalizedString(@"PERCENTAGE_FORMAT", + @"Format for percentages, e.g. 65%. Embeds {{percentage}}, e.g. 65."), + @(progressPercent).stringValue]); + [progressSection + addItem:[OWSTableItem + labelItemWithText:NSLocalizedString(@"SETTINGS_BACKUP_PROGRESS", + @"Label for phase row in the in the backup settings view.") + accessoryText:[NSString + stringWithFormat:NSLocalizedString(@"PERCENTAGE_FORMAT", + @"Format for percentages, e.g. 65%. " + @"Embeds {{percentage}}, e.g. 65."), + @(progressPercent).stringValue]]]; + } + } + } + + switch (OWSBackup.sharedManager.backupExportState) { + case OWSBackupState_Idle: + case OWSBackupState_Failed: + case OWSBackupState_Succeeded: + [progressSection + addItem:[OWSTableItem disclosureItemWithText: + NSLocalizedString(@"SETTINGS_BACKUP_BACKUP_NOW", + @"Label for 'backup now' button in the backup settings view.") + actionBlock:^{ + [OWSBackup.sharedManager tryToExportBackup]; + }]]; + break; + case OWSBackupState_InProgress: + [progressSection + addItem:[OWSTableItem disclosureItemWithText: + NSLocalizedString(@"SETTINGS_BACKUP_CANCEL_BACKUP", + @"Label for 'cancel backup' button in the backup settings view.") + actionBlock:^{ + [OWSBackup.sharedManager cancelExportBackup]; + }]]; + break; + } + + [contents addSection:progressSection]; + } + self.contents = contents; } +- (NSString *)backupExportStateLocalizedDescription +{ + switch (OWSBackup.sharedManager.backupExportState) { + case OWSBackupState_Idle: + return NSLocalizedString(@"SETTINGS_BACKUP_STATUS_IDLE", @"Indicates that app is not backing up."); + case OWSBackupState_InProgress: + return NSLocalizedString(@"SETTINGS_BACKUP_STATUS_IN_PROGRESS", @"Indicates that app is backing up."); + case OWSBackupState_Failed: + return NSLocalizedString(@"SETTINGS_BACKUP_STATUS_FAILED", @"Indicates that the last backup failed."); + case OWSBackupState_Succeeded: + return NSLocalizedString(@"SETTINGS_BACKUP_STATUS_SUCCEEDED", @"Indicates that the last backup succeeded."); + } +} + - (void)isBackupEnabledDidChange:(UISwitch *)sender { [OWSBackup.sharedManager setIsBackupEnabled:sender.isOn]; diff --git a/Signal/src/util/OWSBackup.h b/Signal/src/util/OWSBackup.h index b1f0bf056..5327f78d8 100644 --- a/Signal/src/util/OWSBackup.h +++ b/Signal/src/util/OWSBackup.h @@ -33,6 +33,7 @@ typedef NS_ENUM(NSUInteger, OWSBackupState) { #pragma mark - Backup Export @property (nonatomic, readonly) OWSBackupState backupExportState; + // If a "backup export" is in progress (see backupExportState), // backupExportDescription _might_ contain a string that describes // the current phase and backupExportProgress _might_ contain a @@ -44,9 +45,13 @@ typedef NS_ENUM(NSUInteger, OWSBackupState) { - (BOOL)isBackupEnabled; - (void)setIsBackupEnabled:(BOOL)value; +- (void)tryToExportBackup; +- (void)cancelExportBackup; + #pragma mark - Backup Import @property (nonatomic, readonly) OWSBackupState backupImportState; + // If a "backup import" is in progress (see backupImportState), // backupImportDescription _might_ contain a string that describes // the current phase and backupImportProgress _might_ contain a diff --git a/Signal/src/util/OWSBackup.m b/Signal/src/util/OWSBackup.m index 645807a16..e41c6978e 100644 --- a/Signal/src/util/OWSBackup.m +++ b/Signal/src/util/OWSBackup.m @@ -134,6 +134,41 @@ NS_ASSUME_NONNULL_BEGIN #pragma mark - Backup Export +- (void)tryToExportBackup +{ + OWSAssertIsOnMainThread(); + OWSAssert(!self.backupExportJob); + + if (!self.canBackupExport) { + // TODO: Offer a reason in the UI. + return; + } + + // In development, make sure there's no export or import in progress. + [self.backupExportJob cancel]; + self.backupExportJob = nil; + [self.backupImportJob cancel]; + self.backupImportJob = nil; + + _backupExportState = OWSBackupState_InProgress; + + self.backupExportJob = + [[OWSBackupExportJob alloc] initWithDelegate:self primaryStorage:[OWSPrimaryStorage sharedManager]]; + [self.backupExportJob startAsync]; + + [self postDidChangeNotification]; +} + +- (void)cancelExportBackup +{ + [self.backupExportJob cancel]; + self.backupExportJob = nil; + + _backupExportState = OWSBackupState_Idle; + + [self postDidChangeNotification]; +} + - (void)setLastExportSuccessDate:(NSDate *)value { OWSAssert(value); @@ -190,7 +225,7 @@ NS_ASSUME_NONNULL_BEGIN [self ensureBackupExportState]; } -- (BOOL)shouldHaveBackupExport +- (BOOL)canBackupExport { if (!self.isBackupEnabled) { return NO; @@ -202,6 +237,18 @@ NS_ASSUME_NONNULL_BEGIN if (![TSAccountManager isRegistered]) { return NO; } + return YES; +} + +- (BOOL)shouldHaveBackupExport +{ + if (!self.canBackupExport) { + return NO; + } + if (self.backupExportJob) { + // If there's already a job in progress, let it complete. + return YES; + } NSDate *_Nullable lastExportSuccessDate = self.lastExportSuccessDate; NSDate *_Nullable lastExportFailureDate = self.lastExportFailureDate; // Wait N hours before retrying after a success. diff --git a/Signal/translations/en.lproj/Localizable.strings b/Signal/translations/en.lproj/Localizable.strings index dbe987f29..aeedab255 100644 --- a/Signal/translations/en.lproj/Localizable.strings +++ b/Signal/translations/en.lproj/Localizable.strings @@ -1213,6 +1213,9 @@ /* A display format for oversize text messages. */ "OVERSIZE_TEXT_DISPLAY_FORMAT" = "%@…"; +/* Format for percentages, e.g. 65%. Embeds {{percentage}}, e.g. 65. */ +"PERCENTAGE_FORMAT" = "%@%%"; + /* A format for a label showing an example phone number. Embeds {{the example phone number}}. */ "PHONE_NUMBER_EXAMPLE_FORMAT" = "Example: %@"; @@ -1585,9 +1588,36 @@ /* Label for the backup view in app settings. */ "SETTINGS_BACKUP" = "Backup"; +/* Label for 'backup now' button in the backup settings view. */ +"SETTINGS_BACKUP_BACKUP_NOW" = "Backup Now"; + +/* Label for 'cancel backup' button in the backup settings view. */ +"SETTINGS_BACKUP_CANCEL_BACKUP" = "Cancel Backup"; + /* Label for switch in settings that controls whether or not backup is enabled. */ "SETTINGS_BACKUP_ENABLING_SWITCH" = "Backup Enabled"; +/* Label for phase row in the in the backup settings view. */ +"SETTINGS_BACKUP_PHASE" = "Phase"; + +/* Label for phase row in the in the backup settings view. */ +"SETTINGS_BACKUP_PROGRESS" = "Progress"; + +/* Label for status row in the in the backup settings view. */ +"SETTINGS_BACKUP_STATUS" = "Status"; + +/* Indicates that the last backup failed. */ +"SETTINGS_BACKUP_STATUS_FAILED" = "Backup Failed"; + +/* Indicates that app is not backing up. */ +"SETTINGS_BACKUP_STATUS_IDLE" = "Waiting"; + +/* Indicates that app is backing up. */ +"SETTINGS_BACKUP_STATUS_IN_PROGRESS" = "Backing Up"; + +/* Indicates that the last backup succeeded. */ +"SETTINGS_BACKUP_STATUS_SUCCEEDED" = "Backup Successful"; + /* A label for the 'add phone number' button in the block list table. */ "SETTINGS_BLOCK_LIST_ADD_BUTTON" = "Add…"; From fc4a66365e7a91e27e6cff8f5ed942985d7bdd95 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Tue, 13 Mar 2018 12:20:26 -0300 Subject: [PATCH 06/19] Sketch out backup export UI. --- Signal/src/util/OWSBackup.m | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/Signal/src/util/OWSBackup.m b/Signal/src/util/OWSBackup.m index e41c6978e..8f340bff9 100644 --- a/Signal/src/util/OWSBackup.m +++ b/Signal/src/util/OWSBackup.m @@ -164,9 +164,7 @@ NS_ASSUME_NONNULL_BEGIN [self.backupExportJob cancel]; self.backupExportJob = nil; - _backupExportState = OWSBackupState_Idle; - - [self postDidChangeNotification]; + [self ensureBackupExportState]; } - (void)setLastExportSuccessDate:(NSDate *)value From 3c2aae3b9cae6b597c5afbedb9a31adedf415e82 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Tue, 13 Mar 2018 12:24:07 -0300 Subject: [PATCH 07/19] Backup import clears database contents. --- .../ViewControllers/DebugUI/DebugUIBackup.m | 6 ------ Signal/src/util/OWSBackupImportJob.m | 18 +++++++++++------- 2 files changed, 11 insertions(+), 13 deletions(-) diff --git a/Signal/src/ViewControllers/DebugUI/DebugUIBackup.m b/Signal/src/ViewControllers/DebugUI/DebugUIBackup.m index bad3604d4..ee9d39b33 100644 --- a/Signal/src/ViewControllers/DebugUI/DebugUIBackup.m +++ b/Signal/src/ViewControllers/DebugUI/DebugUIBackup.m @@ -99,12 +99,6 @@ NS_ASSUME_NONNULL_BEGIN actionWithTitle:@"Restore" style:UIAlertActionStyleDefault handler:^(UIAlertAction *_Nonnull action) { - [[OWSPrimaryStorage.sharedManager newDatabaseConnection] - readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { - [transaction removeAllObjectsInCollection:[TSThread collection]]; - [transaction removeAllObjectsInCollection:[TSInteraction collection]]; - [transaction removeAllObjectsInCollection:[TSAttachment collection]]; - }]; [OWSBackup.sharedManager tryToImportBackup]; }]]; [controller addAction:[OWSAlerts cancelAction]]; diff --git a/Signal/src/util/OWSBackupImportJob.m b/Signal/src/util/OWSBackupImportJob.m index 7bf192ac3..d187d318e 100644 --- a/Signal/src/util/OWSBackupImportJob.m +++ b/Signal/src/util/OWSBackupImportJob.m @@ -415,17 +415,21 @@ NSString *const kOWSBackup_ImportDatabaseKeySpec = @"kOWSBackup_ImportDatabaseKe continue; } if ([dstTransaction numberOfKeysInCollection:collection] > 0) { - DDLogError(@"%@ cannot restore into non-empty database (%@).", self.logTag, collection); - aborted = YES; - return completion(NO); + DDLogError(@"%@ unexpected contents in database (%@).", self.logTag, collection); } } - // Clear existing migrations. + // Clear existing database contents. // - // This is safe since we only ever import into an empty database. - // Non-database migrations should be idempotent. - [dstTransaction removeAllObjectsInCollection:[OWSDatabaseMigration collection]]; + // This should be safe since we only ever import into an empty database. + // + // Note that if the app receives a message after registering and before restoring + // backup, it will be lost. + // + // Note that this will clear all migrations. + for (NSString *collection in collectionsToRestore) { + [dstTransaction removeAllObjectsInCollection:collection]; + } // Copy database entities. for (NSString *collection in collectionsToRestore) { From 061ce8cb13205dfe355f84da0f217112d7fe22c7 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Tue, 13 Mar 2018 12:30:38 -0300 Subject: [PATCH 08/19] Add database validity check. --- Signal/src/util/OWSBackupExportJob.m | 7 +++++++ Signal/src/util/OWSBackupImportJob.m | 8 ++++++++ Signal/src/util/OWSBackupJob.h | 3 +++ Signal/src/util/OWSBackupJob.m | 3 +++ 4 files changed, 21 insertions(+) diff --git a/Signal/src/util/OWSBackupExportJob.m b/Signal/src/util/OWSBackupExportJob.m index f49039f32..4dd855c27 100644 --- a/Signal/src/util/OWSBackupExportJob.m +++ b/Signal/src/util/OWSBackupExportJob.m @@ -248,6 +248,10 @@ NSString *const kOWSBackup_ExportDatabaseKeySpec = @"kOWSBackup_ExportDatabaseKe [primaryDBConnection readWithBlock:^(YapDatabaseReadTransaction *srcTransaction) { [tempDBConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *dstTransaction) { + [dstTransaction setObject:@(YES) + forKey:kOWSBackup_Snapshot_ValidKey + inCollection:kOWSBackup_Snapshot_Collection]; + // Copy threads. [srcTransaction enumerateKeysAndObjectsInCollection:[TSThread collection] @@ -347,6 +351,9 @@ NSString *const kOWSBackup_ExportDatabaseKeySpec = @"kOWSBackup_ExportDatabaseKe }]; }]; + if (self.isComplete) { + return NO; + } // TODO: Should we do a database checkpoint? DDLogInfo(@"%@ copiedThreads: %llu", self.logTag, copiedThreads); diff --git a/Signal/src/util/OWSBackupImportJob.m b/Signal/src/util/OWSBackupImportJob.m index d187d318e..8d8c8c63a 100644 --- a/Signal/src/util/OWSBackupImportJob.m +++ b/Signal/src/util/OWSBackupImportJob.m @@ -408,6 +408,14 @@ NSString *const kOWSBackup_ImportDatabaseKeySpec = @"kOWSBackup_ImportDatabaseKe __block BOOL aborted = NO; [tempDBConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *srcTransaction) { [primaryDBConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *dstTransaction) { + if (![srcTransaction boolForKey:kOWSBackup_Snapshot_ValidKey + inCollection:kOWSBackup_Snapshot_Collection + defaultValue:NO]) { + DDLogError(@"%@ invalid database.", self.logTag); + aborted = YES; + return completion(NO); + } + for (NSString *collection in collectionsToRestore) { if ([collection isEqualToString:[OWSDatabaseMigration collection]]) { // It's okay if there are existing migrations; we'll clear those diff --git a/Signal/src/util/OWSBackupJob.h b/Signal/src/util/OWSBackupJob.h index bb723a7f7..ee5b4aa0b 100644 --- a/Signal/src/util/OWSBackupJob.h +++ b/Signal/src/util/OWSBackupJob.h @@ -8,6 +8,9 @@ extern NSString *const kOWSBackup_ManifestKey_DatabaseFiles; extern NSString *const kOWSBackup_ManifestKey_AttachmentFiles; extern NSString *const kOWSBackup_ManifestKey_DatabaseKeySpec; +extern NSString *const kOWSBackup_Snapshot_Collection; +extern NSString *const kOWSBackup_Snapshot_ValidKey; + typedef void (^OWSBackupJobBoolCompletion)(BOOL success); typedef void (^OWSBackupJobCompletion)(NSError *_Nullable error); diff --git a/Signal/src/util/OWSBackupJob.m b/Signal/src/util/OWSBackupJob.m index 6c1d7697f..895845e50 100644 --- a/Signal/src/util/OWSBackupJob.m +++ b/Signal/src/util/OWSBackupJob.m @@ -16,6 +16,9 @@ NSString *const kOWSBackup_ManifestKey_DatabaseKeySpec = @"database_key_spec"; NSString *const kOWSBackup_KeychainService = @"kOWSBackup_KeychainService"; +NSString *const kOWSBackup_Snapshot_Collection = @"kOWSBackup_Snapshot_Collection"; +NSString *const kOWSBackup_Snapshot_ValidKey = @"kOWSBackup_Snapshot_ValidKey"; + @interface OWSBackupJob () @property (nonatomic, weak) id delegate; From 2915c533b2c18ddcd9b932f9299600367f3f0eee Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Tue, 13 Mar 2018 12:42:50 -0300 Subject: [PATCH 09/19] Streamline database configuration and cleanup. --- .../OWSBackupSettingsViewController.m | 7 -- Signal/src/util/OWSBackupExportJob.m | 95 ++++++++++++++----- Signal/src/util/OWSBackupImportJob.m | 24 ++--- Signal/src/util/OWSBackupJob.m | 2 + 4 files changed, 86 insertions(+), 42 deletions(-) diff --git a/Signal/src/ViewControllers/AppSettings/OWSBackupSettingsViewController.m b/Signal/src/ViewControllers/AppSettings/OWSBackupSettingsViewController.m index 9651a72e8..f8624f8da 100644 --- a/Signal/src/ViewControllers/AppSettings/OWSBackupSettingsViewController.m +++ b/Signal/src/ViewControllers/AppSettings/OWSBackupSettingsViewController.m @@ -88,13 +88,6 @@ NS_ASSUME_NONNULL_BEGIN if (OWSBackup.sharedManager.backupExportProgress) { NSUInteger progressPercent = (NSUInteger)round(OWSBackup.sharedManager.backupExportProgress.floatValue * 100); - DDLogVerbose(@"%@ '%@', '%@'", - self.logTag, - @(progressPercent).stringValue, - [NSString - stringWithFormat:NSLocalizedString(@"PERCENTAGE_FORMAT", - @"Format for percentages, e.g. 65%. Embeds {{percentage}}, e.g. 65."), - @(progressPercent).stringValue]); [progressSection addItem:[OWSTableItem labelItemWithText:NSLocalizedString(@"SETTINGS_BACKUP_PROGRESS", diff --git a/Signal/src/util/OWSBackupExportJob.m b/Signal/src/util/OWSBackupExportJob.m index 4dd855c27..59fd7438f 100644 --- a/Signal/src/util/OWSBackupExportJob.m +++ b/Signal/src/util/OWSBackupExportJob.m @@ -86,10 +86,10 @@ NSString *const kOWSBackup_ExportDatabaseKeySpec = @"kOWSBackup_ExportDatabaseKe @interface OWSBackupExportJob () -@property (nonatomic, nullable) OWSBackupStorage *backupStorage; - @property (nonatomic, nullable) OWSBackgroundTask *backgroundTask; +@property (nonatomic, nullable) OWSBackupStorage *backupStorage; + @property (nonatomic) NSMutableArray *databaseFilePaths; // A map of "record name"-to-"file name". @property (nonatomic) NSMutableDictionary *databaseRecordMap; @@ -135,8 +135,8 @@ NSString *const kOWSBackup_ExportDatabaseKeySpec = @"kOWSBackup_ExportDatabaseKe progress:nil]; __weak OWSBackupExportJob *weakSelf = self; - [self configureExport:^(BOOL success) { - if (!success) { + [self configureExport:^(BOOL configureExportSuccess) { + if (!configureExportSuccess) { [self failWithErrorDescription: NSLocalizedString(@"BACKUP_EXPORT_ERROR_COULD_NOT_EXPORT", @"Error indicating the a backup export could not export the user's data.")]; @@ -149,26 +149,29 @@ NSString *const kOWSBackup_ExportDatabaseKeySpec = @"kOWSBackup_ExportDatabaseKe [self updateProgressWithDescription:NSLocalizedString(@"BACKUP_EXPORT_PHASE_EXPORT", @"Indicates that the backup export data is being exported.") progress:nil]; - if (![self exportDatabase]) { - [self failWithErrorDescription: - NSLocalizedString(@"BACKUP_EXPORT_ERROR_COULD_NOT_EXPORT", - @"Error indicating the a backup export could not export the user's data.")]; - return; - } - if (self.isComplete) { - return; - } - [self saveToCloud:^(NSError *_Nullable saveError) { - if (saveError) { - [weakSelf failWithError:saveError]; + [self exportDatabase:^(BOOL exportDatabaseSuccess) { + if (!exportDatabaseSuccess) { + [self failWithErrorDescription: + NSLocalizedString(@"BACKUP_EXPORT_ERROR_COULD_NOT_EXPORT", + @"Error indicating the a backup export could not export the user's data.")]; return; } - [self cleanUpCloud:^(NSError *_Nullable cleanUpError) { - if (cleanUpError) { - [weakSelf failWithError:cleanUpError]; + + if (self.isComplete) { + return; + } + [self saveToCloud:^(NSError *_Nullable saveError) { + if (saveError) { + [weakSelf failWithError:saveError]; return; } - [weakSelf succeed]; + [self cleanUpCloud:^(NSError *_Nullable cleanUpError) { + if (cleanUpError) { + [weakSelf failWithError:cleanUpError]; + return; + } + [weakSelf succeed]; + }]; }]; }]; }]; @@ -184,6 +187,48 @@ NSString *const kOWSBackup_ExportDatabaseKeySpec = @"kOWSBackup_ExportDatabaseKe OWSProdLogAndFail(@"%@ Could not create jobTempDirPath.", self.logTag); return completion(NO); } + dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{ + completion(YES); + }); + + // TSRequest *currentSignedPreKey = [OWSRequestFactory currentSignedPreKeyRequest]; + // [[TSNetworkManager sharedManager] makeRequest:currentSignedPreKey + // success:^(NSURLSessionDataTask *task, NSDictionary *responseObject) { + // NSString *keyIdDictKey = @"keyId"; + // NSNumber *keyId = [responseObject objectForKey:keyIdDictKey]; + // OWSAssert(keyId); + // + // OWSPrimaryStorage *primaryStorage = [OWSPrimaryStorage + // sharedManager]; NSNumber *currentSignedPrekeyId = [primaryStorage + // currentSignedPrekeyId]; + // + // if (!keyId || !currentSignedPrekeyId || ![currentSignedPrekeyId + // isEqualToNumber:keyId]) { + // DDLogError( + // @"%@ Local and service 'current signed prekey ids' + // did not match. %@ == %@ == %d.", self.logTag, keyId, + // currentSignedPrekeyId, + // [currentSignedPrekeyId isEqualToNumber:keyId]); + // } + // } + // failure:^(NSURLSessionDataTask *task, NSError *error) { + // if (!IsNSErrorNetworkFailure(error)) { + // OWSProdError([OWSAnalyticsEvents + // errorPrekeysCurrentSignedPrekeyRequestFailed]); + // } + // DDLogWarn(@"%@ Could not retrieve current signed key from the + // service.", self.logTag); + // + // // Mark the prekeys as _NOT_ checked on failure. + // [self markPreKeysAsNotChecked]; + // }]; +} + +- (void)exportDatabase:(OWSBackupJobBoolCompletion)completion +{ + OWSAssert(completion); + + DDLogVerbose(@"%@ %s", self.logTag, __PRETTY_FUNCTION__); if (![OWSBackupJob generateRandomDatabaseKeySpecWithKeychainKey:kOWSBackup_ExportDatabaseKeySpec]) { OWSProdLogAndFail(@"%@ Could not generate database key spec for export.", self.logTag); @@ -205,6 +250,7 @@ NSString *const kOWSBackup_ExportDatabaseKeySpec = @"kOWSBackup_ExportDatabaseKe } return databaseKeySpec; }; + self.backupStorage = [[OWSBackupStorage alloc] initBackupStorageWithDatabaseDirPath:jobDatabaseDirPath keySpecBlock:keySpecBlock]; if (!self.backupStorage) { @@ -213,17 +259,18 @@ NSString *const kOWSBackup_ExportDatabaseKeySpec = @"kOWSBackup_ExportDatabaseKe } // TODO: Do we really need to run these registrations on the main thread? + __weak OWSBackupExportJob *weakSelf = self; dispatch_async(dispatch_get_main_queue(), ^{ - [self.backupStorage runSyncRegistrations]; - [self.backupStorage runAsyncRegistrationsWithCompletion:^{ + [weakSelf.backupStorage runSyncRegistrations]; + [weakSelf.backupStorage runAsyncRegistrationsWithCompletion:^{ dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{ - completion(YES); + completion([weakSelf exportDatabaseContents]); }); }]; }); } -- (BOOL)exportDatabase +- (BOOL)exportDatabaseContents { DDLogVerbose(@"%@ %s", self.logTag, __PRETTY_FUNCTION__); diff --git a/Signal/src/util/OWSBackupImportJob.m b/Signal/src/util/OWSBackupImportJob.m index 8d8c8c63a..1ca547a60 100644 --- a/Signal/src/util/OWSBackupImportJob.m +++ b/Signal/src/util/OWSBackupImportJob.m @@ -24,6 +24,8 @@ NSString *const kOWSBackup_ImportDatabaseKeySpec = @"kOWSBackup_ImportDatabaseKe @property (nonatomic, nullable) OWSBackgroundTask *backgroundTask; +@property (nonatomic, nullable) OWSBackupStorage *backupStorage; + // A map of "record name"-to-"file name". @property (nonatomic) NSMutableDictionary *databaseRecordMap; @@ -347,9 +349,9 @@ NSString *const kOWSBackup_ImportDatabaseKeySpec = @"kOWSBackup_ImportDatabaseKe } return databaseKeySpec; }; - OWSBackupStorage *_Nullable backupStorage = + self.backupStorage = [[OWSBackupStorage alloc] initBackupStorageWithDatabaseDirPath:jobDatabaseDirPath keySpecBlock:keySpecBlock]; - if (!backupStorage) { + if (!self.backupStorage) { OWSProdLogAndFail(@"%@ Could not create backupStorage.", self.logTag); return completion(NO); } @@ -357,18 +359,18 @@ NSString *const kOWSBackup_ImportDatabaseKeySpec = @"kOWSBackup_ImportDatabaseKe // TODO: Do we really need to run these registrations on the main thread? __weak OWSBackupImportJob *weakSelf = self; dispatch_async(dispatch_get_main_queue(), ^{ - [backupStorage runSyncRegistrations]; - [backupStorage runAsyncRegistrationsWithCompletion:^{ + [weakSelf.backupStorage runSyncRegistrations]; + [weakSelf.backupStorage runAsyncRegistrationsWithCompletion:^{ dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{ - [weakSelf restoreDatabaseContents:backupStorage completion:completion]; + [weakSelf restoreDatabaseContents:completion]; }); }]; }); } -- (void)restoreDatabaseContents:(OWSBackupStorage *)backupStorage completion:(OWSBackupJobBoolCompletion)completion +- (void)restoreDatabaseContents:(OWSBackupJobBoolCompletion)completion { - OWSAssert(backupStorage); + OWSAssert(self.backupStorage); OWSAssert(completion); DDLogVerbose(@"%@ %s", self.logTag, __PRETTY_FUNCTION__); @@ -377,7 +379,7 @@ NSString *const kOWSBackup_ImportDatabaseKeySpec = @"kOWSBackup_ImportDatabaseKe return completion(NO); } - YapDatabaseConnection *_Nullable tempDBConnection = backupStorage.newDatabaseConnection; + YapDatabaseConnection *_Nullable tempDBConnection = self.backupStorage.newDatabaseConnection; if (!tempDBConnection) { OWSProdLogAndFail(@"%@ Could not create tempDBConnection.", self.logTag); return completion(NO); @@ -480,13 +482,13 @@ NSString *const kOWSBackup_ImportDatabaseKeySpec = @"kOWSBackup_ImportDatabaseKe } DDLogInfo(@"%@ copiedEntities: %llu", self.logTag, copiedEntities); - [backupStorage logFileSizes]; + [self.backupStorage logFileSizes]; // Close the database. tempDBConnection = nil; - backupStorage = nil; + self.backupStorage = nil; - return completion(YES); + completion(YES); } - (void)ensureMigrations:(OWSBackupJobBoolCompletion)completion diff --git a/Signal/src/util/OWSBackupJob.m b/Signal/src/util/OWSBackupJob.m index 895845e50..eda6a4a4a 100644 --- a/Signal/src/util/OWSBackupJob.m +++ b/Signal/src/util/OWSBackupJob.m @@ -97,6 +97,7 @@ NSString *const kOWSBackup_Snapshot_ValidKey = @"kOWSBackup_Snapshot_ValidKey"; dispatch_async(dispatch_get_main_queue(), ^{ if (self.isComplete) { + OWSAssert(!self.hasSucceeded); return; } self.isComplete = YES; @@ -120,6 +121,7 @@ NSString *const kOWSBackup_Snapshot_ValidKey = @"kOWSBackup_Snapshot_ValidKey"; OWSProdLogAndFail(@"%@ %s %@", self.logTag, __PRETTY_FUNCTION__, error); dispatch_async(dispatch_get_main_queue(), ^{ + OWSAssert(!self.hasSucceeded); if (self.isComplete) { return; } From 59fc23212897a5f822d29fea08f7a3fb3f26fe74 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Tue, 13 Mar 2018 12:49:20 -0300 Subject: [PATCH 10/19] Backup export needs to verify that we have a valid account. --- Signal/src/util/OWSBackupExportJob.m | 53 +++++++++++----------------- 1 file changed, 20 insertions(+), 33 deletions(-) diff --git a/Signal/src/util/OWSBackupExportJob.m b/Signal/src/util/OWSBackupExportJob.m index 59fd7438f..3a831c4cb 100644 --- a/Signal/src/util/OWSBackupExportJob.m +++ b/Signal/src/util/OWSBackupExportJob.m @@ -187,41 +187,28 @@ NSString *const kOWSBackup_ExportDatabaseKeySpec = @"kOWSBackup_ExportDatabaseKe OWSProdLogAndFail(@"%@ Could not create jobTempDirPath.", self.logTag); return completion(NO); } - dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{ - completion(YES); - }); - // TSRequest *currentSignedPreKey = [OWSRequestFactory currentSignedPreKeyRequest]; - // [[TSNetworkManager sharedManager] makeRequest:currentSignedPreKey - // success:^(NSURLSessionDataTask *task, NSDictionary *responseObject) { - // NSString *keyIdDictKey = @"keyId"; - // NSNumber *keyId = [responseObject objectForKey:keyIdDictKey]; - // OWSAssert(keyId); - // - // OWSPrimaryStorage *primaryStorage = [OWSPrimaryStorage - // sharedManager]; NSNumber *currentSignedPrekeyId = [primaryStorage - // currentSignedPrekeyId]; - // - // if (!keyId || !currentSignedPrekeyId || ![currentSignedPrekeyId - // isEqualToNumber:keyId]) { - // DDLogError( - // @"%@ Local and service 'current signed prekey ids' - // did not match. %@ == %@ == %d.", self.logTag, keyId, - // currentSignedPrekeyId, - // [currentSignedPrekeyId isEqualToNumber:keyId]); - // } - // } - // failure:^(NSURLSessionDataTask *task, NSError *error) { - // if (!IsNSErrorNetworkFailure(error)) { - // OWSProdError([OWSAnalyticsEvents - // errorPrekeysCurrentSignedPrekeyRequestFailed]); - // } - // DDLogWarn(@"%@ Could not retrieve current signed key from the - // service.", self.logTag); + // We need to verify that we have a valid account. + // Otherwise, if we re-register on another device, we + // continue to backup on our old device, overwriting + // backups from the new device. // - // // Mark the prekeys as _NOT_ checked on failure. - // [self markPreKeysAsNotChecked]; - // }]; + // We use an arbitrary request that requires authentication + // to verify our account state. + TSRequest *currentSignedPreKey = [OWSRequestFactory currentSignedPreKeyRequest]; + [[TSNetworkManager sharedManager] makeRequest:currentSignedPreKey + success:^(NSURLSessionDataTask *task, NSDictionary *responseObject) { + dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{ + completion(YES); + }); + } + failure:^(NSURLSessionDataTask *task, NSError *error) { + // TODO: We may want to surface this in the UI. + DDLogError(@"%@ could not verify account status: %@.", self.logTag, error); + dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{ + completion(NO); + }); + }]; } - (void)exportDatabase:(OWSBackupJobBoolCompletion)completion From 0bcbb5918b4ddd1fd403392ee2ec42ee188d7ff6 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Tue, 13 Mar 2018 13:01:44 -0300 Subject: [PATCH 11/19] Improve backup progress. --- Signal/src/util/OWSBackupImportJob.m | 25 +++++++++++++++++-- .../translations/en.lproj/Localizable.strings | 22 ++++++++++++---- 2 files changed, 40 insertions(+), 7 deletions(-) diff --git a/Signal/src/util/OWSBackupImportJob.m b/Signal/src/util/OWSBackupImportJob.m index 1ca547a60..172a948bd 100644 --- a/Signal/src/util/OWSBackupImportJob.m +++ b/Signal/src/util/OWSBackupImportJob.m @@ -242,10 +242,11 @@ NSString *const kOWSBackup_ImportDatabaseKeySpec = @"kOWSBackup_ImportDatabaseKe // A map of "record name"-to-"downloaded file path". self.downloadedFileMap = [NSMutableDictionary new]; - [self downloadNextFileFromCloud:recordNames completion:completion]; + [self downloadNextFileFromCloud:recordNames recordCount:recordNames.count completion:completion]; } - (void)downloadNextFileFromCloud:(NSMutableArray *)recordNames + recordCount:(NSUInteger)recordCount completion:(OWSBackupJobCompletion)completion { OWSAssert(recordNames); @@ -263,11 +264,15 @@ NSString *const kOWSBackup_ImportDatabaseKeySpec = @"kOWSBackup_ImportDatabaseKe NSString *recordName = recordNames.lastObject; [recordNames removeLastObject]; + [self updateProgressWithDescription:NSLocalizedString(@"BACKUP_IMPORT_PHASE_DOWNLOAD", + @"Indicates that the backup import data is being downloaded.") + progress:@((recordCount - recordNames.count) / (CGFloat)recordCount)]; + if (![recordName isKindOfClass:[NSString class]]) { DDLogError(@"%@ invalid record name in manifest: %@", self.logTag, [recordName class]); // Invalid record name in the manifest. This may be recoverable. // Ignore this for now and proceed with the other downloads. - return [self downloadNextFileFromCloud:recordNames completion:completion]; + return [self downloadNextFileFromCloud:recordNames recordCount:recordCount completion:completion]; } // Use a predictable file path so that multiple "import backup" attempts @@ -302,11 +307,18 @@ NSString *const kOWSBackup_ImportDatabaseKeySpec = @"kOWSBackup_ImportDatabaseKe NSString *attachmentsDirPath = [TSAttachmentStream attachmentsFolder]; + NSUInteger count = 0; for (NSString *recordName in self.attachmentRecordMap) { if (self.isComplete) { return; } + count++; + [self updateProgressWithDescription:NSLocalizedString(@"BACKUP_IMPORT_PHASE_RESTORING_FILES", + @"Indicates that the backup import data is being restored.") + progress:@(count / (CGFloat)self.attachmentRecordMap.count)]; + + NSString *dstRelativePath = self.attachmentRecordMap[recordName]; if (! [self restoreFileWithRecordName:recordName dstRelativePath:dstRelativePath dstDirPath:attachmentsDirPath]) { @@ -322,6 +334,10 @@ NSString *const kOWSBackup_ImportDatabaseKeySpec = @"kOWSBackup_ImportDatabaseKe DDLogVerbose(@"%@ %s", self.logTag, __PRETTY_FUNCTION__); + [self updateProgressWithDescription:NSLocalizedString(@"BACKUP_IMPORT_PHASE_RESTORING_DATABASE", + @"Indicates that the backup database is being restored.") + progress:nil]; + NSString *jobDatabaseDirPath = [self.jobTempDirPath stringByAppendingPathComponent:@"database"]; if (![OWSFileSystem ensureDirectoryExists:jobDatabaseDirPath]) { OWSProdLogAndFail(@"%@ Could not create jobDatabaseDirPath.", self.logTag); @@ -497,6 +513,11 @@ NSString *const kOWSBackup_ImportDatabaseKeySpec = @"kOWSBackup_ImportDatabaseKe DDLogVerbose(@"%@ %s", self.logTag, __PRETTY_FUNCTION__); + [self updateProgressWithDescription:NSLocalizedString(@"BACKUP_IMPORT_PHASE_FINALIZING", + @"Indicates that the backup import data is being finalized.") + progress:nil]; + + // It's okay that we do this in a separate transaction from the // restoration of backup contents. If some of migrations don't // complete, they'll be run the next time the app launches. diff --git a/Signal/translations/en.lproj/Localizable.strings b/Signal/translations/en.lproj/Localizable.strings index aeedab255..7a89b1173 100644 --- a/Signal/translations/en.lproj/Localizable.strings +++ b/Signal/translations/en.lproj/Localizable.strings @@ -164,16 +164,16 @@ "BACKUP_EXPORT_ERROR_SAVE_FILE_TO_CLOUD_FAILED" = "Backup could not upload data."; /* Indicates that the cloud is being cleaned up. */ -"BACKUP_EXPORT_PHASE_CLEAN_UP" = "Cleaning up backup data."; +"BACKUP_EXPORT_PHASE_CLEAN_UP" = "Cleaning Up Backup"; /* Indicates that the backup export is being configured. */ -"BACKUP_EXPORT_PHASE_CONFIGURATION" = "Initializing backup."; +"BACKUP_EXPORT_PHASE_CONFIGURATION" = "Initializing Backup"; /* Indicates that the backup export data is being exported. */ -"BACKUP_EXPORT_PHASE_EXPORT" = "Exporting backup data."; +"BACKUP_EXPORT_PHASE_EXPORT" = "Exporting Backup"; /* Indicates that the backup export data is being uploaded. */ -"BACKUP_EXPORT_PHASE_UPLOAD" = "Uploading backup data."; +"BACKUP_EXPORT_PHASE_UPLOAD" = "Uploading Backup"; /* Error indicating the a backup import could not import the user's data. */ "BACKUP_IMPORT_ERROR_COULD_NOT_IMPORT" = "Backup could not be imported."; @@ -182,11 +182,23 @@ "BACKUP_IMPORT_ERROR_DOWNLOAD_FILE_FROM_CLOUD_FAILED" = "Could not download backup data."; /* Indicates that the backup import is being configured. */ -"BACKUP_IMPORT_PHASE_CONFIGURATION" = "Configuring backup restore."; +"BACKUP_IMPORT_PHASE_CONFIGURATION" = "Configuring Backup"; + +/* Indicates that the backup import data is being downloaded. */ +"BACKUP_IMPORT_PHASE_DOWNLOAD" = "Downloading Backup Data"; + +/* Indicates that the backup import data is being finalized. */ +"BACKUP_IMPORT_PHASE_FINALIZING" = "Finalizing Backup"; /* Indicates that the backup import data is being imported. */ "BACKUP_IMPORT_PHASE_IMPORT" = "Importing backup."; +/* Indicates that the backup database is being restored. */ +"BACKUP_IMPORT_PHASE_RESTORING_DATABASE" = "Restoring Database"; + +/* Indicates that the backup import data is being restored. */ +"BACKUP_IMPORT_PHASE_RESTORING_FILES" = "Restoring Files"; + /* An explanation of the consequences of blocking another user. */ "BLOCK_BEHAVIOR_EXPLANATION" = "Blocked users will not be able to call you or send you messages."; From f164d5e94be0dd396fd2f5361bc2fe91d80db92a Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Tue, 13 Mar 2018 13:05:01 -0300 Subject: [PATCH 12/19] Improve backup progress. --- Signal/src/util/OWSBackupImportJob.m | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Signal/src/util/OWSBackupImportJob.m b/Signal/src/util/OWSBackupImportJob.m index 172a948bd..eae4ad2d9 100644 --- a/Signal/src/util/OWSBackupImportJob.m +++ b/Signal/src/util/OWSBackupImportJob.m @@ -283,7 +283,7 @@ NSString *const kOWSBackup_ImportDatabaseKeySpec = @"kOWSBackup_ImportDatabaseKe if ([NSFileManager.defaultManager fileExistsAtPath:tempFilePath]) { [OWSFileSystem protectFileOrFolderAtPath:tempFilePath]; self.downloadedFileMap[recordName] = tempFilePath; - [self downloadNextFileFromCloud:recordNames completion:completion]; + [self downloadNextFileFromCloud:recordNames recordCount:recordCount completion:completion]; return; } @@ -293,7 +293,7 @@ NSString *const kOWSBackup_ImportDatabaseKeySpec = @"kOWSBackup_ImportDatabaseKe dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{ [OWSFileSystem protectFileOrFolderAtPath:tempFilePath]; self.downloadedFileMap[recordName] = tempFilePath; - [self downloadNextFileFromCloud:recordNames completion:completion]; + [self downloadNextFileFromCloud:recordNames recordCount:recordCount completion:completion]; }); } failure:^(NSError *error) { From 05db8e3f7f1e8d7d422a3008ae75b8844612fb17 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Tue, 13 Mar 2018 14:05:51 -0300 Subject: [PATCH 13/19] Retry backup failures. --- .../AppSettings/AppSettingsViewController.m | 3 +- Signal/src/util/OWSBackup.m | 4 - Signal/src/util/OWSBackupAPI.swift | 283 ++++++++++++------ 3 files changed, 195 insertions(+), 95 deletions(-) diff --git a/Signal/src/ViewControllers/AppSettings/AppSettingsViewController.m b/Signal/src/ViewControllers/AppSettings/AppSettingsViewController.m index e60226c28..15dfcd95b 100644 --- a/Signal/src/ViewControllers/AppSettings/AppSettingsViewController.m +++ b/Signal/src/ViewControllers/AppSettings/AppSettingsViewController.m @@ -92,7 +92,8 @@ [self updateTableContents]; dispatch_async(dispatch_get_main_queue(), ^{ - [self showBackup]; + // [self showBackup]; + [self showDebugUI]; }); } diff --git a/Signal/src/util/OWSBackup.m b/Signal/src/util/OWSBackup.m index 8f340bff9..d05368211 100644 --- a/Signal/src/util/OWSBackup.m +++ b/Signal/src/util/OWSBackup.m @@ -251,15 +251,11 @@ NS_ASSUME_NONNULL_BEGIN NSDate *_Nullable lastExportFailureDate = self.lastExportFailureDate; // Wait N hours before retrying after a success. const NSTimeInterval kRetryAfterSuccess = 24 * kHourInterval; - // TODO: Remove. - // const NSTimeInterval kRetryAfterSuccess = 0; if (lastExportSuccessDate && fabs(lastExportSuccessDate.timeIntervalSinceNow) < kRetryAfterSuccess) { return NO; } // Wait N hours before retrying after a failure. const NSTimeInterval kRetryAfterFailure = 6 * kHourInterval; - // TODO: Remove. - // const NSTimeInterval kRetryAfterFailure = 0; if (lastExportFailureDate && fabs(lastExportFailureDate.timeIntervalSinceNow) < kRetryAfterFailure) { return NO; } diff --git a/Signal/src/util/OWSBackupAPI.swift b/Signal/src/util/OWSBackupAPI.swift index f5b375339..c4140abc5 100644 --- a/Signal/src/util/OWSBackupAPI.swift +++ b/Signal/src/util/OWSBackupAPI.swift @@ -13,16 +13,24 @@ import CloudKit static let signalBackupRecordType = "signalBackup" static let manifestRecordName = "manifest" static let payloadKey = "payload" + static let maxImmediateRetries = 5 - @objc - public class func recordIdForTest() -> String { + private class func recordIdForTest() -> String { return "test-\(NSUUID().uuidString)" } + private class func database() -> CKDatabase { + let myContainer = CKContainer.default() + let privateDatabase = myContainer.privateCloudDatabase + return privateDatabase + } + + // MARK: - Upload + @objc public class func saveTestFileToCloud(fileUrl: URL, - success: @escaping (String) -> Void, - failure: @escaping (Error) -> Void) { + success: @escaping (String) -> Swift.Void, + failure: @escaping (Error) -> Swift.Void) { saveFileToCloud(fileUrl: fileUrl, recordName: NSUUID().uuidString, recordType: signalBackupRecordType, @@ -36,13 +44,13 @@ import CloudKit // complete. @objc public class func saveEphemeralDatabaseFileToCloud(fileUrl: URL, - success: @escaping (String) -> Void, - failure: @escaping (Error) -> Void) { + success: @escaping (String) -> Swift.Void, + failure: @escaping (Error) -> Swift.Void) { saveFileToCloud(fileUrl: fileUrl, recordName: "ephemeralFile-\(NSUUID().uuidString)", - recordType: signalBackupRecordType, - success: success, - failure: failure) + recordType: signalBackupRecordType, + success: success, + failure: failure) } // "Persistent" files may be shared between backup export; they should only be saved @@ -50,9 +58,9 @@ import CloudKit // backups can reuse the same record. @objc public class func savePersistentFileOnceToCloud(fileId: String, - fileUrlBlock: @escaping (()) -> URL?, - success: @escaping (String) -> Void, - failure: @escaping (Error) -> Void) { + fileUrlBlock: @escaping (Swift.Void) -> URL?, + success: @escaping (String) -> Swift.Void, + failure: @escaping (Error) -> Swift.Void) { saveFileOnceToCloud(recordName: "persistentFile-\(fileId)", recordType: signalBackupRecordType, fileUrlBlock: fileUrlBlock, @@ -62,8 +70,8 @@ import CloudKit @objc public class func upsertManifestFileToCloud(fileUrl: URL, - success: @escaping (String) -> Void, - failure: @escaping (Error) -> Void) { + success: @escaping (String) -> Swift.Void, + failure: @escaping (Error) -> Swift.Void) { // We want to use a well-known record id and type for manifest files. upsertFileToCloud(fileUrl: fileUrl, recordName: manifestRecordName, @@ -76,8 +84,8 @@ import CloudKit public class func saveFileToCloud(fileUrl: URL, recordName: String, recordType: String, - success: @escaping (String) -> Void, - failure: @escaping (Error) -> Void) { + success: @escaping (String) -> Swift.Void, + failure: @escaping (Error) -> Swift.Void) { let recordID = CKRecordID(recordName: recordName) let record = CKRecord(recordType: recordType, recordID: recordID) let asset = CKAsset(fileURL: fileUrl) @@ -90,49 +98,45 @@ import CloudKit @objc public class func saveRecordToCloud(record: CKRecord, - success: @escaping (String) -> Void, - failure: @escaping (Error) -> Void) { - - let myContainer = CKContainer.default() - let privateDatabase = myContainer.privateCloudDatabase - privateDatabase.save(record) { - (record, error) in - - if let error = error { - Logger.error("\(self.logTag) error saving record: \(error)") - failure(error) - } else { - guard let recordName = record?.recordID.recordName else { - Logger.error("\(self.logTag) error retrieving saved record's name.") - failure(OWSErrorWithCodeDescription(.exportBackupError, - NSLocalizedString("BACKUP_EXPORT_ERROR_SAVE_FILE_TO_CLOUD_FAILED", - comment: "Error indicating the a backup export failed to save a file to the cloud."))) - return - } - Logger.info("\(self.logTag) saved record.") - success(recordName) - } - } + success: @escaping (String) -> Swift.Void, + failure: @escaping (Error) -> Swift.Void) { + saveRecordToCloud(record: record, + remainingRetries: maxImmediateRetries, + success: success, + failure: failure) } - @objc - public class func deleteRecordFromCloud(recordName: String, - success: @escaping (()) -> Void, - failure: @escaping (Error) -> Void) { + private class func saveRecordToCloud(record: CKRecord, + remainingRetries: Int, + success: @escaping (String) -> Swift.Void, + failure: @escaping (Error) -> Swift.Void) { - let recordID = CKRecordID(recordName: recordName) + database().save(record) { + (_, error) in - let myContainer = CKContainer.default() - let privateDatabase = myContainer.privateCloudDatabase - privateDatabase.delete(withRecordID: recordID) { - (record, error) in - - if let error = error { - Logger.error("\(self.logTag) error deleting record: \(error)") - failure(error) - } else { - Logger.info("\(self.logTag) deleted record.") - success() + let response = responseForCloudKitError(error: error, + remainingRetries: remainingRetries, + label: "Save Record") + switch response { + case .success: + let recordName = record.recordID.recordName + success(recordName) + case .failureDoNotRetry(let responseError): + failure(responseError) + case .failureRetryAfterDelay(let retryDelay): + DispatchQueue.global().asyncAfter(deadline: DispatchTime.now() + retryDelay, execute: { + saveRecordToCloud(record: record, + remainingRetries: remainingRetries - 1, + success: success, + failure: failure) + }) + case .failureRetryWithoutDelay: + DispatchQueue.global().async { + saveRecordToCloud(record: record, + remainingRetries: remainingRetries - 1, + success: success, + failure: failure) + } } } } @@ -146,8 +150,8 @@ import CloudKit public class func upsertFileToCloud(fileUrl: URL, recordName: String, recordType: String, - success: @escaping (String) -> Void, - failure: @escaping (Error) -> Void) { + success: @escaping (String) -> Swift.Void, + failure: @escaping (Error) -> Swift.Void) { checkForFileInCloud(recordName: recordName, success: { (record) in @@ -178,9 +182,9 @@ import CloudKit @objc public class func saveFileOnceToCloud(recordName: String, recordType: String, - fileUrlBlock: @escaping (()) -> URL?, - success: @escaping (String) -> Void, - failure: @escaping (Error) -> Void) { + fileUrlBlock: @escaping (Swift.Void) -> URL?, + success: @escaping (String) -> Swift.Void, + failure: @escaping (Error) -> Swift.Void) { checkForFileInCloud(recordName: recordName, success: { (record) in @@ -207,9 +211,59 @@ import CloudKit failure: failure) } + // MARK: - Delete + + @objc + public class func deleteRecordFromCloud(recordName: String, + success: @escaping (Swift.Void) -> Swift.Void, + failure: @escaping (Error) -> Swift.Void) { + deleteRecordFromCloud(recordName: recordName, + remainingRetries: maxImmediateRetries, + success: success, + failure: failure) + } + + private class func deleteRecordFromCloud(recordName: String, + remainingRetries: Int, + success: @escaping (Swift.Void) -> Swift.Void, + failure: @escaping (Error) -> Swift.Void) { + + let recordID = CKRecordID(recordName: recordName) + + database().delete(withRecordID: recordID) { + (record, error) in + + let response = responseForCloudKitError(error: error, + remainingRetries: remainingRetries, + label: "Delete Record") + switch response { + case .success: + success() + case .failureDoNotRetry(let responseError): + failure(responseError) + case .failureRetryAfterDelay(let retryDelay): + DispatchQueue.global().asyncAfter(deadline: DispatchTime.now() + retryDelay, execute: { + deleteRecordFromCloud(recordName: recordName, + remainingRetries: remainingRetries - 1, + success: success, + failure: failure) + }) + case .failureRetryWithoutDelay: + DispatchQueue.global().async { + deleteRecordFromCloud(recordName: recordName, + remainingRetries: remainingRetries - 1, + success: success, + failure: failure) + } + } + } + } + + // MARK: - Exists? + private class func checkForFileInCloud(recordName: String, - success: @escaping (CKRecord?) -> Void, - failure: @escaping (Error) -> Void) { + success: @escaping (CKRecord?) -> Swift.Void, + failure: @escaping (Error) -> Swift.Void) { let recordId = CKRecordID(recordName: recordName) let fetchOperation = CKFetchRecordsOperation(recordIDs: [recordId ]) // Don't download the file; we're just using the fetch to check whether or @@ -240,14 +294,12 @@ import CloudKit // Record found. success(record) } - let myContainer = CKContainer.default() - let privateDatabase = myContainer.privateCloudDatabase - privateDatabase.add(fetchOperation) + database().add(fetchOperation) } @objc - public class func checkForManifestInCloud(success: @escaping (Bool) -> Void, - failure: @escaping (Error) -> Void) { + public class func checkForManifestInCloud(success: @escaping (Bool) -> Swift.Void, + failure: @escaping (Error) -> Swift.Void) { checkForFileInCloud(recordName: manifestRecordName, success: { (record) in @@ -257,8 +309,8 @@ import CloudKit } @objc - public class func fetchAllRecordNames(success: @escaping ([String]) -> Void, - failure: @escaping (Error) -> Void) { + public class func fetchAllRecordNames(success: @escaping ([String]) -> Swift.Void, + failure: @escaping (Error) -> Swift.Void) { let query = CKQuery(recordType: signalBackupRecordType, predicate: NSPredicate(value: true)) // Fetch the first page of results for this query. @@ -272,12 +324,12 @@ import CloudKit private class func fetchAllRecordNamesStep(query: CKQuery, previousRecordNames: [String], cursor: CKQueryCursor?, - success: @escaping ([String]) -> Void, - failure: @escaping (Error) -> Void) { + success: @escaping ([String]) -> Swift.Void, + failure: @escaping (Error) -> Swift.Void) { var allRecordNames = previousRecordNames - let queryOperation = CKQueryOperation(query: query) + let queryOperation = CKQueryOperation(query: query) // If this isn't the first page of results for this query, resume // where we left off. queryOperation.cursor = cursor @@ -306,25 +358,24 @@ import CloudKit Logger.info("\(self.logTag) fetched \(allRecordNames.count) record names.") success(allRecordNames) } - - let myContainer = CKContainer.default() - let privateDatabase = myContainer.privateCloudDatabase - privateDatabase.add(queryOperation) + database().add(queryOperation) } + // MARK: - Download + @objc public class func downloadManifestFromCloud( - success: @escaping (Data) -> Void, - failure: @escaping (Error) -> Void) { + success: @escaping (Data) -> Swift.Void, + failure: @escaping (Error) -> Swift.Void) { downloadDataFromCloud(recordName: manifestRecordName, - success: success, - failure: failure) + success: success, + failure: failure) } @objc public class func downloadDataFromCloud(recordName: String, - success: @escaping (Data) -> Void, - failure: @escaping (Error) -> Void) { + success: @escaping (Data) -> Swift.Void, + failure: @escaping (Error) -> Swift.Void) { downloadFromCloud(recordName: recordName, success: { (asset) in @@ -346,8 +397,8 @@ import CloudKit @objc public class func downloadFileFromCloud(recordName: String, toFileUrl: URL, - success: @escaping (()) -> Void, - failure: @escaping (Error) -> Void) { + success: @escaping (Swift.Void) -> Swift.Void, + failure: @escaping (Error) -> Swift.Void) { downloadFromCloud(recordName: recordName, success: { (asset) in @@ -367,8 +418,8 @@ import CloudKit } private class func downloadFromCloud(recordName: String, - success: @escaping (CKAsset) -> Void, - failure: @escaping (Error) -> Void) { + success: @escaping (CKAsset) -> Swift.Void, + failure: @escaping (Error) -> Swift.Void) { let recordId = CKRecordID(recordName: recordName) let fetchOperation = CKFetchRecordsOperation(recordIDs: [recordId ]) @@ -394,13 +445,13 @@ import CloudKit } success(asset) } - let myContainer = CKContainer.default() - let privateDatabase = myContainer.privateCloudDatabase - privateDatabase.add(fetchOperation) + database().add(fetchOperation) } + // MARK: - Access + @objc - public class func checkCloudKitAccess(completion: @escaping (Bool) -> Void) { + public class func checkCloudKitAccess(completion: @escaping (Bool) -> Swift.Void) { CKContainer.default().accountStatus(completionHandler: { (accountStatus, error) in DispatchQueue.main.async { switch accountStatus { @@ -422,4 +473,56 @@ import CloudKit } }) } + + // MARK: - Retry + + private enum CKErrorResponse { + case success + case failureDoNotRetry(error:Error) + case failureRetryAfterDelay(retryDelay: Double) + case failureRetryWithoutDelay + } + + private class func responseForCloudKitError(error: Error?, + remainingRetries: Int, + label: String) -> CKErrorResponse { + if let error = error as? CKError { + Logger.error("\(self.logTag) \(label) failed: \(error)") + if remainingRetries < 1 { + Logger.verbose("\(self.logTag) \(label) no more retries.") + return .failureDoNotRetry(error:error) + } + + if #available(iOS 11, *) { + if error.code == CKError.serverResponseLost { + Logger.verbose("\(self.logTag) \(label) retry without delay.") + return .failureRetryWithoutDelay + } + } + + switch error { + case CKError.requestRateLimited, CKError.serviceUnavailable, CKError.zoneBusy: + let retryDelay = error.retryAfterSeconds ?? 3.0 + Logger.verbose("\(self.logTag) \(label) retry with delay: \(retryDelay).") + return .failureRetryAfterDelay(retryDelay:retryDelay) + case CKError.networkFailure: + Logger.verbose("\(self.logTag) \(label) retry without delay.") + return .failureRetryWithoutDelay + default: + Logger.verbose("\(self.logTag) \(label) unknown CKError.") + return .failureDoNotRetry(error:error) + } + } else if let error = error { + Logger.error("\(self.logTag) \(label) failed: \(error)") + if remainingRetries < 1 { + Logger.verbose("\(self.logTag) \(label) no more retries.") + return .failureDoNotRetry(error:error) + } + Logger.verbose("\(self.logTag) \(label) unknown error.") + return .failureDoNotRetry(error:error) + } else { + Logger.info("\(self.logTag) \(label) succeeded.") + return .success + } + } } From cf13a780e9e69a4115ff4f0cde62c01ce5a7354e Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Tue, 13 Mar 2018 14:39:00 -0300 Subject: [PATCH 14/19] Retry backup failures. --- Signal/src/util/OWSBackupAPI.swift | 213 +++++++++++++++++++-------- SignalServiceKit/src/Util/OWSError.h | 2 + 2 files changed, 151 insertions(+), 64 deletions(-) diff --git a/Signal/src/util/OWSBackupAPI.swift b/Signal/src/util/OWSBackupAPI.swift index c4140abc5..52f1972f4 100644 --- a/Signal/src/util/OWSBackupAPI.swift +++ b/Signal/src/util/OWSBackupAPI.swift @@ -13,7 +13,7 @@ import CloudKit static let signalBackupRecordType = "signalBackup" static let manifestRecordName = "manifest" static let payloadKey = "payload" - static let maxImmediateRetries = 5 + static let maxRetries = 5 private class func recordIdForTest() -> String { return "test-\(NSUUID().uuidString)" @@ -25,6 +25,12 @@ import CloudKit return privateDatabase } + private class func invalidServiceResponseError() -> Error { + return OWSErrorWithCodeDescription(.backupFailure, + NSLocalizedString("BACKUP_EXPORT_ERROR_INVALID_CLOUDKIT_RESPONSE", + comment: "Error indicating that the app received an invalid response from CloudKit.")) + } + // MARK: - Upload @objc @@ -101,7 +107,7 @@ import CloudKit success: @escaping (String) -> Swift.Void, failure: @escaping (Error) -> Swift.Void) { saveRecordToCloud(record: record, - remainingRetries: maxImmediateRetries, + remainingRetries: maxRetries, success: success, failure: failure) } @@ -137,6 +143,9 @@ import CloudKit success: success, failure: failure) } + case .unknownItem: + owsFail("\(self.logTag) unexpected CloudKit response.") + failure(invalidServiceResponseError()) } } } @@ -154,6 +163,7 @@ import CloudKit failure: @escaping (Error) -> Swift.Void) { checkForFileInCloud(recordName: recordName, + remainingRetries: maxRetries, success: { (record) in if let record = record { // Record found, updating existing record. @@ -187,6 +197,7 @@ import CloudKit failure: @escaping (Error) -> Swift.Void) { checkForFileInCloud(recordName: recordName, + remainingRetries: maxRetries, success: { (record) in if record != nil { // Record found, skipping save. @@ -218,7 +229,7 @@ import CloudKit success: @escaping (Swift.Void) -> Swift.Void, failure: @escaping (Error) -> Swift.Void) { deleteRecordFromCloud(recordName: recordName, - remainingRetries: maxImmediateRetries, + remainingRetries: maxRetries, success: success, failure: failure) } @@ -231,7 +242,7 @@ import CloudKit let recordID = CKRecordID(recordName: recordName) database().delete(withRecordID: recordID) { - (record, error) in + (_, error) in let response = responseForCloudKitError(error: error, remainingRetries: remainingRetries, @@ -255,6 +266,9 @@ import CloudKit success: success, failure: failure) } + case .unknownItem: + owsFail("\(self.logTag) unexpected CloudKit response.") + failure(invalidServiceResponseError()) } } } @@ -262,6 +276,7 @@ import CloudKit // MARK: - Exists? private class func checkForFileInCloud(recordName: String, + remainingRetries: Int, success: @escaping (CKRecord?) -> Swift.Void, failure: @escaping (Error) -> Swift.Void) { let recordId = CKRecordID(recordName: recordName) @@ -270,29 +285,39 @@ import CloudKit // not this record already exists. fetchOperation.desiredKeys = [] fetchOperation.perRecordCompletionBlock = { (record, recordId, error) in - if let error = error { - if let ckerror = error as? CKError { - if ckerror.code == .unknownItem { - // Record not found. - success(nil) - return - } - Logger.error("\(self.logTag) error fetching record: \(error) \(ckerror.code).") - } else { - Logger.error("\(self.logTag) error fetching record: \(error).") + + let response = responseForCloudKitError(error: error, + remainingRetries: remainingRetries, + label: "Check for Record") + switch response { + case .success: + guard let record = record else { + owsFail("\(self.logTag) missing fetching record.") + failure(invalidServiceResponseError()) + return } - failure(error) - return - } - guard let record = record else { - Logger.error("\(self.logTag) missing fetching record.") - failure(OWSErrorWithCodeDescription(.exportBackupError, - NSLocalizedString("BACKUP_EXPORT_ERROR_SAVE_FILE_TO_CLOUD_FAILED", - comment: "Error indicating the a backup export failed to save a file to the cloud."))) - return + // Record found. + success(record) + case .failureDoNotRetry(let responseError): + failure(responseError) + case .failureRetryAfterDelay(let retryDelay): + DispatchQueue.global().asyncAfter(deadline: DispatchTime.now() + retryDelay, execute: { + checkForFileInCloud(recordName: recordName, + remainingRetries: remainingRetries - 1, + success: success, + failure: failure) + }) + case .failureRetryWithoutDelay: + DispatchQueue.global().async { + checkForFileInCloud(recordName: recordName, + remainingRetries: remainingRetries - 1, + success: success, + failure: failure) + } + case .unknownItem: + // Record not found. + success(nil) } - // Record found. - success(record) } database().add(fetchOperation) } @@ -302,6 +327,7 @@ import CloudKit failure: @escaping (Error) -> Swift.Void) { checkForFileInCloud(recordName: manifestRecordName, + remainingRetries: maxRetries, success: { (record) in success(record != nil) }, @@ -317,6 +343,7 @@ import CloudKit fetchAllRecordNamesStep(query: query, previousRecordNames: [String](), cursor: nil, + remainingRetries: maxRetries, success: success, failure: failure) } @@ -324,6 +351,7 @@ import CloudKit private class func fetchAllRecordNamesStep(query: CKQuery, previousRecordNames: [String], cursor: CKQueryCursor?, + remainingRetries: Int, success: @escaping ([String]) -> Swift.Void, failure: @escaping (Error) -> Swift.Void) { @@ -340,23 +368,49 @@ import CloudKit allRecordNames.append(record.recordID.recordName) } queryOperation.queryCompletionBlock = { (cursor, error) in - if let error = error { - Logger.error("\(self.logTag) error fetching all record names: \(error).") - failure(error) - return - } - if let cursor = cursor { - Logger.verbose("\(self.logTag) fetching more record names \(allRecordNames.count).") - // There are more pages of results, continue fetching. - fetchAllRecordNamesStep(query: query, - previousRecordNames: allRecordNames, - cursor: cursor, - success: success, - failure: failure) - return + + let response = responseForCloudKitError(error: error, + remainingRetries: remainingRetries, + label: "Fetch All Records") + switch response { + case .success: + if let cursor = cursor { + Logger.verbose("\(self.logTag) fetching more record names \(allRecordNames.count).") + // There are more pages of results, continue fetching. + fetchAllRecordNamesStep(query: query, + previousRecordNames: allRecordNames, + cursor: cursor, + remainingRetries: maxRetries, + success: success, + failure: failure) + return + } + Logger.info("\(self.logTag) fetched \(allRecordNames.count) record names.") + success(allRecordNames) + case .failureDoNotRetry(let responseError): + failure(responseError) + case .failureRetryAfterDelay(let retryDelay): + DispatchQueue.global().asyncAfter(deadline: DispatchTime.now() + retryDelay, execute: { + fetchAllRecordNamesStep(query: query, + previousRecordNames: allRecordNames, + cursor: cursor, + remainingRetries: remainingRetries - 1, + success: success, + failure: failure) + }) + case .failureRetryWithoutDelay: + DispatchQueue.global().async { + fetchAllRecordNamesStep(query: query, + previousRecordNames: allRecordNames, + cursor: cursor, + remainingRetries: remainingRetries - 1, + success: success, + failure: failure) + } + case .unknownItem: + owsFail("\(self.logTag) unexpected CloudKit response.") + failure(invalidServiceResponseError()) } - Logger.info("\(self.logTag) fetched \(allRecordNames.count) record names.") - success(allRecordNames) } database().add(queryOperation) } @@ -378,6 +432,7 @@ import CloudKit failure: @escaping (Error) -> Swift.Void) { downloadFromCloud(recordName: recordName, + remainingRetries: maxRetries, success: { (asset) in DispatchQueue.global().async { do { @@ -385,9 +440,7 @@ import CloudKit success(data) } catch { Logger.error("\(self.logTag) couldn't load asset file: \(error).") - failure(OWSErrorWithCodeDescription(.exportBackupError, - NSLocalizedString("BACKUP_IMPORT_ERROR_DOWNLOAD_FILE_FROM_CLOUD_FAILED", - comment: "Error indicating the a backup import failed to download a file from the cloud."))) + failure(invalidServiceResponseError()) } } }, @@ -401,6 +454,7 @@ import CloudKit failure: @escaping (Error) -> Swift.Void) { downloadFromCloud(recordName: recordName, + remainingRetries: maxRetries, success: { (asset) in DispatchQueue.global().async { do { @@ -408,16 +462,20 @@ import CloudKit success() } catch { Logger.error("\(self.logTag) couldn't copy asset file: \(error).") - failure(OWSErrorWithCodeDescription(.exportBackupError, - NSLocalizedString("BACKUP_IMPORT_ERROR_DOWNLOAD_FILE_FROM_CLOUD_FAILED", - comment: "Error indicating the a backup import failed to download a file from the cloud."))) + failure(invalidServiceResponseError()) } } }, failure: failure) } + // We return the CKAsset and not its fileUrl because + // CloudKit offers no guarantees around how long it'll + // keep around the underlying file. Presumably we can + // defer cleanup by maintaining a strong reference to + // the asset. private class func downloadFromCloud(recordName: String, + remainingRetries: Int, success: @escaping (CKAsset) -> Swift.Void, failure: @escaping (Error) -> Swift.Void) { @@ -425,25 +483,43 @@ import CloudKit let fetchOperation = CKFetchRecordsOperation(recordIDs: [recordId ]) // Download all keys for this record. fetchOperation.perRecordCompletionBlock = { (record, recordId, error) in - if let error = error { - failure(error) - return - } - guard let record = record else { + + let response = responseForCloudKitError(error: error, + remainingRetries: remainingRetries, + label: "Download Record") + switch response { + case .success: + guard let record = record else { + Logger.error("\(self.logTag) missing fetching record.") + failure(invalidServiceResponseError()) + return + } + guard let asset = record[payloadKey] as? CKAsset else { + Logger.error("\(self.logTag) record missing payload.") + failure(invalidServiceResponseError()) + return + } + success(asset) + case .failureDoNotRetry(let responseError): + failure(responseError) + case .failureRetryAfterDelay(let retryDelay): + DispatchQueue.global().asyncAfter(deadline: DispatchTime.now() + retryDelay, execute: { + downloadFromCloud(recordName: recordName, + remainingRetries: remainingRetries - 1, + success: success, + failure: failure) + }) + case .failureRetryWithoutDelay: + DispatchQueue.global().async { + downloadFromCloud(recordName: recordName, + remainingRetries: remainingRetries - 1, + success: success, + failure: failure) + } + case .unknownItem: Logger.error("\(self.logTag) missing fetching record.") - failure(OWSErrorWithCodeDescription(.exportBackupError, - NSLocalizedString("BACKUP_IMPORT_ERROR_DOWNLOAD_FILE_FROM_CLOUD_FAILED", - comment: "Error indicating the a backup import failed to download a file from the cloud."))) - return + failure(invalidServiceResponseError()) } - guard let asset = record[payloadKey] as? CKAsset else { - Logger.error("\(self.logTag) record missing payload.") - failure(OWSErrorWithCodeDescription(.exportBackupError, - NSLocalizedString("BACKUP_IMPORT_ERROR_DOWNLOAD_FILE_FROM_CLOUD_FAILED", - comment: "Error indicating the a backup import failed to download a file from the cloud."))) - return - } - success(asset) } database().add(fetchOperation) } @@ -481,13 +557,22 @@ import CloudKit case failureDoNotRetry(error:Error) case failureRetryAfterDelay(retryDelay: Double) case failureRetryWithoutDelay + // This only applies to fetches. + case unknownItem } private class func responseForCloudKitError(error: Error?, remainingRetries: Int, label: String) -> CKErrorResponse { if let error = error as? CKError { + if error.code == CKError.unknownItem { + // This is not always an error for our purposes. + Logger.verbose("\(self.logTag) \(label) unknown item.") + return .unknownItem + } + Logger.error("\(self.logTag) \(label) failed: \(error)") + if remainingRetries < 1 { Logger.verbose("\(self.logTag) \(label) no more retries.") return .failureDoNotRetry(error:error) diff --git a/SignalServiceKit/src/Util/OWSError.h b/SignalServiceKit/src/Util/OWSError.h index 29c9bfa7c..0a06d88ca 100644 --- a/SignalServiceKit/src/Util/OWSError.h +++ b/SignalServiceKit/src/Util/OWSError.h @@ -39,6 +39,8 @@ typedef NS_ENUM(NSInteger, OWSErrorCode) { OWSErrorCodeImportBackupFailed = 777417, // A possibly recoverable error occured while importing a backup. OWSErrorCodeImportBackupError = 777418, + // A non-recoverable while importing or exporting a backup. + OWSErrorCodeBackupFailure = 777419, }; extern NSString *const OWSErrorRecipientIdentifierKey; From b0d56dcd556d547ab9c428b6fd11190bfa3e452c Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Tue, 13 Mar 2018 14:39:38 -0300 Subject: [PATCH 15/19] Clean up ahead of PR. --- .../ViewControllers/AppSettings/AppSettingsViewController.m | 5 ----- Signal/src/ViewControllers/HomeViewController.m | 4 ---- 2 files changed, 9 deletions(-) diff --git a/Signal/src/ViewControllers/AppSettings/AppSettingsViewController.m b/Signal/src/ViewControllers/AppSettings/AppSettingsViewController.m index 15dfcd95b..26f94e5e9 100644 --- a/Signal/src/ViewControllers/AppSettings/AppSettingsViewController.m +++ b/Signal/src/ViewControllers/AppSettings/AppSettingsViewController.m @@ -90,11 +90,6 @@ self.title = NSLocalizedString(@"SETTINGS_NAV_BAR_TITLE", @"Title for settings activity"); [self updateTableContents]; - - dispatch_async(dispatch_get_main_queue(), ^{ - // [self showBackup]; - [self showDebugUI]; - }); } - (void)viewWillAppear:(BOOL)animated diff --git a/Signal/src/ViewControllers/HomeViewController.m b/Signal/src/ViewControllers/HomeViewController.m index 497a898c1..dccd63e15 100644 --- a/Signal/src/ViewControllers/HomeViewController.m +++ b/Signal/src/ViewControllers/HomeViewController.m @@ -284,10 +284,6 @@ typedef NS_ENUM(NSInteger, CellState) { kArchiveState, kInboxState }; } [self updateBarButtonItems]; - - dispatch_async(dispatch_get_main_queue(), ^{ - [self settingsButtonPressed:nil]; - }); } - (void)viewDidAppear:(BOOL)animated From 54eecd5b1d2f2aff7a64174ce4d7dcbbc284a973 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Tue, 13 Mar 2018 15:57:06 -0300 Subject: [PATCH 16/19] Protect backup directories. --- Signal/src/util/OWSBackupJob.m | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Signal/src/util/OWSBackupJob.m b/Signal/src/util/OWSBackupJob.m index eda6a4a4a..84ba42b56 100644 --- a/Signal/src/util/OWSBackupJob.m +++ b/Signal/src/util/OWSBackupJob.m @@ -79,6 +79,10 @@ NSString *const kOWSBackup_Snapshot_ValidKey = @"kOWSBackup_Snapshot_ValidKey"; OWSProdLogAndFail(@"%@ Could not create jobTempDirPath.", self.logTag); return NO; } + if (![OWSFileSystem protectFileOrFolderAtPath:self.jobTempDirPath]) { + OWSProdLogAndFail(@"%@ Could not protect jobTempDirPath.", self.logTag); + return NO; + } return YES; } From 62da17a0ccb7970cc2244515f9f6e0fd84af09c6 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Wed, 14 Mar 2018 10:12:20 -0300 Subject: [PATCH 17/19] Clean up ahead of PR. --- Signal/src/util/OWSBackupAPI.swift | 128 ++++++++++++++--------------- 1 file changed, 64 insertions(+), 64 deletions(-) diff --git a/Signal/src/util/OWSBackupAPI.swift b/Signal/src/util/OWSBackupAPI.swift index 52f1972f4..331f40f3d 100644 --- a/Signal/src/util/OWSBackupAPI.swift +++ b/Signal/src/util/OWSBackupAPI.swift @@ -35,8 +35,8 @@ import CloudKit @objc public class func saveTestFileToCloud(fileUrl: URL, - success: @escaping (String) -> Swift.Void, - failure: @escaping (Error) -> Swift.Void) { + success: @escaping (String) -> Void, + failure: @escaping (Error) -> Void) { saveFileToCloud(fileUrl: fileUrl, recordName: NSUUID().uuidString, recordType: signalBackupRecordType, @@ -50,8 +50,8 @@ import CloudKit // complete. @objc public class func saveEphemeralDatabaseFileToCloud(fileUrl: URL, - success: @escaping (String) -> Swift.Void, - failure: @escaping (Error) -> Swift.Void) { + success: @escaping (String) -> Void, + failure: @escaping (Error) -> Void) { saveFileToCloud(fileUrl: fileUrl, recordName: "ephemeralFile-\(NSUUID().uuidString)", recordType: signalBackupRecordType, @@ -64,9 +64,9 @@ import CloudKit // backups can reuse the same record. @objc public class func savePersistentFileOnceToCloud(fileId: String, - fileUrlBlock: @escaping (Swift.Void) -> URL?, - success: @escaping (String) -> Swift.Void, - failure: @escaping (Error) -> Swift.Void) { + fileUrlBlock: @escaping (()) -> URL?, + success: @escaping (String) -> Void, + failure: @escaping (Error) -> Void) { saveFileOnceToCloud(recordName: "persistentFile-\(fileId)", recordType: signalBackupRecordType, fileUrlBlock: fileUrlBlock, @@ -76,8 +76,8 @@ import CloudKit @objc public class func upsertManifestFileToCloud(fileUrl: URL, - success: @escaping (String) -> Swift.Void, - failure: @escaping (Error) -> Swift.Void) { + success: @escaping (String) -> Void, + failure: @escaping (Error) -> Void) { // We want to use a well-known record id and type for manifest files. upsertFileToCloud(fileUrl: fileUrl, recordName: manifestRecordName, @@ -90,8 +90,8 @@ import CloudKit public class func saveFileToCloud(fileUrl: URL, recordName: String, recordType: String, - success: @escaping (String) -> Swift.Void, - failure: @escaping (Error) -> Swift.Void) { + success: @escaping (String) -> Void, + failure: @escaping (Error) -> Void) { let recordID = CKRecordID(recordName: recordName) let record = CKRecord(recordType: recordType, recordID: recordID) let asset = CKAsset(fileURL: fileUrl) @@ -104,8 +104,8 @@ import CloudKit @objc public class func saveRecordToCloud(record: CKRecord, - success: @escaping (String) -> Swift.Void, - failure: @escaping (Error) -> Swift.Void) { + success: @escaping (String) -> Void, + failure: @escaping (Error) -> Void) { saveRecordToCloud(record: record, remainingRetries: maxRetries, success: success, @@ -114,21 +114,21 @@ import CloudKit private class func saveRecordToCloud(record: CKRecord, remainingRetries: Int, - success: @escaping (String) -> Swift.Void, - failure: @escaping (Error) -> Swift.Void) { + success: @escaping (String) -> Void, + failure: @escaping (Error) -> Void) { database().save(record) { (_, error) in - let response = responseForCloudKitError(error: error, + let outcome = outcomeForCloudKitError(error: error, remainingRetries: remainingRetries, label: "Save Record") - switch response { + switch outcome { case .success: let recordName = record.recordID.recordName success(recordName) - case .failureDoNotRetry(let responseError): - failure(responseError) + case .failureDoNotRetry(let outcomeError): + failure(outcomeError) case .failureRetryAfterDelay(let retryDelay): DispatchQueue.global().asyncAfter(deadline: DispatchTime.now() + retryDelay, execute: { saveRecordToCloud(record: record, @@ -159,8 +159,8 @@ import CloudKit public class func upsertFileToCloud(fileUrl: URL, recordName: String, recordType: String, - success: @escaping (String) -> Swift.Void, - failure: @escaping (Error) -> Swift.Void) { + success: @escaping (String) -> Void, + failure: @escaping (Error) -> Void) { checkForFileInCloud(recordName: recordName, remainingRetries: maxRetries, @@ -192,9 +192,9 @@ import CloudKit @objc public class func saveFileOnceToCloud(recordName: String, recordType: String, - fileUrlBlock: @escaping (Swift.Void) -> URL?, - success: @escaping (String) -> Swift.Void, - failure: @escaping (Error) -> Swift.Void) { + fileUrlBlock: @escaping (()) -> URL?, + success: @escaping (String) -> Void, + failure: @escaping (Error) -> Void) { checkForFileInCloud(recordName: recordName, remainingRetries: maxRetries, @@ -226,8 +226,8 @@ import CloudKit @objc public class func deleteRecordFromCloud(recordName: String, - success: @escaping (Swift.Void) -> Swift.Void, - failure: @escaping (Error) -> Swift.Void) { + success: @escaping (()) -> Void, + failure: @escaping (Error) -> Void) { deleteRecordFromCloud(recordName: recordName, remainingRetries: maxRetries, success: success, @@ -236,22 +236,22 @@ import CloudKit private class func deleteRecordFromCloud(recordName: String, remainingRetries: Int, - success: @escaping (Swift.Void) -> Swift.Void, - failure: @escaping (Error) -> Swift.Void) { + success: @escaping (()) -> Void, + failure: @escaping (Error) -> Void) { let recordID = CKRecordID(recordName: recordName) database().delete(withRecordID: recordID) { (_, error) in - let response = responseForCloudKitError(error: error, + let outcome = outcomeForCloudKitError(error: error, remainingRetries: remainingRetries, label: "Delete Record") - switch response { + switch outcome { case .success: success() - case .failureDoNotRetry(let responseError): - failure(responseError) + case .failureDoNotRetry(let outcomeError): + failure(outcomeError) case .failureRetryAfterDelay(let retryDelay): DispatchQueue.global().asyncAfter(deadline: DispatchTime.now() + retryDelay, execute: { deleteRecordFromCloud(recordName: recordName, @@ -277,8 +277,8 @@ import CloudKit private class func checkForFileInCloud(recordName: String, remainingRetries: Int, - success: @escaping (CKRecord?) -> Swift.Void, - failure: @escaping (Error) -> Swift.Void) { + success: @escaping (CKRecord?) -> Void, + failure: @escaping (Error) -> Void) { let recordId = CKRecordID(recordName: recordName) let fetchOperation = CKFetchRecordsOperation(recordIDs: [recordId ]) // Don't download the file; we're just using the fetch to check whether or @@ -286,10 +286,10 @@ import CloudKit fetchOperation.desiredKeys = [] fetchOperation.perRecordCompletionBlock = { (record, recordId, error) in - let response = responseForCloudKitError(error: error, + let outcome = outcomeForCloudKitError(error: error, remainingRetries: remainingRetries, label: "Check for Record") - switch response { + switch outcome { case .success: guard let record = record else { owsFail("\(self.logTag) missing fetching record.") @@ -298,8 +298,8 @@ import CloudKit } // Record found. success(record) - case .failureDoNotRetry(let responseError): - failure(responseError) + case .failureDoNotRetry(let outcomeError): + failure(outcomeError) case .failureRetryAfterDelay(let retryDelay): DispatchQueue.global().asyncAfter(deadline: DispatchTime.now() + retryDelay, execute: { checkForFileInCloud(recordName: recordName, @@ -323,8 +323,8 @@ import CloudKit } @objc - public class func checkForManifestInCloud(success: @escaping (Bool) -> Swift.Void, - failure: @escaping (Error) -> Swift.Void) { + public class func checkForManifestInCloud(success: @escaping (Bool) -> Void, + failure: @escaping (Error) -> Void) { checkForFileInCloud(recordName: manifestRecordName, remainingRetries: maxRetries, @@ -335,8 +335,8 @@ import CloudKit } @objc - public class func fetchAllRecordNames(success: @escaping ([String]) -> Swift.Void, - failure: @escaping (Error) -> Swift.Void) { + public class func fetchAllRecordNames(success: @escaping ([String]) -> Void, + failure: @escaping (Error) -> Void) { let query = CKQuery(recordType: signalBackupRecordType, predicate: NSPredicate(value: true)) // Fetch the first page of results for this query. @@ -352,8 +352,8 @@ import CloudKit previousRecordNames: [String], cursor: CKQueryCursor?, remainingRetries: Int, - success: @escaping ([String]) -> Swift.Void, - failure: @escaping (Error) -> Swift.Void) { + success: @escaping ([String]) -> Void, + failure: @escaping (Error) -> Void) { var allRecordNames = previousRecordNames @@ -369,10 +369,10 @@ import CloudKit } queryOperation.queryCompletionBlock = { (cursor, error) in - let response = responseForCloudKitError(error: error, + let outcome = outcomeForCloudKitError(error: error, remainingRetries: remainingRetries, label: "Fetch All Records") - switch response { + switch outcome { case .success: if let cursor = cursor { Logger.verbose("\(self.logTag) fetching more record names \(allRecordNames.count).") @@ -387,8 +387,8 @@ import CloudKit } Logger.info("\(self.logTag) fetched \(allRecordNames.count) record names.") success(allRecordNames) - case .failureDoNotRetry(let responseError): - failure(responseError) + case .failureDoNotRetry(let outcomeError): + failure(outcomeError) case .failureRetryAfterDelay(let retryDelay): DispatchQueue.global().asyncAfter(deadline: DispatchTime.now() + retryDelay, execute: { fetchAllRecordNamesStep(query: query, @@ -419,8 +419,8 @@ import CloudKit @objc public class func downloadManifestFromCloud( - success: @escaping (Data) -> Swift.Void, - failure: @escaping (Error) -> Swift.Void) { + success: @escaping (Data) -> Void, + failure: @escaping (Error) -> Void) { downloadDataFromCloud(recordName: manifestRecordName, success: success, failure: failure) @@ -428,8 +428,8 @@ import CloudKit @objc public class func downloadDataFromCloud(recordName: String, - success: @escaping (Data) -> Swift.Void, - failure: @escaping (Error) -> Swift.Void) { + success: @escaping (Data) -> Void, + failure: @escaping (Error) -> Void) { downloadFromCloud(recordName: recordName, remainingRetries: maxRetries, @@ -450,8 +450,8 @@ import CloudKit @objc public class func downloadFileFromCloud(recordName: String, toFileUrl: URL, - success: @escaping (Swift.Void) -> Swift.Void, - failure: @escaping (Error) -> Swift.Void) { + success: @escaping (()) -> Void, + failure: @escaping (Error) -> Void) { downloadFromCloud(recordName: recordName, remainingRetries: maxRetries, @@ -476,18 +476,18 @@ import CloudKit // the asset. private class func downloadFromCloud(recordName: String, remainingRetries: Int, - success: @escaping (CKAsset) -> Swift.Void, - failure: @escaping (Error) -> Swift.Void) { + success: @escaping (CKAsset) -> Void, + failure: @escaping (Error) -> Void) { let recordId = CKRecordID(recordName: recordName) let fetchOperation = CKFetchRecordsOperation(recordIDs: [recordId ]) // Download all keys for this record. fetchOperation.perRecordCompletionBlock = { (record, recordId, error) in - let response = responseForCloudKitError(error: error, + let outcome = outcomeForCloudKitError(error: error, remainingRetries: remainingRetries, label: "Download Record") - switch response { + switch outcome { case .success: guard let record = record else { Logger.error("\(self.logTag) missing fetching record.") @@ -500,8 +500,8 @@ import CloudKit return } success(asset) - case .failureDoNotRetry(let responseError): - failure(responseError) + case .failureDoNotRetry(let outcomeError): + failure(outcomeError) case .failureRetryAfterDelay(let retryDelay): DispatchQueue.global().asyncAfter(deadline: DispatchTime.now() + retryDelay, execute: { downloadFromCloud(recordName: recordName, @@ -527,7 +527,7 @@ import CloudKit // MARK: - Access @objc - public class func checkCloudKitAccess(completion: @escaping (Bool) -> Swift.Void) { + public class func checkCloudKitAccess(completion: @escaping (Bool) -> Void) { CKContainer.default().accountStatus(completionHandler: { (accountStatus, error) in DispatchQueue.main.async { switch accountStatus { @@ -552,7 +552,7 @@ import CloudKit // MARK: - Retry - private enum CKErrorResponse { + private enum CKOutcome { case success case failureDoNotRetry(error:Error) case failureRetryAfterDelay(retryDelay: Double) @@ -561,9 +561,9 @@ import CloudKit case unknownItem } - private class func responseForCloudKitError(error: Error?, + private class func outcomeForCloudKitError(error: Error?, remainingRetries: Int, - label: String) -> CKErrorResponse { + label: String) -> CKOutcome { if let error = error as? CKError { if error.code == CKError.unknownItem { // This is not always an error for our purposes. From 0ba47808a8b45b586063e16038c71efb68ec0ee8 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Wed, 14 Mar 2018 10:26:53 -0300 Subject: [PATCH 18/19] Clean up ahead of PR. --- SignalServiceKit/src/Storage/OWSStorage.m | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/SignalServiceKit/src/Storage/OWSStorage.m b/SignalServiceKit/src/Storage/OWSStorage.m index de7ae50c3..0b84a0e78 100644 --- a/SignalServiceKit/src/Storage/OWSStorage.m +++ b/SignalServiceKit/src/Storage/OWSStorage.m @@ -74,7 +74,7 @@ typedef NSData *_Nullable (^CreateDatabaseMetadataBlock)(void); { id delegate = self.delegate; OWSAssert(delegate); - // OWSAssert(delegate.areAllRegistrationsComplete || self.canWriteBeforeStorageReady); + OWSAssert(delegate.areAllRegistrationsComplete || self.canWriteBeforeStorageReady); OWSBackgroundTask *_Nullable backgroundTask = nil; if (CurrentAppContext().isMainApp) { From 24cc95585f3919639ef2400e9c83609b2dfdc78d Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Sat, 17 Mar 2018 08:50:09 -0300 Subject: [PATCH 19/19] Respond to CR. --- .../AppSettings/OWSBackupSettingsViewController.m | 11 ++++++----- Signal/src/util/OWSBackupAPI.swift | 6 +++--- Signal/translations/en.lproj/Localizable.strings | 9 +++------ 3 files changed, 12 insertions(+), 14 deletions(-) diff --git a/Signal/src/ViewControllers/AppSettings/OWSBackupSettingsViewController.m b/Signal/src/ViewControllers/AppSettings/OWSBackupSettingsViewController.m index f8624f8da..a2569aa2c 100644 --- a/Signal/src/ViewControllers/AppSettings/OWSBackupSettingsViewController.m +++ b/Signal/src/ViewControllers/AppSettings/OWSBackupSettingsViewController.m @@ -88,15 +88,16 @@ NS_ASSUME_NONNULL_BEGIN if (OWSBackup.sharedManager.backupExportProgress) { NSUInteger progressPercent = (NSUInteger)round(OWSBackup.sharedManager.backupExportProgress.floatValue * 100); + NSNumberFormatter *numberFormatter = [[NSNumberFormatter alloc] init]; + [numberFormatter setNumberStyle:NSNumberFormatterPercentStyle]; + [numberFormatter setMaximumFractionDigits:0]; + [numberFormatter setMultiplier:@1]; + NSString *progressString = [numberFormatter stringFromNumber:@(progressPercent)]; [progressSection addItem:[OWSTableItem labelItemWithText:NSLocalizedString(@"SETTINGS_BACKUP_PROGRESS", @"Label for phase row in the in the backup settings view.") - accessoryText:[NSString - stringWithFormat:NSLocalizedString(@"PERCENTAGE_FORMAT", - @"Format for percentages, e.g. 65%. " - @"Embeds {{percentage}}, e.g. 65."), - @(progressPercent).stringValue]]]; + accessoryText:progressString]]; } } } diff --git a/Signal/src/util/OWSBackupAPI.swift b/Signal/src/util/OWSBackupAPI.swift index 331f40f3d..79344a34e 100644 --- a/Signal/src/util/OWSBackupAPI.swift +++ b/Signal/src/util/OWSBackupAPI.swift @@ -552,10 +552,10 @@ import CloudKit // MARK: - Retry - private enum CKOutcome { + private enum APIOutcome { case success case failureDoNotRetry(error:Error) - case failureRetryAfterDelay(retryDelay: Double) + case failureRetryAfterDelay(retryDelay: TimeInterval) case failureRetryWithoutDelay // This only applies to fetches. case unknownItem @@ -563,7 +563,7 @@ import CloudKit private class func outcomeForCloudKitError(error: Error?, remainingRetries: Int, - label: String) -> CKOutcome { + label: String) -> APIOutcome { if let error = error as? CKError { if error.code == CKError.unknownItem { // This is not always an error for our purposes. diff --git a/Signal/translations/en.lproj/Localizable.strings b/Signal/translations/en.lproj/Localizable.strings index 7a89b1173..0278713a1 100644 --- a/Signal/translations/en.lproj/Localizable.strings +++ b/Signal/translations/en.lproj/Localizable.strings @@ -160,6 +160,9 @@ /* Error indicating the a backup export could not export the user's data. */ "BACKUP_EXPORT_ERROR_COULD_NOT_EXPORT" = "Backup data could be exported."; +/* Error indicating that the app received an invalid response from CloudKit. */ +"BACKUP_EXPORT_ERROR_INVALID_CLOUDKIT_RESPONSE" = "Invalid server response."; + /* Error indicating the a backup export failed to save a file to the cloud. */ "BACKUP_EXPORT_ERROR_SAVE_FILE_TO_CLOUD_FAILED" = "Backup could not upload data."; @@ -178,9 +181,6 @@ /* Error indicating the a backup import could not import the user's data. */ "BACKUP_IMPORT_ERROR_COULD_NOT_IMPORT" = "Backup could not be imported."; -/* Error indicating the a backup import failed to download a file from the cloud. */ -"BACKUP_IMPORT_ERROR_DOWNLOAD_FILE_FROM_CLOUD_FAILED" = "Could not download backup data."; - /* Indicates that the backup import is being configured. */ "BACKUP_IMPORT_PHASE_CONFIGURATION" = "Configuring Backup"; @@ -1225,9 +1225,6 @@ /* A display format for oversize text messages. */ "OVERSIZE_TEXT_DISPLAY_FORMAT" = "%@…"; -/* Format for percentages, e.g. 65%. Embeds {{percentage}}, e.g. 65. */ -"PERCENTAGE_FORMAT" = "%@%%"; - /* A format for a label showing an example phone number. Embeds {{the example phone number}}. */ "PHONE_NUMBER_EXAMPLE_FORMAT" = "Example: %@";