Handle lineage graph cycles on the client (MarquezProject#2506)

* init

Signed-off-by: John Lukenoff <johnlukenoff@asana.com>

* repro

Signed-off-by: John Lukenoff <johnlukenoff@asana.com>

* remove log lines

Signed-off-by: John Lukenoff <johnlukenoff@asana.com>

* formatting

Signed-off-by: John Lukenoff <johnlukenoff@asana.com>

* cleanup

Signed-off-by: John Lukenoff <johnlukenoff@asana.com>

* Create a smaller cycle

Signed-off-by: John Lukenoff <johnlukenoff@asana.com>

* remove stub metadata

Signed-off-by: John Lukenoff <johnlukenoff@asana.com>

* Test draft

Signed-off-by: John Lukenoff <johnlukenoff@asana.com>

* try stubbing some methods

Signed-off-by: John Lukenoff <johnlukenoff@asana.com>

* Test rendering component

Signed-off-by: John Lukenoff <johnlukenoff@asana.com>

* test without mounting

Signed-off-by: John Lukenoff <johnlukenoff@asana.com>

* Formatting

Signed-off-by: John Lukenoff <johnlukenoff@asana.com>

* Remove unnecessary types

Signed-off-by: John Lukenoff <johnlukenoff@asana.com>

* Revert new module installations

Signed-off-by: John Lukenoff <johnlukenoff@asana.com>

* oops

Signed-off-by: John Lukenoff <johnlukenoff@asana.com>

* Add a test for success

Signed-off-by: John Lukenoff <johnlukenoff@asana.com>

* dont use ternary

Signed-off-by: John Lukenoff <johnlukenoff@asana.com>

* improve path checking

Signed-off-by: John Lukenoff <johnlukenoff@asana.com>

---------

Signed-off-by: John Lukenoff <johnlukenoff@asana.com>
Signed-off-by: Xavier-Cliquennois <xavier.cliquennois@wearegraphite.io>

Author: jlukenoff

web/src/__tests__/components/Lineage.test.tsx

@@ -0,0 +1,112 @@
+// Copyright 2018-2023 contributors to the Marquez project
+// SPDX-License-Identifier: Apache-2.0
+
+import { Lineage, LineageProps } from '../../components/lineage/Lineage'
+import { LineageNode } from '../../components/lineage/types'
+
+const mockGraphWithCycle = [
+  {
+    id: 'job_foo',
+    inEdges: [
+      {
+        origin: 'dataset_foo',
+        destination: 'job_foo'
+      }
+    ],
+    outEdges: [
+      {
+        origin: 'job_foo',
+        destination: 'dataset_bar'
+      }
+    ]
+  },
+  {
+    id: 'dataset_bar',
+    inEdges: [
+      {
+        origin: 'job_foo',
+        destination: 'dataset_bar'
+      }
+    ],
+    outEdges: [
+      {
+        origin: 'dataset_bar',
+        destination: 'job_bar'
+      }
+    ]
+  },
+  {
+    id: 'job_bar',
+    inEdges: [
+      {
+        origin: 'dataset_bar',
+        destination: 'job_bar'
+      }
+    ],
+    outEdges: [
+      {
+        origin: 'job_bar',
+        destination: 'dataset_foo'
+      }
+    ]
+  },
+  {
+    id: 'dataset_foo',
+    inEdges: [
+      {
+        origin: 'job_bar',
+        destination: 'dataset_foo'
+      }
+    ],
+    outEdges: [
+      {
+        origin: 'dataset_foo',
+        destination: 'job_foo'
+      }
+    ]
+  }
+]
+
+function mockSetState(newState: any) {
+  this.state = { ...this.state, ...newState }
+}
+
+describe('Lineage Component', () => {
+  let instance: Lineage
+
+  beforeEach(() => {
+    instance = new Lineage(({ selectedNode: 'job_foo' } as unknown) as LineageProps)
+    instance.setState = mockSetState
+    instance.initGraph()
+    instance.buildGraphAll((mockGraphWithCycle as unknown) as LineageNode[])
+  })
+
+  it("doesn't follow cycles in the lineage graph", () => {
+    const paths = instance.getSelectedPaths()
+
+    const pathCounts = paths.reduce((acc, p) => {
+      const pathId = p.join(':')
+      acc[pathId] = (acc[pathId] || 0) + 1
+      return acc
+    }, {} as Record<string, number>)
+
+    expect(Object.values(pathCounts).some(c => c > 2)).toBe(false)
+  })
+
+  it('renders a valid cycle', () => {
+    const actualPaths = instance.getSelectedPaths()
+
+    const expectedPaths = [
+      ['job_foo', 'dataset_bar'],
+      ['dataset_bar', 'job_bar'],
+      ['job_bar', 'dataset_foo'],
+      ['dataset_foo', 'job_foo'],
+      ['dataset_foo', 'job_foo'],
+      ['job_bar', 'dataset_foo'],
+      ['dataset_bar', 'job_bar'],
+      ['job_foo', 'dataset_bar']
+    ]
+
+    expect(actualPaths).toEqual(expectedPaths)
+  })
+})

web/src/components/lineage/Lineage.tsx

@@ -64,14 +64,14 @@ export interface JobOrDatasetMatchParams {
   nodeType: string
 }
 
-type LineageProps = WithStyles<typeof styles> &
+export type LineageProps = WithStyles<typeof styles> &
   StateProps &
   DispatchProps &
   RouteComponentProps<JobOrDatasetMatchParams>
 
 let g: graphlib.Graph<MqNode>
 
-class Lineage extends React.Component<LineageProps, LineageState> {
+export class Lineage extends React.Component<LineageProps, LineageState> {
   constructor(props: LineageProps) {
     super(props)
     this.state = {
@@ -143,7 +143,16 @@ class Lineage extends React.Component<LineageProps, LineageState> {
   getSelectedPaths = () => {
     const paths = [] as Array<[string, string]>
 
+    // Sets used to detect cycles and break out of the recursive loop
+    const visitedNodes = {
+      successors: new Set(),
+      predecessors: new Set()
+    }
+
     const getSuccessors = (node: string) => {
+      if (visitedNodes.successors.has(node)) return
+      visitedNodes.successors.add(node)
+
       const successors = g?.successors(node)
       if (successors?.length) {
         for (let i = 0; i < node.length - 1; i++) {
@@ -156,6 +165,9 @@ class Lineage extends React.Component<LineageProps, LineageState> {
     }
 
     const getPredecessors = (node: string) => {
+      if (visitedNodes.predecessors.has(node)) return
+      visitedNodes.predecessors.add(node)
+
       const predecessors = g?.predecessors(node)
       if (predecessors?.length) {
         for (let i = 0; i < node.length - 1; i++) {