Skip to content

Commit

Permalink
Fix completion queue bug in ImageDownloader (#163)
Browse files Browse the repository at this point in the history
### 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.
  • Loading branch information
eneko authored Jul 8, 2021
1 parent fdf1e5c commit f070673
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 2 deletions.
4 changes: 2 additions & 2 deletions Sources/Conduit/Networking/Images/ImageDownloader.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
}
}
Expand Down
22 changes: 22 additions & 0 deletions Tests/ConduitTests/Networking/Images/ImageDownloaderTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down

0 comments on commit f070673

Please sign in to comment.