Merge pull request #1589 from onevcat/feature/async-request-modifier

Feature async request modifier

Sources/General/KingfisherOptionsInfo.swift

@@ -127,7 +127,7 @@ public enum KingfisherOptionsInfoItem {
     /// This is the last chance you can modify the image download request. You can modify the request for some
     /// customizing purpose, such as adding auth token to the header, do basic HTTP auth or something like url mapping.
     /// The original request will be sent without any modification by default.
-    case requestModifier(ImageDownloadRequestModifier)
+    case requestModifier(AsyncImageDownloadRequestModifier)
     
     /// The `ImageDownloadRedirectHandler` contained will be used to change the request before redirection.
     /// This is the possibility you can modify the image download request during redirect. You can modify the request for
@@ -272,7 +272,7 @@ public struct KingfisherParsedOptionsInfo {
     public var preloadAllAnimationData = false
     public var callbackQueue: CallbackQueue = .mainCurrentOrAsync
     public var scaleFactor: CGFloat = 1.0
-    public var requestModifier: ImageDownloadRequestModifier? = nil
+    public var requestModifier: AsyncImageDownloadRequestModifier? = nil
     public var redirectHandler: ImageDownloadRedirectHandler? = nil
     public var processor: ImageProcessor = DefaultImageProcessor.default
     public var imageModifier: ImageModifier? = nil

Sources/Networking/ImageDownloader.swift

@@ -30,6 +30,8 @@ import AppKit
 import UIKit
 #endif
 
+typealias DownloadResult = Result<ImageLoadingResult, KingfisherError>
+
 /// Represents a success result of an image downloading progress.
 public struct ImageLoadingResult {
 
@@ -193,131 +195,205 @@ open class ImageDownloader {
         }
     }
 
-    // MARK: Dowloading Task
-    /// Downloads an image with a URL and option. Invoked internally by Kingfisher. Subclasses must invoke super.
-    ///
-    /// - Parameters:
-    ///   - url: Target URL.
-    ///   - options: The options could control download behavior. See `KingfisherOptionsInfo`.
-    ///   - completionHandler: Called when the download progress finishes. This block will be called in the queue
-    ///                        defined in `.callbackQueue` in `options` parameter.
-    /// - Returns: A downloading task. You could call `cancel` on it to stop the download task.
-    @discardableResult
-    open func downloadImage(
+    // Wraps `completionHandler` to `onCompleted` respectively.
+    private func createCompletionCallBack(_ completionHandler: ((DownloadResult) -> Void)?) -> Delegate<DownloadResult, Void>? {
+        return completionHandler.map { block -> Delegate<DownloadResult, Void> in
+
+            let delegate =  Delegate<Result<ImageLoadingResult, KingfisherError>, Void>()
+            delegate.delegate(on: self) { (self, callback) in
+                block(callback)
+            }
+            return delegate
+        }
+    }
+
+    private func createTaskCallback(
+        _ completionHandler: ((DownloadResult) -> Void)?,
+        options: KingfisherParsedOptionsInfo
+    ) -> SessionDataTask.TaskCallback
+    {
+        return SessionDataTask.TaskCallback(
+            onCompleted: createCompletionCallBack(completionHandler),
+            options: options
+        )
+    }
+
+    private func createDownloadContext(
         with url: URL,
         options: KingfisherParsedOptionsInfo,
-        completionHandler: ((Result<ImageLoadingResult, KingfisherError>) -> Void)? = nil) -> DownloadTask?
+        done: @escaping ((Result<DownloadingContext, KingfisherError>) -> Void)
+    )
     {
+        func checkRequestAndDone(r: URLRequest) {
+
+            // There is a possibility that request modifier changed the url to `nil` or empty.
+            // In this case, throw an error.
+            guard let url = r.url, !url.absoluteString.isEmpty else {
+                done(.failure(KingfisherError.requestError(reason: .invalidURL(request: r))))
+                return
+            }
+
+            done(.success(DownloadingContext(url: url, request: r, options: options)))
+        }
+
         // Creates default request.
         var request = URLRequest(url: url, cachePolicy: .reloadIgnoringLocalCacheData, timeoutInterval: downloadTimeout)
         request.httpShouldUsePipelining = requestsUsePipelining
 
         if let requestModifier = options.requestModifier {
             // Modifies request before sending.
-            guard let r = requestModifier.modified(for: request) else {
-                options.callbackQueue.execute {
-                    completionHandler?(.failure(KingfisherError.requestError(reason: .emptyRequest)))
+            requestModifier.modified(for: request) { result in
+                guard let finalRequest = result else {
+                    done(.failure(KingfisherError.requestError(reason: .emptyRequest)))
+                    return
                 }
-                return nil
+                checkRequestAndDone(r: finalRequest)
             }
-            request = r
+        } else {
+            checkRequestAndDone(r: request)
         }
-        
-        // There is a possibility that request modifier changed the url to `nil` or empty.
-        // In this case, throw an error.
-        guard let url = request.url, !url.absoluteString.isEmpty else {
-            options.callbackQueue.execute {
-                completionHandler?(.failure(KingfisherError.requestError(reason: .invalidURL(request: request))))
-            }
-            return nil
+    }
+
+    private func addDownloadTask(
+        context: DownloadingContext,
+        callback: SessionDataTask.TaskCallback
+    ) -> DownloadTask
+    {
+        // Ready to start download. Add it to session task manager (`sessionHandler`)
+        let downloadTask: DownloadTask
+        if let existingTask = sessionDelegate.task(for: context.url) {
+            downloadTask = sessionDelegate.append(existingTask, url: context.url, callback: callback)
+        } else {
+            let sessionDataTask = session.dataTask(with: context.request)
+            sessionDataTask.priority = context.options.downloadPriority
+            downloadTask = sessionDelegate.add(sessionDataTask, url: context.url, callback: callback)
         }
+        return downloadTask
+    }
 
-        // Wraps `completionHandler` to `onCompleted` respectively.
 
-        let onCompleted = completionHandler.map {
-            block -> Delegate<Result<ImageLoadingResult, KingfisherError>, Void> in
-            let delegate =  Delegate<Result<ImageLoadingResult, KingfisherError>, Void>()
-            delegate.delegate(on: self) { (_, callback) in
-                block(callback)
-            }
-            return delegate
-        }
+    private func reportWillDownloadImage(url: URL, request: URLRequest) {
+        delegate?.imageDownloader(self, willDownloadImageForURL: url, with: request)
+    }
 
-        // SessionDataTask.TaskCallback is a wrapper for `onCompleted` and `options` (for processor info)
-        let callback = SessionDataTask.TaskCallback(
-            onCompleted: onCompleted,
-            options: options
+    private func reportDidDownloadImageData(result: Result<(Data, URLResponse?), KingfisherError>, url: URL) {
+        var response: URLResponse?
+        var err: Error?
+        do {
+            response = try result.get().1
+        } catch {
+            err = error
+        }
+        self.delegate?.imageDownloader(
+            self,
+            didFinishDownloadingImageForURL: url,
+            with: response,
+            error: err
         )
+    }
 
-        // Ready to start download. Add it to session task manager (`sessionHandler`)
-
-        let downloadTask: DownloadTask
-        if let existingTask = sessionDelegate.task(for: url) {
-            downloadTask = sessionDelegate.append(existingTask, url: url, callback: callback)
-        } else {
-            let sessionDataTask = session.dataTask(with: request)
-            sessionDataTask.priority = options.downloadPriority
-            downloadTask = sessionDelegate.add(sessionDataTask, url: url, callback: callback)
+    private func reportDidProcessImage(
+        result: Result<KFCrossPlatformImage, KingfisherError>, url: URL, response: URLResponse?
+    )
+    {
+        if let image = try? result.get() {
+            self.delegate?.imageDownloader(self, didDownload: image, for: url, with: response)
         }
 
+    }
+
+    private func startDownloadTask(
+        context: DownloadingContext,
+        callback: SessionDataTask.TaskCallback
+    ) -> DownloadTask
+    {
+
+        let downloadTask = addDownloadTask(context: context, callback: callback)
+
         let sessionTask = downloadTask.sessionTask
+        guard !sessionTask.started else {
+            return downloadTask
+        }
 
-        // Start the session task if not started yet.
-        if !sessionTask.started {
-            sessionTask.onTaskDone.delegate(on: self) { (self, done) in
-                // Underlying downloading finishes.
-                // result: Result<(Data, URLResponse?)>, callbacks: [TaskCallback]
-                let (result, callbacks) = done
-
-                // Before processing the downloaded data.
-                do {
-                    let value = try result.get()
-                    self.delegate?.imageDownloader(
-                        self,
-                        didFinishDownloadingImageForURL: url,
-                        with: value.1,
-                        error: nil
-                    )
-                } catch {
-                    self.delegate?.imageDownloader(
-                        self,
-                        didFinishDownloadingImageForURL: url,
-                        with: nil,
-                        error: error
-                    )
+        sessionTask.onTaskDone.delegate(on: self) { (self, done) in
+            // Underlying downloading finishes.
+            // result: Result<(Data, URLResponse?)>, callbacks: [TaskCallback]
+            let (result, callbacks) = done
+
+            // Before processing the downloaded data.
+            self.reportDidDownloadImageData(result: result, url: context.url)
+
+            switch result {
+            // Download finished. Now process the data to an image.
+            case .success(let (data, response)):
+                let processor = ImageDataProcessor(
+                    data: data, callbacks: callbacks, processingQueue: context.options.processingQueue
+                )
+                processor.onImageProcessed.delegate(on: self) { (self, done) in
+                    // `onImageProcessed` will be called for `callbacks.count` times, with each
+                    // `SessionDataTask.TaskCallback` as the input parameter.
+                    // result: Result<Image>, callback: SessionDataTask.TaskCallback
+                    let (result, callback) = done
+
+                    self.reportDidProcessImage(result: result, url: context.url, response: response)
+
+                    let imageResult = result.map { ImageLoadingResult(image: $0, url: context.url, originalData: data) }
+                    let queue = callback.options.callbackQueue
+                    queue.execute { callback.onCompleted?.call(imageResult) }
                 }
+                processor.process()
+
+            case .failure(let error):
+                callbacks.forEach { callback in
+                    let queue = callback.options.callbackQueue
+                    queue.execute { callback.onCompleted?.call(.failure(error)) }
+                }
+            }
+        }
+
+        reportWillDownloadImage(url: context.url, request: context.request)
+        sessionTask.resume()
+        return downloadTask
+    }
 
-                switch result {
-                // Download finished. Now process the data to an image.
-                case .success(let (data, response)):
-                    let processor = ImageDataProcessor(
-                        data: data, callbacks: callbacks, processingQueue: options.processingQueue)
-                    processor.onImageProcessed.delegate(on: self) { (self, result) in
-                        // `onImageProcessed` will be called for `callbacks.count` times, with each
-                        // `SessionDataTask.TaskCallback` as the input parameter.
-                        // result: Result<Image>, callback: SessionDataTask.TaskCallback
-                        let (result, callback) = result
-
-                        if let image = try? result.get() {
-                            self.delegate?.imageDownloader(self, didDownload: image, for: url, with: response)
-                        }
-
-                        let imageResult = result.map { ImageLoadingResult(image: $0, url: url, originalData: data) }
-                        let queue = callback.options.callbackQueue
-                        queue.execute { callback.onCompleted?.call(imageResult) }
-                    }
-                    processor.process()
-
-                case .failure(let error):
-                    callbacks.forEach { callback in
-                        let queue = callback.options.callbackQueue
-                        queue.execute { callback.onCompleted?.call(.failure(error)) }
-                    }
+    // MARK: Downloading Task
+    /// Downloads an image with a URL and option. Invoked internally by Kingfisher. Subclasses must invoke super.
+    ///
+    /// - Parameters:
+    ///   - url: Target URL.
+    ///   - options: The options could control download behavior. See `KingfisherOptionsInfo`.
+    ///   - completionHandler: Called when the download progress finishes. This block will be called in the queue
+    ///                        defined in `.callbackQueue` in `options` parameter.
+    /// - Returns: A downloading task. You could call `cancel` on it to stop the download task.
+    @discardableResult
+    open func downloadImage(
+        with url: URL,
+        options: KingfisherParsedOptionsInfo,
+        completionHandler: ((Result<ImageLoadingResult, KingfisherError>) -> Void)? = nil) -> DownloadTask?
+    {
+        var downloadTask: DownloadTask?
+        createDownloadContext(with: url, options: options) { result in
+            switch result {
+            case .success(let context):
+                // `downloadTask` will be set if the downloading started immediately. This is the case when no request
+                // modifier or a sync modifier (`ImageDownloadRequestModifier`) is used. Otherwise, when an
+                // `AsyncImageDownloadRequestModifier` is used the returned `downloadTask` of this method will be `nil`
+                // and the actual "delayed" task is given in `AsyncImageDownloadRequestModifier.onDownloadTaskStarted`
+                // callback.
+                downloadTask = self.startDownloadTask(
+                    context: context,
+                    callback: self.createTaskCallback(completionHandler, options: options)
+                )
+                if let modifier = options.requestModifier {
+                    modifier.onDownloadTaskStarted?(downloadTask)
+                }
+            case .failure(let error):
+                options.callbackQueue.execute {
+                    completionHandler?(.failure(error))
                 }
             }
-            delegate?.imageDownloader(self, willDownloadImageForURL: url, with: request)
-            sessionTask.resume()
         }
+
         return downloadTask
     }
 
@@ -396,3 +472,11 @@ extension ImageDownloader: AuthenticationChallengeResponsable {}
 
 // Use the default implementation from extension of `ImageDownloaderDelegate`.
 extension ImageDownloader: ImageDownloaderDelegate {}
+
+extension ImageDownloader {
+    struct DownloadingContext {
+        let url: URL
+        let request: URLRequest
+        let options: KingfisherParsedOptionsInfo
+    }
+}

Sources/Networking/RequestModifier.swift

@@ -26,10 +26,38 @@
 
 import Foundation
 
+/// Represents and wraps a method for modifying request before an image download request starts in an asynchronous way.
+public protocol AsyncImageDownloadRequestModifier {
+
+    /// This method will be called just before the `request` being sent.
+    /// This is the last chance you can modify the image download request. You can modify the request for some
+    /// customizing purpose, such as adding auth token to the header, do basic HTTP auth or something like url mapping.
+    /// When you have done with the modification, call the `reportModified` block with the modified request and the data
+    /// download will happen with this request.
+    ///
+    /// Usually, you pass an `AsyncImageDownloadRequestModifier` as the associated value of
+    /// `KingfisherOptionsInfoItem.requestModifier` and use it as the `options` parameter in related methods.
+    ///
+    /// If you do nothing with the input `request` and return it as is, a downloading process will start with it.
+    ///
+    /// - Parameters:
+    ///   - request: The input request contains necessary information like `url`. This request is generated
+    ///              according to your resource url as a GET request.
+    ///   - reportModified: The callback block you need to call after the asynchronous modifying done.
+    ///
+    func modified(for request: URLRequest, reportModified: @escaping (URLRequest?) -> Void)
+
+    /// A block will be called when the download task started.
+    ///
+    /// If an `AsyncImageDownloadRequestModifier` and the asynchronous modification happens before the download, the
+    /// related download method will not return a valid `DownloadTask` value. Instead, you can get one from this method.
+    var onDownloadTaskStarted: ((DownloadTask?) -> Void)? { get }
+}
+
 /// Represents and wraps a method for modifying request before an image download request starts.
-public protocol ImageDownloadRequestModifier {
+public protocol ImageDownloadRequestModifier: AsyncImageDownloadRequestModifier {
 
-    /// A method will be called just before the `request` being sent.
+    /// This method will be called just before the `request` being sent.
     /// This is the last chance you can modify the image download request. You can modify the request for some
     /// customizing purpose, such as adding auth token to the header, do basic HTTP auth or something like url mapping.
     ///
@@ -46,6 +74,17 @@ public protocol ImageDownloadRequestModifier {
     func modified(for request: URLRequest) -> URLRequest?
 }
 
+extension ImageDownloadRequestModifier {
+    public func modified(for request: URLRequest, reportModified: @escaping (URLRequest?) -> Void) {
+        let request = modified(for: request)
+        reportModified(request)
+    }
+
+    /// This is `nil` for a sync `ImageDownloadRequestModifier` by default. You can get the `DownloadTask` from the
+    /// return value of downloader method.
+    public var onDownloadTaskStarted: ((DownloadTask?) -> Void)? { return nil }
+}
+
 /// A wrapper for creating an `ImageDownloadRequestModifier` easier.
 /// This type conforms to `ImageDownloadRequestModifier` and wraps an image modify block.
 public struct AnyModifier: ImageDownloadRequestModifier {

Tests/KingfisherTests/ImageDownloaderTests.swift

@@ -112,11 +112,40 @@ class ImageDownloaderTests: XCTestCase {
         modifier.url = url
         
         let someURL = URL(string: "some_strange_url")!
-        downloader.downloadImage(with: someURL, options: [.requestModifier(modifier)]) { result in
+        let task = downloader.downloadImage(with: someURL, options: [.requestModifier(modifier)]) { result in
             XCTAssertNotNil(result.value)
             XCTAssertEqual(result.value?.url, url)
             exp.fulfill()
         }
+        XCTAssertNotNil(task)
+        waitForExpectations(timeout: 3, handler: nil)
+    }
+
+    func testDownloadWithAsyncModifyingRequest() {
+        let exp = expectation(description: #function)
+
+        let url = testURLs[0]
+        stub(url, data: testImageData)
+
+        var downloadTaskCalled = false
+
+        let asyncModifier = AsyncURLModifier()
+        asyncModifier.url = url
+        asyncModifier.onDownloadTaskStarted = { task in
+            XCTAssertNotNil(task)
+            downloadTaskCalled = true
+        }
+
+
+        let someURL = URL(string: "some_strage_url")!
+        let task = downloader.downloadImage(with: someURL, options: [.requestModifier(asyncModifier)]) { result in
+            XCTAssertNotNil(result.value)
+            XCTAssertEqual(result.value?.url, url)
+            XCTAssertTrue(downloadTaskCalled)
+            exp.fulfill()
+        }
+        // The returned task is nil since the download is not starting immediately.
+        XCTAssertNil(task)
         waitForExpectations(timeout: 3, handler: nil)
     }
 
@@ -535,3 +564,15 @@ class URLModifier: ImageDownloadRequestModifier {
     }
 }
 
+class AsyncURLModifier: AsyncImageDownloadRequestModifier {
+    var url: URL? = nil
+    var onDownloadTaskStarted: ((DownloadTask?) -> Void)?
+
+    func modified(for request: URLRequest, reportModified: @escaping (URLRequest?) -> Void) {
+        var r = request
+        r.url = url
+        DispatchQueue.main.async {
+            reportModified(r)
+        }
+    }
+}