From 304f0824fe9b719ef62ac2c00d1eed3927d0873f Mon Sep 17 00:00:00 2001
From: Matthew Chen <matthew@signal.org>
Date: Wed, 5 Dec 2018 10:00:36 -0500
Subject: [PATCH] Respond to CR.

---
 Signal/src/util/Backup/OWSBackupAPI.swift   | 17 ++++-------
 Signal/src/util/Backup/OWSBackupExportJob.m | 33 +++++++++++----------
 2 files changed, 23 insertions(+), 27 deletions(-)

diff --git a/Signal/src/util/Backup/OWSBackupAPI.swift b/Signal/src/util/Backup/OWSBackupAPI.swift
index 27c6c640f..27bf5aeea 100644
--- a/Signal/src/util/Backup/OWSBackupAPI.swift
+++ b/Signal/src/util/Backup/OWSBackupAPI.swift
@@ -135,22 +135,15 @@ import PromiseKit
 
     public class func saveRecordsToCloud(records: [CKRecord]) -> Promise<Void> {
 
-        var remainder = records
-        var promise = Promise.value(())
         // CloudKit's internal limit is 400, but I haven't found a constant for this.
         let kMaxBatchSize = 100
-        while remainder.count > 0 {
-            let batch = Array(remainder[0..<kMaxBatchSize])
-            remainder = Array(remainder[kMaxBatchSize..<remainder.count])
-            promise = promise.then(on: DispatchQueue.global()) { _ in
-                return saveRecordsToCloud(records: batch,
-                                          remainingRetries: maxRetries)
-            }.then(on: DispatchQueue.global()) { _ -> Promise<Void> in
-                Logger.verbose("Saved batch: \(batch.count)")
-                return Promise.value(())
+        return records.chunked(by: kMaxBatchSize).reduce(Promise.value(())) { (promise, batch) -> Promise<Void> in
+            return promise.then(on: .global()) {
+                saveRecordsToCloud(records: batch, remainingRetries: maxRetries)
+                }.done {
+                    Logger.verbose("Saved batch: \(batch.count)")
             }
         }
-        return promise
     }
 
     private class func saveRecordsToCloud(records: [CKRecord],
diff --git a/Signal/src/util/Backup/OWSBackupExportJob.m b/Signal/src/util/Backup/OWSBackupExportJob.m
index e22c462e2..bc6255bfa 100644
--- a/Signal/src/util/Backup/OWSBackupExportJob.m
+++ b/Signal/src/util/Backup/OWSBackupExportJob.m
@@ -794,7 +794,7 @@ NS_ASSUME_NONNULL_BEGIN
                 // OWSAttachmentExport is used to lazily write an encrypted copy of the
                 // attachment to disk.
                 if (![attachmentExport prepareForUpload]) {
-                    // Attachment files are non-critical so any error uploading them is recoverable.
+                    // Attachment files are non-critical so any error preparing them is recoverable.
                     return @(1);
                 }
                 OWSAssertDebug(attachmentExport.relativeFilePath.length > 0);
@@ -814,7 +814,7 @@ NS_ASSUME_NONNULL_BEGIN
             }();
 
             if (!fileUrl) {
-                // Attachment files are non-critical so any error uploading them is recoverable.
+                // Attachment files are non-critical so any error preparing them is recoverable.
                 return @(1);
             }
 
@@ -828,21 +828,20 @@ NS_ASSUME_NONNULL_BEGIN
         });
     }
 
+    void (^cleanup)(void) = ^{
+        for (OWSAttachmentExport *attachmentExport in items) {
+            if (![attachmentExport cleanUp]) {
+                OWSLogError(@"couldn't clean up attachment export.");
+                // Attachment files are non-critical so any error uploading them is recoverable.
+            }
+        }
+    };
+
     // TODO: Expose progress.
-    dispatch_queue_t backgroundQueue = dispatch_get_global_queue(0, 0);
     return promise
         .thenInBackground(^{
             return [OWSBackupAPI saveRecordsToCloudObjcWithRecords:records];
         })
-        .ensureOn(backgroundQueue,
-            ^{
-                for (OWSAttachmentExport *attachmentExport in items) {
-                    if (![attachmentExport cleanUp]) {
-                        OWSLogError(@"couldn't clean up attachment export.");
-                        // Attachment files are non-critical so any error uploading them is recoverable.
-                    }
-                }
-            })
         .thenInBackground(^{
             OWSAssertDebug(items.count == records.count);
             NSUInteger count = MIN(items.count, records.count);
@@ -874,9 +873,13 @@ NS_ASSUME_NONNULL_BEGIN
                     attachmentExport.relativeFilePath);
             }
         })
-        .catchInBackground(^{
-            // Attachment files are non-critical so any error uploading them is recoverable.
-            return [AnyPromise promiseWithValue:@(1)];
+        .thenInBackground(^{
+            cleanup();
+        })
+        .catchInBackground(^(NSError *error) {
+            cleanup();
+
+            return error;
         });
 }