Merge pull request #105 from snyk/fix/count-cycles

fix: Fix countPathsToRoot for cyclic graphs
snyk · May 9, 2023 · 5be7203 · 5be7203
2 parents cf57280 + 4b15635
commit 5be7203
Show file tree

Hide file tree

Showing 7 changed files with 308 additions and 141 deletions.
diff --git a/src/core/dep-graph.ts b/src/core/dep-graph.ts
@@ -152,9 +152,14 @@ class DepGraphImpl implements types.DepGraphInternal {
   public countPathsToRoot(pkg: types.Pkg): number {
     let count = 0;
     for (const nodeId of this.getPkgNodeIds(pkg)) {
-      count += this.countNodePathsToRoot(nodeId);
+      if (this._countNodePathsToRootCache.has(nodeId)) {
+        count += this._countNodePathsToRootCache.get(nodeId)!;
+      } else {
+        const c = this.countNodePathsToRoot(nodeId);
+        this._countNodePathsToRootCache.set(nodeId, c);
+        count += c;
+      }
     }
-
     return count;
   }
 
@@ -367,30 +372,17 @@ class DepGraphImpl implements types.DepGraphInternal {
     return allPaths;
   }
 
-  private countNodePathsToRoot(
-    nodeId: string,
-    ancestors: string[] = [],
-  ): number {
-    if (ancestors.includes(nodeId)) {
-      return 0;
-    }
-
-    if (this._countNodePathsToRootCache.has(nodeId)) {
-      return this._countNodePathsToRootCache.get(nodeId) || 0;
-    }
-
-    const parentNodesIds = this.getNodeParentsNodeIds(nodeId);
-    if (parentNodesIds.length === 0) {
-      this._countNodePathsToRootCache.set(nodeId, 1);
+  private countNodePathsToRoot(nodeId: string, visited: string[] = []): number {
+    if (nodeId === this._rootNodeId) {
       return 1;
     }
-
-    ancestors = ancestors.concat(nodeId);
-    const count = parentNodesIds.reduce((acc, parentNodeId) => {
-      return acc + this.countNodePathsToRoot(parentNodeId, ancestors);
-    }, 0);
-
-    this._countNodePathsToRootCache.set(nodeId, count);
+    visited = visited.concat(nodeId);
+    let count = 0;
+    for (const parentNodeId of this.getNodeParentsNodeIds(nodeId)) {
+      if (!visited.includes(parentNodeId)) {
+        count += this.countNodePathsToRoot(parentNodeId, visited);
+      }
+    }
     return count;
   }
 }
diff --git a/test/core/__snapshots__/cycles.test.ts.snap b/test/core/__snapshots__/cycles.test.ts.snap
diff --git a/test/core/__snapshots__/pkg-paths-to-root.test.ts.snap b/test/core/__snapshots__/pkg-paths-to-root.test.ts.snap
@@ -0,0 +1,234 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`pkgPathsToRoot cycles returns expected paths for all packages: a@1 1`] = `
+Array [
+  Array [
+    Object {
+      "name": "a",
+      "version": "1",
+    },
+  ],
+]
+`;
+
+exports[`pkgPathsToRoot cycles returns expected paths for all packages: b@2 1`] = `
+Array [
+  Array [
+    Object {
+      "name": "b",
+      "version": "2",
+    },
+    Object {
+      "name": "a",
+      "version": "1",
+    },
+  ],
+]
+`;
+
+exports[`pkgPathsToRoot cycles returns expected paths for all packages: c@3 1`] = `
+Array [
+  Array [
+    Object {
+      "name": "c",
+      "version": "3",
+    },
+    Object {
+      "name": "a",
+      "version": "1",
+    },
+  ],
+]
+`;
+
+exports[`pkgPathsToRoot cycles returns expected paths for all packages: d@4 1`] = `
+Array [
+  Array [
+    Object {
+      "name": "d",
+      "version": "4",
+    },
+    Object {
+      "name": "a",
+      "version": "1",
+    },
+  ],
+]
+`;
+
+exports[`pkgPathsToRoot cycles returns expected paths for all packages: e@5 1`] = `
+Array [
+  Array [
+    Object {
+      "name": "e",
+      "version": "5",
+    },
+    Object {
+      "name": "b",
+      "version": "2",
+    },
+    Object {
+      "name": "a",
+      "version": "1",
+    },
+  ],
+  Array [
+    Object {
+      "name": "e",
+      "version": "5",
+    },
+    Object {
+      "name": "c",
+      "version": "3",
+    },
+    Object {
+      "name": "a",
+      "version": "1",
+    },
+  ],
+  Array [
+    Object {
+      "name": "e",
+      "version": "5",
+    },
+    Object {
+      "name": "g",
+      "version": "7",
+    },
+    Object {
+      "name": "f",
+      "version": "6",
+    },
+    Object {
+      "name": "d",
+      "version": "4",
+    },
+    Object {
+      "name": "a",
+      "version": "1",
+    },
+  ],
+]
+`;
+
+exports[`pkgPathsToRoot cycles returns expected paths for all packages: f@6 1`] = `
+Array [
+  Array [
+    Object {
+      "name": "f",
+      "version": "6",
+    },
+    Object {
+      "name": "d",
+      "version": "4",
+    },
+    Object {
+      "name": "a",
+      "version": "1",
+    },
+  ],
+  Array [
+    Object {
+      "name": "f",
+      "version": "6",
+    },
+    Object {
+      "name": "e",
+      "version": "5",
+    },
+    Object {
+      "name": "b",
+      "version": "2",
+    },
+    Object {
+      "name": "a",
+      "version": "1",
+    },
+  ],
+  Array [
+    Object {
+      "name": "f",
+      "version": "6",
+    },
+    Object {
+      "name": "e",
+      "version": "5",
+    },
+    Object {
+      "name": "c",
+      "version": "3",
+    },
+    Object {
+      "name": "a",
+      "version": "1",
+    },
+  ],
+]
+`;
+
+exports[`pkgPathsToRoot cycles returns expected paths for all packages: g@7 1`] = `
+Array [
+  Array [
+    Object {
+      "name": "g",
+      "version": "7",
+    },
+    Object {
+      "name": "f",
+      "version": "6",
+    },
+    Object {
+      "name": "d",
+      "version": "4",
+    },
+    Object {
+      "name": "a",
+      "version": "1",
+    },
+  ],
+  Array [
+    Object {
+      "name": "g",
+      "version": "7",
+    },
+    Object {
+      "name": "f",
+      "version": "6",
+    },
+    Object {
+      "name": "e",
+      "version": "5",
+    },
+    Object {
+      "name": "b",
+      "version": "2",
+    },
+    Object {
+      "name": "a",
+      "version": "1",
+    },
+  ],
+  Array [
+    Object {
+      "name": "g",
+      "version": "7",
+    },
+    Object {
+      "name": "f",
+      "version": "6",
+    },
+    Object {
+      "name": "e",
+      "version": "5",
+    },
+    Object {
+      "name": "c",
+      "version": "3",
+    },
+    Object {
+      "name": "a",
+      "version": "1",
+    },
+  ],
+]
+`;