From 51265c2359bec3cc34dfd8cbd57abab0f07c660a Mon Sep 17 00:00:00 2001 From: Kim de Vos Date: Wed, 18 Sep 2019 12:55:42 +0200 Subject: [PATCH 01/11] Introducing delegate if there is GraphQL errors --- Sources/Apollo/HTTPNetworkTransport.swift | 34 +++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/Sources/Apollo/HTTPNetworkTransport.swift b/Sources/Apollo/HTTPNetworkTransport.swift index 4315ebbd8f..ffd9345bed 100644 --- a/Sources/Apollo/HTTPNetworkTransport.swift +++ b/Sources/Apollo/HTTPNetworkTransport.swift @@ -65,6 +65,21 @@ public protocol HTTPNetworkTransportRetryDelegate: HTTPNetworkTransportDelegate retryHandler: @escaping (_ shouldRetry: Bool) -> Void) } +/// Methods which will be called after some kind of response has been received and it contains GraphQLErrors +public protocol NetworkGraphQLErrorDelegate: HTTPNetworkTransportDelegate { + + + /// Called when response contains one or more GraphQL errors. + /// NOTE: Don't just call the `retryHandler` with `true` all the time, or you can potentially wind up in an infinite loop of errors + /// + /// - Parameters: + /// - networkTransport: The network transport which received the error + /// - errors: The received GraphQL errors + /// - retryHandler: A closure indicating whether the operation should be retried. Asyncrhonous to allow for re-authentication or other async operations to complete. + func networkTransport(_ networkTransport: HTTPNetworkTransport, receivedGraphQLErrors errors: [GraphQLError], retryHandler: @escaping (_ shouldRetry: Bool) -> Void) +} + + // MARK: - /// A network transport that uses HTTP POST requests to send GraphQL operations to a server, and that uses `URLSession` as the networking implementation. @@ -194,6 +209,25 @@ public class HTTPNetworkTransport { return task } + + private func handleGraphQLErrorsOrComplete(operation: Operation, + response: GraphQLResponse, + completionHandler: @escaping (_ result: Result, Error>) -> Void) throws { + if let delegate = self.delegate as? NetworkGraphQLErrorDelegate, + let graphQLErrors = try response.parseResultFast().errors, + !graphQLErrors.isEmpty { + delegate.networkTransport(self, receivedGraphQLErrors: graphQLErrors, retryHandler: { [weak self] shouldRetry in + guard shouldRetry else { + completionHandler(.success(response)) + return + } + + _ = self?.send(operation: operation, completionHandler: completionHandler) + }) + } else { + completionHandler(.success(response)) + } + } private func handleGraphQLErrorsIfNeeded(operation: Operation, for request: URLRequest, From e85cee186aea01b612bc2fcbcf6cf4487106d55f Mon Sep 17 00:00:00 2001 From: Kim de Vos Date: Thu, 19 Sep 2019 07:35:21 +0200 Subject: [PATCH 02/11] Add files in `handleErrorOrRetry` --- Sources/Apollo/HTTPNetworkTransport.swift | 47 +++++++++++++---------- 1 file changed, 26 insertions(+), 21 deletions(-) diff --git a/Sources/Apollo/HTTPNetworkTransport.swift b/Sources/Apollo/HTTPNetworkTransport.swift index ffd9345bed..0614f75043 100644 --- a/Sources/Apollo/HTTPNetworkTransport.swift +++ b/Sources/Apollo/HTTPNetworkTransport.swift @@ -145,11 +145,12 @@ public class HTTPNetworkTransport { error: error) if let receivedError = error { - self.handleErrorOrRetry(operation: operation, - error: receivedError, - for: request, - response: response, - completionHandler: completionHandler) + self?.handleErrorOrRetry(operation: operation, + files: files, + error: receivedError, + for: request, + response: response, + completionHandler: completionHandler) return } @@ -161,11 +162,12 @@ public class HTTPNetworkTransport { let unsuccessfulError = GraphQLHTTPResponseError(body: data, response: httpResponse, kind: .errorResponse) - self.handleErrorOrRetry(operation: operation, - error: unsuccessfulError, - for: request, - response: response, - completionHandler: completionHandler) + self?.handleErrorOrRetry(operation: operation, + files: files, + error: unsuccessfulError, + for: request, + response: response, + completionHandler: completionHandler) return } @@ -173,11 +175,12 @@ public class HTTPNetworkTransport { let error = GraphQLHTTPResponseError(body: nil, response: httpResponse, kind: .invalidResponse) - self.handleErrorOrRetry(operation: operation, - error: error, - for: request, - response: response, - completionHandler: completionHandler) + self?.handleErrorOrRetry(operation: operation, + files: files, + error: error, + for: request, + response: response, + completionHandler: completionHandler) return } @@ -197,11 +200,12 @@ public class HTTPNetworkTransport { completionHandler(.success(graphQLResponse)) } } catch let parsingError { - self.handleErrorOrRetry(operation: operation, - error: parsingError, - for: request, - response: response, - completionHandler: completionHandler) + self?.handleErrorOrRetry(operation: operation, + files: files, + error: parsingError, + for: request, + response: response, + completionHandler: completionHandler) } } @@ -251,6 +255,7 @@ public class HTTPNetworkTransport { } private func handleErrorOrRetry(operation: Operation, + files: [GraphQLFile]?, error: Error, for request: URLRequest, response: URLResponse?, @@ -273,7 +278,7 @@ public class HTTPNetworkTransport { return } - _ = self?.send(operation: operation, completionHandler: completionHandler) + _ = self?.send(operation: operation, files: files, completionHandler: completionHandler) }) } From c6e2737fdcabac212e41f2b2787478f8ab96b695 Mon Sep 17 00:00:00 2001 From: Kim de Vos Date: Thu, 19 Sep 2019 07:36:14 +0200 Subject: [PATCH 03/11] Only parse errors and use guard instead of `if let` --- Sources/Apollo/GraphQLResponse.swift | 8 +++++++ Sources/Apollo/HTTPNetworkTransport.swift | 28 ++++++++++++----------- 2 files changed, 23 insertions(+), 13 deletions(-) diff --git a/Sources/Apollo/GraphQLResponse.swift b/Sources/Apollo/GraphQLResponse.swift index 9f68473fd6..abd2795ddb 100644 --- a/Sources/Apollo/GraphQLResponse.swift +++ b/Sources/Apollo/GraphQLResponse.swift @@ -56,4 +56,12 @@ public final class GraphQLResponse { return GraphQLResult(data: nil, errors: errors, source: .server, dependentKeys: nil) } } + + func parseErrorsOnlyFast() -> [GraphQLError]? { + guard let errorsEntry = self.body["errors"] as? [JSONObject] else { + return nil + } + + return errorsEntry.map(GraphQLError.init) + } } diff --git a/Sources/Apollo/HTTPNetworkTransport.swift b/Sources/Apollo/HTTPNetworkTransport.swift index 0614f75043..3ec12316d9 100644 --- a/Sources/Apollo/HTTPNetworkTransport.swift +++ b/Sources/Apollo/HTTPNetworkTransport.swift @@ -215,22 +215,24 @@ public class HTTPNetworkTransport { } private func handleGraphQLErrorsOrComplete(operation: Operation, + files: [GraphQLFile]?, response: GraphQLResponse, completionHandler: @escaping (_ result: Result, Error>) -> Void) throws { - if let delegate = self.delegate as? NetworkGraphQLErrorDelegate, - let graphQLErrors = try response.parseResultFast().errors, - !graphQLErrors.isEmpty { - delegate.networkTransport(self, receivedGraphQLErrors: graphQLErrors, retryHandler: { [weak self] shouldRetry in - guard shouldRetry else { - completionHandler(.success(response)) - return - } - - _ = self?.send(operation: operation, completionHandler: completionHandler) - }) - } else { - completionHandler(.success(response)) + guard let delegate = self.delegate as? NetworkGraphQLErrorDelegate, + let graphQLErrors = response.parseErrorsOnlyFast(), + !graphQLErrors.isEmpty else { + completionHandler(.success(response)) + return } + + delegate.networkTransport(self, receivedGraphQLErrors: graphQLErrors, retryHandler: { [weak self] shouldRetry in + guard shouldRetry else { + completionHandler(.success(response)) + return + } + + _ = self?.send(operation: operation, files: files, completionHandler: completionHandler) + }) } private func handleGraphQLErrorsIfNeeded(operation: Operation, From ad96a178e257fa77a30aeee06f1d30757107455a Mon Sep 17 00:00:00 2001 From: Kim de Vos Date: Thu, 19 Sep 2019 07:52:01 +0200 Subject: [PATCH 04/11] Updated name to match other protocols --- Sources/Apollo/HTTPNetworkTransport.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Sources/Apollo/HTTPNetworkTransport.swift b/Sources/Apollo/HTTPNetworkTransport.swift index 3ec12316d9..8e1dd3d035 100644 --- a/Sources/Apollo/HTTPNetworkTransport.swift +++ b/Sources/Apollo/HTTPNetworkTransport.swift @@ -66,7 +66,7 @@ public protocol HTTPNetworkTransportRetryDelegate: HTTPNetworkTransportDelegate } /// Methods which will be called after some kind of response has been received and it contains GraphQLErrors -public protocol NetworkGraphQLErrorDelegate: HTTPNetworkTransportDelegate { +public protocol HTTPNetworkTransportGraphQLErrorDelegate: HTTPNetworkTransportDelegate { /// Called when response contains one or more GraphQL errors. @@ -218,7 +218,7 @@ public class HTTPNetworkTransport { files: [GraphQLFile]?, response: GraphQLResponse, completionHandler: @escaping (_ result: Result, Error>) -> Void) throws { - guard let delegate = self.delegate as? NetworkGraphQLErrorDelegate, + guard let delegate = self.delegate as? HTTPNetworkTransportGraphQLErrorDelegate, let graphQLErrors = response.parseErrorsOnlyFast(), !graphQLErrors.isEmpty else { completionHandler(.success(response)) From 05aeb47716318eb9bcbcf3666d14a6003e0d383e Mon Sep 17 00:00:00 2001 From: Kim de Vos Date: Tue, 15 Oct 2019 15:14:46 +0200 Subject: [PATCH 05/11] Fixed conflicts after rebase --- Sources/Apollo/GraphQLResponse.swift | 8 ------ Sources/Apollo/HTTPNetworkTransport.swift | 30 ++++++++++++++++++----- 2 files changed, 24 insertions(+), 14 deletions(-) diff --git a/Sources/Apollo/GraphQLResponse.swift b/Sources/Apollo/GraphQLResponse.swift index abd2795ddb..9f68473fd6 100644 --- a/Sources/Apollo/GraphQLResponse.swift +++ b/Sources/Apollo/GraphQLResponse.swift @@ -56,12 +56,4 @@ public final class GraphQLResponse { return GraphQLResult(data: nil, errors: errors, source: .server, dependentKeys: nil) } } - - func parseErrorsOnlyFast() -> [GraphQLError]? { - guard let errorsEntry = self.body["errors"] as? [JSONObject] else { - return nil - } - - return errorsEntry.map(GraphQLError.init) - } } diff --git a/Sources/Apollo/HTTPNetworkTransport.swift b/Sources/Apollo/HTTPNetworkTransport.swift index 8e1dd3d035..97c8a33455 100644 --- a/Sources/Apollo/HTTPNetworkTransport.swift +++ b/Sources/Apollo/HTTPNetworkTransport.swift @@ -145,7 +145,7 @@ public class HTTPNetworkTransport { error: error) if let receivedError = error { - self?.handleErrorOrRetry(operation: operation, + self.handleErrorOrRetry(operation: operation, files: files, error: receivedError, for: request, @@ -162,7 +162,7 @@ public class HTTPNetworkTransport { let unsuccessfulError = GraphQLHTTPResponseError(body: data, response: httpResponse, kind: .errorResponse) - self?.handleErrorOrRetry(operation: operation, + self.handleErrorOrRetry(operation: operation, files: files, error: unsuccessfulError, for: request, @@ -175,7 +175,7 @@ public class HTTPNetworkTransport { let error = GraphQLHTTPResponseError(body: nil, response: httpResponse, kind: .invalidResponse) - self?.handleErrorOrRetry(operation: operation, + self.handleErrorOrRetry(operation: operation, files: files, error: error, for: request, @@ -188,7 +188,9 @@ public class HTTPNetworkTransport { 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, @@ -200,7 +202,7 @@ public class HTTPNetworkTransport { completionHandler(.success(graphQLResponse)) } } catch let parsingError { - self?.handleErrorOrRetry(operation: operation, + self.handleErrorOrRetry(operation: operation, files: files, error: parsingError, for: request, @@ -226,12 +228,20 @@ public class HTTPNetworkTransport { } delegate.networkTransport(self, receivedGraphQLErrors: graphQLErrors, retryHandler: { [weak self] shouldRetry in + guard let self = self else { + // None of the rest of this really matters + return + } + guard shouldRetry else { completionHandler(.success(response)) return } - _ = self?.send(operation: operation, files: files, completionHandler: completionHandler) + _ = self.send(operation: operation, + isPersistedQueryRetry: self.enableAutoPersistedQueries, + files: files, + completionHandler: completionHandler) }) } @@ -275,12 +285,20 @@ public class HTTPNetworkTransport { for: request, response: response, retryHandler: { [weak self] shouldRetry in + guard let self = self else { + // None of the rest of this really matters + return + } + guard shouldRetry else { completionHandler(.failure(error)) return } - _ = self?.send(operation: operation, files: files, completionHandler: completionHandler) + _ = self.send(operation: operation, + isPersistedQueryRetry: self.enableAutoPersistedQueries, + files: files, + completionHandler: completionHandler) }) } From 8e5516ac906d279fb01e30c9a119c2cafe0ca277 Mon Sep 17 00:00:00 2001 From: Kim de Vos Date: Wed, 16 Oct 2019 07:54:43 +0200 Subject: [PATCH 06/11] Fix indentation --- Sources/Apollo/HTTPNetworkTransport.swift | 56 +++++++++++------------ 1 file changed, 28 insertions(+), 28 deletions(-) diff --git a/Sources/Apollo/HTTPNetworkTransport.swift b/Sources/Apollo/HTTPNetworkTransport.swift index 97c8a33455..34a7d3f2d7 100644 --- a/Sources/Apollo/HTTPNetworkTransport.swift +++ b/Sources/Apollo/HTTPNetworkTransport.swift @@ -146,11 +146,11 @@ public class HTTPNetworkTransport { if let receivedError = error { self.handleErrorOrRetry(operation: operation, - files: files, - error: receivedError, - for: request, - response: response, - completionHandler: completionHandler) + files: files, + error: receivedError, + for: request, + response: response, + completionHandler: completionHandler) return } @@ -163,11 +163,11 @@ public class HTTPNetworkTransport { response: httpResponse, kind: .errorResponse) self.handleErrorOrRetry(operation: operation, - files: files, - error: unsuccessfulError, - for: request, - response: response, - completionHandler: completionHandler) + files: files, + error: unsuccessfulError, + for: request, + response: response, + completionHandler: completionHandler) return } @@ -176,11 +176,11 @@ public class HTTPNetworkTransport { response: httpResponse, kind: .invalidResponse) self.handleErrorOrRetry(operation: operation, - files: files, - error: error, - for: request, - response: response, - completionHandler: completionHandler) + files: files, + error: error, + for: request, + response: response, + completionHandler: completionHandler) return } @@ -188,9 +188,9 @@ public class HTTPNetworkTransport { 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, @@ -203,11 +203,11 @@ public class HTTPNetworkTransport { } } catch let parsingError { self.handleErrorOrRetry(operation: operation, - files: files, - error: parsingError, - for: request, - response: response, - completionHandler: completionHandler) + files: files, + error: parsingError, + for: request, + response: response, + completionHandler: completionHandler) } } @@ -254,11 +254,11 @@ public class HTTPNetworkTransport { let errorMessages = errors.compactMap { $0.message } if self.enableAutoPersistedQueries, errorMessages.contains("PersistedQueryNotFound") { - // We need to retry this with the full body. - _ = self.send(operation: operation, - isPersistedQueryRetry: true, - files: nil, - completionHandler: completionHandler) + // We need to retry this with the full body. + _ = self.send(operation: operation, + isPersistedQueryRetry: true, + files: nil, + completionHandler: completionHandler) } else { // Pass the response on to the rest of the chain let response = GraphQLResponse(operation: operation, body: body) @@ -346,7 +346,7 @@ public class HTTPNetworkTransport { sendQueryDocument: sendQueryDocument, autoPersistQueries: autoPersistQueries) } - + private func createRequest(for operation: Operation, files: [GraphQLFile]?, httpMethod: GraphQLHTTPMethod, From 980527de05f54594b369e4f2d80bd9a4f41aca46 Mon Sep 17 00:00:00 2001 From: Kim de Vos Date: Wed, 16 Oct 2019 10:17:58 +0200 Subject: [PATCH 07/11] Added files for retry --- Sources/Apollo/HTTPNetworkTransport.swift | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/Sources/Apollo/HTTPNetworkTransport.swift b/Sources/Apollo/HTTPNetworkTransport.swift index 34a7d3f2d7..4c333f7b1b 100644 --- a/Sources/Apollo/HTTPNetworkTransport.swift +++ b/Sources/Apollo/HTTPNetworkTransport.swift @@ -194,6 +194,7 @@ public class HTTPNetworkTransport { if let errors = graphQLResponse.parseErrorsOnlyFast() { // Handle specific errors from response self.handleGraphQLErrorsIfNeeded(operation: operation, + files: files, for: request, body: body, errors: errors, @@ -219,7 +220,7 @@ public class HTTPNetworkTransport { private func handleGraphQLErrorsOrComplete(operation: Operation, files: [GraphQLFile]?, response: GraphQLResponse, - completionHandler: @escaping (_ result: Result, Error>) -> Void) throws { + completionHandler: @escaping (_ result: Result, Error>) -> Void) { guard let delegate = self.delegate as? HTTPNetworkTransportGraphQLErrorDelegate, let graphQLErrors = response.parseErrorsOnlyFast(), !graphQLErrors.isEmpty else { @@ -246,11 +247,12 @@ public class HTTPNetworkTransport { } private func handleGraphQLErrorsIfNeeded(operation: Operation, + files: [GraphQLFile]?, for request: URLRequest, body: JSONObject, errors: [GraphQLError], completionHandler: @escaping (_ results: Result, Error>) -> Void) { - + let errorMessages = errors.compactMap { $0.message } if self.enableAutoPersistedQueries, errorMessages.contains("PersistedQueryNotFound") { @@ -262,10 +264,10 @@ public class HTTPNetworkTransport { } else { // Pass the response on to the rest of the chain let response = GraphQLResponse(operation: operation, body: body) - completionHandler(.success(response)) + handleGraphQLErrorsOrComplete(operation: operation, files: files, response: response, completionHandler: completionHandler) } } - + private func handleErrorOrRetry(operation: Operation, files: [GraphQLFile]?, error: Error, From 18f25da9ea74dec27e369bb7f402e50ee2949667 Mon Sep 17 00:00:00 2001 From: Kim de Vos Date: Wed, 16 Oct 2019 10:18:28 +0200 Subject: [PATCH 08/11] Added tests for HTTPNetworkTransportGraphQLErrorDelegate --- Tests/ApolloTestSupport/MockURLSession.swift | 8 ++- Tests/ApolloTests/HTTPTransportTests.swift | 57 +++++++++++++++++++- 2 files changed, 63 insertions(+), 2 deletions(-) diff --git a/Tests/ApolloTestSupport/MockURLSession.swift b/Tests/ApolloTestSupport/MockURLSession.swift index 7562119a9a..f49f65d062 100644 --- a/Tests/ApolloTestSupport/MockURLSession.swift +++ b/Tests/ApolloTestSupport/MockURLSession.swift @@ -9,7 +9,11 @@ import Foundation public final class MockURLSession: URLSession { public private (set) var lastRequest: URLRequest? - + + public var data: Data? + public var response: URLResponse? + public var error: Error? + override public func dataTask(with request: URLRequest) -> URLSessionDataTask { lastRequest = request return URLSessionDataTaskMock() @@ -17,11 +21,13 @@ public final class MockURLSession: URLSession { override public func dataTask(with request: URLRequest, completionHandler: @escaping (Data?, URLResponse?, Error?) -> Void) -> URLSessionDataTask { lastRequest = request + completionHandler(data, response, error) return URLSessionDataTaskMock() } } private final class URLSessionDataTaskMock: URLSessionDataTask { override func resume() { + } } diff --git a/Tests/ApolloTests/HTTPTransportTests.swift b/Tests/ApolloTests/HTTPTransportTests.swift index 1efb8fd8d1..d56fd0759d 100644 --- a/Tests/ApolloTests/HTTPTransportTests.swift +++ b/Tests/ApolloTests/HTTPTransportTests.swift @@ -9,6 +9,7 @@ import XCTest @testable import Apollo import StarWarsAPI +import ApolloTestSupport class HTTPTransportTests: XCTestCase { @@ -23,6 +24,8 @@ class HTTPTransportTests: XCTestCase { private var shouldModifyURLInWillSend = false private var retryCount = 0 + private var graphQlErrors = [GraphQLError]() + private lazy var url = URL(string: "http://localhost:8080/graphql")! private lazy var networkTransport = HTTPNetworkTransport(url: self.url, useGETForQueries: true, @@ -59,7 +62,7 @@ class HTTPTransportTests: XCTestCase { line: line) } } - + func testPreflightDelegateTellingRequestNotToSend() { self.shouldSend = false @@ -193,6 +196,50 @@ class HTTPTransportTests: XCTestCase { delegate: self) XCTAssertNotEqual(self.networkTransport, nonIdenticalTransport) } + + func testErrorDelegateWithErrors() throws { + self.graphQlErrors = [] + let query = HeroNameQuery() + 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 network = HTTPNetworkTransport(url: url, session: mockSession, delegate: self) + + let _ = network.send(operation: query) { _ in } + + guard let request = mockSession.lastRequest else { + XCTFail("last request should not be nil") + return + } + XCTAssertEqual(request.url?.host, network.url.host) + XCTAssertEqual(request.httpMethod, "POST") + + XCTAssertEqual(self.graphQlErrors.count, 1) + } + + func testErrorDelegateWithNoErrors() throws { + self.graphQlErrors = [] + let query = HeroNameQuery() + 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 network = HTTPNetworkTransport(url: url, session: mockSession, delegate: self) + + let _ = network.send(operation: query) { _ in } + + guard let request = mockSession.lastRequest else { + XCTFail("last request should not be nil") + return + } + XCTAssertEqual(request.url?.host, network.url.host) + XCTAssertEqual(request.httpMethod, "POST") + + XCTAssertEqual(self.graphQlErrors.count, 0) + } } // MARK: - HTTPNetworkTransportPreflightDelegate @@ -266,3 +313,11 @@ extension HTTPTransportTests: HTTPNetworkTransportRetryDelegate { } } } + +// MARK: - HTTPNetworkTransportGraphQLErrorDelegate + +extension HTTPTransportTests: HTTPNetworkTransportGraphQLErrorDelegate { + func networkTransport(_ networkTransport: HTTPNetworkTransport, receivedGraphQLErrors errors: [GraphQLError], retryHandler: @escaping (Bool) -> Void) { + self.graphQlErrors = errors + } +} From cb8d93e2e877d0e2af11d940d2ae06858b9bc911 Mon Sep 17 00:00:00 2001 From: Kim de Vos Date: Wed, 16 Oct 2019 14:04:34 +0200 Subject: [PATCH 09/11] Only run completion if response provided. --- Tests/ApolloTestSupport/MockURLSession.swift | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Tests/ApolloTestSupport/MockURLSession.swift b/Tests/ApolloTestSupport/MockURLSession.swift index f49f65d062..1ab1399759 100644 --- a/Tests/ApolloTestSupport/MockURLSession.swift +++ b/Tests/ApolloTestSupport/MockURLSession.swift @@ -11,7 +11,7 @@ public final class MockURLSession: URLSession { public private (set) var lastRequest: URLRequest? public var data: Data? - public var response: URLResponse? + public var response: HTTPURLResponse? public var error: Error? override public func dataTask(with request: URLRequest) -> URLSessionDataTask { @@ -21,7 +21,9 @@ public final class MockURLSession: URLSession { override public func dataTask(with request: URLRequest, completionHandler: @escaping (Data?, URLResponse?, Error?) -> Void) -> URLSessionDataTask { lastRequest = request - completionHandler(data, response, error) + if let response = response { + completionHandler(data, response, error) + } return URLSessionDataTaskMock() } } From ec5ede7d2ba999b5976ab8bdfc9452d450e2b642 Mon Sep 17 00:00:00 2001 From: Kim de Vos Date: Fri, 18 Oct 2019 10:46:30 +0200 Subject: [PATCH 10/11] Added expectation to retry error tests and retryHandler --- Tests/ApolloTests/HTTPTransportTests.swift | 31 +++++++++++++++++++--- 1 file changed, 28 insertions(+), 3 deletions(-) diff --git a/Tests/ApolloTests/HTTPTransportTests.swift b/Tests/ApolloTests/HTTPTransportTests.swift index d56fd0759d..7be7498c53 100644 --- a/Tests/ApolloTests/HTTPTransportTests.swift +++ b/Tests/ApolloTests/HTTPTransportTests.swift @@ -198,6 +198,7 @@ class HTTPTransportTests: XCTestCase { } func testErrorDelegateWithErrors() throws { + self.retryCount = 0 self.graphQlErrors = [] let query = HeroNameQuery() let body = ["errors": [["message": "Test graphql error"]]] @@ -206,8 +207,16 @@ class HTTPTransportTests: XCTestCase { mockSession.response = HTTPURLResponse(url: url, statusCode: 200, httpVersion: nil, headerFields: nil) mockSession.data = try JSONSerialization.data(withJSONObject: body, options: .prettyPrinted) let network = HTTPNetworkTransport(url: url, session: mockSession, delegate: self) + let expectation = self.expectation(description: "Send operation completed") - let _ = network.send(operation: query) { _ in } + let _ = network.send(operation: query) { result in + switch result { + case .success: + expectation.fulfill() + case .failure: + break + } + } guard let request = mockSession.lastRequest else { XCTFail("last request should not be nil") @@ -217,9 +226,12 @@ class HTTPTransportTests: XCTestCase { XCTAssertEqual(request.httpMethod, "POST") XCTAssertEqual(self.graphQlErrors.count, 1) + XCTAssertEqual(retryCount, 1) + wait(for: [expectation], timeout: 1) } func testErrorDelegateWithNoErrors() throws { + self.retryCount = 0 self.graphQlErrors = [] let query = HeroNameQuery() let body = ["errors": []] @@ -228,8 +240,16 @@ class HTTPTransportTests: XCTestCase { mockSession.response = HTTPURLResponse(url: url, statusCode: 200, httpVersion: nil, headerFields: nil) mockSession.data = try JSONSerialization.data(withJSONObject: body, options: .prettyPrinted) let network = HTTPNetworkTransport(url: url, session: mockSession, delegate: self) + let expectation = self.expectation(description: "Send operation completed") - let _ = network.send(operation: query) { _ in } + let _ = network.send(operation: query) { result in + switch result { + case .success: + expectation.fulfill() + case .failure: + break + } + } guard let request = mockSession.lastRequest else { XCTFail("last request should not be nil") @@ -237,8 +257,10 @@ class HTTPTransportTests: XCTestCase { } XCTAssertEqual(request.url?.host, network.url.host) XCTAssertEqual(request.httpMethod, "POST") - + XCTAssertEqual(self.retryCount, 0) XCTAssertEqual(self.graphQlErrors.count, 0) + wait(for: [expectation], timeout: 1) + } } @@ -318,6 +340,9 @@ extension HTTPTransportTests: HTTPNetworkTransportRetryDelegate { extension HTTPTransportTests: HTTPNetworkTransportGraphQLErrorDelegate { func networkTransport(_ networkTransport: HTTPNetworkTransport, receivedGraphQLErrors errors: [GraphQLError], retryHandler: @escaping (Bool) -> Void) { + self.retryCount += 1 + let shouldRetry = retryCount == 2 self.graphQlErrors = errors + retryHandler(shouldRetry) } } From 2e8d94a6e7de846239c9bbebf4724b5e78280879 Mon Sep 17 00:00:00 2001 From: Kim de Vos Date: Tue, 29 Oct 2019 18:05:43 +0100 Subject: [PATCH 11/11] Added TODOs --- Tests/ApolloTests/HTTPTransportTests.swift | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Tests/ApolloTests/HTTPTransportTests.swift b/Tests/ApolloTests/HTTPTransportTests.swift index 7be7498c53..7310a1d630 100644 --- a/Tests/ApolloTests/HTTPTransportTests.swift +++ b/Tests/ApolloTests/HTTPTransportTests.swift @@ -201,6 +201,7 @@ class HTTPTransportTests: XCTestCase { self.retryCount = 0 self.graphQlErrors = [] let query = HeroNameQuery() + // 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() @@ -234,6 +235,7 @@ class HTTPTransportTests: XCTestCase { self.retryCount = 0 self.graphQlErrors = [] let query = HeroNameQuery() + // TODO: Replace this with once it is codable https://github.com/apollographql/apollo-ios/issues/467 let body = ["errors": []] let mockSession = MockURLSession()