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

Allow periods in arguments to be ignored when parsing cacheKeys #2057

Merged
merged 6 commits into from
Dec 15, 2021
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
4 changes: 4 additions & 0 deletions Apollo.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
/* Begin PBXBuildFile section */
19E9F6AC26D58A9A003AB80E /* OperationMessageIdCreatorTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 19E9F6AA26D58A92003AB80E /* OperationMessageIdCreatorTests.swift */; };
19E9F6B526D6BF25003AB80E /* OperationMessageIdCreator.swift in Sources */ = {isa = PBXBuildFile; fileRef = 19E9F6A826D5867E003AB80E /* OperationMessageIdCreator.swift */; };
2EE7FFD0276802E30035DC39 /* CacheKeyConstructionTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2EE7FFCF276802E30035DC39 /* CacheKeyConstructionTests.swift */; };
54DDB0921EA045870009DD99 /* InMemoryNormalizedCache.swift in Sources */ = {isa = PBXBuildFile; fileRef = 54DDB0911EA045870009DD99 /* InMemoryNormalizedCache.swift */; };
5AC6CA4322AAF7B200B7C94D /* GraphQLHTTPMethod.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5AC6CA4222AAF7B200B7C94D /* GraphQLHTTPMethod.swift */; };
5BB2C0232380836100774170 /* VersionNumberTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5BB2C0222380836100774170 /* VersionNumberTests.swift */; };
Expand Down Expand Up @@ -497,6 +498,7 @@
/* Begin PBXFileReference section */
19E9F6A826D5867E003AB80E /* OperationMessageIdCreator.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OperationMessageIdCreator.swift; sourceTree = "<group>"; };
19E9F6AA26D58A92003AB80E /* OperationMessageIdCreatorTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OperationMessageIdCreatorTests.swift; sourceTree = "<group>"; };
2EE7FFCF276802E30035DC39 /* CacheKeyConstructionTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CacheKeyConstructionTests.swift; sourceTree = "<group>"; };
54DDB0911EA045870009DD99 /* InMemoryNormalizedCache.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = InMemoryNormalizedCache.swift; sourceTree = "<group>"; };
5AC6CA4222AAF7B200B7C94D /* GraphQLHTTPMethod.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = GraphQLHTTPMethod.swift; sourceTree = "<group>"; };
5BB2C0222380836100774170 /* VersionNumberTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = VersionNumberTests.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -1733,6 +1735,7 @@
9F8622F71EC2004200C38162 /* ReadWriteFromStoreTests.swift */,
9FD03C2D25527CE6002227DC /* StoreConcurrencyTests.swift */,
9FA6ABCB1EC0A9F7000017BE /* WatchQueryTests.swift */,
2EE7FFCF276802E30035DC39 /* CacheKeyConstructionTests.swift */,
);
path = Cache;
sourceTree = "<group>";
Expand Down Expand Up @@ -2682,6 +2685,7 @@
DED45EC4261BA0ED0086EF63 /* SplitNetworkTransportTests.swift in Sources */,
9B21FD752422C29D00998B5C /* GraphQLFileTests.swift in Sources */,
E86D8E05214B32FD0028EFE1 /* JSONTests.swift in Sources */,
2EE7FFD0276802E30035DC39 /* CacheKeyConstructionTests.swift in Sources */,
9F8622FA1EC2117C00C38162 /* FragmentConstructionAndConversionTests.swift in Sources */,
DED45C2A2615319E0086EF63 /* DefaultInterceptorProviderTests.swift in Sources */,
9F21730E2567E6F000566121 /* DataLoaderTests.swift in Sources */,
Expand Down
43 changes: 42 additions & 1 deletion Sources/ApolloSQLite/SQLiteNormalizedCache.swift
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ public final class SQLiteNormalizedCache {
}

