From f0706731744079d7d9efd876cd51aab537ebd558 Mon Sep 17 00:00:00 2001 From: Eneko Alonso Date: Thu, 8 Jul 2021 11:56:01 -0700 Subject: [PATCH] Fix completion queue bug in ImageDownloader (#163) ### Summary In most cases, `ImageDownloader` will be executed from the main thread. However, in scenarios where this might not be the case, we'd want to make sure the completion queue is executed in the `.current` queue (when no custom queue is specified). ### Implementation Store `.current` queue outside of any closures to ensure the queue is the same the method was called in. --- .../Networking/Images/ImageDownloader.swift | 4 ++-- .../Images/ImageDownloaderTests.swift | 22 +++++++++++++++++++ 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/Sources/Conduit/Networking/Images/ImageDownloader.swift b/Sources/Conduit/Networking/Images/ImageDownloader.swift index 6cf78db..bb390df 100644 --- a/Sources/Conduit/Networking/Images/ImageDownloader.swift +++ b/Sources/Conduit/Networking/Images/ImageDownloader.swift @@ -91,6 +91,7 @@ public final class ImageDownloader: ImageDownloaderType { @discardableResult public func downloadImage(for request: URLRequest, completion: @escaping CompletionHandler) -> SessionTaskProxyType? { var proxy: SessionTaskProxyType? + let completionQueue = self.completionQueue ?? .current ?? .main serialQueue.sync { [weak self] in guard let `self` = self else { @@ -133,10 +134,9 @@ public final class ImageDownloader: ImageDownloaderType { } let response = Response(image: image, error: error, urlResponse: response, isFromCache: false) - let queue = strongSelf.completionQueue ?? .current ?? .main func execute(handler: @escaping CompletionHandler) { - queue.addOperation { + completionQueue.addOperation { handler(response) } } diff --git a/Tests/ConduitTests/Networking/Images/ImageDownloaderTests.swift b/Tests/ConduitTests/Networking/Images/ImageDownloaderTests.swift index 493c6c8..70e5dfe 100644 --- a/Tests/ConduitTests/Networking/Images/ImageDownloaderTests.swift +++ b/Tests/ConduitTests/Networking/Images/ImageDownloaderTests.swift @@ -134,6 +134,28 @@ class ImageDownloaderTests: XCTestCase { waitForExpectations(timeout: 5) } + func testCurrentOperationQueue() throws { + // GIVEN an operation queue + let expectedQueue = OperationQueue() + + // AND a configured Image Downloader instance + let imageDownloadedExpectation = expectation(description: "image downloaded") + let sut = ImageDownloader(cache: AutoPurgingURLImageCache()) + let url = try URL(absoluteString: "https://httpbin.org/image/jpeg") + let imageRequest = URLRequest(url: url) + + // WHEN downloading an image from our background queue + expectedQueue.addOperation { + sut.downloadImage(for: imageRequest) { _ in + // THEN the completion handler is called in the expected queue + XCTAssertEqual(OperationQueue.current, expectedQueue) + imageDownloadedExpectation.fulfill() + } + } + + waitForExpectations(timeout: 5) + } + func testCustomOperationQueue() throws { // GIVEN a custom operation queue let customQueue = OperationQueue()