diff --git a/Apollo.xcodeproj/project.pbxproj b/Apollo.xcodeproj/project.pbxproj index 3260c052c5..4cea3b6500 100644 --- a/Apollo.xcodeproj/project.pbxproj +++ b/Apollo.xcodeproj/project.pbxproj @@ -785,7 +785,6 @@ E69BEDA52798B86D00000D10 /* InputObjectTemplate.swift in Sources */ = {isa = PBXBuildFile; fileRef = E69BEDA42798B86D00000D10 /* InputObjectTemplate.swift */; }; E69BEDA72798B89600000D10 /* InputObjectTemplateTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = E69BEDA62798B89600000D10 /* InputObjectTemplateTests.swift */; }; E69F436C29B81182006FF548 /* InterceptorRequestChain.swift in Sources */ = {isa = PBXBuildFile; fileRef = E69F436B29B81182006FF548 /* InterceptorRequestChain.swift */; }; - E69F436E29B812FC006FF548 /* ApolloInterceptorReentrantWrapper.swift in Sources */ = {isa = PBXBuildFile; fileRef = E69F436D29B812FC006FF548 /* ApolloInterceptorReentrantWrapper.swift */; }; E69F437229BBD958006FF548 /* MultipartResponseParsingInterceptor.swift in Sources */ = {isa = PBXBuildFile; fileRef = E69F437129BBD958006FF548 /* MultipartResponseParsingInterceptor.swift */; }; E6A6866427F63AEF008A1D13 /* FileGeneratorTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = E6A6866327F63AEF008A1D13 /* FileGeneratorTests.swift */; }; E6A6866627F63BDC008A1D13 /* FileGenerator_ResolvePath_Tests.swift in Sources */ = {isa = PBXBuildFile; fileRef = E6A6866527F63BDC008A1D13 /* FileGenerator_ResolvePath_Tests.swift */; }; @@ -1969,7 +1968,6 @@ E69BEDA42798B86D00000D10 /* InputObjectTemplate.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = InputObjectTemplate.swift; sourceTree = ""; }; E69BEDA62798B89600000D10 /* InputObjectTemplateTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = InputObjectTemplateTests.swift; sourceTree = ""; }; E69F436B29B81182006FF548 /* InterceptorRequestChain.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = InterceptorRequestChain.swift; sourceTree = ""; }; - E69F436D29B812FC006FF548 /* ApolloInterceptorReentrantWrapper.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ApolloInterceptorReentrantWrapper.swift; sourceTree = ""; }; E69F437129BBD958006FF548 /* MultipartResponseParsingInterceptor.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MultipartResponseParsingInterceptor.swift; sourceTree = ""; }; E6A6866327F63AEF008A1D13 /* FileGeneratorTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FileGeneratorTests.swift; sourceTree = ""; }; E6A6866527F63BDC008A1D13 /* FileGenerator_ResolvePath_Tests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = FileGenerator_ResolvePath_Tests.swift; sourceTree = ""; }; @@ -2327,7 +2325,6 @@ children = ( 9BC742AF24D09F880029282C /* DefaultInterceptors */, 9B260BEA245A020300562176 /* ApolloInterceptor.swift */, - E69F436D29B812FC006FF548 /* ApolloInterceptorReentrantWrapper.swift */, 9BC742AB24CFB2FF0029282C /* ApolloErrorInterceptor.swift */, 9B260C07245A437400562176 /* InterceptorProvider.swift */, ); @@ -5509,7 +5506,6 @@ DE100B18287F3FB100BE11C2 /* Documentation.docc in Sources */, 9FA6F3681E65DF4700BF8D73 /* GraphQLResultAccumulator.swift in Sources */, 9FF90A651DDDEB100034C3B6 /* GraphQLExecutor.swift in Sources */, - E69F436E29B812FC006FF548 /* ApolloInterceptorReentrantWrapper.swift in Sources */, 9BDE43DF22C6708600FD7C7F /* GraphQLHTTPRequestError.swift in Sources */, 9B1CCDD92360F02C007C9032 /* Bundle+Helpers.swift in Sources */, 9B260BF5245A028D00562176 /* HTTPResponse.swift in Sources */, diff --git a/Sources/Apollo/ApolloInterceptor.swift b/Sources/Apollo/ApolloInterceptor.swift index 007757f722..ea47cf9515 100644 --- a/Sources/Apollo/ApolloInterceptor.swift +++ b/Sources/Apollo/ApolloInterceptor.swift @@ -4,6 +4,8 @@ import ApolloAPI /// A protocol to set up a chainable unit of networking work. public protocol ApolloInterceptor { + + var id: String { get set } /// Called when this interceptor should do its work. /// diff --git a/Sources/Apollo/ApolloInterceptorReentrantWrapper.swift b/Sources/Apollo/ApolloInterceptorReentrantWrapper.swift deleted file mode 100644 index 0ef9b15936..0000000000 --- a/Sources/Apollo/ApolloInterceptorReentrantWrapper.swift +++ /dev/null @@ -1,107 +0,0 @@ -#if !COCOAPODS -import ApolloAPI -#endif - -/// A custom implementation of RequestChain that wraps an ApolloInterceptor instance to provide -/// re-entrant behaviour to the request chain. This is required for the request chain to support -/// network flows such as GraphQL Subscriptions where interceptors can call back into the request -/// chain multiple times. -class ApolloInterceptorReentrantWrapper: RequestChain { - @Atomic var isCancelled: Bool = false - - let wrappedInterceptor: ApolloInterceptor - let requestChain: Unmanaged - let index: Int - - init( - interceptor: ApolloInterceptor, - requestChain: Unmanaged, - index: Int - ) { - self.wrappedInterceptor = interceptor - self.requestChain = requestChain - self.index = index - } - - func kickoff( - request: HTTPRequest, - completion: @escaping (Result, Error>) -> Void - ) where Operation : GraphQLOperation { - requestChain.takeUnretainedValue().kickoff(request: request, completion: completion) - } - - func proceedAsync( - request: HTTPRequest, - response: HTTPResponse?, - completion: @escaping (Result, Error>) -> Void - ) where Operation : GraphQLOperation { - requestChain.takeUnretainedValue().proceedAsync( - request: request, - response: response, - completion: completion, - interceptor: self - ) - } - - func cancel() { - guard !self.isCancelled else { - // Do not proceed, this chain has been cancelled. - return - } - - self.$isCancelled.mutate { $0 = true } - - if let cancellableInterceptor = wrappedInterceptor as? Cancellable { - cancellableInterceptor.cancel() - } - } - - func retry( - request: HTTPRequest, - completion: @escaping (Result, Error>) -> Void - ) where Operation : GraphQLOperation { - requestChain.takeUnretainedValue().retry(request: request, completion: completion) - } - - func handleErrorAsync( - _ error: Error, - request: HTTPRequest, - response: HTTPResponse?, - completion: @escaping (Result, Error>) -> Void - ) where Operation : GraphQLOperation { - requestChain.takeUnretainedValue().handleErrorAsync( - error, - request: request, - response: response, - completion: completion - ) - } - - func returnValueAsync( - for request: HTTPRequest, - value: GraphQLResult, - completion: @escaping (Result, Error>) -> Void - ) where Operation : GraphQLOperation { - requestChain.takeUnretainedValue().returnValueAsync( - for: request, - value: value, - completion: completion - ) - } -} - -extension ApolloInterceptorReentrantWrapper : ApolloInterceptor { - func interceptAsync( - chain: RequestChain, - request: HTTPRequest, - response: HTTPResponse?, - completion: @escaping (Result, Error>) -> Void - ) where Operation : GraphQLOperation { - wrappedInterceptor.interceptAsync( - chain: self, - request: request, - response: response, - completion: completion - ) - } -} diff --git a/Sources/Apollo/AutomaticPersistedQueryInterceptor.swift b/Sources/Apollo/AutomaticPersistedQueryInterceptor.swift index 80cde13d69..0efbc00fdb 100644 --- a/Sources/Apollo/AutomaticPersistedQueryInterceptor.swift +++ b/Sources/Apollo/AutomaticPersistedQueryInterceptor.swift @@ -23,6 +23,8 @@ public struct AutomaticPersistedQueryInterceptor: ApolloInterceptor { } } } + + public var id: String = UUID().uuidString /// Designated initializer public init() {} @@ -36,35 +38,46 @@ public struct AutomaticPersistedQueryInterceptor: ApolloInterceptor { guard let jsonRequest = request as? JSONRequest, jsonRequest.autoPersistQueries else { // Not a request that handles APQs, continue along - chain.proceedAsync(request: request, - response: response, - completion: completion) + chain.proceedAsync( + request: request, + response: response, + interceptor: self, + completion: completion + ) return } guard let result = response?.parsedResponse else { // This is in the wrong order - this needs to be parsed before we can check it. - chain.handleErrorAsync(APQError.noParsedResponse, - request: request, - response: response, - completion: completion) + chain.handleErrorAsync( + APQError.noParsedResponse, + request: request, + response: response, + completion: completion + ) return } guard let errors = result.errors else { // No errors were returned so no retry is necessary, continue along. - chain.proceedAsync(request: request, - response: response, - completion: completion) + chain.proceedAsync( + request: request, + response: response, + interceptor: self, + completion: completion + ) return } let errorMessages = errors.compactMap { $0.message } guard errorMessages.contains("PersistedQueryNotFound") else { // The errors were not APQ errors, continue along. - chain.proceedAsync(request: request, - response: response, - completion: completion) + chain.proceedAsync( + request: request, + response: response, + interceptor: self, + completion: completion + ) return } @@ -93,7 +106,6 @@ public struct AutomaticPersistedQueryInterceptor: ApolloInterceptor { // We need to retry this query with the full body. jsonRequest.isPersistedQueryRetry = true - chain.retry(request: jsonRequest, - completion: completion) + chain.retry(request: jsonRequest, completion: completion) } } diff --git a/Sources/Apollo/CacheReadInterceptor.swift b/Sources/Apollo/CacheReadInterceptor.swift index 8be99e07e1..111a1df455 100644 --- a/Sources/Apollo/CacheReadInterceptor.swift +++ b/Sources/Apollo/CacheReadInterceptor.swift @@ -1,3 +1,4 @@ +import Foundation #if !COCOAPODS import ApolloAPI #endif @@ -6,6 +7,8 @@ import ApolloAPI public struct CacheReadInterceptor: ApolloInterceptor { private let store: ApolloStore + + public var id: String = UUID().uuidString /// Designated initializer /// @@ -24,17 +27,25 @@ public struct CacheReadInterceptor: ApolloInterceptor { case .mutation, .subscription: // Mutations and subscriptions don't need to hit the cache. - chain.proceedAsync(request: request, - response: response, - completion: completion) + chain.proceedAsync( + request: request, + response: response, + interceptor: self, + completion: completion + ) + case .query: switch request.cachePolicy { case .fetchIgnoringCacheCompletely, .fetchIgnoringCacheData: // Don't bother with the cache, just keep going - chain.proceedAsync(request: request, - response: response, - completion: completion) + chain.proceedAsync( + request: request, + response: response, + interceptor: self, + completion: completion + ) + case .returnCacheDataAndFetch: self.fetchFromCache(for: request, chain: chain) { cacheFetchResult in switch cacheFetchResult { @@ -42,29 +53,40 @@ public struct CacheReadInterceptor: ApolloInterceptor { // Don't return a cache miss error, just keep going break case .success(let graphQLResult): - chain.returnValueAsync(for: request, - value: graphQLResult, - completion: completion) + chain.returnValueAsync( + for: request, + value: graphQLResult, + completion: completion + ) } // In either case, keep going asynchronously - chain.proceedAsync(request: request, - response: response, - completion: completion) + chain.proceedAsync( + request: request, + response: response, + interceptor: self, + completion: completion + ) } case .returnCacheDataElseFetch: self.fetchFromCache(for: request, chain: chain) { cacheFetchResult in switch cacheFetchResult { case .failure: // Cache miss, proceed to network without returning error - chain.proceedAsync(request: request, - response: response, - completion: completion) + chain.proceedAsync( + request: request, + response: response, + interceptor: self, + completion: completion + ) + case .success(let graphQLResult): // Cache hit! We're done. - chain.returnValueAsync(for: request, - value: graphQLResult, - completion: completion) + chain.returnValueAsync( + for: request, + value: graphQLResult, + completion: completion + ) } } case .returnCacheDataDontFetch: @@ -72,14 +94,19 @@ public struct CacheReadInterceptor: ApolloInterceptor { switch cacheFetchResult { case .failure(let error): // Cache miss - don't hit the network, just return the error. - chain.handleErrorAsync(error, - request: request, - response: response, - completion: completion) + chain.handleErrorAsync( + error, + request: request, + response: response, + completion: completion + ) + case .success(let result): - chain.returnValueAsync(for: request, - value: result, - completion: completion) + chain.returnValueAsync( + for: request, + value: result, + completion: completion + ) } } } diff --git a/Sources/Apollo/CacheWriteInterceptor.swift b/Sources/Apollo/CacheWriteInterceptor.swift index 7e5ed0c26f..350e8845b1 100644 --- a/Sources/Apollo/CacheWriteInterceptor.swift +++ b/Sources/Apollo/CacheWriteInterceptor.swift @@ -18,6 +18,7 @@ public struct CacheWriteInterceptor: ApolloInterceptor { } public let store: ApolloStore + public var id: String = UUID().uuidString /// Designated initializer /// @@ -34,19 +35,24 @@ public struct CacheWriteInterceptor: ApolloInterceptor { guard request.cachePolicy != .fetchIgnoringCacheCompletely else { // If we're ignoring the cache completely, we're not writing to it. - chain.proceedAsync(request: request, - response: response, - completion: completion) + chain.proceedAsync( + request: request, + response: response, + interceptor: self, + completion: completion + ) return } guard let createdResponse = response, let legacyResponse = createdResponse.legacyResponse else { - chain.handleErrorAsync(CacheWriteError.noResponseToParse, - request: request, - response: response, - completion: completion) + chain.handleErrorAsync( + CacheWriteError.noResponseToParse, + request: request, + response: response, + completion: completion + ) return } @@ -61,14 +67,20 @@ public struct CacheWriteInterceptor: ApolloInterceptor { self.store.publish(records: records, identifier: request.contextIdentifier) } - chain.proceedAsync(request: request, - response: createdResponse, - completion: completion) + chain.proceedAsync( + request: request, + response: createdResponse, + interceptor: self, + completion: completion + ) + } catch { - chain.handleErrorAsync(error, - request: request, - response: response, - completion: completion) + chain.handleErrorAsync( + error, + request: request, + response: response, + completion: completion + ) } } } diff --git a/Sources/Apollo/DefaultInterceptorProvider.swift b/Sources/Apollo/DefaultInterceptorProvider.swift index 3cfff9b493..bef366f9a8 100644 --- a/Sources/Apollo/DefaultInterceptorProvider.swift +++ b/Sources/Apollo/DefaultInterceptorProvider.swift @@ -29,7 +29,9 @@ open class DefaultInterceptorProvider: InterceptorProvider { } } - open func interceptors(for operation: Operation) -> [ApolloInterceptor] { + open func interceptors( + for operation: Operation + ) -> [any ApolloInterceptor] { return [ MaxRetryInterceptor(), CacheReadInterceptor(store: self.store), diff --git a/Sources/Apollo/InterceptorProvider.swift b/Sources/Apollo/InterceptorProvider.swift index bd31104911..c2ec81ff88 100644 --- a/Sources/Apollo/InterceptorProvider.swift +++ b/Sources/Apollo/InterceptorProvider.swift @@ -10,7 +10,7 @@ public protocol InterceptorProvider { /// Creates a new array of interceptors when called /// /// - Parameter operation: The operation to provide interceptors for - func interceptors(for operation: Operation) -> [ApolloInterceptor] + func interceptors(for operation: Operation) -> [any ApolloInterceptor] /// Provides an additional error interceptor for any additional handling of errors /// before returning to the UI, such as logging. diff --git a/Sources/Apollo/InterceptorRequestChain.swift b/Sources/Apollo/InterceptorRequestChain.swift index ccee647a34..df6315e78f 100644 --- a/Sources/Apollo/InterceptorRequestChain.swift +++ b/Sources/Apollo/InterceptorRequestChain.swift @@ -9,6 +9,7 @@ final public class InterceptorRequestChain: Cancellable, RequestChain { public enum ChainError: Error, LocalizedError { case invalidIndex(chain: RequestChain, index: Int) case noInterceptors + case unknownInterceptor(id: String) public var errorDescription: String? { switch self { @@ -16,21 +17,19 @@ final public class InterceptorRequestChain: Cancellable, RequestChain { return "No interceptors were provided to this chain. This is a developer error." case .invalidIndex(_, let index): return "`proceedAsync` was called for index \(index), which is out of bounds of the receiver for this chain. Double-check the order of your interceptors." + case let .unknownInterceptor(id): + return "`proceedAsync` was called by unknown interceptor \(id)." } } } - private var interceptors: [ApolloInterceptorReentrantWrapper] - private var callbackQueue: DispatchQueue - @Atomic public var isCancelled: Bool = false + private let interceptors: [any ApolloInterceptor] + private let callbackQueue: DispatchQueue - private var managedSelf: Unmanaged! - // This is to fix #2932, caused by overreleasing managedSelf on cancellation of a query or - // mutation. It can now only be released once. - private lazy var releaseManagedSelf: Void = { - self.managedSelf.release() - }() + private var interceptorIndexes: [String: Int] = [:] + private var currentIndex: Int + @Atomic public var isCancelled: Bool = false /// Something which allows additional error handling to occur when some kind of error has happened. public var additionalErrorHandler: ApolloErrorInterceptor? @@ -41,20 +40,15 @@ final public class InterceptorRequestChain: Cancellable, RequestChain { /// - callbackQueue: The `DispatchQueue` to call back on when an error or result occurs. /// Defaults to `.main`. public init( - interceptors: [ApolloInterceptor], + interceptors: [any ApolloInterceptor], callbackQueue: DispatchQueue = .main ) { - self.interceptors = [] + self.interceptors = interceptors self.callbackQueue = callbackQueue + self.currentIndex = 0 - managedSelf = Unmanaged.passRetained(self) - - self.interceptors = interceptors.enumerated().map { (index, interceptor) in - ApolloInterceptorReentrantWrapper( - interceptor: interceptor, - requestChain: managedSelf, - index: index - ) + for (index, interceptor) in interceptors.enumerated() { + self.interceptorIndexes[interceptor.id] = index } } @@ -67,6 +61,8 @@ final public class InterceptorRequestChain: Cancellable, RequestChain { request: HTTPRequest, completion: @escaping (Result, Error>) -> Void ) { + assert(self.currentIndex == 0, "The interceptor index should be zero when calling this method") + guard let firstInterceptor = self.interceptors.first else { handleErrorAsync( ChainError.noInterceptors, @@ -97,8 +93,14 @@ final public class InterceptorRequestChain: Cancellable, RequestChain { response: HTTPResponse?, completion: @escaping (Result, Error>) -> Void ) { - // Empty implementation, proceedAsync(request:response:completion:interceptor:) should be - // used instead. + let nextIndex = self.currentIndex + 1 + + proceedAsync( + interceptorIndex: nextIndex, + request: request, + response: response, + completion: completion + ) } /// Proceeds to the next interceptor in the array. @@ -106,24 +108,50 @@ final public class InterceptorRequestChain: Cancellable, RequestChain { /// - Parameters: /// - request: The in-progress request object /// - response: [optional] The in-progress response object, if received yet - /// - completion: The completion closure to call when data has been processed and should be - /// returned to the UI. /// - interceptor: The interceptor that has completed processing and is ready to pass control /// on to the next interceptor in the chain. - func proceedAsync( + /// - completion: The completion closure to call when data has been processed and should be + /// returned to the UI. + public func proceedAsync( request: HTTPRequest, response: HTTPResponse?, - completion: @escaping (Result, Error>) -> Void, - interceptor: ApolloInterceptorReentrantWrapper + interceptor: any ApolloInterceptor, + completion: @escaping (Result, Error>) -> Void + ) { + guard let currentIndex = interceptorIndexes[interceptor.id] else { + self.handleErrorAsync( + ChainError.unknownInterceptor(id: interceptor.id), + request: request, + response: response, + completion: completion + ) + return + } + + let nextIndex = currentIndex + 1 + + proceedAsync( + interceptorIndex: nextIndex, + request: request, + response: response, + completion: completion + ) + } + + private func proceedAsync( + interceptorIndex: Int, + request: HTTPRequest, + response: HTTPResponse?, + completion: @escaping (Result, Error>) -> Void ) { guard !self.isCancelled else { // Do not proceed, this chain has been cancelled. return } - let nextIndex = interceptor.index + 1 - if self.interceptors.indices.contains(nextIndex) { - let interceptor = self.interceptors[nextIndex] + if self.interceptors.indices.contains(interceptorIndex) { + self.currentIndex = interceptorIndex + let interceptor = self.interceptors[interceptorIndex] interceptor.interceptAsync( chain: self, @@ -131,6 +159,7 @@ final public class InterceptorRequestChain: Cancellable, RequestChain { response: response, completion: completion ) + } else { if let result = response?.parsedResponse { // We got to the end of the chain with a parsed response. Yay! Return it. @@ -140,14 +169,10 @@ final public class InterceptorRequestChain: Cancellable, RequestChain { completion: completion ) - if Operation.operationType != .subscription { - _ = self.releaseManagedSelf - } } else { - // We got to the end of the chain and no parsed response is there, there needs to be more - // processing. + // We got to the end of the chain and no parsed response is there, there needs to be more processing. self.handleErrorAsync( - ChainError.invalidIndex(chain: self, index: nextIndex), + ChainError.invalidIndex(chain: self, index: interceptorIndex), request: request, response: response, completion: completion @@ -165,13 +190,12 @@ final public class InterceptorRequestChain: Cancellable, RequestChain { self.$isCancelled.mutate { $0 = true } - // If an interceptor adheres to `Cancellable`, it should have its in-flight work cancelled as - // well. + // If an interceptor adheres to `Cancellable`, it should have its in-flight work cancelled as well. for interceptor in self.interceptors { - interceptor.cancel() + if let cancellableInterceptor = interceptor as? Cancellable { + cancellableInterceptor.cancel() + } } - - _ = self.releaseManagedSelf } /// Restarts the request starting from the first interceptor. @@ -188,6 +212,7 @@ final public class InterceptorRequestChain: Cancellable, RequestChain { return } + self.currentIndex = 0 self.kickoff(request: request, completion: completion) } diff --git a/Sources/Apollo/JSONResponseParsingInterceptor.swift b/Sources/Apollo/JSONResponseParsingInterceptor.swift index 4b38d57dba..6e35ce917c 100644 --- a/Sources/Apollo/JSONResponseParsingInterceptor.swift +++ b/Sources/Apollo/JSONResponseParsingInterceptor.swift @@ -29,6 +29,8 @@ public struct JSONResponseParsingInterceptor: ApolloInterceptor { } } + public var id: String = UUID().uuidString + public init() { } public func interceptAsync( @@ -38,16 +40,19 @@ public struct JSONResponseParsingInterceptor: ApolloInterceptor { completion: @escaping (Result, Error>) -> Void ) { guard let createdResponse = response else { - chain.handleErrorAsync(JSONResponseParsingError.noResponseToParse, - request: request, - response: response, - completion: completion) + chain.handleErrorAsync( + JSONResponseParsingError.noResponseToParse, + request: request, + response: response, + completion: completion + ) return } do { - guard let body = try? JSONSerializationFormat - .deserialize(data: createdResponse.rawData) as? JSONObject else { + guard + let body = try? JSONSerializationFormat.deserialize(data: createdResponse.rawData) as? JSONObject + else { throw JSONResponseParsingError.couldNotParseToJSON(data: createdResponse.rawData) } @@ -57,15 +62,20 @@ public struct JSONResponseParsingInterceptor: ApolloInterceptor { let result = try parseResult(from: graphQLResponse, cachePolicy: request.cachePolicy) createdResponse.parsedResponse = result - chain.proceedAsync(request: request, - response: createdResponse, - completion: completion) + chain.proceedAsync( + request: request, + response: createdResponse, + interceptor: self, + completion: completion + ) } catch { - chain.handleErrorAsync(error, - request: request, - response: createdResponse, - completion: completion) + chain.handleErrorAsync( + error, + request: request, + response: createdResponse, + completion: completion + ) } } diff --git a/Sources/Apollo/MaxRetryInterceptor.swift b/Sources/Apollo/MaxRetryInterceptor.swift index 465f94837f..ed25961abb 100644 --- a/Sources/Apollo/MaxRetryInterceptor.swift +++ b/Sources/Apollo/MaxRetryInterceptor.swift @@ -8,6 +8,8 @@ public class MaxRetryInterceptor: ApolloInterceptor { private let maxRetries: Int private var hitCount = 0 + + public var id: String = UUID().uuidString public enum RetryError: Error, LocalizedError { case hitMaxRetryCount(count: Int, operationName: String) @@ -33,18 +35,27 @@ public class MaxRetryInterceptor: ApolloInterceptor { response: HTTPResponse?, completion: @escaping (Result, Error>) -> Void) { guard self.hitCount <= self.maxRetries else { - let error = RetryError.hitMaxRetryCount(count: self.maxRetries, - operationName: Operation.operationName) - chain.handleErrorAsync(error, - request: request, - response: response, - completion: completion) + let error = RetryError.hitMaxRetryCount( + count: self.maxRetries, + operationName: Operation.operationName + ) + + chain.handleErrorAsync( + error, + request: request, + response: response, + completion: completion + ) + return } self.hitCount += 1 - chain.proceedAsync(request: request, - response: response, - completion: completion) + chain.proceedAsync( + request: request, + response: response, + interceptor: self, + completion: completion + ) } } diff --git a/Sources/Apollo/MultipartResponseParsingInterceptor.swift b/Sources/Apollo/MultipartResponseParsingInterceptor.swift index 151bdfae94..66a4f26179 100644 --- a/Sources/Apollo/MultipartResponseParsingInterceptor.swift +++ b/Sources/Apollo/MultipartResponseParsingInterceptor.swift @@ -43,6 +43,8 @@ public struct MultipartResponseParsingInterceptor: ApolloInterceptor { private static let contentTypeHeader: StaticString = "content-type:" private static let heartbeat: StaticString = "{}" + public var id: String = UUID().uuidString + public init() { } public func interceptAsync( @@ -63,7 +65,12 @@ public struct MultipartResponseParsingInterceptor: ApolloInterceptor { } if !response.httpResponse.isMultipart { - chain.proceedAsync(request: request, response: response, completion: completion) + chain.proceedAsync( + request: request, + response: response, + interceptor: self, + completion: completion + ) return } @@ -148,7 +155,12 @@ public struct MultipartResponseParsingInterceptor: ApolloInterceptor { rawData: data, parsedResponse: nil ) - chain.proceedAsync(request: request, response: response, completion: completion) + chain.proceedAsync( + request: request, + response: response, + interceptor: self, + completion: completion + ) case .unknown: chain.handleErrorAsync( diff --git a/Sources/Apollo/NetworkFetchInterceptor.swift b/Sources/Apollo/NetworkFetchInterceptor.swift index 290bb516d0..a477cf6e2b 100644 --- a/Sources/Apollo/NetworkFetchInterceptor.swift +++ b/Sources/Apollo/NetworkFetchInterceptor.swift @@ -7,6 +7,8 @@ import ApolloAPI public class NetworkFetchInterceptor: ApolloInterceptor, Cancellable { let client: URLSessionClient @Atomic private var currentTask: URLSessionTask? + + public var id: String = UUID().uuidString /// Designated initializer. /// @@ -25,10 +27,12 @@ public class NetworkFetchInterceptor: ApolloInterceptor, Cancellable { do { urlRequest = try request.toURLRequest() } catch { - chain.handleErrorAsync(error, - request: request, - response: response, - completion: completion) + chain.handleErrorAsync( + error, + request: request, + response: response, + completion: completion + ) return } @@ -49,17 +53,26 @@ public class NetworkFetchInterceptor: ApolloInterceptor, Cancellable { switch result { case .failure(let error): - chain.handleErrorAsync(error, - request: request, - response: response, - completion: completion) + chain.handleErrorAsync( + error, + request: request, + response: response, + completion: completion + ) + case .success(let (data, httpResponse)): - let response = HTTPResponse(response: httpResponse, - rawData: data, - parsedResponse: nil) - chain.proceedAsync(request: request, - response: response, - completion: completion) + let response = HTTPResponse( + response: httpResponse, + rawData: data, + parsedResponse: nil + ) + + chain.proceedAsync( + request: request, + response: response, + interceptor: self, + completion: completion + ) } } diff --git a/Sources/Apollo/RequestChain.swift b/Sources/Apollo/RequestChain.swift index 11f0a127b4..f1f057ff02 100644 --- a/Sources/Apollo/RequestChain.swift +++ b/Sources/Apollo/RequestChain.swift @@ -9,12 +9,20 @@ public protocol RequestChain: Cancellable { completion: @escaping (Result, Error>) -> Void ) where Operation : GraphQLOperation + @available(*, deprecated, renamed: "proceedAsync(request:response:interceptor:completion:)") func proceedAsync( request: HTTPRequest, response: HTTPResponse?, completion: @escaping (Result, Error>) -> Void ) where Operation : GraphQLOperation + func proceedAsync( + request: HTTPRequest, + response: HTTPResponse?, + interceptor: any ApolloInterceptor, + completion: @escaping (Result, Error>) -> Void + ) where Operation : GraphQLOperation + func cancel() func retry( diff --git a/Sources/Apollo/ResponseCodeInterceptor.swift b/Sources/Apollo/ResponseCodeInterceptor.swift index 2fc5cbaaa9..128ceddda4 100644 --- a/Sources/Apollo/ResponseCodeInterceptor.swift +++ b/Sources/Apollo/ResponseCodeInterceptor.swift @@ -5,6 +5,8 @@ import ApolloAPI /// An interceptor to check the response code returned with a request. public struct ResponseCodeInterceptor: ApolloInterceptor { + + public var id: String = UUID().uuidString public enum ResponseCodeError: Error, LocalizedError { case invalidResponseCode(response: HTTPURLResponse?, rawData: Data?) @@ -44,18 +46,25 @@ public struct ResponseCodeInterceptor: ApolloInterceptor { guard response?.httpResponse.isSuccessful == true else { - let error = ResponseCodeError.invalidResponseCode(response: response?.httpResponse, - rawData: response?.rawData) + let error = ResponseCodeError.invalidResponseCode( + response: response?.httpResponse, + rawData: response?.rawData + ) - chain.handleErrorAsync(error, - request: request, - response: response, - completion: completion) + chain.handleErrorAsync( + error, + request: request, + response: response, + completion: completion + ) return } - chain.proceedAsync(request: request, - response: response, - completion: completion) + chain.proceedAsync( + request: request, + response: response, + interceptor: self, + completion: completion + ) } } diff --git a/Tests/ApolloInternalTestHelpers/InterceptorTester.swift b/Tests/ApolloInternalTestHelpers/InterceptorTester.swift index 42b02e6ecb..ac76b2d36c 100644 --- a/Tests/ApolloInternalTestHelpers/InterceptorTester.swift +++ b/Tests/ApolloInternalTestHelpers/InterceptorTester.swift @@ -4,9 +4,9 @@ import Apollo /// `InterceptorRequestChain` and end the interceptor list with `JSONResponseParsingInterceptor` /// to get a parsed `GraphQLResult` for the standard request chain callback. public class InterceptorTester { - let interceptor: ApolloInterceptor + let interceptor: any ApolloInterceptor - public init(interceptor: ApolloInterceptor) { + public init(interceptor: any ApolloInterceptor) { self.interceptor = interceptor } @@ -47,6 +47,15 @@ fileprivate class ResponseCaptureRequestChain: RequestChain { self.completion(.success(response?.rawData)) } + func proceedAsync( + request: HTTPRequest, + response: HTTPResponse?, + interceptor: any ApolloInterceptor, + completion: @escaping (Result, Error>) -> Void + ) { + self.completion(.success(response?.rawData)) + } + func cancel() {} func retry( diff --git a/Tests/ApolloInternalTestHelpers/MockInterceptorProvider.swift b/Tests/ApolloInternalTestHelpers/MockInterceptorProvider.swift index 5b83d2f0d0..f3a29119a2 100644 --- a/Tests/ApolloInternalTestHelpers/MockInterceptorProvider.swift +++ b/Tests/ApolloInternalTestHelpers/MockInterceptorProvider.swift @@ -1,13 +1,13 @@ import Apollo public struct MockInterceptorProvider: InterceptorProvider { - let interceptors: [ApolloInterceptor] + let interceptors: [any ApolloInterceptor] - public init(_ interceptors: [ApolloInterceptor]) { + public init(_ interceptors: [any ApolloInterceptor]) { self.interceptors = interceptors } - public func interceptors(for operation: Operation) -> [ApolloInterceptor] { + public func interceptors(for operation: Operation) -> [any ApolloInterceptor] { self.interceptors } } diff --git a/Tests/ApolloInternalTestHelpers/MockNetworkTransport.swift b/Tests/ApolloInternalTestHelpers/MockNetworkTransport.swift index 8ddf4e0a75..627f1facb8 100644 --- a/Tests/ApolloInternalTestHelpers/MockNetworkTransport.swift +++ b/Tests/ApolloInternalTestHelpers/MockNetworkTransport.swift @@ -19,7 +19,9 @@ public final class MockNetworkTransport: RequestChainNetworkTransport { let store: ApolloStore let server: MockGraphQLServer - func interceptors(for operation: Operation) -> [ApolloInterceptor] where Operation: GraphQLOperation { + func interceptors( + for operation: Operation + ) -> [any ApolloInterceptor] where Operation: GraphQLOperation { return [ MaxRetryInterceptor(), CacheReadInterceptor(store: self.store), @@ -41,6 +43,8 @@ private final class MockTask: Cancellable { private class MockGraphQLServerInterceptor: ApolloInterceptor { let server: MockGraphQLServer + + public var id: String = UUID().uuidString init(server: MockGraphQLServer) { self.server = server @@ -66,6 +70,7 @@ private class MockGraphQLServerInterceptor: ApolloInterceptor { parsedResponse: nil) chain.proceedAsync(request: request, response: response, + interceptor: self, completion: completion) } } diff --git a/Tests/ApolloTests/BlindRetryingTestInterceptor.swift b/Tests/ApolloTests/BlindRetryingTestInterceptor.swift index d9c638682b..c35eeaad74 100644 --- a/Tests/ApolloTests/BlindRetryingTestInterceptor.swift +++ b/Tests/ApolloTests/BlindRetryingTestInterceptor.swift @@ -7,6 +7,8 @@ class BlindRetryingTestInterceptor: ApolloInterceptor { var hitCount = 0 private(set) var hasBeenCancelled = false + public var id: String = UUID().uuidString + func interceptAsync( chain: RequestChain, request: HTTPRequest, diff --git a/Tests/ApolloTests/CancellationHandlingInterceptor.swift b/Tests/ApolloTests/CancellationHandlingInterceptor.swift index 995c02a3cc..9955ea7a2f 100644 --- a/Tests/ApolloTests/CancellationHandlingInterceptor.swift +++ b/Tests/ApolloTests/CancellationHandlingInterceptor.swift @@ -12,6 +12,8 @@ import ApolloAPI class CancellationHandlingInterceptor: ApolloInterceptor, Cancellable { private(set) var hasBeenCancelled = false + + public var id: String = UUID().uuidString func interceptAsync( chain: RequestChain, @@ -24,9 +26,12 @@ class CancellationHandlingInterceptor: ApolloInterceptor, Cancellable { } DispatchQueue.main.asyncAfter(deadline: .now() + 0.2) { - chain.proceedAsync(request: request, - response: response, - completion: completion) + chain.proceedAsync( + request: request, + response: response, + interceptor: self, + completion: completion + ) } } diff --git a/Tests/ApolloTests/Interceptors/MaxRetryInterceptorTests.swift b/Tests/ApolloTests/Interceptors/MaxRetryInterceptorTests.swift index f7f3860830..c21568baf2 100644 --- a/Tests/ApolloTests/Interceptors/MaxRetryInterceptorTests.swift +++ b/Tests/ApolloTests/Interceptors/MaxRetryInterceptorTests.swift @@ -9,7 +9,10 @@ class MaxRetryInterceptorTests: XCTestCase { class TestProvider: InterceptorProvider { let testInterceptor = BlindRetryingTestInterceptor() let retryCount = 15 - func interceptors(for operation: Operation) -> [ApolloInterceptor] { + + func interceptors( + for operation: Operation + ) -> [any ApolloInterceptor] { [ MaxRetryInterceptor(maxRetriesAllowed: self.retryCount), self.testInterceptor @@ -63,7 +66,9 @@ class MaxRetryInterceptorTests: XCTestCase { return client }() - func interceptors(for operation: Operation) -> [ApolloInterceptor] { + func interceptors( + for operation: Operation + ) -> [any ApolloInterceptor] { [ MaxRetryInterceptor(maxRetriesAllowed: self.retryCount), self.testInterceptor, diff --git a/Tests/ApolloTests/Interceptors/ResponseCodeInterceptorTests.swift b/Tests/ApolloTests/Interceptors/ResponseCodeInterceptorTests.swift index 31309339aa..30dcfa3bcd 100644 --- a/Tests/ApolloTests/Interceptors/ResponseCodeInterceptorTests.swift +++ b/Tests/ApolloTests/Interceptors/ResponseCodeInterceptorTests.swift @@ -16,7 +16,9 @@ class ResponseCodeInterceptorTests: XCTestCase { return client }() - func interceptors(for operation: Operation) -> [ApolloInterceptor] { + func interceptors( + for operation: Operation + ) -> [any ApolloInterceptor] { [ NetworkFetchInterceptor(client: self.mockClient), ResponseCodeInterceptor(), @@ -72,7 +74,9 @@ class ResponseCodeInterceptorTests: XCTestCase { return client }() - func interceptors(for operation: Operation) -> [ApolloInterceptor] { + func interceptors( + for operation: Operation + ) -> [any ApolloInterceptor] { [ NetworkFetchInterceptor(client: self.mockClient), ResponseCodeInterceptor(), diff --git a/Tests/ApolloTests/RequestChainTests.swift b/Tests/ApolloTests/RequestChainTests.swift index 987d1e9d1d..36f49bee25 100644 --- a/Tests/ApolloTests/RequestChainTests.swift +++ b/Tests/ApolloTests/RequestChainTests.swift @@ -7,7 +7,9 @@ class RequestChainTests: XCTestCase { func testEmptyInterceptorArrayReturnsCorrectError() { class TestProvider: InterceptorProvider { - func interceptors(for operation: Operation) -> [ApolloInterceptor] { + func interceptors( + for operation: Operation + ) -> [any ApolloInterceptor] { [] } } @@ -19,7 +21,7 @@ class RequestChainTests: XCTestCase { defer { expectation.fulfill() } - + switch result { case .success: XCTFail("This should not have succeeded") @@ -33,17 +35,19 @@ class RequestChainTests: XCTestCase { } } } - - + + self.wait(for: [expectation], timeout: 1) } - + func testCancellingChainCallsCancelOnInterceptorsWhichImplementCancellableAndNotOnOnesThatDont() { class TestProvider: InterceptorProvider { let cancellationInterceptor = CancellationHandlingInterceptor() let retryInterceptor = BlindRetryingTestInterceptor() - func interceptors(for operation: Operation) -> [ApolloInterceptor] { + func interceptors( + for operation: Operation + ) -> [any ApolloInterceptor] { [ self.cancellationInterceptor, self.retryInterceptor @@ -60,38 +64,40 @@ class RequestChainTests: XCTestCase { XCTFail("This should not have gone through") expectation.fulfill() } - + cancellable.cancel() XCTAssertTrue(provider.cancellationInterceptor.hasBeenCancelled) XCTAssertFalse(provider.retryInterceptor.hasBeenCancelled) self.wait(for: [expectation], timeout: 2) } - + func test__send__ErrorInterceptorGetsCalledAfterAnErrorIsReceived() { class ErrorInterceptor: ApolloErrorInterceptor { var error: Error? = nil - + func handleErrorAsync( error: Error, chain: RequestChain, request: HTTPRequest, response: HTTPResponse?, completion: @escaping (Result, Error>) -> Void) { - + self.error = error completion(.failure(error)) } } - + class TestProvider: InterceptorProvider { let errorInterceptor = ErrorInterceptor() - func interceptors(for operation: Operation) -> [ApolloInterceptor] { + func interceptors( + for operation: Operation + ) -> [any ApolloInterceptor] { return [ // An interceptor which will error without a response AutomaticPersistedQueryInterceptor() ] } - + func additionalErrorInterceptor(for operation: Operation) -> ApolloErrorInterceptor? { return self.errorInterceptor } @@ -101,7 +107,7 @@ class RequestChainTests: XCTestCase { let transport = RequestChainNetworkTransport(interceptorProvider: provider, endpointURL: TestURL.mockServer.url, autoPersistQueries: true) - + let expectation = self.expectation(description: "Hero name query complete") _ = transport.send(operation: MockQuery.mock()) { result in defer { @@ -120,9 +126,9 @@ class RequestChainTests: XCTestCase { } } } - + self.wait(for: [expectation], timeout: 1) - + switch provider.errorInterceptor.error { case .some(let error): switch error { @@ -155,7 +161,9 @@ class RequestChainTests: XCTestCase { class TestProvider: InterceptorProvider { let errorInterceptor = ErrorInterceptor() - func interceptors(for operation: Operation) -> [ApolloInterceptor] { + func interceptors( + for operation: Operation + ) -> [any ApolloInterceptor] { return [ // An interceptor which will error without a response ResponseCodeInterceptor() @@ -213,33 +221,35 @@ class RequestChainTests: XCTestCase { XCTFail("Error interceptor did not receive an error!") } } - + func testErrorInterceptorGetsCalledInDefaultInterceptorProviderSubclass() { class ErrorInterceptor: ApolloErrorInterceptor { var error: Error? = nil - + func handleErrorAsync( error: Error, chain: RequestChain, request: HTTPRequest, response: HTTPResponse?, completion: @escaping (Result, Error>) -> Void) { - + self.error = error completion(.failure(error)) } } - + class TestProvider: DefaultInterceptorProvider { let errorInterceptor = ErrorInterceptor() - - override func interceptors(for operation: Operation) -> [ApolloInterceptor] { + + override func interceptors( + for operation: Operation + ) -> [any ApolloInterceptor] { return [ // An interceptor which will error without a response AutomaticPersistedQueryInterceptor() ] } - + override func additionalErrorInterceptor(for operation: Operation) -> ApolloErrorInterceptor? { return self.errorInterceptor } @@ -249,7 +259,7 @@ class RequestChainTests: XCTestCase { let transport = RequestChainNetworkTransport(interceptorProvider: provider, endpointURL: TestURL.mockServer.url, autoPersistQueries: true) - + let expectation = self.expectation(description: "Hero name query complete") _ = transport.send(operation: MockQuery.mock()) { result in defer { @@ -268,9 +278,9 @@ class RequestChainTests: XCTestCase { } } } - + self.wait(for: [expectation], timeout: 1) - + switch provider.errorInterceptor.error { case .some(let error): switch error { @@ -286,10 +296,12 @@ class RequestChainTests: XCTestCase { } // MARK: Multipart subscription tests - + struct RequestTrapInterceptor: ApolloInterceptor { let callback: (URLRequest) -> (Void) + public var id: String = UUID().uuidString + init(_ callback: @escaping (URLRequest) -> (Void)) { self.callback = callback } @@ -418,7 +430,33 @@ class RequestChainTests: XCTestCase { var name: String { __data["name"] } } - func test__retain_release__givenQuery_shouldNotHaveRetainCycle() throws { + struct DelayInterceptor: ApolloInterceptor { + let seconds: Double + + public var id: String = UUID().uuidString + + init(_ seconds: Double) { + self.seconds = seconds + } + + func interceptAsync( + chain: RequestChain, + request: HTTPRequest, + response: HTTPResponse?, + completion: @escaping (Result, Error> + ) -> Void) { + DispatchQueue.main.asyncAfter(wallDeadline: DispatchWallTime.now() + seconds) { + chain.proceedAsync( + request: request, + response: response, + interceptor: self, + completion: completion + ) + } + } + } + + func test__memory_management__givenQuery_whenCompleted_shouldNotHaveRetainCycle() throws { // given let client = MockURLSessionClient( response: .mock( @@ -479,7 +517,7 @@ class RequestChainTests: XCTestCase { XCTAssertNil(weakRequestChain) } - func test__retain_release__givenSubscription_whenCancelled_shouldNotHaveRetainCycle() throws { + func test__memory_management__givenSubscription_whenCancelled_shouldNotHaveRetainCycle() throws { // given let client = MockURLSessionClient( response: .mock( @@ -560,7 +598,7 @@ class RequestChainTests: XCTestCase { XCTAssertNil(weakRequestChain) } - func test__managedSelf__givenQuery_whenCancelled_shouldNotCrash() throws { + func test__memory_management__givenQuery_whenCancelled_shouldNotCrash() throws { // given let client = MockURLSessionClient( response: .mock( @@ -580,24 +618,481 @@ class RequestChainTests: XCTestCase { ) let provider = MockInterceptorProvider([ + DelayInterceptor(0.5), NetworkFetchInterceptor(client: client), JSONResponseParsingInterceptor() ]) + let transport = RequestChainNetworkTransport( interceptorProvider: provider, endpointURL: TestURL.mockServer.url ) let expectation = expectation(description: "Response received") + expectation.isInverted = true + + let cancellable = transport.send(operation: MockQuery()) { result in + XCTFail("Unexpected response: \(result)") - let cancellable = transport.send(operation: MockQuery.mock()) { result in expectation.fulfill() } + DispatchQueue.main.async { + cancellable.cancel() + } + + wait(for: [expectation], timeout: 1) + } + + func test__memory_management__givenQuery_whenCancelledAfterInterceptorChainFinished_shouldNotCrash() throws { + // given + let client = MockURLSessionClient( + response: .mock( + url: TestURL.mockServer.url, + statusCode: 200, + httpVersion: nil, + headerFields: nil + ), + data: """ + { + "data": { + "__typename": "Hero", + "name": "R2-D2" + } + } + """.data(using: .utf8) + ) + + let provider = MockInterceptorProvider([ + NetworkFetchInterceptor(client: client), + JSONResponseParsingInterceptor() + ]) + let transport = RequestChainNetworkTransport( + interceptorProvider: provider, + endpointURL: TestURL.mockServer.url + ) + + let expectedData = try Hero(data: [ + "__typename": "Hero", + "name": "R2-D2" + ], variables: nil) + + let expectation = expectation(description: "Response received") + + let cancellable = transport.send(operation: MockQuery()) { result in + defer { + expectation.fulfill() + } + + switch result { + case let .success(data): + XCTAssertEqual(data.data, expectedData) + case let .failure(error): + XCTFail("Unexpected failure result: \(error)") + } + } + wait(for: [expectation], timeout: 1) DispatchQueue.main.async { cancellable.cancel() } } + + func test__memory_management__givenOperation_withEarlyInterceptorChainExit_success_shouldNotHaveRetainCycle() throws { + // given + let store = ApolloStore(cache: InMemoryNormalizedCache(records: [ + "QUERY_ROOT": [ + "__typename": "Hero", + "name": "R2-D2" + ] + ])) + + let client = MockURLSessionClient( + response: .mock( + url: TestURL.mockServer.url, + statusCode: 200, + httpVersion: nil, + headerFields: nil + ), + data: nil + ) + + var requestChain: RequestChain? = InterceptorRequestChain(interceptors: [ + CacheReadInterceptor(store: store), + NetworkFetchInterceptor(client: client), + JSONResponseParsingInterceptor() + ]) + weak var weakRequestChain: RequestChain? = requestChain + + let expectedData = try Hero(data: [ + "__typename": "Hero", + "name": "R2-D2" + ], variables: nil) + + let expectation = expectation(description: "Response received") + + let request = JSONRequest( + operation: MockQuery(), + graphQLEndpoint: TestURL.mockServer.url, + clientName: "test-client", + clientVersion: "test-client-version", + cachePolicy: .returnCacheDataDontFetch // early exit achieved by only wanting cache data + ) + + // when + requestChain?.kickoff(request: request) { result in + defer { + expectation.fulfill() + } + + switch (result) { + case let .success(data): + XCTAssertEqual(data.data, expectedData) + case let .failure(error): + XCTFail("Unexpected failure result - \(error)") + } + } + + wait(for: [expectation], timeout: 1) + + // then + XCTAssertNotNil(weakRequestChain) + requestChain = nil + XCTAssertNil(weakRequestChain) + } + + func test__memory_management__givenOperation_withEarlyInterceptorChainExit_failure_shouldNotHaveRetainCycle() throws { + // given + let client = MockURLSessionClient( + response: .mock( + url: TestURL.mockServer.url, + statusCode: 200, + httpVersion: nil, + headerFields: nil + ), + data: nil + ) + + var requestChain: RequestChain? = InterceptorRequestChain(interceptors: [ + CacheReadInterceptor(store: ApolloStore()), + NetworkFetchInterceptor(client: client), + JSONResponseParsingInterceptor() + ]) + + weak var weakRequestChain: RequestChain? = requestChain + + let expectation = expectation(description: "Response received") + + let request = JSONRequest( + operation: MockQuery(), + graphQLEndpoint: TestURL.mockServer.url, + clientName: "test-client", + clientVersion: "test-client-version", + cachePolicy: .returnCacheDataDontFetch // early exit achieved by only wanting cache data + ) + + // when + requestChain?.kickoff(request: request) { result in + defer { + expectation.fulfill() + } + + switch (result) { + case let .success(data): + XCTFail("Unexpected success result - \(data)") + case .failure: + break + } + } + + wait(for: [expectation], timeout: 1) + + // then + XCTAssertNotNil(weakRequestChain) + requestChain = nil + XCTAssertNil(weakRequestChain) + } + + func test__memory_management__givenOperation_withEarlyAndFinalInterceptorChainExit_shouldNotHaveRetainCycle_andShouldNotCrash() throws { + // given + let store = ApolloStore(cache: InMemoryNormalizedCache(records: [ + "QUERY_ROOT": [ + "__typename": "Hero", + "name": "R2-D2" + ] + ])) + + let client = MockURLSessionClient( + response: .mock( + url: TestURL.mockServer.url, + statusCode: 200, + httpVersion: nil, + headerFields: nil + ), + data: """ + { + "data": { + "__typename": "Hero", + "name": "R2-D2" + } + } + """.data(using: .utf8) + ) + + var requestChain: RequestChain? = InterceptorRequestChain(interceptors: [ + CacheReadInterceptor(store: store), + NetworkFetchInterceptor(client: client), + JSONResponseParsingInterceptor() + ]) + weak var weakRequestChain: RequestChain? = requestChain + + let expectedData = try Hero(data: [ + "__typename": "Hero", + "name": "R2-D2" + ], variables: nil) + + let expectation = expectation(description: "Response received") + expectation.expectedFulfillmentCount = 2 + + let request = JSONRequest( + operation: MockQuery(), + graphQLEndpoint: TestURL.mockServer.url, + clientName: "test-client", + clientVersion: "test-client-version", + cachePolicy: .returnCacheDataAndFetch // early return achieved by wanting cache data too + ) + + // when + requestChain?.kickoff(request: request) { result in + defer { + expectation.fulfill() + } + + switch (result) { + case let .success(data): + XCTAssertEqual(data.data, expectedData) + case let .failure(error): + XCTFail("Unexpected failure result - \(error)") + } + } + + wait(for: [expectation], timeout: 1) + + // then + XCTAssertNotNil(weakRequestChain) + requestChain = nil + XCTAssertNil(weakRequestChain) + } + + func test__memory_management__givenOperation_whenRetryInterceptorChain_shouldNotHaveRetainCycle_andShouldNotCrash() throws { + // given + let store = ApolloStore() + + let client = MockURLSessionClient( + response: .mock( + url: TestURL.mockServer.url, + statusCode: 200, + httpVersion: nil, + headerFields: nil + ), + data: nil + ) + + var requestChain: RequestChain? = InterceptorRequestChain(interceptors: [ + CacheReadInterceptor(store: store), + NetworkFetchInterceptor(client: client), + JSONResponseParsingInterceptor() + ]) + + weak var weakRequestChain: RequestChain? = requestChain + + let expectation = expectation(description: "Response received") + expectation.expectedFulfillmentCount = 2 + + let expectedData = try Hero(data: [ + "__typename": "Hero", + "name": "Han Solo" + ], variables: nil) + + let request = JSONRequest( + operation: MockQuery(), + graphQLEndpoint: TestURL.mockServer.url, + clientName: "test-client", + clientVersion: "test-client-version", + cachePolicy: .returnCacheDataDontFetch // early exit achieved by only wanting cache data + ) + + // when + requestChain?.kickoff(request: request) { result in + defer { + expectation.fulfill() + } + + switch (result) { + case let .success(data): + XCTFail("Unexpected success result - \(data)") + case .failure: + store.publish(records: [ + "QUERY_ROOT": [ + "__typename": "Hero", + "name": "Han Solo" + ] + ]) + + requestChain?.retry(request: request) { result in + defer { + expectation.fulfill() + } + + switch result { + case let .success(data): + XCTAssertEqual(data.data, expectedData) + case let .failure(error): + XCTFail("Unexpected failure result - \(error)") + } + } + break + } + } + + wait(for: [expectation], timeout: 2) + + // then + XCTAssertNotNil(weakRequestChain) + requestChain = nil + XCTAssertNil(weakRequestChain) + } + + // MARK: `proceedAsync` Tests + + struct SimpleForwardingInterceptor_deprecated: ApolloInterceptor { + var id: String = UUID().uuidString + + let expectation: XCTestExpectation + + func interceptAsync( + chain: Apollo.RequestChain, + request: Apollo.HTTPRequest, + response: Apollo.HTTPResponse?, + completion: @escaping (Result, Error>) -> Void + ) { + expectation.fulfill() + + chain.proceedAsync(request: request, response: response, completion: completion) + } + } + + struct SimpleForwardingInterceptor: ApolloInterceptor { + var id: String = UUID().uuidString + + let expectation: XCTestExpectation + + func interceptAsync( + chain: Apollo.RequestChain, + request: Apollo.HTTPRequest, + response: Apollo.HTTPResponse?, + completion: @escaping (Result, Error>) -> Void + ) { + expectation.fulfill() + + chain.proceedAsync( + request: request, + response: response, + interceptor: self, + completion: completion + ) + } + } + + func test__proceedAsync__givenInterceptors_usingDeprecatedFunction_shouldCallAllInterceptors() throws { + let expectations = [ + expectation(description: "Interceptor 1 executed"), + expectation(description: "Interceptor 2 executed"), + expectation(description: "Interceptor 3 executed") + ] + + let requestChain = InterceptorRequestChain(interceptors: [ + SimpleForwardingInterceptor_deprecated(expectation: expectations[0]), + SimpleForwardingInterceptor_deprecated(expectation: expectations[1]), + SimpleForwardingInterceptor_deprecated(expectation: expectations[2]) + ]) + + let request = JSONRequest( + operation: MockQuery(), + graphQLEndpoint: TestURL.mockServer.url, + clientName: "test-client", + clientVersion: "test-client-version" + ) + + // when + requestChain.kickoff(request: request) { result in } + + // then + wait(for: expectations, timeout: 1, enforceOrder: true) + } + + func test__proceedAsync__givenInterceptors_usingNewFunction_shouldCallAllInterceptors() throws { + let expectations = [ + expectation(description: "Interceptor 1 executed"), + expectation(description: "Interceptor 2 executed"), + expectation(description: "Interceptor 3 executed") + ] + + let requestChain = InterceptorRequestChain(interceptors: [ + SimpleForwardingInterceptor(expectation: expectations[0]), + SimpleForwardingInterceptor(expectation: expectations[1]), + SimpleForwardingInterceptor(expectation: expectations[2]) + ]) + + let request = JSONRequest( + operation: MockQuery(), + graphQLEndpoint: TestURL.mockServer.url, + clientName: "test-client", + clientVersion: "test-client-version" + ) + + // when + requestChain.kickoff(request: request) { result in } + + // then + wait(for: expectations, timeout: 1, enforceOrder: true) + } + + func test__proceedAsync__givenInterceptors_usingBothFunctions_shouldCallAllInterceptors() throws { + let expectations = [ + expectation(description: "Interceptor 1 executed"), + expectation(description: "Interceptor 2 executed"), + expectation(description: "Interceptor 3 executed"), + expectation(description: "Interceptor 4 executed"), + expectation(description: "Interceptor 5 executed"), + expectation(description: "Interceptor 6 executed"), + expectation(description: "Interceptor 7 executed"), + expectation(description: "Interceptor 8 executed") + ] + + let requestChain = InterceptorRequestChain(interceptors: [ + SimpleForwardingInterceptor(expectation: expectations[0]), + SimpleForwardingInterceptor_deprecated(expectation: expectations[1]), + SimpleForwardingInterceptor(expectation: expectations[2]), + SimpleForwardingInterceptor_deprecated(expectation: expectations[3]), + SimpleForwardingInterceptor_deprecated(expectation: expectations[4]), + SimpleForwardingInterceptor(expectation: expectations[5]), + SimpleForwardingInterceptor(expectation: expectations[6]), + SimpleForwardingInterceptor_deprecated(expectation: expectations[7]) + ]) + + let request = JSONRequest( + operation: MockQuery(), + graphQLEndpoint: TestURL.mockServer.url, + clientName: "test-client", + clientVersion: "test-client-version" + ) + + // when + requestChain.kickoff(request: request) { result in } + + // then + wait(for: expectations, timeout: 1, enforceOrder: true) + } } diff --git a/Tests/ApolloTests/RetryToCountThenSucceedInterceptor.swift b/Tests/ApolloTests/RetryToCountThenSucceedInterceptor.swift index d95711575e..062c396d75 100644 --- a/Tests/ApolloTests/RetryToCountThenSucceedInterceptor.swift +++ b/Tests/ApolloTests/RetryToCountThenSucceedInterceptor.swift @@ -13,6 +13,8 @@ import ApolloAPI class RetryToCountThenSucceedInterceptor: ApolloInterceptor { let timesToCallRetry: Int var timesRetryHasBeenCalled = 0 + + public var id: String = UUID().uuidString init(timesToCallRetry: Int) { self.timesToCallRetry = timesToCallRetry @@ -28,9 +30,12 @@ class RetryToCountThenSucceedInterceptor: ApolloInterceptor { chain.retry(request: request, completion: completion) } else { - chain.proceedAsync(request: request, - response: response, - completion: completion) + chain.proceedAsync( + request: request, + response: response, + interceptor: self, + completion: completion + ) } } } diff --git a/docs/source/config.json b/docs/source/config.json index af40a6cdbf..6baaa5fb89 100644 --- a/docs/source/config.json +++ b/docs/source/config.json @@ -11,7 +11,8 @@ "Migration Guides": [ { "v1.0": "/migrations/1.0", - "v1.2": "/migrations/1.2" + "v1.2": "/migrations/1.2", + "v1.3": "/migrations/1.3" }, true ], diff --git a/docs/source/migrations/1.3.mdx b/docs/source/migrations/1.3.mdx new file mode 100644 index 0000000000..7fd9cc0fe5 --- /dev/null +++ b/docs/source/migrations/1.3.mdx @@ -0,0 +1,31 @@ +--- +title: Apollo iOS 1.3 migration guide +description: From 1.2 to 1.3 +--- + +This guide describes the process of migrating your code from version 1.2 to version 1.3 of Apollo iOS. Please follow the relevant migration guides if you're on a version other than 1.2. + +Though 1.3 is a minor version bump, a few critical bugs were fixed in this version that require some breaking changes during the upgrade. While we strive to make the upgrade path for minor versions seamless, these issues could not be reasonably resolved without requiring this migration. + +## Request Chain Interceptors + +The `ApolloInterceptor` protocol implemented by request chain interceptors has had a minor change in this version. Any custom interceptors you use are now required to be able to identify themselves through a new property. + +The `RequestChain` protocol has also had a minor change in this version. The `proceedAsync(request:response:completion:)` function has been deprecated and replaced with a function named identically except for the inclusion of the interceptor so that it can be identified. This removes the need for the request chain to maintain index positioning of the list of interceptors. + +### Migration Steps + +In order for your custom interceptors to conform to the protocol change you can simply add the following line to your interceptor. +```swift title="Interceptor identification" +public var id: String = UUID().uuidString +``` + +Wherever your custom interceptors call back to the request chain you should replace the call to `proceedAsync(request:response:completion:)` with a call to the new function. +```swift title="Resolve deprecation warning" +chain.proceedAsync( + request: request, + response: response, + interceptor: self, + completion: completion +) +```