Split completion exection contexts to allow for background thread des…

…erialization

Source/SwiftyDropbox/Shared/Handwritten/DropboxTransportClient.swift

@@ -13,6 +13,8 @@ enum ApiClientConstants {
 }
 
 public class DropboxTransportClientImpl: DropboxTransportClientInternal {
+    public static var serializeOnBackgroundThread: Bool = false
+
     public var identifier: String? {
         manager.identifier
     }

Source/SwiftyDropbox/Shared/Handwritten/DropboxTransportClientInterface.swift

@@ -11,6 +11,8 @@ public protocol DropboxTransportClient {
     var accessTokenProvider: AccessTokenProvider? { get set }
     var isBackgroundClient: Bool { get }
 
+    static var serializeOnBackgroundThread: Bool { get set }
+
     var identifier: String? { get }
 
     func request<ASerial, RSerial, ESerial>(

Source/SwiftyDropbox/Shared/Handwritten/MockApiRequest.swift

@@ -16,7 +16,7 @@ class MockApiRequest: ApiRequest {
     }
 
     var requestUrl: URL?
-    var completionHandler: RequestCompletionHandler? {
+    var completionHandler: RequestCompletionHandlerProvider? {
         didSet {
             guard completionHandler != nil else {
                 return
@@ -60,8 +60,8 @@ class MockApiRequest: ApiRequest {
 
     func setProgressHandler(_ handler: @escaping (Progress) -> Void) -> Self { self }
 
-    func setCompletionHandler(queue: DispatchQueue?, completionHandler: RequestCompletionHandler) -> Self {
-        self.completionHandler = completionHandler
+    func setCompletionHandlerProvider(queue: DispatchQueue?, completionHandlerProvider: RequestCompletionHandlerProvider) -> Self {
+        self.completionHandler = completionHandlerProvider
         return self
     }
 
@@ -108,19 +108,19 @@ extension MockApiRequest {
 
         func callCompletion(data: Data?, response: HTTPURLResponse?, error: Error?, downloadLocation: URL? = nil) {
             switch completionHandler {
-            case .dataCompletionHandler(let handler):
-                handler(.init(
+            case .dataCompletionHandlerProvider(let handlerProvider):
+                handlerProvider(.init(
                     data: data,
                     response: response,
                     error: error.flatMap { .urlSessionError($0) }
-                ))
-            case .downloadFileCompletionHandler(let handler):
-                handler(.init(
+                ))()
+            case .downloadFileCompletionHandlerProvider(let handlerProvider):
+                handlerProvider(.init(
                     url: downloadLocation,
                     response: response,
                     error: error.flatMap { .urlSessionError($0) },
                     errorDataFromLocation: { _ in .init() }
-                ))
+                ))()
             case .none:
                 break
             }

Source/SwiftyDropbox/Shared/Handwritten/MockDropboxTransportClient.swift

@@ -28,6 +28,8 @@ enum MockingUtilities {
 }
 
 class MockDropboxTransportClient: DropboxTransportClient {
+    public static var serializeOnBackgroundThread: Bool = false
+
     var identifier: String?
     let filesAccess: FilesAccess = FilesAccessImpl()
 

Source/SwiftyDropbox/Shared/Handwritten/NoopRequest.swift

@@ -30,7 +30,7 @@ class NoopApiRequest: ApiRequest {
         self
     }
 
-    func setCompletionHandler(queue: DispatchQueue?, completionHandler: RequestCompletionHandler) -> Self {
+    func setCompletionHandlerProvider(queue: DispatchQueue?, completionHandlerProvider: RequestCompletionHandlerProvider) -> Self {
         self
     }
 

Source/SwiftyDropbox/Shared/Handwritten/Request+TokenRefresh.swift

@@ -3,12 +3,16 @@
 ///
 import Foundation
 
+typealias WrappedCompletionHandler = () -> Void
+typealias DataCompletionHandlerProvider = (NetworkDataTaskResult) -> WrappedCompletionHandler
+typealias DownloadCompletionHandlerProvider = (NetworkDownloadTaskResult) -> WrappedCompletionHandler
+
 /// Completion handler for ApiRequest.
-enum RequestCompletionHandler {
-    /// Handler for data requests whose results are in memory.
-    case dataCompletionHandler((NetworkDataTaskResult) -> Void)
-    /// Handler for download request which stores download result into a file.
-    case downloadFileCompletionHandler((NetworkDownloadTaskResult) -> Void)
+enum RequestCompletionHandlerProvider {
+    /// Provider of handler for data requests whose results are in memory.
+    case dataCompletionHandlerProvider(DataCompletionHandlerProvider)
+    /// Provider of handler for download request which stores download result into a file.
+    case downloadFileCompletionHandlerProvider(DownloadCompletionHandlerProvider)
 }
 
 /// Protocol specifying an entity that can recieve networking info from a NetworkSessionDelegate
@@ -46,10 +50,10 @@ protocol RequestControlling {
     /// Sets a completion handler for the request.
     ///
     /// - Parameters:
-    ///     - completionHandler The completion handler.
-    ///     - queue: The queue where the completion handler will be called from.
+    ///     - completionHandlerProvider The completion handler provider.
+    ///     - queue: The queue where the provided completion handler will be called from.
     @discardableResult
-    func setCompletionHandler(queue: DispatchQueue?, completionHandler: RequestCompletionHandler) -> Self
+    func setCompletionHandlerProvider(queue: DispatchQueue?, completionHandlerProvider: RequestCompletionHandlerProvider) -> Self
 
     /// Cancels the request.
     func cancel()
@@ -141,7 +145,7 @@ class RequestWithTokenRefresh: ApiRequest {
         fileprivate var cancelled = false
         fileprivate var isComplete = false
         fileprivate var responseQueue: DispatchQueue?
-        fileprivate var completionHandler: RequestCompletionHandler?
+        fileprivate var completionHandler: RequestCompletionHandlerProvider?
         fileprivate var progressHandler: ((Progress) -> Void)?
         fileprivate var cleanupHandler: (() -> Void)?
 
@@ -266,13 +270,13 @@ class RequestWithTokenRefresh: ApiRequest {
         self.filesAccess = filesAccess
     }
 
-    func setCompletionHandler(queue: DispatchQueue?, completionHandler: RequestCompletionHandler) -> Self {
+    func setCompletionHandlerProvider(queue: DispatchQueue?, completionHandlerProvider: RequestCompletionHandlerProvider) -> Self {
         accessStateWithLock { mutableState in
             mutableState.responseQueue = queue
             if mutableState.isComplete {
-                call(completionHandler: completionHandler, error: mutableState.request?.clientError, mutableState: mutableState)
+                call(completionHandler: completionHandlerProvider, error: mutableState.request?.clientError, mutableState: mutableState)
             } else {
-                mutableState.completionHandler = completionHandler
+                mutableState.completionHandler = completionHandlerProvider
             }
         }
         return self
@@ -341,22 +345,8 @@ class RequestWithTokenRefresh: ApiRequest {
 
     private func completeWithError(_ error: ClientError) {
         accessStateWithLock { mutableState in
-            switch mutableState.completionHandler {
-            case .dataCompletionHandler(let handler):
-                mutableState.completionHandlerQueue.async {
-                    handler(NetworkDataTaskResult(data: nil, response: nil, error: error))
-                }
-            case .downloadFileCompletionHandler(let handler):
-                mutableState.completionHandlerQueue.async {
-                    handler(NetworkDownloadTaskResult(
-                        url: nil,
-                        response: nil,
-                        error: error,
-                        errorDataFromLocation: self.filesAccess.errorData(from:)
-                    ))
-                }
-            case .none:
-                break
+            if let completionHandler = mutableState.completionHandler {
+                call(completionHandler: completionHandler, error: error, mutableState: mutableState)
             }
         }
 
@@ -429,32 +419,81 @@ extension RequestWithTokenRefresh {
         }
     }
 
-    private func call(completionHandler: RequestCompletionHandler, error: ClientError?, mutableState: MutableState) {
+    private func call(completionHandler: RequestCompletionHandlerProvider, error: ClientError?, mutableState: MutableState) {
         switch completionHandler {
-        case .dataCompletionHandler(let handler):
-            let data = mutableState.data
-            let response = mutableState.response?.copy() as? HTTPURLResponse
+        case .dataCompletionHandlerProvider(let handlerProvider):
+            callDataCompletionHandler(error: error, mutableState: mutableState, handlerProvider: handlerProvider)
+        case .downloadFileCompletionHandlerProvider(let handlerProvider):
+            callDownloadCompletionHandler(error: error, mutableState: mutableState, handlerProvider: handlerProvider)
+        }
+    }
 
-            mutableState.completionHandlerQueue.async {
-                handler(.init(
+    private func callDataCompletionHandler(error: ClientError?, mutableState: MutableState, handlerProvider: @escaping ((NetworkDataTaskResult) -> WrappedCompletionHandler)) {
+        // copy for use out of lock
+        let data = mutableState.data
+        let response = mutableState.response
+        let completionQueue = mutableState.completionHandlerQueue
+
+        // lock is held above this line but not below, do not again reference mutableState
+
+        if DropboxTransportClientImpl.serializeOnBackgroundThread {
+            DispatchQueue.global(qos: .userInitiated).async {
+                let handler = handlerProvider(.init(
+                    data: data,
+                    response: response,
+                    error: error
+                ))
+
+                completionQueue.async {
+                    handler()
+                    self.cleanup()
+                }
+            }
+        } else {
+            completionQueue.async {
+                let handler = handlerProvider(.init(
                     data: data,
                     response: response,
                     error: error
                 ))
+                handler()
                 self.cleanup()
             }
-        case .downloadFileCompletionHandler(let handler):
-            let temporaryDownloadURL = mutableState.temporaryDownloadURL
-            let response = mutableState.response?.copy() as? HTTPURLResponse
-            let moveDownloadError: ClientError? = mutableState.moveDownloadError.map { .fileAccessError($0) }
+        }
+    }
 
-            mutableState.completionHandlerQueue.async {
-                handler(.init(
+    private func callDownloadCompletionHandler(error: ClientError?, mutableState: MutableState, handlerProvider: @escaping ((NetworkDownloadTaskResult) -> WrappedCompletionHandler)) {
+        // copy for use out of lock
+        let temporaryDownloadURL = mutableState.temporaryDownloadURL
+        let response = mutableState.response
+        let moveDownloadError: ClientError? = mutableState.moveDownloadError.map { .fileAccessError($0) }
+        let completionQueue = mutableState.completionHandlerQueue
+
+        // lock is held above this line but not below, do not again reference mutableState
+
+        if DropboxTransportClientImpl.serializeOnBackgroundThread {
+            DispatchQueue.global(qos: .userInitiated).async {
+                let handler = handlerProvider(.init(
+                    url: temporaryDownloadURL,
+                    response: response,
+                    error: error ?? moveDownloadError,
+                    errorDataFromLocation: self.filesAccess.errorData(from:)
+                ))
+
+                completionQueue.async {
+                    handler()
+                    self.cleanup()
+                }
+            }
+        } else {
+            completionQueue.async {
+                let handler = handlerProvider(.init(
                     url: temporaryDownloadURL,
                     response: response,
                     error: error ?? moveDownloadError,
                     errorDataFromLocation: self.filesAccess.errorData(from:)
                 ))
+                handler()
                 self.cleanup()
             }
         }