From 78fe423176fb46f2c8fcc1727f7be2b85f9c2cb8 Mon Sep 17 00:00:00 2001
From: Ellen Shapiro <designatednerd@gmail.com>
Date: Fri, 17 Apr 2020 13:27:03 -0500
Subject: [PATCH 01/12] Add URL session client and tests
---
Apollo.xcodeproj/project.pbxproj | 12 ++
Sources/Apollo/URLSessionClient.swift | 197 ++++++++++++++++++
Tests/ApolloTests/HTTPBinAPI.swift | 43 ++++
Tests/ApolloTests/URLSessionClientTests.swift | 129 ++++++++++++
4 files changed, 381 insertions(+)
create mode 100644 Sources/Apollo/URLSessionClient.swift
create mode 100644 Tests/ApolloTests/HTTPBinAPI.swift
create mode 100644 Tests/ApolloTests/URLSessionClientTests.swift
diff --git a/Apollo.xcodeproj/project.pbxproj b/Apollo.xcodeproj/project.pbxproj
index 41890cdde3..8a1fa4948d 100644
--- a/Apollo.xcodeproj/project.pbxproj
+++ b/Apollo.xcodeproj/project.pbxproj
@@ -20,6 +20,9 @@
9B21FD772422C8CC00998B5C /* TestFileHelper.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9B21FD762422C8CC00998B5C /* TestFileHelper.swift */; };
9B21FD782424305700998B5C /* ExpectedEnumWithDifferentCases.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9B68F05F2416F80C00E97318 /* ExpectedEnumWithDifferentCases.swift */; };
9B21FD792424305E00998B5C /* ExpectedEnumWithSanitizedCases.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9B68F063241703B200E97318 /* ExpectedEnumWithSanitizedCases.swift */; };
+ 9B4F453F244A27B900C2CF7D /* URLSessionClient.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9B4F453E244A27B900C2CF7D /* URLSessionClient.swift */; };
+ 9B4F4541244A2A9200C2CF7D /* HTTPBinAPI.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9B4F4540244A2A9200C2CF7D /* HTTPBinAPI.swift */; };
+ 9B4F4543244A2AD300C2CF7D /* URLSessionClientTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9B4F4542244A2AD300C2CF7D /* URLSessionClientTests.swift */; };
9B518C87235F819E004C426D /* CLIDownloader.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9B518C85235F8125004C426D /* CLIDownloader.swift */; };
9B518C8C235F8B5F004C426D /* ApolloFilePathHelper.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9B518C8A235F8B05004C426D /* ApolloFilePathHelper.swift */; };
9B518C8D235F8B9E004C426D /* CLIDownloaderTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9B518C88235F8AD4004C426D /* CLIDownloaderTests.swift */; };
@@ -365,6 +368,9 @@
9B21FD742422C29D00998B5C /* GraphQLFileTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = GraphQLFileTests.swift; sourceTree = "<group>"; };
9B21FD762422C8CC00998B5C /* TestFileHelper.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TestFileHelper.swift; sourceTree = "<group>"; };
9B4AA8AD239EFDC9003E1300 /* Apollo-Target-CodegenTests.xcconfig */ = {isa = PBXFileReference; lastKnownFileType = text.xcconfig; path = "Apollo-Target-CodegenTests.xcconfig"; sourceTree = "<group>"; };
+ 9B4F453E244A27B900C2CF7D /* URLSessionClient.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = URLSessionClient.swift; sourceTree = "<group>"; };
+ 9B4F4540244A2A9200C2CF7D /* HTTPBinAPI.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = HTTPBinAPI.swift; sourceTree = "<group>"; };
+ 9B4F4542244A2AD300C2CF7D /* URLSessionClientTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = URLSessionClientTests.swift; sourceTree = "<group>"; };
9B518C85235F8125004C426D /* CLIDownloader.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CLIDownloader.swift; sourceTree = "<group>"; };
9B518C88235F8AD4004C426D /* CLIDownloaderTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CLIDownloaderTests.swift; sourceTree = "<group>"; };
9B518C8A235F8B05004C426D /* ApolloFilePathHelper.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ApolloFilePathHelper.swift; sourceTree = "<group>"; };
@@ -727,6 +733,7 @@
C3279FC52345233000224790 /* TestCustomRequestCreator.swift */,
9B64F6752354D219002D1BB5 /* URL+QueryDict.swift */,
9B21FD762422C8CC00998B5C /* TestFileHelper.swift */,
+ 9B4F4540244A2A9200C2CF7D /* HTTPBinAPI.swift */,
);
name = TestHelpers;
sourceTree = "<group>";
@@ -1169,6 +1176,7 @@
9FF90A6B1DDDEB420034C3B6 /* ReadFieldValueTests.swift */,
C338DF1622DD9DE9006AF33E /* RequestCreatorTests.swift */,
9F19D8451EED8D3B00C57247 /* ResultOrPromiseTests.swift */,
+ 9B4F4542244A2AD300C2CF7D /* URLSessionClientTests.swift */,
5BB2C0222380836100774170 /* VersionNumberTests.swift */,
C304EBD322DDC7B200748F72 /* a.txt */,
C35D43BE22DDD3C100BCBABE /* b.txt */,
@@ -1203,6 +1211,7 @@
C377CCAA22D7992E00572E03 /* MultipartFormData.swift */,
9F69FFA81D42855900E000B1 /* NetworkTransport.swift */,
9BEDC79D22E5D2CF00549BF6 /* RequestCreator.swift */,
+ 9B4F453E244A27B900C2CF7D /* URLSessionClient.swift */,
);
name = Network;
sourceTree = "<group>";
@@ -1997,6 +2006,7 @@
5AC6CA4322AAF7B200B7C94D /* GraphQLHTTPMethod.swift in Sources */,
9FE941D01E62C771007CDD89 /* Promise.swift in Sources */,
9BA1245E22DE116B00BF1D24 /* Result+Helpers.swift in Sources */,
+ 9B4F453F244A27B900C2CF7D /* URLSessionClient.swift in Sources */,
9FC750631D2A59F600458D91 /* ApolloClient.swift in Sources */,
9BA3130E2302BEA5007B7FC5 /* DispatchQueue+Optional.swift in Sources */,
9F86B6901E65533D00B885FF /* GraphQLResponseGenerator.swift in Sources */,
@@ -2013,6 +2023,7 @@
9FC9A9C81E2EFE6E0023C4D5 /* CacheKeyForFieldTests.swift in Sources */,
9F91CF8F1F6C0DB2008DD0BE /* MutatingResultsTests.swift in Sources */,
F82E62E122BCD223000C311B /* AutomaticPersistedQueriesTests.swift in Sources */,
+ 9B4F4543244A2AD300C2CF7D /* URLSessionClientTests.swift in Sources */,
9F19D8461EED8D3B00C57247 /* ResultOrPromiseTests.swift in Sources */,
9F533AB31E6C4A4200CBE097 /* BatchedLoadTests.swift in Sources */,
C3279FC72345234D00224790 /* TestCustomRequestCreator.swift in Sources */,
@@ -2030,6 +2041,7 @@
9FF90A711DDDEB420034C3B6 /* ReadFieldValueTests.swift in Sources */,
9F295E311E27534800A24949 /* NormalizeQueryResults.swift in Sources */,
9FF90A731DDDEB420034C3B6 /* ParseQueryResponseTests.swift in Sources */,
+ 9B4F4541244A2A9200C2CF7D /* HTTPBinAPI.swift in Sources */,
9BF1A94F22CA5784005292C2 /* HTTPTransportTests.swift in Sources */,
);
runOnlyForDeploymentPostprocessing = 0;
diff --git a/Sources/Apollo/URLSessionClient.swift b/Sources/Apollo/URLSessionClient.swift
new file mode 100644
index 0000000000..720318e732
--- /dev/null
+++ b/Sources/Apollo/URLSessionClient.swift
@@ -0,0 +1,197 @@
+import Foundation
+
+open class URLSessionClient: NSObject, URLSessionDelegate, URLSessionTaskDelegate, URLSessionDataDelegate {
+
+ public enum URLSessionClientError: Error {
+ case sessionBecameInvalidWithoutUnderlyingError
+ case dataForTaskNotFound(taskIdentifier: Int)
+ }
+
+ public typealias Completion = (Result<(Data, HTTPURLResponse?), Error>) -> Void
+
+ private var completionBlocks = [Int: Completion]()
+ private var datas = [Int: Data]()
+ private var responses = [Int: HTTPURLResponse]()
+
+ open private(set) var session: URLSession!
+
+ public init(sessionConfiguration: URLSessionConfiguration = .default,
+ callbackQueue: OperationQueue? = .main) {
+ super.init()
+ self.session = URLSession(configuration: sessionConfiguration,
+ delegate: self,
+ delegateQueue: callbackQueue)
+ }
+
+ private func clearTask(with identifier: Int) {
+ self.completionBlocks.removeValue(forKey: identifier)
+ self.datas.removeValue(forKey: identifier)
+ self.responses.removeValue(forKey: identifier)
+ }
+
+ private func clearAllTasks() {
+ self.completionBlocks.removeAll()
+ self.datas.removeAll()
+ self.responses.removeAll()
+ }
+
+ @discardableResult
+ open func sendRequest(_ request: URLRequest, completion: @escaping Completion) -> URLSessionTask {
+ let dataTask = self.session.dataTask(with: request)
+ self.completionBlocks[dataTask.taskIdentifier] = completion
+ self.datas[dataTask.taskIdentifier] = Data()
+ dataTask.resume()
+
+ return dataTask
+ }
+
+ open func cancel(task: URLSessionTask) {
+ let taskID = task.taskIdentifier
+ self.clearTask(with: taskID)
+ task.cancel()
+ }
+
+ // MARK: - URLSessionDelegate
+
+ open func urlSession(_ session: URLSession, didBecomeInvalidWithError error: Error?) {
+ let finalError = error ?? URLSessionClientError.sessionBecameInvalidWithoutUnderlyingError
+ for block in completionBlocks.values {
+ block(.failure(finalError))
+ }
+
+ self.clearAllTasks()
+ }
+
+ @available(OSX 10.12, iOS 10.0, *)
+ open func urlSession(_ session: URLSession,
+ task: URLSessionTask,
+ didFinishCollecting metrics: URLSessionTaskMetrics) {
+ // No default implementation
+ }
+
+ open func urlSession(_ session: URLSession,
+ didReceive challenge: URLAuthenticationChallenge,
+ completionHandler: @escaping (URLSession.AuthChallengeDisposition, URLCredential?) -> Void) {
+ completionHandler(.performDefaultHandling, nil)
+ }
+
+
+ #if os(iOS) || os(tvOS) || os(watchOS)
+ open func urlSessionDidFinishEvents(forBackgroundURLSession session: URLSession) {
+ // No default implementation
+ }
+ #endif
+
+ // MARK: - NSURLSessionTaskDelegate
+
+ open func urlSession(_ session: URLSession,
+ task: URLSessionTask,
+ didReceive challenge: URLAuthenticationChallenge,
+ completionHandler: @escaping (URLSession.AuthChallengeDisposition, URLCredential?) -> Void) {
+ completionHandler(.performDefaultHandling, nil)
+ }
+
+ open func urlSession(_ session: URLSession,
+ taskIsWaitingForConnectivity task: URLSessionTask) {
+ // No default implementation
+ }
+
+ open func urlSession(_ session: URLSession,
+ task: URLSessionTask,
+ didCompleteWithError error: Error?) {
+ defer {
+ self.clearTask(with: task.taskIdentifier)
+ }
+
+ guard let completion = self.completionBlocks.removeValue(forKey: task.taskIdentifier) else {
+ // No completion block, the task has likely been cancelled. Bail out.
+ return
+ }
+
+
+ if let finalError = error {
+ completion(.failure(finalError))
+ } else {
+ let taskIdentifier = task.taskIdentifier
+ guard let data = self.datas[taskIdentifier] else {
+ // Data is immediately created for a task on creation, so if it's not there, something's gone wrong.
+ completion(.failure(URLSessionClientError.dataForTaskNotFound(taskIdentifier: taskIdentifier)))
+ return
+ }
+
+ let response = self.responses[taskIdentifier]
+ completion(.success((data, response)))
+ }
+ }
+
+ open func urlSession(_ session: URLSession,
+ task: URLSessionTask,
+ needNewBodyStream completionHandler: @escaping (InputStream?) -> Void) {
+ completionHandler(nil)
+ }
+
+ open func urlSession(_ session: URLSession,
+ task: URLSessionTask,
+ didSendBodyData bytesSent: Int64,
+ totalBytesSent: Int64,
+ totalBytesExpectedToSend: Int64) {
+ // No default implementation
+ }
+
+ @available(iOS 11.0, OSXApplicationExtension 10.13, *)
+ public func urlSession(_ session: URLSession,
+ task: URLSessionTask,
+ willBeginDelayedRequest request: URLRequest,
+ completionHandler: @escaping (URLSession.DelayedRequestDisposition, URLRequest?) -> Void) {
+ completionHandler(.continueLoading, request)
+ }
+
+ public func urlSession(_ session: URLSession,
+ task: URLSessionTask,
+ willPerformHTTPRedirection response: HTTPURLResponse,
+ newRequest request: URLRequest,
+ completionHandler: @escaping (URLRequest?) -> Void) {
+ completionHandler(request)
+ }
+
+ // MARK: - URLSessionDataDelegate
+
+ open func urlSession(_ session: URLSession,
+ dataTask: URLSessionDataTask,
+ didReceive data: Data) {
+ self.datas[dataTask.taskIdentifier]?.append(data)
+ }
+
+ @available(iOS 9.0, OSXApplicationExtension 10.11, *)
+ open func urlSession(_ session: URLSession,
+ dataTask: URLSessionDataTask,
+ didBecome streamTask: URLSessionStreamTask) {
+ // No default implementation
+ }
+
+ open func urlSession(_ session: URLSession,
+ dataTask: URLSessionDataTask,
+ didBecome downloadTask: URLSessionDownloadTask) {
+ // No default implementation
+ }
+
+ open func urlSession(_ session: URLSession,
+ dataTask: URLSessionDataTask,
+ willCacheResponse proposedResponse: CachedURLResponse,
+ completionHandler: @escaping (CachedURLResponse?) -> Void) {
+ completionHandler(proposedResponse)
+ }
+
+ open func urlSession(_ session: URLSession,
+ dataTask: URLSessionDataTask,
+ didReceive response: URLResponse,
+ completionHandler: @escaping (URLSession.ResponseDisposition) -> Void) {
+ defer {
+ completionHandler(.allow)
+ }
+
+ if let httpResponse = response as? HTTPURLResponse {
+ self.responses[dataTask.taskIdentifier] = httpResponse
+ }
+ }
+}
diff --git a/Tests/ApolloTests/HTTPBinAPI.swift b/Tests/ApolloTests/HTTPBinAPI.swift
new file mode 100644
index 0000000000..77eb1921ea
--- /dev/null
+++ b/Tests/ApolloTests/HTTPBinAPI.swift
@@ -0,0 +1,43 @@
+import Foundation
+
+enum HTTPBinAPI {
+ static let baseURL = URL(string: "https://httpbin.org/")!
+ enum Endpoint {
+ case bytes(count: Int)
+ case get
+ case headers
+ case image
+ case post
+
+ var toString: String {
+
+ switch self {
+ case .bytes(let count):
+ return "bytes/\(count)"
+ case .get:
+ return "get"
+ case .headers:
+ return "headers"
+ case .image:
+ return "image/jpeg"
+ case .post:
+ return "post"
+ }
+ }
+
+ var toURL: URL {
+ HTTPBinAPI.baseURL.appendingPathComponent(self.toString)
+ }
+ }
+}
+
+struct HTTPBinResponse: Codable {
+
+ let headers: [String: String]
+ let url: String
+ let json: [String: String]?
+
+ init(data: Data) throws {
+ self = try JSONDecoder().decode(Self.self, from: data)
+ }
+}
diff --git a/Tests/ApolloTests/URLSessionClientTests.swift b/Tests/ApolloTests/URLSessionClientTests.swift
new file mode 100644
index 0000000000..d1beade0a0
--- /dev/null
+++ b/Tests/ApolloTests/URLSessionClientTests.swift
@@ -0,0 +1,129 @@
+import XCTest
+@testable import Apollo
+
+class URLSessionClientLiveTests: XCTestCase {
+
+ lazy var client = URLSessionClient()
+
+ private func request(for endpoint: HTTPBinAPI.Endpoint) -> URLRequest {
+ URLRequest(url: endpoint.toURL,
+ cachePolicy: URLRequest.CachePolicy.reloadIgnoringCacheData,
+ timeoutInterval: 30)
+ }
+
+ func testBasicGet() {
+ let request = self.request(for: .get)
+ let expectation = self.expectation(description: "Basic GET request completed")
+ self.client.sendRequest(request) { result in
+ defer {
+ expectation.fulfill()
+ }
+
+ switch result {
+ case .failure(let error):
+ XCTFail("Unexpected error: \(error)")
+ case .success(let (data, httpResponse)):
+ XCTAssertFalse(data.isEmpty)
+ XCTAssertEqual(request.url, httpResponse?.url)
+ }
+ }
+
+ self.wait(for: [expectation], timeout: 10)
+ }
+
+ func testGettingImage() {
+ let request = self.request(for: .image)
+ let expectation = self.expectation(description: "GET request for image completed")
+ self.client.sendRequest(request) { result in
+ defer {
+ expectation.fulfill()
+ }
+
+ switch result {
+ case .failure(let error):
+ XCTFail("Unexpected error: \(error)")
+ case .success(let (data, httpResponse)):
+ XCTAssertFalse(data.isEmpty)
+ XCTAssertEqual(httpResponse!.allHeaderFields["Content-Type"] as! String, "image/jpeg")
+ let image = UIImage(data: data)
+ XCTAssertNotNil(image)
+ XCTAssertEqual(request.url, httpResponse?.url)
+ }
+ }
+
+ self.wait(for: [expectation], timeout: 10)
+ }
+
+ func testGettingBytes() throws {
+ let randomInt = Int.random(in: 1...102_400) // 102400 is max from HTTPBin
+ let request = self.request(for: .bytes(count: randomInt))
+
+ let expectation = self.expectation(description: "GET request for a random amount of data completed")
+ self.client.sendRequest(request) { result in
+ defer {
+ expectation.fulfill()
+ }
+
+ switch result {
+ case .failure(let error):
+ XCTFail("Unexpected error: \(error)")
+ case .success(let (data, response)):
+ XCTAssertEqual(data.count,
+ randomInt,
+ "Expected \(randomInt) bytes, got \(data.count)")
+ XCTAssertEqual(request.url, response?.url)
+ }
+ }
+
+ self.wait(for: [expectation], timeout: 10)
+ }
+
+ func testPostingJSON() throws {
+ let testJSON = ["key": "value"]
+ let data = try JSONSerialization.data(withJSONObject: testJSON, options: .prettyPrinted)
+
+ var request = self.request(for: .post)
+ request.httpBody = data
+ request.httpMethod = GraphQLHTTPMethod.POST.rawValue
+ request.addValue("application/json", forHTTPHeaderField: "Content-Type")
+
+ let expectation = self.expectation(description: "POST request with JSON completed")
+ self.client.sendRequest(request) { result in
+ defer {
+ expectation.fulfill()
+ }
+
+ switch result {
+ case .failure(let error):
+ XCTFail("Unexpected error: \(error)")
+ case .success(let (data, httpResponse)):
+ XCTAssertEqual(request.url, httpResponse?.url)
+
+ do {
+ let parsed = try HTTPBinResponse(data: data)
+ XCTAssertEqual(parsed.json, testJSON)
+ } catch {
+ XCTFail("Unexpected error: \(error)")
+ }
+ }
+ }
+
+ self.wait(for: [expectation], timeout: 10)
+ }
+
+ func testCancelling() throws {
+ let request = self.request(for: .bytes(count: 102400)) // 102400 is max from HTTPBin
+
+ let expectation = self.expectation(description: "Cancelled task completed anyway")
+ expectation.isInverted = true
+ let task = self.client.sendRequest(request) { result in
+ // This shouldn't get hit since we cancel the task immediately
+ expectation.fulfill()
+ }
+
+ self.client.cancel(task: task)
+
+ self.wait(for: [expectation], timeout: 10)
+
+ }
+}
From bb9354affc4966b5eb9a8aea03257fc283cb2428 Mon Sep 17 00:00:00 2001
From: Ellen Shapiro <designatednerd@gmail.com>
Date: Fri, 17 Apr 2020 15:52:33 -0500
Subject: [PATCH 02/12] replace URLSession with URLSession client, add raw
result callback and go to requiring http response
---
Sources/Apollo/HTTPNetworkTransport.swift | 125 +++++++++-------------
Sources/Apollo/URLSessionClient.swift | 41 +++++--
2 files changed, 82 insertions(+), 84 deletions(-)
diff --git a/Sources/Apollo/HTTPNetworkTransport.swift b/Sources/Apollo/HTTPNetworkTransport.swift
index fdcba682e5..7955075cc3 100644
--- a/Sources/Apollo/HTTPNetworkTransport.swift
+++ b/Sources/Apollo/HTTPNetworkTransport.swift
@@ -103,7 +103,7 @@ public class HTTPNetworkTransport {
}
let url: URL
- let session: URLSession
+ let client: URLSessionClient
let serializationFormat = JSONSerializationFormat.self
let useGETForQueries: Bool
let enableAutoPersistedQueries: Bool
@@ -127,14 +127,14 @@ public class HTTPNetworkTransport {
/// - enableAutoPersistedQueries: Whether to send persistedQuery extension. QueryDocument will be absent at 1st request, retry with QueryDocument if server respond PersistedQueryNotFound or PersistedQueryNotSupport. Defaults to false.
/// - useGETForPersistedQueryRetry: Whether to retry persistedQuery request with HttpGetMethod. Defaults to false.
public init(url: URL,
- session: URLSession = .shared,
+ client: URLSessionClient = URLSessionClient(),
sendOperationIdentifiers: Bool = false,
useGETForQueries: Bool = false,
enableAutoPersistedQueries: Bool = false,
useGETForPersistedQueryRetry: Bool = false,
requestCreator: RequestCreator = ApolloRequestCreator()) {
self.url = url
- self.session = session
+ self.client = client
self.sendOperationIdentifiers = sendOperationIdentifiers
self.useGETForQueries = useGETForQueries
self.enableAutoPersistedQueries = enableAutoPersistedQueries
@@ -155,88 +155,67 @@ public class HTTPNetworkTransport {
completionHandler(.failure(error))
return EmptyCancellable()
}
-
- let task = session.dataTask(with: request) { [weak self] data, response, error in
+
+ let task = self.client.sendRequest(request, rawTaskCompletionHandler: { [weak self] data, response, error in
+ self?.rawTaskCompleted(request: request, data: data, response: response, error: error)
+ }, completion: { [weak self] result in
guard let self = self else {
// None of the rest of this really matters
return
}
-
- self.rawTaskCompleted(request: request,
- data: data,
- response: response,
- error: error)
-
- if let receivedError = error {
- self.handleErrorOrRetry(operation: operation,
- files: files,
- error: receivedError,
- for: request,
- response: response,
- completionHandler: completionHandler)
- return
- }
-
- guard let httpResponse = response as? HTTPURLResponse else {
- fatalError("Response should be an HTTPURLResponse")
- }
-
- guard httpResponse.isSuccessful else {
- let unsuccessfulError = GraphQLHTTPResponseError(body: data,
- response: httpResponse,
- kind: .errorResponse)
- self.handleErrorOrRetry(operation: operation,
- files: files,
- error: unsuccessfulError,
- for: request,
- response: response,
- completionHandler: completionHandler)
- return
- }
-
- guard let data = data else {
- let error = GraphQLHTTPResponseError(body: nil,
- response: httpResponse,
- kind: .invalidResponse)
+
+ switch result {
+ case .failure(let error):
self.handleErrorOrRetry(operation: operation,
files: files,
error: error,
for: request,
- response: response,
+ response: nil,
completionHandler: completionHandler)
- return
- }
-
- do {
- guard let body = try self.serializationFormat.deserialize(data: data) as? JSONObject else {
- throw GraphQLHTTPResponseError(body: data, response: httpResponse, kind: .invalidResponse)
+ case .success(let (data, httpResponse)):
+ guard httpResponse.isSuccessful == true else {
+ let unsuccessfulError = GraphQLHTTPResponseError(body: data,
+ response: httpResponse,
+ kind: .errorResponse)
+ self.handleErrorOrRetry(operation: operation,
+ files: files,
+ error: unsuccessfulError,
+ for: request,
+ response: httpResponse,
+ completionHandler: completionHandler)
+ return
}
-
- let graphQLResponse = GraphQLResponse(operation: operation, body: body)
-
- if let errors = graphQLResponse.parseErrorsOnlyFast() {
- // Handle specific errors from response
- self.handleGraphQLErrorsIfNeeded(operation: operation,
- files: files,
- for: request,
- body: body,
- errors: errors,
- completionHandler: completionHandler)
- } else {
- completionHandler(.success(graphQLResponse))
+
+ do {
+ guard let body = try self.serializationFormat.deserialize(data: data) as? JSONObject else {
+ throw GraphQLHTTPResponseError(body: data, response: httpResponse, kind: .invalidResponse)
+ }
+
+ let graphQLResponse = GraphQLResponse(operation: operation, body: body)
+
+ if let errors = graphQLResponse.parseErrorsOnlyFast() {
+ // Handle specific errors from response
+ self.handleGraphQLErrorsIfNeeded(operation: operation,
+ files: files,
+ for: request,
+ body: body,
+ errors: errors,
+ completionHandler: completionHandler)
+ } else {
+ completionHandler(.success(graphQLResponse))
+ }
+ } catch let parsingError {
+ self.handleErrorOrRetry(operation: operation,
+ files: files,
+ error: parsingError,
+ for: request,
+ response: httpResponse,
+ completionHandler: completionHandler)
}
- } catch let parsingError {
- self.handleErrorOrRetry(operation: operation,
- files: files,
- error: parsingError,
- for: request,
- response: response,
- completionHandler: completionHandler)
}
- }
-
- task.resume()
+ })
+ // Task is resumed by underlying framework
return task
}
@@ -485,7 +464,7 @@ extension HTTPNetworkTransport: Equatable {
public static func ==(lhs: HTTPNetworkTransport, rhs: HTTPNetworkTransport) -> Bool {
return lhs.url == rhs.url
- && lhs.session == rhs.session
+ && lhs.client == rhs.client
&& lhs.sendOperationIdentifiers == rhs.sendOperationIdentifiers
&& lhs.useGETForQueries == rhs.useGETForQueries
}
diff --git a/Sources/Apollo/URLSessionClient.swift b/Sources/Apollo/URLSessionClient.swift
index 720318e732..a5b2958adf 100644
--- a/Sources/Apollo/URLSessionClient.swift
+++ b/Sources/Apollo/URLSessionClient.swift
@@ -3,13 +3,16 @@ import Foundation
open class URLSessionClient: NSObject, URLSessionDelegate, URLSessionTaskDelegate, URLSessionDataDelegate {
public enum URLSessionClientError: Error {
+ case noHTTPResponse(url: URL?)
case sessionBecameInvalidWithoutUnderlyingError
case dataForTaskNotFound(taskIdentifier: Int)
}
- public typealias Completion = (Result<(Data, HTTPURLResponse?), Error>) -> Void
+ public typealias RawCompletion = (Data?, URLResponse?, Error?) -> Void
+ public typealias Completion = (Result<(Data, HTTPURLResponse), Error>) -> Void
private var completionBlocks = [Int: Completion]()
+ private var rawCompletions = [Int: RawCompletion]()
private var datas = [Int: Data]()
private var responses = [Int: HTTPURLResponse]()
@@ -23,21 +26,26 @@ open class URLSessionClient: NSObject, URLSessionDelegate, URLSessionTaskDelegat
delegateQueue: callbackQueue)
}
- private func clearTask(with identifier: Int) {
+ open func clearTask(with identifier: Int) {
+ self.rawCompletions.removeValue(forKey: identifier)
self.completionBlocks.removeValue(forKey: identifier)
self.datas.removeValue(forKey: identifier)
self.responses.removeValue(forKey: identifier)
}
- private func clearAllTasks() {
+ open func clearAllTasks() {
+ self.rawCompletions.removeAll()
self.completionBlocks.removeAll()
self.datas.removeAll()
self.responses.removeAll()
}
@discardableResult
- open func sendRequest(_ request: URLRequest, completion: @escaping Completion) -> URLSessionTask {
+ open func sendRequest(_ request: URLRequest,
+ rawTaskCompletionHandler: @escaping RawCompletion,
+ completion: @escaping Completion) -> URLSessionTask {
let dataTask = self.session.dataTask(with: request)
+ self.rawCompletions[dataTask.taskIdentifier] = rawTaskCompletionHandler
self.completionBlocks[dataTask.taskIdentifier] = completion
self.datas[dataTask.taskIdentifier] = Data()
dataTask.resume()
@@ -99,28 +107,39 @@ open class URLSessionClient: NSObject, URLSessionDelegate, URLSessionTaskDelegat
open func urlSession(_ session: URLSession,
task: URLSessionTask,
didCompleteWithError error: Error?) {
+ let taskIdentifier = task.taskIdentifier
defer {
- self.clearTask(with: task.taskIdentifier)
+ self.clearTask(with: taskIdentifier)
}
- guard let completion = self.completionBlocks.removeValue(forKey: task.taskIdentifier) else {
- // No completion block, the task has likely been cancelled. Bail out.
+
+ guard
+ let rawCompletion = self.rawCompletions.removeValue(forKey: taskIdentifier),
+ let completion = self.completionBlocks.removeValue(forKey: taskIdentifier) else {
+ // No completion blocks, the task has likely been cancelled. Bail out.
return
}
+ let data = self.datas[taskIdentifier]
+ let response = self.responses[taskIdentifier]
+
+ rawCompletion(data, response, error)
if let finalError = error {
completion(.failure(finalError))
} else {
- let taskIdentifier = task.taskIdentifier
- guard let data = self.datas[taskIdentifier] else {
+ guard let finalData = data else {
// Data is immediately created for a task on creation, so if it's not there, something's gone wrong.
completion(.failure(URLSessionClientError.dataForTaskNotFound(taskIdentifier: taskIdentifier)))
return
}
- let response = self.responses[taskIdentifier]
- completion(.success((data, response)))
+ guard let finalResponse = response else {
+ completion(.failure(URLSessionClientError.noHTTPResponse(url: task.currentRequest?.url)))
+ return
+ }
+
+ completion(.success((finalData, finalResponse)))
}
}
From 6beec62bf784fdcb62df3208267e7cce661c8332 Mon Sep 17 00:00:00 2001
From: Ellen Shapiro <designatednerd@gmail.com>
Date: Fri, 17 Apr 2020 16:23:08 -0500
Subject: [PATCH 03/12] make raw completion optional, better error handling
---
Sources/Apollo/URLSessionClient.swift | 26 ++++++++++++++------------
1 file changed, 14 insertions(+), 12 deletions(-)
diff --git a/Sources/Apollo/URLSessionClient.swift b/Sources/Apollo/URLSessionClient.swift
index a5b2958adf..409cd91f51 100644
--- a/Sources/Apollo/URLSessionClient.swift
+++ b/Sources/Apollo/URLSessionClient.swift
@@ -3,9 +3,9 @@ import Foundation
open class URLSessionClient: NSObject, URLSessionDelegate, URLSessionTaskDelegate, URLSessionDataDelegate {
public enum URLSessionClientError: Error {
- case noHTTPResponse(url: URL?)
+ case noHTTPResponse(request: URLRequest?)
case sessionBecameInvalidWithoutUnderlyingError
- case dataForTaskNotFound(taskIdentifier: Int)
+ case dataForRequestNotFound(request: URLRequest?)
}
public typealias RawCompletion = (Data?, URLResponse?, Error?) -> Void
@@ -42,10 +42,13 @@ open class URLSessionClient: NSObject, URLSessionDelegate, URLSessionTaskDelegat
@discardableResult
open func sendRequest(_ request: URLRequest,
- rawTaskCompletionHandler: @escaping RawCompletion,
+ rawTaskCompletionHandler: RawCompletion? = nil,
completion: @escaping Completion) -> URLSessionTask {
let dataTask = self.session.dataTask(with: request)
- self.rawCompletions[dataTask.taskIdentifier] = rawTaskCompletionHandler
+ if let rawCompletion = rawTaskCompletionHandler {
+ self.rawCompletions[dataTask.taskIdentifier] = rawCompletion
+ }
+
self.completionBlocks[dataTask.taskIdentifier] = completion
self.datas[dataTask.taskIdentifier] = Data()
dataTask.resume()
@@ -112,10 +115,7 @@ open class URLSessionClient: NSObject, URLSessionDelegate, URLSessionTaskDelegat
self.clearTask(with: taskIdentifier)
}
-
- guard
- let rawCompletion = self.rawCompletions.removeValue(forKey: taskIdentifier),
- let completion = self.completionBlocks.removeValue(forKey: taskIdentifier) else {
+ guard let completion = self.completionBlocks.removeValue(forKey: taskIdentifier) else {
// No completion blocks, the task has likely been cancelled. Bail out.
return
}
@@ -123,19 +123,21 @@ open class URLSessionClient: NSObject, URLSessionDelegate, URLSessionTaskDelegat
let data = self.datas[taskIdentifier]
let response = self.responses[taskIdentifier]
- rawCompletion(data, response, error)
-
+ if let rawCompletion = self.rawCompletions.removeValue(forKey: taskIdentifier) {
+ rawCompletion(data, response, error)
+ }
+
if let finalError = error {
completion(.failure(finalError))
} else {
guard let finalData = data else {
// Data is immediately created for a task on creation, so if it's not there, something's gone wrong.
- completion(.failure(URLSessionClientError.dataForTaskNotFound(taskIdentifier: taskIdentifier)))
+ completion(.failure(URLSessionClientError.dataForRequestNotFound(request: task.originalRequest)))
return
}
guard let finalResponse = response else {
- completion(.failure(URLSessionClientError.noHTTPResponse(url: task.currentRequest?.url)))
+ completion(.failure(URLSessionClientError.noHTTPResponse(request: task.originalRequest)))
return
}
From c55c4d2b54f0b2a8eeff6bcc278b77a872128bdb Mon Sep 17 00:00:00 2001
From: Ellen Shapiro <designatednerd@gmail.com>
Date: Fri, 17 Apr 2020 16:23:33 -0500
Subject: [PATCH 04/12] fix annotations and make a couple methods subclassable
---
Sources/Apollo/URLSessionClient.swift | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/Sources/Apollo/URLSessionClient.swift b/Sources/Apollo/URLSessionClient.swift
index 409cd91f51..22ff4e2fb3 100644
--- a/Sources/Apollo/URLSessionClient.swift
+++ b/Sources/Apollo/URLSessionClient.swift
@@ -159,19 +159,19 @@ open class URLSessionClient: NSObject, URLSessionDelegate, URLSessionTaskDelegat
// No default implementation
}
- @available(iOS 11.0, OSXApplicationExtension 10.13, *)
- public func urlSession(_ session: URLSession,
- task: URLSessionTask,
- willBeginDelayedRequest request: URLRequest,
- completionHandler: @escaping (URLSession.DelayedRequestDisposition, URLRequest?) -> Void) {
+ @available(iOS 11.0, OSXApplicationExtension 10.13, OSX 10.13, *)
+ open func urlSession(_ session: URLSession,
+ task: URLSessionTask,
+ willBeginDelayedRequest request: URLRequest,
+ completionHandler: @escaping (URLSession.DelayedRequestDisposition, URLRequest?) -> Void) {
completionHandler(.continueLoading, request)
}
- public func urlSession(_ session: URLSession,
- task: URLSessionTask,
- willPerformHTTPRedirection response: HTTPURLResponse,
- newRequest request: URLRequest,
- completionHandler: @escaping (URLRequest?) -> Void) {
+ open func urlSession(_ session: URLSession,
+ task: URLSessionTask,
+ willPerformHTTPRedirection response: HTTPURLResponse,
+ newRequest request: URLRequest,
+ completionHandler: @escaping (URLRequest?) -> Void) {
completionHandler(request)
}
From 29cddc5a836c57e495cb16c46f5a4ae8dbfffac0 Mon Sep 17 00:00:00 2001
From: Ellen Shapiro <designatednerd@gmail.com>
Date: Fri, 17 Apr 2020 16:24:00 -0500
Subject: [PATCH 05/12] replace `MockURLSession` with `MockURLSessionClient`
subclass.
---
.../ApolloTestSupport/MockURLSession.swift | 37 ++++++++++++++-----
1 file changed, 27 insertions(+), 10 deletions(-)
diff --git a/Sources/ApolloTestSupport/MockURLSession.swift b/Sources/ApolloTestSupport/MockURLSession.swift
index 1ab1399759..a21b03a96b 100644
--- a/Sources/ApolloTestSupport/MockURLSession.swift
+++ b/Sources/ApolloTestSupport/MockURLSession.swift
@@ -6,25 +6,42 @@
//
import Foundation
+import Apollo
+
+public final class MockURLSessionClient: URLSessionClient {
-public final class MockURLSession: URLSession {
public private (set) var lastRequest: URLRequest?
public var data: Data?
public var response: HTTPURLResponse?
public var error: Error?
- override public func dataTask(with request: URLRequest) -> URLSessionDataTask {
- lastRequest = request
- return URLSessionDataTaskMock()
- }
+ public override func sendRequest(_ request: URLRequest,
+ rawTaskCompletionHandler: URLSessionClient.RawCompletion? = nil,
+ completion: @escaping URLSessionClient.Completion) -> URLSessionTask {
+ self.lastRequest = request
+ rawTaskCompletionHandler?(self.data, self.response, self.error)
+
- override public func dataTask(with request: URLRequest, completionHandler: @escaping (Data?, URLResponse?, Error?) -> Void) -> URLSessionDataTask {
- lastRequest = request
- if let response = response {
- completionHandler(data, response, error)
+ let mockTask = URLSessionDataTaskMock()
+
+ if let error = error {
+ completion(.failure(error))
+ } else {
+ guard let data = self.data else {
+ completion(.failure(URLSessionClientError.dataForRequestNotFound(request: request)))
+ return mockTask
+ }
+
+ guard let response = self.response else {
+ completion(.failure(URLSessionClientError.noHTTPResponse(request: request)))
+ return mockTask
+ }
+
+ completion(.success((data, response)))
}
- return URLSessionDataTaskMock()
+
+ return mockTask
}
}
From 0020b13f22ec79a2a3aac03dc56fc11c4854678b Mon Sep 17 00:00:00 2001
From: Ellen Shapiro <designatednerd@gmail.com>
Date: Fri, 17 Apr 2020 16:24:34 -0500
Subject: [PATCH 06/12] update tests to use mock client instead of mock
session, fix a couple things that don't need to be unwrapped anymore
---
.../AutomaticPersistedQueriesTests.swift | 66 +++++++++----------
Tests/ApolloTests/HTTPTransportTests.swift | 30 +++++----
Tests/ApolloTests/URLSessionClientTests.swift | 21 +++---
3 files changed, 62 insertions(+), 55 deletions(-)
diff --git a/Tests/ApolloTests/AutomaticPersistedQueriesTests.swift b/Tests/ApolloTests/AutomaticPersistedQueriesTests.swift
index 3f7867fe7f..3c6f08b9be 100644
--- a/Tests/ApolloTests/AutomaticPersistedQueriesTests.swift
+++ b/Tests/ApolloTests/AutomaticPersistedQueriesTests.swift
@@ -230,12 +230,12 @@ class AutomaticPersistedQueriesTests: XCTestCase {
// MARK: - Tests
func testRequestBody() throws {
- let mockSession = MockURLSession()
- let network = HTTPNetworkTransport(url: URL(string: endpoint)!, session: mockSession)
+ let mockClient = MockURLSessionClient()
+ let network = HTTPNetworkTransport(url: URL(string: endpoint)!, client: mockClient)
let query = HeroNameQuery()
let _ = network.send(operation: query) { _ in }
- let request = try XCTUnwrap(mockSession.lastRequest,
+ let request = try XCTUnwrap(mockClient.lastRequest,
"last request should not be nil")
XCTAssertEqual(request.url?.host, network.url.host)
@@ -247,12 +247,12 @@ class AutomaticPersistedQueriesTests: XCTestCase {
}
func testRequestBodyWithVariable() throws {
- let mockSession = MockURLSession()
- let network = HTTPNetworkTransport(url: URL(string: endpoint)!, session: mockSession)
+ let mockClient = MockURLSessionClient()
+ let network = HTTPNetworkTransport(url: URL(string: endpoint)!, client: mockClient)
let query = HeroNameQuery(episode: .jedi)
let _ = network.send(operation: query) { _ in }
- let request = try XCTUnwrap(mockSession.lastRequest,
+ let request = try XCTUnwrap(mockClient.lastRequest,
"last request should not be nil")
XCTAssertEqual(request.url?.host, network.url.host)
XCTAssertEqual(request.httpMethod, "POST")
@@ -264,14 +264,14 @@ class AutomaticPersistedQueriesTests: XCTestCase {
func testRequestBodyForAPQsWithVariable() throws {
- let mockSession = MockURLSession()
+ let mockClient = MockURLSessionClient()
let network = HTTPNetworkTransport(url: URL(string: endpoint)!,
- session: mockSession,
+ client: mockClient,
enableAutoPersistedQueries: true)
let query = HeroNameQuery(episode: .empire)
let _ = network.send(operation: query) { _ in }
- let request = try XCTUnwrap(mockSession.lastRequest,
+ let request = try XCTUnwrap(mockClient.lastRequest,
"last request should not be nil")
XCTAssertEqual(request.url?.host, network.url.host)
@@ -283,14 +283,14 @@ class AutomaticPersistedQueriesTests: XCTestCase {
}
func testMutationRequestBodyForAPQs() throws {
- let mockSession = MockURLSession()
+ let mockClient = MockURLSessionClient()
let network = HTTPNetworkTransport(url: URL(string: endpoint)!,
- session: mockSession,
+ client: mockClient,
enableAutoPersistedQueries: true)
let mutation = CreateAwesomeReviewMutation()
let _ = network.send(operation: mutation) { _ in }
- let request = try XCTUnwrap(mockSession.lastRequest,
+ let request = try XCTUnwrap(mockClient.lastRequest,
"last request should not be nil")
XCTAssertEqual(request.url?.host, network.url.host)
@@ -302,15 +302,15 @@ class AutomaticPersistedQueriesTests: XCTestCase {
}
func testQueryStringForAPQsUseGetMethod() throws {
- let mockSession = MockURLSession()
+ let mockClient = MockURLSessionClient()
let network = HTTPNetworkTransport(url: URL(string: endpoint)!,
- session: mockSession,
+ client: mockClient,
enableAutoPersistedQueries: true,
useGETForPersistedQueryRetry: true)
let query = HeroNameQuery()
let _ = network.send(operation: query) { _ in }
- let request = try XCTUnwrap(mockSession.lastRequest,
+ let request = try XCTUnwrap(mockClient.lastRequest,
"last request should not be nil")
XCTAssertEqual(request.url?.host, network.url.host)
@@ -320,15 +320,15 @@ class AutomaticPersistedQueriesTests: XCTestCase {
}
func testQueryStringForAPQsUseGetMethodWithVariable() throws {
- let mockSession = MockURLSession()
+ let mockClient = MockURLSessionClient()
let network = HTTPNetworkTransport(url: URL(string: endpoint)!,
- session: mockSession,
+ client: mockClient,
enableAutoPersistedQueries: true,
useGETForPersistedQueryRetry: true)
let query = HeroNameQuery(episode: .empire)
let _ = network.send(operation: query) { _ in }
- let request = try XCTUnwrap(mockSession.lastRequest,
+ let request = try XCTUnwrap(mockClient.lastRequest,
"last request should not be nil")
XCTAssertEqual(request.url?.host, network.url.host)
@@ -340,14 +340,14 @@ class AutomaticPersistedQueriesTests: XCTestCase {
}
func testUseGETForQueriesRequest() throws {
- let mockSession = MockURLSession()
+ let mockClient = MockURLSessionClient()
let network = HTTPNetworkTransport(url: URL(string: endpoint)!,
- session: mockSession,
+ client: mockClient,
useGETForQueries: true)
let query = HeroNameQuery()
let _ = network.send(operation: query) { _ in }
- let request = try XCTUnwrap(mockSession.lastRequest,
+ let request = try XCTUnwrap(mockClient.lastRequest,
"last request should not be nil")
XCTAssertEqual(request.url?.host, network.url.host)
@@ -359,12 +359,12 @@ class AutomaticPersistedQueriesTests: XCTestCase {
}
func testNotUseGETForQueriesRequest() throws {
- let mockSession = MockURLSession()
- let network = HTTPNetworkTransport(url: URL(string: endpoint)!, session: mockSession)
+ let mockClient = MockURLSessionClient()
+ let network = HTTPNetworkTransport(url: URL(string: endpoint)!, client: mockClient)
let query = HeroNameQuery()
let _ = network.send(operation: query) { _ in }
- let request = try XCTUnwrap(mockSession.lastRequest,
+ let request = try XCTUnwrap(mockClient.lastRequest,
"last request should not be nil")
XCTAssertEqual(request.url?.host, network.url.host)
@@ -376,14 +376,14 @@ class AutomaticPersistedQueriesTests: XCTestCase {
}
func testNotUseGETForQueriesAPQsRequest() throws {
- let mockSession = MockURLSession()
+ let mockClient = MockURLSessionClient()
let network = HTTPNetworkTransport(url: URL(string: endpoint)!,
- session: mockSession,
+ client: mockClient,
enableAutoPersistedQueries: true)
let query = HeroNameQuery(episode: .empire)
let _ = network.send(operation: query) { _ in }
- let request = try XCTUnwrap(mockSession.lastRequest,
+ let request = try XCTUnwrap(mockClient.lastRequest,
"last request should not be nil")
XCTAssertEqual(request.url?.host, network.url.host)
@@ -395,15 +395,15 @@ class AutomaticPersistedQueriesTests: XCTestCase {
}
func testUseGETForQueriesAPQsRequest() throws {
- let mockSession = MockURLSession()
+ let mockClient = MockURLSessionClient()
let network = HTTPNetworkTransport(url: URL(string: endpoint)!,
- session: mockSession,
+ client: mockClient,
useGETForQueries: true,
enableAutoPersistedQueries: true)
let query = HeroNameQuery(episode: .empire)
let _ = network.send(operation: query) { _ in }
- let request = try XCTUnwrap(mockSession.lastRequest,
+ let request = try XCTUnwrap(mockClient.lastRequest,
"last request should not be nil")
XCTAssertEqual(request.url?.host, network.url.host)
@@ -415,15 +415,15 @@ class AutomaticPersistedQueriesTests: XCTestCase {
}
func testNotUseGETForQueriesAPQsGETRequest() throws {
- let mockSession = MockURLSession()
+ let mockClient = MockURLSessionClient()
let network = HTTPNetworkTransport(url: URL(string: endpoint)!,
- session: mockSession,
+ client: mockClient,
enableAutoPersistedQueries: true,
useGETForPersistedQueryRetry: true)
let query = HeroNameQuery(episode: .empire)
let _ = network.send(operation: query) { _ in }
- let request = try XCTUnwrap(mockSession.lastRequest,
+ let request = try XCTUnwrap(mockClient.lastRequest,
"last request should not be nil")
XCTAssertEqual(request.url?.host, network.url.host)
XCTAssertEqual(request.httpMethod, "GET")
diff --git a/Tests/ApolloTests/HTTPTransportTests.swift b/Tests/ApolloTests/HTTPTransportTests.swift
index eeb0231d03..3ccc3f0a9b 100644
--- a/Tests/ApolloTests/HTTPTransportTests.swift
+++ b/Tests/ApolloTests/HTTPTransportTests.swift
@@ -260,10 +260,12 @@ class HTTPTransportTests: XCTestCase {
func testEquality() {
let identicalTransport = HTTPNetworkTransport(url: self.url,
+ client: self.networkTransport.client,
useGETForQueries: true)
XCTAssertEqual(self.networkTransport, identicalTransport)
- let nonIdenticalTransport = HTTPNetworkTransport(url: self.url)
+ let nonIdenticalTransport = HTTPNetworkTransport(url: self.url,
+ client: self.networkTransport.client)
XCTAssertNotEqual(self.networkTransport, nonIdenticalTransport)
}
@@ -274,11 +276,11 @@ class HTTPTransportTests: XCTestCase {
// TODO: Replace this with once it is codable https://github.com/apollographql/apollo-ios/issues/467
let body = ["errors": [["message": "Test graphql error"]]]
- let mockSession = MockURLSession()
- mockSession.response = HTTPURLResponse(url: url, statusCode: 200, httpVersion: nil, headerFields: nil)
- mockSession.data = try JSONSerialization.data(withJSONObject: body, options: .prettyPrinted)
+ let mockClient = MockURLSessionClient()
+ mockClient.response = HTTPURLResponse(url: url, statusCode: 200, httpVersion: nil, headerFields: nil)
+ mockClient.data = try JSONSerialization.data(withJSONObject: body, options: .prettyPrinted)
let network = HTTPNetworkTransport(url: url,
- session: mockSession)
+ client: mockClient)
network.delegate = self
let expectation = self.expectation(description: "Send operation completed")
@@ -291,7 +293,7 @@ class HTTPTransportTests: XCTestCase {
}
}
- let request = try XCTUnwrap(mockSession.lastRequest,
+ let request = try XCTUnwrap(mockClient.lastRequest,
"last request should not be nil")
XCTAssertEqual(request.url?.host, network.url.host)
@@ -309,11 +311,11 @@ class HTTPTransportTests: XCTestCase {
// TODO: Replace this with once it is codable https://github.com/apollographql/apollo-ios/issues/467
let body = ["errors": []]
- let mockSession = MockURLSession()
- mockSession.response = HTTPURLResponse(url: url, statusCode: 200, httpVersion: nil, headerFields: nil)
- mockSession.data = try JSONSerialization.data(withJSONObject: body, options: .prettyPrinted)
+ let mockClient = MockURLSessionClient()
+ mockClient.response = HTTPURLResponse(url: url, statusCode: 200, httpVersion: nil, headerFields: nil)
+ mockClient.data = try JSONSerialization.data(withJSONObject: body, options: .prettyPrinted)
let network = HTTPNetworkTransport(url: url,
- session: mockSession)
+ client: mockClient)
network.delegate = self
let expectation = self.expectation(description: "Send operation completed")
@@ -326,7 +328,7 @@ class HTTPTransportTests: XCTestCase {
}
}
- let request = try XCTUnwrap(mockSession.lastRequest,
+ let request = try XCTUnwrap(mockClient.lastRequest,
"last request should not be nil")
XCTAssertEqual(request.url?.host, network.url.host)
@@ -337,13 +339,13 @@ class HTTPTransportTests: XCTestCase {
}
func testClientNameAndVersionHeadersAreSent() throws {
- let mockSession = MockURLSession()
+ let mockClient = MockURLSessionClient()
let network = HTTPNetworkTransport(url: self.url,
- session: mockSession)
+ client: mockClient)
let query = HeroNameQuery(episode: .empire)
let _ = network.send(operation: query) { _ in }
- let request = try XCTUnwrap(mockSession.lastRequest,
+ let request = try XCTUnwrap(mockClient.lastRequest,
"last request should not be nil")
let clientName = try XCTUnwrap(request.value(forHTTPHeaderField: HTTPNetworkTransport.headerFieldNameApolloClientName),
diff --git a/Tests/ApolloTests/URLSessionClientTests.swift b/Tests/ApolloTests/URLSessionClientTests.swift
index d1beade0a0..fe1091a1f8 100644
--- a/Tests/ApolloTests/URLSessionClientTests.swift
+++ b/Tests/ApolloTests/URLSessionClientTests.swift
@@ -24,7 +24,7 @@ class URLSessionClientLiveTests: XCTestCase {
XCTFail("Unexpected error: \(error)")
case .success(let (data, httpResponse)):
XCTAssertFalse(data.isEmpty)
- XCTAssertEqual(request.url, httpResponse?.url)
+ XCTAssertEqual(request.url, httpResponse.url)
}
}
@@ -44,10 +44,15 @@ class URLSessionClientLiveTests: XCTestCase {
XCTFail("Unexpected error: \(error)")
case .success(let (data, httpResponse)):
XCTAssertFalse(data.isEmpty)
- XCTAssertEqual(httpResponse!.allHeaderFields["Content-Type"] as! String, "image/jpeg")
- let image = UIImage(data: data)
- XCTAssertNotNil(image)
- XCTAssertEqual(request.url, httpResponse?.url)
+ XCTAssertEqual(httpResponse.allHeaderFields["Content-Type"] as! String, "image/jpeg")
+ #if os(macOS)
+ let image = NSImage(data: data)
+ XCTAssertNotNil(image)
+ #else
+ let image = UIImage(data: data)
+ XCTAssertNotNil(image)
+ #endif
+ XCTAssertEqual(request.url, httpResponse.url)
}
}
@@ -71,7 +76,7 @@ class URLSessionClientLiveTests: XCTestCase {
XCTAssertEqual(data.count,
randomInt,
"Expected \(randomInt) bytes, got \(data.count)")
- XCTAssertEqual(request.url, response?.url)
+ XCTAssertEqual(request.url, response.url)
}
}
@@ -97,7 +102,7 @@ class URLSessionClientLiveTests: XCTestCase {
case .failure(let error):
XCTFail("Unexpected error: \(error)")
case .success(let (data, httpResponse)):
- XCTAssertEqual(request.url, httpResponse?.url)
+ XCTAssertEqual(request.url, httpResponse.url)
do {
let parsed = try HTTPBinResponse(data: data)
@@ -123,7 +128,7 @@ class URLSessionClientLiveTests: XCTestCase {
self.client.cancel(task: task)
- self.wait(for: [expectation], timeout: 10)
+ self.wait(for: [expectation], timeout: 5)
}
}
From 08c68dcb51b19c2a6b26c38e7554f8231f4cf919 Mon Sep 17 00:00:00 2001
From: Ellen Shapiro <designatednerd@gmail.com>
Date: Fri, 17 Apr 2020 16:48:47 -0500
Subject: [PATCH 07/12] add inline documentation to URLSessionClient
---
Sources/Apollo/URLSessionClient.swift | 36 ++++++++++++++++++++++++---
1 file changed, 32 insertions(+), 4 deletions(-)
diff --git a/Sources/Apollo/URLSessionClient.swift b/Sources/Apollo/URLSessionClient.swift
index 22ff4e2fb3..b1ef7b32a9 100644
--- a/Sources/Apollo/URLSessionClient.swift
+++ b/Sources/Apollo/URLSessionClient.swift
@@ -1,5 +1,7 @@
import Foundation
+/// A class to handle URL Session calls that will support background execution,
+/// but still (mostly) use callbacks for its primary method of communication.
open class URLSessionClient: NSObject, URLSessionDelegate, URLSessionTaskDelegate, URLSessionDataDelegate {
public enum URLSessionClientError: Error {
@@ -8,7 +10,10 @@ open class URLSessionClient: NSObject, URLSessionDelegate, URLSessionTaskDelegat
case dataForRequestNotFound(request: URLRequest?)
}
- public typealias RawCompletion = (Data?, URLResponse?, Error?) -> Void
+ /// A completion block to be called when the raw task has completed, with the raw information from the session
+ public typealias RawCompletion = (Data?, HTTPURLResponse?, Error?) -> Void
+
+ /// A completion block returning a result. On `.success` it will contain a tuple with non-nil `Data` and its corresponding `HTTPURLResponse`. On `.failure` it will contain an error.
public typealias Completion = (Result<(Data, HTTPURLResponse), Error>) -> Void
private var completionBlocks = [Int: Completion]()
@@ -16,8 +21,14 @@ open class URLSessionClient: NSObject, URLSessionDelegate, URLSessionTaskDelegat
private var datas = [Int: Data]()
private var responses = [Int: HTTPURLResponse]()
+ /// The raw URLSession being used for this client
open private(set) var session: URLSession!
+ /// Designated initializer.
+ ///
+ /// - Parameters:
+ /// - sessionConfiguration: The `URLSessionConfiguration` to use to set up the URL session.
+ /// - callbackQueue: [optional] The `OperationQueue` to tell the URL session to call back to this class on, which will in turn call back to your class. Defaults to `.main`.
public init(sessionConfiguration: URLSessionConfiguration = .default,
callbackQueue: OperationQueue? = .main) {
super.init()
@@ -26,6 +37,9 @@ open class URLSessionClient: NSObject, URLSessionDelegate, URLSessionTaskDelegat
delegateQueue: callbackQueue)
}
+ /// Clears underlying dictionaries of any data related to a particular task identifier.
+ ///
+ /// - Parameter identifier: The identifier of the task to clear.
open func clearTask(with identifier: Int) {
self.rawCompletions.removeValue(forKey: identifier)
self.completionBlocks.removeValue(forKey: identifier)
@@ -33,6 +47,9 @@ open class URLSessionClient: NSObject, URLSessionDelegate, URLSessionTaskDelegat
self.responses.removeValue(forKey: identifier)
}
+ /// Clears underlying dictionaries of any data related to all tasks.
+ ///
+ /// Mostly useful for cleanup and/or after invalidation of the `URLSession`.
open func clearAllTasks() {
self.rawCompletions.removeAll()
self.completionBlocks.removeAll()
@@ -40,6 +57,14 @@ open class URLSessionClient: NSObject, URLSessionDelegate, URLSessionTaskDelegat
self.responses.removeAll()
}
+ /// The main method to perform a request.
+ ///
+ /// - Parameters:
+ /// - request: The request to perform.
+ /// - rawTaskCompletionHandler: [optional] A completion handler to call once the raw task is done, so if an Error requires access to the headers, the user can still access these.
+ /// - completion: A completion handler to call when the task has either completed successfully or failed.
+ ///
+ /// - Returns: The created URLSesssion task, already resumed, because nobody ever remembers to call `resume()`.
@discardableResult
open func sendRequest(_ request: URLRequest,
rawTaskCompletionHandler: RawCompletion? = nil,
@@ -56,9 +81,13 @@ open class URLSessionClient: NSObject, URLSessionDelegate, URLSessionTaskDelegat
return dataTask
}
+ /// Cancels a given task and clears out its underlying data.
+ ///
+ /// NOTE: You will not receive any kind of "This was cancelled" error when this is called.
+ ///
+ /// - Parameter task: The task you wish to cancel.
open func cancel(task: URLSessionTask) {
- let taskID = task.taskIdentifier
- self.clearTask(with: taskID)
+ self.clearTask(with: task.taskIdentifier)
task.cancel()
}
@@ -86,7 +115,6 @@ open class URLSessionClient: NSObject, URLSessionDelegate, URLSessionTaskDelegat
completionHandler(.performDefaultHandling, nil)
}
-
#if os(iOS) || os(tvOS) || os(watchOS)
open func urlSessionDidFinishEvents(forBackgroundURLSession session: URLSession) {
// No default implementation
From b4df0a7ca3fbc5952ec11146b03a920514bbaac6 Mon Sep 17 00:00:00 2001
From: Ellen Shapiro <designatednerd@gmail.com>
Date: Fri, 17 Apr 2020 16:50:36 -0500
Subject: [PATCH 08/12] Update advanced setup docs to include some
documentation for new URLSessionClient class.
---
docs/source/initialization.md | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/docs/source/initialization.md b/docs/source/initialization.md
index 7823049514..ec06b2cbc8 100644
--- a/docs/source/initialization.md
+++ b/docs/source/initialization.md
@@ -35,11 +35,21 @@ The available implementations are:
The initializer for `HTTPNetworkTransport` has several properties which can allow you to get better information and finer-grained control of your HTTP requests and responses:
-- `session` allows you to pass in a custom `URLSession` to set up anything which needs to be done for every single request without alteration. This defaults to `URLSession.shared`.
+- `client` allows you to pass in a [subclass of `URLSessionClient`](#the-urlsessionclient-class) to handle managing a background-compatible URL session, and set up anything which needs to be done for every single request without alteration.
- `sendOperationIdentifiers` allows you send operation identifiers along with your requests. **NOTE:** To send operation identifiers, Apollo types must be generated with `operationIdentifier`s or sending data will crash. Due to this restriction, this option defaults to `false`.
- `useGETForQueries` sends all requests of `query` type using `GET` instead of `POST`. This defaults to `false` to preserve existing behavior in older versions of the client.
- `delegate` Can conform to one or many of several sub-protocols for `HTTPNetworkTransportDelegate`, detailed below.
+### The URLSessionClient class
+
+Since `URLSession` only supports use in the background using the delegate-based API, we have created our own `URLSessionClient` which handles the basics of setup for that.
+
+One thing to be aware of: Because setting up a delegate is only possible in the initializer for `URLSession`, you can only pass in a `URLSessionConfiguration`, **not** an existing `URLSession`, to this class's initializer.
+
+By default, instances of `URLSessionClient` use `URLSessionConfiguration.default` to set up their URL session, and instances of `HTTPNetworkTransport` use the default initializer for `URLSessionClient`.
+
+The `URLSessionClient` class and most of its methods are `open` so you can subclass it if you need to override any of the delegate methods for the `URLSession` delegates we're using or you need to handle additional delegate scenarios.
+
### Using `HTTPNetworkTransportDelegate`
This delegate includes several sub-protocols so that a single parameter can be passed no matter how many sub-protocols it conforms to.
From 89da035e95ed8db4e12ed0b69d22770e32008ee9 Mon Sep 17 00:00:00 2001
From: Ellen Shapiro <designatednerd@gmail.com>
Date: Fri, 17 Apr 2020 17:16:21 -0500
Subject: [PATCH 09/12] test two different types of cancellation
---
Tests/ApolloTests/URLSessionClientTests.swift | 28 +++++++++++++++++--
1 file changed, 26 insertions(+), 2 deletions(-)
diff --git a/Tests/ApolloTests/URLSessionClientTests.swift b/Tests/ApolloTests/URLSessionClientTests.swift
index fe1091a1f8..3836ccc416 100644
--- a/Tests/ApolloTests/URLSessionClientTests.swift
+++ b/Tests/ApolloTests/URLSessionClientTests.swift
@@ -116,10 +116,34 @@ class URLSessionClientLiveTests: XCTestCase {
self.wait(for: [expectation], timeout: 10)
}
- func testCancelling() throws {
+ func testCancellingTaskDirectlyCallsCompletionWithError() throws {
let request = self.request(for: .bytes(count: 102400)) // 102400 is max from HTTPBin
- let expectation = self.expectation(description: "Cancelled task completed anyway")
+ let expectation = self.expectation(description: "Cancelled task completed")
+ let task = self.client.sendRequest(request) { result in
+ defer {
+ expectation.fulfill()
+ }
+
+ switch result {
+ case .failure(let error):
+ let nsError = error as NSError
+ XCTAssertEqual(nsError.domain, NSURLErrorDomain)
+ XCTAssertEqual(nsError.code, NSURLErrorCancelled)
+ case .success:
+ XCTFail("Task succeeded when it should have been cancelled!")
+ }
+ }
+
+ task.cancel()
+
+ self.wait(for: [expectation], timeout: 10)
+ }
+
+ func testCancellingTaskThroughClientDoesNotCallCompletion() throws {
+ let request = self.request(for: .bytes(count: 102400)) // 102400 is max from HTTPBin
+
+ let expectation = self.expectation(description: "Cancelled task completed")
expectation.isInverted = true
let task = self.client.sendRequest(request) { result in
// This shouldn't get hit since we cancel the task immediately
From 97497c54c1831cfc300d2b594a71e522c2b01750 Mon Sep 17 00:00:00 2001
From: Ellen Shapiro <designatednerd@gmail.com>
Date: Sun, 19 Apr 2020 14:19:47 -0500
Subject: [PATCH 10/12] add missing @available warnings for watchOS and tvOS
---
Sources/Apollo/URLSessionClient.swift | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/Sources/Apollo/URLSessionClient.swift b/Sources/Apollo/URLSessionClient.swift
index b1ef7b32a9..b290e381ab 100644
--- a/Sources/Apollo/URLSessionClient.swift
+++ b/Sources/Apollo/URLSessionClient.swift
@@ -102,7 +102,7 @@ open class URLSessionClient: NSObject, URLSessionDelegate, URLSessionTaskDelegat
self.clearAllTasks()
}
- @available(OSX 10.12, iOS 10.0, *)
+ @available(OSX 10.12, iOS 10.0, tvOS 10.0, *)
open func urlSession(_ session: URLSession,
task: URLSessionTask,
didFinishCollecting metrics: URLSessionTaskMetrics) {
@@ -187,7 +187,7 @@ open class URLSessionClient: NSObject, URLSessionDelegate, URLSessionTaskDelegat
// No default implementation
}
- @available(iOS 11.0, OSXApplicationExtension 10.13, OSX 10.13, *)
+ @available(iOS 11.0, OSXApplicationExtension 10.13, OSX 10.13, tvOS 11.0, watchOS 4.0, *)
open func urlSession(_ session: URLSession,
task: URLSessionTask,
willBeginDelayedRequest request: URLRequest,
From 4527d0b72d76567d571f5720baf1c1b938865d7f Mon Sep 17 00:00:00 2001
From: Ellen Shapiro <designatednerd@gmail.com>
Date: Sun, 19 Apr 2020 16:19:32 -0500
Subject: [PATCH 11/12] Add note about delegate methods to the class
documentation of URLSessionClient
---
Sources/Apollo/URLSessionClient.swift | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/Sources/Apollo/URLSessionClient.swift b/Sources/Apollo/URLSessionClient.swift
index b290e381ab..4de3e57030 100644
--- a/Sources/Apollo/URLSessionClient.swift
+++ b/Sources/Apollo/URLSessionClient.swift
@@ -2,6 +2,12 @@ import Foundation
/// A class to handle URL Session calls that will support background execution,
/// but still (mostly) use callbacks for its primary method of communication.
+///
+/// **NOTE:** Delegate methods implemented here are not documented inline because
+/// Apple has their own documentation for them. Please consult Apple's
+/// documentation for how the delegate methods work and what needs to be overridden
+/// and handled within your app, particularly in regards to what needs to be called
+/// when for background sessions.
open class URLSessionClient: NSObject, URLSessionDelegate, URLSessionTaskDelegate, URLSessionDataDelegate {
public enum URLSessionClientError: Error {
From d68300e6e5f5ffb19503e12e6446167473372047 Mon Sep 17 00:00:00 2001
From: Ellen Shapiro <designatednerd@gmail.com>
Date: Fri, 24 Apr 2020 18:21:23 -0500
Subject: [PATCH 12/12] add context to error being returned when something goes
wrong at the network level
---
Sources/Apollo/URLSessionClient.swift | 17 +++++++++--------
Tests/ApolloTests/URLSessionClientTests.swift | 13 ++++++++++---
2 files changed, 19 insertions(+), 11 deletions(-)
diff --git a/Sources/Apollo/URLSessionClient.swift b/Sources/Apollo/URLSessionClient.swift
index 4de3e57030..471282e91f 100644
--- a/Sources/Apollo/URLSessionClient.swift
+++ b/Sources/Apollo/URLSessionClient.swift
@@ -14,6 +14,7 @@ open class URLSessionClient: NSObject, URLSessionDelegate, URLSessionTaskDelegat
case noHTTPResponse(request: URLRequest?)
case sessionBecameInvalidWithoutUnderlyingError
case dataForRequestNotFound(request: URLRequest?)
+ case networkError(data: Data, response: HTTPURLResponse?, underlying: Error)
}
/// A completion block to be called when the raw task has completed, with the raw information from the session
@@ -160,16 +161,16 @@ open class URLSessionClient: NSObject, URLSessionDelegate, URLSessionTaskDelegat
if let rawCompletion = self.rawCompletions.removeValue(forKey: taskIdentifier) {
rawCompletion(data, response, error)
}
-
+
+ guard let finalData = data else {
+ // Data is immediately created for a task on creation, so if it's not there, something's gone wrong.
+ completion(.failure(URLSessionClientError.dataForRequestNotFound(request: task.originalRequest)))
+ return
+ }
+
if let finalError = error {
- completion(.failure(finalError))
+ completion(.failure(URLSessionClientError.networkError(data: finalData, response: response, underlying: finalError)))
} else {
- guard let finalData = data else {
- // Data is immediately created for a task on creation, so if it's not there, something's gone wrong.
- completion(.failure(URLSessionClientError.dataForRequestNotFound(request: task.originalRequest)))
- return
- }
-
guard let finalResponse = response else {
completion(.failure(URLSessionClientError.noHTTPResponse(request: task.originalRequest)))
return
diff --git a/Tests/ApolloTests/URLSessionClientTests.swift b/Tests/ApolloTests/URLSessionClientTests.swift
index 3836ccc416..1602c1ba38 100644
--- a/Tests/ApolloTests/URLSessionClientTests.swift
+++ b/Tests/ApolloTests/URLSessionClientTests.swift
@@ -127,9 +127,16 @@ class URLSessionClientLiveTests: XCTestCase {
switch result {
case .failure(let error):
- let nsError = error as NSError
- XCTAssertEqual(nsError.domain, NSURLErrorDomain)
- XCTAssertEqual(nsError.code, NSURLErrorCancelled)
+ switch error {
+ case URLSessionClient.URLSessionClientError.networkError(let data, let httpResponse, let underlying):
+ XCTAssertTrue(data.isEmpty)
+ XCTAssertNil(httpResponse)
+ let nsError = underlying as NSError
+ XCTAssertEqual(nsError.domain, NSURLErrorDomain)
+ XCTAssertEqual(nsError.code, NSURLErrorCancelled)
+ default:
+ XCTFail("Unexpected error: \(error)")
+ }
case .success:
XCTFail("Task succeeded when it should have been cancelled!")
}