From 2de05818b82052e5c2564475125fbe2c5f79278a Mon Sep 17 00:00:00 2001 From: onevcat Date: Tue, 14 Mar 2023 23:03:47 +0900 Subject: [PATCH 1/2] Expose the receive response delegate --- Sources/General/KingfisherError.swift | 10 ++++++- Sources/Networking/ImageDownloader.swift | 3 ++ .../Networking/ImageDownloaderDelegate.swift | 30 +++++++++++++++++++ Sources/Networking/SessionDelegate.swift | 11 ++++++- 4 files changed, 52 insertions(+), 2 deletions(-) diff --git a/Sources/General/KingfisherError.swift b/Sources/General/KingfisherError.swift index 83d20a1a4..622f63354 100644 --- a/Sources/General/KingfisherError.swift +++ b/Sources/General/KingfisherError.swift @@ -88,6 +88,10 @@ public enum KingfisherError: Error { /// The task is done but no URL response found. Code 2005. /// - task: The failed task. case noURLResponse(task: SessionDataTask) + + /// The task is cancelled by `ImageDownloaderDelegate` due to the `.cancel` response disposition is + /// specified by the delegate method. Code 2006. + case cancelledByDelegate(response: URLResponse) } /// Represents the error reason during Kingfisher caching system. @@ -345,7 +349,10 @@ extension KingfisherError.ResponseErrorReason { case .dataModifyingFailed(let task): return "The data modifying delegate returned `nil` for the downloaded data. Task: \(task)." case .noURLResponse(let task): - return "No URL response received. Task: \(task)," + return "No URL response received. Task: \(task)." + case .cancelledByDelegate(let response): + return "The downloading task is cancelled by the downloader delegate. Response: \(response)." + } } @@ -356,6 +363,7 @@ extension KingfisherError.ResponseErrorReason { case .URLSessionError: return 2003 case .dataModifyingFailed: return 2004 case .noURLResponse: return 2005 + case .cancelledByDelegate: return 2006 } } } diff --git a/Sources/Networking/ImageDownloader.swift b/Sources/Networking/ImageDownloader.swift index 2ab9a5f11..61b0f66d3 100644 --- a/Sources/Networking/ImageDownloader.swift +++ b/Sources/Networking/ImageDownloader.swift @@ -184,6 +184,9 @@ open class ImageDownloader { sessionDelegate.onValidStatusCode.delegate(on: self) { (self, code) in return (self.delegate ?? self).isValidStatusCode(code, for: self) } + sessionDelegate.onResponseReceived.delegate(on: self) { (self, invoke) in + (self.delegate ?? self).imageDownloader(self, didReceive: invoke.0, completionHandler: invoke.1) + } sessionDelegate.onDownloadingFinished.delegate(on: self) { (self, value) in let (url, result) = value do { diff --git a/Sources/Networking/ImageDownloaderDelegate.swift b/Sources/Networking/ImageDownloaderDelegate.swift index 7f986402b..f844ddfc6 100644 --- a/Sources/Networking/ImageDownloaderDelegate.swift +++ b/Sources/Networking/ImageDownloaderDelegate.swift @@ -116,6 +116,28 @@ public protocol ImageDownloaderDelegate: AnyObject { /// - Note: If the default 200 to 400 valid code does not suit your need, /// you can implement this method to change that behavior. func isValidStatusCode(_ code: Int, for downloader: ImageDownloader) -> Bool + + /// Called when the task has received a valid HTTP response after it passes other checks such as the status code. + /// You can perform additional checks or verification on the response to determine if the download should be allowed. + /// + /// For example, it is useful if you want to verify some header values in the response before actually starting the + /// download. + /// + /// If implemented, it is your responsibility to call the `completionHandler` with a proper response disposition, + /// such as `.allow` to start the actual downloading or `.cancel` to cancel the task. If `.cancel` is used as the + /// disposition, the downloader will raise an `KingfisherError` with + /// `ResponseErrorReason.cancelledByDelegate` as its reason. If not implemented, any response which passes other + /// checked will be allowed and the download starts. + /// + /// - Parameters: + /// - downloader: The `ImageDownloader` object which is used for the downloading operation. + /// - response: The original response object of the downloading process. + /// - completionHandler: A completion handler that receives the disposition for the download task. You must call + /// this handler with either `.allow` or `.cancel`. + func imageDownloader( + _ downloader: ImageDownloader, + didReceive response: URLResponse, + completionHandler: @escaping (URLSession.ResponseDisposition) -> Void) } // Default implementation for `ImageDownloaderDelegate`. @@ -151,4 +173,12 @@ extension ImageDownloaderDelegate { public func imageDownloader(_ downloader: ImageDownloader, didDownload data: Data, for url: URL) -> Data? { return data } + + public func imageDownloader( + _ downloader: ImageDownloader, + didReceive response: URLResponse, + completionHandler: @escaping (URLSession.ResponseDisposition) -> Void) { + completionHandler(.allow) + } + } diff --git a/Sources/Networking/SessionDelegate.swift b/Sources/Networking/SessionDelegate.swift index d2582fff3..a4f6bf480 100644 --- a/Sources/Networking/SessionDelegate.swift +++ b/Sources/Networking/SessionDelegate.swift @@ -47,6 +47,7 @@ open class SessionDelegate: NSObject { private let lock = NSLock() let onValidStatusCode = Delegate() + let onResponseReceived = Delegate<(URLResponse, (URLSession.ResponseDisposition) -> Void), Void>() let onDownloadingFinished = Delegate<(URL, Result), Void>() let onDidDownloadData = Delegate() @@ -169,7 +170,15 @@ extension SessionDelegate: URLSessionDataDelegate { completionHandler(.cancel) return } - completionHandler(.allow) + + let inspectedHandler: (URLSession.ResponseDisposition) -> Void = { disposition in + if disposition == .cancel { + let error = KingfisherError.responseError(reason: .cancelledByDelegate(response: response)) + self.onCompleted(task: dataTask, result: .failure(error)) + } + completionHandler(disposition) + } + onResponseReceived.call((response, inspectedHandler)) } open func urlSession(_ session: URLSession, dataTask: URLSessionDataTask, didReceive data: Data) { From 3f149cc9d92a42574c7c7c70705dd46e36cba13c Mon Sep 17 00:00:00 2001 From: onevcat Date: Thu, 16 Mar 2023 22:43:00 +0900 Subject: [PATCH 2/2] Add receive response delegate for response disposition control --- .../ImageDownloaderTests.swift | 89 ++++++++++++++++++- Tests/KingfisherTests/Utils/StubHelpers.swift | 21 ++++- 2 files changed, 104 insertions(+), 6 deletions(-) diff --git a/Tests/KingfisherTests/ImageDownloaderTests.swift b/Tests/KingfisherTests/ImageDownloaderTests.swift index a8dec6734..a0b0b42e4 100644 --- a/Tests/KingfisherTests/ImageDownloaderTests.swift +++ b/Tests/KingfisherTests/ImageDownloaderTests.swift @@ -547,9 +547,9 @@ class ImageDownloaderTests: XCTestCase { func testSessionDelegate() { - class ExtensionDelegate:SessionDelegate { + class ExtensionDelegate: SessionDelegate { //'exp' only for test - public let exp:XCTestExpectation + public let exp: XCTestExpectation init(_ expectation:XCTestExpectation) { exp = expectation } @@ -566,6 +566,78 @@ class ImageDownloaderTests: XCTestCase { } waitForExpectations(timeout: 3, handler: nil) } + + func testDownloaderReceiveResponsePass() { + + let exp = expectation(description: #function) + + let url = testURLs[0] + stub(url, data: testImageData, headers: ["someKey": "someValue"]) + + let handler = TaskResponseCompletion() + let obj = NSObject() + handler.onReceiveResponse.delegate(on: obj) { (obj, response) in + guard let httpResponse = response as? HTTPURLResponse else { + XCTFail("Should be an HTTP response.") + return .cancel + } + XCTAssertEqual(httpResponse.statusCode, 200) + XCTAssertEqual(httpResponse.url, url) + XCTAssertEqual(httpResponse.allHeaderFields["someKey"] as? String, "someValue") + + return .allow + } + + downloader.delegate = handler + downloader.downloadImage(with: url) { result in + XCTAssertNotNil(result.value) + XCTAssertNil(result.error) + + self.downloader.delegate = nil + // hold delegate + _ = handler + exp.fulfill() + } + waitForExpectations(timeout: 3, handler: nil) + } + + func testDownloaderReceiveResponseFailure() { + let exp = expectation(description: #function) + + let url = testURLs[0] + stub(url, data: testImageData, headers: ["someKey": "someValue"]) + + let handler = TaskResponseCompletion() + let obj = NSObject() + handler.onReceiveResponse.delegate(on: obj) { (obj, response) in + guard let httpResponse = response as? HTTPURLResponse else { + XCTFail("Should be an HTTP response.") + return .cancel + } + XCTAssertEqual(httpResponse.statusCode, 200) + XCTAssertEqual(httpResponse.url, url) + XCTAssertEqual(httpResponse.allHeaderFields["someKey"] as? String, "someValue") + + return .cancel + } + + downloader.delegate = handler + downloader.downloadImage(with: url) { result in + XCTAssertNil(result.value) + XCTAssertNotNil(result.error) + + if case .responseError(reason: .cancelledByDelegate) = result.error! { + } else { + XCTFail() + } + + self.downloader.delegate = nil + // hold delegate + _ = handler + exp.fulfill() + } + waitForExpectations(timeout: 3, handler: nil) + } } class URLNilDataModifier: ImageDownloaderDelegate { @@ -580,6 +652,19 @@ class TaskNilDataModifier: ImageDownloaderDelegate { } } +class TaskResponseCompletion: ImageDownloaderDelegate { + + let onReceiveResponse = Delegate() + + func imageDownloader( + _ downloader: ImageDownloader, + didReceive response: URLResponse, + completionHandler: @escaping (URLSession.ResponseDisposition) -> Void + ) { + completionHandler(onReceiveResponse.call(response)!) + } +} + class URLModifier: ImageDownloadRequestModifier { var url: URL? = nil func modified(for request: URLRequest) -> URLRequest? { diff --git a/Tests/KingfisherTests/Utils/StubHelpers.swift b/Tests/KingfisherTests/Utils/StubHelpers.swift index b50406c9c..ce266df1e 100644 --- a/Tests/KingfisherTests/Utils/StubHelpers.swift +++ b/Tests/KingfisherTests/Utils/StubHelpers.swift @@ -27,16 +27,29 @@ import Foundation @discardableResult -func stub(_ url: URL, data: Data, statusCode: Int = 200, length: Int? = nil) -> LSStubResponseDSL { - var stubResult = stubRequest("GET", url.absoluteString as NSString).andReturn(statusCode)?.withBody(data as NSData) +func stub(_ url: URL, + data: Data, + statusCode: Int = 200, + length: Int? = nil, + headers: [String: String] = [:] +) -> LSStubResponseDSL { + var stubResult = stubRequest("GET", url.absoluteString as NSString) + .andReturn(statusCode)? + .withHeaders(headers)? + .withBody(data as NSData) if let length = length { stubResult = stubResult?.withHeader("Content-Length", "\(length)") } return stubResult! } -func delayedStub(_ url: URL, data: Data, statusCode: Int = 200, length: Int? = nil) -> LSStubResponseDSL { - let result = stub(url, data: data, statusCode: statusCode, length: length) +func delayedStub(_ url: URL, + data: Data, + statusCode: Int = 200, + length: Int? = nil, + headers: [String: String] = [:] +) -> LSStubResponseDSL { + let result = stub(url, data: data, statusCode: statusCode, length: length, headers: headers) return result.delay()! }