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

* Allow commas in arguments to be ignored when parsing cacheKeys

* Update

* More tests

* No-split case

Author: Iron-Ham

Apollo.xcodeproj/project.pbxproj

@@ -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 */; };
@@ -617,6 +618,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>"; };
@@ -2225,6 +2227,7 @@
 				9F8622F71EC2004200C38162 /* ReadWriteFromStoreTests.swift */,
 				9FD03C2D25527CE6002227DC /* StoreConcurrencyTests.swift */,
 				9FA6ABCB1EC0A9F7000017BE /* WatchQueryTests.swift */,
+				2EE7FFCF276802E30035DC39 /* CacheKeyConstructionTests.swift */,
 			);
 			path = Cache;
 			sourceTree = "<group>";
@@ -3400,6 +3403,7 @@
 				9B21FD752422C29D00998B5C /* GraphQLFileTests.swift in Sources */,
 				DE2FCF4926E94D150057EA67 /* SelectionTests.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 */,

Sources/ApolloSQLite/SQLiteNormalizedCache.swift

@@ -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 {
@@ -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
+  }
+
+  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
+  }
+}

Tests/ApolloTests/Cache/CacheKeyConstructionTests.swift

@@ -0,0 +1,53 @@
+import XCTest
+@testable import ApolloSQLite
+
+final class CacheKeyConstructionTests: XCTestCase {
+  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)
+  }
+}

Tests/ApolloTests/Cache/SQLite/CachePersistenceTests.swift

@@ -80,6 +80,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())