From 0fd15756818a97ecb7bd0ced4715e07c1d963e6c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E1=84=87=E1=85=A1=E1=86=BC=E1=84=89=E1=85=A5=E1=86=BC?= =?UTF-8?q?=E1=84=8B=E1=85=AF=E1=86=AB?= Date: Sun, 22 Sep 2019 23:48:20 +0900 Subject: [PATCH 1/7] Add diskCacheAccessExtendingExpiration option at KingfisherOptionsInfoItem --- Sources/General/KingfisherOptionsInfo.swift | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/Sources/General/KingfisherOptionsInfo.swift b/Sources/General/KingfisherOptionsInfo.swift index a37dca93a..4e8985b72 100644 --- a/Sources/General/KingfisherOptionsInfo.swift +++ b/Sources/General/KingfisherOptionsInfo.swift @@ -212,6 +212,11 @@ public enum KingfisherOptionsInfoItem { /// expiration in its config for all items. If set, the `DiskStorage.Backend` will use this associated /// value to overwrite the config setting for this caching item. case diskCacheExpiration(StorageExpiration) + + /// The expiration extending setting for disk cache. The item expiration time will be incremented by this value after access. + /// By default, the underlying `DiskStorage.Backend` uses the initial cache expiration as extending value: .cacheTime. + /// To disable extending option at all add diskCacheAccessExtendingExpiration(.none) to options. + case diskCacheAccessExtendingExpiration(ExpirationExtending) /// Decides on which queue the image processing should happen. By default, Kingfisher uses a pre-defined serial /// queue to process images. Use this option to change this behavior. For example, specify a `.mainCurrentOrAsync` @@ -259,6 +264,7 @@ public struct KingfisherParsedOptionsInfo { public var memoryCacheExpiration: StorageExpiration? = nil public var memoryCacheAccessExtendingExpiration: ExpirationExtending = .cacheTime public var diskCacheExpiration: StorageExpiration? = nil + public var diskCacheAccessExtendingExpiration: ExpirationExtending = .cacheTime public var processingQueue: CallbackQueue? = nil public var progressiveJPEG: ImageProgressive? = nil @@ -298,6 +304,7 @@ public struct KingfisherParsedOptionsInfo { case .memoryCacheExpiration(let expiration): memoryCacheExpiration = expiration case .memoryCacheAccessExtendingExpiration(let expirationExtending): memoryCacheAccessExtendingExpiration = expirationExtending case .diskCacheExpiration(let expiration): diskCacheExpiration = expiration + case .diskCacheAccessExtendingExpiration(let expiration): diskCacheAccessExtendingExpiration = expiration case .processingQueue(let queue): processingQueue = queue case .progressiveJPEG(let value): progressiveJPEG = value } From 2c3686dfcdcc2e90776605b2518f653afd14f862 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E1=84=87=E1=85=A1=E1=86=BC=E1=84=89=E1=85=A5=E1=86=BC?= =?UTF-8?q?=E1=84=8B=E1=85=AF=E1=86=AB?= Date: Mon, 23 Sep 2019 00:54:58 +0900 Subject: [PATCH 2/7] Use extendingExpiration option when using DiskMemory --- Sources/Cache/DiskStorage.swift | 44 +++++++++++++++------ Sources/Cache/ImageCache.swift | 2 +- Sources/General/Deprecated.swift | 4 +- Sources/General/KingfisherOptionsInfo.swift | 2 +- 4 files changed, 35 insertions(+), 17 deletions(-) diff --git a/Sources/Cache/DiskStorage.swift b/Sources/Cache/DiskStorage.swift index d292d8770..87cf51fca 100644 --- a/Sources/Cache/DiskStorage.swift +++ b/Sources/Cache/DiskStorage.swift @@ -119,11 +119,16 @@ public enum DiskStorage { config.fileManager.createFile(atPath: fileURL.path, contents: data, attributes: attributes) } - func value(forKey key: String) throws -> T? { - return try value(forKey: key, referenceDate: Date(), actuallyLoad: true) + func value(forKey key: String, extendingExpiration: ExpirationExtending) throws -> T? { + return try value(forKey: key, referenceDate: Date(), actuallyLoad: true, extendingExpiration: extendingExpiration) } - func value(forKey key: String, referenceDate: Date, actuallyLoad: Bool) throws -> T? { + func value( + forKey key: String, + referenceDate: Date, + actuallyLoad: Bool, + extendingExpiration: ExpirationExtending) throws -> T? + { let fileManager = config.fileManager let fileURL = cacheFileURL(forKey: key) let filePath = fileURL.path @@ -148,7 +153,7 @@ public enum DiskStorage { do { let data = try Data(contentsOf: fileURL) let obj = try T.fromData(data) - metaChangingQueue.async { meta.extendExpiration(with: fileManager) } + metaChangingQueue.async { meta.extendExpiration(with: fileManager, extendingExpiration: extendingExpiration) } return obj } catch { throw KingfisherError.cacheError(reason: .cannotLoadDataFromDisk(url: fileURL, error: error)) @@ -161,7 +166,7 @@ public enum DiskStorage { func isCached(forKey key: String, referenceDate: Date) -> Bool { do { - guard let _ = try value(forKey: key, referenceDate: referenceDate, actuallyLoad: false) else { + guard let _ = try value(forKey: key, referenceDate: referenceDate, actuallyLoad: false, extendingExpiration: .none) else { return false } return true @@ -404,19 +409,32 @@ extension DiskStorage { return estimatedExpirationDate?.isPast(referenceDate: referenceDate) ?? true } - func extendExpiration(with fileManager: FileManager) { + func extendExpiration(with fileManager: FileManager, extendingExpiration: ExpirationExtending) { guard let lastAccessDate = lastAccessDate, let lastEstimatedExpiration = estimatedExpirationDate else { return } - - let originalExpiration: StorageExpiration = - .seconds(lastEstimatedExpiration.timeIntervalSince(lastAccessDate)) - let attributes: [FileAttributeKey : Any] = [ - .creationDate: Date().fileAttributeDate, - .modificationDate: originalExpiration.estimatedExpirationSinceNow.fileAttributeDate - ] + + let attributes: [FileAttributeKey : Any] + + switch extendingExpiration { + case .none: + // not extending expiration time here + return + case .cacheTime: + let originalExpiration: StorageExpiration = + .seconds(lastEstimatedExpiration.timeIntervalSince(lastAccessDate)) + attributes = [ + .creationDate: Date().fileAttributeDate, + .modificationDate: originalExpiration.estimatedExpirationSinceNow.fileAttributeDate + ] + case .expirationTime(let expirationTime): + attributes = [ + .creationDate: Date().fileAttributeDate, + .modificationDate: expirationTime.estimatedExpirationSinceNow.fileAttributeDate + ] + } try? fileManager.setAttributes(attributes, ofItemAtPath: url.path) } diff --git a/Sources/Cache/ImageCache.swift b/Sources/Cache/ImageCache.swift index 66a1874e9..65157317c 100644 --- a/Sources/Cache/ImageCache.swift +++ b/Sources/Cache/ImageCache.swift @@ -553,7 +553,7 @@ open class ImageCache { loadingQueue.execute { do { var image: Image? = nil - if let data = try self.diskStorage.value(forKey: computedKey) { + if let data = try self.diskStorage.value(forKey: computedKey, extendingExpiration: options.diskCacheAccessExtendingExpiration) { image = options.cacheSerializer.image(with: data, options: options) } callbackQueue.execute { completionHandler(.success(image)) } diff --git a/Sources/General/Deprecated.swift b/Sources/General/Deprecated.swift index fa47a3333..a58a7d7fb 100644 --- a/Sources/General/Deprecated.swift +++ b/Sources/General/Deprecated.swift @@ -369,10 +369,10 @@ extension ImageCache { message: "Use `Result` based `retrieveImageInDiskCache(forKey:options:callbackQueue:completionHandler:)` instead.", renamed: "retrieveImageInDiskCache(forKey:options:callbackQueue:completionHandler:)") open func retrieveImageInDiskCache(forKey key: String, options: KingfisherOptionsInfo? = nil) -> Image? { - let options = options ?? .empty + let options = KingfisherParsedOptionsInfo(options ?? .empty) let computedKey = key.computedKey(with: options.processor.identifier) do { - if let data = try diskStorage.value(forKey: computedKey) { + if let data = try diskStorage.value(forKey: computedKey, extendingExpiration: options.diskCacheAccessExtendingExpiration) { return options.cacheSerializer.image(with: data, options: options) } } catch {} diff --git a/Sources/General/KingfisherOptionsInfo.swift b/Sources/General/KingfisherOptionsInfo.swift index 4e8985b72..4a843d674 100644 --- a/Sources/General/KingfisherOptionsInfo.swift +++ b/Sources/General/KingfisherOptionsInfo.swift @@ -304,7 +304,7 @@ public struct KingfisherParsedOptionsInfo { case .memoryCacheExpiration(let expiration): memoryCacheExpiration = expiration case .memoryCacheAccessExtendingExpiration(let expirationExtending): memoryCacheAccessExtendingExpiration = expirationExtending case .diskCacheExpiration(let expiration): diskCacheExpiration = expiration - case .diskCacheAccessExtendingExpiration(let expiration): diskCacheAccessExtendingExpiration = expiration + case .diskCacheAccessExtendingExpiration(let expirationExtending): diskCacheAccessExtendingExpiration = expirationExtending case .processingQueue(let queue): processingQueue = queue case .progressiveJPEG(let value): progressiveJPEG = value } From 4af85e8f121e146442161a6283764d32e6569b9a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E1=84=87=E1=85=A1=E1=86=BC=E1=84=89=E1=85=A5=E1=86=BC?= =?UTF-8?q?=E1=84=8B=E1=85=AF=E1=86=AB?= Date: Mon, 23 Sep 2019 01:05:12 +0900 Subject: [PATCH 3/7] make ExpirationExtending parameter default value to cacheTime --- Sources/Cache/DiskStorage.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/Cache/DiskStorage.swift b/Sources/Cache/DiskStorage.swift index 87cf51fca..2717010ec 100644 --- a/Sources/Cache/DiskStorage.swift +++ b/Sources/Cache/DiskStorage.swift @@ -119,7 +119,7 @@ public enum DiskStorage { config.fileManager.createFile(atPath: fileURL.path, contents: data, attributes: attributes) } - func value(forKey key: String, extendingExpiration: ExpirationExtending) throws -> T? { + func value(forKey key: String, extendingExpiration: ExpirationExtending = .cacheTime) throws -> T? { return try value(forKey: key, referenceDate: Date(), actuallyLoad: true, extendingExpiration: extendingExpiration) } From 7ba11a7cd34b1f758af2aa4da5d2b2f6f891dd85 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E1=84=87=E1=85=A1=E1=86=BC=E1=84=89=E1=85=A5=E1=86=BC?= =?UTF-8?q?=E1=84=8B=E1=85=AF=E1=86=AB?= Date: Mon, 23 Sep 2019 01:15:47 +0900 Subject: [PATCH 4/7] Add DiskStorageTests --- Tests/KingfisherTests/DiskStorageTests.swift | 22 ++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/Tests/KingfisherTests/DiskStorageTests.swift b/Tests/KingfisherTests/DiskStorageTests.swift index 38acbfb2d..6e6501897 100644 --- a/Tests/KingfisherTests/DiskStorageTests.swift +++ b/Tests/KingfisherTests/DiskStorageTests.swift @@ -131,6 +131,28 @@ class DiskStorageTests: XCTestCase { waitForExpectations(timeout: 2, handler: nil) } + func testNotExtendExpirationByAccessing() { + + let exp = expectation(description: #function) + let now = Date() + try! storage.store(value: "1", forKey: "1", expiration: .seconds(2)) + XCTAssertTrue(storage.isCached(forKey: "1")) + XCTAssertFalse(storage.isCached(forKey: "1", referenceDate: now.addingTimeInterval(3))) + + delay(1) { + let v = try! self.storage.value(forKey: "1", extendingExpiration: .none) + XCTAssertNotNil(v) + // The meta extending happens on its own queue. + self.storage.metaChangingQueue.async { + XCTAssertFalse(self.storage.isCached(forKey: "1", referenceDate: now.addingTimeInterval(3))) + XCTAssertFalse(self.storage.isCached(forKey: "1", referenceDate: now.addingTimeInterval(10))) + exp.fulfill() + } + } + + waitForExpectations(timeout: 2, handler: nil) + } + func testRemoveExpired() { let expiration = StorageExpiration.seconds(1) From 766196b57ffcc6e2c2ab6971e65b0116b5f02f05 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E1=84=87=E1=85=A1=E1=86=BC=E1=84=89=E1=85=A5=E1=86=BC?= =?UTF-8?q?=E1=84=8B=E1=85=AF=E1=86=AB?= Date: Mon, 23 Sep 2019 02:04:19 +0900 Subject: [PATCH 5/7] Add ImageViewExtensionTests --- .../ImageViewExtensionTests.swift | 71 ++++++++++++++++++- 1 file changed, 69 insertions(+), 2 deletions(-) diff --git a/Tests/KingfisherTests/ImageViewExtensionTests.swift b/Tests/KingfisherTests/ImageViewExtensionTests.swift index 97c1fdc38..d9a07aa08 100644 --- a/Tests/KingfisherTests/ImageViewExtensionTests.swift +++ b/Tests/KingfisherTests/ImageViewExtensionTests.swift @@ -653,7 +653,7 @@ class ImageViewExtensionTests: XCTestCase { waitForExpectations(timeout: 3, handler: nil) } - func testImageCacheExtendingExpirationTask() { + func testMemoryImageCacheExtendingExpirationTask() { let exp = expectation(description: #function) let url = testURLs[0] stub(url, data: testImageData) @@ -688,7 +688,7 @@ class ImageViewExtensionTests: XCTestCase { waitForExpectations(timeout: 3, handler: nil) } - func testImageCacheNotExtendingExpirationTask() { + func testMemoryImageCacheNotExtendingExpirationTask() { let exp = expectation(description: #function) let url = testURLs[0] stub(url, data: testImageData) @@ -720,6 +720,73 @@ class ImageViewExtensionTests: XCTestCase { waitForExpectations(timeout: 3, handler: nil) } + + func testDiskImageCacheExtendingExpirationTask() { + let exp = expectation(description: #function) + let url = testURLs[0] + stub(url, data: testImageData) + + let options: KingfisherOptionsInfo = [.memoryCacheExpiration(.expired), + .diskCacheExpiration(.seconds(1)), + .diskCacheAccessExtendingExpiration(.cacheTime)] // default option + + imageView.kf.setImage(with: url, options: options) { result in + XCTAssertNotNil(result.value?.image) + XCTAssertTrue(result.value!.cacheType == .none) + + delay(1, block: { + self.imageView.kf.setImage(with: url, options: options) { result in + XCTAssertNotNil(result.value?.image) + XCTAssertTrue(result.value!.cacheType == .disk) + delay(1, block: { + self.imageView.kf.setImage(with: url, options: options) { result in + XCTAssertNotNil(result.value?.image) + XCTAssertTrue(result.value!.cacheType == .disk) + + exp.fulfill() + } + }) + } + }) + } + + waitForExpectations(timeout: 3, handler: nil) + } + + func testDiskImageCacheNotExtendingExpirationTask() { + let exp = expectation(description: #function) + let url = testURLs[0] + stub(url, data: testImageData) + + let options: KingfisherOptionsInfo = [.memoryCacheExpiration(.expired), + .diskCacheExpiration(.seconds(1)), + .diskCacheAccessExtendingExpiration(.none)] + + imageView.kf.setImage(with: url, options: options) { result in + XCTAssertNotNil(result.value?.image) + XCTAssertTrue(result.value!.cacheType == .none) + + delay(1, block: { + self.imageView.kf.setImage(with: url, options: options) { result in + XCTAssertNotNil(result.value?.image) + XCTAssertTrue(result.value!.cacheType == .disk) + + delay(1, block: { + self.imageView.kf.setImage(with: url, options: options) { result in + XCTAssertNotNil(result.value?.image) + XCTAssertTrue(result.value!.cacheType == .none) + + exp.fulfill() + } + }) + } + }) + } + + waitForExpectations(timeout: 3, handler: nil) + } + + } extension View: Placeholder {} From 4d482f0b5fe6b06febc52323b1e4106924ec2480 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E1=84=87=E1=85=A1=E1=86=BC=E1=84=89=E1=85=A5=E1=86=BC?= =?UTF-8?q?=E1=84=8B=E1=85=AF=E1=86=AB?= Date: Mon, 23 Sep 2019 22:26:02 +0900 Subject: [PATCH 6/7] Set disk expiration extending option more generously --- Tests/KingfisherTests/ImageViewExtensionTests.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tests/KingfisherTests/ImageViewExtensionTests.swift b/Tests/KingfisherTests/ImageViewExtensionTests.swift index d9a07aa08..8eaa59174 100644 --- a/Tests/KingfisherTests/ImageViewExtensionTests.swift +++ b/Tests/KingfisherTests/ImageViewExtensionTests.swift @@ -728,7 +728,7 @@ class ImageViewExtensionTests: XCTestCase { let options: KingfisherOptionsInfo = [.memoryCacheExpiration(.expired), .diskCacheExpiration(.seconds(1)), - .diskCacheAccessExtendingExpiration(.cacheTime)] // default option + .diskCacheAccessExtendingExpiration(.expirationTime(.seconds(100)))] imageView.kf.setImage(with: url, options: options) { result in XCTAssertNotNil(result.value?.image) From ad420ec559ac54d59a60627454e51647e7457019 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E1=84=87=E1=85=A1=E1=86=BC=E1=84=89=E1=85=A5=E1=86=BC?= =?UTF-8?q?=E1=84=8B=E1=85=AF=E1=86=AB?= Date: Mon, 23 Sep 2019 22:42:08 +0900 Subject: [PATCH 7/7] Change delay in diskCache Tests --- Tests/KingfisherTests/ImageViewExtensionTests.swift | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/Tests/KingfisherTests/ImageViewExtensionTests.swift b/Tests/KingfisherTests/ImageViewExtensionTests.swift index 8eaa59174..70b6df7c1 100644 --- a/Tests/KingfisherTests/ImageViewExtensionTests.swift +++ b/Tests/KingfisherTests/ImageViewExtensionTests.swift @@ -727,7 +727,7 @@ class ImageViewExtensionTests: XCTestCase { stub(url, data: testImageData) let options: KingfisherOptionsInfo = [.memoryCacheExpiration(.expired), - .diskCacheExpiration(.seconds(1)), + .diskCacheExpiration(.seconds(2)), .diskCacheAccessExtendingExpiration(.expirationTime(.seconds(100)))] imageView.kf.setImage(with: url, options: options) { result in @@ -738,7 +738,7 @@ class ImageViewExtensionTests: XCTestCase { self.imageView.kf.setImage(with: url, options: options) { result in XCTAssertNotNil(result.value?.image) XCTAssertTrue(result.value!.cacheType == .disk) - delay(1, block: { + delay(2, block: { self.imageView.kf.setImage(with: url, options: options) { result in XCTAssertNotNil(result.value?.image) XCTAssertTrue(result.value!.cacheType == .disk) @@ -750,7 +750,7 @@ class ImageViewExtensionTests: XCTestCase { }) } - waitForExpectations(timeout: 3, handler: nil) + waitForExpectations(timeout: 5, handler: nil) } func testDiskImageCacheNotExtendingExpirationTask() { @@ -759,7 +759,7 @@ class ImageViewExtensionTests: XCTestCase { stub(url, data: testImageData) let options: KingfisherOptionsInfo = [.memoryCacheExpiration(.expired), - .diskCacheExpiration(.seconds(1)), + .diskCacheExpiration(.seconds(2)), .diskCacheAccessExtendingExpiration(.none)] imageView.kf.setImage(with: url, options: options) { result in @@ -771,7 +771,7 @@ class ImageViewExtensionTests: XCTestCase { XCTAssertNotNil(result.value?.image) XCTAssertTrue(result.value!.cacheType == .disk) - delay(1, block: { + delay(2, block: { self.imageView.kf.setImage(with: url, options: options) { result in XCTAssertNotNil(result.value?.image) XCTAssertTrue(result.value!.cacheType == .none) @@ -783,7 +783,7 @@ class ImageViewExtensionTests: XCTestCase { }) } - waitForExpectations(timeout: 3, handler: nil) + waitForExpectations(timeout: 5, handler: nil) }