From b9bf2274d3800638b959edb6816d0842c2377c5b Mon Sep 17 00:00:00 2001 From: kean Date: Wed, 18 Aug 2021 15:11:17 -0400 Subject: [PATCH] Fix data cache overwrite issue --- Sources/Internal/Tasks/TaskLoadImage.swift | 18 +++++++------ .../ImagePipelineImageCacheTests.swift | 25 +++++++++++++++++++ Tests/MockDataCache.swift | 5 ++++ Tests/MockImageCache.swift | 5 ++++ 4 files changed, 45 insertions(+), 8 deletions(-) diff --git a/Sources/Internal/Tasks/TaskLoadImage.swift b/Sources/Internal/Tasks/TaskLoadImage.swift index 1215489f4..ff6df89c9 100644 --- a/Sources/Internal/Tasks/TaskLoadImage.swift +++ b/Sources/Internal/Tasks/TaskLoadImage.swift @@ -78,7 +78,7 @@ final class TaskLoadImage: ImagePipelineTask { private func didDecodeCachedData(_ response: ImageResponse?) { if let response = response { - decompressImage(response, isCompleted: true) + decompressImage(response, isCompleted: true, isFromDiskCache: true) } else { fetchImage() } @@ -187,14 +187,14 @@ final class TaskLoadImage: ImagePipelineTask { // MARK: Decompression #if os(macOS) - private func decompressImage(_ response: ImageResponse, isCompleted: Bool) { - storeImageInCaches(response) + private func decompressImage(_ response: ImageResponse, isCompleted: Bool, isFromDiskCache: Bool = false) { + storeImageInCaches(response, isFromDiskCache: isFromDiskCache) send(value: response, isCompleted: isCompleted) // There is no decompression on macOS } #else - private func decompressImage(_ response: ImageResponse, isCompleted: Bool) { + private func decompressImage(_ response: ImageResponse, isCompleted: Bool, isFromDiskCache: Bool = false) { guard isDecompressionNeeded(for: response) else { - storeImageInCaches(response) + storeImageInCaches(response, isFromDiskCache: isFromDiskCache) send(value: response, isCompleted: isCompleted) return } @@ -215,7 +215,7 @@ final class TaskLoadImage: ImagePipelineTask { } self.async { - self.storeImageInCaches(response) + self.storeImageInCaches(response, isFromDiskCache: isFromDiskCache) self.send(value: response, isCompleted: isCompleted) } } @@ -230,14 +230,16 @@ final class TaskLoadImage: ImagePipelineTask { // MARK: Caching - private func storeImageInCaches(_ response: ImageResponse) { + private func storeImageInCaches(_ response: ImageResponse, isFromDiskCache: Bool) { guard subscribers.contains(where: { $0 is ImageTask }) else { return // Only store for direct requests } // Memory cache (ImageCaching) pipeline.cache[request] = response.container // Disk cache (DataCaching) - storeImageInDataCache(response) + if !isFromDiskCache { + storeImageInDataCache(response) + } } private func storeImageInDataCache(_ response: ImageResponse) { diff --git a/Tests/ImagePipelineTests/ImagePipelineImageCacheTests.swift b/Tests/ImagePipelineTests/ImagePipelineImageCacheTests.swift index c30fdc2cb..d5aee72c6 100644 --- a/Tests/ImagePipelineTests/ImagePipelineImageCacheTests.swift +++ b/Tests/ImagePipelineTests/ImagePipelineImageCacheTests.swift @@ -312,6 +312,31 @@ class ImagePipelineCacheLayerPriorityTests: XCTestCase { XCTAssertEqual(dataLoader.createdTaskCount, 0) } + func testPolicyStoreEncodedImagesGivenDataAlreadyStored() { + // GIVEN + pipeline = pipeline.reconfigured { + $0.dataCachePolicy = .storeEncodedImages + } + + let request = ImageRequest(url: Test.url, processors: [MockImageProcessor(id: "p1")]) + pipeline.cache.storeCachedImage(Test.container, for: request, caches: [.disk]) + dataCache.resetCounters() + imageCache.resetCounters() + + // WHEN + let record = expect(pipeline).toLoadImage(with: request) + wait() + XCTAssertNotNil(record.image) + XCTAssertEqual(record.response?.cacheType, .disk) + + // THEN + XCTAssertEqual(imageCache.readCount, 1) + XCTAssertEqual(imageCache.writeCount, 1) + XCTAssertEqual(dataCache.readCount, 1) + XCTAssertEqual(dataCache.writeCount, 0) + XCTAssertEqual(dataLoader.createdTaskCount, 0) + } + // MARK: ImageRequest.Options func testGivenOriginalImageInDiskCacheAndDiskReadsDisabled() { diff --git a/Tests/MockDataCache.swift b/Tests/MockDataCache.swift index 8ba580ff8..8b41d84f7 100644 --- a/Tests/MockDataCache.swift +++ b/Tests/MockDataCache.swift @@ -10,6 +10,11 @@ final class MockDataCache: DataCaching { var readCount = 0 var writeCount = 0 + func resetCounters() { + readCount = 0 + writeCount = 0 + } + func cachedData(for key: String) -> Data? { readCount += 1 return store[key] diff --git a/Tests/MockImageCache.swift b/Tests/MockImageCache.swift index 91f4ad232..468ce770f 100644 --- a/Tests/MockImageCache.swift +++ b/Tests/MockImageCache.swift @@ -14,6 +14,11 @@ class MockImageCache: ImageCaching { init() {} + func resetCounters() { + readCount = 0 + writeCount = 0 + } + subscript(key: ImageCacheKey) -> ImageContainer? { get { queue.sync {