From f8c16f21209632beeb8bcd7551ad430eb0365e8f Mon Sep 17 00:00:00 2001 From: ivanlisovyi Date: Thu, 8 Jul 2021 15:05:33 +0200 Subject: [PATCH 1/4] Implement remove by key functionality --- .../xcshareddata/swiftpm/Package.resolved | 26 ++++++++++++++++--- Package.swift | 4 +-- .../Carlos/CacheLevels/BatchAllCache.swift | 9 +++++++ Sources/Carlos/CacheLevels/Composed.swift | 5 ++++ Sources/Carlos/CacheLevels/Conditioned.swift | 1 + .../Carlos/CacheLevels/DiskCacheLevel.swift | 23 ++++++++++++++++ Sources/Carlos/CacheLevels/Fetcher.swift | 5 ++++ .../Carlos/CacheLevels/MemoryCacheLevel.swift | 19 ++++++++++---- .../NSUserDefaultsCacheLevel.swift | 7 +++++ Sources/Carlos/CacheLevels/PoolCache.swift | 4 +++ Sources/Carlos/Carlos.swift | 2 ++ Sources/Carlos/Core/BasicCache.swift | 7 +++++ .../Carlos/Operations/KeyTransformation.swift | 5 ++++ Sources/Carlos/Operations/Normalize.swift | 1 + Sources/Carlos/Operations/PostProcess.swift | 1 + Sources/Carlos/Operations/SwitchCache.swift | 6 +++++ .../Operations/ValueTransformation.swift | 1 + .../ConditionedOutputProcessing.swift | 1 + .../ConditionedValueTransformation.swift | 1 + Tests/CarlosTests/BasicCacheTests.swift | 9 +++++++ Tests/CarlosTests/Fakes/CacheLevelFake.swift | 16 ++++++++++++ 21 files changed, 142 insertions(+), 11 deletions(-) diff --git a/Carlos.xcworkspace/xcshareddata/swiftpm/Package.resolved b/Carlos.xcworkspace/xcshareddata/swiftpm/Package.resolved index 2fe74c3..f99c900 100644 --- a/Carlos.xcworkspace/xcshareddata/swiftpm/Package.resolved +++ b/Carlos.xcworkspace/xcshareddata/swiftpm/Package.resolved @@ -1,13 +1,31 @@ { "object": { "pins": [ + { + "package": "CwlCatchException", + "repositoryURL": "https://github.com/mattgallagher/CwlCatchException.git", + "state": { + "branch": null, + "revision": "682841464136f8c66e04afe5dbd01ab51a3a56f2", + "version": "2.1.0" + } + }, + { + "package": "CwlPreconditionTesting", + "repositoryURL": "https://github.com/mattgallagher/CwlPreconditionTesting.git", + "state": { + "branch": null, + "revision": "02b7a39a99c4da27abe03cab2053a9034379639f", + "version": "2.0.0" + } + }, { "package": "Nimble", "repositoryURL": "https://github.com/Quick/Nimble.git", "state": { "branch": null, - "revision": "2b1809051b4a65c1d7f5233331daa24572cd7fca", - "version": "8.1.1" + "revision": "af1730dde4e6c0d45bf01b99f8a41713ce536790", + "version": "9.2.0" } }, { @@ -15,8 +33,8 @@ "repositoryURL": "https://github.com/Quick/Quick.git", "state": { "branch": null, - "revision": "0038bcbab4292f3b028632556507c124e5ba69f3", - "version": "3.0.0" + "revision": "bd86ca0141e3cfb333546de5a11ede63f0c4a0e6", + "version": "4.0.0" } } ] diff --git a/Package.swift b/Package.swift index 9fa880f..8789b4a 100644 --- a/Package.swift +++ b/Package.swift @@ -17,8 +17,8 @@ let package = Package( ) ], dependencies: [ - .package(url: "https://github.com/Quick/Quick.git", .upToNextMajor(from: "3.0.0")), - .package(url: "https://github.com/Quick/Nimble.git", .upToNextMajor(from: "8.1.0")) + .package(url: "https://github.com/Quick/Quick.git", .upToNextMajor(from: "4.0.0")), + .package(url: "https://github.com/Quick/Nimble.git", .upToNextMajor(from: "9.2.0")) ], targets: [ .target( diff --git a/Sources/Carlos/CacheLevels/BatchAllCache.swift b/Sources/Carlos/CacheLevels/BatchAllCache.swift index b680148..fda1401 100644 --- a/Sources/Carlos/CacheLevels/BatchAllCache.swift +++ b/Sources/Carlos/CacheLevels/BatchAllCache.swift @@ -42,6 +42,15 @@ public final class BatchAllCache: CacheLeve } } + public func remove(_ key: KeyType) -> AnyPublisher { + let all = key.map(cache.remove) + return all.publisher + .setFailureType(to: Error.self) + .collect(all.count) + .map { _ in () } + .eraseToAnyPublisher() + } + public func clear() { cache.clear() } diff --git a/Sources/Carlos/CacheLevels/Composed.swift b/Sources/Carlos/CacheLevels/Composed.swift index 630890c..94bf172 100644 --- a/Sources/Carlos/CacheLevels/Composed.swift +++ b/Sources/Carlos/CacheLevels/Composed.swift @@ -35,6 +35,11 @@ extension CacheLevel { .map { _ in () } .eraseToAnyPublisher() }, + removeClosure: { key in + Publishers.Zip(self.remove(key), cache.remove(key)) + .map { _ in () } + .eraseToAnyPublisher() + }, clearClosure: { self.clear() cache.clear() diff --git a/Sources/Carlos/CacheLevels/Conditioned.swift b/Sources/Carlos/CacheLevels/Conditioned.swift index 7e56e99..2cd8595 100644 --- a/Sources/Carlos/CacheLevels/Conditioned.swift +++ b/Sources/Carlos/CacheLevels/Conditioned.swift @@ -15,6 +15,7 @@ extension CacheLevel { BasicCache( getClosure: conditionedClosure(get, condition: condition), setClosure: set, + removeClosure: remove, clearClosure: clear, memoryClosure: onMemoryWarning ) diff --git a/Sources/Carlos/CacheLevels/DiskCacheLevel.swift b/Sources/Carlos/CacheLevels/DiskCacheLevel.swift index befa173..5f578d7 100644 --- a/Sources/Carlos/CacheLevels/DiskCacheLevel.swift +++ b/Sources/Carlos/CacheLevels/DiskCacheLevel.swift @@ -117,6 +117,29 @@ public final class DiskCacheLevel: CacheLevel .eraseToAnyPublisher() } + public func remove(_ key: K) -> AnyPublisher { + AnyPublisher.create { [weak self] promise in + guard let path = self?.pathForKey(key) else { + return + } + + do { + Logger.log("DiskCacheLevel| Removing \(key.toString()) at path: \(path) on the disk cache", .info) + try self?.fileManager.removeItem(atPath: path) + + self?.calculateSize() + + promise(.success(())) + } catch { + Logger.log("DiskCacheLevel| Failed to remove \(key.toString()) at path: \(path) on the disk cache", .error) + + promise(.failure(error)) + } + } + .subscribe(on: cacheQueue) + .eraseToAnyPublisher() + } + /** Asynchronously clears the contents of the cache diff --git a/Sources/Carlos/CacheLevels/Fetcher.swift b/Sources/Carlos/CacheLevels/Fetcher.swift index e9b2b43..3282930 100644 --- a/Sources/Carlos/CacheLevels/Fetcher.swift +++ b/Sources/Carlos/CacheLevels/Fetcher.swift @@ -13,6 +13,11 @@ public protocol Fetcher: CacheLevel {} /// Extending the Fetcher protocol to have a default no-op implementation for clear, onMemoryWarning and set extension Fetcher { + /// No-op + public func remove(_ key: KeyType) -> AnyPublisher { + Empty(completeImmediately: true).eraseToAnyPublisher() + } + /// No-op public func clear() {} diff --git a/Sources/Carlos/CacheLevels/MemoryCacheLevel.swift b/Sources/Carlos/CacheLevels/MemoryCacheLevel.swift index e74681f..04546f7 100644 --- a/Sources/Carlos/CacheLevels/MemoryCacheLevel.swift +++ b/Sources/Carlos/CacheLevels/MemoryCacheLevel.swift @@ -39,6 +39,16 @@ public final class MemoryCacheLevel: CacheLe .eraseToAnyPublisher() } + public func remove(_ key: K) -> AnyPublisher { + AnyPublisher.create { [weak self] promise in + Logger.log("MemoryCacheLevel| Removing \(key.toString()) on memory level.", .info) + + self?.internalCache.removeObject(forKey: key.toString() as NSString) + + promise(.success(())) + } + } + /** Clears the contents of the cache */ @@ -53,12 +63,11 @@ public final class MemoryCacheLevel: CacheLe - parameter key: The key for the value */ public func set(_ value: T, forKey key: K) -> AnyPublisher { - Logger.log("MemoryCacheLevel| Setting a value for the key \(key.toString()) on the memory cache \(self).", .info) - internalCache.setObject(value, forKey: key.toString() as NSString, cost: value.cost) + AnyPublisher.create { [weak self] promise in + self?.internalCache.setObject(value, forKey: key.toString() as NSString, cost: value.cost) - return Just(()) - .setFailureType(to: Error.self) - .eraseToAnyPublisher() + promise(.success(())) + } } /** diff --git a/Sources/Carlos/CacheLevels/NSUserDefaultsCacheLevel.swift b/Sources/Carlos/CacheLevels/NSUserDefaultsCacheLevel.swift index 3094fd0..8b1f4ee 100644 --- a/Sources/Carlos/CacheLevels/NSUserDefaultsCacheLevel.swift +++ b/Sources/Carlos/CacheLevels/NSUserDefaultsCacheLevel.swift @@ -111,6 +111,13 @@ public final class NSUserDefaultsCacheLevel: .eraseToAnyPublisher() } + public func remove(_ key: K) -> AnyPublisher { + AnyPublisher.create { [weak self] promise in + self?.userDefaults.removeObject(forKey: key.toString()) + promise(.success(())) + } + } + /** Completely clears the contents of this cache diff --git a/Sources/Carlos/CacheLevels/PoolCache.swift b/Sources/Carlos/CacheLevels/PoolCache.swift index 640c369..ffa6d52 100644 --- a/Sources/Carlos/CacheLevels/PoolCache.swift +++ b/Sources/Carlos/CacheLevels/PoolCache.swift @@ -69,6 +69,10 @@ public final class PoolCache: CacheLevel where C.KeyType: Hashabl internalCache.set(value, forKey: key) } + public func remove(_ key: C.KeyType) -> AnyPublisher { + internalCache.remove(key) + } + /// Clears the managed cache public func clear() { internalCache.clear() diff --git a/Sources/Carlos/Carlos.swift b/Sources/Carlos/Carlos.swift index b1cbbe5..2a72386 100644 --- a/Sources/Carlos/Carlos.swift +++ b/Sources/Carlos/Carlos.swift @@ -29,6 +29,8 @@ public protocol CacheLevel: AnyObject { /// - Returns: A `Publisher that will reflect the status of the set operation func set(_ value: OutputType, forKey key: KeyType) -> AnyPublisher + func remove(_ key: KeyType) -> AnyPublisher + /// Asks to clear the cache level func clear() diff --git a/Sources/Carlos/Core/BasicCache.swift b/Sources/Carlos/Core/BasicCache.swift index 9303591..5e4a4ce 100644 --- a/Sources/Carlos/Core/BasicCache.swift +++ b/Sources/Carlos/Core/BasicCache.swift @@ -8,6 +8,7 @@ public final class BasicCache: CacheLevel { private let getClosure: (_ key: A) -> AnyPublisher private let setClosure: (_ value: B, _ key: A) -> AnyPublisher + private let removeClosure: (_ key: A) -> AnyPublisher private let clearClosure: () -> Void private let memoryClosure: () -> Void @@ -22,11 +23,13 @@ public final class BasicCache: CacheLevel { public init( getClosure: @escaping (_ key: A) -> AnyPublisher, setClosure: @escaping (_ value: B, _ key: A) -> AnyPublisher, + removeClosure: @escaping (_ key: A) -> AnyPublisher, clearClosure: @escaping () -> Void, memoryClosure: @escaping () -> Void ) { self.getClosure = getClosure self.setClosure = setClosure + self.removeClosure = removeClosure self.clearClosure = clearClosure self.memoryClosure = memoryClosure } @@ -54,6 +57,10 @@ public final class BasicCache: CacheLevel { setClosure(value, key) } + public func remove(_ key: A) -> AnyPublisher { + removeClosure(key) + } + /** Asks the cache to clear its contents diff --git a/Sources/Carlos/Operations/KeyTransformation.swift b/Sources/Carlos/Operations/KeyTransformation.swift index fea876a..c05fffa 100644 --- a/Sources/Carlos/Operations/KeyTransformation.swift +++ b/Sources/Carlos/Operations/KeyTransformation.swift @@ -25,6 +25,11 @@ extension CacheLevel { } .eraseToAnyPublisher() }, + removeClosure: { + transformer.transform($0) + .flatMap(self.remove) + .eraseToAnyPublisher() + }, clearClosure: clear, memoryClosure: onMemoryWarning ) diff --git a/Sources/Carlos/Operations/Normalize.swift b/Sources/Carlos/Operations/Normalize.swift index df80174..4debae7 100644 --- a/Sources/Carlos/Operations/Normalize.swift +++ b/Sources/Carlos/Operations/Normalize.swift @@ -14,6 +14,7 @@ extension CacheLevel { return BasicCache( getClosure: get, setClosure: set, + removeClosure: remove, clearClosure: clear, memoryClosure: onMemoryWarning ) diff --git a/Sources/Carlos/Operations/PostProcess.swift b/Sources/Carlos/Operations/PostProcess.swift index b43bcfb..05fabbc 100644 --- a/Sources/Carlos/Operations/PostProcess.swift +++ b/Sources/Carlos/Operations/PostProcess.swift @@ -18,6 +18,7 @@ extension CacheLevel { .eraseToAnyPublisher() }, setClosure: set, + removeClosure: remove, clearClosure: clear, memoryClosure: onMemoryWarning ) diff --git a/Sources/Carlos/Operations/SwitchCache.swift b/Sources/Carlos/Operations/SwitchCache.swift index a628996..a9f156b 100644 --- a/Sources/Carlos/Operations/SwitchCache.swift +++ b/Sources/Carlos/Operations/SwitchCache.swift @@ -1,4 +1,5 @@ import Foundation +import Combine /// Convenience enumeration to specify which of two switched cache levels should be used for a get or set operation public enum CacheLevelSwitchResult { @@ -36,6 +37,11 @@ public func switchLevels(cacheA: A, cacheB: B, swi return cacheB.set(value, forKey: key) } }, + removeClosure: { + Publishers.Zip(cacheA.remove($0), cacheB.remove($0)) + .map { _ in () } + .eraseToAnyPublisher() + }, clearClosure: { cacheA.clear() cacheB.clear() diff --git a/Sources/Carlos/Operations/ValueTransformation.swift b/Sources/Carlos/Operations/ValueTransformation.swift index 3face2e..3cdf463 100644 --- a/Sources/Carlos/Operations/ValueTransformation.swift +++ b/Sources/Carlos/Operations/ValueTransformation.swift @@ -25,6 +25,7 @@ extension CacheLevel { } .eraseToAnyPublisher() }, + removeClosure: remove, clearClosure: clear, memoryClosure: onMemoryWarning ) diff --git a/Sources/Carlos/Transformers/ConditionedOutputProcessing.swift b/Sources/Carlos/Transformers/ConditionedOutputProcessing.swift index f799165..0a85890 100644 --- a/Sources/Carlos/Transformers/ConditionedOutputProcessing.swift +++ b/Sources/Carlos/Transformers/ConditionedOutputProcessing.swift @@ -20,6 +20,7 @@ extension CacheLevel { .eraseToAnyPublisher() }, setClosure: set, + removeClosure: remove, clearClosure: clear, memoryClosure: onMemoryWarning ) diff --git a/Sources/Carlos/Transformers/ConditionedValueTransformation.swift b/Sources/Carlos/Transformers/ConditionedValueTransformation.swift index 086cf1f..d66f153 100644 --- a/Sources/Carlos/Transformers/ConditionedValueTransformation.swift +++ b/Sources/Carlos/Transformers/ConditionedValueTransformation.swift @@ -27,6 +27,7 @@ extension CacheLevel { } .eraseToAnyPublisher() }, + removeClosure: remove, clearClosure: clear, memoryClosure: onMemoryWarning ) diff --git a/Tests/CarlosTests/BasicCacheTests.swift b/Tests/CarlosTests/BasicCacheTests.swift index fd5f6af..73c2cad 100644 --- a/Tests/CarlosTests/BasicCacheTests.swift +++ b/Tests/CarlosTests/BasicCacheTests.swift @@ -19,11 +19,15 @@ final class BasicCacheTests: QuickSpec { var numberOfTimesCalledOnMemoryWarning = 0 var numberOfTimesCalledGet = 0 var numberOfTimesCalledSet = 0 + var numberOfTimesCalledRemove = 0 var didSetKey: String? var didSetValue: Int? var didGetKey: String? + var removeKey: String? + var getSubject: PassthroughSubject! var setSubject: PassthroughSubject! + var removeSubject: PassthroughSubject! var cancellable: AnyCancellable? @@ -50,6 +54,11 @@ final class BasicCacheTests: QuickSpec { return setSubject.eraseToAnyPublisher() }, + removeClosure: { key in + numberOfTimesCalledRemove += 1 + removeKey = key + return removeSubject.eraseToAnyPublisher() + }, clearClosure: { numberOfTimesCalledClear += 1 }, diff --git a/Tests/CarlosTests/Fakes/CacheLevelFake.swift b/Tests/CarlosTests/Fakes/CacheLevelFake.swift index e518f54..fa9014d 100644 --- a/Tests/CarlosTests/Fakes/CacheLevelFake.swift +++ b/Tests/CarlosTests/Fakes/CacheLevelFake.swift @@ -7,6 +7,10 @@ class CacheLevelFake: CacheLevel { typealias KeyType = A typealias OutputType = B + var numberOfTimesRemoveCalled = 0 + var removeKey: KeyType? + var removeSubject: PassthroughSubject! + init() {} // MARK: Get @@ -60,11 +64,23 @@ class CacheLevelFake: CacheLevel { return newSubject.eraseToAnyPublisher() } + // MARK: - Remove + + func remove(_ key: A) -> AnyPublisher { + numberOfTimesRemoveCalled += 1 + removeKey = key + + return removeSubject.eraseToAnyPublisher() + } + + // MARK: - Clear + var numberOfTimesCalledClear = 0 func clear() { numberOfTimesCalledClear += 1 } + // MARK: - On Memory Warning var numberOfTimesCalledOnMemoryWarning = 0 func onMemoryWarning() { numberOfTimesCalledOnMemoryWarning += 1 From 1bf34f495f566a9fb0bede6bb531ad6cf205707b Mon Sep 17 00:00:00 2001 From: ivanlisovyi Date: Thu, 8 Jul 2021 15:40:46 +0200 Subject: [PATCH 2/4] Add unit tests for remove functionality --- Tests/CarlosTests/DiskCacheTests.swift | 25 +++++++++ Tests/CarlosTests/MemoryCacheLevelTests.swift | 52 +++++++++++++------ .../NSUserDefaultsCacheLevelTests.swift | 27 +++++++++- Tests/CarlosTests/NetworkFetcherTests.swift | 2 +- 4 files changed, 89 insertions(+), 17 deletions(-) diff --git a/Tests/CarlosTests/DiskCacheTests.swift b/Tests/CarlosTests/DiskCacheTests.swift index a632681..7800517 100644 --- a/Tests/CarlosTests/DiskCacheTests.swift +++ b/Tests/CarlosTests/DiskCacheTests.swift @@ -221,6 +221,31 @@ final class DiskCacheTests: QuickSpec { } } + context("when calling remove") { + var result = false + let key = "test-key" + + beforeEach { + result = false + + cache.clear() + } + + it("shall remove object from cache for given key") { + cache.set("value".data(using: .utf8)! as NSData, forKey: key) + .flatMap { cache.remove(key) } + .sink( + receiveCompletion: { _ in }, + receiveValue: { _ in + result = true + } + ) + .store(in: &cancellables) + + expect(result).toEventually(beTrue()) + } + } + context("when calling clear") { beforeEach { result = nil diff --git a/Tests/CarlosTests/MemoryCacheLevelTests.swift b/Tests/CarlosTests/MemoryCacheLevelTests.swift index db0fc6c..57ce00b 100644 --- a/Tests/CarlosTests/MemoryCacheLevelTests.swift +++ b/Tests/CarlosTests/MemoryCacheLevelTests.swift @@ -127,22 +127,22 @@ final class MemoryCacheLevelTests: QuickSpec { context("when setting a different value for the same key") { let newValue = "another value" - beforeEach { - _ = cache.set(newValue as NSString, forKey: key) - } - - context("when calling get") { - beforeEach { - cancellable = cache.get(key).sink(receiveCompletion: { completion in - if case .failure = completion { - failureSentinel = true + it("should succeed with the overwritten value") { + cancellable = + cache.set(newValue as NSString, forKey: key) + .flatMap { _ in cache.get(key) } + .sink( + receiveCompletion: { completion in + if case .failure = completion { + failureSentinel = true + } + }, + receiveValue: { + result = $0 } - }, receiveValue: { result = $0 }) - } + ) - it("should succeed with the overwritten value") { - expect(result).toEventually(equal(newValue as NSString)) - } + expect(result).toEventually(equal(newValue as NSString)) } } @@ -171,7 +171,7 @@ final class MemoryCacheLevelTests: QuickSpec { }, receiveValue: { _ in }) } - expect(evictedAtLeastOne).toEventually(beTrue(), timeout: 5) + expect(evictedAtLeastOne).toEventually(beTrue(), timeout: .seconds(5)) } } @@ -229,6 +229,28 @@ final class MemoryCacheLevelTests: QuickSpec { } } } + + context("when calling remove") { + var result = false + let key = "test-key" + + beforeEach { + result = false + } + + it("shall remove object from cache for given key") { + cancellable = cache.set("value", forKey: key) + .flatMap { cache.remove(key) } + .sink( + receiveCompletion: { _ in }, + receiveValue: { _ in + result = true + } + ) + + expect(result).toEventually(beTrue()) + } + } } } } diff --git a/Tests/CarlosTests/NSUserDefaultsCacheLevelTests.swift b/Tests/CarlosTests/NSUserDefaultsCacheLevelTests.swift index f28a1ae..cbd2a9f 100644 --- a/Tests/CarlosTests/NSUserDefaultsCacheLevelTests.swift +++ b/Tests/CarlosTests/NSUserDefaultsCacheLevelTests.swift @@ -161,11 +161,36 @@ final class NSUserDefaultsCacheLevelTests: QuickSpec { } it("should succeed with the overwritten value") { - expect(result).toEventually(equal(newValue as NSString), timeout: 5) + expect(result).toEventually(equal(newValue as NSString), timeout: .seconds(5)) } } } + context("when calling remove") { + var result = false + let key = "test-key" + + beforeEach { + result = false + + cache.clear() + } + + it("shall remove object from cache for given key") { + cache.set("value", forKey: key) + .flatMap { cache.remove(key) } + .sink( + receiveCompletion: { _ in }, + receiveValue: { _ in + result = true + } + ) + .store(in: &cancellables) + + expect(result).toEventually(beTrue()) + } + } + context("when calling clear") { beforeEach { failureSentinel = nil diff --git a/Tests/CarlosTests/NetworkFetcherTests.swift b/Tests/CarlosTests/NetworkFetcherTests.swift index 58a5b7d..8f541ca 100644 --- a/Tests/CarlosTests/NetworkFetcherTests.swift +++ b/Tests/CarlosTests/NetworkFetcherTests.swift @@ -47,7 +47,7 @@ final class NetworkFetcherTests: QuickSpec { } it("should complete all requests") { - expect(finished).toEventually(equal(simultaneousRequests), timeout: 10) + expect(finished).toEventually(equal(simultaneousRequests), timeout: .seconds(10)) } } } From 4107ed75d12c8db1d6201e5a21cb6e7ca464871d Mon Sep 17 00:00:00 2001 From: ivanlisovyi Date: Thu, 8 Jul 2021 15:43:15 +0200 Subject: [PATCH 3/4] Add code documentation for remove functionality --- Sources/Carlos/Carlos.swift | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/Sources/Carlos/Carlos.swift b/Sources/Carlos/Carlos.swift index 2a72386..e800217 100644 --- a/Sources/Carlos/Carlos.swift +++ b/Sources/Carlos/Carlos.swift @@ -26,9 +26,14 @@ public protocol CacheLevel: AnyObject { /// - Parameter value: The bytes to set on the cache level /// - Parameter key: The key of the value you're trying to set /// - /// - Returns: A `Publisher that will reflect the status of the set operation + /// - Returns: A `Publisher` that will reflect the status of the set operation func set(_ value: OutputType, forKey key: KeyType) -> AnyPublisher + /// Remove value from cache for a given key. + /// + /// - Parameter key: They key of the value to be removed + /// + /// - Returns: A `Publisher` that reflects a status of the remove operation. func remove(_ key: KeyType) -> AnyPublisher /// Asks to clear the cache level From 2af9441b84b2522e407e5a347e23f1162fe9767c Mon Sep 17 00:00:00 2001 From: ivanlisovyi Date: Thu, 8 Jul 2021 17:47:43 +0200 Subject: [PATCH 4/4] Fix potential memory leaks --- Sources/Carlos/CacheLevels/Composed.swift | 32 +++++++++++++------ .../Carlos/CacheLevels/DiskCacheLevel.swift | 4 +-- Sources/Carlos/CacheLevels/PoolCache.swift | 6 ++-- .../Core/FetcherValueTransformation.swift | 8 +++-- .../Carlos/Operations/KeyTransformation.swift | 24 ++++++++++---- .../Operations/ValueTransformation.swift | 16 +++++++--- .../ConditionedValueTransformation.swift | 16 +++++++--- 7 files changed, 75 insertions(+), 31 deletions(-) diff --git a/Sources/Carlos/CacheLevels/Composed.swift b/Sources/Carlos/CacheLevels/Composed.swift index 94bf172..359e22b 100644 --- a/Sources/Carlos/CacheLevels/Composed.swift +++ b/Sources/Carlos/CacheLevels/Composed.swift @@ -11,8 +11,12 @@ extension CacheLevel { */ public func compose(_ cache: A) -> BasicCache where A.KeyType == KeyType, A.OutputType == OutputType { BasicCache( - getClosure: { key in - self.get(key) + getClosure: { [weak self] key in + guard let self = self else { + return Empty(completeImmediately: true).eraseToAnyPublisher() + } + + return self.get(key) .catch { _ -> AnyPublisher in Logger.log("Composed| error on getting value for key \(key) on cache \(String(describing: self)).", .info) @@ -27,25 +31,33 @@ extension CacheLevel { .eraseToAnyPublisher() }.eraseToAnyPublisher() }, - setClosure: { value, key in - Publishers.Zip( + setClosure: { [weak self] value, key in + guard let self = self else { + return Empty(completeImmediately: true).eraseToAnyPublisher() + } + + return Publishers.Zip( self.set(value, forKey: key), cache.set(value, forKey: key) ) .map { _ in () } .eraseToAnyPublisher() }, - removeClosure: { key in - Publishers.Zip(self.remove(key), cache.remove(key)) + removeClosure: { [weak self] key in + guard let self = self else { + return Empty(completeImmediately: true).eraseToAnyPublisher() + } + + return Publishers.Zip(self.remove(key), cache.remove(key)) .map { _ in () } .eraseToAnyPublisher() }, - clearClosure: { - self.clear() + clearClosure: { [weak self] in + self?.clear() cache.clear() }, - memoryClosure: { - self.onMemoryWarning() + memoryClosure: { [weak self] in + self?.onMemoryWarning() cache.onMemoryWarning() } ) diff --git a/Sources/Carlos/CacheLevels/DiskCacheLevel.swift b/Sources/Carlos/CacheLevels/DiskCacheLevel.swift index 5f578d7..bc8fbe2 100644 --- a/Sources/Carlos/CacheLevels/DiskCacheLevel.swift +++ b/Sources/Carlos/CacheLevels/DiskCacheLevel.swift @@ -53,7 +53,7 @@ public final class DiskCacheLevel: CacheLevel _ = try? fileManager.createDirectory(atPath: path, withIntermediateDirectories: true, attributes: [:]) - cacheQueue.async { () -> Void in + cacheQueue.async { self.calculateSize() self.controlCapacity() } @@ -146,7 +146,7 @@ public final class DiskCacheLevel: CacheLevel All the cached files will be removed from the disk storage */ public func clear() { - cacheQueue.async { () -> Void in + cacheQueue.async { self.itemsInDirectory(self.path).forEach { filePath in _ = try? self.fileManager.removeItem(atPath: filePath) } diff --git a/Sources/Carlos/CacheLevels/PoolCache.swift b/Sources/Carlos/CacheLevels/PoolCache.swift index ffa6d52..5df82de 100644 --- a/Sources/Carlos/CacheLevels/PoolCache.swift +++ b/Sources/Carlos/CacheLevels/PoolCache.swift @@ -53,9 +53,9 @@ public final class PoolCache: CacheLevel where C.KeyType: Hashabl Logger.log("Creating a new request \(request) for key \(key)") return request - .handleEvents(receiveCompletion: { _ in - self.lock.locked { - self.requestsPool[key] = nil + .handleEvents(receiveCompletion: { [weak self] _ in + self?.lock.locked { + self?.requestsPool[key] = nil } }) .eraseToAnyPublisher() diff --git a/Sources/Carlos/Core/FetcherValueTransformation.swift b/Sources/Carlos/Core/FetcherValueTransformation.swift index c1f7562..f0a9d3d 100644 --- a/Sources/Carlos/Core/FetcherValueTransformation.swift +++ b/Sources/Carlos/Core/FetcherValueTransformation.swift @@ -13,8 +13,12 @@ extension Fetcher { */ public func transformValues(_ transformer: A) -> BasicFetcher where OutputType == A.TypeIn { BasicFetcher( - getClosure: { key in - self.get(key) + getClosure: { [weak self] key in + guard let self = self else { + return Empty(completeImmediately: true).eraseToAnyPublisher() + } + + return self.get(key) .flatMap(transformer.transform) .eraseToAnyPublisher() } diff --git a/Sources/Carlos/Operations/KeyTransformation.swift b/Sources/Carlos/Operations/KeyTransformation.swift index c05fffa..7bd665a 100644 --- a/Sources/Carlos/Operations/KeyTransformation.swift +++ b/Sources/Carlos/Operations/KeyTransformation.swift @@ -13,20 +13,32 @@ extension CacheLevel { */ public func transformKeys(_ transformer: A) -> BasicCache where KeyType == A.TypeOut { BasicCache( - getClosure: { key in - transformer.transform(key) + getClosure: { [weak self] key in + guard let self = self else { + return Empty(completeImmediately: true).eraseToAnyPublisher() + } + + return transformer.transform(key) .flatMap(self.get) .eraseToAnyPublisher() }, - setClosure: { value, key in - transformer.transform(key) + setClosure: { [weak self] value, key in + guard let self = self else { + return Empty(completeImmediately: true).eraseToAnyPublisher() + } + + return transformer.transform(key) .flatMap { transformedKey in self.set(value, forKey: transformedKey) } .eraseToAnyPublisher() }, - removeClosure: { - transformer.transform($0) + removeClosure: { [weak self] in + guard let self = self else { + return Empty(completeImmediately: true).eraseToAnyPublisher() + } + + return transformer.transform($0) .flatMap(self.remove) .eraseToAnyPublisher() }, diff --git a/Sources/Carlos/Operations/ValueTransformation.swift b/Sources/Carlos/Operations/ValueTransformation.swift index 3cdf463..23065a9 100644 --- a/Sources/Carlos/Operations/ValueTransformation.swift +++ b/Sources/Carlos/Operations/ValueTransformation.swift @@ -13,13 +13,21 @@ extension CacheLevel { */ public func transformValues(_ transformer: A) -> BasicCache where OutputType == A.TypeIn { BasicCache( - getClosure: { key in - self.get(key) + getClosure: { [weak self] key in + guard let self = self else { + return Empty(completeImmediately: true).eraseToAnyPublisher() + } + + return self.get(key) .flatMap(transformer.transform) .eraseToAnyPublisher() }, - setClosure: { value, key in - transformer.inverseTransform(value) + setClosure: { [weak self] value, key in + guard let self = self else { + return Empty(completeImmediately: true).eraseToAnyPublisher() + } + + return transformer.inverseTransform(value) .flatMap { transformedValue in self.set(transformedValue, forKey: key) } diff --git a/Sources/Carlos/Transformers/ConditionedValueTransformation.swift b/Sources/Carlos/Transformers/ConditionedValueTransformation.swift index d66f153..adc681b 100644 --- a/Sources/Carlos/Transformers/ConditionedValueTransformation.swift +++ b/Sources/Carlos/Transformers/ConditionedValueTransformation.swift @@ -15,13 +15,21 @@ extension CacheLevel { */ public func conditionedValueTransformation(transformer: A) -> BasicCache where OutputType == A.TypeIn, A.KeyType == KeyType { BasicCache( - getClosure: { key -> AnyPublisher in - self.get(key) + getClosure: { [weak self] key -> AnyPublisher in + guard let self = self else { + return Empty(completeImmediately: true).eraseToAnyPublisher() + } + + return self.get(key) .flatMap { transformer.conditionalTransform(key: key, value: $0) } .eraseToAnyPublisher() }, - setClosure: { value, key in - transformer.conditionalInverseTransform(key: key, value: value) + setClosure: { [weak self] value, key in + guard let self = self else { + return Empty(completeImmediately: true).eraseToAnyPublisher() + } + + return transformer.conditionalInverseTransform(key: key, value: value) .flatMap { transformedValue in self.set(transformedValue, forKey: key) }