Fix cascade SQL cache remove loops forever on cyclic references (#6137)

apollographql · Sep 3, 2024 · 342b21c · 342b21c
1 parent f81fce0
commit 342b21c
Show file tree

Hide file tree

Showing 2 changed files with 59 additions and 1 deletion.
diff --git a/...src/commonMain/kotlin/com/apollographql/apollo/cache/normalized/sql/SqlNormalizedCache.kt b/...src/commonMain/kotlin/com/apollographql/apollo/cache/normalized/sql/SqlNormalizedCache.kt
@@ -121,14 +121,19 @@ class SqlNormalizedCache internal constructor(
   /**
    * Assume an enclosing transaction
    */
-  private fun internalDeleteRecord(key: String, cascade: Boolean): Boolean {
+  private fun internalDeleteRecord(key: String, cascade: Boolean, visited: MutableSet<String> = mutableSetOf()): Boolean {
     if (cascade) {
+      // If we've already visited this key, return to prevent infinite loop
+      if (key in visited) return false
+      visited.add(key)
+
       recordDatabase.select(key)
           ?.referencedFields()
           ?.forEach {
             internalDeleteRecord(
                 key = it.key,
                 cascade = true,
+                visited = visited
             )
           }
     }

diff --git a/...commonTest/kotlin/com/apollographql/apollo/cache/normalized/sql/SqlNormalizedCacheTest.kt b/...commonTest/kotlin/com/apollographql/apollo/cache/normalized/sql/SqlNormalizedCacheTest.kt
@@ -356,6 +356,59 @@ class SqlNormalizedCacheTest {
     assertEquals("bad cache", throwable!!.cause!!.message)
   }
 
+  @Test
+  fun testCascadeDeleteWithSelfReference() {
+    // Creating a self-referencing record
+    cache.merge(
+        record = Record(
+            key = "selfRefKey",
+            fields = mapOf(
+                "field1" to "value1",
+                "selfRef" to CacheKey("selfRefKey"),
+            ),
+        ),
+        cacheHeaders = CacheHeaders.NONE,
+    )
+
+    val result = cache.remove(cacheKey = CacheKey("selfRefKey"), cascade = true)
+
+    assertTrue(result)
+    val record = cache.loadRecord("selfRefKey", CacheHeaders.NONE)
+    assertNull(record)
+  }
+
+  @Test
+  fun testCascadeDeleteWithCyclicReferences() {
+    // Creating two records that reference each other
+    cache.merge(
+        record = Record(
+            key = "key1",
+            fields = mapOf(
+                "field1" to "value1",
+                "refToKey2" to CacheKey("key2"),
+            ),
+        ),
+        cacheHeaders = CacheHeaders.NONE,
+    )
+
+    cache.merge(
+        record = Record(
+            key = "key2",
+            fields = mapOf(
+                "field1" to "value2",
+                "refToKey1" to CacheKey("key1"),
+            ),
+        ),
+        cacheHeaders = CacheHeaders.NONE,
+    )
+
+    val result = cache.remove(cacheKey = CacheKey("key1"), cascade = true)
+
+    assertTrue(result)
+    assertNull(cache.loadRecord("key1", CacheHeaders.NONE))
+    assertNull(cache.loadRecord("key2", CacheHeaders.NONE))
+  }
+
   private fun createRecord(key: String) {
     cache.merge(
         record = Record(