Cache mutation nullability bug

Apollo.xcodeproj/project.pbxproj

@@ -124,7 +124,6 @@
 		9F69FFA91D42855900E000B1 /* NetworkTransport.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9F69FFA81D42855900E000B1 /* NetworkTransport.swift */; };
 		9F8622FA1EC2117C00C38162 /* FragmentConstructionAndConversionTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9F8622F91EC2117C00C38162 /* FragmentConstructionAndConversionTests.swift */; };
 		9F86B68B1E6438D700B885FF /* GraphQLSelectionSetMapper.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9F86B68A1E6438D700B885FF /* GraphQLSelectionSetMapper.swift */; };
-		9F86B6901E65533D00B885FF /* GraphQLResponseGenerator.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9F86B68F1E65533D00B885FF /* GraphQLResponseGenerator.swift */; };
 		9F8A958D1EC0FFAB00304A2D /* ApolloInternalTestHelpers.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 9F8A95781EC0FC1200304A2D /* ApolloInternalTestHelpers.framework */; };
 		9F8E0BD325668552000D9FA5 /* DataLoader.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9FADC84E1E6B865E00C677E6 /* DataLoader.swift */; };
 		9F8E0BE325668559000D9FA5 /* PossiblyDeferred.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9F33D6A32566475600A1543F /* PossiblyDeferred.swift */; };
@@ -1260,7 +1259,6 @@
 		9F8622F71EC2004200C38162 /* ReadWriteFromStoreTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ReadWriteFromStoreTests.swift; sourceTree = "<group>"; };
 		9F8622F91EC2117C00C38162 /* FragmentConstructionAndConversionTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = FragmentConstructionAndConversionTests.swift; sourceTree = "<group>"; };
 		9F86B68A1E6438D700B885FF /* GraphQLSelectionSetMapper.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = GraphQLSelectionSetMapper.swift; sourceTree = "<group>"; };
-		9F86B68F1E65533D00B885FF /* GraphQLResponseGenerator.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = GraphQLResponseGenerator.swift; sourceTree = "<group>"; };
 		9F8A95781EC0FC1200304A2D /* ApolloInternalTestHelpers.framework */ = {isa = PBXFileReference; explicitFileType = wrapper.framework; includeInIndex = 0; path = ApolloInternalTestHelpers.framework; sourceTree = BUILT_PRODUCTS_DIR; };
 		9F91CF8E1F6C0DB2008DD0BE /* MutatingResultsTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MutatingResultsTests.swift; sourceTree = "<group>"; };
 		9FA6ABC51EC0A9F7000017BE /* FetchQueryTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = FetchQueryTests.swift; sourceTree = "<group>"; };
@@ -3750,7 +3748,6 @@
 			children = (
 				9FC2333C1E66BBF7001E4541 /* GraphQLDependencyTracker.swift */,
 				9F295E371E277B2A00A24949 /* GraphQLResultNormalizer.swift */,
-				9F86B68F1E65533D00B885FF /* GraphQLResponseGenerator.swift */,
 				9F86B68A1E6438D700B885FF /* GraphQLSelectionSetMapper.swift */,
 			);
 			name = Accumulators;
@@ -5420,7 +5417,6 @@
 				9BC742AC24CFB2FF0029282C /* ApolloErrorInterceptor.swift in Sources */,
 				9FC750631D2A59F600458D91 /* ApolloClient.swift in Sources */,
 				9BA3130E2302BEA5007B7FC5 /* DispatchQueue+Optional.swift in Sources */,
-				9F86B6901E65533D00B885FF /* GraphQLResponseGenerator.swift in Sources */,
 			);
 			runOnlyForDeploymentPostprocessing = 0;
 		};

Sources/Apollo/ApolloStore.swift

