Update for UIButton related tests

Kingfisher.xcodeproj/project.pbxproj

@@ -1556,7 +1556,7 @@
 				SWIFT_OPTIMIZATION_LEVEL = "-Onone";
 				SWIFT_VERSION = 3.0;
 				TARGETED_DEVICE_FAMILY = "1,2";
-				TVOS_DEPLOYMENT_TARGET = 9.0;
+				TVOS_DEPLOYMENT_TARGET = 10.0;
 				WATCHOS_DEPLOYMENT_TARGET = 3.0;
 			};
 			name = Debug;
@@ -1607,7 +1607,7 @@
 				SWIFT_OPTIMIZATION_LEVEL = "-Owholemodule";
 				SWIFT_VERSION = 3.0;
 				TARGETED_DEVICE_FAMILY = "1,2";
-				TVOS_DEPLOYMENT_TARGET = 9.0;
+				TVOS_DEPLOYMENT_TARGET = 10.0;
 				VALIDATE_PRODUCT = YES;
 				WATCHOS_DEPLOYMENT_TARGET = 3.0;
 			};

Kingfisher.xcodeproj/xcshareddata/xcschemes/Kingfisher.xcscheme

@@ -38,6 +38,11 @@
                BlueprintName = "KingfisherTests"
                ReferencedContainer = "container:Kingfisher.xcodeproj">
             </BuildableReference>
+            <SkippedTests>
+               <Test
+                  Identifier = "ImagePrefetcherTests">
+               </Test>
+            </SkippedTests>
          </TestableReference>
       </Testables>
       <MacroExpansion>

Sources/Extensions/UIButton+Kingfisher.swift

