From fee7b8e9a17d5d79233f84f93fe65c3d1e9f05b4 Mon Sep 17 00:00:00 2001 From: onevcat Date: Sat, 4 Feb 2017 15:06:11 +0900 Subject: [PATCH] Modify disk and download task cancel logic Remove cancelable disk task. Change the download task behavior to make it could be cancelled even before the task starts. --- Sources/ImageDownloader.swift | 1 + Sources/ImageView+Kingfisher.swift | 2 +- Sources/KingfisherManager.swift | 16 ++++------------ .../ImageViewExtensionTests.swift | 7 +++++-- 4 files changed, 11 insertions(+), 15 deletions(-) diff --git a/Sources/ImageDownloader.swift b/Sources/ImageDownloader.swift index 7d3f5b003..46039b465 100755 --- a/Sources/ImageDownloader.swift +++ b/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 } diff --git a/Sources/ImageView+Kingfisher.swift b/Sources/ImageView+Kingfisher.swift index 77d94a983..94715068d 100755 --- a/Sources/ImageView+Kingfisher.swift +++ b/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() } } diff --git a/Sources/KingfisherManager.swift b/Sources/KingfisherManager.swift index 0e51e62ce..eb29f3d9d 100755 --- a/Sources/KingfisherManager.swift +++ b/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 } } diff --git a/Tests/KingfisherTests/ImageViewExtensionTests.swift b/Tests/KingfisherTests/ImageViewExtensionTests.swift index 8947ff725..8fedf1dda 100755 --- a/Tests/KingfisherTests/ImageViewExtensionTests.swift +++ b/Tests/KingfisherTests/ImageViewExtensionTests.swift @@ -148,6 +148,7 @@ 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() @@ -155,7 +156,7 @@ class ImageViewExtensionTests: XCTestCase { 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.") }