@@ -101,9 +101,11 @@ public class ApolloStore {
   ///   - 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<T>(_ body: @escaping (ReadTransaction) throws -> T,
-                                       callbackQueue: DispatchQueue? = nil,
-                                       completion: ((Result<T, Swift.Error>) -> Void)? = nil) {
+  public func withinReadTransaction<T>(
+    _ body: @escaping (ReadTransaction) throws -> T,
+    callbackQueue: DispatchQueue? = nil,
+    completion: ((Result<T, Swift.Error>) -> Void)? = nil
+  ) {
     self.queue.async {
       do {
         let returnValue = try body(ReadTransaction(store: self))
@@ -129,9 +131,11 @@ public class ApolloStore {
   ///   - 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<T>(_ body: @escaping (ReadWriteTransaction) throws -> T,
-                                            callbackQueue: DispatchQueue? = nil,
-                                            completion: ((Result<T, Swift.Error>) -> Void)? = nil) {
+  public func withinReadWriteTransaction<T>(
+    _ body: @escaping (ReadWriteTransaction) throws -> T,
+    callbackQueue: DispatchQueue? = nil,
+    completion: ((Result<T, Swift.Error>) -> Void)? = nil
+  ) {
     self.queue.async(flags: .barrier) {
       do {
         let returnValue = try body(ReadWriteTransaction(store: self))
@@ -156,7 +160,11 @@ public class ApolloStore {
   /// - Parameters:
   ///   - query: The query to load results for
   ///   - resultHandler: The completion handler to execute on success or error
-  public func load<Operation: GraphQLOperation>(_ operation: Operation, callbackQueue: DispatchQueue? = nil, resultHandler: @escaping GraphQLResultHandler<Operation.Data>) {
+  public func load<Operation: GraphQLOperation>(
+    _ operation: Operation,
+    callbackQueue: DispatchQueue? = nil,
+    resultHandler: @escaping GraphQLResultHandler<Operation.Data>
+  ) {
     withinReadTransaction({ transaction in
       let (data, dependentKeys) = try transaction.readObject(
         ofType: Operation.Data.self,
@@ -185,13 +193,13 @@ public class ApolloStore {
 
     fileprivate lazy var loader: DataLoader<CacheKey, Record> = 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)
+      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 init(store: ApolloStore) {
       self.cache = store.cache
@@ -235,7 +243,7 @@ public class ApolloStore {
       )
     }
 
-    private final func loadObject(forKey key: CacheKey) -> PossiblyDeferred<JSONObject> {
+    fileprivate final func loadObject(forKey key: CacheKey) -> PossiblyDeferred<JSONObject> {
       self.loader[key].map { record in
         guard let record = record else { throw JSONDecodingError.missingValue }
         return record.fields
@@ -270,9 +278,16 @@ public class ApolloStore {
       variables: GraphQLOperation.Variables? = nil,
       _ body: (inout SelectionSet) throws -> Void
     ) throws {
-      var object = try readObject(ofType: type,
-                                  withKey: key,
-                                  variables: variables)
+      var object = try readObject(
+        ofType: type,
+        withKey: key,
+        variables: variables,
+        accumulator: GraphQLSelectionSetMapper<SelectionSet>(
+          stripNullValues: false,
+          allowMissingValuesForOptionalFields: true
+        )
+      )
+
       try body(&object)
       try write(selectionSet: object, withKey: key, variables: variables)
     }

Sources/Apollo/GraphQLDependencyTracker.swift

@@ -13,6 +13,10 @@ final class GraphQLDependencyTracker: GraphQLResultAccumulator {
     dependentKeys.insert(info.cachePath.joined)
   }
 
+  func acceptMissingValue(info: FieldExecutionInfo) throws -> () {
+    dependentKeys.insert(info.cachePath.joined)
+  }
+
   func accept(list: [Void], info: FieldExecutionInfo) {
     dependentKeys.insert(info.cachePath.joined)
   }

Sources/Apollo/GraphQLExecutor.swift

@@ -308,10 +308,7 @@ final class GraphQLExecutor {
     }
 
     return PossiblyDeferred {
-      guard let value = fieldResolver(object, fieldInfo) else {
-        throw JSONDecodingError.missingValue
-      }
-      return value
+      fieldResolver(object, fieldInfo)
     }.flatMap {
       return self.complete(fields: fieldInfo,
                            withValue: $0,
@@ -329,7 +326,7 @@ final class GraphQLExecutor {
 
   private func complete<Accumulator: GraphQLResultAccumulator>(
     fields fieldInfo: FieldExecutionInfo,
-    withValue value: JSONValue,
+    withValue value: JSONValue?,
     accumulator: Accumulator
   ) -> PossiblyDeferred<Accumulator.PartialResult> {
     complete(fields: fieldInfo,
@@ -343,10 +340,14 @@ final class GraphQLExecutor {
   /// continues recursively.
   private func complete<Accumulator: GraphQLResultAccumulator>(
     fields fieldInfo: FieldExecutionInfo,
-    withValue value: JSONValue,
+    withValue value: JSONValue?,
     asType returnType: Selection.Field.OutputType,
     accumulator: Accumulator
   ) -> PossiblyDeferred<Accumulator.PartialResult> {
+    guard let value else {
+      return PossiblyDeferred { try accumulator.acceptMissingValue(info: fieldInfo) }
+    }
+
     if value is NSNull && returnType.isNullable {
       return PossiblyDeferred { try accumulator.acceptNullValue(info: fieldInfo) }
     }

Sources/Apollo/GraphQLResultAccumulator.swift

@@ -10,6 +10,7 @@ protocol GraphQLResultAccumulator: AnyObject {
 
   func accept(scalar: JSONValue, info: FieldExecutionInfo) throws -> PartialResult
   func acceptNullValue(info: FieldExecutionInfo) throws -> PartialResult
+  func acceptMissingValue(info: FieldExecutionInfo) throws -> PartialResult
   func accept(list: [PartialResult], info: FieldExecutionInfo) throws -> PartialResult
   func accept(childObject: ObjectResult, info: FieldExecutionInfo) throws -> PartialResult
 
@@ -51,6 +52,11 @@ final class Zip2Accumulator<Accumulator1: GraphQLResultAccumulator, Accumulator2
             try accumulator2.acceptNullValue(info: info))
   }
 
+  func acceptMissingValue(info: FieldExecutionInfo) throws -> PartialResult {
+    return (try accumulator1.acceptMissingValue(info: info),
+            try accumulator2.acceptMissingValue(info: info))
+  }
+
   func accept(list: [PartialResult], info: FieldExecutionInfo) throws -> PartialResult {
     let (list1, list2) = unzip(list)
     return (try accumulator1.accept(list: list1, info: info),
@@ -110,6 +116,12 @@ final class Zip3Accumulator<Accumulator1: GraphQLResultAccumulator, Accumulator2
             try accumulator3.acceptNullValue(info: info))
   }
 
+  func acceptMissingValue(info: FieldExecutionInfo) throws -> PartialResult {
+    return (try accumulator1.acceptMissingValue(info: info),
+            try accumulator2.acceptMissingValue(info: info),
+            try accumulator3.acceptMissingValue(info: info))
+  }
+
   func accept(list: [PartialResult], info: FieldExecutionInfo) throws -> PartialResult {
     let (list1, list2, list3) = unzip(list)
     return (try accumulator1.accept(list: list1, info: info),

Sources/Apollo/GraphQLResultNormalizer.swift

@@ -6,23 +6,28 @@ import ApolloAPI
 final class GraphQLResultNormalizer: GraphQLResultAccumulator {
   private var records: RecordSet = [:]
 
-  func accept(scalar: JSONValue, info: FieldExecutionInfo) -> JSONValue {
+  func accept(scalar: JSONValue, info: FieldExecutionInfo) -> JSONValue? {
     return scalar
   }
 
-  func acceptNullValue(info: FieldExecutionInfo) -> JSONValue {
+  func acceptNullValue(info: FieldExecutionInfo) -> JSONValue? {
     return NSNull()
   }
 
-  func accept(list: [JSONValue], info: FieldExecutionInfo) -> JSONValue {
+  func acceptMissingValue(info: FieldExecutionInfo) -> JSONValue? {
+    return nil
+  }
+
+  func accept(list: [JSONValue?], info: FieldExecutionInfo) -> JSONValue? {
     return list
   }
 
-  func accept(childObject: CacheReference, info: FieldExecutionInfo) throws -> JSONValue {
+  func accept(childObject: CacheReference, info: FieldExecutionInfo) -> JSONValue? {
     return childObject
   }
 
-  func accept(fieldEntry: JSONValue, info: FieldExecutionInfo) -> (key: String, value: JSONValue)? {
+  func accept(fieldEntry: JSONValue?, info: FieldExecutionInfo) -> (key: String, value: JSONValue)? {
+    guard let fieldEntry else { return nil }
     return (info.cacheKeyForField, fieldEntry)
   }
 

Sources/Apollo/GraphQLSelectionSetMapper.swift

@@ -2,8 +2,22 @@
 import ApolloAPI
 #endif
 
+import Foundation
+
 /// An accumulator that converts executed data to the correct values to create a `SelectionSet`.
 final class GraphQLSelectionSetMapper<SelectionSet: AnySelectionSet>: GraphQLResultAccumulator {
+
+  let stripNullValues: Bool
+  let allowMissingValuesForOptionalFields: Bool
+
+  init(
+    stripNullValues: Bool = true,
+    allowMissingValuesForOptionalFields: Bool = false
+  ) {
+    self.stripNullValues = stripNullValues
+    self.allowMissingValuesForOptionalFields = allowMissingValuesForOptionalFields
+  }
+
   func accept(scalar: JSONValue, info: FieldExecutionInfo) throws -> JSONValue? {
     switch info.field.type.namedType {
     case let .scalar(decodable as any JSONDecodable.Type),
@@ -17,6 +31,13 @@ final class GraphQLSelectionSetMapper<SelectionSet: AnySelectionSet>: GraphQLRes
   }
 
   func acceptNullValue(info: FieldExecutionInfo) -> JSONValue? {
+    return stripNullValues ? nil : NSNull()
+  }
+
+  func acceptMissingValue(info: FieldExecutionInfo) throws -> JSONValue? {
+    guard allowMissingValuesForOptionalFields && info.field.type.isNullable else {
+      throw JSONDecodingError.missingValue
+    }
     return nil
   }
 

Sources/Apollo/PossiblyDeferred.swift

@@ -69,7 +69,7 @@ enum PossiblyDeferred<Value> {
     }
   }
 
-  /// Returns a new possibly deferred  result, mapping any success value using the given
+  /// Returns a new possibly deferred result, mapping any success value using the given
   /// transformation and unwrapping the produced result.
   ///
   /// Use this method to avoid a nested result when your transformation