@@ -56,12 +56,12 @@ extension KingfisherClass where Base: UIButton {
                          placeholder: UIImage? = nil,
                          options: KingfisherOptionsInfo? = nil,
                          progressBlock: DownloadProgressBlock? = nil,
-                         completionHandler: CompletionHandler? = nil) -> DownloadTask?
+                         completionHandler: ((Result<RetrieveImageResult>) -> Void)? = nil) -> DownloadTask?
     {
         guard let resource = resource else {
             base.setImage(placeholder, for: state)
             setWebURL(nil, for: state)
-            completionHandler?(nil, nil, .none, nil)
+            completionHandler?(.failure(KingfisherError2.imageSettingError(reason: .emptyResource)))
             return nil
         }
         
@@ -75,25 +75,29 @@ extension KingfisherClass where Base: UIButton {
             with: resource,
             options: options,
             progressBlock: { receivedSize, totalSize in
-                guard resource.downloadURL == self.webURL(for: state) else {
-                    return
-                }
+                guard resource.downloadURL == self.webURL(for: state) else { return }
                 if let progressBlock = progressBlock {
                     progressBlock(receivedSize, totalSize)
                 }
             },
-            completionHandler: { [weak base] image, error, cacheType, imageURL in
+            completionHandler: { result in
                 DispatchQueue.main.safeAsync {
-                    guard let strongBase = base, imageURL == self.webURL(for: state) else {
-                        completionHandler?(image, error, cacheType, imageURL)
+                    guard resource.downloadURL == self.webURL(for: state) else {
+                        let error = KingfisherError2.imageSettingError(
+                            reason: .notCurrentResource(result: result, resource: resource))
+                        completionHandler?(.failure(error))
                         return
                     }
+                    
                     self.setImageTask(nil)
-                    if image != nil {
-                        strongBase.setImage(image, for: state)
-                    }
 
-                    completionHandler?(image, error, cacheType, imageURL)
+                    switch result {
+                    case .success(let value):
+                        self.base.setImage(value.image, for: state)
+                        completionHandler?(result)
+                    case .failure:
+                        completionHandler?(result)
+                    }
                 }
             })
         
@@ -106,8 +110,7 @@ extension KingfisherClass where Base: UIButton {
      Nothing will happen if the downloading has already finished.
      */
     public func cancelImageDownloadTask() {
-        #warning("Cancel proper task with token")
-//        imageTask?.cancel()
+        imageTask?.cancel()
     }
     
     /**
@@ -135,12 +138,12 @@ extension KingfisherClass where Base: UIButton {
                                    placeholder: UIImage? = nil,
                                    options: KingfisherOptionsInfo? = nil,
                                    progressBlock: DownloadProgressBlock? = nil,
-                                   completionHandler: CompletionHandler? = nil) -> DownloadTask?
+                                   completionHandler: ((Result<RetrieveImageResult>) -> Void)? = nil) -> DownloadTask?
     {
         guard let resource = resource else {
             base.setBackgroundImage(placeholder, for: state)
             setBackgroundWebURL(nil, for: state)
-            completionHandler?(nil, nil, .none, nil)
+            completionHandler?(.failure(KingfisherError2.imageSettingError(reason: .emptyResource)))
             return nil
         }
         
@@ -161,17 +164,24 @@ extension KingfisherClass where Base: UIButton {
                     progressBlock(receivedSize, totalSize)
                 }
             },
-            completionHandler: { [weak base] image, error, cacheType, imageURL in
+            completionHandler: { result in
                 DispatchQueue.main.safeAsync {
-                    guard let strongBase = base, imageURL == self.backgroundWebURL(for: state) else {
-                        completionHandler?(image, error, cacheType, imageURL)
+                    guard resource.downloadURL == self.backgroundWebURL(for: state) else {
+                        let error = KingfisherError2.imageSettingError(
+                            reason: .notCurrentResource(result: result, resource: resource))
+                        completionHandler?(.failure(error))
                         return
                     }
+                    
                     self.setBackgroundImageTask(nil)
-                    if image != nil {
-                        strongBase.setBackgroundImage(image, for: state)
+                    
+                    switch result {
+                    case .success(let value):
+                        self.base.setBackgroundImage(value.image, for: state)
+                        completionHandler?(result)
+                    case .failure:
+                        completionHandler?(result)
                     }
-                    completionHandler?(image, error, cacheType, imageURL)
                 }
             })
         
@@ -184,8 +194,7 @@ extension KingfisherClass where Base: UIButton {
      Nothing will happen if the downloading has already finished.
      */
     public func cancelBackgroundImageDownloadTask() {
-        #warning("Cancel proper task with token")
-//        backgroundImageTask?.cancel()
+        backgroundImageTask?.cancel()
     }
 
 }
@@ -195,22 +204,20 @@ private var lastURLKey: Void?
 private var imageTaskKey: Void?
 
 extension KingfisherClass where Base: UIButton {
-    /**
-     Get the image URL binded to this button for a specified state.
-     
-     - parameter state: The state that uses the specified image.
-     
-     - returns: Current URL for image.
-     */
+
+    /// Gets the image URL binded to this button for a specified state.
+    ///
+    /// - Parameter state: The state that uses the specified image.
+    /// - Returns: Current URL for image.
     public func webURL(for state: UIControl.State) -> URL? {
         return webURLs[NSNumber(value:state.rawValue)] as? URL
     }
     
-    fileprivate func setWebURL(_ url: URL?, for state: UIControl.State) {
+    private func setWebURL(_ url: URL?, for state: UIControl.State) {
         webURLs[NSNumber(value:state.rawValue)] = url
     }
     
-    fileprivate var webURLs: NSMutableDictionary {
+    private var webURLs: NSMutableDictionary {
         var dictionary = objc_getAssociatedObject(base, &lastURLKey) as? NSMutableDictionary
         if dictionary == nil {
             dictionary = NSMutableDictionary()

Sources/General/Deprecated.swift

@@ -62,7 +62,7 @@ extension KingfisherClass where Base: Image {
     }
 }
 
-@available(*, deprecated, message: "Use `Result<ImageResult>` based callback instead")
+@available(*, deprecated, message: "Use `Result<RetrieveImageResult>` based callback instead")
 public typealias CompletionHandler = ((_ image: Image?, _ error: NSError?, _ cacheType: CacheType, _ imageURL: URL?) -> Void)
 
 @available(*, deprecated, message: "Use `Result<ImageDownloadResult>` based callback instead")
@@ -153,6 +153,63 @@ extension KingfisherClass where Base: ImageView {
         }
     }
 }
+
+extension KingfisherClass where Base: UIButton {
+    @available(*, deprecated, message: "Use `Result` based callback instead.")
+    @discardableResult
+    public func setImage(
+        with resource: Resource?,
+        for state: UIControl.State,
+        placeholder: UIImage? = nil,
+        options: KingfisherOptionsInfo? = nil,
+        progressBlock: DownloadProgressBlock? = nil,
+        completionHandler: CompletionHandler?) -> DownloadTask?
+    {
+        return setImage(
+            with: resource,
+            for: state,
+            placeholder: placeholder,
+            options: options,
+            progressBlock: progressBlock)
+        {
+            result in
+            switch result {
+            case .success(let value):
+                completionHandler?(value.image, nil, value.cacheType, value.imageURL)
+            case .failure(let error):
+                completionHandler?(nil, error as NSError, .none, nil)
+            }
+        }
+    }
+    
+    @available(*, deprecated, message: "Use `Result` based callback instead.")
+    @discardableResult
+    public func setBackgroundImage(
+        with resource: Resource?,
+        for state: UIControl.State,
+        placeholder: UIImage? = nil,
+        options: KingfisherOptionsInfo? = nil,
+        progressBlock: DownloadProgressBlock? = nil,
+        completionHandler: CompletionHandler?) -> DownloadTask?
+    {
+        return setBackgroundImage(
+            with: resource,
+            for: state,
+            placeholder: placeholder,
+            options: options,
+            progressBlock: progressBlock)
+        {
+            result in
+            switch result {
+            case .success(let value):
+                completionHandler?(value.image, nil, value.cacheType, value.imageURL)
+            case .failure(let error):
+                completionHandler?(nil, error as NSError, .none, nil)
+            }
+        }
+    }
+}
+
 #endif
 
 extension ImageCache {

Sources/Networking/ImageDownloader.swift

@@ -212,7 +212,7 @@ open class ImageDownloader {
             }
         }
 
-        if !task.running {
+        if !task.started {
             delegate?.imageDownloader(self, willDownloadImageForURL: url, with: request)
             task.resume()
         }
@@ -281,7 +281,7 @@ class SessionDelegate: NSObject {
                 let (token, callback) = value
                 let error = KingfisherError2.requestError(reason: .taskCancelled(task: task, token: token))
                 task.onTaskDone.call((.failure(error), [callback]))
-                if !task.running {
+                if !task.containsCallbacks {
                     self.tasks[url] = nil
                 }
             }
@@ -448,7 +448,15 @@ public class SessionDataTask {
     let onTaskDone = Delegate<(Result<(Data, URLResponse?)>, [TaskCallback]), Void>()
     let onTaskCancelled = Delegate<(CancelToken, TaskCallback), Void>()
 
-    var running: Bool { return task.state == .running }
+    var started = false
+    var containsCallbacks: Bool {
+        // We should be able to use `task.state != .running` to check it.
+        // However, in some rare cases, cancelling the task does not change
+        // task state to `.cancelling`, but still in `.running`. So we need
+        // to check callbacks count to for sure that it is safe to remove the
+        // task in delegate.
+        return !callbacks.isEmpty
+    }
     
     init(session: URLSession, request: URLRequest) {
         task = session.dataTask(with: request)
@@ -474,6 +482,7 @@ public class SessionDataTask {
     }
     
     func resume() {
+        started = true
         task.resume()
     }
 

Tests/KingfisherTests/ImageCacheTests.swift

@@ -66,11 +66,11 @@ class ImageCacheTests: XCTestCase {
             for: .cachesDirectory, in: .userDomainMask, appropriateFor: nil, create: true)
         let subFolder = cacheURL.appendingPathComponent("temp")
 
-        let customPath = subFolder.absoluteString
+        let customPath = subFolder.path
         let cache = try! ImageCache(name: "test", path: customPath)
         XCTAssertEqual(
-            cache.diskStorage.directoryURL.absoluteString,
-            customPath + "com.onevcat.Kingfisher.ImageCache.test/")
+            cache.diskStorage.directoryURL.path,
+            (customPath as NSString).appendingPathComponent("com.onevcat.Kingfisher.ImageCache.test"))
     }
     
     func testMaxCachePeriodInSecond() {

Tests/KingfisherTests/ImageDownloaderTests.swift

@@ -324,11 +324,14 @@ class ImageDownloaderTests: XCTestCase {
         downloader.downloadImage(with: url) {
             result in
             XCTAssertNotNil(result.value)
+            if let error = result.error {
+                print(error)
+            }
             group.leave()
         }
         
         group.notify(queue: .main, execute: exp.fulfill)
-        waitForExpectations(timeout: 5, handler: nil)
+        waitForExpectations(timeout: 1, handler: nil)
     }
     
     func testDownloadTaskNil() {

Tests/KingfisherTests/ImagePrefetcherTests.swift

@@ -67,7 +67,7 @@ class ImagePrefetcherTests: XCTestCase {
         }
         
         var progressCalledCount = 0
-        let prefetcher = ImagePrefetcher(urls: urls, options: nil,
+        let prefetcher = ImagePrefetcher(urls: urls, options: [.waitForCache],
                             progressBlock: { skippedResources, failedResources, completedResources in
                                 progressCalledCount += 1
                             },
@@ -98,7 +98,7 @@ class ImagePrefetcherTests: XCTestCase {
         }
         
         let maxConcurrentCount = 2
-        let prefetcher = ImagePrefetcher(urls: urls, options: nil,
+        let prefetcher = ImagePrefetcher(urls: urls, options: [.waitForCache],
                             progressBlock: { skippedResources, failedResources, completedResources in
                             },
                             completionHandler: { skippedResources, failedResources, completedResources in
@@ -128,7 +128,7 @@ class ImagePrefetcherTests: XCTestCase {
             urls.append(URL(string: URLString)!)
         }
         
-        let prefetcher = ImagePrefetcher(urls: urls, options: nil, progressBlock: { skippedResources, failedResources, completedResources in
+        let prefetcher = ImagePrefetcher(urls: urls, options: [.waitForCache], progressBlock: { skippedResources, failedResources, completedResources in
 
             }) { skippedResources, failedResources, completedResources in
                 expectation.fulfill()
@@ -174,7 +174,7 @@ class ImagePrefetcherTests: XCTestCase {
     
     func testPrefetchWithWrongInitParameters() {
         let expectation = self.expectation(description: "wait for prefetching images")
-        let prefetcher = ImagePrefetcher(urls: [], options: nil, progressBlock: nil) { skippedResources, failedResources, completedResources in
+        let prefetcher = ImagePrefetcher(urls: [], options: [.waitForCache], progressBlock: nil) { skippedResources, failedResources, completedResources in
             expectation.fulfill()
             
             XCTAssertEqual(skippedResources.count, 0, "There should be no item skipped.")

Tests/KingfisherTests/UIButtonExtensionTests.swift

@@ -38,137 +38,128 @@ class UIButtonExtensionTests: 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.
         button = UIButton()
         KingfisherManager.shared.downloader = ImageDownloader(name: "testDownloader")
+        KingfisherManager.shared.defaultOptions = [.waitForCache]
+        
         cleanDefaultCache()
     }
     
     override func tearDown() {
-        // Put teardown code here. This method is called after the invocation of each test method in the class.
         LSNocilla.sharedInstance().clearStubs()
         button = nil
-        
         cleanDefaultCache()
-        
+        KingfisherManager.shared.defaultOptions = .empty
         super.tearDown()
     }
 
     func testDownloadAndSetImage() {
-        let expectation = self.expectation(description: "wait for downloading image")
-        
-        let URLString = testKeys[0]
-        _ = stubRequest("GET", URLString).andReturn(200)?.withBody(testImageData)
-        let url = URL(string: URLString)!
-        
+        let exp = expectation(description: #function)
+        let url = testURLs[0]
+        stub(url, data: testImageData2, length: 123)
+
         var progressBlockIsCalled = false
         
-        cleanDefaultCache()
-        
-        button.kf.setImage(with: url, for: .highlighted, placeholder: nil, options: nil, progressBlock: { receivedSize, totalSize in
+        button.kf.setImage(with: url, for: .normal, progressBlock: { _, _ in
             progressBlockIsCalled = true
-        }) { image, error, cacheType, imageURL in
-            expectation.fulfill()
+        })
+        {
+            result in
+            XCTAssertTrue(progressBlockIsCalled)
+            let image = result.value?.image
+            XCTAssertNotNil(image)
+            
+            XCTAssertTrue(image!.renderEqual(to: testImage))
+            XCTAssertTrue(self.button.image(for: .normal)!.renderEqual(to: testImage))
+            
+            XCTAssertEqual(self.button.kf.webURL(for: .normal), result.value!.imageURL)
+            XCTAssertEqual(result.value!.cacheType, .none)
             
-            XCTAssert(progressBlockIsCalled, "progressBlock should be called at least once.")
-            XCTAssert(image != nil, "Downloaded image should exist.")
-            XCTAssert(image! == testImage, "Downloaded image should be the same as test image.")
-            XCTAssert(self.button.image(for: UIControl.State.highlighted)! == testImage, "Downloaded image should be already set to the image for state")
-            XCTAssert(self.button.kf.webURL(for: .highlighted) == imageURL, "Web URL should equal to the downloaded url.")
-            XCTAssert(cacheType == .none, "The cache type should be none here. This image was just downloaded. But now is: \(cacheType)")
+            exp.fulfill()
         }
-        waitForExpectations(timeout: 5, handler: nil)
+        waitForExpectations(timeout: 1, handler: nil)
     }
     
     func testDownloadAndSetBackgroundImage() {
-        let expectation = self.expectation(description: "wait for downloading image")
-        
-        let URLString = testKeys[0]
-        _ = stubRequest("GET", URLString).andReturn(200)?.withBody(testImageData)
-        let url = Foundation.URL(string: URLString)!
+        let exp = expectation(description: #function)
+        let url = testURLs[0]
+        stub(url, data: testImageData2, length: 123)
         
         var progressBlockIsCalled = false
-        button.kf.setBackgroundImage(with: url, for: .normal, placeholder: nil, options: nil, progressBlock: { receivedSize, totalSize in
+        button.kf.setBackgroundImage(with: url, for: .normal, progressBlock: { _, _ in
             progressBlockIsCalled = true
-        }) { image, error, cacheType, imageURL in
-            expectation.fulfill()
-                
-            XCTAssert(progressBlockIsCalled, "progressBlock should be called at least once.")
-            XCTAssert(image != nil, "Downloaded image should exist.")
-            XCTAssert(image! == testImage, "Downloaded image should be the same as test image.")
-            XCTAssert(self.button.backgroundImage(for: .normal)! == testImage, "Downloaded image should be already set to the image for state")
-            XCTAssert(self.button.kf.backgroundWebURL(for: .normal) == imageURL, "Web URL should equal to the downloaded url.")
-            XCTAssert(cacheType == .none, "cacheType should be .None since the image was just downloaded.")
+        })
+        {
+            result in
+    
+            XCTAssertTrue(progressBlockIsCalled)
+            
+            let image = result.value?.image
+            XCTAssertNotNil(image)
+            XCTAssertTrue(image!.renderEqual(to: testImage))
+            XCTAssertTrue(self.button.backgroundImage(for: .normal)!.renderEqual(to: testImage))
+            XCTAssertEqual(result.value!.cacheType, .none)
+            
+            exp.fulfill()
         }
-        waitForExpectations(timeout: 5, handler: nil)
+        waitForExpectations(timeout: 1, handler: nil)
     }
     
     func testCacnelImageTask() {
-        let expectation = self.expectation(description: "wait for downloading image")
-        
-        let URLString = testKeys[0]
-        let stub = stubRequest("GET", URLString).andReturn(200)?.withBody(testImageData)?.delay()
-        let url = URL(string: URLString)!
+        let exp = expectation(description: #function)
+        let url = testURLs[0]
+        let stub = delayedStub(url, data: testImageData2)
 
-        button.kf.setImage(with: url, for: UIControl.State.highlighted, placeholder: nil, options: nil, progressBlock: { receivedSize, totalSize in
-                
-        }) { image, error, cacheType, imageURL in
-            XCTAssertNotNil(error)
-            XCTAssertEqual(error?.code, NSURLErrorCancelled)
-            expectation.fulfill()
-        }
-        delay(0.1) { 
-            self.button.kf.cancelImageDownloadTask()
-            _ = stub!.go()
+        button.kf.setImage(with: url, for: .highlighted) { result in
+            XCTAssertNotNil(result.error)
+            XCTAssertTrue((result.error as! KingfisherError2).isTaskCancelled)
         }
+        
+        self.button.kf.cancelImageDownloadTask()
+        _ = stub.go()
+        
+        delay(0.1) { exp.fulfill() }
 
-        waitForExpectations(timeout: 5, handler: nil)
+        waitForExpectations(timeout: 1, handler: nil)
     }
     
     func testCacnelBackgroundImageTask() {
-        let expectation = self.expectation(description: "wait for downloading image")
-        
-        let URLString = testKeys[0]
-        let stub = stubRequest("GET", URLString).andReturn(200)?.withBody(testImageData)?.delay()
-        let url = URL(string: URLString)!
+        let exp = expectation(description: #function)
+        let url = testURLs[0]
+        let stub = delayedStub(url, data: testImageData2)
         
-        button.kf.setBackgroundImage(with: url, for: UIControl.State(), placeholder: nil, options: nil, progressBlock: { receivedSize, totalSize in
-            XCTFail("Progress block should not be called.")
-        }) { image, error, cacheType, imageURL in
-            XCTAssertNotNil(error)
-            XCTAssertEqual(error?.code, NSURLErrorCancelled)
-                
-            expectation.fulfill()
-        }
-        delay(0.1) { 
-            self.button.kf.cancelBackgroundImageDownloadTask()
-            _ = stub!.go()
+        button.kf.setBackgroundImage(with: url, for: .highlighted) { result in
+            XCTAssertNotNil(result.error)
+            XCTAssertTrue((result.error as! KingfisherError2).isTaskCancelled)
         }
         
-        waitForExpectations(timeout: 5, handler: nil)
+        self.button.kf.cancelBackgroundImageDownloadTask()
+        _ = stub.go()
+        delay(0.1) { exp.fulfill() }
+        
+        waitForExpectations(timeout: 1, handler: nil)
     }
     
     func testSettingNilURL() {
-        let expectation = self.expectation(description: "wait for downloading image")
+        let exp = expectation(description: #function)
         
         let url: URL? = nil
-        button.kf.setBackgroundImage(with: url, for: UIControl.State(), placeholder: nil, options: nil, progressBlock: { receivedSize, totalSize in
-            XCTFail("Progress block should not be called.")
-        }) { image, error, cacheType, imageURL in
-            XCTAssertNil(image)
-            XCTAssertNil(error)
-            XCTAssertEqual(cacheType, CacheType.none)
-            XCTAssertNil(imageURL)
-            
-            expectation.fulfill()
+        button.kf.setBackgroundImage(with: url, for: .normal) { result in
+            XCTAssertNil(result.value)
+            XCTAssertNotNil(result.error)
+            guard case KingfisherError2.imageSettingError(reason: .emptyResource) = result.error! else {
+                XCTFail()
+                return
+            }
+            exp.fulfill()
         }
         
-        waitForExpectations(timeout: 5, handler: nil)
+        waitForExpectations(timeout: 1, handler: nil)
     }
 }