private func recordCacheKey(forFieldCacheKey fieldCacheKey: CacheKey) -> CacheKey {
let components = fieldCacheKey.components(separatedBy: ".")
let components = fieldCacheKey.splitIntoCacheKeyComponents()
var updatedComponents = [String]()
if components.first?.contains("_ROOT") == true {
for component in components {
Expand Down Expand Up @@ -117,3 +117,44 @@ extension SQLiteNormalizedCache: NormalizedCache {
try self.database.clearDatabase(shouldVacuumOnClear: self.shouldVacuumOnClear)
}
}

extension String {
private var isBalanced: Bool {
guard contains("(") || contains(")") else { return true }

var stack = [Character]()
for character in self where ["(", ")"].contains(character) {
if character == "(" {
stack.append(character)
} else if !stack.isEmpty && character == ")" {
_ = stack.popLast()
}
}

return stack.isEmpty
}
Comment on lines +122 to +135
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe this isn't the best name for this since this isn't strictly checking for balanced parentheses pairs.


func splitIntoCacheKeyComponents() -> [String] {
var result = [String]()
var unbalancedString = ""
let tmp = split(separator: ".", omittingEmptySubsequences: false)
tmp
.enumerated()
.forEach { index, item in
let value = String(item)
if value.isBalanced && unbalancedString == "" {
result.append(value)
} else {
unbalancedString += unbalancedString == "" ? value : ".\(value)"
if unbalancedString.isBalanced {
result.append(unbalancedString)
unbalancedString = ""
}
}
if unbalancedString != "" && index == tmp.count - 1 {
result.append(unbalancedString)
}
}
return result
}
}
53 changes: 53 additions & 0 deletions Tests/ApolloTests/Cache/CacheKeyConstructionTests.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
import XCTest
@testable import ApolloSQLite

final class CacheKeyConstructionTests: XCTestCase {
Iron-Ham marked this conversation as resolved.
Show resolved Hide resolved
func testCacheKeySplitsPeriods() {
let input = "my.chemical.romance"
let expected = ["my", "chemical", "romance"]

XCTAssertEqual(input.splitIntoCacheKeyComponents(), expected)
}

func testCacheKeySplitsPeriodsButIgnoresParentheses() {
let input = "my.chemical.romance(xWv.CD-RIP.whole-album)"
let expected = ["my", "chemical", "romance(xWv.CD-RIP.whole-album)"]

XCTAssertEqual(input.splitIntoCacheKeyComponents(), expected)
}

func testCacheKeyIgnoresNestedParentheses() {
let input = "my.chemical.romance(the.(very)hidden.albums)"
let expected = ["my", "chemical", "romance(the.(very)hidden.albums)"]

XCTAssertEqual(input.splitIntoCacheKeyComponents(), expected)
}

func testDoubleNestedInput() {
let input = "my.chemical.romance(name:imnotokay.rip(xWv(the.original).HIGH-QUALITY)).mp3"
let expected = ["my", "chemical", "romance(name:imnotokay.rip(xWv(the.original).HIGH-QUALITY))", "mp3"]

XCTAssertEqual(input.splitIntoCacheKeyComponents(), expected)
}

func testUnbalancedInput() {
let input = "my.chemical.romance(name: )(.thebest.)()"
let expected = ["my", "chemical", "romance(name: )(.thebest.)()"]

XCTAssertEqual(input.splitIntoCacheKeyComponents(), expected)
}

func testUnbalancedInputContinued() {
let input = "my.chemical.romance(name: )(.thebest.)().count"
let expected = ["my", "chemical", "romance(name: )(.thebest.)()", "count"]

XCTAssertEqual(input.splitIntoCacheKeyComponents(), expected)
}

func testNoSplits() {
let input = "mychemicalromance"
let expected = ["mychemicalromance"]

XCTAssertEqual(input.splitIntoCacheKeyComponents(), expected)
}
}
61 changes: 61 additions & 0 deletions Tests/ApolloTests/Cache/SQLite/CachePersistenceTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,67 @@ class CachePersistenceTests: XCTestCase {
}
}

func testFetchAndPersistWithPeriodArguments() throws {
let query = SearchQuery(term: "Luke.Skywalker")
let sqliteFileURL = SQLiteTestCacheProvider.temporarySQLiteFileURL()

try SQLiteTestCacheProvider.withCache(fileURL: sqliteFileURL) { (cache) in
let store = ApolloStore(cache: cache)

let server = MockGraphQLServer()
let networkTransport = MockNetworkTransport(server: server, store: store)

let client = ApolloClient(networkTransport: networkTransport, store: store)

_ = server.expect(SearchQuery.self) { request in
[
"data": [
"search": [
[
"id": "1000",
"name": "Luke Skywalker",
"__typename": "Human"
]
]
]
]
}
let networkExpectation = self.expectation(description: "Fetching query from network")
let newCacheExpectation = self.expectation(description: "Fetch query from new cache")

client.fetch(query: query, cachePolicy: .fetchIgnoringCacheData) { outerResult in
defer { networkExpectation.fulfill() }

switch outerResult {
case .failure(let error):
XCTFail("Unexpected error: \(error)")
return
case .success(let graphQLResult):
XCTAssertEqual(graphQLResult.data?.search?.first??.asHuman?.name, "Luke Skywalker")
// Do another fetch from cache to ensure that data is cached before creating new cache
client.fetch(query: query, cachePolicy: .returnCacheDataDontFetch) { innerResult in
try! SQLiteTestCacheProvider.withCache(fileURL: sqliteFileURL) { cache in
let newStore = ApolloStore(cache: cache)
let newClient = ApolloClient(networkTransport: networkTransport, store: newStore)
newClient.fetch(query: query, cachePolicy: .returnCacheDataDontFetch) { newClientResult in
defer { newCacheExpectation.fulfill() }
switch newClientResult {
case .success(let newClientGraphQLResult):
XCTAssertEqual(newClientGraphQLResult.data?.search?.first??.asHuman?.name, "Luke Skywalker")
case .failure(let error):
XCTFail("Unexpected error with new client: \(error)")
}
_ = newClient // Workaround for a bug - ensure that newClient is retained until this block is run
}
}
}
}
}

self.waitForExpectations(timeout: 2, handler: nil)
}
}

func testPassInConnectionDoesNotThrow() {
do {
let database = try SQLiteDotSwiftDatabase(connection: Connection())
Expand Down