From 5f8755f2eb94b4ac52d2857135dabb8e2f7dffaa Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Wed, 21 Nov 2018 09:25:24 -0800 Subject: [PATCH] Respond to CR. --- Signal/src/environment/AppEnvironment.swift | 3 +- Signal/src/util/Backup/OWSBackup.h | 4 +-- Signal/src/util/Backup/OWSBackup.m | 36 ++++++++++++------- Signal/src/util/Backup/OWSBackupExportJob.m | 6 ++-- .../util/Backup/OWSBackupLazyRestoreJob.swift | 3 +- .../src/Messages/Attachments/TSAttachment.h | 10 +++--- .../src/Messages/Attachments/TSAttachment.m | 14 ++++---- .../Attachments/TSAttachmentPointer.h | 3 +- .../Attachments/TSAttachmentPointer.m | 12 +++---- 9 files changed, 52 insertions(+), 39 deletions(-) diff --git a/Signal/src/environment/AppEnvironment.swift b/Signal/src/environment/AppEnvironment.swift index 1c70054b6..44368d989 100644 --- a/Signal/src/environment/AppEnvironment.swift +++ b/Signal/src/environment/AppEnvironment.swift @@ -69,8 +69,7 @@ import SignalMessaging self.pushRegistrationManager = PushRegistrationManager() self.pushManager = PushManager() self.sessionResetJobQueue = SessionResetJobQueue() - assert(SSKEnvironment.shared.primaryStorage != nil) - self.backup = OWSBackup(primaryStorage: SSKEnvironment.shared.primaryStorage) + self.backup = OWSBackup() super.init() diff --git a/Signal/src/util/Backup/OWSBackup.h b/Signal/src/util/Backup/OWSBackup.h index 6894a33d1..0f0823b0f 100644 --- a/Signal/src/util/Backup/OWSBackup.h +++ b/Signal/src/util/Backup/OWSBackup.h @@ -21,14 +21,12 @@ typedef NS_ENUM(NSUInteger, OWSBackupState) { }; @class OWSBackupIO; -@class OWSPrimaryStorage; @class TSAttachmentPointer; @class TSThread; @interface OWSBackup : NSObject -- (instancetype)init NS_UNAVAILABLE; -- (instancetype)initWithPrimaryStorage:(OWSPrimaryStorage *)primaryStorage NS_DESIGNATED_INITIALIZER; +- (instancetype)init NS_DESIGNATED_INITIALIZER; + (instancetype)sharedManager NS_SWIFT_NAME(shared()); diff --git a/Signal/src/util/Backup/OWSBackup.m b/Signal/src/util/Backup/OWSBackup.m index e470f6a58..8e3ea7a93 100644 --- a/Signal/src/util/Backup/OWSBackup.m +++ b/Signal/src/util/Backup/OWSBackup.m @@ -51,7 +51,7 @@ NS_ASSUME_NONNULL_BEGIN return AppEnvironment.shared.backup; } -- (instancetype)initWithPrimaryStorage:(OWSPrimaryStorage *)primaryStorage +- (instancetype)init { self = [super init]; @@ -59,10 +59,6 @@ NS_ASSUME_NONNULL_BEGIN return self; } - OWSAssertDebug(primaryStorage); - - _dbConnection = primaryStorage.newDatabaseConnection; - _backupExportState = OWSBackupState_Idle; _backupImportState = OWSBackupState_Idle; @@ -101,6 +97,25 @@ NS_ASSUME_NONNULL_BEGIN }); } +- (YapDatabaseConnection *)dbConnection +{ + @synchronized(self) { + if (!_dbConnection) { + _dbConnection = self.primaryStorage.newDatabaseConnection; + } + return _dbConnection; + } +} + +#pragma mark - Dependencies + +- (OWSPrimaryStorage *)primaryStorage +{ + OWSAssertDebug(SSKEnvironment.shared.primaryStorage); + + return SSKEnvironment.shared.primaryStorage; +} + #pragma mark - Backup Export - (void)tryToExportBackup @@ -121,8 +136,7 @@ NS_ASSUME_NONNULL_BEGIN _backupExportState = OWSBackupState_InProgress; - self.backupExportJob = - [[OWSBackupExportJob alloc] initWithDelegate:self primaryStorage:[OWSPrimaryStorage sharedManager]]; + self.backupExportJob = [[OWSBackupExportJob alloc] initWithDelegate:self primaryStorage:self.primaryStorage]; [self.backupExportJob startAsync]; [self postDidChangeNotification]; @@ -254,8 +268,7 @@ NS_ASSUME_NONNULL_BEGIN [self.backupExportJob cancel]; self.backupExportJob = nil; } else if (self.shouldHaveBackupExport && !self.backupExportJob) { - self.backupExportJob = - [[OWSBackupExportJob alloc] initWithDelegate:self primaryStorage:[OWSPrimaryStorage sharedManager]]; + self.backupExportJob = [[OWSBackupExportJob alloc] initWithDelegate:self primaryStorage:self.primaryStorage]; [self.backupExportJob startAsync]; } @@ -321,8 +334,7 @@ NS_ASSUME_NONNULL_BEGIN _backupImportState = OWSBackupState_InProgress; - self.backupImportJob = - [[OWSBackupImportJob alloc] initWithDelegate:self primaryStorage:[OWSPrimaryStorage sharedManager]]; + self.backupImportJob = [[OWSBackupImportJob alloc] initWithDelegate:self primaryStorage:self.primaryStorage]; [self.backupImportJob startAsync]; [self postDidChangeNotification]; @@ -619,7 +631,7 @@ NS_ASSUME_NONNULL_BEGIN } if (![OWSFileSystem deleteFileIfExists:attachmentFilePath]) { - OWSLogError(@"Couldn't delete exist file at attachment path."); + OWSFailDebug(@"Couldn't delete existing file at attachment path."); return completion(NO); } diff --git a/Signal/src/util/Backup/OWSBackupExportJob.m b/Signal/src/util/Backup/OWSBackupExportJob.m index 21c9f6e75..9a052dfd5 100644 --- a/Signal/src/util/Backup/OWSBackupExportJob.m +++ b/Signal/src/util/Backup/OWSBackupExportJob.m @@ -547,8 +547,10 @@ NS_ASSUME_NONNULL_BEGIN [TSAttachment class], ^(id object) { if (![object isKindOfClass:[TSAttachmentStream class]]) { - // No need to backup the contents of attachment pointers. - return NO; + // No need to backup the contents (e.g. the file on disk) + // of attachment pointers. + // After a restore, users will be able "tap to retry". + return YES; } TSAttachmentStream *attachmentStream = object; NSString *_Nullable filePath = attachmentStream.originalFilePath; diff --git a/Signal/src/util/Backup/OWSBackupLazyRestoreJob.swift b/Signal/src/util/Backup/OWSBackupLazyRestoreJob.swift index 2aace7dd1..e58189cce 100644 --- a/Signal/src/util/Backup/OWSBackupLazyRestoreJob.swift +++ b/Signal/src/util/Backup/OWSBackupLazyRestoreJob.swift @@ -68,8 +68,7 @@ public class OWSBackupLazyRestoreJob: NSObject { return } attachmentIdsCopy.removeLast() - guard let attachment = TSAttachment.fetch(uniqueId: attachmentId), - let attachmentPointer = attachment as? TSAttachmentPointer else { + guard let attachmentPointer = TSAttachment.fetch(uniqueId: attachmentId) as? TSAttachmentPointer else { Logger.warn("could not load attachment.") // Not necessarily an error. // The attachment might have been deleted since the job began. diff --git a/SignalServiceKit/src/Messages/Attachments/TSAttachment.h b/SignalServiceKit/src/Messages/Attachments/TSAttachment.h index 9e50d6c1d..01bb3c40f 100644 --- a/SignalServiceKit/src/Messages/Attachments/TSAttachment.h +++ b/SignalServiceKit/src/Messages/Attachments/TSAttachment.h @@ -65,11 +65,11 @@ typedef NS_ENUM(NSUInteger, TSAttachmentType) { // This constructor is used for new instances of TSAttachmentPointer, // i.e. undownloaded restoring attachments. -- (instancetype)initWithUniqueId:(NSString *)uniqueId - contentType:(NSString *)contentType - sourceFilename:(nullable NSString *)sourceFilename - caption:(nullable NSString *)caption - albumMessageId:(nullable NSString *)albumMessageId; +- (instancetype)initForRestoreWithUniqueId:(NSString *)uniqueId + contentType:(NSString *)contentType + sourceFilename:(nullable NSString *)sourceFilename + caption:(nullable NSString *)caption + albumMessageId:(nullable NSString *)albumMessageId; // This constructor is used for new instances of TSAttachmentStream // that represent new, un-uploaded outgoing attachments. diff --git a/SignalServiceKit/src/Messages/Attachments/TSAttachment.m b/SignalServiceKit/src/Messages/Attachments/TSAttachment.m index 3f7c1c22e..22955fc29 100644 --- a/SignalServiceKit/src/Messages/Attachments/TSAttachment.m +++ b/SignalServiceKit/src/Messages/Attachments/TSAttachment.m @@ -68,11 +68,11 @@ NSUInteger const TSAttachmentSchemaVersion = 4; // This constructor is used for new instances of TSAttachmentPointer, // i.e. undownloaded restoring attachments. -- (instancetype)initWithUniqueId:(NSString *)uniqueId - contentType:(NSString *)contentType - sourceFilename:(nullable NSString *)sourceFilename - caption:(nullable NSString *)caption - albumMessageId:(nullable NSString *)albumMessageId +- (instancetype)initForRestoreWithUniqueId:(NSString *)uniqueId + contentType:(NSString *)contentType + sourceFilename:(nullable NSString *)sourceFilename + caption:(nullable NSString *)caption + albumMessageId:(nullable NSString *)albumMessageId { OWSAssertDebug(uniqueId.length > 0); if (contentType.length < 1) { @@ -82,7 +82,9 @@ NSUInteger const TSAttachmentSchemaVersion = 4; } OWSAssertDebug(contentType.length > 0); - // Once saved, this AttachmentStream will replace the AttachmentPointer in the attachments collection. + // If saved, this AttachmentPointer would replace the AttachmentStream in the attachments collection. + // However we only use this AttachmentPointer should only be used during the export process so it + // won't be saved until we restore the backup (when there will be no AttachmentStream to replace). self = [super initWithUniqueId:uniqueId]; if (!self) { return self; diff --git a/SignalServiceKit/src/Messages/Attachments/TSAttachmentPointer.h b/SignalServiceKit/src/Messages/Attachments/TSAttachmentPointer.h index 156c88d7d..a4a19bd28 100644 --- a/SignalServiceKit/src/Messages/Attachments/TSAttachmentPointer.h +++ b/SignalServiceKit/src/Messages/Attachments/TSAttachmentPointer.h @@ -12,7 +12,8 @@ NS_ASSUME_NONNULL_BEGIN @class TSMessage; typedef NS_ENUM(NSUInteger, TSAttachmentPointerType) { - TSAttachmentPointerTypeIncoming = 0, + TSAttachmentPointerTypeUnknown = 0, + TSAttachmentPointerTypeIncoming = 1, TSAttachmentPointerTypeRestoring = 2, }; diff --git a/SignalServiceKit/src/Messages/Attachments/TSAttachmentPointer.m b/SignalServiceKit/src/Messages/Attachments/TSAttachmentPointer.m index 49e62a349..1fc16e38d 100644 --- a/SignalServiceKit/src/Messages/Attachments/TSAttachmentPointer.m +++ b/SignalServiceKit/src/Messages/Attachments/TSAttachmentPointer.m @@ -36,7 +36,7 @@ NS_ASSUME_NONNULL_BEGIN _state = TSAttachmentPointerStateFailed; } - if (![coder containsValueForKey:@"pointerType"]) { + if (_pointerType == TSAttachmentPointerTypeUnknown) { _pointerType = TSAttachmentPointerTypeIncoming; } @@ -76,11 +76,11 @@ NS_ASSUME_NONNULL_BEGIN { OWSAssertDebug(attachmentStream); - self = [super initWithUniqueId:attachmentStream.uniqueId - contentType:attachmentStream.contentType - sourceFilename:attachmentStream.sourceFilename - caption:attachmentStream.caption - albumMessageId:attachmentStream.albumMessageId]; + self = [super initForRestoreWithUniqueId:attachmentStream.uniqueId + contentType:attachmentStream.contentType + sourceFilename:attachmentStream.sourceFilename + caption:attachmentStream.caption + albumMessageId:attachmentStream.albumMessageId]; if (!self) { return self; }