Clean up ahead of PR.

Signal/src/util/OWSBackupExportJob.m

@@ -28,14 +28,12 @@ NS_ASSUME_NONNULL_BEGIN
 
 @property (nonatomic) NSString *recordName;
 
-// This property is optional and represents the location of this
-// item relative to the root directory for items of this type.
-//@property (nonatomic, nullable) NSString *fileRelativePath;
-
 // This property is optional and is only used for attachments.
 @property (nonatomic, nullable) OWSAttachmentExport *attachmentExport;
 
 // This property is optional.
+//
+// See comments in `OWSBackupIO`.
 @property (nonatomic, nullable) NSNumber *uncompressedDataLength;
 
 - (instancetype)init NS_UNAVAILABLE;
@@ -63,6 +61,19 @@ NS_ASSUME_NONNULL_BEGIN
 
 #pragma mark -
 
+// Used to serialize database snapshot contents.
+// Writes db entities using protobufs into snapshot fragments.
+// Snapshot fragments are compressed (they compress _very well_,
+// around 20x smaller) then encrypted.  Ordering matters in
+// snapshot contents (entities should we restored in the same
+// order they are serialized), so we are always careful to preserve
+// ordering of entities within a snapshot AND ordering of snapshot
+// fragments within a bakckup.
+//
+// This stream is used to write entities one at a time and takes
+// care of sharding them into fragments, compressing and encrypting
+// those fragments.  Fragment size is fixed to reduce worst case
+// memory usage.
 @interface OWSDBExportStream : NSObject
 
 @property (nonatomic) OWSBackupIO *backupIO;
@@ -97,6 +108,10 @@ NS_ASSUME_NONNULL_BEGIN
     return self;
 }
 
+
+// It isn't strictly necessary to capture the entity type (the importer doesn't
+// use this state), but I think it'll be helpful to have around to future-proof
+// this work, help with debugging issue, etc.
 - (BOOL)writeObject:(TSYapDatabaseObject *)object
          entityType:(OWSSignalServiceProtosBackupSnapshotBackupEntityType)entityType
 {
@@ -133,15 +148,17 @@ NS_ASSUME_NONNULL_BEGIN
 }
 
 // Write cached data to disk, if necessary.
+//
+// Returns YES on success.
 - (BOOL)flush
 {
     if (!self.backupSnapshotBuilder) {
+        // No data to flush to disk.
         return YES;
     }
 
     // Try to release allocated buffers ASAP.
     @autoreleasepool {
-
         NSData *_Nullable uncompressedData = [self.backupSnapshotBuilder build].data;
         NSUInteger uncompressedDataLength = uncompressedData.length;
         self.backupSnapshotBuilder = nil;
@@ -171,6 +188,12 @@ NS_ASSUME_NONNULL_BEGIN
 
 #pragma mark -
 
+// This class is used to:
+//
+// * Lazy-encrypt and eagerly cleanup attachment uploads.
+//   To reduce disk footprint of backup export process,
+//   we only want to have one attachment export on disk
+//   at a time.
 @interface OWSAttachmentExport : NSObject
 
 @property (nonatomic) OWSBackupIO *backupIO;
@@ -210,9 +233,13 @@ NS_ASSUME_NONNULL_BEGIN
 {
     // Surface memory leaks by logging the deallocation.
     DDLogVerbose(@"Dealloc: %@", self.class);
+
+    [self cleanUp];
 }
 
 // On success, encryptedItem will be non-nil.
+//
+// Returns YES on success.
 - (BOOL)prepareForUpload
 {
     OWSAssert(self.attachmentId.length > 0);
@@ -241,6 +268,12 @@ NS_ASSUME_NONNULL_BEGIN
     return YES;
 }
 
+// Returns YES on success.
+- (BOOL)cleanUp
+{
+    return [OWSFileSystem deleteFileIfExists:self.encryptedItem.filePath];
+}
+
 @end
 
 #pragma mark -
@@ -669,6 +702,11 @@ NS_ASSUME_NONNULL_BEGIN
                     return;
                 }
 
+                if (![attachmentExport cleanUp]) {
+                    DDLogError(@"%@ couldn't clean up attachment export.", self.logTag);
+                    // Attachment files are non-critical so any error uploading them is recoverable.
+                }
+
                 OWSBackupExportItem *exportItem = [OWSBackupExportItem new];
                 exportItem.encryptedItem = attachmentExport.encryptedItem;
                 exportItem.recordName = recordName;
@@ -685,6 +723,11 @@ NS_ASSUME_NONNULL_BEGIN
         failure:^(NSError *error) {
             // Ensure that we continue to work off the main thread.
             dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{
+                if (![attachmentExport cleanUp]) {
+                    DDLogError(@"%@ couldn't clean up attachment export.", self.logTag);
+                    // Attachment files are non-critical so any error uploading them is recoverable.
+                }
+
                 // Attachment files are non-critical so any error uploading them is recoverable.
                 [weakSelf saveNextFileToCloudWithCompletion:completion];
             });

Signal/src/util/OWSBackupIO.h

@@ -45,6 +45,11 @@ NS_ASSUME_NONNULL_BEGIN
 
 - (nullable NSData *)compressData:(NSData *)srcData;
 
+// I'm using the (new in iOS 9) compressionlib.  One of its weaknesses is that it
+// requires you to pre-allocate output buffers during compression and decompression.
+// During decompression this is particularly tricky since there's no way to safely
+// predict how large the output will be based on the input.  So, we store the
+// uncompressed size for compressed backup items.
 - (nullable NSData *)decompressData:(NSData *)srcData uncompressedDataLength:(NSUInteger)uncompressedDataLength;
 
 @end

SignalServiceKit/src/Util/OWSFileSystem.h

@@ -28,9 +28,9 @@ NS_ASSUME_NONNULL_BEGIN
 // Returns NO IFF the directory does not exist and could not be created.
 + (BOOL)ensureDirectoryExists:(NSString *)dirPath;
 
-+ (void)deleteFile:(NSString *)filePath;
++ (BOOL)deleteFile:(NSString *)filePath;
 
-+ (void)deleteFileIfExists:(NSString *)filePath;
++ (BOOL)deleteFileIfExists:(NSString *)filePath;
 
 + (NSArray<NSString *> *_Nullable)allFilesInDirectoryRecursive:(NSString *)dirPath error:(NSError **)error;
 

SignalServiceKit/src/Util/OWSFileSystem.m

@@ -227,20 +227,23 @@ NS_ASSUME_NONNULL_BEGIN
     }
 }
 
-+ (void)deleteFile:(NSString *)filePath
++ (BOOL)deleteFile:(NSString *)filePath
 {
     NSError *error;
-    [[NSFileManager defaultManager] removeItemAtPath:filePath error:&error];
+    BOOL success = [[NSFileManager defaultManager] removeItemAtPath:filePath error:&error];
-    if (error) {
+    if (!success || error) {
         DDLogError(@"%@ Failed to delete file: %@", self.logTag, error.description);
+        return NO;
     }
+    return YES;
 }
 
-+ (void)deleteFileIfExists:(NSString *)filePath
++ (BOOL)deleteFileIfExists:(NSString *)filePath
 {
-    if ([[NSFileManager defaultManager] fileExistsAtPath:filePath]) {
+    if (![[NSFileManager defaultManager] fileExistsAtPath:filePath]) {
-        [self deleteFile:filePath];
+        return YES;
     }
+    return [self deleteFile:filePath];
 }
 
 + (NSArray<NSString *> *_Nullable)allFilesInDirectoryRecursive:(NSString *)dirPath error:(NSError **)error