From b691926a50106e862b0d24d2526806e65f11627c Mon Sep 17 00:00:00 2001 From: Ellen Shapiro Date: Mon, 12 Aug 2019 20:46:42 +0200 Subject: [PATCH 01/16] de-public Promise and all its methods --- Sources/Apollo/Promise.swift | 36 ++++++++++++++-------------- Sources/Apollo/ResultOrPromise.swift | 20 ++++++++-------- 2 files changed, 28 insertions(+), 28 deletions(-) diff --git a/Sources/Apollo/Promise.swift b/Sources/Apollo/Promise.swift index 3b44580fc7..c838279c94 100644 --- a/Sources/Apollo/Promise.swift +++ b/Sources/Apollo/Promise.swift @@ -1,6 +1,6 @@ import Dispatch -public func whenAll(_ promises: [Promise], notifyOn queue: DispatchQueue = .global()) -> Promise<[Value]> { +func whenAll(_ promises: [Promise], notifyOn queue: DispatchQueue = .global()) -> Promise<[Value]> { return Promise { (fulfill, reject) in let group = DispatchGroup() @@ -20,7 +20,7 @@ public func whenAll(_ promises: [Promise], notifyOn queue: Dispatc } } -public func firstly(_ body: () throws -> Promise) -> Promise { +func firstly(_ body: () throws -> Promise) -> Promise { do { return try body() } catch { @@ -28,26 +28,26 @@ public func firstly(_ body: () throws -> Promise) -> Promise { } } -public final class Promise { +final class Promise { private let lock = Mutex() private var state: State private typealias ResultHandler = (Result) -> Void private var resultHandlers: [ResultHandler] = [] - public init(resolved result: Result) { + init(resolved result: Result) { state = .resolved(result) } - public init(fulfilled value: Value) { + init(fulfilled value: Value) { state = .resolved(.success(value)) } - public init(rejected error: Error) { + init(rejected error: Error) { state = .resolved(.failure(error)) } - public init(_ body: () throws -> Value) { + init(_ body: () throws -> Value) { do { let value = try body() state = .resolved(.success(value)) @@ -56,7 +56,7 @@ public final class Promise { } } - public init(_ body: (_ fulfill: @escaping (Value) -> Void, _ reject: @escaping (Error) -> Void) throws -> Void) { + init(_ body: (_ fulfill: @escaping (Value) -> Void, _ reject: @escaping (Error) -> Void) throws -> Void) { state = .pending do { @@ -66,13 +66,13 @@ public final class Promise { } } - public var isPending: Bool { + var isPending: Bool { return lock.withLock { state.isPending } } - public var result: Result? { + var result: Result? { return lock.withLock { switch state { case .pending: @@ -83,7 +83,7 @@ public final class Promise { } } - public func wait() { + func wait() { let semaphore = DispatchSemaphore(value: 0) whenResolved { result in @@ -93,7 +93,7 @@ public final class Promise { semaphore.wait() } - public func await() throws -> Value { + func await() throws -> Value { let semaphore = DispatchSemaphore(value: 0) var result: Result? = nil @@ -108,7 +108,7 @@ public final class Promise { return try result!.get() } - @discardableResult public func andThen(_ whenFulfilled: @escaping (Value) throws -> Void) -> Promise { + @discardableResult func andThen(_ whenFulfilled: @escaping (Value) throws -> Void) -> Promise { return Promise { fulfill, reject in whenResolved { result in switch result { @@ -126,7 +126,7 @@ public final class Promise { } } - @discardableResult public func `catch`(_ whenRejected: @escaping (Error) throws -> Void) -> Promise { + @discardableResult func `catch`(_ whenRejected: @escaping (Error) throws -> Void) -> Promise { return Promise { fulfill, reject in whenResolved { result in switch result { @@ -144,12 +144,12 @@ public final class Promise { } } - @discardableResult public func finally(_ whenResolved: @escaping () -> Void) -> Promise { + @discardableResult func finally(_ whenResolved: @escaping () -> Void) -> Promise { self.whenResolved { _ in whenResolved() } return self } - public func map(_ transform: @escaping (Value) throws -> T) -> Promise { + func map(_ transform: @escaping (Value) throws -> T) -> Promise { return Promise { fulfill, reject in whenResolved { result in switch result { @@ -166,7 +166,7 @@ public final class Promise { } } - public func flatMap(_ transform: @escaping (Value) throws -> Promise) -> Promise { + func flatMap(_ transform: @escaping (Value) throws -> Promise) -> Promise { return Promise { fulfill, reject in whenResolved { result in switch result { @@ -183,7 +183,7 @@ public final class Promise { } } - public func on(queue: DispatchQueue) -> Promise { + func on(queue: DispatchQueue) -> Promise { return Promise { fulfill, reject in whenResolved { result in switch result { diff --git a/Sources/Apollo/ResultOrPromise.swift b/Sources/Apollo/ResultOrPromise.swift index 129d2b24fa..555ef871e7 100644 --- a/Sources/Apollo/ResultOrPromise.swift +++ b/Sources/Apollo/ResultOrPromise.swift @@ -1,6 +1,6 @@ import Dispatch -public func whenAll(_ resultsOrPromises: [ResultOrPromise], notifyOn queue: DispatchQueue = .global()) -> ResultOrPromise<[Value]> { +func whenAll(_ resultsOrPromises: [ResultOrPromise], notifyOn queue: DispatchQueue = .global()) -> ResultOrPromise<[Value]> { onlyResults: do { var results: [Result] = [] for resultOrPromise in resultsOrPromises { @@ -36,11 +36,11 @@ public func whenAll(_ resultsOrPromises: [ResultOrPromise], notify }) } -public enum ResultOrPromise { +enum ResultOrPromise { case result(Result) case promise(Promise) - public init(_ body: () throws -> Value) { + init(_ body: () throws -> Value) { do { let value = try body() self = .result(.success(value)) @@ -49,7 +49,7 @@ public enum ResultOrPromise { } } - public var result: Result? { + var result: Result? { switch self { case .result(let result): return result @@ -58,7 +58,7 @@ public enum ResultOrPromise { } } - public func await() throws -> Value { + func await() throws -> Value { switch self { case .result(let result): return try result.get() @@ -76,7 +76,7 @@ public enum ResultOrPromise { } } - @discardableResult public func andThen(_ whenFulfilled: @escaping (Value) throws -> Void) -> ResultOrPromise { + @discardableResult func andThen(_ whenFulfilled: @escaping (Value) throws -> Void) -> ResultOrPromise { switch self { case .result(.success(let value)): do { @@ -92,7 +92,7 @@ public enum ResultOrPromise { } } - @discardableResult public func `catch`(_ whenRejected: @escaping (Error) throws -> Void) -> ResultOrPromise { + @discardableResult func `catch`(_ whenRejected: @escaping (Error) throws -> Void) -> ResultOrPromise { switch self { case .result(.success(let value)): return .result(.success(value)) @@ -108,7 +108,7 @@ public enum ResultOrPromise { } } - public func map(_ transform: @escaping (Value) throws -> T) -> ResultOrPromise { + func map(_ transform: @escaping (Value) throws -> T) -> ResultOrPromise { switch self { case .result(.success(let value)): do { @@ -125,7 +125,7 @@ public enum ResultOrPromise { } } - public func flatMap(_ transform: @escaping (Value) throws -> ResultOrPromise) -> ResultOrPromise { + func flatMap(_ transform: @escaping (Value) throws -> ResultOrPromise) -> ResultOrPromise { switch self { case .result(.success(let value)): do { @@ -142,7 +142,7 @@ public enum ResultOrPromise { } } - public func on(queue: DispatchQueue) -> ResultOrPromise { + func on(queue: DispatchQueue) -> ResultOrPromise { if case .promise(let promise) = self { return .promise(promise.on(queue: queue)) } else { From 2fb133571ef692f77814eb2931decb048d92d2d2 Mon Sep 17 00:00:00 2001 From: Ellen Shapiro Date: Mon, 12 Aug 2019 21:57:56 +0200 Subject: [PATCH 02/16] Update normalized cache protocol to not use promises --- Sources/Apollo/NormalizedCache.swift | 35 ++++++++++++++++++++-------- 1 file changed, 25 insertions(+), 10 deletions(-) diff --git a/Sources/Apollo/NormalizedCache.swift b/Sources/Apollo/NormalizedCache.swift index 180d5e9c46..98d68309a6 100644 --- a/Sources/Apollo/NormalizedCache.swift +++ b/Sources/Apollo/NormalizedCache.swift @@ -1,16 +1,31 @@ public protocol NormalizedCache { - + /// Loads records corresponding to the given keys. - /// - returns: A promise that fulfills with an array, with each index containing either the - /// record corresponding to the key at that index or nil if not found. - func loadRecords(forKeys keys: [CacheKey]) -> Promise<[Record?]> - + /// + /// - Parameters: + /// - keys: The cache keys to load data for + /// - callbackQueue: [optional] An alternate queue to fire the completion closure on. If nil, will fire on the current queue. + /// - completion: A completion closure to fire when the load has completed. If successful, will contain an array. Each index will contain either the record corresponding to the key at the same index in the passed-in array of cache keys, or nil if that record was not found. + func loadRecords(forKeys keys: [CacheKey], + callbackQueue: DispatchQueue?, + completion: @escaping (Result<[Record?], Error>) -> Void) + /// Merges a set of records into the cache. - /// - returns: A promise that fulfills with a set of keys corresponding to *fields* that have - /// changed (i.e. QUERY_ROOT.Foo.myField). These are the same type of keys as are - /// returned by RecordSet.merge(records:). - func merge(records: RecordSet) -> Promise> + /// + /// - Parameters: + /// - records: The set of records to merge. + /// - callbackQueue: [optional] An alternate queue to fire the completion closure on. If nil, will fire on the current queue. + /// - completion: A completion closure to fire when the merge has completed. If successful, will contain a set of keys corresponding to *fields* that have changed (i.e. QUERY_ROOT.Foo.myField). These are the same type of keys as are returned by RecordSet.merge(records:). + func merge(records: RecordSet, + callbackQueue: DispatchQueue?, + completion: @escaping (Result, Error>) -> Void) // Clears all records - func clear() -> Promise + /// + /// - Parameters: + /// - callbackQueue: [optional] An alternate queue to fire the completion closure on. If nil, will fire on the current queue. + /// - completion: [optional] A completion closure to fire when the clear function has completed. + func clear(callbackQueue: DispatchQueue?, + completion: ((Result) -> Void)?) } + From 263e9f983903d3031d57af71cb527351146086ff Mon Sep 17 00:00:00 2001 From: Ellen Shapiro Date: Mon, 12 Aug 2019 22:03:55 +0200 Subject: [PATCH 03/16] add internal promise methods for things that still need to be promises internally --- Sources/Apollo/ApolloClient.swift | 4 +- Sources/Apollo/ApolloClientProtocol.swift | 6 +- Sources/Apollo/ApolloStore.swift | 76 +++++++++++++++++++---- 3 files changed, 70 insertions(+), 16 deletions(-) diff --git a/Sources/Apollo/ApolloClient.swift b/Sources/Apollo/ApolloClient.swift index ab0ec8bf88..c39da2df56 100644 --- a/Sources/Apollo/ApolloClient.swift +++ b/Sources/Apollo/ApolloClient.swift @@ -118,8 +118,8 @@ extension ApolloClient: ApolloClientProtocol { } } - public func clearCache() -> Promise { - return self.store.clearCache() + public func clearCache(callbackQueue: DispatchQueue = .main, completion: (() -> Void)? = nil) { + self.store.clearCache(completion: completion) } @discardableResult public func fetch(query: Query, diff --git a/Sources/Apollo/ApolloClientProtocol.swift b/Sources/Apollo/ApolloClientProtocol.swift index 714c005b0b..fbbb276365 100644 --- a/Sources/Apollo/ApolloClientProtocol.swift +++ b/Sources/Apollo/ApolloClientProtocol.swift @@ -12,8 +12,10 @@ public protocol ApolloClientProtocol: class { /// Clears the underlying cache. /// Be aware: In more complex setups, the same underlying cache can be used across multiple instances, so if you call this on one instance, it'll clear that cache across all instances which share that cache. /// - /// - Returns: Promise which fulfills when clear is complete. - func clearCache() -> Promise + /// - Parameters: + /// - callbackQueue: The queue to fall back on. Should default to the main queue. + /// - completion: [optional] A completion closure to execute when clearing has completed. Should default to nil. + func clearCache(callbackQueue: DispatchQueue, completion: (() -> Void)?) /// Fetches a query from the server or from the local cache, depending on the current contents of the cache and the specified cache policy. /// diff --git a/Sources/Apollo/ApolloStore.swift b/Sources/Apollo/ApolloStore.swift index 92bb295009..b898f04fd1 100644 --- a/Sources/Apollo/ApolloStore.swift +++ b/Sources/Apollo/ApolloStore.swift @@ -50,14 +50,17 @@ public final class ApolloStore { /// Clears the instance of the cache. Note that a cache can be shared across multiple `ApolloClient` objects, so clearing that underlying cache will clear it for all clients. /// /// - Returns: A promise which fulfills when the Cache is cleared. - public func clearCache() -> Promise { - return Promise { fulfill, reject in - queue.async(flags: .barrier) { - self.cacheLock.withWriteLock { - self.cache.clear() + public func clearCache(callbackQueue: DispatchQueue = .main, completion: (() -> Void)? = nil) { + queue.async(flags: .barrier) { + self.cacheLock.withWriteLock { + self.cache.clearPromise() }.andThen { - fulfill(()) - } + guard let completion = completion else { + return + } + callbackQueue.async { + completion() + } } } } @@ -66,7 +69,7 @@ public final class ApolloStore { return Promise { fulfill, reject in queue.async(flags: .barrier) { self.cacheLock.withWriteLock { - self.cache.merge(records: records) + self.cache.mergePromise(records: records) }.andThen { changedKeys in self.didChangeKeys(changedKeys, context: context) fulfill(()) @@ -152,7 +155,7 @@ public final class ApolloStore { fileprivate let cache: NormalizedCache fileprivate let cacheKeyForObject: CacheKeyForObject? - fileprivate lazy var loader: DataLoader = DataLoader(self.cache.loadRecords) + fileprivate lazy var loader: DataLoader = DataLoader(self.cache.loadRecordsPromise) init(cache: NormalizedCache, cacheKeyForObject: CacheKeyForObject?) { self.cache = cache @@ -168,8 +171,12 @@ public final class ApolloStore { return try execute(selections: type.selections, onObjectWithKey: key, variables: variables, accumulator: mapper).await() } - public func loadRecords(forKeys keys: [CacheKey]) -> Promise<[Record?]> { - return cache.loadRecords(forKeys: keys) + public func loadRecords(forKeys keys: [CacheKey], + callbackQueue: DispatchQueue = .main, + completion: @escaping (Result<[Record?], Error>) -> Void) { + self.cache.loadRecords(forKeys: keys, + callbackQueue: callbackQueue, + completion: completion) } private final func complete(value: Any?) -> ResultOrPromise { @@ -249,7 +256,7 @@ public final class ApolloStore { _ = try executor.execute(selections: selections, on: object, withKey: key, variables: variables, accumulator: normalizer) .flatMap { - self.cache.merge(records: $0) + self.cache.mergePromise(records: $0) }.andThen { changedKeys in if let didChangeKeysFunc = self.updateChangedKeysFunc { didChangeKeysFunc(changedKeys, nil) @@ -258,3 +265,48 @@ public final class ApolloStore { } } } + +internal extension NormalizedCache { + func loadRecordsPromise(forKeys keys: [CacheKey]) -> Promise<[Record?]> { + return Promise { fulfill, reject in + self.loadRecords( + forKeys: keys, + callbackQueue: nil) { result in + switch result { + case .success(let records): + fulfill(records) + case .failure(let error): + reject(error) + } + } + } + } + + func mergePromise(records: RecordSet) -> Promise> { + return Promise { fulfill, reject in + self.merge( + records: records, + callbackQueue: nil) { result in + switch result { + case .success(let cacheKeys): + fulfill(cacheKeys) + case .failure(let error): + reject(error) + } + } + } + } + + func clearPromise() -> Promise { + return Promise { fulfill, reject in + self.clear(callbackQueue: nil) { result in + switch result { + case .success(let success): + fulfill(success) + case .failure(let error): + reject(error) + } + } + } + } +} From 6b6b859aa49b0ff0e2efb4255a7e2ac90a7fa4c0 Mon Sep 17 00:00:00 2001 From: Ellen Shapiro Date: Mon, 12 Aug 2019 22:06:09 +0200 Subject: [PATCH 04/16] update in-memory normalized cache to use callbacks --- Sources/Apollo/InMemoryNormalizedCache.swift | 48 ++++++++++++++++---- 1 file changed, 39 insertions(+), 9 deletions(-) diff --git a/Sources/Apollo/InMemoryNormalizedCache.swift b/Sources/Apollo/InMemoryNormalizedCache.swift index 1576833ce5..4df7e8cdc4 100644 --- a/Sources/Apollo/InMemoryNormalizedCache.swift +++ b/Sources/Apollo/InMemoryNormalizedCache.swift @@ -4,18 +4,48 @@ public final class InMemoryNormalizedCache: NormalizedCache { public init(records: RecordSet = RecordSet()) { self.records = records } - - public func loadRecords(forKeys keys: [CacheKey]) -> Promise<[Record?]> { + + public func loadRecords(forKeys keys: [CacheKey], + callbackQueue: DispatchQueue?, + completion: @escaping (Result<[Record?], Error>) -> Void) { let records = keys.map { self.records[$0] } - return Promise(fulfilled: records) + if let callbackQueue = callbackQueue { + callbackQueue.async { + completion(.success(records)) + } + } else { + completion(.success(records)) + } } - - public func merge(records: RecordSet) -> Promise> { - return Promise(fulfilled: self.records.merge(records: records)) + + public func merge(records: RecordSet, + callbackQueue: DispatchQueue?, + completion: @escaping (Result, Error>) -> Void) { + let cacheKeys = self.records.merge(records: records) + + if let callbackQueue = callbackQueue { + callbackQueue.async { + completion(.success(cacheKeys)) + } + } else { + completion(.success(cacheKeys)) + } } - public func clear() -> Promise { - records.clear() - return Promise(fulfilled: ()) + public func clear(callbackQueue: DispatchQueue?, + completion: ((Result) -> Void)?) { + self.records.clear() + + guard let completion = completion else { + return + } + + if let callbackQueue = callbackQueue { + callbackQueue.async { + completion(.success(())) + } + } else { + completion(.success(())) + } } } From 55295ddcd2011c0babbc547eef9962086a36ebf9 Mon Sep 17 00:00:00 2001 From: Ellen Shapiro Date: Tue, 13 Aug 2019 11:51:33 +0200 Subject: [PATCH 05/16] Add convenience method to perform async if needed --- Apollo.xcodeproj/project.pbxproj | 4 ++++ Sources/Apollo/DispatchQueue+Optional.swift | 24 ++++++++++++++++++++ Sources/Apollo/InMemoryNormalizedCache.swift | 19 +++------------- 3 files changed, 31 insertions(+), 16 deletions(-) create mode 100644 Sources/Apollo/DispatchQueue+Optional.swift diff --git a/Apollo.xcodeproj/project.pbxproj b/Apollo.xcodeproj/project.pbxproj index 64f1554a6b..c521db2977 100644 --- a/Apollo.xcodeproj/project.pbxproj +++ b/Apollo.xcodeproj/project.pbxproj @@ -14,6 +14,7 @@ 9B95EDC022CAA0B000702BB2 /* GETTransformerTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9B95EDBF22CAA0AF00702BB2 /* GETTransformerTests.swift */; }; 9BA1244A22D8A8EA00BF1D24 /* JSONSerialziation+Sorting.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9BA1244922D8A8EA00BF1D24 /* JSONSerialziation+Sorting.swift */; }; 9BA1245E22DE116B00BF1D24 /* Result+Helpers.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9BA1245D22DE116B00BF1D24 /* Result+Helpers.swift */; }; + 9BA3130E2302BEA5007B7FC5 /* DispatchQueue+Optional.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9BA3130D2302BEA5007B7FC5 /* DispatchQueue+Optional.swift */; }; 9BDE43D122C6655300FD7C7F /* Cancellable.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9BDE43D022C6655200FD7C7F /* Cancellable.swift */; }; 9BDE43DD22C6705300FD7C7F /* GraphQLHTTPResponseError.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9BDE43DC22C6705300FD7C7F /* GraphQLHTTPResponseError.swift */; }; 9BDE43DF22C6708600FD7C7F /* GraphQLHTTPRequestError.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9BDE43DE22C6708600FD7C7F /* GraphQLHTTPRequestError.swift */; }; @@ -270,6 +271,7 @@ 9B95EDBF22CAA0AF00702BB2 /* GETTransformerTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = GETTransformerTests.swift; sourceTree = ""; }; 9BA1244922D8A8EA00BF1D24 /* JSONSerialziation+Sorting.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "JSONSerialziation+Sorting.swift"; sourceTree = ""; }; 9BA1245D22DE116B00BF1D24 /* Result+Helpers.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "Result+Helpers.swift"; sourceTree = ""; }; + 9BA3130D2302BEA5007B7FC5 /* DispatchQueue+Optional.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "DispatchQueue+Optional.swift"; sourceTree = ""; }; 9BDE43D022C6655200FD7C7F /* Cancellable.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Cancellable.swift; sourceTree = ""; }; 9BDE43DC22C6705300FD7C7F /* GraphQLHTTPResponseError.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = GraphQLHTTPResponseError.swift; sourceTree = ""; }; 9BDE43DE22C6708600FD7C7F /* GraphQLHTTPRequestError.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = GraphQLHTTPRequestError.swift; sourceTree = ""; }; @@ -711,6 +713,7 @@ children = ( 9F578D8F1D8D2CB300C0EA36 /* Utilities.swift */, 9FCDFD221E33A0D8007519DC /* AsynchronousOperation.swift */, + 9BA3130D2302BEA5007B7FC5 /* DispatchQueue+Optional.swift */, 9FE941CF1E62C771007CDD89 /* Promise.swift */, 9BA1245D22DE116B00BF1D24 /* Result+Helpers.swift */, 9F19D8431EED568200C57247 /* ResultOrPromise.swift */, @@ -1230,6 +1233,7 @@ 9FE941D01E62C771007CDD89 /* Promise.swift in Sources */, 9BA1245E22DE116B00BF1D24 /* Result+Helpers.swift in Sources */, 9FC750631D2A59F600458D91 /* ApolloClient.swift in Sources */, + 9BA3130E2302BEA5007B7FC5 /* DispatchQueue+Optional.swift in Sources */, 9F86B6901E65533D00B885FF /* GraphQLResponseGenerator.swift in Sources */, 9FC9A9C21E2D3CAF0023C4D5 /* GraphQLInputValue.swift in Sources */, ); diff --git a/Sources/Apollo/DispatchQueue+Optional.swift b/Sources/Apollo/DispatchQueue+Optional.swift new file mode 100644 index 0000000000..79d686d8c4 --- /dev/null +++ b/Sources/Apollo/DispatchQueue+Optional.swift @@ -0,0 +1,24 @@ +// +// DispatchQueue+Optional.swift +// Apollo +// +// Created by Ellen Shapiro on 8/13/19. +// Copyright © 2019 Apollo GraphQL. All rights reserved. +// + +import Foundation + +extension DispatchQueue { + + static func performAsyncIfNeeded(on callbackQueue: DispatchQueue?, action: @escaping () -> Void) { + if let callbackQueue = callbackQueue { + // A callback queue was provided, perform the action on that queue + callbackQueue.async { + action() + } + } else { + // Perform the action on the current queue + action() + } + } +} diff --git a/Sources/Apollo/InMemoryNormalizedCache.swift b/Sources/Apollo/InMemoryNormalizedCache.swift index 4df7e8cdc4..0038c24fba 100644 --- a/Sources/Apollo/InMemoryNormalizedCache.swift +++ b/Sources/Apollo/InMemoryNormalizedCache.swift @@ -9,11 +9,7 @@ public final class InMemoryNormalizedCache: NormalizedCache { callbackQueue: DispatchQueue?, completion: @escaping (Result<[Record?], Error>) -> Void) { let records = keys.map { self.records[$0] } - if let callbackQueue = callbackQueue { - callbackQueue.async { - completion(.success(records)) - } - } else { + DispatchQueue.performAsyncIfNeeded(on: callbackQueue) { completion(.success(records)) } } @@ -22,12 +18,7 @@ public final class InMemoryNormalizedCache: NormalizedCache { callbackQueue: DispatchQueue?, completion: @escaping (Result, Error>) -> Void) { let cacheKeys = self.records.merge(records: records) - - if let callbackQueue = callbackQueue { - callbackQueue.async { - completion(.success(cacheKeys)) - } - } else { + DispatchQueue.performAsyncIfNeeded(on: callbackQueue) { completion(.success(cacheKeys)) } } @@ -40,11 +31,7 @@ public final class InMemoryNormalizedCache: NormalizedCache { return } - if let callbackQueue = callbackQueue { - callbackQueue.async { - completion(.success(())) - } - } else { + DispatchQueue.performAsyncIfNeeded(on: callbackQueue) { completion(.success(())) } } From 14a4ab4e18fe68db6c2f81e0576060ba48218345 Mon Sep 17 00:00:00 2001 From: Ellen Shapiro Date: Tue, 13 Aug 2019 11:55:58 +0200 Subject: [PATCH 06/16] update methods in batched load tests --- Tests/ApolloTests/BatchedLoadTests.swift | 36 +++++++++++++----------- 1 file changed, 20 insertions(+), 16 deletions(-) diff --git a/Tests/ApolloTests/BatchedLoadTests.swift b/Tests/ApolloTests/BatchedLoadTests.swift index cfca777706..b784fb27c2 100644 --- a/Tests/ApolloTests/BatchedLoadTests.swift +++ b/Tests/ApolloTests/BatchedLoadTests.swift @@ -12,31 +12,35 @@ private final class MockBatchedNormalizedCache: NormalizedCache { self.records = records } - func loadRecords(forKeys keys: [CacheKey]) -> Promise<[Record?]> { + func loadRecords(forKeys keys: [CacheKey], + callbackQueue: DispatchQueue?, + completion: @escaping (Result<[Record?], Error>) -> Void) { OSAtomicIncrement32(&numberOfBatchLoads) - return Promise { fulfill, reject in - DispatchQueue.global().asyncAfter(deadline: .now() + .milliseconds(1)) { - let records = keys.map { self.records[$0] } - fulfill(records) + DispatchQueue.global().asyncAfter(deadline: .now() + .milliseconds(1)) { + let records = keys.map { self.records[$0] } + DispatchQueue.performAsyncIfNeeded(on: callbackQueue) { + completion(.success(records)) } } } - func merge(records: RecordSet) -> Promise> { - return Promise { fulfill, reject in - DispatchQueue.global().asyncAfter(deadline: .now() + .milliseconds(1)) { - let changedKeys = self.records.merge(records: records) - fulfill(changedKeys) + func merge(records: RecordSet, + callbackQueue: DispatchQueue?, + completion: @escaping (Result, Error>) -> Void) { + DispatchQueue.global().asyncAfter(deadline: .now() + .milliseconds(1)) { + let changedKeys = self.records.merge(records: records) + DispatchQueue.performAsyncIfNeeded(on: callbackQueue) { + completion(.success(changedKeys)) } } } - - func clear() -> Promise { - return Promise { fulfill, reject in - DispatchQueue.global().asyncAfter(deadline: .now() + .milliseconds(1)) { - self.records.clear() - fulfill(()) + + func clear(callbackQueue: DispatchQueue?, completion: ((Result) -> Void)?) { + DispatchQueue.global().asyncAfter(deadline: .now() + .milliseconds(1)) { + self.records.clear() + DispatchQueue.performAsyncIfNeeded(on: callbackQueue) { + completion?(.success(())) } } } From d0037baaa6f7b0bf3c841ac62f38e825cea03f02 Mon Sep 17 00:00:00 2001 From: Ellen Shapiro Date: Tue, 13 Aug 2019 11:56:17 +0200 Subject: [PATCH 07/16] fix indentation in fetch query tests --- .../FetchQueryTests.swift | 232 +++++++++--------- 1 file changed, 116 insertions(+), 116 deletions(-) diff --git a/Tests/ApolloCacheDependentTests/FetchQueryTests.swift b/Tests/ApolloCacheDependentTests/FetchQueryTests.swift index ff4c4a037d..8d5a5d93c0 100644 --- a/Tests/ApolloCacheDependentTests/FetchQueryTests.swift +++ b/Tests/ApolloCacheDependentTests/FetchQueryTests.swift @@ -6,7 +6,7 @@ import StarWarsAPI class FetchQueryTests: XCTestCase { func testFetchIgnoringCacheData() throws { let query = HeroNameQuery() - + let initialRecords: RecordSet = [ "QUERY_ROOT": ["hero": Reference(key: "hero")], "hero": [ @@ -14,10 +14,10 @@ class FetchQueryTests: XCTestCase { "__typename": "Droid", ] ] - + withCache(initialRecords: initialRecords) { cache in let store = ApolloStore(cache: cache) - + let networkTransport = MockNetworkTransport(body: [ "data": [ "hero": [ @@ -26,14 +26,14 @@ class FetchQueryTests: XCTestCase { ] ] ]) - + let client = ApolloClient(networkTransport: networkTransport, store: store) - + let expectation = self.expectation(description: "Fetching query") - + client.fetch(query: query, cachePolicy: .fetchIgnoringCacheData) { result in defer { expectation.fulfill() } - + switch result { case .success(let queryResult): XCTAssertEqual(queryResult.data?.hero?.name, "Luke Skywalker") @@ -41,7 +41,7 @@ class FetchQueryTests: XCTestCase { XCTFail("Error: \(error)") } } - + self.waitForExpectations(timeout: 5, handler: nil) } } @@ -96,7 +96,7 @@ class FetchQueryTests: XCTestCase { func testReturnCacheDataElseFetchWithCachedData() throws { let query = HeroNameQuery() - + let initialRecords: RecordSet = [ "QUERY_ROOT": ["hero": Reference(key: "QUERY_ROOT.hero")], "QUERY_ROOT.hero": [ @@ -104,10 +104,10 @@ class FetchQueryTests: XCTestCase { "__typename": "Droid", ] ] - + withCache(initialRecords: initialRecords) { (cache) in let store = ApolloStore(cache: cache) - + let networkTransport = MockNetworkTransport(body: [ "data": [ "hero": [ @@ -116,14 +116,14 @@ class FetchQueryTests: XCTestCase { ] ] ]) - + let client = ApolloClient(networkTransport: networkTransport, store: store) - + let expectation = self.expectation(description: "Fetching query") - + client.fetch(query: query, cachePolicy: .returnCacheDataElseFetch) { result in defer { expectation.fulfill() } - + switch result { case .success(let graphQLResult): XCTAssertEqual(graphQLResult.data?.hero?.name, "R2-D2") @@ -131,24 +131,24 @@ class FetchQueryTests: XCTestCase { XCTFail("Unexpected error: \(error)") } } - + self.waitForExpectations(timeout: 5, handler: nil) } } func testReturnCacheDataElseFetchWithMissingData() throws { let query = HeroNameQuery() - + let initialRecords: RecordSet = [ "QUERY_ROOT": ["hero": Reference(key: "hero")], "hero": [ "name": "R2-D2", ] ] - + withCache(initialRecords: initialRecords) { (cache) in let store = ApolloStore(cache: cache) - + let networkTransport = MockNetworkTransport(body: [ "data": [ "hero": [ @@ -157,14 +157,14 @@ class FetchQueryTests: XCTestCase { ] ] ]) - + let client = ApolloClient(networkTransport: networkTransport, store: store) - + let expectation = self.expectation(description: "Fetching query") - + client.fetch(query: query, cachePolicy: .returnCacheDataElseFetch) { result in defer { expectation.fulfill() } - + switch result { case .success(let graphQLResult): XCTAssertEqual(graphQLResult.data?.hero?.name, "Luke Skywalker") @@ -172,14 +172,14 @@ class FetchQueryTests: XCTestCase { XCTFail("Unexpected error: \(error)") } } - + self.waitForExpectations(timeout: 5, handler: nil) } } func testReturnCacheDataDontFetchWithCachedData() throws { let query = HeroNameQuery() - + let initialRecords: RecordSet = [ "QUERY_ROOT": ["hero": Reference(key: "hero")], "hero": [ @@ -187,10 +187,10 @@ class FetchQueryTests: XCTestCase { "__typename": "Droid", ] ] - + withCache(initialRecords: initialRecords) { (cache) in let store = ApolloStore(cache: cache) - + let networkTransport = MockNetworkTransport(body: [ "data": [ "hero": [ @@ -199,11 +199,11 @@ class FetchQueryTests: XCTestCase { ] ] ]) - + let client = ApolloClient(networkTransport: networkTransport, store: store) - + let expectation = self.expectation(description: "Fetching query") - + client.fetch(query: query, cachePolicy: .returnCacheDataDontFetch) { result in defer { expectation.fulfill() } @@ -214,14 +214,14 @@ class FetchQueryTests: XCTestCase { XCTFail("Unexpected error: \(error)") } } - + self.waitForExpectations(timeout: 5, handler: nil) } } - + func testClearCache() throws { let query = HeroNameQuery() - + let initialRecords: RecordSet = [ "QUERY_ROOT": ["hero": Reference(key: "hero")], "hero": [ @@ -229,78 +229,78 @@ class FetchQueryTests: XCTestCase { "__typename": "Droid", ] ] - + withCache(initialRecords: initialRecords) { (cache) in - let store = ApolloStore(cache: cache) - - let networkTransport = MockNetworkTransport(body: [ - "data": [ - "hero": [ - "name": "Luke Skywalker", - "__typename": "Human" - ] + let store = ApolloStore(cache: cache) + + let networkTransport = MockNetworkTransport(body: [ + "data": [ + "hero": [ + "name": "Luke Skywalker", + "__typename": "Human" ] + ] ]) - - let client = ApolloClient(networkTransport: networkTransport, store: store) - - let expectation = self.expectation(description: "Fetching query") - - client.fetch(query: query, cachePolicy: .returnCacheDataDontFetch) { result in - defer { expectation.fulfill() } - - switch result { - case .success(let graphQLResult): - XCTAssertEqual(graphQLResult.data?.hero?.name, "R2-D2") - case .failure(let error): - XCTFail("Unexpected error: \(error)") - } - } - - self.waitForExpectations(timeout: 5, handler: nil) - - do { try client.clearCache().await() } - catch { XCTFail() } - - let expectation2 = self.expectation(description: "Fetching query") - - client.fetch(query: query, cachePolicy: .returnCacheDataDontFetch) { result in - defer { expectation2.fulfill() } - switch result { - case .success: - XCTFail("This should have returned an error") - case .failure(let error): - if let resultError = error as? JSONDecodingError { - switch resultError { - case .missingValue: - // Correct error! - break - default: - XCTFail("Unexpected JSON error: \(error)") - } - } else { - XCTFail("Unexpected error: \(error)") + + let client = ApolloClient(networkTransport: networkTransport, store: store) + + let expectation = self.expectation(description: "Fetching query") + + client.fetch(query: query, cachePolicy: .returnCacheDataDontFetch) { result in + defer { expectation.fulfill() } + + switch result { + case .success(let graphQLResult): + XCTAssertEqual(graphQLResult.data?.hero?.name, "R2-D2") + case .failure(let error): + XCTFail("Unexpected error: \(error)") + } + } + + self.waitForExpectations(timeout: 5, handler: nil) + + do { try client.clearCache().await() } + catch { XCTFail() } + + let expectation2 = self.expectation(description: "Fetching query") + + client.fetch(query: query, cachePolicy: .returnCacheDataDontFetch) { result in + defer { expectation2.fulfill() } + switch result { + case .success: + XCTFail("This should have returned an error") + case .failure(let error): + if let resultError = error as? JSONDecodingError { + switch resultError { + case .missingValue: + // Correct error! + break + default: + XCTFail("Unexpected JSON error: \(error)") } + } else { + XCTFail("Unexpected error: \(error)") } } - - self.waitForExpectations(timeout: 5, handler: nil) } + + self.waitForExpectations(timeout: 5, handler: nil) + } } func testReturnCacheDataDontFetchWithMissingData() throws { let query = HeroNameQuery() - + let initialRecords: RecordSet = [ "QUERY_ROOT": ["hero": Reference(key: "hero")], "hero": [ "name": "R2-D2", ] ] - + withCache(initialRecords: initialRecords) { (cache) in let store = ApolloStore(cache: cache) - + let networkTransport = MockNetworkTransport(body: [ "data": [ "hero": [ @@ -308,12 +308,12 @@ class FetchQueryTests: XCTestCase { "__typename": "Human" ] ] - ]) - + ]) + let client = ApolloClient(networkTransport: networkTransport, store: store) - + let expectation = self.expectation(description: "Fetching query") - + client.fetch(query: query, cachePolicy: .returnCacheDataDontFetch) { result in defer { expectation.fulfill() } switch result { @@ -323,23 +323,23 @@ class FetchQueryTests: XCTestCase { if let resultError = error as? GraphQLResultError, let underlyingError = resultError.underlying as? JSONDecodingError { - switch underlyingError { - case .missingValue: - // Correct error! - break - default: - XCTFail("Unexpected JSON error: \(error)") - } + switch underlyingError { + case .missingValue: + // Correct error! + break + default: + XCTFail("Unexpected JSON error: \(error)") + } } else { XCTFail("Unexpected error: \(error)") } } } - + self.waitForExpectations(timeout: 5, handler: nil) } } - + func testCompletionHandlerIsCalledOnTheSpecifiedQueue() { let queue = DispatchQueue(label: "label") @@ -349,27 +349,27 @@ class FetchQueryTests: XCTestCase { let query = HeroNameQuery() let networkTransport = MockNetworkTransport(body: [ - "data": [ - "hero": [ - "name": "Luke Skywalker", - "__typename": "Human" - ] + "data": [ + "hero": [ + "name": "Luke Skywalker", + "__typename": "Human" ] - ]) + ] + ]) withCache { (cache) in - let store = ApolloStore(cache: cache) - let client = ApolloClient(networkTransport: networkTransport, store: store) - - let expectation = self.expectation(description: "Fetching query") - - client.fetch(query: query, cachePolicy: .fetchIgnoringCacheData, queue: queue) { _ in - defer { expectation.fulfill() } - - XCTAssertNotNil(DispatchQueue.getSpecific(key: key)) - } + let store = ApolloStore(cache: cache) + let client = ApolloClient(networkTransport: networkTransport, store: store) + + let expectation = self.expectation(description: "Fetching query") + + client.fetch(query: query, cachePolicy: .fetchIgnoringCacheData, queue: queue) { _ in + defer { expectation.fulfill() } - waitForExpectations(timeout: 5, handler: nil) + XCTAssertNotNil(DispatchQueue.getSpecific(key: key)) + } + + waitForExpectations(timeout: 5, handler: nil) } } } From 88b0b52219335fc1878b913b3a089e50efb0e74d Mon Sep 17 00:00:00 2001 From: Ellen Shapiro Date: Tue, 13 Aug 2019 12:12:09 +0200 Subject: [PATCH 08/16] Update sqlite normalized cache and give dispatch queue method a prefix --- Sources/Apollo/DispatchQueue+Optional.swift | 4 +- Sources/Apollo/InMemoryNormalizedCache.swift | 6 +-- .../ApolloSQLite/SQLiteNormalizedCache.swift | 43 +++++++++++++++---- 3 files changed, 40 insertions(+), 13 deletions(-) diff --git a/Sources/Apollo/DispatchQueue+Optional.swift b/Sources/Apollo/DispatchQueue+Optional.swift index 79d686d8c4..1b7f6d4efa 100644 --- a/Sources/Apollo/DispatchQueue+Optional.swift +++ b/Sources/Apollo/DispatchQueue+Optional.swift @@ -8,9 +8,9 @@ import Foundation -extension DispatchQueue { +public extension DispatchQueue { - static func performAsyncIfNeeded(on callbackQueue: DispatchQueue?, action: @escaping () -> Void) { + static func apollo_performAsyncIfNeeded(on callbackQueue: DispatchQueue?, action: @escaping () -> Void) { if let callbackQueue = callbackQueue { // A callback queue was provided, perform the action on that queue callbackQueue.async { diff --git a/Sources/Apollo/InMemoryNormalizedCache.swift b/Sources/Apollo/InMemoryNormalizedCache.swift index 0038c24fba..55b2fbbc79 100644 --- a/Sources/Apollo/InMemoryNormalizedCache.swift +++ b/Sources/Apollo/InMemoryNormalizedCache.swift @@ -9,7 +9,7 @@ public final class InMemoryNormalizedCache: NormalizedCache { callbackQueue: DispatchQueue?, completion: @escaping (Result<[Record?], Error>) -> Void) { let records = keys.map { self.records[$0] } - DispatchQueue.performAsyncIfNeeded(on: callbackQueue) { + DispatchQueue.apollo_performAsyncIfNeeded(on: callbackQueue) { completion(.success(records)) } } @@ -18,7 +18,7 @@ public final class InMemoryNormalizedCache: NormalizedCache { callbackQueue: DispatchQueue?, completion: @escaping (Result, Error>) -> Void) { let cacheKeys = self.records.merge(records: records) - DispatchQueue.performAsyncIfNeeded(on: callbackQueue) { + DispatchQueue.apollo_performAsyncIfNeeded(on: callbackQueue) { completion(.success(cacheKeys)) } } @@ -31,7 +31,7 @@ public final class InMemoryNormalizedCache: NormalizedCache { return } - DispatchQueue.performAsyncIfNeeded(on: callbackQueue) { + DispatchQueue.apollo_performAsyncIfNeeded(on: callbackQueue) { completion(.success(())) } } diff --git a/Sources/ApolloSQLite/SQLiteNormalizedCache.swift b/Sources/ApolloSQLite/SQLiteNormalizedCache.swift index ce2f2b8cb7..666ae02a0a 100644 --- a/Sources/ApolloSQLite/SQLiteNormalizedCache.swift +++ b/Sources/ApolloSQLite/SQLiteNormalizedCache.swift @@ -94,12 +94,25 @@ public final class SQLiteNormalizedCache { extension SQLiteNormalizedCache: NormalizedCache { - public func merge(records: RecordSet) -> Promise> { - return Promise { try self.mergeRecords(records: records) } + public func merge(records: RecordSet, + callbackQueue: DispatchQueue?, + completion: @escaping (Swift.Result, Error>) -> Void) { + do { + let records = try self.mergeRecords(records: records) + DispatchQueue.apollo_performAsyncIfNeeded(on: callbackQueue) { + completion(.success(records)) + } + } catch { + DispatchQueue.apollo_performAsyncIfNeeded(on: callbackQueue) { + completion(.failure(error)) + } + } } - public func loadRecords(forKeys keys: [CacheKey]) -> Promise<[Record?]> { - return Promise { + public func loadRecords(forKeys keys: [CacheKey], + callbackQueue: DispatchQueue?, + completion: @escaping (Swift.Result<[Record?], Error>) -> Void) { + do { let records = try self.selectRecords(forKeys: keys) let recordsOrNil: [Record?] = keys.map { key in if let recordIndex = records.firstIndex(where: { $0.key == key }) { @@ -107,13 +120,27 @@ extension SQLiteNormalizedCache: NormalizedCache { } return nil } - return recordsOrNil + + DispatchQueue.apollo_performAsyncIfNeeded(on: callbackQueue) { + completion(.success(recordsOrNil)) + } + } catch { + DispatchQueue.apollo_performAsyncIfNeeded(on: callbackQueue) { + completion(.failure(error)) + } } } - public func clear() -> Promise { - return Promise { - return try self.clearRecords() + public func clear(callbackQueue: DispatchQueue?, completion: ((Swift.Result) -> Void)?) { + do { + try self.clearRecords() + DispatchQueue.apollo_performAsyncIfNeeded(on: callbackQueue) { + completion?(.success(())) + } + } catch { + DispatchQueue.apollo_performAsyncIfNeeded(on: callbackQueue) { + completion?(.failure(error)) + } } } } From 7e42e4ae704e423826c313a5c6ff1bdf73f7cb32 Mon Sep 17 00:00:00 2001 From: Ellen Shapiro Date: Tue, 13 Aug 2019 12:12:39 +0200 Subject: [PATCH 09/16] update client and store clear cache signatures --- Sources/Apollo/ApolloClient.swift | 2 +- Sources/Apollo/ApolloClientProtocol.swift | 2 +- Sources/Apollo/ApolloStore.swift | 14 +++++++------- .../FetchQueryTests.swift | 17 ++++++++++++++--- 4 files changed, 23 insertions(+), 12 deletions(-) diff --git a/Sources/Apollo/ApolloClient.swift b/Sources/Apollo/ApolloClient.swift index c39da2df56..5e5ad88f61 100644 --- a/Sources/Apollo/ApolloClient.swift +++ b/Sources/Apollo/ApolloClient.swift @@ -118,7 +118,7 @@ extension ApolloClient: ApolloClientProtocol { } } - public func clearCache(callbackQueue: DispatchQueue = .main, completion: (() -> Void)? = nil) { + public func clearCache(callbackQueue: DispatchQueue = .main, completion: ((Result) -> Void)? = nil) { self.store.clearCache(completion: completion) } diff --git a/Sources/Apollo/ApolloClientProtocol.swift b/Sources/Apollo/ApolloClientProtocol.swift index fbbb276365..912f33b8ec 100644 --- a/Sources/Apollo/ApolloClientProtocol.swift +++ b/Sources/Apollo/ApolloClientProtocol.swift @@ -15,7 +15,7 @@ public protocol ApolloClientProtocol: class { /// - Parameters: /// - callbackQueue: The queue to fall back on. Should default to the main queue. /// - completion: [optional] A completion closure to execute when clearing has completed. Should default to nil. - func clearCache(callbackQueue: DispatchQueue, completion: (() -> Void)?) + func clearCache(callbackQueue: DispatchQueue, completion: ((Result) -> Void)?) /// Fetches a query from the server or from the local cache, depending on the current contents of the cache and the specified cache policy. /// diff --git a/Sources/Apollo/ApolloStore.swift b/Sources/Apollo/ApolloStore.swift index b898f04fd1..d085e37e59 100644 --- a/Sources/Apollo/ApolloStore.swift +++ b/Sources/Apollo/ApolloStore.swift @@ -50,7 +50,7 @@ public final class ApolloStore { /// Clears the instance of the cache. Note that a cache can be shared across multiple `ApolloClient` objects, so clearing that underlying cache will clear it for all clients. /// /// - Returns: A promise which fulfills when the Cache is cleared. - public func clearCache(callbackQueue: DispatchQueue = .main, completion: (() -> Void)? = nil) { + public func clearCache(callbackQueue: DispatchQueue = .main, completion: ((Result) -> Void)? = nil) { queue.async(flags: .barrier) { self.cacheLock.withWriteLock { self.cache.clearPromise() @@ -59,7 +59,7 @@ public final class ApolloStore { return } callbackQueue.async { - completion() + completion(.success(())) } } } @@ -90,7 +90,7 @@ public final class ApolloStore { } } - public func withinReadTransaction(_ body: @escaping (ReadTransaction) throws -> Promise) -> Promise { + func withinReadTransaction(_ body: @escaping (ReadTransaction) throws -> Promise) -> Promise { return Promise { fulfill, reject in self.queue.async { self.cacheLock.lockForReading() @@ -103,13 +103,13 @@ public final class ApolloStore { } } - public func withinReadTransaction(_ body: @escaping (ReadTransaction) throws -> T) -> Promise { + func withinReadTransaction(_ body: @escaping (ReadTransaction) throws -> T) -> Promise { return withinReadTransaction { Promise(fulfilled: try body($0)) } } - public func withinReadWriteTransaction(_ body: @escaping (ReadWriteTransaction) throws -> Promise) -> Promise { + func withinReadWriteTransaction(_ body: @escaping (ReadWriteTransaction) throws -> Promise) -> Promise { return Promise { fulfill, reject in self.queue.async(flags: .barrier) { self.cacheLock.lockForWriting() @@ -121,13 +121,13 @@ public final class ApolloStore { } } - public func withinReadWriteTransaction(_ body: @escaping (ReadWriteTransaction) throws -> T) -> Promise { + func withinReadWriteTransaction(_ body: @escaping (ReadWriteTransaction) throws -> T) -> Promise { return withinReadWriteTransaction { Promise(fulfilled: try body($0)) } } - public func load(query: Query) -> Promise> { + func load(query: Query) -> Promise> { return withinReadTransaction { transaction in let mapper = GraphQLSelectionSetMapper() let dependencyTracker = GraphQLDependencyTracker() diff --git a/Tests/ApolloCacheDependentTests/FetchQueryTests.swift b/Tests/ApolloCacheDependentTests/FetchQueryTests.swift index 8d5a5d93c0..e6cd93e924 100644 --- a/Tests/ApolloCacheDependentTests/FetchQueryTests.swift +++ b/Tests/ApolloCacheDependentTests/FetchQueryTests.swift @@ -254,13 +254,24 @@ class FetchQueryTests: XCTestCase { XCTAssertEqual(graphQLResult.data?.hero?.name, "R2-D2") case .failure(let error): XCTFail("Unexpected error: \(error)") - } + } } self.waitForExpectations(timeout: 5, handler: nil) - do { try client.clearCache().await() } - catch { XCTFail() } + let clearCacheExpectation = self.expectation(description: "cache cleared") + client.clearCache(completion: { result in + switch result { + case .success: + break + case .failure(let error): + XCTFail("Error clearing cache: \(error)") + } + + clearCacheExpectation.fulfill() + }) + + self.waitForExpectations(timeout: 1, handler: nil) let expectation2 = self.expectation(description: "Fetching query") From 18b1d775297a08879880bfb3d89e194f7f05b849 Mon Sep 17 00:00:00 2001 From: Ellen Shapiro Date: Tue, 13 Aug 2019 12:18:30 +0200 Subject: [PATCH 10/16] Switch to expectations for clearing cache --- .../TestCacheProvider.swift | 4 +++- .../CachePersistenceTests.swift | 19 ++++++++++++------- 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/Tests/ApolloSQLiteTestSupport/TestCacheProvider.swift b/Tests/ApolloSQLiteTestSupport/TestCacheProvider.swift index 365f24da31..4f59b9f899 100644 --- a/Tests/ApolloSQLiteTestSupport/TestCacheProvider.swift +++ b/Tests/ApolloSQLiteTestSupport/TestCacheProvider.swift @@ -13,7 +13,9 @@ public class SQLiteTestCacheProvider: TestCacheProvider { let fileURL = fileURL ?? temporarySQLiteFileURL() let cache = try! SQLiteNormalizedCache(fileURL: fileURL) if let initialRecords = initialRecords { - _ = cache.merge(records: initialRecords) // This is synchronous + cache.merge(records: initialRecords, callbackQueue: nil, completion: { _ in + // Theoretically, this should be syncrhonous + }) // This is synchronous } try test(cache) } diff --git a/Tests/ApolloSQLiteTests/CachePersistenceTests.swift b/Tests/ApolloSQLiteTests/CachePersistenceTests.swift index ab6ab2aa18..bac74d1f4c 100644 --- a/Tests/ApolloSQLiteTests/CachePersistenceTests.swift +++ b/Tests/ApolloSQLiteTests/CachePersistenceTests.swift @@ -87,14 +87,19 @@ class CachePersistenceTests: XCTestCase { case .success(let graphQLResult): XCTAssertEqual(graphQLResult.data?.hero?.name, "Luke Skywalker") } + + let cacheClearExpectation = self.expectation(description: "cache cleared") + client.clearCache(completion: { result in + switch result { + case .success: + break + case .failure(let error): + XCTFail("Error clearing cache: \(error)") + } + cacheClearExpectation.fulfill() + }) - do { - try client.clearCache().await() - } catch { - XCTFail("Error Clearing cache: \(error)") - emptyCacheExpectation.fulfill() - return - } + self.waitForExpectations(timeout: 1, handler: nil) client.fetch(query: query, cachePolicy: .returnCacheDataDontFetch) { innerResult in defer { emptyCacheExpectation.fulfill() } From 7cf841c5ef302a0fb7d0e3dab8265c67460b9713 Mon Sep 17 00:00:00 2001 From: Ellen Shapiro Date: Tue, 13 Aug 2019 12:23:53 +0200 Subject: [PATCH 11/16] pull expectation outside of the fetch block --- Tests/ApolloSQLiteTests/CachePersistenceTests.swift | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/Tests/ApolloSQLiteTests/CachePersistenceTests.swift b/Tests/ApolloSQLiteTests/CachePersistenceTests.swift index bac74d1f4c..12ae394435 100644 --- a/Tests/ApolloSQLiteTests/CachePersistenceTests.swift +++ b/Tests/ApolloSQLiteTests/CachePersistenceTests.swift @@ -77,6 +77,7 @@ class CachePersistenceTests: XCTestCase { let networkExpectation = self.expectation(description: "Fetching query from network") let emptyCacheExpectation = self.expectation(description: "Fetch query from empty cache") + let cacheClearExpectation = self.expectation(description: "cache cleared") client.fetch(query: query, cachePolicy: .fetchIgnoringCacheData) { outerResult in defer { networkExpectation.fulfill() } @@ -88,7 +89,6 @@ class CachePersistenceTests: XCTestCase { XCTAssertEqual(graphQLResult.data?.hero?.name, "Luke Skywalker") } - let cacheClearExpectation = self.expectation(description: "cache cleared") client.clearCache(completion: { result in switch result { case .success: @@ -99,8 +99,6 @@ class CachePersistenceTests: XCTestCase { cacheClearExpectation.fulfill() }) - self.waitForExpectations(timeout: 1, handler: nil) - client.fetch(query: query, cachePolicy: .returnCacheDataDontFetch) { innerResult in defer { emptyCacheExpectation.fulfill() } From fbdb1f7bfa18749a84bb651938d3254eeefb9427 Mon Sep 17 00:00:00 2001 From: Ellen Shapiro Date: Tue, 13 Aug 2019 14:30:26 +0200 Subject: [PATCH 12/16] re-expose methods for direct transaction access without promises --- Sources/Apollo/ApolloStore.swift | 58 ++++++++++++++++++++++++++------ 1 file changed, 47 insertions(+), 11 deletions(-) diff --git a/Sources/Apollo/ApolloStore.swift b/Sources/Apollo/ApolloStore.swift index d085e37e59..1d9c2e473a 100644 --- a/Sources/Apollo/ApolloStore.swift +++ b/Sources/Apollo/ApolloStore.swift @@ -90,7 +90,7 @@ public final class ApolloStore { } } - func withinReadTransaction(_ body: @escaping (ReadTransaction) throws -> Promise) -> Promise { + func withinReadTransactionPromise(_ body: @escaping (ReadTransaction) throws -> Promise) -> Promise { return Promise { fulfill, reject in self.queue.async { self.cacheLock.lockForReading() @@ -102,14 +102,32 @@ public final class ApolloStore { self.cacheLock.unlock() } } - - func withinReadTransaction(_ body: @escaping (ReadTransaction) throws -> T) -> Promise { - return withinReadTransaction { - Promise(fulfilled: try body($0)) + + /// Performs an operation within a read transaction + /// + /// - Parameters: + /// - body: The body of the operation to perform. + /// - callbackQueue: [optional] The callback queue to use to perform the completion block on. Will perform on the current queue if not provided. Defaults to nil. + /// - completion: [optional] The completion block to perform when the read transaction completes. Defaults to nil. + public func withinReadTransaction(_ body: @escaping (ReadTransaction) throws -> T, + callbackQueue: DispatchQueue? = nil, + completion: ((Result) -> Void)? = nil) { + _ = self.withinReadTransactionPromise { + Promise(fulfilled: try body($0)) + } + .andThen { object in + DispatchQueue.apollo_performAsyncIfNeeded(on: callbackQueue) { + completion?(.success(object)) + } + } + .catch { error in + DispatchQueue.apollo_performAsyncIfNeeded(on: callbackQueue) { + completion?(.failure(error)) + } } } - func withinReadWriteTransaction(_ body: @escaping (ReadWriteTransaction) throws -> Promise) -> Promise { + func withinReadWriteTransactionPromise(_ body: @escaping (ReadWriteTransaction) throws -> Promise) -> Promise { return Promise { fulfill, reject in self.queue.async(flags: .barrier) { self.cacheLock.lockForWriting() @@ -120,15 +138,33 @@ public final class ApolloStore { self.cacheLock.unlock() } } - - func withinReadWriteTransaction(_ body: @escaping (ReadWriteTransaction) throws -> T) -> Promise { - return withinReadWriteTransaction { - Promise(fulfilled: try body($0)) + + /// Performs an operation within a read-write transaction + /// + /// - Parameters: + /// - body: The body of the operation to perform + /// - callbackQueue: [optional] a callback queue to perform the action on. Will perform on the current queue if not provided. Defaults to nil. + /// - completion: [optional] a completion block to fire when the read-write transaction completes. Defaults to nil. + public func withinReadWriteTransaction(_ body: @escaping (ReadWriteTransaction) throws -> T, + callbackQueue: DispatchQueue? = nil, + completion: ((Result) -> Void)? = nil) { + _ = self.withinReadWriteTransactionPromise { + Promise(fulfilled: try body($0)) + } + .andThen { object in + DispatchQueue.apollo_performAsyncIfNeeded(on: callbackQueue) { + completion?(.success(object)) + } + } + .catch { error in + DispatchQueue.apollo_performAsyncIfNeeded(on: callbackQueue) { + completion?(.failure(error)) + } } } func load(query: Query) -> Promise> { - return withinReadTransaction { transaction in + return withinReadTransactionPromise { transaction in let mapper = GraphQLSelectionSetMapper() let dependencyTracker = GraphQLDependencyTracker() From 1fc4b86a9a71563da93187f0c671daa44c01f723 Mon Sep 17 00:00:00 2001 From: Ellen Shapiro Date: Tue, 13 Aug 2019 17:04:27 +0200 Subject: [PATCH 13/16] fix perform async if needed in load tests --- Tests/ApolloTests/BatchedLoadTests.swift | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Tests/ApolloTests/BatchedLoadTests.swift b/Tests/ApolloTests/BatchedLoadTests.swift index b784fb27c2..ebb6d93b22 100644 --- a/Tests/ApolloTests/BatchedLoadTests.swift +++ b/Tests/ApolloTests/BatchedLoadTests.swift @@ -19,7 +19,7 @@ private final class MockBatchedNormalizedCache: NormalizedCache { DispatchQueue.global().asyncAfter(deadline: .now() + .milliseconds(1)) { let records = keys.map { self.records[$0] } - DispatchQueue.performAsyncIfNeeded(on: callbackQueue) { + DispatchQueue.apollo_performAsyncIfNeeded(on: callbackQueue) { completion(.success(records)) } } @@ -30,7 +30,7 @@ private final class MockBatchedNormalizedCache: NormalizedCache { completion: @escaping (Result, Error>) -> Void) { DispatchQueue.global().asyncAfter(deadline: .now() + .milliseconds(1)) { let changedKeys = self.records.merge(records: records) - DispatchQueue.performAsyncIfNeeded(on: callbackQueue) { + DispatchQueue.apollo_performAsyncIfNeeded(on: callbackQueue) { completion(.success(changedKeys)) } } @@ -39,7 +39,7 @@ private final class MockBatchedNormalizedCache: NormalizedCache { func clear(callbackQueue: DispatchQueue?, completion: ((Result) -> Void)?) { DispatchQueue.global().asyncAfter(deadline: .now() + .milliseconds(1)) { self.records.clear() - DispatchQueue.performAsyncIfNeeded(on: callbackQueue) { + DispatchQueue.apollo_performAsyncIfNeeded(on: callbackQueue) { completion?(.success(())) } } From 0f31624e1c09cad3a625da9872ab0b717c0c37e4 Mon Sep 17 00:00:00 2001 From: Ellen Shapiro Date: Tue, 13 Aug 2019 17:06:29 +0200 Subject: [PATCH 14/16] Update tests to use non-promise reading + writing --- .../ReadWriteFromStoreTests.swift | 122 ++++++++++++------ .../WatchQueryTests.swift | 7 +- 2 files changed, 91 insertions(+), 38 deletions(-) diff --git a/Tests/ApolloCacheDependentTests/ReadWriteFromStoreTests.swift b/Tests/ApolloCacheDependentTests/ReadWriteFromStoreTests.swift index b0ab715cec..73104c0871 100644 --- a/Tests/ApolloCacheDependentTests/ReadWriteFromStoreTests.swift +++ b/Tests/ApolloCacheDependentTests/ReadWriteFromStoreTests.swift @@ -9,19 +9,22 @@ class ReadWriteFromStoreTests: XCTestCase { "QUERY_ROOT": ["hero": Reference(key: "hero")], "hero": ["__typename": "Droid", "name": "R2-D2"] ] - - try withCache(initialRecords: initialRecords) { (cache) in + let expectation = self.expectation(description: "transaction'd") + withCache(initialRecords: initialRecords) { (cache) in let store = ApolloStore(cache: cache) let query = HeroNameQuery() - - try await(store.withinReadTransaction { transaction in + + store.withinReadTransaction({transaction in let data = try transaction.read(query: query) XCTAssertEqual(data.hero?.__typename, "Droid") XCTAssertEqual(data.hero?.name, "R2-D2") + expectation.fulfill() }) } + + self.waitForExpectations(timeout: 1, handler: nil) } func testReadHeroNameQueryWithVariable() throws { @@ -30,18 +33,22 @@ class ReadWriteFromStoreTests: XCTestCase { "hero(episode:JEDI)": ["__typename": "Droid", "name": "R2-D2"] ] - try withCache(initialRecords: initialRecords) { (cache) in + let expectation = self.expectation(description: "transaction'd") + withCache(initialRecords: initialRecords) { (cache) in let store = ApolloStore(cache: cache) let query = HeroNameQuery(episode: .jedi) - - try await(store.withinReadTransaction { transaction in + + store.withinReadTransaction({ transaction in let data = try transaction.read(query: query) XCTAssertEqual(data.hero?.__typename, "Droid") XCTAssertEqual(data.hero?.name, "R2-D2") + expectation.fulfill() }) } + + self.waitForExpectations(timeout: 1, handler: nil) } func testReadHeroNameQueryWithMissingName() throws { @@ -50,12 +57,13 @@ class ReadWriteFromStoreTests: XCTestCase { "hero": ["__typename": "Droid"] ] - try withCache(initialRecords: initialRecords) { (cache) in + let expectation = self.expectation(description: "transaction'd") + withCache(initialRecords: initialRecords) { (cache) in let store = ApolloStore(cache: cache) let query = HeroNameQuery() - try await(store.withinReadTransaction { transaction in + store.withinReadTransaction({ transaction in XCTAssertThrowsError(try transaction.read(query: query)) { error in if case let error as GraphQLResultError = error { XCTAssertEqual(error.path, ["hero", "name"]) @@ -63,8 +71,11 @@ class ReadWriteFromStoreTests: XCTestCase { } else { XCTFail("Unexpected error: \(error)") } + expectation.fulfill() } }) + + self.waitForExpectations(timeout: 1, handler: nil) } } @@ -78,12 +89,16 @@ class ReadWriteFromStoreTests: XCTestCase { let store = ApolloStore(cache: cache) let query = HeroNameQuery() + let expectation = self.expectation(description: "transaction'd") - try await(store.withinReadWriteTransaction { transaction in + store.withinReadWriteTransaction({ transaction in try transaction.update(query: query) { (data: inout HeroNameQuery.Data) in data.hero?.name = "Artoo" + expectation.fulfill() } }) + + self.waitForExpectations(timeout: 1, handler: nil) let result = try await(store.load(query: query)) @@ -93,28 +108,38 @@ class ReadWriteFromStoreTests: XCTestCase { } func testWriteHeroNameQueryWhenWriteErrorIsThrown() throws { + let expectation = self.expectation(description: "transaction'd") do { - try withCache(initialRecords: nil) { (cache) in + withCache(initialRecords: nil) { (cache) in let store = ApolloStore(cache: cache) - try store.withinReadWriteTransaction { transaction in + store.withinReadWriteTransaction({ transaction in let data = HeroNameQuery.Data(unsafeResultMap: [:]) try transaction.write(data: data, forQuery: HeroNameQuery(episode: nil)) - }.await() - XCTFail("write should fail") - } - } catch { - guard let error = error as? GraphQLResultError, - let jsonError = error.underlying as? JSONDecodingError else { - XCTFail("unexpected error") - return - } - - switch jsonError { - case .missingValue: break - default: XCTFail("unexpected error") + }, completion: { result in + defer { + expectation.fulfill() + } + switch result { + case .success: + XCTFail("write should fail") + case .failure(let error): + guard let error = error as? GraphQLResultError, + let jsonError = error.underlying as? JSONDecodingError else { + XCTFail("unexpected error") + return + } + + switch jsonError { + case .missingValue: break + default: XCTFail("unexpected error") + } + } + }) } } + + self.waitForExpectations(timeout: 1, handler: nil) } func testReadHeroAndFriendsNamesQuery() throws { @@ -134,18 +159,22 @@ class ReadWriteFromStoreTests: XCTestCase { "1003": ["__typename": "Human", "name": "Leia Organa"], ] - try withCache(initialRecords: initialRecords) { (cache) in + withCache(initialRecords: initialRecords) { (cache) in let store = ApolloStore(cache: cache) let query = HeroAndFriendsNamesQuery() - try await(store.withinReadTransaction { transaction in + let expectation = self.expectation(description: "transaction'd") + store.withinReadTransaction({ transaction in let data = try transaction.read(query: query) XCTAssertEqual(data.hero?.name, "R2-D2") let friendsNames = data.hero?.friends?.compactMap { $0?.name } XCTAssertEqual(friendsNames, ["Luke Skywalker", "Han Solo", "Leia Organa"]) + expectation.fulfill() }) + + self.waitForExpectations(timeout: 1, handler: nil) } } @@ -171,11 +200,14 @@ class ReadWriteFromStoreTests: XCTestCase { let query = HeroAndFriendsNamesQuery() - try await(store.withinReadWriteTransaction { transaction in + let expectation = self.expectation(description: "transaction'd") + store.withinReadWriteTransaction({ transaction in try transaction.update(query: query) { (data: inout HeroAndFriendsNamesQuery.Data) in data.hero?.friends?.append(.makeDroid(name: "C-3PO")) + expectation.fulfill() } }) + self.waitForExpectations(timeout: 1, handler: nil) let result = try await(store.load(query: query)) guard let data = result.data else { XCTFail(); return } @@ -208,11 +240,14 @@ class ReadWriteFromStoreTests: XCTestCase { let query = HeroAndFriendsNamesQuery(episode: Episode.newhope) - try await(store.withinReadWriteTransaction { transaction in + let expectation = self.expectation(description: "transaction'd") + store.withinReadWriteTransaction({ transaction in try transaction.update(query: query) { (data: inout HeroAndFriendsNamesQuery.Data) in data.hero?.friends?.append(.makeDroid(name: "C-3PO")) + expectation.fulfill() } }) + self.waitForExpectations(timeout: 1, handler: nil) let result = try await(store.load(query: query)) guard let data = result.data else { XCTFail(); return } @@ -228,15 +263,19 @@ class ReadWriteFromStoreTests: XCTestCase { "2001": ["name": "R2-D2", "__typename": "Droid", "primaryFunction": "Protocol"] ] - try withCache(initialRecords: initialRecords) { (cache) in + withCache(initialRecords: initialRecords) { (cache) in let store = ApolloStore(cache: cache) - try await(store.withinReadTransaction { transaction in + let expectation = self.expectation(description: "transaction'd") + store.withinReadTransaction({ transaction in let r2d2 = try transaction.readObject(ofType: HeroDetails.self, withKey: "2001") XCTAssertEqual(r2d2.name, "R2-D2") XCTAssertEqual(r2d2.asDroid?.primaryFunction, "Protocol") + expectation.fulfill() }) + + self.waitForExpectations(timeout: 1, handler: nil) } } @@ -245,10 +284,11 @@ class ReadWriteFromStoreTests: XCTestCase { "2001": ["name": "R2-D2", "__typename": "Droid"] ] - try withCache(initialRecords: initialRecords) { (cache) in + withCache(initialRecords: initialRecords) { (cache) in let store = ApolloStore(cache: cache) - try await(store.withinReadTransaction { transaction in + let expectation = self.expectation(description: "transaction'd") + store.withinReadTransaction({ transaction in XCTAssertThrowsError(try transaction.readObject(ofType: HeroDetails.self, withKey: "2001")) { error in if case let error as GraphQLResultError = error { XCTAssertEqual(error.path, ["primaryFunction"]) @@ -256,8 +296,11 @@ class ReadWriteFromStoreTests: XCTestCase { } else { XCTFail("Unexpected error: \(error)") } + + expectation.fulfill() } }) + self.waitForExpectations(timeout: 1, handler: nil) } } @@ -278,15 +321,19 @@ class ReadWriteFromStoreTests: XCTestCase { "1003": ["__typename": "Human", "name": "Leia Organa"], ] - try withCache(initialRecords: initialRecords) { (cache) in + withCache(initialRecords: initialRecords) { (cache) in let store = ApolloStore(cache: cache) - try await(store.withinReadTransaction { transaction in + let expectation = self.expectation(description: "transaction'd") + store.withinReadTransaction({ transaction in let friendsNamesFragment = try transaction.readObject(ofType: FriendsNames.self, withKey: "2001") let friendsNames = friendsNamesFragment.friends?.compactMap { $0?.name } XCTAssertEqual(friendsNames, ["Luke Skywalker", "Han Solo", "Leia Organa"]) + expectation.fulfill() }) + + self.waitForExpectations(timeout: 1, handler: nil) } } @@ -310,11 +357,14 @@ class ReadWriteFromStoreTests: XCTestCase { try withCache(initialRecords: initialRecords) { (cache) in let store = ApolloStore(cache: cache) - try await(store.withinReadWriteTransaction { transaction in + let expectation = self.expectation(description: "transaction'd") + store.withinReadWriteTransaction({ transaction in try transaction.updateObject(ofType: FriendsNames.self, withKey: "2001") { (friendsNames: inout FriendsNames) in friendsNames.friends?.append(.makeDroid(name: "C-3PO")) + expectation.fulfill() } }) + self.waitForExpectations(timeout: 1, handler: nil) let result = try await(store.load(query: HeroAndFriendsNamesQuery())) guard let data = result.data else { XCTFail(); return } diff --git a/Tests/ApolloCacheDependentTests/WatchQueryTests.swift b/Tests/ApolloCacheDependentTests/WatchQueryTests.swift index d3e8c1fbe6..c20da78a6c 100644 --- a/Tests/ApolloCacheDependentTests/WatchQueryTests.swift +++ b/Tests/ApolloCacheDependentTests/WatchQueryTests.swift @@ -312,7 +312,7 @@ class WatchQueryTests: XCTestCase { "QUERY_ROOT.hero.friends.1": ["__typename": "Human", "name": "Han Solo"], "QUERY_ROOT.hero.friends.2": ["__typename": "Human", "name": "Leia Organa"], ] - try withCache(initialRecords: initialRecords) { (cache) in + withCache(initialRecords: initialRecords) { (cache) in let networkTransport = MockNetworkTransport(body: [:]) let store = ApolloStore(cache: cache) @@ -353,11 +353,14 @@ class WatchQueryTests: XCTestCase { waitForExpectations(timeout: 5, handler: nil) let nameQuery = HeroNameQuery() - try await(store.withinReadWriteTransaction { transaction in + expectation = self.expectation(description: "transaction'd") + store.withinReadWriteTransaction({ transaction in try transaction.update(query: nameQuery) { (data: inout HeroNameQuery.Data) in data.hero?.name = "Artoo" } + expectation.fulfill() }) + self.waitForExpectations(timeout: 1, handler: nil) verifyResult = { result in switch result { From a1b2450214c4588ec33e16fee244a3e1db4f6f04 Mon Sep 17 00:00:00 2001 From: Ellen Shapiro Date: Thu, 5 Sep 2019 17:15:49 -0500 Subject: [PATCH 15/16] add convenience method to return a result asynchronously that can handle optional completion blocks --- Sources/Apollo/ApolloStore.swift | 24 ++++++------ Sources/Apollo/DispatchQueue+Optional.swift | 10 +++++ Sources/Apollo/InMemoryNormalizedCache.swift | 18 ++++----- .../ApolloSQLite/SQLiteNormalizedCache.swift | 39 ++++++++++--------- Tests/ApolloTests/BatchedLoadTests.swift | 18 ++++----- 5 files changed, 61 insertions(+), 48 deletions(-) diff --git a/Sources/Apollo/ApolloStore.swift b/Sources/Apollo/ApolloStore.swift index 1d9c2e473a..c834f061b4 100644 --- a/Sources/Apollo/ApolloStore.swift +++ b/Sources/Apollo/ApolloStore.swift @@ -116,14 +116,14 @@ public final class ApolloStore { Promise(fulfilled: try body($0)) } .andThen { object in - DispatchQueue.apollo_performAsyncIfNeeded(on: callbackQueue) { - completion?(.success(object)) - } + DispatchQueue.apollo_returnResultAsyncIfNeeded(on: callbackQueue, + action: completion, + result: .success(object)) } .catch { error in - DispatchQueue.apollo_performAsyncIfNeeded(on: callbackQueue) { - completion?(.failure(error)) - } + DispatchQueue.apollo_returnResultAsyncIfNeeded(on: callbackQueue, + action: completion, + result: .failure(error)) } } @@ -152,15 +152,15 @@ public final class ApolloStore { Promise(fulfilled: try body($0)) } .andThen { object in - DispatchQueue.apollo_performAsyncIfNeeded(on: callbackQueue) { - completion?(.success(object)) - } + DispatchQueue.apollo_returnResultAsyncIfNeeded(on: callbackQueue, + action: completion, + result: .success(object)) } .catch { error in - DispatchQueue.apollo_performAsyncIfNeeded(on: callbackQueue) { - completion?(.failure(error)) + DispatchQueue.apollo_returnResultAsyncIfNeeded(on: callbackQueue, + action: completion, + result: .failure(error)) } - } } func load(query: Query) -> Promise> { diff --git a/Sources/Apollo/DispatchQueue+Optional.swift b/Sources/Apollo/DispatchQueue+Optional.swift index 1b7f6d4efa..38e0bcf031 100644 --- a/Sources/Apollo/DispatchQueue+Optional.swift +++ b/Sources/Apollo/DispatchQueue+Optional.swift @@ -21,4 +21,14 @@ public extension DispatchQueue { action() } } + + static func apollo_returnResultAsyncIfNeeded(on callbackQueue: DispatchQueue?, action: ((Result) -> Void)?, result: Result) { + guard let action = action else { + return + } + + self.apollo_performAsyncIfNeeded(on: callbackQueue) { + action(result) + } + } } diff --git a/Sources/Apollo/InMemoryNormalizedCache.swift b/Sources/Apollo/InMemoryNormalizedCache.swift index 55b2fbbc79..0e0d948c7a 100644 --- a/Sources/Apollo/InMemoryNormalizedCache.swift +++ b/Sources/Apollo/InMemoryNormalizedCache.swift @@ -9,18 +9,18 @@ public final class InMemoryNormalizedCache: NormalizedCache { callbackQueue: DispatchQueue?, completion: @escaping (Result<[Record?], Error>) -> Void) { let records = keys.map { self.records[$0] } - DispatchQueue.apollo_performAsyncIfNeeded(on: callbackQueue) { - completion(.success(records)) - } + DispatchQueue.apollo_returnResultAsyncIfNeeded(on: callbackQueue, + action: completion, + result: .success(records)) } public func merge(records: RecordSet, callbackQueue: DispatchQueue?, completion: @escaping (Result, Error>) -> Void) { let cacheKeys = self.records.merge(records: records) - DispatchQueue.apollo_performAsyncIfNeeded(on: callbackQueue) { - completion(.success(cacheKeys)) - } + DispatchQueue.apollo_returnResultAsyncIfNeeded(on: callbackQueue, + action: completion, + result: .success(cacheKeys)) } public func clear(callbackQueue: DispatchQueue?, @@ -31,8 +31,8 @@ public final class InMemoryNormalizedCache: NormalizedCache { return } - DispatchQueue.apollo_performAsyncIfNeeded(on: callbackQueue) { - completion(.success(())) - } + DispatchQueue.apollo_returnResultAsyncIfNeeded(on: callbackQueue, + action: completion, + result: .success(())) } } diff --git a/Sources/ApolloSQLite/SQLiteNormalizedCache.swift b/Sources/ApolloSQLite/SQLiteNormalizedCache.swift index 666ae02a0a..3f819152f7 100644 --- a/Sources/ApolloSQLite/SQLiteNormalizedCache.swift +++ b/Sources/ApolloSQLite/SQLiteNormalizedCache.swift @@ -97,21 +97,23 @@ extension SQLiteNormalizedCache: NormalizedCache { public func merge(records: RecordSet, callbackQueue: DispatchQueue?, completion: @escaping (Swift.Result, Error>) -> Void) { + let result: Swift.Result, Error> do { let records = try self.mergeRecords(records: records) - DispatchQueue.apollo_performAsyncIfNeeded(on: callbackQueue) { - completion(.success(records)) - } + result = .success(records) } catch { - DispatchQueue.apollo_performAsyncIfNeeded(on: callbackQueue) { - completion(.failure(error)) - } + result = .failure(error) } + + DispatchQueue.apollo_returnResultAsyncIfNeeded(on: callbackQueue, + action: completion, + result: result) } public func loadRecords(forKeys keys: [CacheKey], callbackQueue: DispatchQueue?, completion: @escaping (Swift.Result<[Record?], Error>) -> Void) { + let result: Swift.Result<[Record?], Error> do { let records = try self.selectRecords(forKeys: keys) let recordsOrNil: [Record?] = keys.map { key in @@ -121,26 +123,27 @@ extension SQLiteNormalizedCache: NormalizedCache { return nil } - DispatchQueue.apollo_performAsyncIfNeeded(on: callbackQueue) { - completion(.success(recordsOrNil)) - } + result = .success(recordsOrNil) } catch { - DispatchQueue.apollo_performAsyncIfNeeded(on: callbackQueue) { - completion(.failure(error)) - } + result = .failure(error) } + + DispatchQueue.apollo_returnResultAsyncIfNeeded(on: callbackQueue, + action: completion, + result: result) } public func clear(callbackQueue: DispatchQueue?, completion: ((Swift.Result) -> Void)?) { + let result: Swift.Result do { try self.clearRecords() - DispatchQueue.apollo_performAsyncIfNeeded(on: callbackQueue) { - completion?(.success(())) - } + result = .success(()) } catch { - DispatchQueue.apollo_performAsyncIfNeeded(on: callbackQueue) { - completion?(.failure(error)) - } + result = .failure(error) } + + DispatchQueue.apollo_returnResultAsyncIfNeeded(on: callbackQueue, + action: completion, + result: result) } } diff --git a/Tests/ApolloTests/BatchedLoadTests.swift b/Tests/ApolloTests/BatchedLoadTests.swift index ebb6d93b22..5bda78292b 100644 --- a/Tests/ApolloTests/BatchedLoadTests.swift +++ b/Tests/ApolloTests/BatchedLoadTests.swift @@ -19,9 +19,9 @@ private final class MockBatchedNormalizedCache: NormalizedCache { DispatchQueue.global().asyncAfter(deadline: .now() + .milliseconds(1)) { let records = keys.map { self.records[$0] } - DispatchQueue.apollo_performAsyncIfNeeded(on: callbackQueue) { - completion(.success(records)) - } + DispatchQueue.apollo_returnResultAsyncIfNeeded(on: callbackQueue, + action: completion, + result: .success(records)) } } @@ -30,18 +30,18 @@ private final class MockBatchedNormalizedCache: NormalizedCache { completion: @escaping (Result, Error>) -> Void) { DispatchQueue.global().asyncAfter(deadline: .now() + .milliseconds(1)) { let changedKeys = self.records.merge(records: records) - DispatchQueue.apollo_performAsyncIfNeeded(on: callbackQueue) { - completion(.success(changedKeys)) - } + DispatchQueue.apollo_returnResultAsyncIfNeeded(on: callbackQueue, + action: completion, + result: .success(changedKeys)) } } func clear(callbackQueue: DispatchQueue?, completion: ((Result) -> Void)?) { DispatchQueue.global().asyncAfter(deadline: .now() + .milliseconds(1)) { self.records.clear() - DispatchQueue.apollo_performAsyncIfNeeded(on: callbackQueue) { - completion?(.success(())) - } + DispatchQueue.apollo_returnResultAsyncIfNeeded(on: callbackQueue, + action: completion, + result: .success(())) } } } From 8f24739cbeacd732b0d50c3a712e22e6caf86294 Mon Sep 17 00:00:00 2001 From: Ellen Shapiro Date: Thu, 5 Sep 2019 20:15:36 -0500 Subject: [PATCH 16/16] =?UTF-8?q?use=20return=20result=20async=20if=20need?= =?UTF-8?q?ed=20where=20actual=20comment=20was=20=F0=9F=98=9B?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- Sources/Apollo/ApolloStore.swift | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/Sources/Apollo/ApolloStore.swift b/Sources/Apollo/ApolloStore.swift index c834f061b4..47c1d6e003 100644 --- a/Sources/Apollo/ApolloStore.swift +++ b/Sources/Apollo/ApolloStore.swift @@ -55,12 +55,9 @@ public final class ApolloStore { self.cacheLock.withWriteLock { self.cache.clearPromise() }.andThen { - guard let completion = completion else { - return - } - callbackQueue.async { - completion(.success(())) - } + DispatchQueue.apollo_returnResultAsyncIfNeeded(on: callbackQueue, + action: completion, + result: .success(())) } } }