From 5a6cff01930ccc9e179c11feb7ed5718c3420203 Mon Sep 17 00:00:00 2001 From: Ellen Shapiro Date: Tue, 2 Jul 2019 10:03:11 +0200 Subject: [PATCH] Add request completion delegate, rename pre-flight delegate --- Sources/Apollo/GraphQLHTTPRequestError.swift | 2 +- Sources/Apollo/HTTPNetworkTransport.swift | 43 +++++++++++++++--- Tests/ApolloTests/HTTPTransportTests.swift | 46 +++++++++++++++++--- 3 files changed, 79 insertions(+), 12 deletions(-) diff --git a/Sources/Apollo/GraphQLHTTPRequestError.swift b/Sources/Apollo/GraphQLHTTPRequestError.swift index 9b8dd7156e..87d9de9ee9 100644 --- a/Sources/Apollo/GraphQLHTTPRequestError.swift +++ b/Sources/Apollo/GraphQLHTTPRequestError.swift @@ -7,7 +7,7 @@ public enum GraphQLHTTPRequestError: Error, LocalizedError { public var errorDescription: String? { switch self { case .cancelledByDeveloper: - return "The request was cancelled by the developer using the HTTPNetworkTransportDelegate." + return "The request was cancelled by the developer using the HTTPNetworkTransportPreflightDelegate." case .serializedBodyMessageError: return "JSONSerialization error: Error while serializing request's body" case .serializedQueryParamsMessageError: diff --git a/Sources/Apollo/HTTPNetworkTransport.swift b/Sources/Apollo/HTTPNetworkTransport.swift index cf3df65126..92bb479cf7 100644 --- a/Sources/Apollo/HTTPNetworkTransport.swift +++ b/Sources/Apollo/HTTPNetworkTransport.swift @@ -1,6 +1,7 @@ import Foundation -public protocol HTTPNetworkTransportDelegate: class { +/// Methods which will be called prior to a request being sent to the server. +public protocol HTTPNetworkTransportPreflightDelegate: class { /// Called when a request is about to send, to validate that it should be sent. /// Good for early-exiting if your user is not logged in, for example. @@ -20,13 +21,33 @@ public protocol HTTPNetworkTransportDelegate: class { func networkTransport(_ networkTransport: HTTPNetworkTransport, willSend request: inout URLRequest) } +// MARK: - + +/// Methods which will be called after some kind of response has been received to a `URLSessionTask`. +public protocol HTTPNetworkTransportTaskCompletedDelegate { + + /// A callback to allow hooking in URL session responses for things like logging and examining headers. + /// NOTE: This will call back on whatever thread the URL session calls back on, which is never the main thread. Call `DispatchQueue.main.async` before touching your UI! + /// + /// - Parameters: + /// - networkTransport: The network transport that completed a task + /// - request: The request which was completed by the task + /// - data: [optional] Any data received. Passed through from `URLSession`. + /// - response: [optional] Any response received. Passed through from `URLSession`. + /// - error: [optional] Any error received. Passed through from `URLSession`. + func networkTransport(_ networkTransport: HTTPNetworkTransport, completedRequest request: URLRequest, withData data: Data?, response: URLResponse?, error: Error?) +} + +// MARK: - + /// A network transport that uses HTTP POST requests to send GraphQL operations to a server, and that uses `URLSession` as the networking implementation. public class HTTPNetworkTransport: NetworkTransport { let url: URL let session: URLSession let serializationFormat = JSONSerializationFormat.self let useGETForQueries: Bool - let delegate: HTTPNetworkTransportDelegate? + let preflightDelegate: HTTPNetworkTransportPreflightDelegate? + let taskCompletedDelegate: HTTPNetworkTransportTaskCompletedDelegate? /// Creates a network transport with the specified server URL and session configuration. /// @@ -35,16 +56,20 @@ public class HTTPNetworkTransport: NetworkTransport { /// - configuration: A session configuration used to configure the session. Defaults to `URLSessionConfiguration.default`. /// - sendOperationIdentifiers: Whether to send operation identifiers rather than full operation text, for use with servers that support query persistence. Defaults to false. /// - useGETForQueries: If query operation should be sent using GET instead of POST. Defaults to false. + /// - preflightDelegate: A delegate to check with before sending a request. + /// - requestCompletionDelegate: A delegate to notify when the URLSessionTask has completed. public init(url: URL, configuration: URLSessionConfiguration = .default, sendOperationIdentifiers: Bool = false, useGETForQueries: Bool = false, - delegate: HTTPNetworkTransportDelegate? = nil) { + preflightDelegate: HTTPNetworkTransportPreflightDelegate? = nil, + requestCompletionDelegate: HTTPNetworkTransportTaskCompletedDelegate? = nil) { self.url = url self.session = URLSession(configuration: configuration) self.sendOperationIdentifiers = sendOperationIdentifiers self.useGETForQueries = useGETForQueries - self.delegate = delegate + self.preflightDelegate = preflightDelegate + self.taskCompletedDelegate = requestCompletionDelegate } /// Send a GraphQL operation to a server and return a response. @@ -65,6 +90,14 @@ public class HTTPNetworkTransport: NetworkTransport { } let task = session.dataTask(with: request) { (data: Data?, response: URLResponse?, error: Error?) in + if let requestCompletion = self.taskCompletedDelegate { + requestCompletion.networkTransport(self, + completedRequest: request, + withData: data, + response: response, + error: error) + } + if error != nil { completionHandler(nil, error) return @@ -126,7 +159,7 @@ public class HTTPNetworkTransport: NetworkTransport { request.setValue("application/json", forHTTPHeaderField: "Content-Type") // If there's a delegate, do a pre-flight check and allow modifications to the request. - if let delegate = self.delegate { + if let delegate = self.preflightDelegate { guard delegate.networkTransport(self, shouldSend: request) else { throw GraphQLHTTPRequestError.cancelledByDeveloper } diff --git a/Tests/ApolloTests/HTTPTransportTests.swift b/Tests/ApolloTests/HTTPTransportTests.swift index f9987b0f9e..7d71196585 100644 --- a/Tests/ApolloTests/HTTPTransportTests.swift +++ b/Tests/ApolloTests/HTTPTransportTests.swift @@ -14,13 +14,19 @@ class HTTPTransportTests: XCTestCase { private var updatedHeaders: [String: String]? private var shouldSend = true + + private var completedRequest: URLRequest? + private var completedData: Data? + private var completedResponse: URLResponse? + private var completedError: Error? private lazy var url = URL(string: "http://localhost:8080/graphql")! private lazy var networkTransport = HTTPNetworkTransport(url: self.url, useGETForQueries: true, - delegate: self) + preflightDelegate: self, + requestCompletionDelegate: self) - func testDelegateTellingRequestNotToSend() { + func testPreflightDelegateTellingRequestNotToSend() { self.shouldSend = false let expectation = self.expectation(description: "Send operation completed") @@ -44,7 +50,7 @@ class HTTPTransportTests: XCTestCase { } } - guard (cancellable as? ErrorCancellable) != nil else { + guard (cancellable as? EmptyCancellable) != nil else { XCTFail("Wrong cancellable type returned!") cancellable.cancel() expectation.fulfill() @@ -53,9 +59,15 @@ class HTTPTransportTests: XCTestCase { // This should fail without hitting the network. self.wait(for: [expectation], timeout: 1) + + // The request shouldn't have fired, so all these objects should be nil + XCTAssertNil(self.completedRequest) + XCTAssertNil(self.completedData) + XCTAssertNil(self.completedResponse) + XCTAssertNil(self.completedError) } - func testDelgateModifyingRequest() { + func testPreflightDelgateModifyingRequest() { self.updatedHeaders = ["Authorization": "Bearer HelloApollo"] let expectation = self.expectation(description: "Send operation completed") @@ -100,9 +112,15 @@ class HTTPTransportTests: XCTestCase { // This will come through after hitting the network. self.wait(for: [expectation], timeout: 10) + + // We should have everything except an error since the request should have proceeded + XCTAssertNotNil(self.completedRequest) + XCTAssertNotNil(self.completedData) + XCTAssertNotNil(self.completedResponse) + XCTAssertNil(self.completedError) } - func testDelegateNeitherModifyingOrStoppingRequest() { + func testPreflightDelegateNeitherModifyingOrStoppingRequest() { let expectation = self.expectation(description: "Send operation completed") let cancellable = self.networkTransport.send(operation: HeroNameQuery()) { (response, error) in @@ -145,10 +163,16 @@ class HTTPTransportTests: XCTestCase { // This will come through after hitting the network. self.wait(for: [expectation], timeout: 10) + + // We should have everything except an error since the request should have proceeded + XCTAssertNotNil(self.completedRequest) + XCTAssertNotNil(self.completedData) + XCTAssertNotNil(self.completedResponse) + XCTAssertNil(self.completedError) } } -extension HTTPTransportTests: HTTPNetworkTransportDelegate { +extension HTTPTransportTests: HTTPNetworkTransportPreflightDelegate { func networkTransport(_ networkTransport: HTTPNetworkTransport, shouldSend request: URLRequest) -> Bool { return self.shouldSend } @@ -167,3 +191,13 @@ extension HTTPTransportTests: HTTPNetworkTransportDelegate { } } } + +extension HTTPTransportTests: HTTPNetworkTransportTaskCompletedDelegate { + + func networkTransport(_ networkTransport: HTTPNetworkTransport, completedRequest request: URLRequest, withData data: Data?, response: URLResponse?, error: Error?) { + self.completedRequest = request + self.completedData = data + self.completedResponse = response + self.completedError = error + } +}