Detect cycles when creating subgraphs (#264)

* Cycle detection working * Add performance test to guard the cycle detection doesn't introduce regression * Add performance test to guard the cycle detection doesn't introduce regression - separate test
jraska · Jul 23, 2024 · abc7391 · abc7391
1 parent a99a276
commit abc7391
Show file tree

Hide file tree

Showing 3 changed files with 87 additions and 15 deletions.
diff --git a/plugin/src/main/kotlin/com/jraska/module/graph/DependencyGraph.kt b/plugin/src/main/kotlin/com/jraska/module/graph/DependencyGraph.kt
@@ -62,7 +62,7 @@ class DependencyGraph private constructor() {
     require(nodes.contains(key)) { "Dependency Tree doesn't contain module: $key" }
 
     val connections = mutableListOf<Pair<String, String>>()
-    addConnections(nodes.getValue(key), connections)
+    addConnections(nodes.getValue(key), connections, mutableSetOf())
 
     return if (connections.isEmpty()) {
       createSingular(key)
@@ -78,11 +78,23 @@ class DependencyGraph private constructor() {
     )
   }
 
-  private fun addConnections(node: Node, into: MutableList<Pair<String, String>>) {
-    node.dependsOn.forEach {
-      into.add(node.key to it.key)
-      addConnections(it, into)
+  private fun addConnections(
+    node: Node, into: MutableList<Pair<String, String>>,
+    path: MutableSet<Node>
+  ) {
+    path.add(node)
+    node.dependsOn.forEach { dependant ->
+      into.add(node.key to dependant.key)
+
+      val nodeInCurrentPath = path.contains(dependant)
+      if (nodeInCurrentPath) {
+        val pathText = path.joinToString(separator = ", ") { it.key }
+        throw IllegalStateException("Dependency cycle detected! Cycle in nodes: '${pathText}'.")
+      }
+      addConnections(dependant, into, path)
     }
+
+    path.remove(node)
   }
 
   private fun countEdges(): Int {

diff --git a/plugin/src/test/kotlin/com/jraska/module/graph/DependencyGraphPerformanceTest.kt b/plugin/src/test/kotlin/com/jraska/module/graph/DependencyGraphPerformanceTest.kt
@@ -28,4 +28,10 @@ class DependencyGraphPerformanceTest {
     assert(statistics.edgesCount == 15259)
     assert(statistics.longestPath.pathString().startsWith("23 -> 31 -> 36 -> 57 -> 61 -> 72 -> 74 -> 75"))
   }
+
+  @Test(timeout = 10000)
+  fun whenTheGraphIsLarge_statisticsSubGraphCreatedFast() {
+    val subGraphStatistics = dependencyGraph.subTree("500").statistics()
+    assert(subGraphStatistics.modulesCount == 281)
+  }
 }
diff --git a/plugin/src/test/kotlin/com/jraska/module/graph/DependencyGraphTest.kt b/plugin/src/test/kotlin/com/jraska/module/graph/DependencyGraphTest.kt
@@ -18,13 +18,11 @@ class DependencyGraphTest {
   @Test
   fun findsProperLongestPath() {
     val dependencyTree = DependencyGraph.create(
-
       "app" to "feature",
       "app" to "lib",
       "app" to "core",
       "feature" to "lib",
       "lib" to "core"
-
     )
 
     assert(dependencyTree.longestPath("app").nodeNames == listOf("app", "feature", "lib", "core"))
@@ -38,7 +36,6 @@ class DependencyGraphTest {
       "app" to "feature",
       "app" to "lib",
       "app" to "core"
-
     )
 
     assert(dependencyTree.findRoot().key == "app")
@@ -150,12 +147,69 @@ class DependencyGraphTest {
     assert(statistics.height == 5)
     assert(statistics.modulesCount == 16)
     assert(statistics.edgesCount == 45)
-    assert(statistics.longestPath.nodeNames == listOf(
-      ":feature:settings",
-      ":app",
-      ":feature:settings_entrance",
-      ":lib:dynamic-features",
-      ":core-android",
-      ":core"))
+    assert(
+      statistics.longestPath.nodeNames == listOf(
+        ":feature:settings",
+        ":app",
+        ":feature:settings_entrance",
+        ":lib:dynamic-features",
+        ":core-android",
+        ":core"
+      )
+    )
+  }
+
+  @Test(expected = IllegalStateException::class)
+  fun cyclesDoNotOverflow() {
+    val dependencyTree = DependencyGraph.create(
+      "feature" to "lib",
+      "lib" to "core",
+      "lib" to "feature",
+      "feature" to "core",
+      "app" to "feature"
+    )
+
+    dependencyTree.subTree("app")
+  }
+
+  @Test
+  fun doesNotDetectCycleOnMultiEntry() {
+    val dependencyTree = DependencyGraph.create(
+      "feature" to "lib",
+      "app" to "lib",
+      "app" to "feature"
+    )
+
+    assert(dependencyTree.subTree("app").findRoot().key == "app")
+  }
+
+  @Test(expected = IllegalStateException::class)
+  fun detectsTwoNodesCycle() {
+    val dependencyTree = DependencyGraph.create(
+      "feature" to "app",
+      "app" to "feature"
+    )
+
+    dependencyTree.subTree("app")
+  }
+
+  @Test(expected = IllegalStateException::class)
+  fun detectsThreeNodesCycle() {
+    val dependencyTree = DependencyGraph.create(
+      "feature" to "app",
+      "app" to "lib",
+      "lib" to "feature"
+    )
+
+    dependencyTree.subTree("app")
+  }
+
+  @Test(expected = IllegalStateException::class)
+  fun detectsSingleNodeCycle() {
+    val dependencyTree = DependencyGraph.create(
+      "app" to "app",
+    )
+
+    dependencyTree.subTree("app")
   }
 }