Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix selection set value serialization when writing to cache #2778

Merged
merged 1 commit into from
Jan 14, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Sources/Apollo/ApolloStore.swift
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,7 @@ public class ApolloStore {
withKey key: CacheKey,
variables: GraphQLOperation.Variables? = nil
) throws {
let normalizer = GraphQLResultNormalizer()
let normalizer = ResultNormalizerFactory.selectionSetDataNormalizer()
let executor = GraphQLExecutor { object, info in
return object[info.responseKeyForField]
}
Expand Down
4 changes: 4 additions & 0 deletions Sources/Apollo/GraphQLDependencyTracker.swift
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@ final class GraphQLDependencyTracker: GraphQLResultAccumulator {
dependentKeys.insert(info.cachePath.joined)
}

func accept(customScalar: JSONValue, info: FieldExecutionInfo) {
dependentKeys.insert(info.cachePath.joined)
}

func acceptNullValue(info: FieldExecutionInfo) {
dependentKeys.insert(info.cachePath.joined)
}
Expand Down
5 changes: 4 additions & 1 deletion Sources/Apollo/GraphQLExecutor.swift
Original file line number Diff line number Diff line change
Expand Up @@ -362,9 +362,12 @@ final class GraphQLExecutor {
asType: innerType,
accumulator: accumulator)

case .scalar, .customScalar:
case .scalar:
return PossiblyDeferred { try accumulator.accept(scalar: value, info: fieldInfo) }

case .customScalar:
return PossiblyDeferred { try accumulator.accept(customScalar: value, info: fieldInfo) }

case .list(let innerType):
guard let array = value as? [JSONValue] else {
return .immediate(.failure(JSONDecodingError.wrongType))
Expand Down
2 changes: 1 addition & 1 deletion Sources/Apollo/GraphQLResponse.swift
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ public final class GraphQLResponse<Data: RootSelectionSet> {
public func parseResult() throws -> (GraphQLResult<Data>, RecordSet?) {
let accumulator = zip(
GraphQLSelectionSetMapper<Data>(),
GraphQLResultNormalizer(),
ResultNormalizerFactory.networkResponseDataNormalizer(),
GraphQLDependencyTracker()
)

Expand Down
12 changes: 12 additions & 0 deletions Sources/Apollo/GraphQLResultAccumulator.swift
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ protocol GraphQLResultAccumulator: AnyObject {
associatedtype FinalResult

func accept(scalar: JSONValue, info: FieldExecutionInfo) throws -> PartialResult
func accept(customScalar: 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
Expand Down Expand Up @@ -47,6 +48,11 @@ final class Zip2Accumulator<Accumulator1: GraphQLResultAccumulator, Accumulator2
try accumulator2.accept(scalar: scalar, info: info))
}

func accept(customScalar: JSONValue, info: FieldExecutionInfo) throws -> PartialResult {
return (try accumulator1.accept(customScalar: customScalar, info: info),
try accumulator2.accept(customScalar: customScalar, info: info))
}

func acceptNullValue(info: FieldExecutionInfo) throws -> PartialResult {
return (try accumulator1.acceptNullValue(info: info),
try accumulator2.acceptNullValue(info: info))
Expand Down Expand Up @@ -110,6 +116,12 @@ final class Zip3Accumulator<Accumulator1: GraphQLResultAccumulator, Accumulator2
try accumulator3.accept(scalar: scalar, info: info))
}

func accept(customScalar: JSONValue, info: FieldExecutionInfo) throws -> PartialResult {
return (try accumulator1.accept(customScalar: customScalar, info: info),
try accumulator2.accept(customScalar: customScalar, info: info),
try accumulator3.accept(customScalar: customScalar, info: info))
}

func acceptNullValue(info: FieldExecutionInfo) throws -> PartialResult {
return (try accumulator1.acceptNullValue(info: info),
try accumulator2.acceptNullValue(info: info),
Expand Down
49 changes: 39 additions & 10 deletions Sources/Apollo/GraphQLResultNormalizer.swift
Original file line number Diff line number Diff line change
Expand Up @@ -3,47 +3,76 @@ import Foundation
import ApolloAPI
#endif

final class GraphQLResultNormalizer: GraphQLResultAccumulator {
struct ResultNormalizerFactory {
private init() {}

static func selectionSetDataNormalizer() -> SelectionSetDataResultNormalizer {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I decided to create separate functions rather than normalizer(for: DataSource) because this way, each function returns the concrete type (instead of BaseGraphQLResultNormalizer). This allows the generic functions in the GraphQLExecutor to still specialize the function calls, using static dispatch instead of dynamic dispatch.

SelectionSetDataResultNormalizer()
}

static func networkResponseDataNormalizer() -> RawJSONResultNormalizer {
RawJSONResultNormalizer()
}
}

class BaseGraphQLResultNormalizer: GraphQLResultAccumulator {
private var records: RecordSet = [:]

func accept(scalar: JSONValue, info: FieldExecutionInfo) -> JSONValue? {
fileprivate init() {}

final func accept(scalar: JSONValue, info: FieldExecutionInfo) -> JSONValue? {
return scalar
}

func acceptNullValue(info: FieldExecutionInfo) -> JSONValue? {
func accept(customScalar: JSONValue, info: FieldExecutionInfo) -> JSONValue? {
return customScalar
}

final func acceptNullValue(info: FieldExecutionInfo) -> JSONValue? {
return NSNull()
}

func acceptMissingValue(info: FieldExecutionInfo) -> JSONValue? {
final func acceptMissingValue(info: FieldExecutionInfo) -> JSONValue? {
return nil
}

func accept(list: [JSONValue?], info: FieldExecutionInfo) -> JSONValue? {
final func accept(list: [JSONValue?], info: FieldExecutionInfo) -> JSONValue? {
return list
}

func accept(childObject: CacheReference, info: FieldExecutionInfo) -> JSONValue? {
final func accept(childObject: CacheReference, info: FieldExecutionInfo) -> JSONValue? {
return childObject
}

func accept(fieldEntry: JSONValue?, info: FieldExecutionInfo) -> (key: String, value: JSONValue)? {
final func accept(fieldEntry: JSONValue?, info: FieldExecutionInfo) -> (key: String, value: JSONValue)? {
guard let fieldEntry else { return nil }
return (info.cacheKeyForField, fieldEntry)
}

func accept(
final func accept(
fieldEntries: [(key: String, value: JSONValue)],
info: ObjectExecutionInfo
) throws -> CacheReference {
let cachePath = info.cachePath.joined

let object = JSONObject(fieldEntries, uniquingKeysWith: { (_, last) in last })
records.merge(record: Record(key: cachePath, object))

return CacheReference(cachePath)
}

func finish(rootValue: CacheReference, info: ObjectExecutionInfo) throws -> RecordSet {
final func finish(rootValue: CacheReference, info: ObjectExecutionInfo) throws -> RecordSet {
return records
}
}

final class RawJSONResultNormalizer: BaseGraphQLResultNormalizer {}

final class SelectionSetDataResultNormalizer: BaseGraphQLResultNormalizer {
override final func accept(customScalar: JSONValue, info: FieldExecutionInfo) -> JSONValue? {
if let customScalar = customScalar as? JSONEncodable {
return customScalar._jsonValue
}
return customScalar
}
}
14 changes: 12 additions & 2 deletions Sources/Apollo/GraphQLSelectionSetMapper.swift
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,7 @@ final class GraphQLSelectionSetMapper<SelectionSet: AnySelectionSet>: GraphQLRes

func accept(scalar: JSONValue, info: FieldExecutionInfo) throws -> JSONValue? {
switch info.field.type.namedType {
case let .scalar(decodable as any JSONDecodable.Type),
let .customScalar(decodable as any JSONDecodable.Type):
case let .scalar(decodable as any JSONDecodable.Type):
// This will convert a JSON value to the expected value type,
// which could be a custom scalar or an enum.
return try decodable.init(_jsonValue: scalar)._asAnyHashable
Expand All @@ -30,6 +29,17 @@ final class GraphQLSelectionSetMapper<SelectionSet: AnySelectionSet>: GraphQLRes
}
}

func accept(customScalar: JSONValue, info: FieldExecutionInfo) throws -> JSONValue? {
switch info.field.type.namedType {
case let .customScalar(decodable as any JSONDecodable.Type):
// This will convert a JSON value to the expected value type,
// which could be a custom scalar or an enum.
return try decodable.init(_jsonValue: customScalar)._asAnyHashable
default:
preconditionFailure()
}
}

func acceptNullValue(info: FieldExecutionInfo) -> JSONValue? {
return stripNullValues ? nil : NSNull()
}
Expand Down
71 changes: 70 additions & 1 deletion Tests/ApolloTests/Cache/ReadWriteFromStoreTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -484,7 +484,7 @@ class ReadWriteFromStoreTests: XCTestCase, CacheDependentTesting, StoreLoading {
data.hero.name = "Artoo"
}
}, completion: { result in
defer { updateCompletedExpectation.fulfill() }
defer { updateCompletedExpectation.fulfill() }
XCTAssertSuccessResult(result)
})

Expand Down Expand Up @@ -1294,6 +1294,75 @@ class ReadWriteFromStoreTests: XCTestCase, CacheDependentTesting, StoreLoading {
}
}

func test_updateCacheMutation_givenEnumField_enumFieldIsSerializedAndCanBeRead() throws {
// given
enum HeroType: String, EnumType {
case droid
}

struct GivenSelectionSet: MockMutableRootSelectionSet {
public var __data: DataDict = DataDict([:], variables: nil)
init(data: DataDict) { __data = data }

static var __selections: [Selection] { [
.field("hero", Hero.self)
]}

var hero: Hero {
get { __data["hero"] }
set { __data["hero"] = newValue }
}

struct Hero: MockMutableRootSelectionSet {
public var __data: DataDict = DataDict([:], variables: nil)
init(data: DataDict) { __data = data }

static var __selections: [Selection] { [
.field("type", GraphQLEnum<HeroType>.self)
]}

var type: GraphQLEnum<HeroType> {
get { __data["type"] }
set { __data["type"] = newValue }
}
}
}

let cacheMutation = MockLocalCacheMutation<GivenSelectionSet>()

mergeRecordsIntoCache([
"QUERY_ROOT": ["hero": CacheReference("QUERY_ROOT.hero")],
"QUERY_ROOT.hero": ["__typename": "Droid", "type": "droid"]
])

runActivity("update mutation") { _ in
let updateCompletedExpectation = expectation(description: "Update completed")

store.withinReadWriteTransaction({ transaction in
try transaction.update(cacheMutation) { data in
// noop
}
}, completion: { result in
defer { updateCompletedExpectation.fulfill() }
XCTAssertSuccessResult(result)
})

self.wait(for: [updateCompletedExpectation], timeout: Self.defaultWaitTimeout)
}

let query = MockQuery<GivenSelectionSet>()

loadFromStore(operation: query) { result in
try XCTAssertSuccessResult(result) { graphQLResult in
XCTAssertEqual(graphQLResult.source, .cache)
XCTAssertNil(graphQLResult.errors)

let data = try XCTUnwrap(graphQLResult.data)
XCTAssertEqual(data.hero.type, .case(.droid))
}
}
}

func test_writeDataForCacheMutation_givenMutationOperation_updateNestedField_updatesObjectAtMutationRoot() throws {
// given
struct GivenSelectionSet: MockMutableRootSelectionSet {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ class GraphQLExecutor_ResultNormalizer_FromResponse_Tests: XCTestCase {
on: object,
withRootCacheReference: CacheReference.RootQuery,
variables: variables,
accumulator: GraphQLResultNormalizer()
accumulator: ResultNormalizerFactory.networkResponseDataNormalizer()
)
}

Expand Down