Some refactor for ImageDownloadRedirectHandler type

Sources/Cache/ImageCache.swift

@@ -445,11 +445,10 @@ open class ImageCache {
     {
         // No completion handler. No need to start working and early return.
         guard let completionHandler = completionHandler else { return }
-        let imageModifier = options.imageModifier
 
         // Try to check the image from memory cache first.
         if let image = retrieveImageInMemoryCache(forKey: key, options: options) {
-            let image = imageModifier.modify(image)
+            let image = options.imageModifier?.modify(image) ?? image
             callbackQueue.execute { completionHandler(.success(.memory(image))) }
         } else if options.fromMemoryCacheOrRefresh {
             callbackQueue.execute { completionHandler(.success(.none)) }
@@ -460,25 +459,27 @@ open class ImageCache {
                 // The callback queue is already correct in this closure.
                 switch result {
                 case .success(let image):
-                    guard let image = imageModifier.modify(image) else {
+
+                    guard let image = image else {
                         // No image found in disk storage.
                         completionHandler(.success(.none))
                         return
                     }
 
+                    let finalImage = options.imageModifier?.modify(image) ?? image
                     // Cache the disk image to memory.
                     // We are passing `false` to `toDisk`, the memory cache does not change
                     // callback queue, we can call `completionHandler` without another dispatch.
                     var cacheOptions = options
                     cacheOptions.callbackQueue = .untouch
                     self.store(
-                        image,
+                        finalImage,
                         forKey: key,
                         options: cacheOptions,
                         toDisk: false)
                     {
                         _ in
-                        completionHandler(.success(.disk(image)))
+                        completionHandler(.success(.disk(finalImage)))
                     }
                 case .failure(let error):
                     completionHandler(.failure(error))

Sources/General/Deprecated.swift

@@ -565,7 +565,7 @@ public extension Collection where Iterator.Element == KingfisherOptionsInfoItem
     /// The `ImageDownloadRequestModifier` will be used before sending a download request.
     @available(*, deprecated,
     message: "Create a `KingfisherParsedOptionsInfo` from `KingfisherOptionsInfo` and use `requestModifier` instead.")
-    public var modifier: ImageDownloadRequestModifier {
+    public var modifier: ImageDownloadRequestModifier? {
         return KingfisherParsedOptionsInfo(Array(self)).requestModifier
     }
 
@@ -579,7 +579,7 @@ public extension Collection where Iterator.Element == KingfisherOptionsInfoItem
     /// `ImageModifier` for modifying right before the image is displayed.
     @available(*, deprecated,
     message: "Create a `KingfisherParsedOptionsInfo` from `KingfisherOptionsInfo` and use `imageModifier` instead.")
-    public var imageModifier: ImageModifier {
+    public var imageModifier: ImageModifier? {
         return KingfisherParsedOptionsInfo(Array(self)).imageModifier
     }
 
@@ -633,3 +633,16 @@ public extension Collection where Iterator.Element == KingfisherOptionsInfoItem
         return KingfisherParsedOptionsInfo(Array(self)).loadDiskFileSynchronously
     }
 }
+
+/// The default modifier.
+/// It does nothing and returns the image as is.
+@available(*, deprecated, message: "Use `nil` in KingfisherOptionsInfo to indicate no modifier.")
+public struct DefaultImageModifier: ImageModifier {
+
+    /// A default `DefaultImageModifier` which can be used everywhere.
+    public static let `default` = DefaultImageModifier()
+    private init() {}
+
+    /// Modifies an input `Image`. See `ImageModifier` protocol for more.
+    public func modify(_ image: Image) -> Image { return image }
+}

Sources/General/KingfisherOptionsInfo.swift

@@ -237,10 +237,10 @@ public struct KingfisherParsedOptionsInfo {
     public var preloadAllAnimationData = false
     public var callbackQueue: CallbackQueue = .mainCurrentOrAsync
     public var scaleFactor: CGFloat = 1.0
-    public var requestModifier: ImageDownloadRequestModifier = NoModifier.default
-    public var redirectHandler: ImageDownloadRedirectHandler = NoHandler.default
+    public var requestModifier: ImageDownloadRequestModifier? = nil
+    public var redirectHandler: ImageDownloadRedirectHandler? = nil
     public var processor: ImageProcessor = DefaultImageProcessor.default
-    public var imageModifier: ImageModifier = DefaultImageModifier.default
+    public var imageModifier: ImageModifier? = nil
     public var cacheSerializer: CacheSerializer = DefaultCacheSerializer.default
     public var keepCurrentImageWhileLoading = false
     public var onlyLoadFirstFrame = false

Sources/Networking/ImageDataProcessor.swift

@@ -61,8 +61,10 @@ class ImageDataProcessor {
 
             let result: Result<Image, KingfisherError>
             if let image = image {
-                let imageModifier = callback.options.imageModifier
-                var finalImage = imageModifier.modify(image)
+                var finalImage = image
+                if let imageModifier = callback.options.imageModifier {
+                    finalImage = imageModifier.modify(image)
+                }
                 if callback.options.backgroundDecode {
                     finalImage = finalImage.kf.decoded
                 }

Sources/Networking/ImageDownloader.swift

@@ -179,14 +179,16 @@ open class ImageDownloader {
         var request = URLRequest(url: url, cachePolicy: .reloadIgnoringLocalCacheData, timeoutInterval: downloadTimeout)
         request.httpShouldUsePipelining = requestsUsePipelining
 
-        // Modifies request before sending.
-        guard let r = options.requestModifier.modified(for: request) else {
-            options.callbackQueue.execute {
-                completionHandler?(.failure(KingfisherError.requestError(reason: .emptyRequest)))
+        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)))
+                }
+                return nil
             }
-            return nil
+            request = r
         }
-        request = r
         
         // There is a possibility that request modifier changed the url to `nil` or empty.
         // In this case, throw an error.

Sources/Networking/ImageModifier.swift

@@ -43,27 +43,6 @@ public protocol ImageModifier {
     func modify(_ image: Image) -> Image
 }
 
-extension ImageModifier {
-    func modify(_ image: Image?) -> Image? {
-        guard let image = image else {
-            return nil
-        }
-        return modify(image)
-    }
-}
-
-/// The default modifier.
-/// It does nothing and returns the image as is.
-public struct DefaultImageModifier: ImageModifier {
-
-    /// A default `DefaultImageModifier` which can be used everywhere.
-    public static let `default` = DefaultImageModifier()
-    private init() {}
-
-    /// Modifies an input `Image`. See `ImageModifier` protocol for more.
-    public func modify(_ image: Image) -> Image { return image }
-}
-
 /// A wrapper for creating an `ImageModifier` easier.
 /// This type conforms to `ImageModifier` and wraps an image modify block.
 /// If the `block` throws an error, the original image will be used.

Sources/Networking/RedirectHandler.swift

@@ -28,41 +28,41 @@ import Foundation
 
 /// Represents and wraps a method for modifying request during an image download request redirection.
 public protocol ImageDownloadRedirectHandler {
-    
+
     /// The `ImageDownloadRedirectHandler` contained will be used to change the request before redirection.
-    /// This is the posibility you can modify the image download request during redirection. 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.
+    /// This is the posibility you can modify the image download request during redirection. 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.
     ///
     /// Usually, you pass an `ImageDownloadRedirectHandler` as the associated value of
     /// `KingfisherOptionsInfoItem.redirectHandler` 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 redirect with it.
     ///
-    /// - Parameter task: current SessionDataTask.
-    ///             response: response received during redirection.
-    ///             newRequest: request for redirection which can be modified.
-    ///             completionHandler: closure for modifying request
-    ///
-    func handleHTTPRedirection(for task: SessionDataTask, response: HTTPURLResponse, newRequest: URLRequest, completionHandler: @escaping (URLRequest?) -> Void)
-}
-
-struct NoHandler: ImageDownloadRedirectHandler {
-    static let `default` = NoHandler()
-    private init() {}
-    
-    func handleHTTPRedirection(for task: SessionDataTask, response: HTTPURLResponse, newRequest: URLRequest, completionHandler: @escaping (URLRequest?) -> Void) {
-        completionHandler(newRequest)
-    }
+    /// - Parameters:
+    ///   - task: The current `SessionDataTask` which triggers this redirect.
+    ///   - response: The response received during redirection.
+    ///   - newRequest: The request for redirection which can be modified.
+    ///   - completionHandler: A closure for being called with modified request.
+    func handleHTTPRedirection(
+        for task: SessionDataTask,
+        response: HTTPURLResponse,
+        newRequest: URLRequest,
+        completionHandler: @escaping (URLRequest?) -> Void)
 }
 
 /// A wrapper for creating an `ImageDownloadRedirectHandler` easier.
-/// This type conforms to `ImageDownloadRedirectHandler` and wraps an image modify block.
-public struct AnyHandler: ImageDownloadRedirectHandler {
+/// This type conforms to `ImageDownloadRedirectHandler` and wraps an redirect request modify block.
+public struct AnyRedirectHandler: ImageDownloadRedirectHandler {
     
     let block: (SessionDataTask, HTTPURLResponse, URLRequest, (URLRequest?) -> Void) -> Void
 
-    public func handleHTTPRedirection(for task: SessionDataTask, response: HTTPURLResponse, newRequest: URLRequest, completionHandler: @escaping (URLRequest?) -> Void) {
+    public func handleHTTPRedirection(
+        for task: SessionDataTask,
+        response: HTTPURLResponse,
+        newRequest: URLRequest,
+        completionHandler: @escaping (URLRequest?) -> Void)
+    {
         block(task, response, newRequest, completionHandler)
     }
     

Sources/Networking/RequestModifier.swift

@@ -46,14 +46,6 @@ public protocol ImageDownloadRequestModifier {
     func modified(for request: URLRequest) -> URLRequest?
 }
 
-struct NoModifier: ImageDownloadRequestModifier {
-    static let `default` = NoModifier()
-    private init() {}
-    func modified(for request: URLRequest) -> URLRequest? {
-        return request
-    }
-}
-
 /// A wrapper for creating an `ImageDownloadRequestModifier` easier.
 /// This type conforms to `ImageDownloadRequestModifier` and wraps an image modify block.
 public struct AnyModifier: ImageDownloadRequestModifier {

Sources/Networking/SessionDelegate.swift

@@ -221,16 +221,27 @@ extension SessionDelegate: URLSessionDataDelegate {
         onReceiveSessionTaskChallenge.call((session, task, challenge, completionHandler))
     }
     
-    func urlSession(_ session: URLSession, task: URLSessionTask, willPerformHTTPRedirection response: HTTPURLResponse, newRequest request: URLRequest, completionHandler: @escaping (URLRequest?) -> Void)
+    func urlSession(
+        _ session: URLSession,
+        task: URLSessionTask,
+        willPerformHTTPRedirection response: HTTPURLResponse,
+        newRequest request: URLRequest,
+        completionHandler: @escaping (URLRequest?) -> Void)
     {
-        guard let url = response.url,
-            let sessionDataTask = self.task(for: url),
-            let options = Array(sessionDataTask.callbacks).last?.options else {
-                completionHandler(request)
-                return
+        lock.lock()
+        defer { lock.unlock() }
+        guard let sessionDataTask = self.task(for: task),
+              let redirectHandler = Array(sessionDataTask.callbacks).last?.options.redirectHandler else
+        {
+            completionHandler(request)
+            return
         }
         
-        options.redirectHandler.handleHTTPRedirection(for: sessionDataTask, response: response, newRequest: request, completionHandler: completionHandler)
+        redirectHandler.handleHTTPRedirection(
+            for: sessionDataTask,
+            response: response,
+            newRequest: request,
+            completionHandler: completionHandler)
     }
 
     private func onCompleted(task: URLSessionTask, result: Result<(Data, URLResponse?), KingfisherError>) {

Tests/KingfisherTests/KingfisherOptionsInfoTests.swift

@@ -63,7 +63,7 @@ class KingfisherOptionsInfoTests: XCTestCase {
         let testRedirectHandler = TestRedirectHandler()
         let processor = RoundCornerImageProcessor(cornerRadius: 20)
         let serializer = FormatIndicatedCacheSerializer.png
-        let modifier = DefaultImageModifier.default
+        let modifier = AnyImageModifier { i in return i }
 
         var options = KingfisherParsedOptionsInfo([
             .targetCache(cache),
@@ -120,7 +120,7 @@ class KingfisherOptionsInfoTests: XCTestCase {
         XCTAssertTrue(options.redirectHandler is TestRedirectHandler)
         XCTAssertEqual(options.processor.identifier, processor.identifier)
         XCTAssertTrue(options.cacheSerializer is FormatIndicatedCacheSerializer)
-        XCTAssertTrue(options.imageModifier is DefaultImageModifier)
+        XCTAssertTrue(options.imageModifier is AnyImageModifier)
         XCTAssertTrue(options.keepCurrentImageWhileLoading)
         XCTAssertTrue(options.onlyLoadFirstFrame)
         XCTAssertTrue(options.cacheOriginalImage)