Merge pull request #578 from onevcat/fix/cancel-behavior

Modify disk and download task cancel logic

Sources/ImageDownloader.swift

@@ -279,6 +279,7 @@ extension ImageDownloader {
               completionHandler: ImageDownloaderCompletionHandler?) -> RetrieveImageDownloadTask?
     {
         if let retrieveImageTask = retrieveImageTask, retrieveImageTask.cancelledBeforeDownloadStarting {
+            completionHandler?(nil, NSError(domain: KingfisherErrorDomain, code: KingfisherError.downloadCancelledBeforeStarting.rawValue, userInfo: nil), nil, nil)
             return nil
         }
 

Sources/ImageView+Kingfisher.swift

@@ -136,7 +136,7 @@ extension Kingfisher where Base: ImageView {
      Nothing will happen if the downloading has already finished.
      */
     public func cancelDownloadTask() {
-        imageTask?.downloadTask?.cancel()
+        imageTask?.cancel()
     }
 }
 

Sources/KingfisherManager.swift

@@ -44,22 +44,17 @@ public class RetrieveImageTask {
     var cancelledBeforeDownloadStarting: Bool = false
 
     /// The disk retrieve task in this image task. Kingfisher will try to look up in cache first. This task represent the cache search task.
+    @available(*, deprecated,
+    message: "diskRetrieveTask is not in use anymore. You cannot cancel a disk retrieve task anymore once it started.")
     public var diskRetrieveTask: RetrieveImageDiskTask?
 
     /// The network retrieve task in this image task.
     public var downloadTask: RetrieveImageDownloadTask?
 
     /**
-    Cancel current task. If this task does not begin or already done, do nothing.
+    Cancel current task. If this task is already done, do nothing.
     */
     public func cancel() {
-        // From Xcode 7 beta 6, the `dispatch_block_cancel` will crash at runtime.
-        // It fixed in Xcode 7.1.
-        // See https://github.com/onevcat/Kingfisher/issues/99 for more.
-        if let diskRetrieveTask = diskRetrieveTask {
-            diskRetrieveTask.cancel()
-        }
-
         if let downloadTask = downloadTask {
             downloadTask.cancel()
         } else {
@@ -184,13 +179,11 @@ public class KingfisherManager {
                                         options: KingfisherOptionsInfo?)
     {
         let diskTaskCompletionHandler: CompletionHandler = { (image, error, cacheType, imageURL) -> () in
-            // Break retain cycle created inside diskTask closure below
-            retrieveImageTask.diskRetrieveTask = nil
             completionHandler?(image, error, cacheType, imageURL)
         }
 
         let targetCache = options?.targetCache ?? cache
-        let diskTask = targetCache.retrieveImage(forKey: key, options: options,
+        targetCache.retrieveImage(forKey: key, options: options,
             completionHandler: { image, cacheType in
                 if image != nil {
                     diskTaskCompletionHandler(image, nil, cacheType, url)
@@ -208,6 +201,5 @@ public class KingfisherManager {
                 }
             }
         )
-        retrieveImageTask.diskRetrieveTask = diskTask
     }
 }

Tests/KingfisherTests/ImageViewExtensionTests.swift

@@ -148,14 +148,15 @@ class ImageViewExtensionTests: XCTestCase {
             progressBlockIsCalled = true
         }) { (image, error, cacheType, imageURL) -> () in
             completionBlockIsCalled = true
+            XCTAssertEqual(error?.code, KingfisherError.downloadCancelledBeforeStarting.rawValue, "The error should be downloadCancelledBeforeStarting")
         }
 
         task.cancel()
 
         DispatchQueue.main.asyncAfter(deadline: DispatchTime.now() + Double(Int64(Double(NSEC_PER_SEC) * 0.09)) / Double(NSEC_PER_SEC)) { () -> Void in
             expectation.fulfill()
             XCTAssert(progressBlockIsCalled == false, "ProgressBlock should not be called since it is canceled.")
-            XCTAssert(completionBlockIsCalled == false, "CompletionBlock should not be called since it is canceled.")
+            XCTAssert(completionBlockIsCalled == true, "CompletionBlock should be called since it is canceled.")
         }
 
         waitForExpectations(timeout: 5, handler: nil)
@@ -209,7 +210,9 @@ class ImageViewExtensionTests: XCTestCase {
         let task1 = imageView.kf.setImage(with: url, placeholder: nil, options: nil, progressBlock: { (receivedSize, totalSize) -> () in
 
             }) { (image, error, cacheType, imageURL) -> () in
+                XCTAssertNil(image)
                 task1Completion = true
+                XCTAssertEqual(error?.code, KingfisherError.downloadCancelledBeforeStarting.rawValue, "The error should be downloadCancelledBeforeStarting")
         }
 
         let _ = imageView.kf.setImage(with: url, placeholder: nil, options: nil, progressBlock: { (receivedSize, totalSize) -> () in
@@ -233,7 +236,7 @@ class ImageViewExtensionTests: XCTestCase {
 
         DispatchQueue.main.asyncAfter(deadline: DispatchTime.now() + Double(Int64(Double(NSEC_PER_SEC) * 0.2)) / Double(NSEC_PER_SEC)) { () -> Void in
             expectation.fulfill()
-            XCTAssert(task1Completion == false, "Task 1 should be not completed since it is cancelled before downloading started.")
+            XCTAssert(task1Completion == true, "Task 1 should be completed.")
             XCTAssert(task2Completion == true, "Task 2 should be completed.")
             XCTAssert(task3Completion == true, "Task 3 should be completed.")
         }