From 01945b7fc122a08a7ba8d596ff984af30fbc78db Mon Sep 17 00:00:00 2001 From: "Kamin, Grant" Date: Mon, 3 Feb 2020 11:36:07 -0600 Subject: [PATCH 1/5] Fix SQLite Cache Key Bug If your query has "." in it this would create an undefined cache key because it would just drop the query after the ".". For all "_ROOT" queries we just want the first part anyways so just return that. --- Sources/ApolloSQLite/SQLiteNormalizedCache.swift | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Sources/ApolloSQLite/SQLiteNormalizedCache.swift b/Sources/ApolloSQLite/SQLiteNormalizedCache.swift index 3f819152f7..56deb40f42 100644 --- a/Sources/ApolloSQLite/SQLiteNormalizedCache.swift +++ b/Sources/ApolloSQLite/SQLiteNormalizedCache.swift @@ -34,6 +34,9 @@ public final class SQLiteNormalizedCache { private func recordCacheKey(forFieldCacheKey fieldCacheKey: CacheKey) -> CacheKey { var components = fieldCacheKey.components(separatedBy: ".") + if fieldCacheKey.contains("_ROOT"), let rootKey = components.first { + return rootKey + } if components.count > 1 { components.removeLast() } From 2f23f66ee995037b46e80173f70af678cc9df81a Mon Sep 17 00:00:00 2001 From: "Kamin, Grant" Date: Mon, 3 Feb 2020 15:16:26 -0600 Subject: [PATCH 2/5] Update SQL Cache Key Logic Check if the separator is being used in a float and if it is don't treat it as a separator. --- .../ApolloSQLite/SQLiteNormalizedCache.swift | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/Sources/ApolloSQLite/SQLiteNormalizedCache.swift b/Sources/ApolloSQLite/SQLiteNormalizedCache.swift index 56deb40f42..c05302454e 100644 --- a/Sources/ApolloSQLite/SQLiteNormalizedCache.swift +++ b/Sources/ApolloSQLite/SQLiteNormalizedCache.swift @@ -33,14 +33,19 @@ public final class SQLiteNormalizedCache { } private func recordCacheKey(forFieldCacheKey fieldCacheKey: CacheKey) -> CacheKey { - var components = fieldCacheKey.components(separatedBy: ".") - if fieldCacheKey.contains("_ROOT"), let rootKey = components.first { - return rootKey + let components = fieldCacheKey.components(separatedBy: ".") + var updatedComponents = [String]() + for component in components { + if updatedComponents.last?.last?.isNumber ?? false && component.first?.isNumber ?? false { + updatedComponents[updatedComponents.count - 1].append(".\(component)") + } else { + updatedComponents.append(component) + } } - if components.count > 1 { - components.removeLast() + if updatedComponents.count > 1 { + updatedComponents.removeLast() } - return components.joined(separator: ".") + return updatedComponents.joined(separator: ".") } private func createTableIfNeeded() throws { From 4962c27a5d46dd203c9fb57ca6507502c5b79e95 Mon Sep 17 00:00:00 2001 From: "Kamin, Grant" Date: Mon, 3 Feb 2020 15:16:55 -0600 Subject: [PATCH 3/5] Add Float SQL Test Add test with query for starship length to test SQL cache keys with floats. --- .../LoadQueryFromStoreTests.swift | 39 +++++- Tests/StarWarsAPI/API.swift | 115 ++++++++++++++++++ Tests/StarWarsAPI/Starship.graphql | 8 ++ Tests/StarWarsAPI/operationIdsPath.json | 4 + Tests/StarWarsAPI/schema.json | 29 ++++- 5 files changed, 193 insertions(+), 2 deletions(-) diff --git a/Tests/ApolloCacheDependentTests/LoadQueryFromStoreTests.swift b/Tests/ApolloCacheDependentTests/LoadQueryFromStoreTests.swift index b49b8b6c51..47f14b1118 100644 --- a/Tests/ApolloCacheDependentTests/LoadQueryFromStoreTests.swift +++ b/Tests/ApolloCacheDependentTests/LoadQueryFromStoreTests.swift @@ -319,7 +319,44 @@ class LoadQueryFromStoreTests: XCTestCase { } } } - + + + func testLoadingQueryWithFloats() throws { + let starshipLength = 1234.5 + + let initialRecords: RecordSet = [ + "QUERY_ROOT": ["starshipLength(length:1234.5)": Reference(key: "starshipLength(length:1234.5)")], + "starshipLength(length:1234.5)": ["__typename": "Starship", + "name": "Millennium Falcon", + "length": starshipLength, + "coordinates": [[0.0,1.0], [2.0,3.0]]] + ] + + + withCache(initialRecords: initialRecords) { (cache) in + store = ApolloStore(cache: cache) + + let query = StarshipLengthQuery(length: starshipLength) + + load(query: query) { result in + switch result { + case .success(let graphQLResult): + XCTAssertNil(graphQLResult.errors) + + guard let data = graphQLResult.data else { + XCTFail("No data returned with result") + return + } + + XCTAssertEqual(data.starshipLength?.name, "Millennium Falcon") + XCTAssertEqual(data.starshipLength?.length, starshipLength) + case .failure(let error): + XCTFail("Unexpected error: \(error)") + } + } + } + } + // MARK: - Helpers private func load(query: Query, resultHandler: @escaping GraphQLResultHandler) { diff --git a/Tests/StarWarsAPI/API.swift b/Tests/StarWarsAPI/API.swift index 432b9e9829..0f536aeba2 100644 --- a/Tests/StarWarsAPI/API.swift +++ b/Tests/StarWarsAPI/API.swift @@ -5548,6 +5548,121 @@ public final class StarshipQuery: GraphQLQuery { } } +public final class StarshipLengthQuery: GraphQLQuery { + /// The raw GraphQL definition of this operation. + public let operationDefinition = + """ + query StarshipLength($length: Float!) { + starshipLength(length: $length) { + __typename + name + coordinates + length + } + } + """ + + public let operationName = "StarshipLength" + + public let operationIdentifier: String? = "f890b11aaa5d41e6d53ced4549beb27b9157c8c301d47e076909262ed9edbfb1" + + public var length: Double + + public init(length: Double) { + self.length = length + } + + public var variables: GraphQLMap? { + return ["length": length] + } + + public struct Data: GraphQLSelectionSet { + public static let possibleTypes = ["Query"] + + public static let selections: [GraphQLSelection] = [ + GraphQLField("starshipLength", arguments: ["length": GraphQLVariable("length")], type: .object(StarshipLength.selections)), + ] + + public private(set) var resultMap: ResultMap + + public init(unsafeResultMap: ResultMap) { + self.resultMap = unsafeResultMap + } + + public init(starshipLength: StarshipLength? = nil) { + self.init(unsafeResultMap: ["__typename": "Query", "starshipLength": starshipLength.flatMap { (value: StarshipLength) -> ResultMap in value.resultMap }]) + } + + public var starshipLength: StarshipLength? { + get { + return (resultMap["starshipLength"] as? ResultMap).flatMap { StarshipLength(unsafeResultMap: $0) } + } + set { + resultMap.updateValue(newValue?.resultMap, forKey: "starshipLength") + } + } + + public struct StarshipLength: GraphQLSelectionSet { + public static let possibleTypes = ["Starship"] + + public static let selections: [GraphQLSelection] = [ + GraphQLField("__typename", type: .nonNull(.scalar(String.self))), + GraphQLField("name", type: .nonNull(.scalar(String.self))), + GraphQLField("coordinates", type: .list(.nonNull(.list(.nonNull(.scalar(Double.self)))))), + GraphQLField("length", type: .scalar(Double.self)), + ] + + public private(set) var resultMap: ResultMap + + public init(unsafeResultMap: ResultMap) { + self.resultMap = unsafeResultMap + } + + public init(name: String, coordinates: [[Double]]? = nil, length: Double? = nil) { + self.init(unsafeResultMap: ["__typename": "Starship", "name": name, "coordinates": coordinates, "length": length]) + } + + public var __typename: String { + get { + return resultMap["__typename"]! as! String + } + set { + resultMap.updateValue(newValue, forKey: "__typename") + } + } + + /// The name of the starship + public var name: String { + get { + return resultMap["name"]! as! String + } + set { + resultMap.updateValue(newValue, forKey: "name") + } + } + + public var coordinates: [[Double]]? { + get { + return resultMap["coordinates"] as? [[Double]] + } + set { + resultMap.updateValue(newValue, forKey: "coordinates") + } + } + + /// Length of the starship, along the longest axis + public var length: Double? { + get { + return resultMap["length"] as? Double + } + set { + resultMap.updateValue(newValue, forKey: "length") + } + } + } + } +} + public final class ReviewAddedSubscription: GraphQLSubscription { /// The raw GraphQL definition of this operation. public let operationDefinition = diff --git a/Tests/StarWarsAPI/Starship.graphql b/Tests/StarWarsAPI/Starship.graphql index bb84f13c79..3a64704c3e 100644 --- a/Tests/StarWarsAPI/Starship.graphql +++ b/Tests/StarWarsAPI/Starship.graphql @@ -4,3 +4,11 @@ query Starship { coordinates } } + +query StarshipLength($length: Float!) { + starshipLength(length: $length) { + name + coordinates + length + } +} diff --git a/Tests/StarWarsAPI/operationIdsPath.json b/Tests/StarWarsAPI/operationIdsPath.json index 2a9c3bf0bd..5965b65c3a 100644 --- a/Tests/StarWarsAPI/operationIdsPath.json +++ b/Tests/StarWarsAPI/operationIdsPath.json @@ -127,6 +127,10 @@ "name": "Starship", "source": "query Starship {\n starship(id: 3000) {\n __typename\n name\n coordinates\n }\n}" }, + "f890b11aaa5d41e6d53ced4549beb27b9157c8c301d47e076909262ed9edbfb1": { + "name": "StarshipLength", + "source": "query StarshipLength($length: Float!) {\n starshipLength(length: $length) {\n __typename\n name\n coordinates\n length\n }\n}" + }, "38644c5e7cf4fd506b91d2e7010cabf84e63dfcd33cf1deb443b4b32b55e2cbe": { "name": "ReviewAdded", "source": "subscription ReviewAdded($episode: Episode) {\n reviewAdded(episode: $episode) {\n __typename\n episode\n stars\n commentary\n }\n}" diff --git a/Tests/StarWarsAPI/schema.json b/Tests/StarWarsAPI/schema.json index 11f02ed009..d78c60b929 100644 --- a/Tests/StarWarsAPI/schema.json +++ b/Tests/StarWarsAPI/schema.json @@ -204,6 +204,33 @@ }, "isDeprecated": false, "deprecationReason": null + }, + { + "name": "starshipLength", + "description": "", + "args": [ + { + "name": "length", + "description": "", + "type": { + "kind": "NON_NULL", + "name": null, + "ofType": { + "kind": "SCALAR", + "name": "Float", + "ofType": null + } + }, + "defaultValue": null + } + ], + "type": { + "kind": "OBJECT", + "name": "Starship", + "ofType": null + }, + "isDeprecated": false, + "deprecationReason": null } ], "inputFields": null, @@ -2184,4 +2211,4 @@ ] } } -} \ No newline at end of file +} From d896fb36d67ff0ab910beee59c0329ea45412190 Mon Sep 17 00:00:00 2001 From: "Kamin, Grant" Date: Tue, 4 Feb 2020 07:53:22 -0600 Subject: [PATCH 4/5] Address Comments --- .../ApolloSQLite/SQLiteNormalizedCache.swift | 19 ++++++++----------- .../LoadQueryFromStoreTests.swift | 1 + 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/Sources/ApolloSQLite/SQLiteNormalizedCache.swift b/Sources/ApolloSQLite/SQLiteNormalizedCache.swift index c05302454e..14180115ec 100644 --- a/Sources/ApolloSQLite/SQLiteNormalizedCache.swift +++ b/Sources/ApolloSQLite/SQLiteNormalizedCache.swift @@ -33,19 +33,16 @@ public final class SQLiteNormalizedCache { } private func recordCacheKey(forFieldCacheKey fieldCacheKey: CacheKey) -> CacheKey { - let components = fieldCacheKey.components(separatedBy: ".") - var updatedComponents = [String]() - for component in components { - if updatedComponents.last?.last?.isNumber ?? false && component.first?.isNumber ?? false { - updatedComponents[updatedComponents.count - 1].append(".\(component)") - } else { - updatedComponents.append(component) - } + var components = fieldCacheKey.components(separatedBy: ".") + if components.count > 1, let lastComponent = components.last, + lastComponent.first?.isNumber ?? false && components[components.count - 2].last?.isNumber ?? false { + components[components.count - 2].append(".\(lastComponent)") + components.removeLast() } - if updatedComponents.count > 1 { - updatedComponents.removeLast() + if components.count > 1 { + components.removeLast() } - return updatedComponents.joined(separator: ".") + return components.joined(separator: ".") } private func createTableIfNeeded() throws { diff --git a/Tests/ApolloCacheDependentTests/LoadQueryFromStoreTests.swift b/Tests/ApolloCacheDependentTests/LoadQueryFromStoreTests.swift index 47f14b1118..bd2be9d426 100644 --- a/Tests/ApolloCacheDependentTests/LoadQueryFromStoreTests.swift +++ b/Tests/ApolloCacheDependentTests/LoadQueryFromStoreTests.swift @@ -350,6 +350,7 @@ class LoadQueryFromStoreTests: XCTestCase { XCTAssertEqual(data.starshipLength?.name, "Millennium Falcon") XCTAssertEqual(data.starshipLength?.length, starshipLength) + XCTAssertEqual(data.starshipLength?.coordinates, [[0.0,1.0], [2.0,3.0]]) case .failure(let error): XCTFail("Unexpected error: \(error)") } From 5c8d9a2be43f57983f48f179478dce39c3b8cb08 Mon Sep 17 00:00:00 2001 From: "Kamin, Grant" Date: Mon, 10 Feb 2020 15:11:41 -0600 Subject: [PATCH 5/5] Update With More Complicated Test Case Updated loadingQueryWithFloats test with more complicated case that matches the original issue I was seeing. --- .../ApolloSQLite/SQLiteNormalizedCache.swift | 24 +++++++++----- .../LoadQueryFromStoreTests.swift | 20 ++++++------ Tests/StarWarsAPI/API.swift | 32 +++++++++---------- Tests/StarWarsAPI/Starship.graphql | 4 +-- Tests/StarWarsAPI/operationIdsPath.json | 6 ++-- Tests/StarWarsAPI/schema.json | 24 ++++++++++---- 6 files changed, 65 insertions(+), 45 deletions(-) diff --git a/Sources/ApolloSQLite/SQLiteNormalizedCache.swift b/Sources/ApolloSQLite/SQLiteNormalizedCache.swift index 14180115ec..1474b86578 100644 --- a/Sources/ApolloSQLite/SQLiteNormalizedCache.swift +++ b/Sources/ApolloSQLite/SQLiteNormalizedCache.swift @@ -33,16 +33,24 @@ public final class SQLiteNormalizedCache { } private func recordCacheKey(forFieldCacheKey fieldCacheKey: CacheKey) -> CacheKey { - var components = fieldCacheKey.components(separatedBy: ".") - if components.count > 1, let lastComponent = components.last, - lastComponent.first?.isNumber ?? false && components[components.count - 2].last?.isNumber ?? false { - components[components.count - 2].append(".\(lastComponent)") - components.removeLast() + let components = fieldCacheKey.components(separatedBy: ".") + var updatedComponents = [String]() + if components.first?.contains("_ROOT") == true { + for component in components { + if updatedComponents.last?.last?.isNumber ?? false && component.first?.isNumber ?? false { + updatedComponents[updatedComponents.count - 1].append(".\(component)") + } else { + updatedComponents.append(component) + } + } + } else { + updatedComponents = components } - if components.count > 1 { - components.removeLast() + + if updatedComponents.count > 1 { + updatedComponents.removeLast() } - return components.joined(separator: ".") + return updatedComponents.joined(separator: ".") } private func createTableIfNeeded() throws { diff --git a/Tests/ApolloCacheDependentTests/LoadQueryFromStoreTests.swift b/Tests/ApolloCacheDependentTests/LoadQueryFromStoreTests.swift index bd2be9d426..e6860233cf 100644 --- a/Tests/ApolloCacheDependentTests/LoadQueryFromStoreTests.swift +++ b/Tests/ApolloCacheDependentTests/LoadQueryFromStoreTests.swift @@ -323,20 +323,20 @@ class LoadQueryFromStoreTests: XCTestCase { func testLoadingQueryWithFloats() throws { let starshipLength = 1234.5 + let coordinates = [[38.857150, -94.798464]] let initialRecords: RecordSet = [ - "QUERY_ROOT": ["starshipLength(length:1234.5)": Reference(key: "starshipLength(length:1234.5)")], - "starshipLength(length:1234.5)": ["__typename": "Starship", - "name": "Millennium Falcon", - "length": starshipLength, - "coordinates": [[0.0,1.0], [2.0,3.0]]] + "QUERY_ROOT": ["starshipCoordinates(coordinates:\(coordinates))": Reference(key: "starshipCoordinates(coordinates:\(coordinates))")], + "starshipCoordinates(coordinates:\(coordinates))": ["__typename": "Starship", + "name": "Millennium Falcon", + "length": starshipLength, + "coordinates": coordinates] ] - withCache(initialRecords: initialRecords) { (cache) in store = ApolloStore(cache: cache) - let query = StarshipLengthQuery(length: starshipLength) + let query = StarshipCoordinatesQuery(coordinates: coordinates) load(query: query) { result in switch result { @@ -348,9 +348,9 @@ class LoadQueryFromStoreTests: XCTestCase { return } - XCTAssertEqual(data.starshipLength?.name, "Millennium Falcon") - XCTAssertEqual(data.starshipLength?.length, starshipLength) - XCTAssertEqual(data.starshipLength?.coordinates, [[0.0,1.0], [2.0,3.0]]) + XCTAssertEqual(data.starshipCoordinates?.name, "Millennium Falcon") + XCTAssertEqual(data.starshipCoordinates?.length, starshipLength) + XCTAssertEqual(data.starshipCoordinates?.coordinates, coordinates) case .failure(let error): XCTFail("Unexpected error: \(error)") } diff --git a/Tests/StarWarsAPI/API.swift b/Tests/StarWarsAPI/API.swift index 0f536aeba2..908ca35641 100644 --- a/Tests/StarWarsAPI/API.swift +++ b/Tests/StarWarsAPI/API.swift @@ -5548,12 +5548,12 @@ public final class StarshipQuery: GraphQLQuery { } } -public final class StarshipLengthQuery: GraphQLQuery { +public final class StarshipCoordinatesQuery: GraphQLQuery { /// The raw GraphQL definition of this operation. public let operationDefinition = """ - query StarshipLength($length: Float!) { - starshipLength(length: $length) { + query StarshipCoordinates($coordinates: [[Float!]!]) { + starshipCoordinates(coordinates: $coordinates) { __typename name coordinates @@ -5562,25 +5562,25 @@ public final class StarshipLengthQuery: GraphQLQuery { } """ - public let operationName = "StarshipLength" + public let operationName = "StarshipCoordinates" - public let operationIdentifier: String? = "f890b11aaa5d41e6d53ced4549beb27b9157c8c301d47e076909262ed9edbfb1" + public let operationIdentifier: String? = "8dd77d4bc7494c184606da092a665a7c2ca3c2a3f14d3b23fa5e469e207b3406" - public var length: Double + public var coordinates: [[Double]]? - public init(length: Double) { - self.length = length + public init(coordinates: [[Double]]?) { + self.coordinates = coordinates } public var variables: GraphQLMap? { - return ["length": length] + return ["coordinates": coordinates] } public struct Data: GraphQLSelectionSet { public static let possibleTypes = ["Query"] public static let selections: [GraphQLSelection] = [ - GraphQLField("starshipLength", arguments: ["length": GraphQLVariable("length")], type: .object(StarshipLength.selections)), + GraphQLField("starshipCoordinates", arguments: ["coordinates": GraphQLVariable("coordinates")], type: .object(StarshipCoordinate.selections)), ] public private(set) var resultMap: ResultMap @@ -5589,20 +5589,20 @@ public final class StarshipLengthQuery: GraphQLQuery { self.resultMap = unsafeResultMap } - public init(starshipLength: StarshipLength? = nil) { - self.init(unsafeResultMap: ["__typename": "Query", "starshipLength": starshipLength.flatMap { (value: StarshipLength) -> ResultMap in value.resultMap }]) + public init(starshipCoordinates: StarshipCoordinate? = nil) { + self.init(unsafeResultMap: ["__typename": "Query", "starshipCoordinates": starshipCoordinates.flatMap { (value: StarshipCoordinate) -> ResultMap in value.resultMap }]) } - public var starshipLength: StarshipLength? { + public var starshipCoordinates: StarshipCoordinate? { get { - return (resultMap["starshipLength"] as? ResultMap).flatMap { StarshipLength(unsafeResultMap: $0) } + return (resultMap["starshipCoordinates"] as? ResultMap).flatMap { StarshipCoordinate(unsafeResultMap: $0) } } set { - resultMap.updateValue(newValue?.resultMap, forKey: "starshipLength") + resultMap.updateValue(newValue?.resultMap, forKey: "starshipCoordinates") } } - public struct StarshipLength: GraphQLSelectionSet { + public struct StarshipCoordinate: GraphQLSelectionSet { public static let possibleTypes = ["Starship"] public static let selections: [GraphQLSelection] = [ diff --git a/Tests/StarWarsAPI/Starship.graphql b/Tests/StarWarsAPI/Starship.graphql index 3a64704c3e..1bfef3a16b 100644 --- a/Tests/StarWarsAPI/Starship.graphql +++ b/Tests/StarWarsAPI/Starship.graphql @@ -5,8 +5,8 @@ query Starship { } } -query StarshipLength($length: Float!) { - starshipLength(length: $length) { +query StarshipCoordinates($coordinates: [[Float!]!]) { + starshipCoordinates(coordinates: $coordinates) { name coordinates length diff --git a/Tests/StarWarsAPI/operationIdsPath.json b/Tests/StarWarsAPI/operationIdsPath.json index 5965b65c3a..a94b69687b 100644 --- a/Tests/StarWarsAPI/operationIdsPath.json +++ b/Tests/StarWarsAPI/operationIdsPath.json @@ -127,9 +127,9 @@ "name": "Starship", "source": "query Starship {\n starship(id: 3000) {\n __typename\n name\n coordinates\n }\n}" }, - "f890b11aaa5d41e6d53ced4549beb27b9157c8c301d47e076909262ed9edbfb1": { - "name": "StarshipLength", - "source": "query StarshipLength($length: Float!) {\n starshipLength(length: $length) {\n __typename\n name\n coordinates\n length\n }\n}" + "8dd77d4bc7494c184606da092a665a7c2ca3c2a3f14d3b23fa5e469e207b3406": { + "name": "StarshipCoordinates", + "source": "query StarshipCoordinates($coordinates: [[Float!]!]) {\n starshipCoordinates(coordinates: $coordinates) {\n __typename\n name\n coordinates\n length\n }\n}" }, "38644c5e7cf4fd506b91d2e7010cabf84e63dfcd33cf1deb443b4b32b55e2cbe": { "name": "ReviewAdded", diff --git a/Tests/StarWarsAPI/schema.json b/Tests/StarWarsAPI/schema.json index d78c60b929..b9a35aa09d 100644 --- a/Tests/StarWarsAPI/schema.json +++ b/Tests/StarWarsAPI/schema.json @@ -206,19 +206,31 @@ "deprecationReason": null }, { - "name": "starshipLength", + "name": "starshipCoordinates", "description": "", "args": [ { - "name": "length", + "name": "coordinates", "description": "", "type": { - "kind": "NON_NULL", + "kind": "LIST", "name": null, "ofType": { - "kind": "SCALAR", - "name": "Float", - "ofType": null + "kind": "NON_NULL", + "name": null, + "ofType": { + "kind": "LIST", + "name": null, + "ofType": { + "kind": "NON_NULL", + "name": null, + "ofType": { + "kind": "SCALAR", + "name": "Float", + "ofType": null + } + } + } } }, "defaultValue": null