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

apollographql · Sep 11, 2024 · f0e64bc · f0e64bc
1 parent f94cd8d
commit f0e64bc
Show file tree

Hide file tree

Showing 2 changed files with 62 additions and 1 deletion.
diff --git a/...bating/src/commonMain/kotlin/com/apollographql/cache/normalized/sql/SqlNormalizedCache.kt b/...bating/src/commonMain/kotlin/com/apollographql/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/...ng/src/commonTest/kotlin/com/apollographql/cache/normalized/sql/SqlNormalizedCacheTest.kt b/...ng/src/commonTest/kotlin/com/apollographql/cache/normalized/sql/SqlNormalizedCacheTest.kt
@@ -253,6 +253,62 @@ 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,
+        recordMerger = DefaultRecordMerger,
+    )
+
+    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,
+        recordMerger = DefaultRecordMerger,
+    )
+
+    cache.merge(
+        record = Record(
+            key = "key2",
+            fields = mapOf(
+                "field1" to "value2",
+                "refToKey1" to CacheKey("key1"),
+            ),
+        ),
+        cacheHeaders = CacheHeaders.NONE,
+        recordMerger = DefaultRecordMerger,
+    )
+
+    val result = cache.remove(cacheKey = CacheKey("key1"), cascade = true)
+
+    assertTrue(result)
+    assertNull(cache.loadRecord("key1", CacheHeaders.NONE))
+    assertNull(cache.loadRecord("key2", CacheHeaders.NONE))
+  }
+
   private val BadDriver = object : SqlDriver {
     override fun close() {
       throw IllegalStateException("bad cache")