Fix an issue where original data was stored using the thumbnail optio…

…ns keys

Sources/Nuke/Tasks/TaskFetchOriginalImageData.swift

@@ -159,6 +159,7 @@ final class TaskFetchOriginalImageData: ImagePipelineTask<(Data, URLResponse?)>
 
 extension ImagePipelineTask where Value == (Data, URLResponse?) {
     func storeDataInCacheIfNeeded(_ data: Data) {
+        let request = makeSanitizedRequest()
         guard let dataCache = pipeline.delegate.dataCache(for: request, pipeline: pipeline), shouldStoreDataInDiskCache() else {
             return
         }
@@ -170,6 +171,15 @@ extension ImagePipelineTask where Value == (Data, URLResponse?) {
         }
     }
 
+    /// Returns a request that doesn't contain any information non-related
+    /// to data loading.
+    private func makeSanitizedRequest() -> ImageRequest {
+        var request = request
+        request.processors = []
+        request.userInfo[.thumbnailKey] = nil
+        return request
+    }
+
     private func shouldStoreDataInDiskCache() -> Bool {
         let imageTasks = imageTasks
         guard imageTasks.contains(where: { !$0.request.options.contains(.disableDiskCacheWrites) }) else {

Sources/Nuke/Tasks/TaskLoadImage.swift

@@ -188,15 +188,16 @@ final class TaskLoadImage: ImagePipelineTask<ImageResponse> {
               !(request.url?.isLocalResource ?? false) else {
             return false
         }
+        let isProcessed = !request.processors.isEmpty || request.thumbnail != nil
         switch pipeline.configuration.dataCachePolicy {
         case .automatic:
-            return !request.processors.isEmpty
+            return isProcessed
         case .storeOriginalData:
             return false
         case .storeEncodedImages:
             return true
         case .storeAll:
-            return !request.processors.isEmpty
+            return isProcessed
         }
     }
 

Tests/Helpers.swift

@@ -33,7 +33,7 @@ enum Test {
         return try! ImageDecoders.Default().decode(data)
     }
 
-    static let url = URL(string: "http://test.com")!
+    static let url = URL(string: "http://test.com/example.jpeg")!
 
     static let data: Data = Test.data(name: "fixture", extension: "jpeg")
 

Tests/NukeTests/ImagePipelineTests/ImagePipelineCoalescingTests.swift

@@ -156,6 +156,34 @@ class ImagePipelineCoalescingTests: XCTestCase {
         }
     }
 
+    func testDeduplicationGivenSameURLButDifferentThumbnailOptionsReversed() {
+        dataLoader.queue.isSuspended = true
+
+        // GIVEN requests with the same URLs but one accesses thumbnail
+        // (in this test, order is reversed)
+        let request1 = ImageRequest(url: Test.url)
+        let request2 = ImageRequest(url: Test.url, userInfo: [.thumbnailKey: ImageRequest.ThumbnailOptions(maxPixelSize: 400)])
+
+        // WHEN loading images for those requests
+        expect(pipeline).toLoadImage(with: request1) { result in
+            // THEN
+            guard let image = result.value?.image else { return XCTFail() }
+            XCTAssertEqual(image.sizeInPixels, CGSize(width: 640.0, height: 480.0))
+        }
+        expect(pipeline).toLoadImage(with: request2) { result in
+            // THEN
+            guard let image = result.value?.image else { return XCTFail() }
+            XCTAssertEqual(image.sizeInPixels, CGSize(width: 400, height: 300))
+        }
+
+        dataLoader.queue.isSuspended = false
+
+        wait { _ in
+            // THEN the image data is fetched once
+            XCTAssertEqual(self.dataLoader.createdTaskCount, 1)
+        }
+    }
+
     // MARK: - Processing
 
     func testProcessorsAreDeduplicated() {

Tests/NukeTests/ImagePipelineTests/ImagePipelineDataCacheTests.swift

@@ -46,26 +46,75 @@ class ImagePipelineDataCachingTests: XCTestCase {
         }
     }
 
-    func testGeneratedThumbnailDataIsStoredIncache() {
-        // When
+    func testThumbnailOptionsDataCacheStoresOriginalDataByDefault() throws {
+        // GIVEN
+        pipeline = pipeline.reconfigured {
+            $0.dataCachePolicy = .storeOriginalData
+            $0.imageCache = MockImageCache()
+            $0.debugIsSyncImageEncoding = true
+        }
+
+        // WHEN
         let request = ImageRequest(url: Test.url, userInfo: [.thumbnailKey: ImageRequest.ThumbnailOptions(size: CGSize(width: 400, height: 400), unit: .pixels, contentMode: .aspectFit)])
         expect(pipeline).toLoadImage(with: request)
-
-        // Then
-        wait { _ in
-            XCTAssertFalse(self.dataCache.store.isEmpty)
-
-            XCTAssertNotNil(self.pipeline.cache.cachedData(for: request))
-
-            guard let container = self.pipeline.cache.cachedImage(for: request, caches: [.disk]) else {
-                return XCTFail()
-            }
-            XCTAssertEqual(container.image.sizeInPixels, CGSize(width: 400, height: 300))
-
-            XCTAssertNil(self.pipeline.cache.cachedData(for: ImageRequest(url: Test.url)))
+
+        // THEN
+        wait()
+
+        do { // Check memory cache
+            // Image does not exists for the original image
+            XCTAssertNil(pipeline.cache.cachedImage(for: ImageRequest(url: Test.url), caches: [.memory]))
+
+            // Image exists for thumbnail
+            let thumbnail = try XCTUnwrap(pipeline.cache.cachedImage(for: request, caches: [.memory]))
+            XCTAssertEqual(thumbnail.image.sizeInPixels, CGSize(width: 400, height: 300))
+        }
+
+        do { // Check disk cache
+            // Data exists for the original image
+            let original = try XCTUnwrap(pipeline.cache.cachedImage(for: ImageRequest(url: Test.url), caches: [.disk]))
+            XCTAssertEqual(original.image.sizeInPixels, CGSize(width: 640, height: 480))
+
+            // Data does not exist for thumbnail
+            XCTAssertNil(pipeline.cache.cachedData(for: request))
         }
     }
-
+
+    func testThumbnailOptionsDataCacheStoresOriginalDataWithStoreAllPolicy() throws {
+        // GIVEN
+        pipeline = pipeline.reconfigured {
+            $0.dataCachePolicy = .storeAll
+            $0.imageCache = MockImageCache()
+            $0.debugIsSyncImageEncoding = true
+        }
+
+        // WHEN
+        let request = ImageRequest(url: Test.url, userInfo: [.thumbnailKey: ImageRequest.ThumbnailOptions(size: CGSize(width: 400, height: 400), unit: .pixels, contentMode: .aspectFit)])
+        expect(pipeline).toLoadImage(with: request)
+
+        // THEN
+        wait()
+
+        do { // Check memory cache
+            // Image does not exists for the original image
+            XCTAssertNil(pipeline.cache.cachedImage(for: ImageRequest(url: Test.url), caches: [.memory]))
+
+            // Image exists for thumbnail
+            let thumbnail = try XCTUnwrap(pipeline.cache.cachedImage(for: request, caches: [.memory]))
+            XCTAssertEqual(thumbnail.image.sizeInPixels, CGSize(width: 400, height: 300))
+        }
+
+        do { // Check disk cache
+            // Data exists for the original image
+            let original = try XCTUnwrap(pipeline.cache.cachedImage(for: ImageRequest(url: Test.url), caches: [.disk]))
+            XCTAssertEqual(original.image.sizeInPixels, CGSize(width: 640, height: 480))
+
+            // Data exists for thumbnail
+            let thumbnail = try XCTUnwrap(pipeline.cache.cachedImage(for: request, caches: [.disk]))
+            XCTAssertEqual(thumbnail.image.sizeInPixels, CGSize(width: 400, height: 300))
+        }
+    }
+
     // MARK: - Updating Priority
 
     func testPriorityUpdated() {
@@ -668,4 +717,19 @@ class ImagePipelineDataCachePolicyTests: XCTestCase {
             XCTAssertNil(self.dataCache.cachedData(for: Test.url.absoluteString + "1"), "Expected processed image data to not be stored")
         }
     }
+
+    // MARK: Integration with Thumbnail Feature
+
+    func testOriginalDataStoredWhenThumbnailRequested() {
+        // GIVEN
+        let options = ImageRequest.ThumbnailOptions(maxPixelSize: 400)
+        let request = ImageRequest(url: Test.url, userInfo: [.thumbnailKey: options])
+
+        // WHEN
+        expect(pipeline).toLoadImage(with: request)
+        wait()
+
+        // THEN
+        XCTAssertTrue(dataCache.containsData(for: "http://test.com/example.jpeg"))
+    }
 }

Tests/NukeTests/ImagePipelineTests/ImagePipelineTests.swift

@@ -368,25 +368,25 @@ class ImagePipelineTests: XCTestCase {
 
     func testCacheKeyForRequest() {
         let request = Test.request
-        XCTAssertEqual(pipeline.cache.makeDataCacheKey(for: request), "http://test.com")
+        XCTAssertEqual(pipeline.cache.makeDataCacheKey(for: request), "http://test.com/example.jpeg")
     }
 
     func testCacheKeyForRequestWithProcessors() {
         var request = Test.request
         request.processors = [ImageProcessors.Anonymous(id: "1", { $0 })]
-        XCTAssertEqual(pipeline.cache.makeDataCacheKey(for: request), "http://test.com1")
+        XCTAssertEqual(pipeline.cache.makeDataCacheKey(for: request), "http://test.com/example.jpeg1")
     }
 
     func testCacheKeyForRequestWithThumbnail() {
         let options = ImageRequest.ThumbnailOptions(maxPixelSize: 400)
         let request = ImageRequest(url: Test.url, userInfo: [.thumbnailKey: options])
-        XCTAssertEqual(pipeline.cache.makeDataCacheKey(for: request), "http://test.comcom.github/kean/nuke/thumbnail?maxPixelSize=400.0,options=truetruetruetrue")
+        XCTAssertEqual(pipeline.cache.makeDataCacheKey(for: request), "http://test.com/example.jpegcom.github/kean/nuke/thumbnail?maxPixelSize=400.0,options=truetruetruetrue")
     }
 
     func testCacheKeyForRequestWithThumbnailFlexibleSize() {
         let options = ImageRequest.ThumbnailOptions(size: CGSize(width: 400, height: 400), unit: .pixels, contentMode: .aspectFit)
         let request = ImageRequest(url: Test.url, userInfo: [.thumbnailKey: options])
-        XCTAssertEqual(pipeline.cache.makeDataCacheKey(for: request), "http://test.comcom.github/kean/nuke/thumbnail?width=400.0,height=400.0,contentMode=.aspectFit,options=truetruetruetrue")
+        XCTAssertEqual(pipeline.cache.makeDataCacheKey(for: request), "http://test.com/example.jpegcom.github/kean/nuke/thumbnail?width=400.0,height=400.0,contentMode=.aspectFit,options=truetruetruetrue")
     }
 
     // MARK: - Invalidate