Merge pull request #1999 from onevcat/fix/issue-1912

Add `originalDataUsed` to serializer

Sources/Cache/CacheSerializer.swift

@@ -52,6 +52,17 @@ public protocol CacheSerializer {
     /// - Returns: An image deserialized or `nil` when no valid image
     ///            could be deserialized.
     func image(with data: Data, options: KingfisherParsedOptionsInfo) -> KFCrossPlatformImage?
+    
+    /// Whether this serializer prefers to cache the original data in its implementation.
+    /// If `true`, after creating the image from the disk data, Kingfisher will continue to apply the processor to get
+    /// the final image.
+    ///
+    /// By default, it is `false` and the actual processed image is assumed to be serialized to the disk.
+    var originalDataUsed: Bool { get }
+}
+
+public extension CacheSerializer {
+    var originalDataUsed: Bool { false }
 }
 
 /// Represents a basic and default `CacheSerializer` used in Kingfisher disk cache system.
@@ -70,6 +81,10 @@ public struct DefaultCacheSerializer: CacheSerializer {
     /// In that case, the serialization will fall back to creating data from image.
     public var preferCacheOriginalData: Bool = false
 
+    /// Returnes the `preferCacheOriginalData` value. When the original data is used, Kingfisher needs to re-apply the
+    /// processors to get the desired final image.
+    public var originalDataUsed: Bool { preferCacheOriginalData }
+    
     /// Creates a cache serializer that serialize and deserialize images in PNG, JPEG and GIF format.
     ///
     /// - Note:

Sources/General/KingfisherManager.swift

@@ -191,6 +191,7 @@ public class KingfisherManager {
     ///    it returns `nil` and invoke the `completionHandler` after the cached image retrieved. Otherwise, it
     ///    will try to load the `source`, store it in cache, then call `completionHandler`.
     ///
+    @discardableResult
     public func retrieveImage(
         with source: Source,
         options: KingfisherOptionsInfo? = nil,
@@ -554,37 +555,59 @@ public class KingfisherManager {
         if validCache {
             targetCache.retrieveImage(forKey: key, options: options) { result in
                 guard let completionHandler = completionHandler else { return }
-                options.callbackQueue.execute {
-                    result.match(
-                        onSuccess: { cacheResult in
-                            let value: Result<RetrieveImageResult, KingfisherError>
-                            if var image = cacheResult.image {
-                                if image.kf.imageFrameCount != nil && image.kf.imageFrameCount != 1, let data = image.kf.animatedImageData {
-                                    // Always recreate animated image representation since it is possible to be loaded in different options.
-                                    // https://github.com/onevcat/Kingfisher/issues/1923
-                                    image = KingfisherWrapper.animatedImage(data: data, options: options.imageCreatingOptions) ?? .init()
-                                }
-                                if let modifier = options.imageModifier {
-                                    image = modifier.modify(image)
+                
+                // TODO: Optimize it when we can use async across all the project.
+                func checkResultImageAndCallback(_ inputImage: KFCrossPlatformImage) {
+                    var image = inputImage
+                    if image.kf.imageFrameCount != nil && image.kf.imageFrameCount != 1, let data = image.kf.animatedImageData {
+                        // Always recreate animated image representation since it is possible to be loaded in different options.
+                        // https://github.com/onevcat/Kingfisher/issues/1923
+                        image = KingfisherWrapper.animatedImage(data: data, options: options.imageCreatingOptions) ?? .init()
+                    }
+                    if let modifier = options.imageModifier {
+                        image = modifier.modify(image)
+                    }
+                    let value = result.map {
+                        RetrieveImageResult(
+                            image: image,
+                            cacheType: $0.cacheType,
+                            source: source,
+                            originalSource: context.originalSource,
+                            data: { options.cacheSerializer.data(with: image, original: nil) }
+                        )
+                    }
+                    completionHandler(value)
+                }
+                
+                result.match { cacheResult in
+                    options.callbackQueue.execute {
+                        guard let image = cacheResult.image else {
+                            completionHandler(.failure(KingfisherError.cacheError(reason: .imageNotExisting(key: key))))
+                            return
+                        }
+                        
+                        if options.cacheSerializer.originalDataUsed {
+                            let processor = options.processor
+                            (options.processingQueue ?? self.processingQueue).execute {
+                                let item = ImageProcessItem.image(image)
+                                guard let processedImage = processor.process(item: item, options: options) else {
+                                    let error = KingfisherError.processorError(
+                                        reason: .processingFailed(processor: processor, item: item))
+                                    options.callbackQueue.execute { completionHandler(.failure(error)) }
+                                    return
                                 }
-                                value = result.map {
-                                    RetrieveImageResult(
-                                        image: image,
-                                        cacheType: $0.cacheType,
-                                        source: source,
-                                        originalSource: context.originalSource,
-                                        data: { options.cacheSerializer.data(with: image, original: nil) }
-                                    )
+                                options.callbackQueue.execute {
+                                    checkResultImageAndCallback(processedImage)
                                 }
-                            } else {
-                                value = .failure(KingfisherError.cacheError(reason: .imageNotExisting(key: key)))
                             }
-                            completionHandler(value)
-                        },
-                        onFailure: { _ in
-                            completionHandler(.failure(KingfisherError.cacheError(reason: .imageNotExisting(key: key))))
+                        } else {
+                            checkResultImageAndCallback(image)
                         }
-                    )
+                    }
+                } onFailure: { error in
+                    options.callbackQueue.execute {
+                        completionHandler(.failure(KingfisherError.cacheError(reason: .imageNotExisting(key: key))))
+                    }
                 }
             }
             return true

Tests/KingfisherTests/ImageProcessorTests.swift

@@ -43,7 +43,6 @@ class ImageProcessorTests: XCTestCase {
 
         let resultFromImage = downsamplingProcessor.process(item: .image(testImage), options: emptyOption)
         XCTAssertEqual(resultFromImage!.size, CGSize(width: 40, height: 40))
-
     }
 
     func testProcessorConcating() {

Tests/KingfisherTests/KingfisherManagerTests.swift

@@ -465,6 +465,63 @@ class KingfisherManagerTests: XCTestCase {
         waitForExpectations(timeout: 3, handler: nil)
     }
     
+    func testCouldProcessDoNotHappenWhenSerializerCachesTheProcessedData() {
+        let exp = expectation(description: #function)
+        let url = testURLs[0]
+        
+        stub(url, data: testImageData)
+        
+        let s = DefaultCacheSerializer()
+
+        let p1 = SimpleProcessor()
+        let options1: KingfisherOptionsInfo = [.processor(p1), .cacheSerializer(s), .waitForCache]
+        let source = Source.network(url)
+        
+        manager.retrieveImage(with: source, options: options1) { result in
+            XCTAssertTrue(p1.processed)
+            
+            let p2 = SimpleProcessor()
+            let options2: KingfisherOptionsInfo = [.processor(p2), .cacheSerializer(s), .waitForCache]
+            self.manager.cache.clearMemoryCache()
+            
+            self.manager.retrieveImage(with: source, options: options2) { result in
+                XCTAssertEqual(result.value?.cacheType, .disk)
+                XCTAssertFalse(p2.processed)
+                exp.fulfill()
+            }
+        }
+        waitForExpectations(timeout: 3, handler: nil)
+    }
+    
+    func testCouldProcessAgainWhenSerializerCachesOriginalData() {
+        let exp = expectation(description: #function)
+        let url = testURLs[0]
+        
+        stub(url, data: testImageData)
+        
+        var s = DefaultCacheSerializer()
+        s.preferCacheOriginalData = true
+
+        let p1 = SimpleProcessor()
+        let options1: KingfisherOptionsInfo = [.processor(p1), .cacheSerializer(s), .waitForCache]
+        let source = Source.network(url)
+        
+        manager.retrieveImage(with: source, options: options1) { result in
+            XCTAssertTrue(p1.processed)
+            
+            let p2 = SimpleProcessor()
+            let options2: KingfisherOptionsInfo = [.processor(p2), .cacheSerializer(s), .waitForCache]
+            self.manager.cache.clearMemoryCache()
+            
+            self.manager.retrieveImage(with: source, options: options2) { result in
+                XCTAssertEqual(result.value?.cacheType, .disk)
+                XCTAssertTrue(p2.processed)
+                exp.fulfill()
+            }
+        }
+        waitForExpectations(timeout: 3, handler: nil)
+    }
+    
     func testWaitForCacheOnRetrieveImage() {
         let exp = expectation(description: #function)
         let url = testURLs[0]