From 63b6bd54be55a62ccd52286f85d7877382edfbb2 Mon Sep 17 00:00:00 2001 From: Jan Gorman Date: Mon, 2 Mar 2020 09:19:06 +0100 Subject: [PATCH 01/32] Change deployment target --- .../project.pbxproj | 4 +- .../xcschemes/MapleBacon Example.xcscheme | 78 +++++++++++++++++++ 2 files changed, 80 insertions(+), 2 deletions(-) create mode 100644 Example/MapleBacon Example.xcodeproj/xcshareddata/xcschemes/MapleBacon Example.xcscheme diff --git a/Example/MapleBacon Example.xcodeproj/project.pbxproj b/Example/MapleBacon Example.xcodeproj/project.pbxproj index 2b9cf27..35aa23a 100644 --- a/Example/MapleBacon Example.xcodeproj/project.pbxproj +++ b/Example/MapleBacon Example.xcodeproj/project.pbxproj @@ -321,7 +321,7 @@ GCC_WARN_UNINITIALIZED_AUTOS = YES_AGGRESSIVE; GCC_WARN_UNUSED_FUNCTION = YES; GCC_WARN_UNUSED_VARIABLE = YES; - IPHONEOS_DEPLOYMENT_TARGET = 13.4; + IPHONEOS_DEPLOYMENT_TARGET = 13.2; MTL_ENABLE_DEBUG_INFO = INCLUDE_SOURCE; MTL_FAST_MATH = YES; ONLY_ACTIVE_ARCH = YES; @@ -375,7 +375,7 @@ GCC_WARN_UNINITIALIZED_AUTOS = YES_AGGRESSIVE; GCC_WARN_UNUSED_FUNCTION = YES; GCC_WARN_UNUSED_VARIABLE = YES; - IPHONEOS_DEPLOYMENT_TARGET = 13.4; + IPHONEOS_DEPLOYMENT_TARGET = 13.2; MTL_ENABLE_DEBUG_INFO = NO; MTL_FAST_MATH = YES; SDKROOT = iphoneos; diff --git a/Example/MapleBacon Example.xcodeproj/xcshareddata/xcschemes/MapleBacon Example.xcscheme b/Example/MapleBacon Example.xcodeproj/xcshareddata/xcschemes/MapleBacon Example.xcscheme new file mode 100644 index 0000000..4b5e829 --- /dev/null +++ b/Example/MapleBacon Example.xcodeproj/xcshareddata/xcschemes/MapleBacon Example.xcscheme @@ -0,0 +1,78 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + From 447bc5b461cd65570fd929bbf58fcb6802b1c41d Mon Sep 17 00:00:00 2001 From: Jan Gorman Date: Mon, 2 Mar 2020 09:32:57 +0100 Subject: [PATCH 02/32] Add back Combine --- MapleBacon/Core/MapleBacon.swift | 23 ++++++++++++++++++ MapleBacon/Extensions/TimePeriod.swift | 3 +++ MapleBaconTests/CacheTests.swift | 2 +- MapleBaconTests/DiskCacheTests.swift | 2 +- MapleBaconTests/MapleBaconTests.swift | 33 +++++++++++++++++++++++++- MapleBaconTests/TestHelpers.swift | 6 +++-- README.md | 14 ++++++++++- 7 files changed, 77 insertions(+), 6 deletions(-) diff --git a/MapleBacon/Core/MapleBacon.swift b/MapleBacon/Core/MapleBacon.swift index 5448814..ae34b13 100644 --- a/MapleBacon/Core/MapleBacon.swift +++ b/MapleBacon/Core/MapleBacon.swift @@ -115,3 +115,26 @@ public final class MapleBacon { } } + +#if canImport(Combine) +import Combine + +@available(iOS 13.0, *) +extension MapleBacon { + + public func image(with url: URL, imageTransformer: ImageTransforming? = nil) -> AnyPublisher { + Future { resolve in + self.image(with: url, imageTransformer: imageTransformer) { result in + switch result { + case .success(let image): + resolve(.success(image)) + case .failure(let error): + resolve(.failure(error)) + } + } + }.eraseToAnyPublisher() + } + +} + +#endif diff --git a/MapleBacon/Extensions/TimePeriod.swift b/MapleBacon/Extensions/TimePeriod.swift index c073af3..accc1d1 100644 --- a/MapleBacon/Extensions/TimePeriod.swift +++ b/MapleBacon/Extensions/TimePeriod.swift @@ -25,6 +25,9 @@ enum TimePeriod { } extension Int { + var second: TimeInterval { + TimePeriod.seconds(self).timeInterval + } var seconds: TimeInterval { TimePeriod.seconds(self).timeInterval } diff --git a/MapleBaconTests/CacheTests.swift b/MapleBaconTests/CacheTests.swift index 21019df..da0d924 100644 --- a/MapleBaconTests/CacheTests.swift +++ b/MapleBaconTests/CacheTests.swift @@ -14,7 +14,7 @@ final class CacheTests: XCTestCase { override func tearDown() { cache.clear(.all) // Clearing the disk is an async operation so we should wait - wait(for: 2.seconds) + wait(for: 1.second) super.tearDown() } diff --git a/MapleBaconTests/DiskCacheTests.swift b/MapleBaconTests/DiskCacheTests.swift index 5682581..5a9fefd 100644 --- a/MapleBaconTests/DiskCacheTests.swift +++ b/MapleBaconTests/DiskCacheTests.swift @@ -13,7 +13,7 @@ final class DiskCacheTests: XCTestCase { let cache = DiskCache(name: Self.cacheName) cache.clear() // Clearing the disk is an async operation so we should wait - wait(for: 2.seconds) + wait(for: 1.second) super.tearDown() } diff --git a/MapleBaconTests/MapleBaconTests.swift b/MapleBaconTests/MapleBaconTests.swift index 39f5ab3..65e65ca 100644 --- a/MapleBaconTests/MapleBaconTests.swift +++ b/MapleBaconTests/MapleBaconTests.swift @@ -2,6 +2,9 @@ // Copyright © 2020 Schnaub. All rights reserved. // +#if canImport(Combine) +import Combine +#endif @testable import MapleBacon import XCTest @@ -11,10 +14,13 @@ final class MapleBaconTests: XCTestCase { private let cache = Cache(name: "MapleBaconTests") + @available(iOS 13.0, *) + private lazy var subscriptions: Set = [] + override func tearDown() { cache.clear(.all) // Clearing the disk is an async operation so we should wait - wait(for: 2.seconds) + wait(for: 1.seconds) super.tearDown() } @@ -82,3 +88,28 @@ final class MapleBaconTests: XCTestCase { } } + +#if canImport(Combine) + +@available(iOS 13.0, *) +extension MapleBaconTests { + + func testIntegrationPublisher() { + let expectation = self.expectation(description: #function) + let configuration = MockURLProtocol.mockedURLSessionConfiguration() + let mapleBacon = MapleBacon(cache: cache, sessionConfiguration: configuration) + + mapleBacon.image(with: Self.url) + .sink(receiveCompletion: { _ in + expectation.fulfill() + }, receiveValue: { image in + XCTAssertEqual(image.pngData(), makeImageData()) + }) + .store(in: &self.subscriptions) + + waitForExpectations(timeout: 5, handler: nil) + } + +} + +#endif diff --git a/MapleBaconTests/TestHelpers.swift b/MapleBaconTests/TestHelpers.swift index 9ffa62c..4500060 100644 --- a/MapleBaconTests/TestHelpers.swift +++ b/MapleBaconTests/TestHelpers.swift @@ -44,8 +44,10 @@ func makeImageData() -> Data { extension XCTestCase { func wait(for interval: TimeInterval) { - let date = Date(timeIntervalSinceNow: interval) - RunLoop.current.run(mode: RunLoop.Mode.default, before: date) + let date = Date(timeIntervalSinceNow: interval) + while date.timeIntervalSinceNow > 0 { + CFRunLoopRunInMode(CFRunLoopMode.defaultMode, 0.1, true) + } } } diff --git a/README.md b/README.md index 7a81b49..24c10cb 100644 --- a/README.md +++ b/README.md @@ -129,7 +129,6 @@ let chainedTransformer = SepiaImageTransformer() >>> DifferentTransformer() >>> (Keep in mind that if you are using Core Image it might not be optimal to chain individual transformers but rather create one transformer that applies multiple `CIFilter`s in one pass. See the [Core Image Programming Guide](https://developer.apple.com/library/content/documentation/GraphicsImaging/Conceptual/CoreImaging/ci_intro/ci_intro.html#//apple_ref/doc/uid/TP30001185).) - ### Caching MapleBacon will cache your images both in memory and on disk. Disk storage is automatically pruned after a week (taking into account the last access date as well) but you can control the maximum cache time yourself too: @@ -139,6 +138,19 @@ let oneDaySeconds: TimeInterval = 60 * 60 * 24 MapleBacon.default.maxCacheAgeSeconds = oneDaySeconds ``` +### Combine + +On iOS13 and above, you can use `Combine` to fetch images from MapleBacon + +```swift +MapleBacon.shared.image(with: url) + .receive(on: DispatchQueue.main) // Dispatch to the right queue if updating the UI + .sink(receiveValue: { image in + // Do something with your image + }) + .store(in: &subscriptions) // Hold on to and dispose your subscriptions +``` + ## Migrating from 5.x There is a small [migration guide](https://github.com/JanGorman/MapleBacon/wiki/Migration-Guide-Version-5.x-→-6.x) in the wiki when moving from the 5.x branch to 6.x From 9763409c379ee8f484b2c59adcdf245e80abf775 Mon Sep 17 00:00:00 2001 From: Jan Gorman Date: Mon, 2 Mar 2020 09:34:37 +0100 Subject: [PATCH 03/32] Move CryptoKit import to extension --- MapleBacon/Core/Cache.swift | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/MapleBacon/Core/Cache.swift b/MapleBacon/Core/Cache.swift index 7f73ee7..710acc2 100644 --- a/MapleBacon/Core/Cache.swift +++ b/MapleBacon/Core/Cache.swift @@ -2,9 +2,6 @@ // Copyright © 2020 Schnaub. All rights reserved. // -#if canImport(CryptoKit) -import CryptoKit -#endif import UIKit enum CacheError: Error { @@ -99,6 +96,8 @@ final class Cache where T.Result == T { } #if canImport(CryptoKit) +import CryptoKit + @available(iOS 13.0, *) private extension Cache { From 6d376d2e543381b3641595e7519a9411fa866fa5 Mon Sep 17 00:00:00 2001 From: Jan Gorman Date: Mon, 2 Mar 2020 09:42:59 +0100 Subject: [PATCH 04/32] Concurrent --- .../ImageTransformerViewController.swift | 24 +------------------ MapleBacon/Core/MapleBacon.swift | 2 +- 2 files changed, 2 insertions(+), 24 deletions(-) diff --git a/Example/MapleBacon Example/Controllers/ImageTransformerViewController.swift b/Example/MapleBacon Example/Controllers/ImageTransformerViewController.swift index f4cfdf8..d3d1867 100644 --- a/Example/MapleBacon Example/Controllers/ImageTransformerViewController.swift +++ b/Example/MapleBacon Example/Controllers/ImageTransformerViewController.swift @@ -8,7 +8,7 @@ import UIKit final class ImageTransformerViewController: UICollectionViewController { private var imageURLs: [URL] = [] - private var imageTransformer = SepiaImageTransformer() >>> VignetteImageTransformer() + private var imageTransformer = SepiaImageTransformer() override func viewDidLoad() { super.viewDidLoad() @@ -57,25 +57,3 @@ private class SepiaImageTransformer: ImageTransforming { } } - -private class VignetteImageTransformer: ImageTransforming { - - let identifier = "com.schnaub.VignetteImageTransformer" - - func transform(image: UIImage) -> UIImage? { - let filter = CIFilter(name: "CIVignette")! - - let ciImage = CIImage(image: image) - filter.setValue(ciImage, forKey: kCIInputImageKey) - - let context = CIContext() - guard let outputImage = filter.outputImage, - let cgImage = context.createCGImage(outputImage, from: outputImage.extent) else { - return image - } - - return UIImage(cgImage: cgImage) - } - -} - diff --git a/MapleBacon/Core/MapleBacon.swift b/MapleBacon/Core/MapleBacon.swift index ae34b13..b0c40a9 100644 --- a/MapleBacon/Core/MapleBacon.swift +++ b/MapleBacon/Core/MapleBacon.swift @@ -36,7 +36,7 @@ public final class MapleBacon { init(cache: Cache, sessionConfiguration: URLSessionConfiguration) { self.cache = cache self.downloader = Downloader(sessionConfiguration: sessionConfiguration) - self.transformerQueue = DispatchQueue(label: Self.queueLabel) + self.transformerQueue = DispatchQueue(label: Self.queueLabel, attributes: .concurrent) } public func image(with url: URL, imageTransformer: ImageTransforming? = nil, completion: @escaping ImageCompletion) { From 2df3b8bf6f33ec33cbd44721bacb1c17f37f5953 Mon Sep 17 00:00:00 2001 From: Jan Gorman Date: Mon, 2 Mar 2020 09:49:58 +0100 Subject: [PATCH 05/32] Public cache clear --- MapleBacon/Core/CacheClearOptions.swift | 14 +++++++++----- MapleBacon/Core/MapleBacon.swift | 20 +++++++++++++------- 2 files changed, 22 insertions(+), 12 deletions(-) diff --git a/MapleBacon/Core/CacheClearOptions.swift b/MapleBacon/Core/CacheClearOptions.swift index 3aadc58..2dae12e 100644 --- a/MapleBacon/Core/CacheClearOptions.swift +++ b/MapleBacon/Core/CacheClearOptions.swift @@ -4,11 +4,15 @@ import Foundation -struct CacheClearOptions: OptionSet { - let rawValue: Int +public struct CacheClearOptions: OptionSet { + public let rawValue: Int - static let memory = CacheClearOptions(rawValue: 1 << 0) - static let disk = CacheClearOptions(rawValue: 1 << 1) + public init(rawValue: Int) { + self.rawValue = rawValue + } - static let all: CacheClearOptions = [.memory, .disk] + public static let memory = CacheClearOptions(rawValue: 1 << 0) + public static let disk = CacheClearOptions(rawValue: 1 << 1) + + public static let all: CacheClearOptions = [.memory, .disk] } diff --git a/MapleBacon/Core/MapleBacon.swift b/MapleBacon/Core/MapleBacon.swift index b0c40a9..9bac10c 100644 --- a/MapleBacon/Core/MapleBacon.swift +++ b/MapleBacon/Core/MapleBacon.swift @@ -50,7 +50,14 @@ public final class MapleBacon { } } - private func fetchImageFromCache(with url: URL, imageTransformer: ImageTransforming?, completion: @escaping ImageCompletion) { + public func clearCache(_ options: CacheClearOptions) { + cache.clear(options) + } + +} + +private extension MapleBacon { + func fetchImageFromCache(with url: URL, imageTransformer: ImageTransforming?, completion: @escaping ImageCompletion) { let cacheKey = makeCacheKey(for: url, imageTransformer: imageTransformer) cache.value(forKey: cacheKey) { result in switch result { @@ -62,7 +69,7 @@ public final class MapleBacon { } } - private func fetchImageFromNetworkAndCache(with url: URL, imageTransformer: ImageTransforming?, completion: @escaping ImageCompletion) { + func fetchImageFromNetworkAndCache(with url: URL, imageTransformer: ImageTransforming?, completion: @escaping ImageCompletion) { fetchImageFromNetwork(with: url) { result in switch result { case .success(let image): @@ -79,11 +86,11 @@ public final class MapleBacon { } } - private func fetchImageFromNetwork(with url: URL, completion: @escaping ImageCompletion) { + func fetchImageFromNetwork(with url: URL, completion: @escaping ImageCompletion) { downloader.fetch(url, completion: completion) } - private func transformImageAndCache(_ image: UIImage, cacheKey: String, imageTransformer: ImageTransforming, completion: @escaping ImageCompletion) { + func transformImageAndCache(_ image: UIImage, cacheKey: String, imageTransformer: ImageTransforming, completion: @escaping ImageCompletion) { transformImage(image, imageTransformer: imageTransformer) { result in switch result { case .success(let image): @@ -95,7 +102,7 @@ public final class MapleBacon { } } - private func transformImage(_ image: UIImage, imageTransformer: ImageTransforming, completion: @escaping ImageCompletion) { + func transformImage(_ image: UIImage, imageTransformer: ImageTransforming, completion: @escaping ImageCompletion) { transformerQueue.async { DispatchQueue.main.async { guard let image = imageTransformer.transform(image: image) else { @@ -107,13 +114,12 @@ public final class MapleBacon { } } - private func makeCacheKey(for url: URL, imageTransformer: ImageTransforming?) -> String { + func makeCacheKey(for url: URL, imageTransformer: ImageTransforming?) -> String { guard let imageTransformer = imageTransformer else { return url.absoluteString } return url.absoluteString + imageTransformer.identifier } - } #if canImport(Combine) From 4ccc0f31962c0bbc804cb08807ca24bb83d2cf65 Mon Sep 17 00:00:00 2001 From: Jan Gorman Date: Tue, 3 Mar 2020 13:53:15 +0100 Subject: [PATCH 06/32] Concurrent download handling --- MapleBacon/Core/Downloader.swift | 121 ++++++++++++++++++++++---- MapleBaconTests/DownloaderTests.swift | 29 ++++++ 2 files changed, 133 insertions(+), 17 deletions(-) diff --git a/MapleBacon/Core/Downloader.swift b/MapleBacon/Core/Downloader.swift index 1d939fe..92cdb6f 100644 --- a/MapleBacon/Core/Downloader.swift +++ b/MapleBacon/Core/Downloader.swift @@ -2,40 +2,127 @@ // Copyright © 2020 Schnaub. All rights reserved. // -import Foundation +import UIKit enum DownloaderError: Error { case dataConversion } +private final class Download { + + let task: URLSessionDataTask + + var completions: [(Result) -> Void] + var data = Data() + + private var backgroundTask: UIBackgroundTaskIdentifier = .invalid + + init(task: URLSessionDataTask, completion: @escaping (Result) -> Void) { + self.task = task + self.completions = [completion] + } + + deinit { + invalidateBackgroundTask() + } + + func start() { + backgroundTask = UIApplication.shared.beginBackgroundTask { + self.invalidateBackgroundTask() + } + } + + func finish() { + invalidateBackgroundTask() + } + + private func invalidateBackgroundTask() { + UIApplication.shared.endBackgroundTask(backgroundTask) + backgroundTask = .invalid + } +} + final class Downloader { let session: URLSession - var task: URLSessionDataTask? + private let mutex: DispatchQueue + private let sessionDelegate: SessionDelegate + + private var downloads: [URL: Download] = [:] + + fileprivate subscript(_ url: URL) -> Download? { + downloads[url] + } init(sessionConfiguration: URLSessionConfiguration = .default) { - self.session = URLSession(configuration: sessionConfiguration) + self.sessionDelegate = SessionDelegate() + self.session = URLSession(configuration: sessionConfiguration, delegate: sessionDelegate, delegateQueue: .main) + self.mutex = DispatchQueue(label: "com.schnaub.Downloader.mutex", attributes: .concurrent) + self.sessionDelegate.delegate = self } func fetch(_ url: URL, completion: @escaping (Result) -> Void) { - let task = session.dataTask(with: url) { data, _, error in - DispatchQueue.main.async { - if let error = error { - completion(.failure(error)) - return - } - guard let data = data, let value = T.convert(from: data) else { - completion(.failure(DownloaderError.dataConversion)) - return - } - completion(.success(value)) + mutex.sync(flags: .barrier) { + let task: URLSessionDataTask + if let download = downloads[url] { + task = download.task + download.completions.append(completion) + } else { + let newTask = session.dataTask(with: url) + let download = Download(task: newTask, completion: completion) + download.start() + downloads[url] = download + task = newTask } - } - defer { + task.resume() } - self.task = task + } + + fileprivate func clearDownload(for url: URL) { + mutex.sync(flags: .barrier) { + downloads[url] = nil + } + } + +} + +private final class SessionDelegate: NSObject, URLSessionDataDelegate { + + weak var delegate: Downloader? + + func urlSession(_ session: URLSession, dataTask: URLSessionDataTask, didReceive data: Data) { + guard let url = dataTask.originalRequest?.url, let download = delegate?[url] else { + return + } + download.data.append(data) + } + + func urlSession(_ session: URLSession, task: URLSessionTask, didCompleteWithError error: Error?) { + guard let url = task.originalRequest?.url, let download = delegate?[url] else { + return + } + + delegate?[url]?.completions.forEach { completion in + if let error = error { + completion(.failure(error)) + return + } + guard let value = T.convert(from: download.data) else { + completion(.failure(DownloaderError.dataConversion)) + return + } + completion(.success(value)) + } + delegate?.clearDownload(for: url) + download.finish() + } + + func urlSession(_ session: URLSession, dataTask: URLSessionDataTask, + willCacheResponse proposedResponse: CachedURLResponse, + completionHandler: @escaping (CachedURLResponse?) -> Void) { + completionHandler(nil) } } diff --git a/MapleBaconTests/DownloaderTests.swift b/MapleBaconTests/DownloaderTests.swift index eb8dcb5..1320761 100644 --- a/MapleBaconTests/DownloaderTests.swift +++ b/MapleBaconTests/DownloaderTests.swift @@ -71,4 +71,33 @@ final class DownloaderTests: XCTestCase { waitForExpectations(timeout: 5, handler: nil) } + func testConcurrentDownloads() { + let configuration = MockURLProtocol.mockedURLSessionConfiguration() + let downloader = Downloader(sessionConfiguration: configuration) + + let firstExpectation = expectation(description: "first") + downloader.fetch(Self.url) { response in + switch response { + case .success(let data): + XCTAssertNotNil(data) + case .failure: + XCTFail() + } + firstExpectation.fulfill() + } + + let secondExpectation = expectation(description: "second") + downloader.fetch(Self.url) { response in + switch response { + case .success(let data): + XCTAssertNotNil(data) + case .failure: + XCTFail() + } + secondExpectation.fulfill() + } + + waitForExpectations(timeout: 5, handler: nil) + } + } From 750bd8b0a9666c67aa4f5b85b04e630b5d9b4d8e Mon Sep 17 00:00:00 2001 From: Jan Gorman Date: Wed, 4 Mar 2020 10:49:01 +0100 Subject: [PATCH 07/32] Less queue jumping --- MapleBacon.xcodeproj/project.pbxproj | 4 +++ MapleBacon/Core/DiskCache.swift | 22 ++++---------- MapleBacon/Core/Downloader.swift | 22 +++++++------- MapleBacon/Core/MapleBacon.swift | 30 ++++++++++++------- .../Extensions/DispatchQueue+MapleBacon.swift | 17 +++++++++++ 5 files changed, 58 insertions(+), 37 deletions(-) create mode 100644 MapleBacon/Extensions/DispatchQueue+MapleBacon.swift diff --git a/MapleBacon.xcodeproj/project.pbxproj b/MapleBacon.xcodeproj/project.pbxproj index f0380b2..c3ace81 100644 --- a/MapleBacon.xcodeproj/project.pbxproj +++ b/MapleBacon.xcodeproj/project.pbxproj @@ -7,6 +7,7 @@ objects = { /* Begin PBXBuildFile section */ + 7778FD28240FAC6B007764C6 /* DispatchQueue+MapleBacon.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7778FD27240FAC6B007764C6 /* DispatchQueue+MapleBacon.swift */; }; F2255E202404600D00193742 /* UIImageView+MapleBacon.swift in Sources */ = {isa = PBXBuildFile; fileRef = F2255E1F2404600D00193742 /* UIImageView+MapleBacon.swift */; }; F2255E222404606E00193742 /* MapleBacon.swift in Sources */ = {isa = PBXBuildFile; fileRef = F2255E212404606E00193742 /* MapleBacon.swift */; }; F2255E24240462D800193742 /* MapleBaconTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = F2255E23240462D800193742 /* MapleBaconTests.swift */; }; @@ -45,6 +46,7 @@ /* End PBXContainerItemProxy section */ /* Begin PBXFileReference section */ + 7778FD27240FAC6B007764C6 /* DispatchQueue+MapleBacon.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "DispatchQueue+MapleBacon.swift"; sourceTree = ""; }; F2255E1F2404600D00193742 /* UIImageView+MapleBacon.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "UIImageView+MapleBacon.swift"; sourceTree = ""; }; F2255E212404606E00193742 /* MapleBacon.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MapleBacon.swift; sourceTree = ""; }; F2255E23240462D800193742 /* MapleBaconTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MapleBaconTests.swift; sourceTree = ""; }; @@ -166,6 +168,7 @@ F2255E1F2404600D00193742 /* UIImageView+MapleBacon.swift */, F2EEE588240BBEF90002C9CA /* DownsamplingImageTransformer.swift */, F2EEE58A240BBFC00002C9CA /* CGSize+MapleBacon.swift */, + 7778FD27240FAC6B007764C6 /* DispatchQueue+MapleBacon.swift */, ); path = Extensions; sourceTree = ""; @@ -291,6 +294,7 @@ F27CBA1E2402D9A8006CD529 /* Downloader.swift in Sources */, F2EEE58B240BBFC00002C9CA /* CGSize+MapleBacon.swift in Sources */, F2EEE583240BBC2E0002C9CA /* ImageTransforming.swift in Sources */, + 7778FD28240FAC6B007764C6 /* DispatchQueue+MapleBacon.swift in Sources */, F27CBA0C2402B147006CD529 /* TimePeriod.swift in Sources */, F27CBA1A2402CC13006CD529 /* CacheClearOptions.swift in Sources */, F27CBA1C2402CC28006CD529 /* CacheResult.swift in Sources */, diff --git a/MapleBacon/Core/DiskCache.swift b/MapleBacon/Core/DiskCache.swift index 794d33e..28fc185 100644 --- a/MapleBacon/Core/DiskCache.swift +++ b/MapleBacon/Core/DiskCache.swift @@ -23,9 +23,7 @@ final class DiskCache { diskQueue.async { var diskError: Error? defer { - DispatchQueue.main.async { - completion?(diskError) - } + completion?(diskError) } do { try self.store(data: data, key: key) @@ -39,18 +37,14 @@ final class DiskCache { diskQueue.async { var diskError: Error? defer { - DispatchQueue.main.async { - if let error = diskError { - completion?(.failure(error)) - } + if let error = diskError { + completion?(.failure(error)) } } do { let url = try self.cacheDirectory().appendingPathComponent(key) let data = try FileManager.default.fileContents(at: url) - DispatchQueue.main.async { - completion?(.success(data)) - } + completion?(.success(data)) } catch { diskError = error } @@ -61,9 +55,7 @@ final class DiskCache { diskQueue.async { var diskError: Error? defer { - DispatchQueue.main.async { - completion?(diskError) - } + completion?(diskError) } do { let cacheDirectory = try self.cacheDirectory() @@ -78,9 +70,7 @@ final class DiskCache { diskQueue.async { var diskError: Error? defer { - DispatchQueue.main.async { - completion?(diskError) - } + completion?(diskError) } do { let expiredFiles = try self.expiredFileURLs() diff --git a/MapleBacon/Core/Downloader.swift b/MapleBacon/Core/Downloader.swift index 92cdb6f..a04c74b 100644 --- a/MapleBacon/Core/Downloader.swift +++ b/MapleBacon/Core/Downloader.swift @@ -51,15 +51,11 @@ final class Downloader { private var downloads: [URL: Download] = [:] - fileprivate subscript(_ url: URL) -> Download? { - downloads[url] - } - init(sessionConfiguration: URLSessionConfiguration = .default) { self.sessionDelegate = SessionDelegate() self.session = URLSession(configuration: sessionConfiguration, delegate: sessionDelegate, delegateQueue: .main) self.mutex = DispatchQueue(label: "com.schnaub.Downloader.mutex", attributes: .concurrent) - self.sessionDelegate.delegate = self + self.sessionDelegate.downloader = self } func fetch(_ url: URL, completion: @escaping (Result) -> Void) { @@ -80,6 +76,12 @@ final class Downloader { } } + fileprivate func download(for url: URL) -> Download? { + mutex.sync(flags: .barrier) { + return downloads[url] + } + } + fileprivate func clearDownload(for url: URL) { mutex.sync(flags: .barrier) { downloads[url] = nil @@ -90,21 +92,21 @@ final class Downloader { private final class SessionDelegate: NSObject, URLSessionDataDelegate { - weak var delegate: Downloader? + weak var downloader: Downloader? func urlSession(_ session: URLSession, dataTask: URLSessionDataTask, didReceive data: Data) { - guard let url = dataTask.originalRequest?.url, let download = delegate?[url] else { + guard let url = dataTask.originalRequest?.url, let download = downloader?.download(for: url) else { return } download.data.append(data) } func urlSession(_ session: URLSession, task: URLSessionTask, didCompleteWithError error: Error?) { - guard let url = task.originalRequest?.url, let download = delegate?[url] else { + guard let url = task.originalRequest?.url, let download = downloader?.download(for: url) else { return } - delegate?[url]?.completions.forEach { completion in + downloader?.download(for: url)?.completions.forEach { completion in if let error = error { completion(.failure(error)) return @@ -115,7 +117,7 @@ private final class SessionDelegate: NSObject, URLSessionDat } completion(.success(value)) } - delegate?.clearDownload(for: url) + downloader?.clearDownload(for: url) download.finish() } diff --git a/MapleBacon/Core/MapleBacon.swift b/MapleBacon/Core/MapleBacon.swift index 9bac10c..8f8166d 100644 --- a/MapleBacon/Core/MapleBacon.swift +++ b/MapleBacon/Core/MapleBacon.swift @@ -43,7 +43,9 @@ public final class MapleBacon { fetchImageFromCache(with: url, imageTransformer: imageTransformer) { result in switch result { case .success(let image): - completion(.success(image)) + DispatchQueue.main.optionalAsync { + completion(.success(image)) + } case .failure: self.fetchImageFromNetworkAndCache(with: url, imageTransformer: imageTransformer, completion: completion) } @@ -78,10 +80,14 @@ private extension MapleBacon { self.transformImageAndCache(image, cacheKey: cacheKey, imageTransformer: transformer, completion: completion) } else { self.cache.store(value: image, forKey: url.absoluteString) - completion(.success(image)) + DispatchQueue.main.optionalAsync { + completion(.success(image)) + } } case .failure(let error): - completion(.failure(error)) + DispatchQueue.main.optionalAsync { + completion(.failure(error)) + } } } } @@ -95,22 +101,24 @@ private extension MapleBacon { switch result { case .success(let image): self.cache.store(value: image, forKey: cacheKey) - completion(.success(image)) + DispatchQueue.main.optionalAsync { + completion(.success(image)) + } case .failure(let error): - completion(.failure(error)) + DispatchQueue.main.optionalAsync { + completion(.failure(error)) + } } } } func transformImage(_ image: UIImage, imageTransformer: ImageTransforming, completion: @escaping ImageCompletion) { transformerQueue.async { - DispatchQueue.main.async { - guard let image = imageTransformer.transform(image: image) else { - completion(.failure(MapleBaconError.imageTransformingError)) - return - } - completion(.success(image)) + guard let image = imageTransformer.transform(image: image) else { + completion(.failure(MapleBaconError.imageTransformingError)) + return } + completion(.success(image)) } } diff --git a/MapleBacon/Extensions/DispatchQueue+MapleBacon.swift b/MapleBacon/Extensions/DispatchQueue+MapleBacon.swift new file mode 100644 index 0000000..747546d --- /dev/null +++ b/MapleBacon/Extensions/DispatchQueue+MapleBacon.swift @@ -0,0 +1,17 @@ +// +// Copyright © 2020 Schnaub. All rights reserved. +// + +import Foundation + +extension DispatchQueue { + func optionalAsync(_ block: @escaping () -> Void) { + if self === DispatchQueue.main && Thread.isMainThread { + block() + } else { + async { + block() + } + } + } +} From 5eaf2d3cb2e7b30985e9716930f0d3beaa47a83b Mon Sep 17 00:00:00 2001 From: Jan Gorman Date: Wed, 4 Mar 2020 14:43:28 +0100 Subject: [PATCH 08/32] Tune dictionary access --- .../xcshareddata/xcschemes/MapleBacon.xcscheme | 1 + MapleBacon/Core/Downloader.swift | 8 ++++---- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/MapleBacon.xcodeproj/xcshareddata/xcschemes/MapleBacon.xcscheme b/MapleBacon.xcodeproj/xcshareddata/xcschemes/MapleBacon.xcscheme index 42c6907..224e7da 100644 --- a/MapleBacon.xcodeproj/xcshareddata/xcschemes/MapleBacon.xcscheme +++ b/MapleBacon.xcodeproj/xcshareddata/xcschemes/MapleBacon.xcscheme @@ -27,6 +27,7 @@ selectedDebuggerIdentifier = "Xcode.DebuggerFoundation.Debugger.LLDB" selectedLauncherIdentifier = "Xcode.DebuggerFoundation.Launcher.LLDB" shouldUseLaunchSchemeArgsEnv = "YES" + enableThreadSanitizer = "YES" codeCoverageEnabled = "YES"> { } fileprivate func download(for url: URL) -> Download? { - mutex.sync(flags: .barrier) { - return downloads[url] + mutex.sync { + downloads[url] } } fileprivate func clearDownload(for url: URL) { - mutex.sync(flags: .barrier) { - downloads[url] = nil + mutex.async(flags: .barrier) { + self.downloads[url] = nil } } From fc863826d7e77b826f05fc9959ed6f5169e18d30 Mon Sep 17 00:00:00 2001 From: Jan Gorman Date: Wed, 4 Mar 2020 20:10:44 +0100 Subject: [PATCH 09/32] Clear cache on each test run --- MapleBacon/Core/Cache.swift | 7 +++++-- MapleBacon/Core/MapleBacon.swift | 4 ++-- MapleBaconTests/CacheTests.swift | 28 ++++++++++++++------------- MapleBaconTests/DiskCacheTests.swift | 21 +++++++++----------- MapleBaconTests/MapleBaconTests.swift | 24 +++++++++++------------ MapleBaconTests/TestHelpers.swift | 10 ---------- 6 files changed, 43 insertions(+), 51 deletions(-) diff --git a/MapleBacon/Core/Cache.swift b/MapleBacon/Core/Cache.swift index 710acc2..f5e400f 100644 --- a/MapleBacon/Core/Cache.swift +++ b/MapleBacon/Core/Cache.swift @@ -64,12 +64,15 @@ final class Cache where T.Result == T { } } - func clear(_ options: CacheClearOptions) { + func clear(_ options: CacheClearOptions, completion: ((Error?) -> Void)? = nil) { if options.contains(.memory) { memoryCache.clear() + if !options.contains(.disk) { + completion?(nil) + } } if options.contains(.disk) { - diskCache.clear() + diskCache.clear(completion) } } diff --git a/MapleBacon/Core/MapleBacon.swift b/MapleBacon/Core/MapleBacon.swift index 8f8166d..df9f370 100644 --- a/MapleBacon/Core/MapleBacon.swift +++ b/MapleBacon/Core/MapleBacon.swift @@ -52,8 +52,8 @@ public final class MapleBacon { } } - public func clearCache(_ options: CacheClearOptions) { - cache.clear(options) + public func clearCache(_ options: CacheClearOptions, completion: ((Error?) -> Void)? = nil) { + cache.clear(options, completion: completion) } } diff --git a/MapleBaconTests/CacheTests.swift b/MapleBaconTests/CacheTests.swift index da0d924..9f8d0bc 100644 --- a/MapleBaconTests/CacheTests.swift +++ b/MapleBaconTests/CacheTests.swift @@ -11,14 +11,6 @@ final class CacheTests: XCTestCase { private let cache = Cache(name: CacheTests.cacheName) - override func tearDown() { - cache.clear(.all) - // Clearing the disk is an async operation so we should wait - wait(for: 1.second) - - super.tearDown() - } - func testStorage() { let expectation = self.expectation(description: #function) @@ -26,7 +18,9 @@ final class CacheTests: XCTestCase { cache.store(value: data, forKey: #function) { error in XCTAssertNil(error) - expectation.fulfill() + self.cache.clear(.all) { _ in + expectation.fulfill() + } } waitForExpectations(timeout: 5, handler: nil) @@ -46,7 +40,9 @@ final class CacheTests: XCTestCase { case .failure: XCTFail() } - expectation.fulfill() + self.cache.clear(.all) { _ in + expectation.fulfill() + } } } @@ -68,7 +64,9 @@ final class CacheTests: XCTestCase { case .failure(let error): XCTAssertNotNil(error) } - expectation.fulfill() + self.cache.clear(.all) { _ in + expectation.fulfill() + } } } @@ -91,7 +89,9 @@ final class CacheTests: XCTestCase { case .failure: XCTFail() } - expectation.fulfill() + self.cache.clear(.all) { _ in + expectation.fulfill() + } } } @@ -118,7 +118,9 @@ final class CacheTests: XCTestCase { } } - expectation.fulfill() + self.cache.clear(.all) { _ in + expectation.fulfill() + } } } diff --git a/MapleBaconTests/DiskCacheTests.swift b/MapleBaconTests/DiskCacheTests.swift index 5a9fefd..292acff 100644 --- a/MapleBaconTests/DiskCacheTests.swift +++ b/MapleBaconTests/DiskCacheTests.swift @@ -9,22 +9,15 @@ final class DiskCacheTests: XCTestCase { private static let cacheName = "DiskCacheTests" - override func tearDown() { - let cache = DiskCache(name: Self.cacheName) - cache.clear() - // Clearing the disk is an async operation so we should wait - wait(for: 1.second) - - super.tearDown() - } - func testWrite() { let expectation = self.expectation(description: #function) let cache = DiskCache(name: Self.cacheName) cache.insert(dummyData(), forKey: "test") { error in XCTAssertNil(error) - expectation.fulfill() + cache.clear { _ in + expectation.fulfill() + } } waitForExpectations(timeout: 5, handler: nil) @@ -44,7 +37,9 @@ final class DiskCacheTests: XCTestCase { case .failure: XCTFail() } - expectation.fulfill() + cache.clear { _ in + expectation.fulfill() + } } } @@ -97,7 +92,9 @@ final class DiskCacheTests: XCTestCase { let expired = try! cache.expiredFileURLs() XCTAssertTrue(expired.isEmpty) - expectation.fulfill() + cache.clear { _ in + expectation.fulfill() + } } } diff --git a/MapleBaconTests/MapleBaconTests.swift b/MapleBaconTests/MapleBaconTests.swift index 65e65ca..8d9ec0f 100644 --- a/MapleBaconTests/MapleBaconTests.swift +++ b/MapleBaconTests/MapleBaconTests.swift @@ -17,14 +17,6 @@ final class MapleBaconTests: XCTestCase { @available(iOS 13.0, *) private lazy var subscriptions: Set = [] - override func tearDown() { - cache.clear(.all) - // Clearing the disk is an async operation so we should wait - wait(for: 1.seconds) - - super.tearDown() - } - func testIntegration() { let expectation = self.expectation(description: #function) let configuration = MockURLProtocol.mockedURLSessionConfiguration() @@ -39,7 +31,9 @@ final class MapleBaconTests: XCTestCase { case .failure: XCTFail() } - expectation.fulfill() + mapleBacon.clearCache(.all) { _ in + expectation.fulfill() + } } waitForExpectations(timeout: 5, handler: nil) @@ -59,7 +53,9 @@ final class MapleBaconTests: XCTestCase { case .failure(let error): XCTAssertNotNil(error) } - expectation.fulfill() + mapleBacon.clearCache(.all) { _ in + expectation.fulfill() + } } waitForExpectations(timeout: 5, handler: nil) @@ -81,7 +77,9 @@ final class MapleBaconTests: XCTestCase { case .failure: XCTFail() } - expectation.fulfill() + mapleBacon.clearCache(.all) { _ in + expectation.fulfill() + } } waitForExpectations(timeout: 5, handler: nil) @@ -101,7 +99,9 @@ extension MapleBaconTests { mapleBacon.image(with: Self.url) .sink(receiveCompletion: { _ in - expectation.fulfill() + mapleBacon.clearCache(.all) { _ in + expectation.fulfill() + } }, receiveValue: { image in XCTAssertEqual(image.pngData(), makeImageData()) }) diff --git a/MapleBaconTests/TestHelpers.swift b/MapleBaconTests/TestHelpers.swift index 4500060..55909e0 100644 --- a/MapleBaconTests/TestHelpers.swift +++ b/MapleBaconTests/TestHelpers.swift @@ -4,7 +4,6 @@ import UIKit import MapleBacon -import XCTest enum MockResponse { case data(Data) @@ -42,15 +41,6 @@ func makeImageData() -> Data { makeImage().pngData()! } -extension XCTestCase { - func wait(for interval: TimeInterval) { - let date = Date(timeIntervalSinceNow: interval) - while date.timeIntervalSinceNow > 0 { - CFRunLoopRunInMode(CFRunLoopMode.defaultMode, 0.1, true) - } - } -} - final class FirstDummyTransformer: ImageTransforming { let identifier = "com.schnaub.FirstDummyTransformer" From afd53c4edbfdd02f773402c7a2441cadb39b990b Mon Sep 17 00:00:00 2001 From: Jan Gorman Date: Wed, 4 Mar 2020 20:35:45 +0100 Subject: [PATCH 10/32] Prefer locking --- .../xcschemes/MapleBacon Example.xcscheme | 1 + MapleBacon/Core/Downloader.swift | 105 ++++++++++-------- 2 files changed, 58 insertions(+), 48 deletions(-) diff --git a/Example/MapleBacon Example.xcodeproj/xcshareddata/xcschemes/MapleBacon Example.xcscheme b/Example/MapleBacon Example.xcodeproj/xcshareddata/xcschemes/MapleBacon Example.xcscheme index 4b5e829..056702c 100644 --- a/Example/MapleBacon Example.xcodeproj/xcshareddata/xcschemes/MapleBacon Example.xcscheme +++ b/Example/MapleBacon Example.xcodeproj/xcshareddata/xcschemes/MapleBacon Example.xcscheme @@ -34,6 +34,7 @@ buildConfiguration = "Debug" selectedDebuggerIdentifier = "Xcode.DebuggerFoundation.Debugger.LLDB" selectedLauncherIdentifier = "Xcode.DebuggerFoundation.Launcher.LLDB" + enableThreadSanitizer = "YES" launchStyle = "0" useCustomWorkingDirectory = "NO" ignoresPersistentStateOnLaunch = "NO" diff --git a/MapleBacon/Core/Downloader.swift b/MapleBacon/Core/Downloader.swift index a8e215d..16478ea 100644 --- a/MapleBacon/Core/Downloader.swift +++ b/MapleBacon/Core/Downloader.swift @@ -8,6 +8,63 @@ enum DownloaderError: Error { case dataConversion } +final class Downloader { + + let session: URLSession + + private let sessionDelegate: SessionDelegate + + private let lock = NSLock() + private var downloads: [URL: Download] = [:] + + init(sessionConfiguration: URLSessionConfiguration = .default) { + self.sessionDelegate = SessionDelegate() + self.session = URLSession(configuration: sessionConfiguration, delegate: sessionDelegate, delegateQueue: .main) + self.sessionDelegate.downloader = self + } + + func fetch(_ url: URL, completion: @escaping (Result) -> Void) { + let task: URLSessionDataTask + if let download = download(for: url) { + task = download.task + download.completions.append(completion) + } else { + let newTask = session.dataTask(with: url) + let download = Download(task: newTask, completion: completion) + download.start() + addDownload(download, for: url) + task = newTask + } + + task.resume() + } + + private func addDownload(_ download: Download, for url: URL) { + defer { + lock.unlock() + } + lock.lock() + downloads[url] = download + } + + fileprivate func download(for url: URL) -> Download? { + defer { + lock.unlock() + } + lock.lock() + return downloads[url] + } + + fileprivate func clearDownload(for url: URL) { + defer { + lock.unlock() + } + lock.lock() + self.downloads[url] = nil + } + +} + private final class Download { let task: URLSessionDataTask @@ -42,54 +99,6 @@ private final class Download { } } -final class Downloader { - - let session: URLSession - - private let mutex: DispatchQueue - private let sessionDelegate: SessionDelegate - - private var downloads: [URL: Download] = [:] - - init(sessionConfiguration: URLSessionConfiguration = .default) { - self.sessionDelegate = SessionDelegate() - self.session = URLSession(configuration: sessionConfiguration, delegate: sessionDelegate, delegateQueue: .main) - self.mutex = DispatchQueue(label: "com.schnaub.Downloader.mutex", attributes: .concurrent) - self.sessionDelegate.downloader = self - } - - func fetch(_ url: URL, completion: @escaping (Result) -> Void) { - mutex.sync(flags: .barrier) { - let task: URLSessionDataTask - if let download = downloads[url] { - task = download.task - download.completions.append(completion) - } else { - let newTask = session.dataTask(with: url) - let download = Download(task: newTask, completion: completion) - download.start() - downloads[url] = download - task = newTask - } - - task.resume() - } - } - - fileprivate func download(for url: URL) -> Download? { - mutex.sync { - downloads[url] - } - } - - fileprivate func clearDownload(for url: URL) { - mutex.async(flags: .barrier) { - self.downloads[url] = nil - } - } - -} - private final class SessionDelegate: NSObject, URLSessionDataDelegate { weak var downloader: Downloader? From 2361d86882cfeb300bb4d6736e9c557e959a3dd9 Mon Sep 17 00:00:00 2001 From: Jan Gorman Date: Wed, 4 Mar 2020 20:46:23 +0100 Subject: [PATCH 11/32] lock at one point --- MapleBacon/Core/Downloader.swift | 55 ++++++++++++--------------- MapleBaconTests/DownloaderTests.swift | 2 + 2 files changed, 26 insertions(+), 31 deletions(-) diff --git a/MapleBacon/Core/Downloader.swift b/MapleBacon/Core/Downloader.swift index 16478ea..34ce963 100644 --- a/MapleBacon/Core/Downloader.swift +++ b/MapleBacon/Core/Downloader.swift @@ -15,7 +15,24 @@ final class Downloader { private let sessionDelegate: SessionDelegate private let lock = NSLock() - private var downloads: [URL: Download] = [:] + + private var _downloads: [URL: Download] = [:] + fileprivate var downloads: [URL: Download] { + get { + defer { + lock.unlock() + } + lock.lock() + return _downloads + } + set { + defer { + lock.unlock() + } + lock.lock() + _downloads = newValue + } + } init(sessionConfiguration: URLSessionConfiguration = .default) { self.sessionDelegate = SessionDelegate() @@ -25,44 +42,20 @@ final class Downloader { func fetch(_ url: URL, completion: @escaping (Result) -> Void) { let task: URLSessionDataTask - if let download = download(for: url) { + if let download = downloads[url] { task = download.task download.completions.append(completion) } else { let newTask = session.dataTask(with: url) let download = Download(task: newTask, completion: completion) download.start() - addDownload(download, for: url) + downloads[url] = download task = newTask } task.resume() } - private func addDownload(_ download: Download, for url: URL) { - defer { - lock.unlock() - } - lock.lock() - downloads[url] = download - } - - fileprivate func download(for url: URL) -> Download? { - defer { - lock.unlock() - } - lock.lock() - return downloads[url] - } - - fileprivate func clearDownload(for url: URL) { - defer { - lock.unlock() - } - lock.lock() - self.downloads[url] = nil - } - } private final class Download { @@ -104,18 +97,18 @@ private final class SessionDelegate: NSObject, URLSessionDat weak var downloader: Downloader? func urlSession(_ session: URLSession, dataTask: URLSessionDataTask, didReceive data: Data) { - guard let url = dataTask.originalRequest?.url, let download = downloader?.download(for: url) else { + guard let url = dataTask.originalRequest?.url, let download = downloader?.downloads[url] else { return } download.data.append(data) } func urlSession(_ session: URLSession, task: URLSessionTask, didCompleteWithError error: Error?) { - guard let url = task.originalRequest?.url, let download = downloader?.download(for: url) else { + guard let url = task.originalRequest?.url, let download = downloader?.downloads[url] else { return } - downloader?.download(for: url)?.completions.forEach { completion in + downloader?.downloads[url]?.completions.forEach { completion in if let error = error { completion(.failure(error)) return @@ -126,7 +119,7 @@ private final class SessionDelegate: NSObject, URLSessionDat } completion(.success(value)) } - downloader?.clearDownload(for: url) + downloader?.downloads[url] = nil download.finish() } diff --git a/MapleBaconTests/DownloaderTests.swift b/MapleBaconTests/DownloaderTests.swift index 1320761..00345f5 100644 --- a/MapleBaconTests/DownloaderTests.swift +++ b/MapleBaconTests/DownloaderTests.swift @@ -75,6 +75,8 @@ final class DownloaderTests: XCTestCase { let configuration = MockURLProtocol.mockedURLSessionConfiguration() let downloader = Downloader(sessionConfiguration: configuration) + setupMockResponse(.data(dummyData())) + let firstExpectation = expectation(description: "first") downloader.fetch(Self.url) { response in switch response { From 356ef2e7596594befb4bf2d7c433841642f4eff1 Mon Sep 17 00:00:00 2001 From: Jan Gorman Date: Wed, 4 Mar 2020 21:37:43 +0100 Subject: [PATCH 12/32] Switch to a set --- MapleBacon/Core/Downloader.swift | 79 +++++++++++++++++++++++---- MapleBacon/Core/MapleBacon.swift | 10 ++-- MapleBaconTests/DownloaderTests.swift | 31 +++++++++-- MapleBaconTests/MapleBaconTests.swift | 2 + 4 files changed, 101 insertions(+), 21 deletions(-) diff --git a/MapleBacon/Core/Downloader.swift b/MapleBacon/Core/Downloader.swift index 34ce963..1d738d5 100644 --- a/MapleBacon/Core/Downloader.swift +++ b/MapleBacon/Core/Downloader.swift @@ -15,9 +15,10 @@ final class Downloader { private let sessionDelegate: SessionDelegate private let lock = NSLock() + private let tokenMaker = TokenMaker() - private var _downloads: [URL: Download] = [:] - fileprivate var downloads: [URL: Download] { + private var _downloads: Set> = [] + private var downloads: Set> { get { defer { lock.unlock() @@ -34,41 +35,85 @@ final class Downloader { } } + fileprivate subscript(url: URL) -> Download? { + get { + downloads.first(where: { $0.url == url }) + } + set { + if let download = downloads.first(where: { $0.url == url }) { + downloads.remove(download) + } + } + } + init(sessionConfiguration: URLSessionConfiguration = .default) { self.sessionDelegate = SessionDelegate() self.session = URLSession(configuration: sessionConfiguration, delegate: sessionDelegate, delegateQueue: .main) self.sessionDelegate.downloader = self } - func fetch(_ url: URL, completion: @escaping (Result) -> Void) { + deinit { + session.invalidateAndCancel() + } + + func fetch(_ url: URL, completion: @escaping (Result) -> Void) -> DownloadToken { let task: URLSessionDataTask - if let download = downloads[url] { + let token: DownloadToken + if let download = self[url] { task = download.task download.completions.append(completion) + token = download.token } else { let newTask = session.dataTask(with: url) - let download = Download(task: newTask, completion: completion) + token = tokenMaker.makeToken() + let download = Download(task: newTask, url: url, token: token, completion: completion) download.start() - downloads[url] = download + downloads.insert(download) task = newTask } task.resume() + return token } + func cancel(token: DownloadToken) { + guard let download = downloads.first(where: {$0.token == token }) else { + return + } + if download.completions.isEmpty { + download.task.cancel() + downloads.remove(download) + } + } + +} + +/// Wrapper class to access a static var because generic classes cannot contain statics +private final class TokenMaker { + + private static var token: Int = 0 + + func makeToken() -> Int { + Self.token += 1 + return Self.token + } } -private final class Download { +private final class Download: Hashable { let task: URLSessionDataTask + let url: URL + let token: DownloadToken var completions: [(Result) -> Void] var data = Data() private var backgroundTask: UIBackgroundTaskIdentifier = .invalid - init(task: URLSessionDataTask, completion: @escaping (Result) -> Void) { + init(task: URLSessionDataTask, url: URL, token: DownloadToken, completion: @escaping (Result) -> Void) { self.task = task + self.url = url + self.token = token self.completions = [completion] } @@ -86,10 +131,20 @@ private final class Download { invalidateBackgroundTask() } + func hash(into hasher: inout Hasher) { + hasher.combine(url) + hasher.combine(token) + } + private func invalidateBackgroundTask() { UIApplication.shared.endBackgroundTask(backgroundTask) backgroundTask = .invalid } + + static func == (lhs: Download, rhs: Download) -> Bool { + lhs.url == rhs.url && lhs.token == rhs.token + } + } private final class SessionDelegate: NSObject, URLSessionDataDelegate { @@ -97,18 +152,18 @@ private final class SessionDelegate: NSObject, URLSessionDat weak var downloader: Downloader? func urlSession(_ session: URLSession, dataTask: URLSessionDataTask, didReceive data: Data) { - guard let url = dataTask.originalRequest?.url, let download = downloader?.downloads[url] else { + guard let url = dataTask.originalRequest?.url, let download = downloader?[url] else { return } download.data.append(data) } func urlSession(_ session: URLSession, task: URLSessionTask, didCompleteWithError error: Error?) { - guard let url = task.originalRequest?.url, let download = downloader?.downloads[url] else { + guard let url = task.originalRequest?.url, let download = downloader?[url] else { return } - downloader?.downloads[url]?.completions.forEach { completion in + downloader?[url]?.completions.forEach { completion in if let error = error { completion(.failure(error)) return @@ -119,7 +174,7 @@ private final class SessionDelegate: NSObject, URLSessionDat } completion(.success(value)) } - downloader?.downloads[url] = nil + downloader?[url] = nil download.finish() } diff --git a/MapleBacon/Core/MapleBacon.swift b/MapleBacon/Core/MapleBacon.swift index df9f370..b563ce7 100644 --- a/MapleBacon/Core/MapleBacon.swift +++ b/MapleBacon/Core/MapleBacon.swift @@ -8,6 +8,8 @@ public enum MapleBaconError: Error { case imageTransformingError } +public typealias DownloadToken = Int + public final class MapleBacon { public typealias ImageCompletion = (Result) -> Void @@ -71,8 +73,8 @@ private extension MapleBacon { } } - func fetchImageFromNetworkAndCache(with url: URL, imageTransformer: ImageTransforming?, completion: @escaping ImageCompletion) { - fetchImageFromNetwork(with: url) { result in + func fetchImageFromNetworkAndCache(with url: URL, imageTransformer: ImageTransforming?, completion: @escaping ImageCompletion) -> DownloadToken { + return fetchImageFromNetwork(with: url) { result in switch result { case .success(let image): if let transformer = imageTransformer { @@ -92,8 +94,8 @@ private extension MapleBacon { } } - func fetchImageFromNetwork(with url: URL, completion: @escaping ImageCompletion) { - downloader.fetch(url, completion: completion) + func fetchImageFromNetwork(with url: URL, completion: @escaping ImageCompletion) -> DownloadToken { + return downloader.fetch(url, completion: completion) } func transformImageAndCache(_ image: UIImage, cacheKey: String, imageTransformer: ImageTransforming, completion: @escaping ImageCompletion) { diff --git a/MapleBaconTests/DownloaderTests.swift b/MapleBaconTests/DownloaderTests.swift index 00345f5..3943d6f 100644 --- a/MapleBaconTests/DownloaderTests.swift +++ b/MapleBaconTests/DownloaderTests.swift @@ -16,7 +16,7 @@ final class DownloaderTests: XCTestCase { setupMockResponse(.data(dummyData())) - downloader.fetch(Self.url) { response in + _ = downloader.fetch(Self.url) { response in switch response { case .success(let data): XCTAssertNotNil(data) @@ -37,7 +37,7 @@ final class DownloaderTests: XCTestCase { setupMockResponse(.data(dummyData())) - downloader.fetch(Self.url) { response in + _ = downloader.fetch(Self.url) { response in switch response { case .success: XCTFail() @@ -58,7 +58,7 @@ final class DownloaderTests: XCTestCase { setupMockResponse(.error) - downloader.fetch(Self.url) { response in + _ = downloader.fetch(Self.url) { response in switch response { case .success: XCTFail() @@ -78,7 +78,7 @@ final class DownloaderTests: XCTestCase { setupMockResponse(.data(dummyData())) let firstExpectation = expectation(description: "first") - downloader.fetch(Self.url) { response in + _ = downloader.fetch(Self.url) { response in switch response { case .success(let data): XCTAssertNotNil(data) @@ -89,7 +89,7 @@ final class DownloaderTests: XCTestCase { } let secondExpectation = expectation(description: "second") - downloader.fetch(Self.url) { response in + _ = downloader.fetch(Self.url) { response in switch response { case .success(let data): XCTAssertNotNil(data) @@ -102,4 +102,25 @@ final class DownloaderTests: XCTestCase { waitForExpectations(timeout: 5, handler: nil) } + func testCancel() { + let expectation = self.expectation(description: #function) + let configuration = MockURLProtocol.mockedURLSessionConfiguration() + let downloader = Downloader(sessionConfiguration: configuration) + + setupMockResponse(.error) + + let token = downloader.fetch(Self.url) { response in + switch response { + case .success: + XCTFail() + case .failure(let error as NSError): + XCTAssertEqual(error.code, -1) + } + expectation.fulfill() + } + downloader.cancel(token: token) + + waitForExpectations(timeout: 5, handler: nil) + } + } diff --git a/MapleBaconTests/MapleBaconTests.swift b/MapleBaconTests/MapleBaconTests.swift index 8d9ec0f..4b103c0 100644 --- a/MapleBaconTests/MapleBaconTests.swift +++ b/MapleBaconTests/MapleBaconTests.swift @@ -97,6 +97,8 @@ extension MapleBaconTests { let configuration = MockURLProtocol.mockedURLSessionConfiguration() let mapleBacon = MapleBacon(cache: cache, sessionConfiguration: configuration) + setupMockResponse(.data(makeImageData())) + mapleBacon.image(with: Self.url) .sink(receiveCompletion: { _ in mapleBacon.clearCache(.all) { _ in From 0d332c1316fe78e51fb51ec0a9100381c81154ac Mon Sep 17 00:00:00 2001 From: Jan Gorman Date: Thu, 5 Mar 2020 11:29:57 +0100 Subject: [PATCH 13/32] Pass in token --- MapleBacon/Core/Downloader.swift | 24 ++++-------------------- MapleBacon/Core/MapleBacon.swift | 24 +++++++++++++++++------- MapleBaconTests/DownloaderTests.swift | 15 ++++++++------- MapleBaconTests/MapleBaconTests.swift | 3 ++- 4 files changed, 31 insertions(+), 35 deletions(-) diff --git a/MapleBacon/Core/Downloader.swift b/MapleBacon/Core/Downloader.swift index 1d738d5..4b22d54 100644 --- a/MapleBacon/Core/Downloader.swift +++ b/MapleBacon/Core/Downloader.swift @@ -15,7 +15,6 @@ final class Downloader { private let sessionDelegate: SessionDelegate private let lock = NSLock() - private let tokenMaker = TokenMaker() private var _downloads: Set> = [] private var downloads: Set> { @@ -56,16 +55,13 @@ final class Downloader { session.invalidateAndCancel() } - func fetch(_ url: URL, completion: @escaping (Result) -> Void) -> DownloadToken { + func fetch(_ url: URL, token: CancelToken, completion: @escaping (Result) -> Void) { let task: URLSessionDataTask - let token: DownloadToken if let download = self[url] { task = download.task download.completions.append(completion) - token = download.token } else { let newTask = session.dataTask(with: url) - token = tokenMaker.makeToken() let download = Download(task: newTask, url: url, token: token, completion: completion) download.start() downloads.insert(download) @@ -73,10 +69,9 @@ final class Downloader { } task.resume() - return token } - func cancel(token: DownloadToken) { + func cancel(token: CancelToken) { guard let download = downloads.first(where: {$0.token == token }) else { return } @@ -88,29 +83,18 @@ final class Downloader { } -/// Wrapper class to access a static var because generic classes cannot contain statics -private final class TokenMaker { - - private static var token: Int = 0 - - func makeToken() -> Int { - Self.token += 1 - return Self.token - } -} - private final class Download: Hashable { let task: URLSessionDataTask let url: URL - let token: DownloadToken + let token: CancelToken var completions: [(Result) -> Void] var data = Data() private var backgroundTask: UIBackgroundTaskIdentifier = .invalid - init(task: URLSessionDataTask, url: URL, token: DownloadToken, completion: @escaping (Result) -> Void) { + init(task: URLSessionDataTask, url: URL, token: CancelToken, completion: @escaping (Result) -> Void) { self.task = task self.url = url self.token = token diff --git a/MapleBacon/Core/MapleBacon.swift b/MapleBacon/Core/MapleBacon.swift index b563ce7..7a9f7fe 100644 --- a/MapleBacon/Core/MapleBacon.swift +++ b/MapleBacon/Core/MapleBacon.swift @@ -8,7 +8,7 @@ public enum MapleBaconError: Error { case imageTransformingError } -public typealias DownloadToken = Int +public typealias CancelToken = Int public final class MapleBacon { @@ -17,6 +17,7 @@ public final class MapleBacon { public static let shared = MapleBacon() private static let queueLabel = "com.schnaub.MapleBacon.transformer" + private static var token: CancelToken = 0 public var maxCacheAgeSeconds: TimeInterval { get { @@ -41,7 +42,9 @@ public final class MapleBacon { self.transformerQueue = DispatchQueue(label: Self.queueLabel, attributes: .concurrent) } - public func image(with url: URL, imageTransformer: ImageTransforming? = nil, completion: @escaping ImageCompletion) { + public func image(with url: URL, imageTransformer: ImageTransforming? = nil, completion: @escaping ImageCompletion) -> CancelToken { + let token = Self.makeToken() + fetchImageFromCache(with: url, imageTransformer: imageTransformer) { result in switch result { case .success(let image): @@ -49,9 +52,10 @@ public final class MapleBacon { completion(.success(image)) } case .failure: - self.fetchImageFromNetworkAndCache(with: url, imageTransformer: imageTransformer, completion: completion) + self.fetchImageFromNetworkAndCache(with: url, token: token, imageTransformer: imageTransformer, completion: completion) } } + return token } public func clearCache(_ options: CacheClearOptions, completion: ((Error?) -> Void)? = nil) { @@ -61,6 +65,12 @@ public final class MapleBacon { } private extension MapleBacon { + + static func makeToken() -> CancelToken { + token += 1 + return token + } + func fetchImageFromCache(with url: URL, imageTransformer: ImageTransforming?, completion: @escaping ImageCompletion) { let cacheKey = makeCacheKey(for: url, imageTransformer: imageTransformer) cache.value(forKey: cacheKey) { result in @@ -73,8 +83,8 @@ private extension MapleBacon { } } - func fetchImageFromNetworkAndCache(with url: URL, imageTransformer: ImageTransforming?, completion: @escaping ImageCompletion) -> DownloadToken { - return fetchImageFromNetwork(with: url) { result in + func fetchImageFromNetworkAndCache(with url: URL, token: CancelToken, imageTransformer: ImageTransforming?, completion: @escaping ImageCompletion) { + fetchImageFromNetwork(with: url, token: token) { result in switch result { case .success(let image): if let transformer = imageTransformer { @@ -94,8 +104,8 @@ private extension MapleBacon { } } - func fetchImageFromNetwork(with url: URL, completion: @escaping ImageCompletion) -> DownloadToken { - return downloader.fetch(url, completion: completion) + func fetchImageFromNetwork(with url: URL, token: CancelToken, completion: @escaping ImageCompletion) { + downloader.fetch(url, token: token, completion: completion) } func transformImageAndCache(_ image: UIImage, cacheKey: String, imageTransformer: ImageTransforming, completion: @escaping ImageCompletion) { diff --git a/MapleBaconTests/DownloaderTests.swift b/MapleBaconTests/DownloaderTests.swift index 3943d6f..fbff7f9 100644 --- a/MapleBaconTests/DownloaderTests.swift +++ b/MapleBaconTests/DownloaderTests.swift @@ -8,6 +8,7 @@ import XCTest final class DownloaderTests: XCTestCase { private static let url = URL(string: "https://example.com/mapleBacon.png")! + private static let cancelToken = 1 func testDownload() { let expectation = self.expectation(description: #function) @@ -16,7 +17,7 @@ final class DownloaderTests: XCTestCase { setupMockResponse(.data(dummyData())) - _ = downloader.fetch(Self.url) { response in + downloader.fetch(Self.url, token: Self.cancelToken) { response in switch response { case .success(let data): XCTAssertNotNil(data) @@ -37,7 +38,7 @@ final class DownloaderTests: XCTestCase { setupMockResponse(.data(dummyData())) - _ = downloader.fetch(Self.url) { response in + downloader.fetch(Self.url, token: Self.cancelToken) { response in switch response { case .success: XCTFail() @@ -58,7 +59,7 @@ final class DownloaderTests: XCTestCase { setupMockResponse(.error) - _ = downloader.fetch(Self.url) { response in + downloader.fetch(Self.url, token: Self.cancelToken) { response in switch response { case .success: XCTFail() @@ -78,7 +79,7 @@ final class DownloaderTests: XCTestCase { setupMockResponse(.data(dummyData())) let firstExpectation = expectation(description: "first") - _ = downloader.fetch(Self.url) { response in + downloader.fetch(Self.url, token: Self.cancelToken) { response in switch response { case .success(let data): XCTAssertNotNil(data) @@ -89,7 +90,7 @@ final class DownloaderTests: XCTestCase { } let secondExpectation = expectation(description: "second") - _ = downloader.fetch(Self.url) { response in + downloader.fetch(Self.url, token: Self.cancelToken) { response in switch response { case .success(let data): XCTAssertNotNil(data) @@ -109,7 +110,7 @@ final class DownloaderTests: XCTestCase { setupMockResponse(.error) - let token = downloader.fetch(Self.url) { response in + downloader.fetch(Self.url, token: Self.cancelToken) { response in switch response { case .success: XCTFail() @@ -118,7 +119,7 @@ final class DownloaderTests: XCTestCase { } expectation.fulfill() } - downloader.cancel(token: token) + downloader.cancel(token: Self.cancelToken) waitForExpectations(timeout: 5, handler: nil) } diff --git a/MapleBaconTests/MapleBaconTests.swift b/MapleBaconTests/MapleBaconTests.swift index 4b103c0..a245698 100644 --- a/MapleBaconTests/MapleBaconTests.swift +++ b/MapleBaconTests/MapleBaconTests.swift @@ -24,7 +24,7 @@ final class MapleBaconTests: XCTestCase { setupMockResponse(.data(makeImageData())) - mapleBacon.image(with: Self.url) { result in + let token = mapleBacon.image(with: Self.url) { result in switch result { case .success(let image): XCTAssertEqual(image.pngData(), makeImageData()) @@ -36,6 +36,7 @@ final class MapleBaconTests: XCTestCase { } } + XCTAssertNotNil(token) waitForExpectations(timeout: 5, handler: nil) } From 39e79c8fa2e49c16f63b6def4e28bcd532f9c4f7 Mon Sep 17 00:00:00 2001 From: Jan Gorman Date: Thu, 5 Mar 2020 11:33:31 +0100 Subject: [PATCH 14/32] And allow to cancel a download --- MapleBacon/Core/MapleBacon.swift | 5 +++++ MapleBaconTests/MapleBaconTests.swift | 23 +++++++++++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/MapleBacon/Core/MapleBacon.swift b/MapleBacon/Core/MapleBacon.swift index 7a9f7fe..ff7de84 100644 --- a/MapleBacon/Core/MapleBacon.swift +++ b/MapleBacon/Core/MapleBacon.swift @@ -42,6 +42,7 @@ public final class MapleBacon { self.transformerQueue = DispatchQueue(label: Self.queueLabel, attributes: .concurrent) } + @discardableResult public func image(with url: URL, imageTransformer: ImageTransforming? = nil, completion: @escaping ImageCompletion) -> CancelToken { let token = Self.makeToken() @@ -62,6 +63,10 @@ public final class MapleBacon { cache.clear(options, completion: completion) } + public func cancel(token: CancelToken) { + downloader.cancel(token: token) + } + } private extension MapleBacon { diff --git a/MapleBaconTests/MapleBaconTests.swift b/MapleBaconTests/MapleBaconTests.swift index a245698..bb53ac7 100644 --- a/MapleBaconTests/MapleBaconTests.swift +++ b/MapleBaconTests/MapleBaconTests.swift @@ -86,6 +86,29 @@ final class MapleBaconTests: XCTestCase { waitForExpectations(timeout: 5, handler: nil) } + func testCancel() { + let expectation = self.expectation(description: #function) + let configuration = MockURLProtocol.mockedURLSessionConfiguration() + let mapleBacon = MapleBacon(cache: cache, sessionConfiguration: configuration) + + setupMockResponse(.error) + + let token = mapleBacon.image(with: Self.url) { result in + switch result { + case .success: + XCTFail() + case .failure(let error as NSError): + XCTAssertEqual(error.code, -1) + } + mapleBacon.clearCache(.all) { _ in + expectation.fulfill() + } + } + mapleBacon.cancel(token: token) + + waitForExpectations(timeout: 5, handler: nil) + } + } #if canImport(Combine) From 004eb5ee9dc49bbf99acf4d4144fee7fb837641d Mon Sep 17 00:00:00 2001 From: Jan Gorman Date: Thu, 5 Mar 2020 11:43:27 +0100 Subject: [PATCH 15/32] Cancel from image extension --- .../Extensions/UIImageView+MapleBacon.swift | 25 +++++++++++++++---- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/MapleBacon/Extensions/UIImageView+MapleBacon.swift b/MapleBacon/Extensions/UIImageView+MapleBacon.swift index 9716bc6..1a03483 100644 --- a/MapleBacon/Extensions/UIImageView+MapleBacon.swift +++ b/MapleBacon/Extensions/UIImageView+MapleBacon.swift @@ -5,6 +5,7 @@ import UIKit private var baconImageUrlKey: UInt8 = 0 +private var cancelTokenKey: UInt8 = 1 extension UIImageView { @@ -17,23 +18,30 @@ extension UIImageView { } } + private var cancelToken: CancelToken? { + get { + objc_getAssociatedObject(self, &cancelTokenKey) as? Int + } + set { + objc_setAssociatedObject(self, &cancelTokenKey, newValue, .OBJC_ASSOCIATION_RETAIN_NONATOMIC) + } + } + public func setImage(with url: URL?, placeholder: UIImage? = nil, displayOptions: [DisplayOptions] = [], imageTransformer: ImageTransforming? = nil, completion: (() -> Void)? = nil) { + cancelDownload() baconImageUrl = url + image = placeholder guard let url = url else { return } - if let placeholder = placeholder { - image = placeholder - } - let transformer = makeTransformer(displayOptions: displayOptions, imageTransformer: imageTransformer) - MapleBacon.shared.image(with: url, imageTransformer: transformer) { [weak self] result in + cancelToken = MapleBacon.shared.image(with: url, imageTransformer: transformer) { [weak self] result in defer { completion?() } @@ -44,6 +52,13 @@ extension UIImageView { } } + private func cancelDownload() { + guard let token = cancelToken else { + return + } + MapleBacon.shared.cancel(token: token) + } + private func makeTransformer(displayOptions: [DisplayOptions] = [], imageTransformer: ImageTransforming?) -> ImageTransforming? { guard displayOptions.contains(.downsampled) else { return imageTransformer From 618b72153400e88f8d5b2aaa9122aa9d9d87df87 Mon Sep 17 00:00:00 2001 From: Jan Gorman Date: Thu, 5 Mar 2020 16:45:25 +0100 Subject: [PATCH 16/32] Switch back to Dictionary. We have the key upfront now --- MapleBacon/Core/Downloader.swift | 21 ++++++++++++--------- MapleBaconTests/DownloaderTests.swift | 6 +++--- 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/MapleBacon/Core/Downloader.swift b/MapleBacon/Core/Downloader.swift index 4b22d54..a25d993 100644 --- a/MapleBacon/Core/Downloader.swift +++ b/MapleBacon/Core/Downloader.swift @@ -6,6 +6,7 @@ import UIKit enum DownloaderError: Error { case dataConversion + case canceled } final class Downloader { @@ -16,8 +17,8 @@ final class Downloader { private let lock = NSLock() - private var _downloads: Set> = [] - private var downloads: Set> { + private var _downloads: [CancelToken: Download] = [:] + private var downloads: [CancelToken: Download] { get { defer { lock.unlock() @@ -36,11 +37,11 @@ final class Downloader { fileprivate subscript(url: URL) -> Download? { get { - downloads.first(where: { $0.url == url }) + downloads.values.first(where: { $0.url == url }) } set { - if let download = downloads.first(where: { $0.url == url }) { - downloads.remove(download) + if let keyValue = downloads.first(where: { $1.url == url }) { + downloads[keyValue.key] = nil } } } @@ -57,6 +58,7 @@ final class Downloader { func fetch(_ url: URL, token: CancelToken, completion: @escaping (Result) -> Void) { let task: URLSessionDataTask + // TODO this is not great for lookup speed if let download = self[url] { task = download.task download.completions.append(completion) @@ -64,7 +66,7 @@ final class Downloader { let newTask = session.dataTask(with: url) let download = Download(task: newTask, url: url, token: token, completion: completion) download.start() - downloads.insert(download) + downloads[token] = download task = newTask } @@ -72,12 +74,13 @@ final class Downloader { } func cancel(token: CancelToken) { - guard let download = downloads.first(where: {$0.token == token }) else { + guard let download = downloads[token] else { return } - if download.completions.isEmpty { + if download.completions.count == 1 { download.task.cancel() - downloads.remove(download) + download.completions.first?(.failure(DownloaderError.canceled)) + downloads[token] = nil } } diff --git a/MapleBaconTests/DownloaderTests.swift b/MapleBaconTests/DownloaderTests.swift index fbff7f9..85eb038 100644 --- a/MapleBaconTests/DownloaderTests.swift +++ b/MapleBaconTests/DownloaderTests.swift @@ -112,10 +112,10 @@ final class DownloaderTests: XCTestCase { downloader.fetch(Self.url, token: Self.cancelToken) { response in switch response { - case .success: + case .failure(let error as DownloaderError): + XCTAssertEqual(error, .canceled) + case .success, .failure: XCTFail() - case .failure(let error as NSError): - XCTAssertEqual(error.code, -1) } expectation.fulfill() } From d31916e2f71f30a60bf974e7c7f64166e23a0c4c Mon Sep 17 00:00:00 2001 From: Jan Gorman Date: Thu, 5 Mar 2020 16:48:20 +0100 Subject: [PATCH 17/32] Remove Hashable conformance --- MapleBacon/Core/Downloader.swift | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/MapleBacon/Core/Downloader.swift b/MapleBacon/Core/Downloader.swift index a25d993..12cf4c2 100644 --- a/MapleBacon/Core/Downloader.swift +++ b/MapleBacon/Core/Downloader.swift @@ -86,7 +86,7 @@ final class Downloader { } -private final class Download: Hashable { +private final class Download { let task: URLSessionDataTask let url: URL @@ -118,20 +118,11 @@ private final class Download: Hashable { invalidateBackgroundTask() } - func hash(into hasher: inout Hasher) { - hasher.combine(url) - hasher.combine(token) - } - private func invalidateBackgroundTask() { UIApplication.shared.endBackgroundTask(backgroundTask) backgroundTask = .invalid } - static func == (lhs: Download, rhs: Download) -> Bool { - lhs.url == rhs.url && lhs.token == rhs.token - } - } private final class SessionDelegate: NSObject, URLSessionDataDelegate { From 50c162e6beda8a06f6da176303b808faaf3f37fc Mon Sep 17 00:00:00 2001 From: Jan Gorman Date: Fri, 6 Mar 2020 11:15:38 +0100 Subject: [PATCH 18/32] Double keyed container --- MapleBacon.xcodeproj/project.pbxproj | 8 +++ ...C1DF3A3C-F888-4084-9C7D-6D8EC38B7591.plist | 32 +++++++++ .../Info.plist | 40 +++++++++++ MapleBacon/Core/DoubleKeyedContainer.swift | 65 +++++++++++++++++ MapleBacon/Core/Downloader.swift | 35 ++++++--- .../DoubleKeyedContainerTests.swift | 71 +++++++++++++++++++ 6 files changed, 240 insertions(+), 11 deletions(-) create mode 100644 MapleBacon.xcodeproj/xcshareddata/xcbaselines/F247A24623FF15410083DB03.xcbaseline/C1DF3A3C-F888-4084-9C7D-6D8EC38B7591.plist create mode 100644 MapleBacon.xcodeproj/xcshareddata/xcbaselines/F247A24623FF15410083DB03.xcbaseline/Info.plist create mode 100644 MapleBacon/Core/DoubleKeyedContainer.swift create mode 100644 MapleBaconTests/DoubleKeyedContainerTests.swift diff --git a/MapleBacon.xcodeproj/project.pbxproj b/MapleBacon.xcodeproj/project.pbxproj index c3ace81..ab4a1e9 100644 --- a/MapleBacon.xcodeproj/project.pbxproj +++ b/MapleBacon.xcodeproj/project.pbxproj @@ -8,6 +8,8 @@ /* Begin PBXBuildFile section */ 7778FD28240FAC6B007764C6 /* DispatchQueue+MapleBacon.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7778FD27240FAC6B007764C6 /* DispatchQueue+MapleBacon.swift */; }; + 77E75F812412445E00611894 /* DoubleKeyedContainer.swift in Sources */ = {isa = PBXBuildFile; fileRef = 77E75F802412445E00611894 /* DoubleKeyedContainer.swift */; }; + 77E75F8324124A4300611894 /* DoubleKeyedContainerTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 77E75F8224124A4300611894 /* DoubleKeyedContainerTests.swift */; }; F2255E202404600D00193742 /* UIImageView+MapleBacon.swift in Sources */ = {isa = PBXBuildFile; fileRef = F2255E1F2404600D00193742 /* UIImageView+MapleBacon.swift */; }; F2255E222404606E00193742 /* MapleBacon.swift in Sources */ = {isa = PBXBuildFile; fileRef = F2255E212404606E00193742 /* MapleBacon.swift */; }; F2255E24240462D800193742 /* MapleBaconTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = F2255E23240462D800193742 /* MapleBaconTests.swift */; }; @@ -47,6 +49,8 @@ /* Begin PBXFileReference section */ 7778FD27240FAC6B007764C6 /* DispatchQueue+MapleBacon.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "DispatchQueue+MapleBacon.swift"; sourceTree = ""; }; + 77E75F802412445E00611894 /* DoubleKeyedContainer.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DoubleKeyedContainer.swift; sourceTree = ""; }; + 77E75F8224124A4300611894 /* DoubleKeyedContainerTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DoubleKeyedContainerTests.swift; sourceTree = ""; }; F2255E1F2404600D00193742 /* UIImageView+MapleBacon.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "UIImageView+MapleBacon.swift"; sourceTree = ""; }; F2255E212404606E00193742 /* MapleBacon.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MapleBacon.swift; sourceTree = ""; }; F2255E23240462D800193742 /* MapleBaconTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MapleBaconTests.swift; sourceTree = ""; }; @@ -132,6 +136,7 @@ children = ( F27CBA0F2402B2F0006CD529 /* CacheTests.swift */, F247A25F23FF19300083DB03 /* DiskCacheTests.swift */, + 77E75F8224124A4300611894 /* DoubleKeyedContainerTests.swift */, F27CBA212402DC0B006CD529 /* DownloaderTests.swift */, F2EEE586240BBE040002C9CA /* ImageTransformingTests.swift */, F247A24E23FF15410083DB03 /* Info.plist */, @@ -150,6 +155,7 @@ F27CBA192402CC13006CD529 /* CacheClearOptions.swift */, F27CBA1B2402CC28006CD529 /* CacheResult.swift */, F247A25D23FF191C0083DB03 /* DiskCache.swift */, + 77E75F802412445E00611894 /* DoubleKeyedContainer.swift */, F27CBA1D2402D9A8006CD529 /* Downloader.swift */, F247A25923FF15980083DB03 /* MemoryCache.swift */, F2255E212404606E00193742 /* MapleBacon.swift */, @@ -288,6 +294,7 @@ F2255E202404600D00193742 /* UIImageView+MapleBacon.swift in Sources */, F247A25E23FF191C0083DB03 /* DiskCache.swift in Sources */, F27CBA182402C551006CD529 /* FileManager.swift in Sources */, + 77E75F812412445E00611894 /* DoubleKeyedContainer.swift in Sources */, F2EEE589240BBEF90002C9CA /* DownsamplingImageTransformer.swift in Sources */, F2EEE58D240BC17F0002C9CA /* DisplayOptions.swift in Sources */, F27CBA0E2402B1FE006CD529 /* Cache.swift in Sources */, @@ -307,6 +314,7 @@ buildActionMask = 2147483647; files = ( F27CBA222402DC0B006CD529 /* DownloaderTests.swift in Sources */, + 77E75F8324124A4300611894 /* DoubleKeyedContainerTests.swift in Sources */, F247A25C23FF163F0083DB03 /* MemoryCacheTests.swift in Sources */, F2255E24240462D800193742 /* MapleBaconTests.swift in Sources */, F247A26023FF19300083DB03 /* DiskCacheTests.swift in Sources */, diff --git a/MapleBacon.xcodeproj/xcshareddata/xcbaselines/F247A24623FF15410083DB03.xcbaseline/C1DF3A3C-F888-4084-9C7D-6D8EC38B7591.plist b/MapleBacon.xcodeproj/xcshareddata/xcbaselines/F247A24623FF15410083DB03.xcbaseline/C1DF3A3C-F888-4084-9C7D-6D8EC38B7591.plist new file mode 100644 index 0000000..170fd9f --- /dev/null +++ b/MapleBacon.xcodeproj/xcshareddata/xcbaselines/F247A24623FF15410083DB03.xcbaseline/C1DF3A3C-F888-4084-9C7D-6D8EC38B7591.plist @@ -0,0 +1,32 @@ + + + + + classNames + + DoubleKeyedContainerTests + + testAccessPerformance() + + com.apple.XCTPerformanceMetric_WallClockTime + + baselineAverage + 9.68e-05 + baselineIntegrationDisplayName + Local Baseline + + + testPerformanceExample() + + com.apple.XCTPerformanceMetric_WallClockTime + + baselineAverage + 5.4116e-05 + baselineIntegrationDisplayName + Local Baseline + + + + + + diff --git a/MapleBacon.xcodeproj/xcshareddata/xcbaselines/F247A24623FF15410083DB03.xcbaseline/Info.plist b/MapleBacon.xcodeproj/xcshareddata/xcbaselines/F247A24623FF15410083DB03.xcbaseline/Info.plist new file mode 100644 index 0000000..eb45cc9 --- /dev/null +++ b/MapleBacon.xcodeproj/xcshareddata/xcbaselines/F247A24623FF15410083DB03.xcbaseline/Info.plist @@ -0,0 +1,40 @@ + + + + + runDestinationsByUUID + + C1DF3A3C-F888-4084-9C7D-6D8EC38B7591 + + localComputer + + busSpeedInMHz + 100 + cpuCount + 1 + cpuKind + Dual-Core Intel Core i5 + cpuSpeedInMHz + 2900 + logicalCPUCoresPerPackage + 4 + modelCode + MacBookPro13,2 + physicalCPUCoresPerPackage + 2 + platformIdentifier + com.apple.platform.macosx + + targetArchitecture + x86_64 + targetDevice + + modelCode + iPhone12,5 + platformIdentifier + com.apple.platform.iphonesimulator + + + + + diff --git a/MapleBacon/Core/DoubleKeyedContainer.swift b/MapleBacon/Core/DoubleKeyedContainer.swift new file mode 100644 index 0000000..20a5aa5 --- /dev/null +++ b/MapleBacon/Core/DoubleKeyedContainer.swift @@ -0,0 +1,65 @@ +// +// Copyright © 2020 Schnaub. All rights reserved. +// + +import Foundation + +/// Container structure that associates two keys with a single value. +/// Internally it keeps two Dictionaries, the first associates the keys with one another +/// and the second one associates one of the keys with the value. To ensure that remove +/// and insert are as fast as possible, removing a value for the second key type will not +/// remove it from the lookup map. Since the link between the keys is broken, this won't +/// lead to unexpected values but bear in mind that the lookup Dictionary will grow indefinitely. +final class DoubleKeyedContainer { + + private var firstContainer: [FirstKey: SecondKey] = [:] + private var secondContainer: [SecondKey: Value] = [:] + + subscript(_ key: FirstKey) -> Value? { + get { + guard let lookup = firstContainer[key] else { + return nil + } + return secondContainer[lookup] + } + } + + subscript(_ key: SecondKey) -> Value? { + get { + secondContainer[key] + } + } + + func insert(_ value: Value, forKeys keys: (FirstKey, SecondKey)) { + firstContainer[keys.0] = keys.1 + secondContainer[keys.1] = value + } + + func update(_ value: Value, forKey key: FirstKey) { + guard let lookup = firstContainer[key] else { + return + } + secondContainer[lookup] = value + } + + func update(_ value: Value, forKey key: SecondKey) { + secondContainer[key] = value + } + + func removeValue(forKeys: (FirstKey, SecondKey)) { + firstContainer[forKeys.0] = nil + secondContainer[forKeys.1] = nil + } + + func removeValue(forKey key: FirstKey) { + guard let lookup = firstContainer[key] else { + return + } + removeValue(forKeys: (key, lookup)) + } + + func removeValue(forKey key: SecondKey) { + secondContainer[key] = nil + } + +} diff --git a/MapleBacon/Core/Downloader.swift b/MapleBacon/Core/Downloader.swift index 12cf4c2..9471342 100644 --- a/MapleBacon/Core/Downloader.swift +++ b/MapleBacon/Core/Downloader.swift @@ -16,33 +16,47 @@ final class Downloader { private let sessionDelegate: SessionDelegate private let lock = NSLock() + private let downloads = DoubleKeyedContainer>() - private var _downloads: [CancelToken: Download] = [:] - private var downloads: [CancelToken: Download] { + fileprivate subscript(_ url: URL) -> Download? { get { defer { lock.unlock() } lock.lock() - return _downloads + return downloads[url] } set { defer { lock.unlock() } lock.lock() - _downloads = newValue + guard let newValue = newValue else { + downloads.removeValue(forKey: url) + return + } + downloads.update(newValue, forKey: url) } } - fileprivate subscript(url: URL) -> Download? { + fileprivate subscript(_ token: CancelToken) -> Download? { get { - downloads.values.first(where: { $0.url == url }) + defer { + lock.unlock() + } + lock.lock() + return downloads[token] } set { - if let keyValue = downloads.first(where: { $1.url == url }) { - downloads[keyValue.key] = nil + defer { + lock.unlock() + } + lock.lock() + guard let newValue = newValue else { + downloads.removeValue(forKey: token) + return } + downloads.update(newValue, forKey: token) } } @@ -58,7 +72,6 @@ final class Downloader { func fetch(_ url: URL, token: CancelToken, completion: @escaping (Result) -> Void) { let task: URLSessionDataTask - // TODO this is not great for lookup speed if let download = self[url] { task = download.task download.completions.append(completion) @@ -66,7 +79,7 @@ final class Downloader { let newTask = session.dataTask(with: url) let download = Download(task: newTask, url: url, token: token, completion: completion) download.start() - downloads[token] = download + downloads.insert(download, forKeys: (token, url)) task = newTask } @@ -80,7 +93,7 @@ final class Downloader { if download.completions.count == 1 { download.task.cancel() download.completions.first?(.failure(DownloaderError.canceled)) - downloads[token] = nil + self[token] = nil } } diff --git a/MapleBaconTests/DoubleKeyedContainerTests.swift b/MapleBaconTests/DoubleKeyedContainerTests.swift new file mode 100644 index 0000000..9d4c291 --- /dev/null +++ b/MapleBaconTests/DoubleKeyedContainerTests.swift @@ -0,0 +1,71 @@ +// +// Copyright © 2020 Schnaub. All rights reserved. +// + +@testable import MapleBacon +import XCTest + +final class DoubleKeyedContainerTests: XCTestCase { + + func testInsert() { + let container = DoubleKeyedContainer() + container.insert("foo", forKeys: (1, "fooKey")) + + XCTAssertEqual(container[1], "foo") + XCTAssertEqual(container["fooKey"], "foo") + XCTAssertNil(container[100]) + XCTAssertNil(container["randomKey"]) + } + + func testRemoveAllKeys() { + let container = DoubleKeyedContainer() + container.insert("foo", forKeys: (1, "fooKey")) + container.removeValue(forKeys: (1, "fooKey")) + + XCTAssertNil(container[1]) + XCTAssertNil(container["fooKey"]) + } + + func testRemoveFirstKey() { + let container = DoubleKeyedContainer() + container.insert("foo", forKeys: (1, "fooKey")) + container.removeValue(forKey: 1) + container.removeValue(forKey: 100) + + XCTAssertNil(container[1]) + XCTAssertNil(container["fooKey"]) + } + + func testRemoveSecondKey() { + let container = DoubleKeyedContainer() + container.insert("foo", forKeys: (1, "fooKey")) + container.removeValue(forKey: "fooKey") + + XCTAssertNil(container[1]) + XCTAssertNil(container["fooKey"]) + } + + func testUpdate() { + let container = DoubleKeyedContainer() + container.insert("foo", forKeys: (1, "fooKey")) + + container.update("bar", forKey: 1) + XCTAssertEqual(container[1], "bar") + XCTAssertEqual(container["fooKey"], "bar") + + container.update("baz", forKey: "fooKey") + XCTAssertEqual(container[1], "baz") + XCTAssertEqual(container["fooKey"], "baz") + } + + func testAccessPerformance() { + let container = DoubleKeyedContainer() + container.insert("foo", forKeys: (1, "fooKey")) + + self.measure { + XCTAssertEqual(container[1], "foo") + XCTAssertEqual(container["fooKey"], "foo") + } + } + +} From 88db03032af52d22692f4ede82380ae07c0c189b Mon Sep 17 00:00:00 2001 From: Jan Gorman Date: Sun, 8 Mar 2020 17:11:51 +0100 Subject: [PATCH 19/32] Hydrate cache method --- MapleBacon/Core/MapleBacon.swift | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/MapleBacon/Core/MapleBacon.swift b/MapleBacon/Core/MapleBacon.swift index ff7de84..d65f1e8 100644 --- a/MapleBacon/Core/MapleBacon.swift +++ b/MapleBacon/Core/MapleBacon.swift @@ -59,6 +59,10 @@ public final class MapleBacon { return token } + public func hydrateCache(urls: [URL]) { + urls.forEach { self.image(with: $0) { _ in } } + } + public func clearCache(_ options: CacheClearOptions, completion: ((Error?) -> Void)? = nil) { cache.clear(options, completion: completion) } From f57f57ab92c92f61297dbaaad8341d59c520dac4 Mon Sep 17 00:00:00 2001 From: Jan Gorman Date: Sun, 8 Mar 2020 17:31:02 +0100 Subject: [PATCH 20/32] Tweak hydrate --- MapleBacon/Core/MapleBacon.swift | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/MapleBacon/Core/MapleBacon.swift b/MapleBacon/Core/MapleBacon.swift index d65f1e8..5eec25e 100644 --- a/MapleBacon/Core/MapleBacon.swift +++ b/MapleBacon/Core/MapleBacon.swift @@ -60,7 +60,18 @@ public final class MapleBacon { } public func hydrateCache(urls: [URL]) { - urls.forEach { self.image(with: $0) { _ in } } + urls.forEach { url in + let cacheKey = makeCacheKey(for: url, imageTransformer: nil) + cache.value(forKey: cacheKey) { result in + switch result { + case .failure: + let token = Self.makeToken() + self.fetchImageFromNetworkAndCache(with: url, token: token, imageTransformer: nil, completion: { _ in }) + default: + break + } + } + } } public func clearCache(_ options: CacheClearOptions, completion: ((Error?) -> Void)? = nil) { From 7df3611458e7be56fd35f13682de143c0f2e73e9 Mon Sep 17 00:00:00 2001 From: Jan Gorman Date: Mon, 9 Mar 2020 20:12:07 +0100 Subject: [PATCH 21/32] Atomic and some more examples --- .../project.pbxproj | 8 ++ .../Base.lproj/Main.storyboard | 87 +++++++++++++++++-- .../Controllers/EntryViewController.swift | 14 +++ .../Controllers/PrefetchViewController.swift | 48 ++++++++++ MapleBacon.xcodeproj/project.pbxproj | 4 + MapleBacon/Core/Atomic.swift | 31 +++++++ MapleBacon/Core/MapleBacon.swift | 31 ++++--- 7 files changed, 201 insertions(+), 22 deletions(-) create mode 100644 Example/MapleBacon Example/Controllers/EntryViewController.swift create mode 100644 Example/MapleBacon Example/Controllers/PrefetchViewController.swift create mode 100644 MapleBacon/Core/Atomic.swift diff --git a/Example/MapleBacon Example.xcodeproj/project.pbxproj b/Example/MapleBacon Example.xcodeproj/project.pbxproj index 35aa23a..c9a5436 100644 --- a/Example/MapleBacon Example.xcodeproj/project.pbxproj +++ b/Example/MapleBacon Example.xcodeproj/project.pbxproj @@ -17,6 +17,8 @@ F2CB22EE240A6953009FB183 /* images.plist in Resources */ = {isa = PBXBuildFile; fileRef = F2CB22ED240A6953009FB183 /* images.plist */; }; F2CB22F5240BB864009FB183 /* DownsamplingViewController.swift in Sources */ = {isa = PBXBuildFile; fileRef = F2CB22F4240BB864009FB183 /* DownsamplingViewController.swift */; }; F2CB22F7240BB8DC009FB183 /* ImageCollectionViewCell.swift in Sources */ = {isa = PBXBuildFile; fileRef = F2CB22F6240BB8DC009FB183 /* ImageCollectionViewCell.swift */; }; + F2D870D62416C63200946A0B /* PrefetchViewController.swift in Sources */ = {isa = PBXBuildFile; fileRef = F2D870D52416C63200946A0B /* PrefetchViewController.swift */; }; + F2D870D92416C6AB00946A0B /* EntryViewController.swift in Sources */ = {isa = PBXBuildFile; fileRef = F2D870D82416C6AB00946A0B /* EntryViewController.swift */; }; F2EEE58F240BCDA60002C9CA /* ImageTransformerViewController.swift in Sources */ = {isa = PBXBuildFile; fileRef = F2EEE58E240BCDA60002C9CA /* ImageTransformerViewController.swift */; }; /* End PBXBuildFile section */ @@ -58,6 +60,8 @@ F2CB22ED240A6953009FB183 /* images.plist */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.plist.xml; path = images.plist; sourceTree = ""; }; F2CB22F4240BB864009FB183 /* DownsamplingViewController.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DownsamplingViewController.swift; sourceTree = ""; }; F2CB22F6240BB8DC009FB183 /* ImageCollectionViewCell.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ImageCollectionViewCell.swift; sourceTree = ""; }; + F2D870D52416C63200946A0B /* PrefetchViewController.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PrefetchViewController.swift; sourceTree = ""; }; + F2D870D82416C6AB00946A0B /* EntryViewController.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = EntryViewController.swift; sourceTree = ""; }; F2EEE58E240BCDA60002C9CA /* ImageTransformerViewController.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ImageTransformerViewController.swift; sourceTree = ""; }; /* End PBXFileReference section */ @@ -122,6 +126,8 @@ F29D6184240595BA00E85F11 /* CollectionViewController.swift */, F2CB22F4240BB864009FB183 /* DownsamplingViewController.swift */, F2EEE58E240BCDA60002C9CA /* ImageTransformerViewController.swift */, + F2D870D52416C63200946A0B /* PrefetchViewController.swift */, + F2D870D82416C6AB00946A0B /* EntryViewController.swift */, ); path = Controllers; sourceTree = ""; @@ -236,8 +242,10 @@ F29D6185240595BA00E85F11 /* CollectionViewController.swift in Sources */, F29D6181240595BA00E85F11 /* AppDelegate.swift in Sources */, F2CB22EC240A68D1009FB183 /* UICollectionView+MapleBacon.swift in Sources */, + F2D870D92416C6AB00946A0B /* EntryViewController.swift in Sources */, F2EEE58F240BCDA60002C9CA /* ImageTransformerViewController.swift in Sources */, F29D6183240595BA00E85F11 /* SceneDelegate.swift in Sources */, + F2D870D62416C63200946A0B /* PrefetchViewController.swift in Sources */, ); runOnlyForDeploymentPostprocessing = 0; }; diff --git a/Example/MapleBacon Example/Base.lproj/Main.storyboard b/Example/MapleBacon Example/Base.lproj/Main.storyboard index 82d898c..7d2b89c 100644 --- a/Example/MapleBacon Example/Base.lproj/Main.storyboard +++ b/Example/MapleBacon Example/Base.lproj/Main.storyboard @@ -1,8 +1,8 @@ - + - + @@ -10,7 +10,7 @@ - + @@ -75,7 +75,27 @@ - + + + + + + + + + + + + + + + @@ -86,7 +106,13 @@ - + + + + + + + @@ -255,7 +281,56 @@ - + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/Example/MapleBacon Example/Controllers/EntryViewController.swift b/Example/MapleBacon Example/Controllers/EntryViewController.swift new file mode 100644 index 0000000..16695b9 --- /dev/null +++ b/Example/MapleBacon Example/Controllers/EntryViewController.swift @@ -0,0 +1,14 @@ +// +// Copyright © 2020 Schnaub. All rights reserved. +// + +import MapleBacon +import UIKit + +final class EntryViewController: UITableViewController { + + @IBAction private func clearCache(_ sender: Any) { + MapleBacon.shared.clearCache(.all) + } + +} diff --git a/Example/MapleBacon Example/Controllers/PrefetchViewController.swift b/Example/MapleBacon Example/Controllers/PrefetchViewController.swift new file mode 100644 index 0000000..bd2e09f --- /dev/null +++ b/Example/MapleBacon Example/Controllers/PrefetchViewController.swift @@ -0,0 +1,48 @@ +// +// Copyright © 2020 Schnaub. All rights reserved. +// + +import MapleBacon +import UIKit + +final class PrefetchViewController: UICollectionViewController { + + private var imageURLs: [URL] = [] + + override func viewDidLoad() { + super.viewDidLoad() + + imageURLs = imageURLsFromBundle() + collectionView.prefetchDataSource = self + collectionView?.reloadData() + } + + private func imageURLsFromBundle() -> [URL] { + let file = Bundle.main.path(forResource: "images", ofType: "plist")! + let urls = NSArray(contentsOfFile: file) as! [String] + return urls.compactMap { URL(string: $0) } + } + + override func collectionView(_ collectionView: UICollectionView, numberOfItemsInSection section: Int) -> Int { + imageURLs.count + } + + override func collectionView(_ collectionView: UICollectionView, cellForItemAt indexPath: IndexPath) -> UICollectionViewCell { + let cell: ImageCollectionViewCell = collectionView.dequeue(indexPath: indexPath) + let url = imageURLs[indexPath.item] + cell.imageView.setImage(with: url) + return cell + } + +} + +extension PrefetchViewController: UICollectionViewDataSourcePrefetching { + + func collectionView(_ collectionView: UICollectionView, prefetchItemsAt indexPaths: [IndexPath]) { + for indexPath in indexPaths { + let url = imageURLs[indexPath.item] + MapleBacon.shared.hydrateCache(url: url) + } + } + +} diff --git a/MapleBacon.xcodeproj/project.pbxproj b/MapleBacon.xcodeproj/project.pbxproj index ab4a1e9..a61e432 100644 --- a/MapleBacon.xcodeproj/project.pbxproj +++ b/MapleBacon.xcodeproj/project.pbxproj @@ -30,6 +30,7 @@ F27CBA1E2402D9A8006CD529 /* Downloader.swift in Sources */ = {isa = PBXBuildFile; fileRef = F27CBA1D2402D9A8006CD529 /* Downloader.swift */; }; F27CBA222402DC0B006CD529 /* DownloaderTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = F27CBA212402DC0B006CD529 /* DownloaderTests.swift */; }; F27CBA242402DC34006CD529 /* MockURLProtocol.swift in Sources */ = {isa = PBXBuildFile; fileRef = F27CBA232402DC34006CD529 /* MockURLProtocol.swift */; }; + F2D870E12416CB5200946A0B /* Atomic.swift in Sources */ = {isa = PBXBuildFile; fileRef = F2D870E02416CB5200946A0B /* Atomic.swift */; }; F2EEE583240BBC2E0002C9CA /* ImageTransforming.swift in Sources */ = {isa = PBXBuildFile; fileRef = F2EEE582240BBC2E0002C9CA /* ImageTransforming.swift */; }; F2EEE587240BBE040002C9CA /* ImageTransformingTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = F2EEE586240BBE040002C9CA /* ImageTransformingTests.swift */; }; F2EEE589240BBEF90002C9CA /* DownsamplingImageTransformer.swift in Sources */ = {isa = PBXBuildFile; fileRef = F2EEE588240BBEF90002C9CA /* DownsamplingImageTransformer.swift */; }; @@ -74,6 +75,7 @@ F27CBA1D2402D9A8006CD529 /* Downloader.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Downloader.swift; sourceTree = ""; }; F27CBA212402DC0B006CD529 /* DownloaderTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DownloaderTests.swift; sourceTree = ""; }; F27CBA232402DC34006CD529 /* MockURLProtocol.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MockURLProtocol.swift; sourceTree = ""; }; + F2D870E02416CB5200946A0B /* Atomic.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Atomic.swift; sourceTree = ""; }; F2EEE582240BBC2E0002C9CA /* ImageTransforming.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ImageTransforming.swift; sourceTree = ""; }; F2EEE586240BBE040002C9CA /* ImageTransformingTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ImageTransformingTests.swift; sourceTree = ""; }; F2EEE588240BBEF90002C9CA /* DownsamplingImageTransformer.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DownsamplingImageTransformer.swift; sourceTree = ""; }; @@ -161,6 +163,7 @@ F2255E212404606E00193742 /* MapleBacon.swift */, F2EEE582240BBC2E0002C9CA /* ImageTransforming.swift */, F2EEE58C240BC17F0002C9CA /* DisplayOptions.swift */, + F2D870E02416CB5200946A0B /* Atomic.swift */, ); path = Core; sourceTree = ""; @@ -299,6 +302,7 @@ F2EEE58D240BC17F0002C9CA /* DisplayOptions.swift in Sources */, F27CBA0E2402B1FE006CD529 /* Cache.swift in Sources */, F27CBA1E2402D9A8006CD529 /* Downloader.swift in Sources */, + F2D870E12416CB5200946A0B /* Atomic.swift in Sources */, F2EEE58B240BBFC00002C9CA /* CGSize+MapleBacon.swift in Sources */, F2EEE583240BBC2E0002C9CA /* ImageTransforming.swift in Sources */, 7778FD28240FAC6B007764C6 /* DispatchQueue+MapleBacon.swift in Sources */, diff --git a/MapleBacon/Core/Atomic.swift b/MapleBacon/Core/Atomic.swift new file mode 100644 index 0000000..d5626a0 --- /dev/null +++ b/MapleBacon/Core/Atomic.swift @@ -0,0 +1,31 @@ +// +// Copyright © 2020 Schnaub. All rights reserved. +// + +import Foundation + +final class Atomic { + + var value: Value { + get { + queue.sync { + self.wrappedValue + } + } + } + + private let queue = DispatchQueue(label: "com.schnaub.MapleBacon.atomic") + + private var wrappedValue: Value + + init(_ value: Value) { + self.wrappedValue = value + } + + func mutate(_ transform: (inout Value) -> Void) { + queue.sync { + transform(&self.wrappedValue) + } + } + +} diff --git a/MapleBacon/Core/MapleBacon.swift b/MapleBacon/Core/MapleBacon.swift index 5eec25e..0439b01 100644 --- a/MapleBacon/Core/MapleBacon.swift +++ b/MapleBacon/Core/MapleBacon.swift @@ -17,7 +17,6 @@ public final class MapleBacon { public static let shared = MapleBacon() private static let queueLabel = "com.schnaub.MapleBacon.transformer" - private static var token: CancelToken = 0 public var maxCacheAgeSeconds: TimeInterval { get { @@ -32,6 +31,8 @@ public final class MapleBacon { private let downloader: Downloader private let transformerQueue: DispatchQueue + private var token = Atomic(0) + public convenience init(name: String = "", sessionConfiguration: URLSessionConfiguration = .default) { self.init(cache: Cache(name: name), sessionConfiguration: sessionConfiguration) } @@ -44,7 +45,7 @@ public final class MapleBacon { @discardableResult public func image(with url: URL, imageTransformer: ImageTransforming? = nil, completion: @escaping ImageCompletion) -> CancelToken { - let token = Self.makeToken() + let token = makeToken() fetchImageFromCache(with: url, imageTransformer: imageTransformer) { result in switch result { @@ -59,17 +60,15 @@ public final class MapleBacon { return token } - public func hydrateCache(urls: [URL]) { - urls.forEach { url in - let cacheKey = makeCacheKey(for: url, imageTransformer: nil) - cache.value(forKey: cacheKey) { result in - switch result { - case .failure: - let token = Self.makeToken() - self.fetchImageFromNetworkAndCache(with: url, token: token, imageTransformer: nil, completion: { _ in }) - default: - break - } + public func hydrateCache(url: URL) { + let cacheKey = makeCacheKey(for: url, imageTransformer: nil) + cache.value(forKey: cacheKey) { result in + switch result { + case .failure: + let token = self.makeToken() + self.fetchImageFromNetworkAndCache(with: url, token: token, imageTransformer: nil, completion: { _ in }) + default: + break } } } @@ -86,9 +85,9 @@ public final class MapleBacon { private extension MapleBacon { - static func makeToken() -> CancelToken { - token += 1 - return token + func makeToken() -> CancelToken { + token.mutate { $0 += 1 } + return token.value } func fetchImageFromCache(with url: URL, imageTransformer: ImageTransforming?, completion: @escaping ImageCompletion) { From ce8bc5b4f851d1f6080eba09e1c2f056e4c1bce0 Mon Sep 17 00:00:00 2001 From: Jan Gorman Date: Mon, 9 Mar 2020 20:27:27 +0100 Subject: [PATCH 22/32] Eliminate data races --- .../Controllers/PrefetchViewController.swift | 6 ++-- MapleBacon/Core/Downloader.swift | 36 ++++++------------- MapleBacon/Core/MapleBacon.swift | 15 ++++++++ 3 files changed, 27 insertions(+), 30 deletions(-) diff --git a/Example/MapleBacon Example/Controllers/PrefetchViewController.swift b/Example/MapleBacon Example/Controllers/PrefetchViewController.swift index bd2e09f..955b63b 100644 --- a/Example/MapleBacon Example/Controllers/PrefetchViewController.swift +++ b/Example/MapleBacon Example/Controllers/PrefetchViewController.swift @@ -39,10 +39,8 @@ final class PrefetchViewController: UICollectionViewController { extension PrefetchViewController: UICollectionViewDataSourcePrefetching { func collectionView(_ collectionView: UICollectionView, prefetchItemsAt indexPaths: [IndexPath]) { - for indexPath in indexPaths { - let url = imageURLs[indexPath.item] - MapleBacon.shared.hydrateCache(url: url) - } + let urls = indexPaths.map { imageURLs[$0.item] } + MapleBacon.shared.hydrateCache(urls: urls) } } diff --git a/MapleBacon/Core/Downloader.swift b/MapleBacon/Core/Downloader.swift index 9471342..3eb19da 100644 --- a/MapleBacon/Core/Downloader.swift +++ b/MapleBacon/Core/Downloader.swift @@ -15,48 +15,31 @@ final class Downloader { private let sessionDelegate: SessionDelegate - private let lock = NSLock() - private let downloads = DoubleKeyedContainer>() + private let downloads = Atomic(DoubleKeyedContainer>()) fileprivate subscript(_ url: URL) -> Download? { get { - defer { - lock.unlock() - } - lock.lock() - return downloads[url] + downloads.value[url] } set { - defer { - lock.unlock() - } - lock.lock() guard let newValue = newValue else { - downloads.removeValue(forKey: url) + downloads.mutate{ $0.removeValue(forKey: url) } return } - downloads.update(newValue, forKey: url) + downloads.mutate { $0.update(newValue, forKey: url) } } } fileprivate subscript(_ token: CancelToken) -> Download? { get { - defer { - lock.unlock() - } - lock.lock() - return downloads[token] + downloads.value[token] } set { - defer { - lock.unlock() - } - lock.lock() guard let newValue = newValue else { - downloads.removeValue(forKey: token) + downloads.mutate { $0.removeValue(forKey: token) } return } - downloads.update(newValue, forKey: token) + downloads.mutate { $0.update(newValue, forKey: token) } } } @@ -79,7 +62,7 @@ final class Downloader { let newTask = session.dataTask(with: url) let download = Download(task: newTask, url: url, token: token, completion: completion) download.start() - downloads.insert(download, forKeys: (token, url)) + downloads.mutate { $0.insert(download, forKeys: (token, url)) } task = newTask } @@ -87,7 +70,7 @@ final class Downloader { } func cancel(token: CancelToken) { - guard let download = downloads[token] else { + guard let download = self[token] else { return } if download.completions.count == 1 { @@ -176,3 +159,4 @@ private final class SessionDelegate: NSObject, URLSessionDat } } + diff --git a/MapleBacon/Core/MapleBacon.swift b/MapleBacon/Core/MapleBacon.swift index 0439b01..67acbbc 100644 --- a/MapleBacon/Core/MapleBacon.swift +++ b/MapleBacon/Core/MapleBacon.swift @@ -73,6 +73,21 @@ public final class MapleBacon { } } + public func hydrateCache(urls: [URL]) { + for url in urls { + let cacheKey = makeCacheKey(for: url, imageTransformer: nil) + cache.value(forKey: cacheKey) { result in + switch result { + case .failure: + let token = self.makeToken() + self.fetchImageFromNetworkAndCache(with: url, token: token, imageTransformer: nil, completion: { _ in }) + default: + break + } + } + } + } + public func clearCache(_ options: CacheClearOptions, completion: ((Error?) -> Void)? = nil) { cache.clear(options, completion: completion) } From 6c5f2d787a7937a1eb27f3233f65930575d1c055 Mon Sep 17 00:00:00 2001 From: Jan Gorman Date: Tue, 10 Mar 2020 17:09:38 +0100 Subject: [PATCH 23/32] nil out associated objects on completion --- MapleBacon/Extensions/UIImageView+MapleBacon.swift | 2 ++ 1 file changed, 2 insertions(+) diff --git a/MapleBacon/Extensions/UIImageView+MapleBacon.swift b/MapleBacon/Extensions/UIImageView+MapleBacon.swift index 1a03483..1743982 100644 --- a/MapleBacon/Extensions/UIImageView+MapleBacon.swift +++ b/MapleBacon/Extensions/UIImageView+MapleBacon.swift @@ -43,6 +43,8 @@ extension UIImageView { cancelToken = MapleBacon.shared.image(with: url, imageTransformer: transformer) { [weak self] result in defer { + self?.baconImageUrl = nil + self?.cancelToken = nil completion?() } guard case let Result.success(image) = result, let self = self, url == self.baconImageUrl else { From 9982573a46a84fce6f036a72530623da345f86d1 Mon Sep 17 00:00:00 2001 From: Jan Gorman Date: Wed, 11 Mar 2020 10:27:12 +0100 Subject: [PATCH 24/32] Add check if file exists on file system --- MapleBacon.xcodeproj/project.pbxproj | 4 + MapleBacon/Core/DiskCache.swift | 15 +++- MapleBacon/Core/Download.swift | 44 +++++++++ MapleBacon/Core/Downloader.swift | 89 +++++-------------- MapleBacon/Core/MapleBacon.swift | 2 +- .../Extensions/UIImageView+MapleBacon.swift | 10 +++ MapleBaconTests/DiskCacheTests.swift | 14 +++ MapleBaconTests/DownloaderTests.swift | 40 ++++----- 8 files changed, 126 insertions(+), 92 deletions(-) create mode 100644 MapleBacon/Core/Download.swift diff --git a/MapleBacon.xcodeproj/project.pbxproj b/MapleBacon.xcodeproj/project.pbxproj index a61e432..9605655 100644 --- a/MapleBacon.xcodeproj/project.pbxproj +++ b/MapleBacon.xcodeproj/project.pbxproj @@ -7,6 +7,7 @@ objects = { /* Begin PBXBuildFile section */ + 775455712418DF4100FF8F5F /* Download.swift in Sources */ = {isa = PBXBuildFile; fileRef = 775455702418DF4100FF8F5F /* Download.swift */; }; 7778FD28240FAC6B007764C6 /* DispatchQueue+MapleBacon.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7778FD27240FAC6B007764C6 /* DispatchQueue+MapleBacon.swift */; }; 77E75F812412445E00611894 /* DoubleKeyedContainer.swift in Sources */ = {isa = PBXBuildFile; fileRef = 77E75F802412445E00611894 /* DoubleKeyedContainer.swift */; }; 77E75F8324124A4300611894 /* DoubleKeyedContainerTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 77E75F8224124A4300611894 /* DoubleKeyedContainerTests.swift */; }; @@ -49,6 +50,7 @@ /* End PBXContainerItemProxy section */ /* Begin PBXFileReference section */ + 775455702418DF4100FF8F5F /* Download.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Download.swift; sourceTree = ""; }; 7778FD27240FAC6B007764C6 /* DispatchQueue+MapleBacon.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "DispatchQueue+MapleBacon.swift"; sourceTree = ""; }; 77E75F802412445E00611894 /* DoubleKeyedContainer.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DoubleKeyedContainer.swift; sourceTree = ""; }; 77E75F8224124A4300611894 /* DoubleKeyedContainerTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DoubleKeyedContainerTests.swift; sourceTree = ""; }; @@ -159,6 +161,7 @@ F247A25D23FF191C0083DB03 /* DiskCache.swift */, 77E75F802412445E00611894 /* DoubleKeyedContainer.swift */, F27CBA1D2402D9A8006CD529 /* Downloader.swift */, + 775455702418DF4100FF8F5F /* Download.swift */, F247A25923FF15980083DB03 /* MemoryCache.swift */, F2255E212404606E00193742 /* MapleBacon.swift */, F2EEE582240BBC2E0002C9CA /* ImageTransforming.swift */, @@ -304,6 +307,7 @@ F27CBA1E2402D9A8006CD529 /* Downloader.swift in Sources */, F2D870E12416CB5200946A0B /* Atomic.swift in Sources */, F2EEE58B240BBFC00002C9CA /* CGSize+MapleBacon.swift in Sources */, + 775455712418DF4100FF8F5F /* Download.swift in Sources */, F2EEE583240BBC2E0002C9CA /* ImageTransforming.swift in Sources */, 7778FD28240FAC6B007764C6 /* DispatchQueue+MapleBacon.swift in Sources */, F27CBA0C2402B147006CD529 /* TimePeriod.swift in Sources */, diff --git a/MapleBacon/Core/DiskCache.swift b/MapleBacon/Core/DiskCache.swift index 28fc185..244715d 100644 --- a/MapleBacon/Core/DiskCache.swift +++ b/MapleBacon/Core/DiskCache.swift @@ -15,7 +15,7 @@ final class DiskCache { init(name: String) { let queueLabel = "\(Self.domain).\(name)" - self.diskQueue = DispatchQueue(label: queueLabel, qos: .background) + self.diskQueue = DispatchQueue(label: queueLabel) self.cacheName = "\(Self.domain).\(name)" } @@ -105,13 +105,22 @@ final class DiskCache { return expiredFileUrls } - private func store(data: Data, key: String) throws { + func isCached(forKey key: String) throws -> Bool { + let url = try self.cacheDirectory().appendingPathComponent(key) + return FileManager.default.fileExists(atPath: url.path) + } + +} + +private extension DiskCache { + + func store(data: Data, key: String) throws { let cacheDirectory = try self.cacheDirectory() let fileURL = cacheDirectory.appendingPathComponent(key) try data.write(to: fileURL) } - private func cacheDirectory() throws -> URL { + func cacheDirectory() throws -> URL { let fileManger = FileManager.default let folderURL = try FileManager.default.url(for: .cachesDirectory, in: .userDomainMask, appropriateFor: nil, create: true) diff --git a/MapleBacon/Core/Download.swift b/MapleBacon/Core/Download.swift new file mode 100644 index 0000000..b9e7985 --- /dev/null +++ b/MapleBacon/Core/Download.swift @@ -0,0 +1,44 @@ +// +// Copyright © 2020 Schnaub. All rights reserved. +// + +import UIKit + +final class Download { + + let task: URLSessionDataTask + let url: URL + let token: CancelToken + + var completions: [(Result) -> Void] + var data = Data() + + private var backgroundTask: UIBackgroundTaskIdentifier = .invalid + + init(task: URLSessionDataTask, url: URL, token: CancelToken, completion: @escaping (Result) -> Void) { + self.task = task + self.url = url + self.token = token + self.completions = [completion] + } + + deinit { + invalidateBackgroundTask() + } + + func start() { + backgroundTask = UIApplication.shared.beginBackgroundTask { + self.invalidateBackgroundTask() + } + } + + func finish() { + invalidateBackgroundTask() + } + + private func invalidateBackgroundTask() { + UIApplication.shared.endBackgroundTask(backgroundTask) + backgroundTask = .invalid + } + +} diff --git a/MapleBacon/Core/Downloader.swift b/MapleBacon/Core/Downloader.swift index 3eb19da..00b4558 100644 --- a/MapleBacon/Core/Downloader.swift +++ b/MapleBacon/Core/Downloader.swift @@ -14,32 +14,24 @@ final class Downloader { let session: URLSession private let sessionDelegate: SessionDelegate + private let lock = NSLock() - private let downloads = Atomic(DoubleKeyedContainer>()) + private var downloads: [URL: Download] = [:] fileprivate subscript(_ url: URL) -> Download? { get { - downloads.value[url] - } - set { - guard let newValue = newValue else { - downloads.mutate{ $0.removeValue(forKey: url) } - return + defer { + lock.unlock() } - downloads.mutate { $0.update(newValue, forKey: url) } - } - } - - fileprivate subscript(_ token: CancelToken) -> Download? { - get { - downloads.value[token] + lock.lock() + return downloads[url] } set { - guard let newValue = newValue else { - downloads.mutate { $0.removeValue(forKey: token) } - return + defer { + lock.unlock() } - downloads.mutate { $0.update(newValue, forKey: token) } + lock.lock() + downloads[url] = newValue } } @@ -62,62 +54,23 @@ final class Downloader { let newTask = session.dataTask(with: url) let download = Download(task: newTask, url: url, token: token, completion: completion) download.start() - downloads.mutate { $0.insert(download, forKeys: (token, url)) } + self[url] = download task = newTask } task.resume() } - func cancel(token: CancelToken) { - guard let download = self[token] else { - return - } - if download.completions.count == 1 { - download.task.cancel() - download.completions.first?(.failure(DownloaderError.canceled)) - self[token] = nil - } - } - -} - -private final class Download { - - let task: URLSessionDataTask - let url: URL - let token: CancelToken - - var completions: [(Result) -> Void] - var data = Data() - - private var backgroundTask: UIBackgroundTaskIdentifier = .invalid - - init(task: URLSessionDataTask, url: URL, token: CancelToken, completion: @escaping (Result) -> Void) { - self.task = task - self.url = url - self.token = token - self.completions = [completion] - } - - deinit { - invalidateBackgroundTask() - } - - func start() { - backgroundTask = UIApplication.shared.beginBackgroundTask { - self.invalidateBackgroundTask() - } - } - - func finish() { - invalidateBackgroundTask() - } - - private func invalidateBackgroundTask() { - UIApplication.shared.endBackgroundTask(backgroundTask) - backgroundTask = .invalid - } +// func cancel(token: CancelToken) { +// guard let download = self[token] else { +// return +// } +// if download.completions.count == 1 { +// download.task.cancel() +// download.completions.first?(.failure(DownloaderError.canceled)) +// self[token] = nil +// } +// } } diff --git a/MapleBacon/Core/MapleBacon.swift b/MapleBacon/Core/MapleBacon.swift index 67acbbc..481b1d5 100644 --- a/MapleBacon/Core/MapleBacon.swift +++ b/MapleBacon/Core/MapleBacon.swift @@ -93,7 +93,7 @@ public final class MapleBacon { } public func cancel(token: CancelToken) { - downloader.cancel(token: token) +// downloader.cancel(token: token) } } diff --git a/MapleBacon/Extensions/UIImageView+MapleBacon.swift b/MapleBacon/Extensions/UIImageView+MapleBacon.swift index 1743982..ff8008f 100644 --- a/MapleBacon/Extensions/UIImageView+MapleBacon.swift +++ b/MapleBacon/Extensions/UIImageView+MapleBacon.swift @@ -6,6 +6,7 @@ import UIKit private var baconImageUrlKey: UInt8 = 0 private var cancelTokenKey: UInt8 = 1 +private var downloadKey: UInt8 = 2 extension UIImageView { @@ -27,6 +28,15 @@ extension UIImageView { } } + private var download: Download? { + get { + objc_getAssociatedObject(self, &downloadKey) as? Download + } + set { + objc_setAssociatedObject(self, &downloadKey, newValue, .OBJC_ASSOCIATION_RETAIN_NONATOMIC) + } + } + public func setImage(with url: URL?, placeholder: UIImage? = nil, displayOptions: [DisplayOptions] = [], diff --git a/MapleBaconTests/DiskCacheTests.swift b/MapleBaconTests/DiskCacheTests.swift index 292acff..13313ac 100644 --- a/MapleBaconTests/DiskCacheTests.swift +++ b/MapleBaconTests/DiskCacheTests.swift @@ -101,4 +101,18 @@ final class DiskCacheTests: XCTestCase { waitForExpectations(timeout: 5, handler: nil) } + func testExists() { + let expectation = self.expectation(description: #function) + let cache = DiskCache(name: Self.cacheName) + + cache.insert(dummyData(), forKey: "test") { _ in + XCTAssertTrue(try! cache.isCached(forKey: "test")) + cache.clear { _ in + expectation.fulfill() + } + } + + waitForExpectations(timeout: 5, handler: nil) + } + } diff --git a/MapleBaconTests/DownloaderTests.swift b/MapleBaconTests/DownloaderTests.swift index 85eb038..3b831a1 100644 --- a/MapleBaconTests/DownloaderTests.swift +++ b/MapleBaconTests/DownloaderTests.swift @@ -103,25 +103,25 @@ final class DownloaderTests: XCTestCase { waitForExpectations(timeout: 5, handler: nil) } - func testCancel() { - let expectation = self.expectation(description: #function) - let configuration = MockURLProtocol.mockedURLSessionConfiguration() - let downloader = Downloader(sessionConfiguration: configuration) - - setupMockResponse(.error) - - downloader.fetch(Self.url, token: Self.cancelToken) { response in - switch response { - case .failure(let error as DownloaderError): - XCTAssertEqual(error, .canceled) - case .success, .failure: - XCTFail() - } - expectation.fulfill() - } - downloader.cancel(token: Self.cancelToken) - - waitForExpectations(timeout: 5, handler: nil) - } +// func testCancel() { +// let expectation = self.expectation(description: #function) +// let configuration = MockURLProtocol.mockedURLSessionConfiguration() +// let downloader = Downloader(sessionConfiguration: configuration) +// +// setupMockResponse(.error) +// +// downloader.fetch(Self.url, token: Self.cancelToken) { response in +// switch response { +// case .failure(let error as DownloaderError): +// XCTAssertEqual(error, .canceled) +// case .success, .failure: +// XCTFail() +// } +// expectation.fulfill() +// } +// downloader.cancel(token: Self.cancelToken) +// +// waitForExpectations(timeout: 5, handler: nil) +// } } From 083b5b7caabff899e13777b5bc5230198cf0c227 Mon Sep 17 00:00:00 2001 From: Jan Gorman Date: Wed, 11 Mar 2020 10:45:31 +0100 Subject: [PATCH 25/32] Use an InputStream --- MapleBacon.xcodeproj/project.pbxproj | 4 ++++ MapleBacon/Extensions/Data+MapleBacon.swift | 26 +++++++++++++++++++++ MapleBacon/Extensions/FileManager.swift | 14 ++++++++++- MapleBaconTests/CacheTests.swift | 4 ++-- 4 files changed, 45 insertions(+), 3 deletions(-) create mode 100644 MapleBacon/Extensions/Data+MapleBacon.swift diff --git a/MapleBacon.xcodeproj/project.pbxproj b/MapleBacon.xcodeproj/project.pbxproj index 9605655..d8fb503 100644 --- a/MapleBacon.xcodeproj/project.pbxproj +++ b/MapleBacon.xcodeproj/project.pbxproj @@ -8,6 +8,7 @@ /* Begin PBXBuildFile section */ 775455712418DF4100FF8F5F /* Download.swift in Sources */ = {isa = PBXBuildFile; fileRef = 775455702418DF4100FF8F5F /* Download.swift */; }; + 775455732418E70B00FF8F5F /* Data+MapleBacon.swift in Sources */ = {isa = PBXBuildFile; fileRef = 775455722418E70B00FF8F5F /* Data+MapleBacon.swift */; }; 7778FD28240FAC6B007764C6 /* DispatchQueue+MapleBacon.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7778FD27240FAC6B007764C6 /* DispatchQueue+MapleBacon.swift */; }; 77E75F812412445E00611894 /* DoubleKeyedContainer.swift in Sources */ = {isa = PBXBuildFile; fileRef = 77E75F802412445E00611894 /* DoubleKeyedContainer.swift */; }; 77E75F8324124A4300611894 /* DoubleKeyedContainerTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 77E75F8224124A4300611894 /* DoubleKeyedContainerTests.swift */; }; @@ -51,6 +52,7 @@ /* Begin PBXFileReference section */ 775455702418DF4100FF8F5F /* Download.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Download.swift; sourceTree = ""; }; + 775455722418E70B00FF8F5F /* Data+MapleBacon.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "Data+MapleBacon.swift"; sourceTree = ""; }; 7778FD27240FAC6B007764C6 /* DispatchQueue+MapleBacon.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "DispatchQueue+MapleBacon.swift"; sourceTree = ""; }; 77E75F802412445E00611894 /* DoubleKeyedContainer.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DoubleKeyedContainer.swift; sourceTree = ""; }; 77E75F8224124A4300611894 /* DoubleKeyedContainerTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DoubleKeyedContainerTests.swift; sourceTree = ""; }; @@ -181,6 +183,7 @@ F2EEE588240BBEF90002C9CA /* DownsamplingImageTransformer.swift */, F2EEE58A240BBFC00002C9CA /* CGSize+MapleBacon.swift */, 7778FD27240FAC6B007764C6 /* DispatchQueue+MapleBacon.swift */, + 775455722418E70B00FF8F5F /* Data+MapleBacon.swift */, ); path = Extensions; sourceTree = ""; @@ -306,6 +309,7 @@ F27CBA0E2402B1FE006CD529 /* Cache.swift in Sources */, F27CBA1E2402D9A8006CD529 /* Downloader.swift in Sources */, F2D870E12416CB5200946A0B /* Atomic.swift in Sources */, + 775455732418E70B00FF8F5F /* Data+MapleBacon.swift in Sources */, F2EEE58B240BBFC00002C9CA /* CGSize+MapleBacon.swift in Sources */, 775455712418DF4100FF8F5F /* Download.swift in Sources */, F2EEE583240BBC2E0002C9CA /* ImageTransforming.swift in Sources */, diff --git a/MapleBacon/Extensions/Data+MapleBacon.swift b/MapleBacon/Extensions/Data+MapleBacon.swift new file mode 100644 index 0000000..5a92fd9 --- /dev/null +++ b/MapleBacon/Extensions/Data+MapleBacon.swift @@ -0,0 +1,26 @@ +// +// Copyright © 2020 Schnaub. All rights reserved. +// + +import Foundation + +extension Data { + + init(from inputStream: InputStream) { + self.init() + + defer { + inputStream.close() + } + inputStream.open() + + let bufferSize = 1024 + let buffer = UnsafeMutablePointer.allocate(capacity: bufferSize) + while inputStream.hasBytesAvailable { + let read = inputStream.read(buffer, maxLength: bufferSize) + append(buffer, count: read) + } + buffer.deallocate() + } + +} diff --git a/MapleBacon/Extensions/FileManager.swift b/MapleBacon/Extensions/FileManager.swift index 5ef60b3..c69a08b 100644 --- a/MapleBacon/Extensions/FileManager.swift +++ b/MapleBacon/Extensions/FileManager.swift @@ -4,8 +4,20 @@ import Foundation +enum MapleBaconInputStreamError: Error { + case uninitializedInputStream + case emptyFile +} + extension FileManager { func fileContents(at url: URL) throws -> Data { - try Data(contentsOf: url, options: .mappedIfSafe) + guard let inputStream = InputStream(url: url) else { + throw MapleBaconInputStreamError.uninitializedInputStream + } + let data = Data(from: inputStream) + guard data.count > 0 else { + throw MapleBaconInputStreamError.emptyFile + } + return data } } diff --git a/MapleBaconTests/CacheTests.swift b/MapleBaconTests/CacheTests.swift index 9f8d0bc..5435e8a 100644 --- a/MapleBaconTests/CacheTests.swift +++ b/MapleBaconTests/CacheTests.swift @@ -54,10 +54,10 @@ final class CacheTests: XCTestCase { let data = dummyData() - cache.store(value: data, forKey: #function) { _ in + cache.store(value: data, forKey: "test") { _ in self.cache.clear(.all) - self.cache.value(forKey: #function) { result in + self.cache.value(forKey: "test") { result in switch result { case .success: XCTFail() From 3a813e9e9b047e1a739956355cbd54f40a9df619 Mon Sep 17 00:00:00 2001 From: Jan Gorman Date: Wed, 11 Mar 2020 11:50:38 +0100 Subject: [PATCH 26/32] Split into extension --- MapleBacon/Core/Cache.swift | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/MapleBacon/Core/Cache.swift b/MapleBacon/Core/Cache.swift index f5e400f..0ec8da2 100644 --- a/MapleBacon/Core/Cache.swift +++ b/MapleBacon/Core/Cache.swift @@ -76,14 +76,22 @@ final class Cache where T.Result == T { } } - private func convertToTargetType(_ data: Data, type: CacheType) -> Result, Error> { + @objc private func cleanDiskOnNotification() { + clear(.disk) + } + +} + +private extension Cache { + + func convertToTargetType(_ data: Data, type: CacheType) -> Result, Error> { guard let targetType = T.convert(from: data) else { return .failure(CacheError.dataConversion) } return .success(.init(value: targetType, type: type)) } - private func safeCacheKey(_ key: String) -> String { + func safeCacheKey(_ key: String) -> String { #if canImport(CryptoKit) if #available(iOS 13.0, *) { return cryptoSafeCacheKey(key) @@ -92,10 +100,6 @@ final class Cache where T.Result == T { return key.components(separatedBy: CharacterSet(charactersIn: "()/")).joined(separator: "-") } - @objc private func cleanDiskOnNotification() { - clear(.disk) - } - } #if canImport(CryptoKit) From f24e86642755c7fee95e4c1a921b06f5e23a5418 Mon Sep 17 00:00:00 2001 From: Jan Gorman Date: Wed, 11 Mar 2020 14:47:05 +0100 Subject: [PATCH 27/32] Check for all cache layers --- MapleBacon.xcodeproj/project.pbxproj | 18 +++++++++---- MapleBacon/Core/{ => Cache}/Cache.swift | 8 ++++++ .../Core/{ => Cache}/CacheClearOptions.swift | 0 MapleBacon/Core/{ => Cache}/CacheResult.swift | 0 MapleBacon/Core/{ => Cache}/DiskCache.swift | 0 MapleBacon/Core/{ => Cache}/MemoryCache.swift | 14 ++++++++--- MapleBacon/Core/Download.swift | 2 +- MapleBacon/Core/MapleBacon.swift | 25 ++++++++++--------- MapleBacon/Extensions/DataConvertible.swift | 10 ++++---- .../Extensions/UIImageView+MapleBacon.swift | 2 +- MapleBaconTests/CacheTests.swift | 20 +++++++++++++++ MapleBaconTests/DiskCacheTests.swift | 2 +- MapleBaconTests/MapleBaconTests.swift | 2 +- MapleBaconTests/MemoryCacheTests.swift | 8 ++++++ 14 files changed, 81 insertions(+), 30 deletions(-) rename MapleBacon/Core/{ => Cache}/Cache.swift (93%) rename MapleBacon/Core/{ => Cache}/CacheClearOptions.swift (100%) rename MapleBacon/Core/{ => Cache}/CacheResult.swift (100%) rename MapleBacon/Core/{ => Cache}/DiskCache.swift (100%) rename MapleBacon/Core/{ => Cache}/MemoryCache.swift (84%) diff --git a/MapleBacon.xcodeproj/project.pbxproj b/MapleBacon.xcodeproj/project.pbxproj index d8fb503..d636dbe 100644 --- a/MapleBacon.xcodeproj/project.pbxproj +++ b/MapleBacon.xcodeproj/project.pbxproj @@ -106,6 +106,18 @@ /* End PBXFrameworksBuildPhase section */ /* Begin PBXGroup section */ + 775455742419208B00FF8F5F /* Cache */ = { + isa = PBXGroup; + children = ( + F27CBA0D2402B1FE006CD529 /* Cache.swift */, + F27CBA192402CC13006CD529 /* CacheClearOptions.swift */, + F27CBA1B2402CC28006CD529 /* CacheResult.swift */, + F247A25D23FF191C0083DB03 /* DiskCache.swift */, + F247A25923FF15980083DB03 /* MemoryCache.swift */, + ); + path = Cache; + sourceTree = ""; + }; F247A23423FF15410083DB03 = { isa = PBXGroup; children = ( @@ -157,14 +169,10 @@ F247A25823FF158E0083DB03 /* Core */ = { isa = PBXGroup; children = ( - F27CBA0D2402B1FE006CD529 /* Cache.swift */, - F27CBA192402CC13006CD529 /* CacheClearOptions.swift */, - F27CBA1B2402CC28006CD529 /* CacheResult.swift */, - F247A25D23FF191C0083DB03 /* DiskCache.swift */, + 775455742419208B00FF8F5F /* Cache */, 77E75F802412445E00611894 /* DoubleKeyedContainer.swift */, F27CBA1D2402D9A8006CD529 /* Downloader.swift */, 775455702418DF4100FF8F5F /* Download.swift */, - F247A25923FF15980083DB03 /* MemoryCache.swift */, F2255E212404606E00193742 /* MapleBacon.swift */, F2EEE582240BBC2E0002C9CA /* ImageTransforming.swift */, F2EEE58C240BC17F0002C9CA /* DisplayOptions.swift */, diff --git a/MapleBacon/Core/Cache.swift b/MapleBacon/Core/Cache/Cache.swift similarity index 93% rename from MapleBacon/Core/Cache.swift rename to MapleBacon/Core/Cache/Cache.swift index 0ec8da2..40f7e30 100644 --- a/MapleBacon/Core/Cache.swift +++ b/MapleBacon/Core/Cache/Cache.swift @@ -76,6 +76,14 @@ final class Cache where T.Result == T { } } + func isCached(forKey key: String) throws -> Bool { + let safeKey = safeCacheKey(key) + if memoryCache.isCached(forKey: safeKey) { + return true + } + return try diskCache.isCached(forKey: safeKey) + } + @objc private func cleanDiskOnNotification() { clear(.disk) } diff --git a/MapleBacon/Core/CacheClearOptions.swift b/MapleBacon/Core/Cache/CacheClearOptions.swift similarity index 100% rename from MapleBacon/Core/CacheClearOptions.swift rename to MapleBacon/Core/Cache/CacheClearOptions.swift diff --git a/MapleBacon/Core/CacheResult.swift b/MapleBacon/Core/Cache/CacheResult.swift similarity index 100% rename from MapleBacon/Core/CacheResult.swift rename to MapleBacon/Core/Cache/CacheResult.swift diff --git a/MapleBacon/Core/DiskCache.swift b/MapleBacon/Core/Cache/DiskCache.swift similarity index 100% rename from MapleBacon/Core/DiskCache.swift rename to MapleBacon/Core/Cache/DiskCache.swift diff --git a/MapleBacon/Core/MemoryCache.swift b/MapleBacon/Core/Cache/MemoryCache.swift similarity index 84% rename from MapleBacon/Core/MemoryCache.swift rename to MapleBacon/Core/Cache/MemoryCache.swift index adc6d6a..f5d4353 100644 --- a/MapleBacon/Core/MemoryCache.swift +++ b/MapleBacon/Core/Cache/MemoryCache.swift @@ -25,23 +25,29 @@ final class MemoryCache { backingCache.name = name } + func isCached(forKey key: Key) -> Bool { + self[key] != nil + } + func clear() { backingCache.removeAllObjects() } - private func insert(_ value: Value, forKey key: Key) { +} + +private extension MemoryCache { + func insert(_ value: Value, forKey key: Key) { backingCache.setObject(Entry(value: value), forKey: WrappedKey(key: key)) } - private func value(forKey key: Key) -> Value? { + func value(forKey key: Key) -> Value? { let entry = backingCache.object(forKey: WrappedKey(key: key)) return entry?.value } - private func removeValue(forKey key: Key) { + func removeValue(forKey key: Key) { backingCache.removeObject(forKey: WrappedKey(key: key)) } - } extension MemoryCache { diff --git a/MapleBacon/Core/Download.swift b/MapleBacon/Core/Download.swift index b9e7985..313f438 100644 --- a/MapleBacon/Core/Download.swift +++ b/MapleBacon/Core/Download.swift @@ -4,7 +4,7 @@ import UIKit -final class Download { +public final class Download { let task: URLSessionDataTask let url: URL diff --git a/MapleBacon/Core/MapleBacon.swift b/MapleBacon/Core/MapleBacon.swift index 481b1d5..e728724 100644 --- a/MapleBacon/Core/MapleBacon.swift +++ b/MapleBacon/Core/MapleBacon.swift @@ -44,20 +44,21 @@ public final class MapleBacon { } @discardableResult - public func image(with url: URL, imageTransformer: ImageTransforming? = nil, completion: @escaping ImageCompletion) -> CancelToken { + public func image(with url: URL, imageTransformer: ImageTransforming? = nil, completion: @escaping ImageCompletion) -> Download? { let token = makeToken() - fetchImageFromCache(with: url, imageTransformer: imageTransformer) { result in - switch result { - case .success(let image): - DispatchQueue.main.optionalAsync { - completion(.success(image)) - } - case .failure: - self.fetchImageFromNetworkAndCache(with: url, token: token, imageTransformer: imageTransformer, completion: completion) - } - } - return token +// fetchImageFromCache(with: url, imageTransformer: imageTransformer) { result in +// switch result { +// case .success(let image): +// DispatchQueue.main.optionalAsync { +// completion(.success(image)) +// } +// case .failure: +// self.fetchImageFromNetworkAndCache(with: url, token: token, imageTransformer: imageTransformer, completion: completion) +// } +// } +// return token + return nil } public func hydrateCache(url: URL) { diff --git a/MapleBacon/Extensions/DataConvertible.swift b/MapleBacon/Extensions/DataConvertible.swift index 76cb812..67825c3 100644 --- a/MapleBacon/Extensions/DataConvertible.swift +++ b/MapleBacon/Extensions/DataConvertible.swift @@ -4,7 +4,7 @@ import UIKit -protocol DataConvertible { +public protocol DataConvertible { associatedtype Result static func convert(from data: Data) -> Result? @@ -13,11 +13,11 @@ protocol DataConvertible { } extension Data: DataConvertible { - static func convert(from data: Data) -> Data? { + public static func convert(from data: Data) -> Data? { data } - func toData() -> Data { + public func toData() -> Data { self } } @@ -38,11 +38,11 @@ extension UIImage: DataConvertible { } } - static func convert(from data: Data) -> UIImage? { + public static func convert(from data: Data) -> UIImage? { UIImage(data: data, scale: UIScreen.main.scale) } - func toData() -> Data { + public func toData() -> Data { hasAlphaChannel ? pngData()! : jpegData(compressionQuality: 1)! } } diff --git a/MapleBacon/Extensions/UIImageView+MapleBacon.swift b/MapleBacon/Extensions/UIImageView+MapleBacon.swift index ff8008f..c1cd7b0 100644 --- a/MapleBacon/Extensions/UIImageView+MapleBacon.swift +++ b/MapleBacon/Extensions/UIImageView+MapleBacon.swift @@ -51,7 +51,7 @@ extension UIImageView { let transformer = makeTransformer(displayOptions: displayOptions, imageTransformer: imageTransformer) - cancelToken = MapleBacon.shared.image(with: url, imageTransformer: transformer) { [weak self] result in + _ = MapleBacon.shared.image(with: url, imageTransformer: transformer) { [weak self] result in defer { self?.baconImageUrl = nil self?.cancelToken = nil diff --git a/MapleBaconTests/CacheTests.swift b/MapleBaconTests/CacheTests.swift index 5435e8a..545926a 100644 --- a/MapleBaconTests/CacheTests.swift +++ b/MapleBaconTests/CacheTests.swift @@ -127,4 +127,24 @@ final class CacheTests: XCTestCase { waitForExpectations(timeout: 5, handler: nil) } + func testIsCached() { + let expectation = self.expectation(description: #function) + + let data = dummyData() + + cache.store(value: data, forKey: #function) { _ in + XCTAssertTrue(try! self.cache.isCached(forKey: #function)) + + self.cache.clear(.memory) { _ in + XCTAssertTrue(try! self.cache.isCached(forKey: #function)) + + self.cache.clear(.all) { _ in + expectation.fulfill() + } + } + } + + waitForExpectations(timeout: 5, handler: nil) + } + } diff --git a/MapleBaconTests/DiskCacheTests.swift b/MapleBaconTests/DiskCacheTests.swift index 13313ac..84ddbde 100644 --- a/MapleBaconTests/DiskCacheTests.swift +++ b/MapleBaconTests/DiskCacheTests.swift @@ -101,7 +101,7 @@ final class DiskCacheTests: XCTestCase { waitForExpectations(timeout: 5, handler: nil) } - func testExists() { + func testIsCached() { let expectation = self.expectation(description: #function) let cache = DiskCache(name: Self.cacheName) diff --git a/MapleBaconTests/MapleBaconTests.swift b/MapleBaconTests/MapleBaconTests.swift index bb53ac7..e465e28 100644 --- a/MapleBaconTests/MapleBaconTests.swift +++ b/MapleBaconTests/MapleBaconTests.swift @@ -104,7 +104,7 @@ final class MapleBaconTests: XCTestCase { expectation.fulfill() } } - mapleBacon.cancel(token: token) +// mapleBacon.cancel(token: token) waitForExpectations(timeout: 5, handler: nil) } diff --git a/MapleBaconTests/MemoryCacheTests.swift b/MapleBaconTests/MemoryCacheTests.swift index e820e47..9a79b65 100644 --- a/MapleBaconTests/MemoryCacheTests.swift +++ b/MapleBaconTests/MemoryCacheTests.swift @@ -46,4 +46,12 @@ final class MemoryCacheTests: XCTestCase { XCTAssertNil(cache["foo"]) } + func testIsCached() { + let cache = MemoryCache() + cache["foo"] = "bar" + + XCTAssertTrue(cache.isCached(forKey: "foo")) + XCTAssertFalse(cache.isCached(forKey: "bar")) + } + } From bf0f09d03e5e66428cc7eae8a955cd8af0f7cd38 Mon Sep 17 00:00:00 2001 From: Jan Gorman Date: Wed, 11 Mar 2020 15:13:54 +0100 Subject: [PATCH 28/32] return the token --- MapleBacon/Core/Downloader.swift | 20 +++++---- MapleBacon/Core/MapleBacon.swift | 60 +++++++++++---------------- MapleBaconTests/DownloaderTests.swift | 10 ++--- 3 files changed, 43 insertions(+), 47 deletions(-) diff --git a/MapleBacon/Core/Downloader.swift b/MapleBacon/Core/Downloader.swift index 00b4558..ef125a2 100644 --- a/MapleBacon/Core/Downloader.swift +++ b/MapleBacon/Core/Downloader.swift @@ -45,20 +45,26 @@ final class Downloader { session.invalidateAndCancel() } - func fetch(_ url: URL, token: CancelToken, completion: @escaping (Result) -> Void) { + func fetch(_ url: URL, token: CancelToken, completion: @escaping (Result) -> Void) -> Download { let task: URLSessionDataTask - if let download = self[url] { - task = download.task - download.completions.append(completion) + let download: Download + + if let existingDownload = self[url] { + task = existingDownload.task + existingDownload.completions.append(completion) + download = existingDownload } else { let newTask = session.dataTask(with: url) - let download = Download(task: newTask, url: url, token: token, completion: completion) - download.start() - self[url] = download + let newDownload = Download(task: newTask, url: url, token: token, completion: completion) + newDownload.start() + self[url] = newDownload task = newTask + download = newDownload } task.resume() + + return download } // func cancel(token: CancelToken) { diff --git a/MapleBacon/Core/MapleBacon.swift b/MapleBacon/Core/MapleBacon.swift index e728724..db33ba7 100644 --- a/MapleBacon/Core/MapleBacon.swift +++ b/MapleBacon/Core/MapleBacon.swift @@ -45,46 +45,27 @@ public final class MapleBacon { @discardableResult public func image(with url: URL, imageTransformer: ImageTransforming? = nil, completion: @escaping ImageCompletion) -> Download? { - let token = makeToken() + if (try? isCached(with: url, imageTransformer: imageTransformer)) == true { + fetchImageFromCache(with: url, imageTransformer: imageTransformer, completion: completion) + return nil + } -// fetchImageFromCache(with: url, imageTransformer: imageTransformer) { result in -// switch result { -// case .success(let image): -// DispatchQueue.main.optionalAsync { -// completion(.success(image)) -// } -// case .failure: -// self.fetchImageFromNetworkAndCache(with: url, token: token, imageTransformer: imageTransformer, completion: completion) -// } -// } -// return token - return nil + let token = makeToken() + return fetchImageFromNetworkAndCache(with: url, token: token, imageTransformer: imageTransformer, completion: completion) } public func hydrateCache(url: URL) { - let cacheKey = makeCacheKey(for: url, imageTransformer: nil) - cache.value(forKey: cacheKey) { result in - switch result { - case .failure: - let token = self.makeToken() - self.fetchImageFromNetworkAndCache(with: url, token: token, imageTransformer: nil, completion: { _ in }) - default: - break - } + if (try? isCached(with: url, imageTransformer: nil)) == false { + let token = self.makeToken() + _ = self.fetchImageFromNetworkAndCache(with: url, token: token, imageTransformer: nil, completion: { _ in }) } } public func hydrateCache(urls: [URL]) { for url in urls { - let cacheKey = makeCacheKey(for: url, imageTransformer: nil) - cache.value(forKey: cacheKey) { result in - switch result { - case .failure: - let token = self.makeToken() - self.fetchImageFromNetworkAndCache(with: url, token: token, imageTransformer: nil, completion: { _ in }) - default: - break - } + if (try? isCached(with: url, imageTransformer: nil)) == false { + let token = self.makeToken() + _ = self.fetchImageFromNetworkAndCache(with: url, token: token, imageTransformer: nil, completion: { _ in }) } } } @@ -106,19 +87,28 @@ private extension MapleBacon { return token.value } + func isCached(with url: URL, imageTransformer: ImageTransforming?) throws -> Bool { + let cacheKey = makeCacheKey(for: url, imageTransformer: imageTransformer) + return try cache.isCached(forKey: cacheKey) + } + func fetchImageFromCache(with url: URL, imageTransformer: ImageTransforming?, completion: @escaping ImageCompletion) { let cacheKey = makeCacheKey(for: url, imageTransformer: imageTransformer) cache.value(forKey: cacheKey) { result in switch result { case .success(let cacheResult): - completion(.success(cacheResult.value)) + DispatchQueue.main.optionalAsync { + completion(.success(cacheResult.value)) + } case .failure(let error): - completion(.failure(error)) + DispatchQueue.main.optionalAsync { + completion(.failure(error)) + } } } } - func fetchImageFromNetworkAndCache(with url: URL, token: CancelToken, imageTransformer: ImageTransforming?, completion: @escaping ImageCompletion) { + func fetchImageFromNetworkAndCache(with url: URL, token: CancelToken, imageTransformer: ImageTransforming?, completion: @escaping ImageCompletion) -> Download { fetchImageFromNetwork(with: url, token: token) { result in switch result { case .success(let image): @@ -139,7 +129,7 @@ private extension MapleBacon { } } - func fetchImageFromNetwork(with url: URL, token: CancelToken, completion: @escaping ImageCompletion) { + func fetchImageFromNetwork(with url: URL, token: CancelToken, completion: @escaping ImageCompletion) -> Download { downloader.fetch(url, token: token, completion: completion) } diff --git a/MapleBaconTests/DownloaderTests.swift b/MapleBaconTests/DownloaderTests.swift index 3b831a1..99c8fa6 100644 --- a/MapleBaconTests/DownloaderTests.swift +++ b/MapleBaconTests/DownloaderTests.swift @@ -17,7 +17,7 @@ final class DownloaderTests: XCTestCase { setupMockResponse(.data(dummyData())) - downloader.fetch(Self.url, token: Self.cancelToken) { response in + _ = downloader.fetch(Self.url, token: Self.cancelToken) { response in switch response { case .success(let data): XCTAssertNotNil(data) @@ -38,7 +38,7 @@ final class DownloaderTests: XCTestCase { setupMockResponse(.data(dummyData())) - downloader.fetch(Self.url, token: Self.cancelToken) { response in + _ = downloader.fetch(Self.url, token: Self.cancelToken) { response in switch response { case .success: XCTFail() @@ -59,7 +59,7 @@ final class DownloaderTests: XCTestCase { setupMockResponse(.error) - downloader.fetch(Self.url, token: Self.cancelToken) { response in + _ = downloader.fetch(Self.url, token: Self.cancelToken) { response in switch response { case .success: XCTFail() @@ -79,7 +79,7 @@ final class DownloaderTests: XCTestCase { setupMockResponse(.data(dummyData())) let firstExpectation = expectation(description: "first") - downloader.fetch(Self.url, token: Self.cancelToken) { response in + _ = downloader.fetch(Self.url, token: Self.cancelToken) { response in switch response { case .success(let data): XCTAssertNotNil(data) @@ -90,7 +90,7 @@ final class DownloaderTests: XCTestCase { } let secondExpectation = expectation(description: "second") - downloader.fetch(Self.url, token: Self.cancelToken) { response in + _ = downloader.fetch(Self.url, token: Self.cancelToken) { response in switch response { case .success(let data): XCTAssertNotNil(data) From ce89c1ccf0bd2c8e98741f9fd810ce4a59d88264 Mon Sep 17 00:00:00 2001 From: Jan Gorman Date: Wed, 11 Mar 2020 16:05:15 +0100 Subject: [PATCH 29/32] Remove container data structure --- MapleBacon.xcodeproj/project.pbxproj | 8 --- MapleBacon/Core/DoubleKeyedContainer.swift | 65 ----------------- .../DoubleKeyedContainerTests.swift | 71 ------------------- 3 files changed, 144 deletions(-) delete mode 100644 MapleBacon/Core/DoubleKeyedContainer.swift delete mode 100644 MapleBaconTests/DoubleKeyedContainerTests.swift diff --git a/MapleBacon.xcodeproj/project.pbxproj b/MapleBacon.xcodeproj/project.pbxproj index d636dbe..0a6c8f0 100644 --- a/MapleBacon.xcodeproj/project.pbxproj +++ b/MapleBacon.xcodeproj/project.pbxproj @@ -10,8 +10,6 @@ 775455712418DF4100FF8F5F /* Download.swift in Sources */ = {isa = PBXBuildFile; fileRef = 775455702418DF4100FF8F5F /* Download.swift */; }; 775455732418E70B00FF8F5F /* Data+MapleBacon.swift in Sources */ = {isa = PBXBuildFile; fileRef = 775455722418E70B00FF8F5F /* Data+MapleBacon.swift */; }; 7778FD28240FAC6B007764C6 /* DispatchQueue+MapleBacon.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7778FD27240FAC6B007764C6 /* DispatchQueue+MapleBacon.swift */; }; - 77E75F812412445E00611894 /* DoubleKeyedContainer.swift in Sources */ = {isa = PBXBuildFile; fileRef = 77E75F802412445E00611894 /* DoubleKeyedContainer.swift */; }; - 77E75F8324124A4300611894 /* DoubleKeyedContainerTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 77E75F8224124A4300611894 /* DoubleKeyedContainerTests.swift */; }; F2255E202404600D00193742 /* UIImageView+MapleBacon.swift in Sources */ = {isa = PBXBuildFile; fileRef = F2255E1F2404600D00193742 /* UIImageView+MapleBacon.swift */; }; F2255E222404606E00193742 /* MapleBacon.swift in Sources */ = {isa = PBXBuildFile; fileRef = F2255E212404606E00193742 /* MapleBacon.swift */; }; F2255E24240462D800193742 /* MapleBaconTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = F2255E23240462D800193742 /* MapleBaconTests.swift */; }; @@ -54,8 +52,6 @@ 775455702418DF4100FF8F5F /* Download.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Download.swift; sourceTree = ""; }; 775455722418E70B00FF8F5F /* Data+MapleBacon.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "Data+MapleBacon.swift"; sourceTree = ""; }; 7778FD27240FAC6B007764C6 /* DispatchQueue+MapleBacon.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "DispatchQueue+MapleBacon.swift"; sourceTree = ""; }; - 77E75F802412445E00611894 /* DoubleKeyedContainer.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DoubleKeyedContainer.swift; sourceTree = ""; }; - 77E75F8224124A4300611894 /* DoubleKeyedContainerTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DoubleKeyedContainerTests.swift; sourceTree = ""; }; F2255E1F2404600D00193742 /* UIImageView+MapleBacon.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "UIImageView+MapleBacon.swift"; sourceTree = ""; }; F2255E212404606E00193742 /* MapleBacon.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MapleBacon.swift; sourceTree = ""; }; F2255E23240462D800193742 /* MapleBaconTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MapleBaconTests.swift; sourceTree = ""; }; @@ -154,7 +150,6 @@ children = ( F27CBA0F2402B2F0006CD529 /* CacheTests.swift */, F247A25F23FF19300083DB03 /* DiskCacheTests.swift */, - 77E75F8224124A4300611894 /* DoubleKeyedContainerTests.swift */, F27CBA212402DC0B006CD529 /* DownloaderTests.swift */, F2EEE586240BBE040002C9CA /* ImageTransformingTests.swift */, F247A24E23FF15410083DB03 /* Info.plist */, @@ -170,7 +165,6 @@ isa = PBXGroup; children = ( 775455742419208B00FF8F5F /* Cache */, - 77E75F802412445E00611894 /* DoubleKeyedContainer.swift */, F27CBA1D2402D9A8006CD529 /* Downloader.swift */, 775455702418DF4100FF8F5F /* Download.swift */, F2255E212404606E00193742 /* MapleBacon.swift */, @@ -311,7 +305,6 @@ F2255E202404600D00193742 /* UIImageView+MapleBacon.swift in Sources */, F247A25E23FF191C0083DB03 /* DiskCache.swift in Sources */, F27CBA182402C551006CD529 /* FileManager.swift in Sources */, - 77E75F812412445E00611894 /* DoubleKeyedContainer.swift in Sources */, F2EEE589240BBEF90002C9CA /* DownsamplingImageTransformer.swift in Sources */, F2EEE58D240BC17F0002C9CA /* DisplayOptions.swift in Sources */, F27CBA0E2402B1FE006CD529 /* Cache.swift in Sources */, @@ -334,7 +327,6 @@ buildActionMask = 2147483647; files = ( F27CBA222402DC0B006CD529 /* DownloaderTests.swift in Sources */, - 77E75F8324124A4300611894 /* DoubleKeyedContainerTests.swift in Sources */, F247A25C23FF163F0083DB03 /* MemoryCacheTests.swift in Sources */, F2255E24240462D800193742 /* MapleBaconTests.swift in Sources */, F247A26023FF19300083DB03 /* DiskCacheTests.swift in Sources */, diff --git a/MapleBacon/Core/DoubleKeyedContainer.swift b/MapleBacon/Core/DoubleKeyedContainer.swift deleted file mode 100644 index 20a5aa5..0000000 --- a/MapleBacon/Core/DoubleKeyedContainer.swift +++ /dev/null @@ -1,65 +0,0 @@ -// -// Copyright © 2020 Schnaub. All rights reserved. -// - -import Foundation - -/// Container structure that associates two keys with a single value. -/// Internally it keeps two Dictionaries, the first associates the keys with one another -/// and the second one associates one of the keys with the value. To ensure that remove -/// and insert are as fast as possible, removing a value for the second key type will not -/// remove it from the lookup map. Since the link between the keys is broken, this won't -/// lead to unexpected values but bear in mind that the lookup Dictionary will grow indefinitely. -final class DoubleKeyedContainer { - - private var firstContainer: [FirstKey: SecondKey] = [:] - private var secondContainer: [SecondKey: Value] = [:] - - subscript(_ key: FirstKey) -> Value? { - get { - guard let lookup = firstContainer[key] else { - return nil - } - return secondContainer[lookup] - } - } - - subscript(_ key: SecondKey) -> Value? { - get { - secondContainer[key] - } - } - - func insert(_ value: Value, forKeys keys: (FirstKey, SecondKey)) { - firstContainer[keys.0] = keys.1 - secondContainer[keys.1] = value - } - - func update(_ value: Value, forKey key: FirstKey) { - guard let lookup = firstContainer[key] else { - return - } - secondContainer[lookup] = value - } - - func update(_ value: Value, forKey key: SecondKey) { - secondContainer[key] = value - } - - func removeValue(forKeys: (FirstKey, SecondKey)) { - firstContainer[forKeys.0] = nil - secondContainer[forKeys.1] = nil - } - - func removeValue(forKey key: FirstKey) { - guard let lookup = firstContainer[key] else { - return - } - removeValue(forKeys: (key, lookup)) - } - - func removeValue(forKey key: SecondKey) { - secondContainer[key] = nil - } - -} diff --git a/MapleBaconTests/DoubleKeyedContainerTests.swift b/MapleBaconTests/DoubleKeyedContainerTests.swift deleted file mode 100644 index 9d4c291..0000000 --- a/MapleBaconTests/DoubleKeyedContainerTests.swift +++ /dev/null @@ -1,71 +0,0 @@ -// -// Copyright © 2020 Schnaub. All rights reserved. -// - -@testable import MapleBacon -import XCTest - -final class DoubleKeyedContainerTests: XCTestCase { - - func testInsert() { - let container = DoubleKeyedContainer() - container.insert("foo", forKeys: (1, "fooKey")) - - XCTAssertEqual(container[1], "foo") - XCTAssertEqual(container["fooKey"], "foo") - XCTAssertNil(container[100]) - XCTAssertNil(container["randomKey"]) - } - - func testRemoveAllKeys() { - let container = DoubleKeyedContainer() - container.insert("foo", forKeys: (1, "fooKey")) - container.removeValue(forKeys: (1, "fooKey")) - - XCTAssertNil(container[1]) - XCTAssertNil(container["fooKey"]) - } - - func testRemoveFirstKey() { - let container = DoubleKeyedContainer() - container.insert("foo", forKeys: (1, "fooKey")) - container.removeValue(forKey: 1) - container.removeValue(forKey: 100) - - XCTAssertNil(container[1]) - XCTAssertNil(container["fooKey"]) - } - - func testRemoveSecondKey() { - let container = DoubleKeyedContainer() - container.insert("foo", forKeys: (1, "fooKey")) - container.removeValue(forKey: "fooKey") - - XCTAssertNil(container[1]) - XCTAssertNil(container["fooKey"]) - } - - func testUpdate() { - let container = DoubleKeyedContainer() - container.insert("foo", forKeys: (1, "fooKey")) - - container.update("bar", forKey: 1) - XCTAssertEqual(container[1], "bar") - XCTAssertEqual(container["fooKey"], "bar") - - container.update("baz", forKey: "fooKey") - XCTAssertEqual(container[1], "baz") - XCTAssertEqual(container["fooKey"], "baz") - } - - func testAccessPerformance() { - let container = DoubleKeyedContainer() - container.insert("foo", forKeys: (1, "fooKey")) - - self.measure { - XCTAssertEqual(container[1], "foo") - XCTAssertEqual(container["fooKey"], "foo") - } - } - -} From bf1c8d41bd3eb1cfd59c02ccaa0ee0168fb59e3b Mon Sep 17 00:00:00 2001 From: Jan Gorman Date: Thu, 12 Mar 2020 10:09:52 +0100 Subject: [PATCH 30/32] Better cancel handling --- MapleBacon.xcodeproj/project.pbxproj | 4 - MapleBacon/Core/Atomic.swift | 31 -------- MapleBacon/Core/Download.swift | 79 ++++++++++++++++--- MapleBacon/Core/Downloader.swift | 39 +++------ MapleBacon/Core/MapleBacon.swift | 30 ++----- .../Extensions/UIImageView+MapleBacon.swift | 26 ++---- MapleBaconTests/DownloaderTests.swift | 53 +++++++------ MapleBaconTests/MapleBaconTests.swift | 12 +-- 8 files changed, 129 insertions(+), 145 deletions(-) delete mode 100644 MapleBacon/Core/Atomic.swift diff --git a/MapleBacon.xcodeproj/project.pbxproj b/MapleBacon.xcodeproj/project.pbxproj index 0a6c8f0..791d5ab 100644 --- a/MapleBacon.xcodeproj/project.pbxproj +++ b/MapleBacon.xcodeproj/project.pbxproj @@ -30,7 +30,6 @@ F27CBA1E2402D9A8006CD529 /* Downloader.swift in Sources */ = {isa = PBXBuildFile; fileRef = F27CBA1D2402D9A8006CD529 /* Downloader.swift */; }; F27CBA222402DC0B006CD529 /* DownloaderTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = F27CBA212402DC0B006CD529 /* DownloaderTests.swift */; }; F27CBA242402DC34006CD529 /* MockURLProtocol.swift in Sources */ = {isa = PBXBuildFile; fileRef = F27CBA232402DC34006CD529 /* MockURLProtocol.swift */; }; - F2D870E12416CB5200946A0B /* Atomic.swift in Sources */ = {isa = PBXBuildFile; fileRef = F2D870E02416CB5200946A0B /* Atomic.swift */; }; F2EEE583240BBC2E0002C9CA /* ImageTransforming.swift in Sources */ = {isa = PBXBuildFile; fileRef = F2EEE582240BBC2E0002C9CA /* ImageTransforming.swift */; }; F2EEE587240BBE040002C9CA /* ImageTransformingTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = F2EEE586240BBE040002C9CA /* ImageTransformingTests.swift */; }; F2EEE589240BBEF90002C9CA /* DownsamplingImageTransformer.swift in Sources */ = {isa = PBXBuildFile; fileRef = F2EEE588240BBEF90002C9CA /* DownsamplingImageTransformer.swift */; }; @@ -75,7 +74,6 @@ F27CBA1D2402D9A8006CD529 /* Downloader.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Downloader.swift; sourceTree = ""; }; F27CBA212402DC0B006CD529 /* DownloaderTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DownloaderTests.swift; sourceTree = ""; }; F27CBA232402DC34006CD529 /* MockURLProtocol.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MockURLProtocol.swift; sourceTree = ""; }; - F2D870E02416CB5200946A0B /* Atomic.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Atomic.swift; sourceTree = ""; }; F2EEE582240BBC2E0002C9CA /* ImageTransforming.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ImageTransforming.swift; sourceTree = ""; }; F2EEE586240BBE040002C9CA /* ImageTransformingTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ImageTransformingTests.swift; sourceTree = ""; }; F2EEE588240BBEF90002C9CA /* DownsamplingImageTransformer.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DownsamplingImageTransformer.swift; sourceTree = ""; }; @@ -170,7 +168,6 @@ F2255E212404606E00193742 /* MapleBacon.swift */, F2EEE582240BBC2E0002C9CA /* ImageTransforming.swift */, F2EEE58C240BC17F0002C9CA /* DisplayOptions.swift */, - F2D870E02416CB5200946A0B /* Atomic.swift */, ); path = Core; sourceTree = ""; @@ -309,7 +306,6 @@ F2EEE58D240BC17F0002C9CA /* DisplayOptions.swift in Sources */, F27CBA0E2402B1FE006CD529 /* Cache.swift in Sources */, F27CBA1E2402D9A8006CD529 /* Downloader.swift in Sources */, - F2D870E12416CB5200946A0B /* Atomic.swift in Sources */, 775455732418E70B00FF8F5F /* Data+MapleBacon.swift in Sources */, F2EEE58B240BBFC00002C9CA /* CGSize+MapleBacon.swift in Sources */, 775455712418DF4100FF8F5F /* Download.swift in Sources */, diff --git a/MapleBacon/Core/Atomic.swift b/MapleBacon/Core/Atomic.swift deleted file mode 100644 index d5626a0..0000000 --- a/MapleBacon/Core/Atomic.swift +++ /dev/null @@ -1,31 +0,0 @@ -// -// Copyright © 2020 Schnaub. All rights reserved. -// - -import Foundation - -final class Atomic { - - var value: Value { - get { - queue.sync { - self.wrappedValue - } - } - } - - private let queue = DispatchQueue(label: "com.schnaub.MapleBacon.atomic") - - private var wrappedValue: Value - - init(_ value: Value) { - self.wrappedValue = value - } - - func mutate(_ transform: (inout Value) -> Void) { - queue.sync { - transform(&self.wrappedValue) - } - } - -} diff --git a/MapleBacon/Core/Download.swift b/MapleBacon/Core/Download.swift index 313f438..82a6954 100644 --- a/MapleBacon/Core/Download.swift +++ b/MapleBacon/Core/Download.swift @@ -4,28 +4,73 @@ import UIKit -public final class Download { +public struct DownloadTask { + + let download: Download + + public let cancelToken: CancelToken + + public func cancel() { + download.cancel(cancelToken: cancelToken) + } + +} + +final class Download { + + typealias Completion = (Result) -> Void let task: URLSessionDataTask - let url: URL - let token: CancelToken - var completions: [(Result) -> Void] - var data = Data() + var completions: [Completion] { + defer { + lock.unlock() + } + lock.lock() + return Array(tokenCompletions.values) + } + + private let lock = NSLock() + private(set) var data = Data() + private var currentToken: CancelToken = 0 private var backgroundTask: UIBackgroundTaskIdentifier = .invalid + private var tokenCompletions: [CancelToken: Completion] = [:] - init(task: URLSessionDataTask, url: URL, token: CancelToken, completion: @escaping (Result) -> Void) { + init(task: URLSessionDataTask) { self.task = task - self.url = url - self.token = token - self.completions = [completion] } deinit { invalidateBackgroundTask() } + func addCompletion(_ completion: @escaping Completion) -> CancelToken { + defer { + currentToken += 1 + lock.unlock() + } + lock.lock() + tokenCompletions[currentToken] = completion + return currentToken + } + + func removeCompletion(for token: CancelToken) -> Completion? { + defer { + lock.unlock() + } + lock.lock() + guard let completion = tokenCompletions[token] else { + return nil + } + tokenCompletions[token] = nil + return completion + } + + func appendData(_ data: Data) { + self.data.append(data) + } + func start() { backgroundTask = UIApplication.shared.beginBackgroundTask { self.invalidateBackgroundTask() @@ -36,7 +81,21 @@ public final class Download { invalidateBackgroundTask() } - private func invalidateBackgroundTask() { + func cancel(cancelToken: CancelToken) { + guard let completion = removeCompletion(for: cancelToken) else { + return + } + if tokenCompletions.isEmpty { + task.cancel() + } + completion(.failure(DownloaderError.canceled)) + } + +} + +private extension Download { + + func invalidateBackgroundTask() { UIApplication.shared.endBackgroundTask(backgroundTask) backgroundTask = .invalid } diff --git a/MapleBacon/Core/Downloader.swift b/MapleBacon/Core/Downloader.swift index ef125a2..67a9ab0 100644 --- a/MapleBacon/Core/Downloader.swift +++ b/MapleBacon/Core/Downloader.swift @@ -45,39 +45,22 @@ final class Downloader { session.invalidateAndCancel() } - func fetch(_ url: URL, token: CancelToken, completion: @escaping (Result) -> Void) -> Download { - let task: URLSessionDataTask - let download: Download - - if let existingDownload = self[url] { - task = existingDownload.task - existingDownload.completions.append(completion) - download = existingDownload - } else { - let newTask = session.dataTask(with: url) - let newDownload = Download(task: newTask, url: url, token: token, completion: completion) - newDownload.start() - self[url] = newDownload - task = newTask - download = newDownload + func fetch(_ url: URL, completion: @escaping (Result) -> Void) -> DownloadTask { + if let download = self[url] { + let token = download.addCompletion(completion) + return DownloadTask(download: download, cancelToken: token) } + let task = session.dataTask(with: url) + let download = Download(task: task) + let token = download.addCompletion(completion) + download.start() + self[url] = download task.resume() - return download + return DownloadTask(download: download, cancelToken: token) } -// func cancel(token: CancelToken) { -// guard let download = self[token] else { -// return -// } -// if download.completions.count == 1 { -// download.task.cancel() -// download.completions.first?(.failure(DownloaderError.canceled)) -// self[token] = nil -// } -// } - } private final class SessionDelegate: NSObject, URLSessionDataDelegate { @@ -88,7 +71,7 @@ private final class SessionDelegate: NSObject, URLSessionDat guard let url = dataTask.originalRequest?.url, let download = downloader?[url] else { return } - download.data.append(data) + download.appendData(data) } func urlSession(_ session: URLSession, task: URLSessionTask, didCompleteWithError error: Error?) { diff --git a/MapleBacon/Core/MapleBacon.swift b/MapleBacon/Core/MapleBacon.swift index db33ba7..446cb84 100644 --- a/MapleBacon/Core/MapleBacon.swift +++ b/MapleBacon/Core/MapleBacon.swift @@ -31,8 +31,6 @@ public final class MapleBacon { private let downloader: Downloader private let transformerQueue: DispatchQueue - private var token = Atomic(0) - public convenience init(name: String = "", sessionConfiguration: URLSessionConfiguration = .default) { self.init(cache: Cache(name: name), sessionConfiguration: sessionConfiguration) } @@ -44,28 +42,25 @@ public final class MapleBacon { } @discardableResult - public func image(with url: URL, imageTransformer: ImageTransforming? = nil, completion: @escaping ImageCompletion) -> Download? { + public func image(with url: URL, imageTransformer: ImageTransforming? = nil, completion: @escaping ImageCompletion) -> DownloadTask? { if (try? isCached(with: url, imageTransformer: imageTransformer)) == true { fetchImageFromCache(with: url, imageTransformer: imageTransformer, completion: completion) return nil } - let token = makeToken() - return fetchImageFromNetworkAndCache(with: url, token: token, imageTransformer: imageTransformer, completion: completion) + return fetchImageFromNetworkAndCache(with: url, imageTransformer: imageTransformer, completion: completion) } public func hydrateCache(url: URL) { if (try? isCached(with: url, imageTransformer: nil)) == false { - let token = self.makeToken() - _ = self.fetchImageFromNetworkAndCache(with: url, token: token, imageTransformer: nil, completion: { _ in }) + _ = self.fetchImageFromNetworkAndCache(with: url, imageTransformer: nil, completion: { _ in }) } } public func hydrateCache(urls: [URL]) { for url in urls { if (try? isCached(with: url, imageTransformer: nil)) == false { - let token = self.makeToken() - _ = self.fetchImageFromNetworkAndCache(with: url, token: token, imageTransformer: nil, completion: { _ in }) + _ = self.fetchImageFromNetworkAndCache(with: url, imageTransformer: nil, completion: { _ in }) } } } @@ -74,19 +69,10 @@ public final class MapleBacon { cache.clear(options, completion: completion) } - public func cancel(token: CancelToken) { -// downloader.cancel(token: token) - } - } private extension MapleBacon { - func makeToken() -> CancelToken { - token.mutate { $0 += 1 } - return token.value - } - func isCached(with url: URL, imageTransformer: ImageTransforming?) throws -> Bool { let cacheKey = makeCacheKey(for: url, imageTransformer: imageTransformer) return try cache.isCached(forKey: cacheKey) @@ -108,8 +94,8 @@ private extension MapleBacon { } } - func fetchImageFromNetworkAndCache(with url: URL, token: CancelToken, imageTransformer: ImageTransforming?, completion: @escaping ImageCompletion) -> Download { - fetchImageFromNetwork(with: url, token: token) { result in + func fetchImageFromNetworkAndCache(with url: URL, imageTransformer: ImageTransforming?, completion: @escaping ImageCompletion) -> DownloadTask { + fetchImageFromNetwork(with: url) { result in switch result { case .success(let image): if let transformer = imageTransformer { @@ -129,8 +115,8 @@ private extension MapleBacon { } } - func fetchImageFromNetwork(with url: URL, token: CancelToken, completion: @escaping ImageCompletion) -> Download { - downloader.fetch(url, token: token, completion: completion) + func fetchImageFromNetwork(with url: URL, completion: @escaping ImageCompletion) -> DownloadTask { + downloader.fetch(url, completion: completion) } func transformImageAndCache(_ image: UIImage, cacheKey: String, imageTransformer: ImageTransforming, completion: @escaping ImageCompletion) { diff --git a/MapleBacon/Extensions/UIImageView+MapleBacon.swift b/MapleBacon/Extensions/UIImageView+MapleBacon.swift index c1cd7b0..0703c79 100644 --- a/MapleBacon/Extensions/UIImageView+MapleBacon.swift +++ b/MapleBacon/Extensions/UIImageView+MapleBacon.swift @@ -5,8 +5,7 @@ import UIKit private var baconImageUrlKey: UInt8 = 0 -private var cancelTokenKey: UInt8 = 1 -private var downloadKey: UInt8 = 2 +private var downloadKey: UInt8 = 1 extension UIImageView { @@ -19,18 +18,9 @@ extension UIImageView { } } - private var cancelToken: CancelToken? { + private var downloadTask: DownloadTask? { get { - objc_getAssociatedObject(self, &cancelTokenKey) as? Int - } - set { - objc_setAssociatedObject(self, &cancelTokenKey, newValue, .OBJC_ASSOCIATION_RETAIN_NONATOMIC) - } - } - - private var download: Download? { - get { - objc_getAssociatedObject(self, &downloadKey) as? Download + objc_getAssociatedObject(self, &downloadKey) as? DownloadTask } set { objc_setAssociatedObject(self, &downloadKey, newValue, .OBJC_ASSOCIATION_RETAIN_NONATOMIC) @@ -51,10 +41,10 @@ extension UIImageView { let transformer = makeTransformer(displayOptions: displayOptions, imageTransformer: imageTransformer) - _ = MapleBacon.shared.image(with: url, imageTransformer: transformer) { [weak self] result in + let task = MapleBacon.shared.image(with: url, imageTransformer: transformer) { [weak self] result in defer { self?.baconImageUrl = nil - self?.cancelToken = nil + self?.downloadTask = nil completion?() } guard case let Result.success(image) = result, let self = self, url == self.baconImageUrl else { @@ -62,13 +52,11 @@ extension UIImageView { } self.image = image } + downloadTask = task } private func cancelDownload() { - guard let token = cancelToken else { - return - } - MapleBacon.shared.cancel(token: token) + downloadTask?.cancel() } private func makeTransformer(displayOptions: [DisplayOptions] = [], imageTransformer: ImageTransforming?) -> ImageTransforming? { diff --git a/MapleBaconTests/DownloaderTests.swift b/MapleBaconTests/DownloaderTests.swift index 99c8fa6..e13fc03 100644 --- a/MapleBaconTests/DownloaderTests.swift +++ b/MapleBaconTests/DownloaderTests.swift @@ -8,7 +8,6 @@ import XCTest final class DownloaderTests: XCTestCase { private static let url = URL(string: "https://example.com/mapleBacon.png")! - private static let cancelToken = 1 func testDownload() { let expectation = self.expectation(description: #function) @@ -17,7 +16,7 @@ final class DownloaderTests: XCTestCase { setupMockResponse(.data(dummyData())) - _ = downloader.fetch(Self.url, token: Self.cancelToken) { response in + _ = downloader.fetch(Self.url) { response in switch response { case .success(let data): XCTAssertNotNil(data) @@ -38,7 +37,7 @@ final class DownloaderTests: XCTestCase { setupMockResponse(.data(dummyData())) - _ = downloader.fetch(Self.url, token: Self.cancelToken) { response in + _ = downloader.fetch(Self.url) { response in switch response { case .success: XCTFail() @@ -59,7 +58,7 @@ final class DownloaderTests: XCTestCase { setupMockResponse(.error) - _ = downloader.fetch(Self.url, token: Self.cancelToken) { response in + _ = downloader.fetch(Self.url) { response in switch response { case .success: XCTFail() @@ -79,7 +78,7 @@ final class DownloaderTests: XCTestCase { setupMockResponse(.data(dummyData())) let firstExpectation = expectation(description: "first") - _ = downloader.fetch(Self.url, token: Self.cancelToken) { response in + _ = downloader.fetch(Self.url) { response in switch response { case .success(let data): XCTAssertNotNil(data) @@ -90,7 +89,7 @@ final class DownloaderTests: XCTestCase { } let secondExpectation = expectation(description: "second") - _ = downloader.fetch(Self.url, token: Self.cancelToken) { response in + _ = downloader.fetch(Self.url) { response in switch response { case .success(let data): XCTAssertNotNil(data) @@ -103,25 +102,27 @@ final class DownloaderTests: XCTestCase { waitForExpectations(timeout: 5, handler: nil) } -// func testCancel() { -// let expectation = self.expectation(description: #function) -// let configuration = MockURLProtocol.mockedURLSessionConfiguration() -// let downloader = Downloader(sessionConfiguration: configuration) -// -// setupMockResponse(.error) -// -// downloader.fetch(Self.url, token: Self.cancelToken) { response in -// switch response { -// case .failure(let error as DownloaderError): -// XCTAssertEqual(error, .canceled) -// case .success, .failure: -// XCTFail() -// } -// expectation.fulfill() -// } -// downloader.cancel(token: Self.cancelToken) -// -// waitForExpectations(timeout: 5, handler: nil) -// } + func testCancel() { + let expectation = self.expectation(description: #function) + let configuration = MockURLProtocol.mockedURLSessionConfiguration() + let downloader = Downloader(sessionConfiguration: configuration) + + setupMockResponse(.error) + + let downloadTask = downloader.fetch(Self.url) { response in + switch response { + case .failure(let error as DownloaderError): + XCTAssertEqual(error, .canceled) + case .success, .failure: + XCTFail() + } + expectation.fulfill() + } + + XCTAssertNotNil(downloadTask) + downloadTask.cancel() + + waitForExpectations(timeout: 5, handler: nil) + } } diff --git a/MapleBaconTests/MapleBaconTests.swift b/MapleBaconTests/MapleBaconTests.swift index e465e28..845e9a6 100644 --- a/MapleBaconTests/MapleBaconTests.swift +++ b/MapleBaconTests/MapleBaconTests.swift @@ -93,18 +93,20 @@ final class MapleBaconTests: XCTestCase { setupMockResponse(.error) - let token = mapleBacon.image(with: Self.url) { result in + let downloadTask = mapleBacon.image(with: Self.url) { result in switch result { - case .success: + case .failure(let error as DownloaderError): + XCTAssertEqual(error, .canceled) + case .success, .failure: XCTFail() - case .failure(let error as NSError): - XCTAssertEqual(error.code, -1) } mapleBacon.clearCache(.all) { _ in expectation.fulfill() } } -// mapleBacon.cancel(token: token) + + XCTAssertNotNil(downloadTask) + downloadTask?.cancel() waitForExpectations(timeout: 5, handler: nil) } From 391bd41ac89638aa4e802cfbd88eb505d4174a2b Mon Sep 17 00:00:00 2001 From: Jan Gorman Date: Sun, 15 Mar 2020 16:14:30 +0100 Subject: [PATCH 31/32] Documentation --- MapleBacon/Core/Download.swift | 1 + MapleBacon/Core/MapleBacon.swift | 26 +++++++++++++++++++ .../Extensions/UIImageView+MapleBacon.swift | 21 ++++++++++++--- 3 files changed, 44 insertions(+), 4 deletions(-) diff --git a/MapleBacon/Core/Download.swift b/MapleBacon/Core/Download.swift index 82a6954..3b43ab9 100644 --- a/MapleBacon/Core/Download.swift +++ b/MapleBacon/Core/Download.swift @@ -4,6 +4,7 @@ import UIKit +/// A download task – this wraps the internal download instance and can be used to cancel an inflight request public struct DownloadTask { let download: Download diff --git a/MapleBacon/Core/MapleBacon.swift b/MapleBacon/Core/MapleBacon.swift index 446cb84..24781b9 100644 --- a/MapleBacon/Core/MapleBacon.swift +++ b/MapleBacon/Core/MapleBacon.swift @@ -14,6 +14,7 @@ public final class MapleBacon { public typealias ImageCompletion = (Result) -> Void + /// The shared instance of MapleBacon public static let shared = MapleBacon() private static let queueLabel = "com.schnaub.MapleBacon.transformer" @@ -31,6 +32,10 @@ public final class MapleBacon { private let downloader: Downloader private let transformerQueue: DispatchQueue + /// Initialise a custom instance of MapleBacon + /// - Parameters: + /// - name: The name to give this instance. Internally this reflects a distinct cache region. + /// - sessionConfiguration: The URLSessionConfiguration to use. Uses `.default` when no parameter is supplied. public convenience init(name: String = "", sessionConfiguration: URLSessionConfiguration = .default) { self.init(cache: Cache(name: name), sessionConfiguration: sessionConfiguration) } @@ -41,6 +46,13 @@ public final class MapleBacon { self.transformerQueue = DispatchQueue(label: Self.queueLabel, attributes: .concurrent) } + /// Return an image for the passed URL. This will check the in-memory and disk cache and fetch the image over the network if nothing is in either cache yet. + /// After a successful download, the image will be stored in both caches. + /// - Parameters: + /// - url: The URL of the image + /// - imageTransformer: An optional image transformer + /// - completion: The completion to call with the image result + /// - Returns: An optional `DownloadTask` if needs to fetch the image over the network. The task can be used to cancel an inflight request @discardableResult public func image(with url: URL, imageTransformer: ImageTransforming? = nil, completion: @escaping ImageCompletion) -> DownloadTask? { if (try? isCached(with: url, imageTransformer: imageTransformer)) == true { @@ -51,12 +63,16 @@ public final class MapleBacon { return fetchImageFromNetworkAndCache(with: url, imageTransformer: imageTransformer, completion: completion) } + /// Hydrate the cache + /// - Parameter url: The URL to fetch public func hydrateCache(url: URL) { if (try? isCached(with: url, imageTransformer: nil)) == false { _ = self.fetchImageFromNetworkAndCache(with: url, imageTransformer: nil, completion: { _ in }) } } + /// Hydrate the cache + /// - Parameter urls: An array of URLs to fetch public func hydrateCache(urls: [URL]) { for url in urls { if (try? isCached(with: url, imageTransformer: nil)) == false { @@ -65,6 +81,10 @@ public final class MapleBacon { } } + /// Clear the cache + /// - Parameters: + /// - options: The `CacheClearOptions`. Clear either only memory, disk or both caches. + /// - completion: The completion to call after clearing the cache public func clearCache(_ options: CacheClearOptions, completion: ((Error?) -> Void)? = nil) { cache.clear(options, completion: completion) } @@ -159,6 +179,12 @@ import Combine @available(iOS 13.0, *) extension MapleBacon { + /// The Combine way to fetch an image for the passed URL. This will check the in-memory and disk cache and fetch the image over the network if nothing is in either cache yet. + /// After a successful download, the image will be stored in both caches. + /// - Parameters: + /// - url: The URL of the image + /// - imageTransformer: An optional image transformer + /// - Returns: `AnyPublisher` public func image(with url: URL, imageTransformer: ImageTransforming? = nil) -> AnyPublisher { Future { resolve in self.image(with: url, imageTransformer: imageTransformer) { result in diff --git a/MapleBacon/Extensions/UIImageView+MapleBacon.swift b/MapleBacon/Extensions/UIImageView+MapleBacon.swift index 0703c79..c35ecb6 100644 --- a/MapleBacon/Extensions/UIImageView+MapleBacon.swift +++ b/MapleBacon/Extensions/UIImageView+MapleBacon.swift @@ -27,35 +27,48 @@ extension UIImageView { } } + /// Set remote image + /// - Parameters: + /// - url: The URL of the image + /// - placeholder: An optional placeholder image to set while fetching the remote image + /// - displayOptions: `DisplayOptions` + /// - imageTransformer: An optional image transformer + /// - completion: An optional completion to call when the image is set + /// - Returns: An optional `DownloadTask` if needs to fetch the image over the network. The task can be used to cancel an inflight request + @discardableResult public func setImage(with url: URL?, placeholder: UIImage? = nil, displayOptions: [DisplayOptions] = [], imageTransformer: ImageTransforming? = nil, - completion: (() -> Void)? = nil) { + completion: ((UIImage?) -> Void)? = nil) -> DownloadTask? { cancelDownload() baconImageUrl = url image = placeholder guard let url = url else { - return + return nil } let transformer = makeTransformer(displayOptions: displayOptions, imageTransformer: imageTransformer) let task = MapleBacon.shared.image(with: url, imageTransformer: transformer) { [weak self] result in + var resultImage: UIImage? defer { self?.baconImageUrl = nil self?.downloadTask = nil - completion?() + completion?(resultImage) } guard case let Result.success(image) = result, let self = self, url == self.baconImageUrl else { return } + resultImage = image self.image = image } downloadTask = task + return task } - private func cancelDownload() { + /// Cancel a running download + public func cancelDownload() { downloadTask?.cancel() } From 64a63ed2a0abe0173661836fd8e62fc5827d369c Mon Sep 17 00:00:00 2001 From: Jan Gorman Date: Sun, 15 Mar 2020 16:14:55 +0100 Subject: [PATCH 32/32] Bump version --- MapleBacon.podspec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/MapleBacon.podspec b/MapleBacon.podspec index dc30a0a..c6f55ce 100644 --- a/MapleBacon.podspec +++ b/MapleBacon.podspec @@ -1,6 +1,6 @@ Pod::Spec.new do |s| s.name = 'MapleBacon' - s.version = '6.0.5' + s.version = '6.1.0' s.swift_version = '5.1' s.summary = 'A lightweight and fast image downloading library iOS.'