From 9927eae82ee3115f21d928d030fbe82855230b4c Mon Sep 17 00:00:00 2001 From: Anthony Miller Date: Thu, 15 Dec 2022 12:57:37 -0800 Subject: [PATCH] Move handling of missing values from Executor to Field Resolvers --- Sources/Apollo/ApolloStore.swift | 27 ++++++++++++------- Sources/Apollo/GraphQLExecutor.swift | 14 +++++----- Sources/Apollo/GraphQLResponse.swift | 5 +++- ..._ResultNormalizer_FromResponse_Tests.swift | 5 +++- ...electionSetMapper_FromResponse_Tests.swift | 5 +++- 5 files changed, 38 insertions(+), 18 deletions(-) diff --git a/Sources/Apollo/ApolloStore.swift b/Sources/Apollo/ApolloStore.swift index 37952f1de8..4adf393e5f 100644 --- a/Sources/Apollo/ApolloStore.swift +++ b/Sources/Apollo/ApolloStore.swift @@ -184,14 +184,17 @@ public class ApolloStore { fileprivate let cache: NormalizedCache fileprivate lazy var loader: DataLoader = DataLoader(self.cache.loadRecords) - fileprivate lazy var executor = GraphQLExecutor { object, info in - return object[info.cacheKeyForField] - } resolveReference: { [weak self] reference in - guard let self = self else { - return .immediate(.failure(ApolloStore.Error.notWithinReadTransaction)) - } - return self.loadObject(forKey: reference.key) + fileprivate lazy var readExecutor = GraphQLExecutor { object, info in + guard let value = object[info.cacheKeyForField] else { + throw JSONDecodingError.missingValue + } + return value + } resolveReference: { [weak self] reference in + guard let self = self else { + return .immediate(.failure(ApolloStore.Error.notWithinReadTransaction)) } + return self.loadObject(forKey: reference.key) + } fileprivate init(store: ApolloStore) { self.cache = store.cache @@ -226,7 +229,7 @@ public class ApolloStore { ) throws -> Accumulator.FinalResult { let object = try loadObject(forKey: key).get() - return try executor.execute( + return try readExecutor.execute( selectionSet: type, on: object, withRootCacheReference: CacheReference(key), @@ -293,7 +296,13 @@ public class ApolloStore { ) throws { let normalizer = GraphQLResultNormalizer() let executor = GraphQLExecutor { object, info in - return object[info.responseKeyForField] + guard let value = object[info.responseKeyForField] else { + guard info.field.type.isNullable else { + throw JSONDecodingError.missingValue + } + return NSNull() + } + return value } let records = try executor.execute( diff --git a/Sources/Apollo/GraphQLExecutor.swift b/Sources/Apollo/GraphQLExecutor.swift index 6bd24e7915..af5943bc4b 100644 --- a/Sources/Apollo/GraphQLExecutor.swift +++ b/Sources/Apollo/GraphQLExecutor.swift @@ -3,8 +3,13 @@ import Foundation import ApolloAPI #endif -/// A field resolver is responsible for resolving a value for a field. -typealias GraphQLFieldResolver = (_ object: JSONObject, _ info: FieldExecutionInfo) -> JSONValue? +/// A field resolver is responsible for resolving a value for a field from a JSON object. +/// +/// Because the field resolver should have context on the source of JSON object, it is responsible +/// for knowing how to handle missing fields. It must either return a valid value or throw an error. +/// If the object has no value for the field, the resolver can return `NSNull` or throw a +/// `JSONDecodingError`. +typealias GraphQLFieldResolver = (_ object: JSONObject, _ info: FieldExecutionInfo) throws -> JSONValue /// A reference resolver is responsible for resolving an object based on its key. These references are /// used in normalized records, and data for these objects has to be loaded from the cache for execution to continue. @@ -308,10 +313,7 @@ final class GraphQLExecutor { } return PossiblyDeferred { - guard let value = fieldResolver(object, fieldInfo) else { - throw JSONDecodingError.missingValue - } - return value + try fieldResolver(object, fieldInfo) }.flatMap { return self.complete(fields: fieldInfo, withValue: $0, diff --git a/Sources/Apollo/GraphQLResponse.swift b/Sources/Apollo/GraphQLResponse.swift index ac5ada0f87..1a8be83a04 100644 --- a/Sources/Apollo/GraphQLResponse.swift +++ b/Sources/Apollo/GraphQLResponse.swift @@ -41,7 +41,10 @@ public final class GraphQLResponse { } let executor = GraphQLExecutor { object, info in - return object[info.responseKeyForField] + guard let value = object[info.responseKeyForField] else { + throw JSONDecodingError.missingValue + } + return value } executor.shouldComputeCachePath = computeCachePaths diff --git a/Tests/ApolloTests/GraphQLExecutor_ResultNormalizer_FromResponse_Tests.swift b/Tests/ApolloTests/GraphQLExecutor_ResultNormalizer_FromResponse_Tests.swift index eb014405f1..6234df9078 100644 --- a/Tests/ApolloTests/GraphQLExecutor_ResultNormalizer_FromResponse_Tests.swift +++ b/Tests/ApolloTests/GraphQLExecutor_ResultNormalizer_FromResponse_Tests.swift @@ -9,7 +9,10 @@ class GraphQLExecutor_ResultNormalizer_FromResponse_Tests: XCTestCase { private static let executor: GraphQLExecutor = { let executor = GraphQLExecutor { object, info in - return object[info.responseKeyForField] + guard let value = object[info.responseKeyForField] else { + throw JSONDecodingError.missingValue + } + return value } executor.shouldComputeCachePath = true diff --git a/Tests/ApolloTests/GraphQLExecutor_SelectionSetMapper_FromResponse_Tests.swift b/Tests/ApolloTests/GraphQLExecutor_SelectionSetMapper_FromResponse_Tests.swift index 1343f65264..e7fcd0e786 100644 --- a/Tests/ApolloTests/GraphQLExecutor_SelectionSetMapper_FromResponse_Tests.swift +++ b/Tests/ApolloTests/GraphQLExecutor_SelectionSetMapper_FromResponse_Tests.swift @@ -11,7 +11,10 @@ class GraphQLExecutor_SelectionSetMapper_FromResponse_Tests: XCTestCase { private static let executor: GraphQLExecutor = { let executor = GraphQLExecutor { object, info in - return object[info.responseKeyForField] + guard let value = object[info.responseKeyForField] else { + throw JSONDecodingError.missingValue + } + return value } executor.shouldComputeCachePath = false return executor