Update image fetcher and tests

Sources/Networking/ImageDownloader.swift

@@ -94,7 +94,8 @@ open class ImageDownloader {
     /// - Parameter name: The name for the downloader. It should not be empty.
     public init(name: String) {
         if name.isEmpty {
-            fatalError("[Kingfisher] You should specify a name for the downloader. A downloader with empty name is not permitted.")
+            fatalError("[Kingfisher] You should specify a name for the downloader. "
+                + "A downloader with empty name is not permitted.")
         }
         
         processQueue = DispatchQueue(label: "com.onevcat.Kingfisher.ImageDownloader.Process.\(name)")

Sources/Networking/ImagePrefetcher.swift

@@ -26,12 +26,11 @@
 
 
 #if os(macOS)
-    import AppKit
+import AppKit
 #else
-    import UIKit
+import UIKit
 #endif
 
-
 /// Progress update block of prefetcher. 
 ///
 /// - `skippedResources`: An array of resources that are already cached before the prefetching starting.
@@ -64,7 +63,7 @@ public class ImagePrefetcher {
     private var progressBlock: PrefetcherProgressBlock?
     private var completionHandler: PrefetcherCompletionHandler?
     
-    private var tasks = [URL: SessionDataTask]()
+    private var tasks = [URL: DownloadTask]()
     
     private var pendingResources: ArraySlice<Resource>
     private var skippedResources = [Resource]()
@@ -137,7 +136,8 @@ public class ImagePrefetcher {
         prefetchQueue = DispatchQueue(label: prefetchQueueName)
         
         // We want all callbacks from our prefetch queue, so we should ignore the call back queue in options
-        var optionsInfoWithoutQueue = options?.removeAllMatchesIgnoringAssociatedValue(.callbackDispatchQueue(nil)) ?? .empty
+        var optionsInfoWithoutQueue = (options ?? .empty)
+            .removeAllMatchesIgnoringAssociatedValue(.callbackDispatchQueue(nil))
         
         // Add our own callback dispatch queue to make sure all callbacks are coming back in our expected queue
         optionsInfoWithoutQueue.append(.callbackDispatchQueue(prefetchQueue))
@@ -152,11 +152,10 @@ public class ImagePrefetcher {
         self.completionHandler = completionHandler
     }
     
-    /**
-     Start to download the resources and cache them. This can be useful for background downloading
-     of assets that are required for later use in an app. This code will not try and update any UI
-     with the results of the process.
-     */
+    
+    /// Start to download the resources and cache them. This can be useful for background downloading
+    /// of assets that are required for later use in an app. This code will not try and update any UI
+    /// with the results of the process.
     public func start()
     {
         // Since we want to handle the resources cancellation in the prefetch queue only.
@@ -179,8 +178,8 @@ public class ImagePrefetcher {
                 return
             }
             
-            let initialConcurentDownloads = min(self.prefetchResources.count, self.maxConcurrentDownloads)
-            for _ in 0 ..< initialConcurentDownloads {
+            let initialConcurrentDownloads = min(self.prefetchResources.count, self.maxConcurrentDownloads)
+            for _ in 0 ..< initialConcurrentDownloads {
                 if let resource = self.pendingResources.popFirst() {
                     self.startPrefetching(resource)
                 }
@@ -188,15 +187,12 @@ public class ImagePrefetcher {
         }
     }
 
-   
-    /**
-     Stop current downloading progress, and cancel any future prefetching activity that might be occuring.
-     */
+    /// Stop current downloading progress, and cancel any future prefetching activity that might be occuring.
     public func stop() {
         prefetchQueue.async {
             if self.finished { return }
             self.stopped = true
-            self.tasks.values.forEach { $0.forceCancel() }
+            self.tasks.values.forEach { $0.cancel() }
         }
     }
     
@@ -229,7 +225,7 @@ public class ImagePrefetcher {
             completionHandler: downloadTaskCompletionHandler)
         
         if let downloadTask = downloadTask {
-            tasks[resource.downloadURL] = downloadTask.sessionTask
+            tasks[resource.downloadURL] = downloadTask
         }
     }
     
@@ -245,8 +241,10 @@ public class ImagePrefetcher {
         if optionsInfo.forceRefresh {
             downloadAndCache(resource)
         } else {
-            let alreadyInCache = manager.cache.imageCachedType(forKey: resource.cacheKey,
-                                                             processorIdentifier: optionsInfo.processor.identifier).cached
+            let alreadyInCache = manager.cache.imageCachedType(
+                forKey: resource.cacheKey,
+                processorIdentifier: optionsInfo.processor.identifier).cached
+            
             if alreadyInCache {
                 append(cached: resource)
             } else {

Tests/KingfisherTests/ImagePrefetcherTests.swift

@@ -41,203 +41,177 @@ class ImagePrefetcherTests: XCTestCase {
     }
     
     override class func tearDown() {
-        super.tearDown()
         LSNocilla.sharedInstance().stop()
+        super.tearDown()
     }
     
     override func setUp() {
         super.setUp()
-        // Put setup code here. This method is called before the invocation of each test method in the class.
         cleanDefaultCache()
     }
     
     override func tearDown() {
-        // Put teardown code here. This method is called after the invocation of each test method in the class.
         cleanDefaultCache()
         super.tearDown()
     }
 
     func testPrefetchingImages() {
-        let expectation = self.expectation(description: "wait for prefetching images")
-        
-        var urls = [URL]()
-        for URLString in testKeys {
-            _ = stubRequest("GET", URLString).andReturn(200)?.withBody(testImageData)
-            urls.append(URL(string: URLString)!)
-        }
+        let exp = expectation(description: #function)
         
+        testURLs.forEach { stub($0, data: testImageData2) }
         var progressCalledCount = 0
-        let prefetcher = ImagePrefetcher(urls: urls, options: [.waitForCache],
-                            progressBlock: { skippedResources, failedResources, completedResources in
-                                progressCalledCount += 1
-                            },
-                            completionHandler: { skippedResources, failedResources, completedResources in
-                                expectation.fulfill()
-                                XCTAssertEqual(skippedResources.count, 0, "There should be no items skipped.")
-                                XCTAssertEqual(failedResources.count, 0, "There should be no failed downloading.")
-                                XCTAssertEqual(completedResources.count, urls.count, "All resources prefetching should be completed.")
-                                XCTAssertEqual(progressCalledCount, urls.count, "Progress should be called the same time of download count.")
-                                for url in urls {
-                                    XCTAssertTrue(KingfisherManager.shared.cache.imageCachedType(forKey: url.absoluteString).cached)
-                                }
-                            })
+        let prefetcher = ImagePrefetcher(
+            urls: testURLs,
+            options: [.waitForCache],
+            progressBlock: { _, _, _ in progressCalledCount += 1 }) {
+                skippedResources, failedResources, completedResources in
+
+                XCTAssertEqual(skippedResources.count, 0)
+                XCTAssertEqual(failedResources.count, 0)
+                XCTAssertEqual(completedResources.count, testURLs.count)
+                XCTAssertEqual(progressCalledCount, testURLs.count)
+                for url in testURLs {
+                    XCTAssertTrue(KingfisherManager.shared.cache.imageCachedType(forKey: url.absoluteString).cached)
+                }
+                exp.fulfill()
+            }
         prefetcher.start()
-        
-        waitForExpectations(timeout: 5, handler: nil)
+        waitForExpectations(timeout: 1, handler: nil)
     }
     
     func testCancelPrefetching() {
-        let expectation = self.expectation(description: "wait for prefetching images")
-        
-        var urls = [URL]()
-        var responses = [LSStubResponseDSL?]()
-        for URLString in testKeys {
-            let response = stubRequest("GET", URLString).andReturn(200)?.withBody(testImageData)?.delay()
-            responses.append(response)
-            urls.append(URL(string: URLString)!)
-        }
+        let exp = expectation(description: #function)
+        let stubs = testURLs.map { delayedStub($0, data: testImageData2) }
         
         let maxConcurrentCount = 2
-        let prefetcher = ImagePrefetcher(urls: urls, options: [.waitForCache],
-                            progressBlock: { skippedResources, failedResources, completedResources in
-                            },
-                            completionHandler: { skippedResources, failedResources, completedResources in
-                                XCTAssertEqual(skippedResources.count, 0, "There should be no items skipped.")
-                                XCTAssertEqual(failedResources.count, urls.count, "The failed count should be the same with started downloads due to cancellation.")
-                                XCTAssertEqual(completedResources.count, 0, "None resources prefetching should complete.")
-                                expectation.fulfill()
-                            })
+        let prefetcher = ImagePrefetcher(
+            urls: testURLs,
+            options: [.waitForCache])
+        {
+            skippedResources, failedResources, completedResources in
+            
+            XCTAssertEqual(skippedResources.count, 0)
+            XCTAssertEqual(failedResources.count, testURLs.count)
+            XCTAssertEqual(completedResources.count, 0)
+            exp.fulfill()
+        }
         
         prefetcher.maxConcurrentDownloads = maxConcurrentCount
         
         prefetcher.start()
-        prefetcher.stop()
         
-        delay(0.1) { responses.forEach { _ = $0!.go() } }
-        waitForExpectations(timeout: 5, handler: nil)
+        DispatchQueue.main.async {
+            prefetcher.stop()
+            delay(0.1) { stubs.forEach { _ = $0.go() } }
+        }
+        waitForExpectations(timeout: 1, handler: nil)
     }
     
 
     func testPrefetcherCouldSkipCachedImages() {
-        let expectation = self.expectation(description: "wait for prefetching images")
+        let exp = expectation(description: #function)
         KingfisherManager.shared.cache.store(Image(), forKey: testKeys[0])
         
-        var urls = [URL]()
-        for URLString in testKeys {
-            _ = stubRequest("GET", URLString).andReturn(200)?.withBody(testImageData)
-            urls.append(URL(string: URLString)!)
-        }
-        
-        let prefetcher = ImagePrefetcher(urls: urls, options: [.waitForCache], progressBlock: { skippedResources, failedResources, completedResources in
-
-            }) { skippedResources, failedResources, completedResources in
-                expectation.fulfill()
-                XCTAssertEqual(skippedResources.count, 1, "There should be 1 item skipped.")
-                XCTAssertEqual(skippedResources[0].downloadURL.absoluteString, testKeys[0], "The correct image key should be skipped.")
-
-                XCTAssertEqual(failedResources.count, 0, "There should be no failed downloading.")
-                XCTAssertEqual(completedResources.count, urls.count - 1, "All resources prefetching should be completed.")
+        testURLs.forEach { stub($0, data: testImageData2) }
+        let prefetcher = ImagePrefetcher(
+            urls: testURLs,
+            options: [.waitForCache])
+        {
+            skippedResources, failedResources, completedResources in
+            XCTAssertEqual(skippedResources.count, 1)
+            XCTAssertEqual(skippedResources[0].downloadURL, testURLs[0])
+            XCTAssertEqual(failedResources.count, 0)
+            XCTAssertEqual(completedResources.count, testURLs.count - 1)
+            exp.fulfill()
         }
         
         prefetcher.start()
         
-        waitForExpectations(timeout: 5, handler: nil)
+        waitForExpectations(timeout: 1, handler: nil)
     }
     
     func testPrefetcherForceRefreshDownloadImages() {
-        let expectation = self.expectation(description: "wait for prefetching images")
-        
-        // Store an image in cache.
+        let exp = expectation(description: #function)
         KingfisherManager.shared.cache.store(Image(), forKey: testKeys[0])
         
-        var urls = [URL]()
-        for URLString in testKeys {
-            _ = stubRequest("GET", URLString).andReturn(200)?.withBody(testImageData)
-            urls.append(URL(string: URLString)!)
-        }
-        
-        // Use `.ForceRefresh` to download it forcely.
-        let prefetcher = ImagePrefetcher(urls: urls, options: [.forceRefresh], progressBlock: { skippedResources, failedResources, completedResources in
-            
-            }) { skippedResources, failedResources, completedResources in
-                expectation.fulfill()
-                
-                XCTAssertEqual(skippedResources.count, 0, "There should be no item skipped.")
-                XCTAssertEqual(failedResources.count, 0, "There should be no failed downloading.")
-                XCTAssertEqual(completedResources.count, urls.count, "All resources prefetching should be completed.")
+        testURLs.forEach { stub($0, data: testImageData2) }
+        let prefetcher = ImagePrefetcher(urls: testURLs, options: [.forceRefresh, .waitForCache]) {
+            skippedResources, failedResources, completedResources in
+            XCTAssertEqual(skippedResources.count, 0)
+            XCTAssertEqual(failedResources.count, 0)
+            XCTAssertEqual(completedResources.count, testURLs.count)
+            exp.fulfill()
         }
         
         prefetcher.start()
-        
-        waitForExpectations(timeout: 5, handler: nil)
+        waitForExpectations(timeout: 1, handler: nil)
     }
     
     func testPrefetchWithWrongInitParameters() {
-        let expectation = self.expectation(description: "wait for prefetching images")
-        let prefetcher = ImagePrefetcher(urls: [], options: [.waitForCache], progressBlock: nil) { skippedResources, failedResources, completedResources in
-            expectation.fulfill()
-            
-            XCTAssertEqual(skippedResources.count, 0, "There should be no item skipped.")
-            XCTAssertEqual(failedResources.count, 0, "There should be no failed downloading.")
-            XCTAssertEqual(completedResources.count, 0, "There should be no completed downloading.")
+        let exp = expectation(description: #function)
+        let prefetcher = ImagePrefetcher(urls: [], options: [.waitForCache]) {
+            skippedResources, failedResources, completedResources in
+            XCTAssertEqual(skippedResources.count, 0)
+            XCTAssertEqual(failedResources.count, 0)
+            XCTAssertEqual(completedResources.count, 0)
+            exp.fulfill()
         }
         
         prefetcher.start()
-        waitForExpectations(timeout: 5, handler: nil)
+        waitForExpectations(timeout: 1, handler: nil)
     }
     
     func testFetchWithProcessor() {
-        let expectation = self.expectation(description: "wait for prefetching images")
-        
-        var urls = [URL]()
-        for URLString in testKeys {
-            _ = stubRequest("GET", URLString).andReturn(200)?.withBody(testImageData)
-            urls.append(URL(string: URLString)!)
-        }
+        let exp = expectation(description: #function)
+        testURLs.forEach { stub($0, data: testImageData2, length: 123) }
         
         let p = RoundCornerImageProcessor(cornerRadius: 20)
         
         func prefetchAgain() {
             var progressCalledCount = 0
-            let prefetcher = ImagePrefetcher(urls: urls, options: [.processor(p)],
-                                             progressBlock: { skippedResources, failedResources, completedResources in
-                                                progressCalledCount += 1
-            },
-                                             completionHandler: { skippedResources, failedResources, completedResources in
+            let prefetcher = ImagePrefetcher(
+                urls: testURLs,
+                options: [.processor(p), .waitForCache],
+                progressBlock: { _, _, _ in progressCalledCount += 1 })
+            {
+                skippedResources, failedResources, completedResources in
                                                 
-                                                XCTAssertEqual(skippedResources.count, urls.count, "There should be one item skipped since it is just prefetched.")
-                                                XCTAssertEqual(failedResources.count, 0, "There should be no failed downloading.")
-                                                XCTAssertEqual(completedResources.count, 0, "No need to prefetch anymore")
-                                                XCTAssertEqual(progressCalledCount, urls.count, "Progress should be called the same time of download count.")
-                                                for url in urls {
-                                                    XCTAssertTrue(KingfisherManager.shared.cache.imageCachedType(forKey: url.absoluteString, processorIdentifier: p.identifier).cached)
-                                                }
-                                                expectation.fulfill()
+                XCTAssertEqual(skippedResources.count, testURLs.count)
+                XCTAssertEqual(failedResources.count, 0)
+                XCTAssertEqual(completedResources.count, 0)
+                XCTAssertEqual(progressCalledCount, testURLs.count)
+                for url in testURLs {
+                    let cached = KingfisherManager.shared.cache.imageCachedType(
+                        forKey: url.absoluteString, processorIdentifier: p.identifier).cached
+                    XCTAssertTrue(cached)
+                }
+                exp.fulfill()
 
-            })
+            }
             prefetcher.start()
         }
         
-        
         var progressCalledCount = 0
-        let prefetcher = ImagePrefetcher(urls: urls, options: [.processor(p)],
-                                         progressBlock: { skippedResources, failedResources, completedResources in
-                                            progressCalledCount += 1
-        },
-                                         completionHandler: { skippedResources, failedResources, completedResources in
-                                            
-                                            XCTAssertEqual(skippedResources.count, 0, "There should be no items skipped.")
-                                            XCTAssertEqual(failedResources.count, 0, "There should be no failed downloading.")
-                                            XCTAssertEqual(completedResources.count, urls.count, "All resources prefetching should be completed.")
-                                            XCTAssertEqual(progressCalledCount, urls.count, "Progress should be called the same time of download count.")
-                                            for url in urls {
-                                                XCTAssertTrue(KingfisherManager.shared.cache.imageCachedType(forKey: url.absoluteString, processorIdentifier: p.identifier).cached)
-                                            }
+        let prefetcher = ImagePrefetcher(
+            urls: testURLs,
+            options: [.processor(p), .waitForCache],
+            progressBlock: { _, _, _ in progressCalledCount += 1 })
+        {
+            skippedResources, failedResources, completedResources in
                                             
-                                            prefetchAgain()
-        })
+            XCTAssertEqual(skippedResources.count, 0)
+            XCTAssertEqual(failedResources.count, 0)
+            XCTAssertEqual(completedResources.count, testURLs.count)
+            XCTAssertEqual(progressCalledCount, testURLs.count)
+            for url in testURLs {
+                let cached = KingfisherManager.shared.cache.imageCachedType(
+                    forKey: url.absoluteString, processorIdentifier: p.identifier).cached
+                XCTAssertTrue(cached)
+            }
+            
+            prefetchAgain()
+        }
         prefetcher.start()
-        
-        waitForExpectations(timeout: 5, handler: nil)
+        waitForExpectations(timeout: 1, handler: nil)
     }
 }