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 SQLite Cache Key Bug #991

Merged
merged 5 commits into from
Feb 10, 2020
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
21 changes: 17 additions & 4 deletions Sources/ApolloSQLite/SQLiteNormalizedCache.swift
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,24 @@ public final class SQLiteNormalizedCache {
}

private func recordCacheKey(forFieldCacheKey fieldCacheKey: CacheKey) -> CacheKey {
var components = fieldCacheKey.components(separatedBy: ".")
if components.count > 1 {
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
}
giantramen marked this conversation as resolved.
Show resolved Hide resolved

if updatedComponents.count > 1 {
updatedComponents.removeLast()
}
return components.joined(separator: ".")
return updatedComponents.joined(separator: ".")
}

private func createTableIfNeeded() throws {
Expand Down
40 changes: 39 additions & 1 deletion Tests/ApolloCacheDependentTests/LoadQueryFromStoreTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,45 @@ class LoadQueryFromStoreTests: XCTestCase {
}
}
}



func testLoadingQueryWithFloats() throws {
let starshipLength = 1234.5
let coordinates = [[38.857150, -94.798464]]

let initialRecords: RecordSet = [
"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 = StarshipCoordinatesQuery(coordinates: coordinates)

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
}
Copy link
Contributor

Choose a reason for hiding this comment

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

You can use let data = try XCTUnwrap(graphQLResult.data) here and not have to deal with writing your own failure mechanism

Copy link
Author

Choose a reason for hiding this comment

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

Maybe I'm missing something but I don't think I can use that in the completion block.

Copy link
Contributor

Choose a reason for hiding this comment

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

I haven't had an issue with that in other tests

Copy link
Author

Choose a reason for hiding this comment

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

It's not used in completion blocks anywhere else, it seems like this test file uses the completion block with XCTFail pattern for all the tests.

Copy link
Contributor

Choose a reason for hiding this comment

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

no worries i'll fight with this once this is merged


XCTAssertEqual(data.starshipCoordinates?.name, "Millennium Falcon")
XCTAssertEqual(data.starshipCoordinates?.length, starshipLength)
XCTAssertEqual(data.starshipCoordinates?.coordinates, coordinates)
case .failure(let error):
XCTFail("Unexpected error: \(error)")
}
}
}
}

// MARK: - Helpers

private func load<Query: GraphQLQuery>(query: Query, resultHandler: @escaping GraphQLResultHandler<Query.Data>) {
Expand Down
115 changes: 115 additions & 0 deletions Tests/StarWarsAPI/API.swift
Original file line number Diff line number Diff line change
Expand Up @@ -5548,6 +5548,121 @@ public final class StarshipQuery: GraphQLQuery {
}
}

public final class StarshipCoordinatesQuery: GraphQLQuery {
/// The raw GraphQL definition of this operation.
public let operationDefinition =
"""
query StarshipCoordinates($coordinates: [[Float!]!]) {
starshipCoordinates(coordinates: $coordinates) {
__typename
name
coordinates
length
}
}
"""

public let operationName = "StarshipCoordinates"

public let operationIdentifier: String? = "8dd77d4bc7494c184606da092a665a7c2ca3c2a3f14d3b23fa5e469e207b3406"

public var coordinates: [[Double]]?

public init(coordinates: [[Double]]?) {
self.coordinates = coordinates
}

public var variables: GraphQLMap? {
return ["coordinates": coordinates]
}

public struct Data: GraphQLSelectionSet {
public static let possibleTypes = ["Query"]

public static let selections: [GraphQLSelection] = [
GraphQLField("starshipCoordinates", arguments: ["coordinates": GraphQLVariable("coordinates")], type: .object(StarshipCoordinate.selections)),
]

public private(set) var resultMap: ResultMap

public init(unsafeResultMap: ResultMap) {
self.resultMap = unsafeResultMap
}

public init(starshipCoordinates: StarshipCoordinate? = nil) {
self.init(unsafeResultMap: ["__typename": "Query", "starshipCoordinates": starshipCoordinates.flatMap { (value: StarshipCoordinate) -> ResultMap in value.resultMap }])
}

public var starshipCoordinates: StarshipCoordinate? {
get {
return (resultMap["starshipCoordinates"] as? ResultMap).flatMap { StarshipCoordinate(unsafeResultMap: $0) }
}
set {
resultMap.updateValue(newValue?.resultMap, forKey: "starshipCoordinates")
}
}

public struct StarshipCoordinate: 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 =
Expand Down
8 changes: 8 additions & 0 deletions Tests/StarWarsAPI/Starship.graphql
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,11 @@ query Starship {
coordinates
}
}

query StarshipCoordinates($coordinates: [[Float!]!]) {
starshipCoordinates(coordinates: $coordinates) {
name
coordinates
length
}
}
4 changes: 4 additions & 0 deletions Tests/StarWarsAPI/operationIdsPath.json
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,10 @@
"name": "Starship",
"source": "query Starship {\n starship(id: 3000) {\n __typename\n name\n coordinates\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",
"source": "subscription ReviewAdded($episode: Episode) {\n reviewAdded(episode: $episode) {\n __typename\n episode\n stars\n commentary\n }\n}"
Expand Down
41 changes: 40 additions & 1 deletion Tests/StarWarsAPI/schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,45 @@
},
"isDeprecated": false,
"deprecationReason": null
},
{
"name": "starshipCoordinates",
"description": "",
"args": [
{
"name": "coordinates",
"description": "",
"type": {
"kind": "LIST",
"name": null,
"ofType": {
"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
}
],
"type": {
"kind": "OBJECT",
"name": "Starship",
"ofType": null
},
"isDeprecated": false,
"deprecationReason": null
}
],
"inputFields": null,
Expand Down Expand Up @@ -2184,4 +2223,4 @@
]
}
}
}
}