Merge pull request #644 from apollographql/change/result-type-returns

`Result` ALL THE THINGS!!!

Author: designatednerd

Sources/Apollo/ApolloClient.swift

@@ -18,12 +18,8 @@ public enum CachePolicy {
 /// A handler for operation results.
 ///
 /// - Parameters:
-///   - result: The result of the performed operation, or `nil` if an error occurred.
-///   - error: An error that indicates why the mutation failed, or `nil` if the mutation was succesful.
-public typealias GraphQLResultHandler<Data> = (_ result: GraphQLResult<Data>?, _ error: Error?) -> Void
-
-@available(*, deprecated, renamed: "GraphQLResultHandler")
-public typealias OperationResultHandler<Operation: GraphQLOperation> = GraphQLResultHandler<Operation.Data>
+///   - result: The result of a performed operation. Will have a `GraphQLResult` with any parsed data and any GraphQL errors on `success`, and an `Error` on `failure`.
+public typealias GraphQLResultHandler<Data> = (Result<GraphQLResult<Data>, Error>) -> Void
 
 /// The `ApolloClient` class provides the core API for Apollo. This API provides methods to fetch and watch queries, and to perform mutations.
 public class ApolloClient {
@@ -103,7 +99,6 @@ public class ApolloClient {
   ///   - queue: A dispatch queue on which the result handler will be called. Defaults to the main queue.
   ///   - resultHandler: An optional closure that is called when query results are available or when an error occurs.
   /// - Returns: A query watcher object that can be used to control the watching behavior.
-  
   public func watch<Query: GraphQLQuery>(query: Query, cachePolicy: CachePolicy = .returnCacheDataElseFetch, queue: DispatchQueue = DispatchQueue.main, resultHandler: @escaping GraphQLResultHandler<Query.Data>) -> GraphQLQueryWatcher<Query> {
     let watcher = GraphQLQueryWatcher(client: self, query: query, resultHandler: wrapResultHandler(resultHandler, queue: queue))
     watcher.fetch(cachePolicy: cachePolicy)
@@ -135,47 +130,47 @@ public class ApolloClient {
   }
 
   fileprivate func send<Operation: GraphQLOperation>(operation: Operation, shouldPublishResultToStore: Bool, context: UnsafeMutableRawPointer?, resultHandler: @escaping GraphQLResultHandler<Operation.Data>) -> Cancellable {
-    return networkTransport.send(operation: operation) { (response, error) in
-      guard let response = response else {
-        resultHandler(nil, error)
-        return
-      }
-      
-      // If there is no need to publish the result to the store, we can use a fast path.
-      if !shouldPublishResultToStore {
-        do {
-          let result = try response.parseResultFast()
-          resultHandler(result, nil)
-        } catch {
-          resultHandler(nil, error)
-        }
-        return
-      }
-      
-      firstly {
-        try response.parseResult(cacheKeyForObject: self.cacheKeyForObject)
-      }.andThen { (result, records) in
-        if let records = records {
-          self.store.publish(records: records, context: context).catch { error in
-            preconditionFailure(String(describing: error))
+    return networkTransport.send(operation: operation) { result in
+      switch result {
+      case .failure(let error):
+        resultHandler(.failure(error))
+      case .success(let response):
+        // If there is no need to publish the result to the store, we can use a fast path.
+        if !shouldPublishResultToStore {
+          do {
+            let result = try response.parseResultFast()
+            resultHandler(.success(result))
+          } catch {
+            resultHandler(.failure(error))
           }
+          return
+        }
+        
+        firstly {
+          try response.parseResult(cacheKeyForObject: self.cacheKeyForObject)
+          }.andThen { (result, records) in
+            if let records = records {
+              self.store.publish(records: records, context: context).catch { error in
+                preconditionFailure(String(describing: error))
+              }
+            }
+            resultHandler(.success(result))
+          }.catch { error in
+            resultHandler(.failure(error))
         }
-        resultHandler(result, nil)
-      }.catch { error in
-        resultHandler(nil, error)
       }
     }
   }
 }
 
 private func wrapResultHandler<Data>(_ resultHandler: GraphQLResultHandler<Data>?, queue handlerQueue: DispatchQueue) -> GraphQLResultHandler<Data> {
   guard let resultHandler = resultHandler else {
-    return { _, _ in }
+    return { _ in }
   }
 
-  return { (result, error) in
+  return { result in
     handlerQueue.async {
-      resultHandler(result, error)
+      resultHandler(result)
     }
   }
 }
@@ -210,34 +205,35 @@ private final class FetchQueryOperation<Query: GraphQLQuery>: AsynchronousOperat
       return
     }
 
-    client.store.load(query: query) { (result, error) in
-      if error == nil {
-        self.resultHandler(result, nil)
-        
-        if self.cachePolicy != .returnCacheDataAndFetch {
-          self.state = .finished
-          return
-        }
-      }
-      
+    client.store.load(query: query) { result in
       if self.isCancelled {
         self.state = .finished
         return
       }
 
-      if self.cachePolicy == .returnCacheDataDontFetch {
-        self.resultHandler(nil, nil)
-        self.state = .finished
-        return
+      switch result {
+      case .success:
+        self.resultHandler(result)
+        
+        if self.cachePolicy != .returnCacheDataAndFetch {
+          self.state = .finished
+          return
+        }
+      case .failure:
+        if self.cachePolicy == .returnCacheDataDontFetch {
+          self.resultHandler(result)
+          self.state = .finished
+          return
+        }
       }
 
       self.fetchFromNetwork()
     }
   }
 
   func fetchFromNetwork() {
-    networkTask = client.send(operation: query, shouldPublishResultToStore: true, context: context) { (result, error) in
-      self.resultHandler(result, error)
+    networkTask = client.send(operation: query, shouldPublishResultToStore: true, context: context) { result in
+      self.resultHandler(result)
       self.state = .finished
       return
     }

Sources/Apollo/ApolloStore.swift

@@ -131,9 +131,9 @@ public final class ApolloStore {
 
   public func load<Query: GraphQLQuery>(query: Query, resultHandler: @escaping GraphQLResultHandler<Query.Data>) {
     load(query: query).andThen { result in
-      resultHandler(result, nil)
+      resultHandler(.success(result))
     }.catch { error in
-      resultHandler(nil, error)
+      resultHandler(.failure(error))
     }
   }
 

Sources/Apollo/GraphQLQueryWatcher.swift

@@ -26,11 +26,17 @@ public final class GraphQLQueryWatcher<Query: GraphQLQuery>: Cancellable, Apollo
   }
 
   func fetch(cachePolicy: CachePolicy) {
-    fetching = client?.fetch(query: query, cachePolicy: cachePolicy, context: &context) { [weak self] (result, error) in
+    fetching = client?.fetch(query: query, cachePolicy: cachePolicy, context: &context) { [weak self] result in
       guard let `self` = self else { return }
-        
-      self.dependentKeys = result?.dependentKeys
-      self.resultHandler(result, error)
+      
+      switch result {
+      case .success(let graphQLResult):
+        self.dependentKeys = graphQLResult.dependentKeys
+      case .failure:
+        break
+      }
+      
+      self.resultHandler(result)
     }
   }
 

Sources/Apollo/HTTPNetworkTransport.swift

@@ -68,7 +68,7 @@ public protocol HTTPNetworkTransportRetryDelegate: HTTPNetworkTransportDelegate
 // 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 {
+public class HTTPNetworkTransport {
   let url: URL
   let session: URLSession
   let serializationFormat = JSONSerializationFormat.self
@@ -96,35 +96,23 @@ public class HTTPNetworkTransport: NetworkTransport {
     self.delegate = delegate
   }
 
-  /// Send a GraphQL operation to a server and return a response.
-  ///
-  /// - Parameters:
-  ///   - operation: The operation to send.
-  ///   - completionHandler: A closure to call when a request completes.
-  ///   - response: The response received from the server, or `nil` if an error occurred.
-  ///   - error: An error that indicates why a request failed, or `nil` if the request was succesful.
-  /// - Returns: An object that can be used to cancel an in progress request.
-  public func send<Operation>(operation: Operation, completionHandler: @escaping (_ response: GraphQLResponse<Operation>?, _ error: Error?) -> Void) -> Cancellable {
-    return send(operation: operation, files: nil, completionHandler: completionHandler)
-  }
-  
   /// Uploads the given files with the given operation. 
   ///
   /// - Parameters:
   ///   - operation: The operation to send
   ///   - files: An array of `GraphQLFile` objects to send.
   ///   - completionHandler: The completion handler to execute when the request completes or errors
   /// - Returns: An object that can be used to cancel an in progress request.
-  public func upload<Operation>(operation: Operation, files: [GraphQLFile], completionHandler: @escaping (_ response: GraphQLResponse<Operation>?, _ error: Error?) -> Void) -> Cancellable {
+  public func upload<Operation>(operation: Operation, files: [GraphQLFile], completionHandler: @escaping (_ result: Result<GraphQLResponse<Operation>, Error>) -> Void) -> Cancellable {
     return send(operation: operation, files: files, completionHandler: completionHandler)
   }
 
-  private func send<Operation>(operation: Operation, files: [GraphQLFile]?, completionHandler: @escaping (_ response: GraphQLResponse<Operation>?, _ error: Error?) -> Void) -> Cancellable {
+  private func send<Operation>(operation: Operation, files: [GraphQLFile]?, completionHandler: @escaping (_ results: Result<GraphQLResponse<Operation>, Error>) -> Void) -> Cancellable {
     let request: URLRequest
     do {
       request = try self.createRequest(for: operation, files: files)
     } catch {
-      completionHandler(nil, error)
+      completionHandler(.failure(error))
       return EmptyCancellable()
     }
 
@@ -176,7 +164,7 @@ public class HTTPNetworkTransport: NetworkTransport {
           throw GraphQLHTTPResponseError(body: data, response: httpResponse, kind: .invalidResponse)
         }
         let response = GraphQLResponse(operation: operation, body: body)
-        completionHandler(response, nil)
+        completionHandler(.success(response))
       } catch let parsingError {
         self?.handleErrorOrRetry(operation: operation,
                                  error: parsingError,
@@ -195,11 +183,11 @@ public class HTTPNetworkTransport: NetworkTransport {
                                              error: Error,
                                              for request: URLRequest,
                                              response: URLResponse?,
-                                             completionHandler: @escaping (_ response: GraphQLResponse<Operation>?, _ error: Error?) -> Void) {
+                                             completionHandler: @escaping (_ result: Result<GraphQLResponse<Operation>, Error>) -> Void) {
     guard
       let delegate = self.delegate,
       let retrier = delegate as? HTTPNetworkTransportRetryDelegate else {
-        completionHandler(nil, error)
+        completionHandler(.failure(error))
         return
     }
 
@@ -210,7 +198,7 @@ public class HTTPNetworkTransport: NetworkTransport {
       response: response,
       retryHandler: { [weak self] shouldRetry in
         guard shouldRetry else {
-          completionHandler(nil, error)
+          completionHandler(.failure(error))
           return
         }
 
@@ -290,3 +278,12 @@ public class HTTPNetworkTransport: NetworkTransport {
     return request
   }
 }
+
+// MARK: - NetworkTransport conformance
+
+extension HTTPNetworkTransport: NetworkTransport {
+  
+  public func send<Operation>(operation: Operation, completionHandler: @escaping (_ result: Result<GraphQLResponse<Operation>, Error>) -> Void) -> Cancellable {
+    return send(operation: operation, files: nil, completionHandler: completionHandler)
+  }
+}

Sources/Apollo/NetworkTransport.swift

@@ -1,12 +1,11 @@
 /// A network transport is responsible for sending GraphQL operations to a server.
 public protocol NetworkTransport {
+  
   /// Send a GraphQL operation to a server and return a response.
   ///
   /// - Parameters:
   ///   - operation: The operation to send.
-  ///   - completionHandler: A closure to call when a request completes.
-  ///   - response: The response received from the server, or `nil` if an error occurred.
-  ///   - error: An error that indicates why a request failed, or `nil` if the request was succesful.
+  ///   - completionHandler: A closure to call when a request completes. On `success` will contain the response received from the server. On `failure` will contain the error which occurred.
   /// - Returns: An object that can be used to cancel an in progress request.
-  func send<Operation>(operation: Operation, completionHandler: @escaping (_ response: GraphQLResponse<Operation>?, _ error: Error?) -> Void) -> Cancellable
+  func send<Operation>(operation: Operation, completionHandler: @escaping (_ result: Result<GraphQLResponse<Operation>, Error>) -> Void) -> Cancellable
 }

Sources/ApolloWebSocket/SplitNetworkTransport.swift

@@ -2,21 +2,31 @@
 import Apollo
 #endif
 
-
-public class SplitNetworkTransport: NetworkTransport {
+/// A network transport that sends subscriptions using one `NetworkTransport` and other requests using another `NetworkTransport`. Ideal for sending subscriptions via a web socket but everything else via HTTP. 
+public class SplitNetworkTransport {
   private let httpNetworkTransport: NetworkTransport
   private let webSocketNetworkTransport: NetworkTransport
 
+  /// Designated initializer
+  ///
+  /// - Parameters:
+  ///   - httpNetworkTransport: A `NetworkTransport` to use for non-subscription requests. Should generally be a `HTTPNetworkTransport` or something similar.
+  ///   - webSocketNetworkTransport: A `NetworkTransport` to use for subscription requests. Should generally be a `WebSocketTransport` or something similar.
   public init(httpNetworkTransport: NetworkTransport, webSocketNetworkTransport: NetworkTransport) {
     self.httpNetworkTransport = httpNetworkTransport
     self.webSocketNetworkTransport = webSocketNetworkTransport
   }
+}
+
+// MARK: - NetworkTransport conformance
+
+extension SplitNetworkTransport: NetworkTransport {
 
-  public func send<Operation>(operation: Operation, completionHandler: @escaping (GraphQLResponse<Operation>?, Error?) -> Void) -> Cancellable {
+  public func send<Operation>(operation: Operation, completionHandler: @escaping (Result<GraphQLResponse<Operation>, Error>) -> Void) -> Cancellable {
     if operation.operationType == .subscription {
-        return webSocketNetworkTransport.send(operation: operation, completionHandler: completionHandler)
+      return webSocketNetworkTransport.send(operation: operation, completionHandler: completionHandler)
     } else {
-        return httpNetworkTransport.send(operation: operation, completionHandler: completionHandler)
+      return httpNetworkTransport.send(operation: operation, completionHandler: completionHandler)
     }
   }
 }