diff --git a/.changeset/containment-auto-layout.md b/.changeset/containment-auto-layout.md new file mode 100644 index 00000000..aae2b559 --- /dev/null +++ b/.changeset/containment-auto-layout.md @@ -0,0 +1,5 @@ +--- +"@serverlessworkflow/diagram-editor": minor +--- + +Add containment support integrated with auto-layout. diff --git a/packages/serverless-workflow-diagram-editor/.storybook/vitest.setup.ts b/packages/serverless-workflow-diagram-editor/.storybook/vitest.setup.ts deleted file mode 100644 index 958ce16a..00000000 --- a/packages/serverless-workflow-diagram-editor/.storybook/vitest.setup.ts +++ /dev/null @@ -1,23 +0,0 @@ -/* - * Copyright 2021-Present The Serverless Workflow Specification Authors - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -import * as a11yAddonAnnotations from "@storybook/addon-a11y/preview"; -import { setProjectAnnotations } from "@storybook/react-vite"; -import * as projectAnnotations from "./preview"; - -// This is an important step to apply the right configuration when testing your stories. -// More info at: https://storybook.js.org/docs/api/portable-stories/portable-stories-vitest#setprojectannotations -setProjectAnnotations([a11yAddonAnnotations, projectAnnotations]); diff --git a/packages/serverless-workflow-diagram-editor/src/react-flow/diagram/autoLayout.ts b/packages/serverless-workflow-diagram-editor/src/react-flow/diagram/autoLayout.ts index ec01f127..d238fe37 100644 --- a/packages/serverless-workflow-diagram-editor/src/react-flow/diagram/autoLayout.ts +++ b/packages/serverless-workflow-diagram-editor/src/react-flow/diagram/autoLayout.ts @@ -39,39 +39,80 @@ export type Size = { export type WayPoints = Point[]; export const ROOT_LAYOUT_OPTIONS: LayoutOptions = { - "elk.algorithm": "org.eclipse.elk.layered", - "elk.hierarchyHandling": "INCLUDE_CHILDREN", - "elk.direction": "DOWN", - "org.eclipse.elk.layered.layering.strategy": "INTERACTIVE", - "org.eclipse.elk.edgeRouting": "ORTHOGONAL", - "elk.layered.unnecessaryBendpoints": "true", + "org.eclipse.elk.algorithm": "org.eclipse.elk.layered", + "org.eclipse.elk.direction": "DOWN", + "org.eclipse.elk.layered.nodePlacement.strategy": "BRANDES_KOEPF", "org.eclipse.elk.layered.nodePlacement.bk.fixedAlignment": "BALANCED", "org.eclipse.elk.layered.nodePlacement.bk.edgeStraightening": "IMPROVE_STRAIGHTNESS", - "org.eclipse.elk.layered.cycleBreaking.strategy": "DEPTH_FIRST", - "org.eclipse.elk.insideSelfLoops.activate": "true", - "elk.separateConnectedComponents": "false", "org.eclipse.elk.layered.nodePlacement.favorStraightEdges": "true", - "org.eclipse.elk.layered.considerModelOrder.strategy": "EDGES", + "org.eclipse.elk.layered.priority.straightness": "10", + "org.eclipse.elk.hierarchyHandling": "INCLUDE_CHILDREN", + "org.eclipse.elk.layered.crossingMinimization.strategy": "LAYER_SWEEP", + "org.eclipse.elk.edgeRouting": "ORTHOGONAL", + "org.eclipse.elk.layered.unnecessaryBendpoints": "true", + "org.eclipse.elk.layered.cycleBreaking.strategy": "GREEDY_MODEL_ORDER", "org.eclipse.elk.layered.considerModelOrder.crossingCounterNodeInfluence": "0.001", - "elk.layered.crossingMinimization.strategy": "INTERACTIVE", - spacing: "75", - "spacing.componentComponent": "70", - "spacing.nodeNodeBetweenLayers": "80", - "elk.layered.spacing.edgeNodeBetweenLayers": "40", - "org.eclipse.elk.spacing.edgeNode": "24", + "org.eclipse.elk.layered.spacing.edgeNode": "24", + "org.eclipse.elk.layered.spacing.edgeNodeBetweenLayers": "40", + "org.eclipse.elk.layered.spacing.nodeNode": "24", + "org.eclipse.elk.layered.spacing.nodeNodeBetweenLayers": "80", + "org.eclipse.elk.layered.spacing.componentComponent": "70", "org.eclipse.elk.layered.mergeEdges": "true", }; +export const PARENT_LAYOUT_OPTIONS: LayoutOptions = { + ...ROOT_LAYOUT_OPTIONS, + "org.eclipse.elk.padding": "[top=60,left=20,bottom=20,right=20]", +}; + +// Helper function to clean up empty edges arrays from nodes +function cleanupEmptyEdges(node: ElkNode): void { + if (node.edges && node.edges.length === 0) { + delete node.edges; + } + if (node.children) { + node.children.forEach(cleanupEmptyEdges); + } +} + +// Helper function to find the common ancestor of two nodes +function findCommonAncestor( + sourceId: string, + targetId: string, + reactFlowNodeMap: Map, +): string { + // Build path from source to root + const sourcePath = new Set(); + let currentId: string | undefined = sourceId; + while (currentId) { + sourcePath.add(currentId); + const node = reactFlowNodeMap.get(currentId); + currentId = node?.parentId; + } + + // Traverse from target to root and find first common node + currentId = targetId; + while (currentId) { + if (sourcePath.has(currentId)) { + return currentId; + } + const node = reactFlowNodeMap.get(currentId); + currentId = node?.parentId; + } + + // If no common ancestor found, return "root" + return "root"; +} + export function buildElkGraphFromReactFlowGraph(reactFlowGraph: ReactFlowGraph): ElkNode { - // Create a map for easy lookup - const nodeMap = new Map( + // Create a map for easy lookup (without width/height initially) + const nodeMap = new Map( reactFlowGraph.nodes.map((node) => [ node.id, { id: node.id, - width: node.measured?.width ?? DEFAULT_NODE_SIZE.width, - height: node.measured?.height ?? DEFAULT_NODE_SIZE.height, children: [] as ElkNode[], + edges: [] as ElkExtendedEdge[], }, ]), ); @@ -81,24 +122,64 @@ export function buildElkGraphFromReactFlowGraph(reactFlowGraph: ReactFlowGraph): reactFlowGraph.nodes.forEach((node) => { const elkNode = nodeMap.get(node.id)!; if (node.parentId && nodeMap.has(node.parentId)) { - nodeMap.get(node.parentId)!.children.push(elkNode); + const parentNode = nodeMap.get(node.parentId)!; + if (!parentNode.children) { + parentNode.children = []; + } + parentNode.children.push(elkNode); } else { rootChildren.push(elkNode); } }); - // edges - const elkEdges: ElkExtendedEdge[] = reactFlowGraph.edges.map((edge) => ({ - id: edge.id, - sources: [edge.source], - targets: [edge.target], - })); + // Apply layout options and dimensions based on whether node has children + reactFlowGraph.nodes.forEach((node) => { + const elkNode = nodeMap.get(node.id)!; + if (elkNode.children && elkNode.children.length > 0) { + // Nodes with children get layout options but no fixed dimensions + elkNode.layoutOptions = { ...PARENT_LAYOUT_OPTIONS }; + } else { + // Leaf nodes get fixed dimensions + elkNode.width = node.measured?.width ?? DEFAULT_NODE_SIZE.width; + elkNode.height = node.measured?.height ?? DEFAULT_NODE_SIZE.height; + } + }); + + const reactFlowNodeMap = new Map(reactFlowGraph.nodes.map((node) => [node.id, node])); + + // Nest edges in the appropriate hierarchy level + const rootEdges: ElkExtendedEdge[] = []; + reactFlowGraph.edges.forEach((edge) => { + const elkEdge: ElkExtendedEdge = { + id: edge.id, + sources: [edge.source], + targets: [edge.target], + }; + + // Find the lowest common ancestor that contains both source and target + const commonAncestor = findCommonAncestor(edge.source, edge.target, reactFlowNodeMap); + + if (commonAncestor === "root") { + rootEdges.push(elkEdge); + } else { + const ancestorNode = nodeMap.get(commonAncestor); + if (ancestorNode) { + if (!ancestorNode.edges) { + ancestorNode.edges = []; + } + ancestorNode.edges.push(elkEdge); + } + } + }); + + // Clean up empty edges arrays from nodes that don't need them + rootChildren.forEach(cleanupEmptyEdges); return { id: "root", layoutOptions: ROOT_LAYOUT_OPTIONS, children: rootChildren, - edges: elkEdges, + edges: rootEdges, }; } @@ -116,6 +197,35 @@ function buildElkNodeMap( return map; } +// Helper function to recursively collect all edges from ELK graph +function buildElkEdgeMap( + elkNode: ElkNode, + map: Map = new Map(), +): Map { + if (elkNode.edges) { + for (const edge of elkNode.edges) { + map.set(edge.id, edge); + } + } + if (elkNode.children) { + for (const child of elkNode.children) { + buildElkEdgeMap(child, map); + } + } + return map; +} + +// Helper function to check if an edge is inside a parent node +function isEdgeInsideParent( + edge: { source: string; target: string }, + nodeMap: Map, +): boolean { + // Edge is inside a parent if the lowest common ancestor is not the root + // This matches the logic used in findCommonAncestor when building the ELK graph + const commonAncestor = findCommonAncestor(edge.source, edge.target, nodeMap); + return commonAncestor !== "root"; +} + // set export function matchReactFlowGraphWithElkLayoutedGraph( graph: ReactFlowGraph, @@ -123,7 +233,12 @@ export function matchReactFlowGraphWithElkLayoutedGraph( ): ReactFlowGraph { // Build flat maps for O(1) lookups const elkNodeMap = buildElkNodeMap(layoutedGraph); - const elkEdgeMap = new Map(layoutedGraph.edges?.map((e) => [e.id, e]) || []); + const elkEdgeMap = buildElkEdgeMap(layoutedGraph); + + // Build node map for O(1) lookups in isEdgeInsideParent + const reactFlowNodeMap = new Map( + graph.nodes.map((node) => [node.id, { id: node.id, parentId: node.parentId }]), + ); // Map node positions const layoutedNodes = graph.nodes.map((node) => { @@ -143,15 +258,27 @@ export function matchReactFlowGraphWithElkLayoutedGraph( const layoutedEdges = graph.edges.map((edge) => { const elkEdge = elkEdgeMap.get(edge.id); if (elkEdge) { - // Reconstruct data without old wayPoints to avoid stale routing whenever ELK produced this edge. + // Reconstruct data without old wayPoints to avoid stale routing const { wayPoints: _oldWayPoints, ...restData } = edge.data || {}; const bendPoints = elkEdge.sections?.flatMap((section) => section.bendPoints || []) || []; + + // Always create new data object, only add wayPoints if there are bend points + const newData = { ...restData }; + if (bendPoints.length > 0) { + // Drop ELK-provided way points for edges nested inside a parent to avoid React Flow rendering distortion + const isInsideParent = isEdgeInsideParent(edge, reactFlowNodeMap); + if (isInsideParent) { + // There is an incompatibility with the react flow library, the wayPoints are calculated correctly by ELK + // but the way react flow render edges inside parent nodes cause path distortions. + newData.wayPoints = undefined; + } else { + newData.wayPoints = bendPoints; + } + } + return { ...edge, - data: { - ...restData, - ...(bendPoints.length > 0 && { wayPoints: bendPoints }), - }, + data: newData, }; } return edge; diff --git a/packages/serverless-workflow-diagram-editor/src/react-flow/diagram/diagramBuilder.ts b/packages/serverless-workflow-diagram-editor/src/react-flow/diagram/diagramBuilder.ts index 28471350..268d7d3e 100644 --- a/packages/serverless-workflow-diagram-editor/src/react-flow/diagram/diagramBuilder.ts +++ b/packages/serverless-workflow-diagram-editor/src/react-flow/diagram/diagramBuilder.ts @@ -92,6 +92,7 @@ function buildReactFlowNode( height: DEFAULT_NODE_SIZE.height, width: DEFAULT_NODE_SIZE.width, position: { x: 0, y: 0 }, + ...(graphNode.parentId !== "root" && { parentId: graphNode.parentId, extent: "parent" }), }; } @@ -121,12 +122,9 @@ export function buildDiagramElements(model: sdk.Specification.Workflow | null): const graph = buildFlatGraph(model); const catchContainerIds = getCatchContainerNodeIds(graph); - graph.nodes.forEach((graphNode) => { - // TODO: only nodes on root level are supported for now - if (graphNode.parentId === "root") { - nodes.push(buildReactFlowNode(graphNode, catchContainerIds)); - } - }); + graph.nodes.forEach((graphNode) => + nodes.push(buildReactFlowNode(graphNode, catchContainerIds)), + ); // Precompute node ID set for O(1) membership checks const nodeIdSet = new Set(nodes.map((node) => node.id)); diff --git a/packages/serverless-workflow-diagram-editor/src/react-flow/nodes/Nodes.tsx b/packages/serverless-workflow-diagram-editor/src/react-flow/nodes/Nodes.tsx index dafeadfa..31ae80e2 100644 --- a/packages/serverless-workflow-diagram-editor/src/react-flow/nodes/Nodes.tsx +++ b/packages/serverless-workflow-diagram-editor/src/react-flow/nodes/Nodes.tsx @@ -27,6 +27,8 @@ export const CATCH_CONTAINER_NODE_TYPE = "catch-container"; export const ReactFlowNodeTypes: RF.NodeTypes = { [GraphNodeType.Start]: StartNode, [GraphNodeType.End]: EndNode, + [GraphNodeType.Entry]: EntryNode, + [GraphNodeType.Exit]: ExitNode, [GraphNodeType.Call]: CallNode, [GraphNodeType.Do]: DoNode, [GraphNodeType.Emit]: EmitNode, @@ -158,6 +160,20 @@ export function EndNode({ id, data, selected, type }: RF.NodeProps) return ; } +/* entry node */ +export type EntryNodeType = RF.Node; +export function EntryNode({ id, data, selected, type }: RF.NodeProps) { + // TODO: This component is just a placeholder + return ; +} + +/* exit node */ +export type ExitNodeType = RF.Node; +export function ExitNode({ id, data, selected, type }: RF.NodeProps) { + // TODO: This component is just a placeholder + return ; +} + /* call leaf node */ export type CallNodeType = RF.Node, typeof GraphNodeType.Call>; export function CallNode({ id, data, selected, type }: RF.NodeProps) { diff --git a/packages/serverless-workflow-diagram-editor/tests-e2e/diagram-editor.spec.ts b/packages/serverless-workflow-diagram-editor/tests-e2e/diagram-editor.spec.ts index 371d0805..d0a8fe4b 100644 --- a/packages/serverless-workflow-diagram-editor/tests-e2e/diagram-editor.spec.ts +++ b/packages/serverless-workflow-diagram-editor/tests-e2e/diagram-editor.spec.ts @@ -27,9 +27,9 @@ test("diagram editor renders correctly", async ({ page }) => { // Check total nodes const nodes = page.locator('[data-testid^="rf__node-"]'); - await expect(nodes).toHaveCount(6); + await expect(nodes).toHaveCount(9); // Check total edge const edges = page.locator('[data-testid^="rf__edge-"]'); - await expect(edges).toHaveCount(5); + await expect(edges).toHaveCount(7); }); diff --git a/packages/serverless-workflow-diagram-editor/tests/react-flow/diagram/autoLayout.integration.test.ts b/packages/serverless-workflow-diagram-editor/tests/react-flow/diagram/autoLayout.integration.test.ts index a00b4db3..5cc19149 100644 --- a/packages/serverless-workflow-diagram-editor/tests/react-flow/diagram/autoLayout.integration.test.ts +++ b/packages/serverless-workflow-diagram-editor/tests/react-flow/diagram/autoLayout.integration.test.ts @@ -23,6 +23,7 @@ import { applyAutoLayout, DEFAULT_NODE_SIZE, ROOT_LAYOUT_OPTIONS, + PARENT_LAYOUT_OPTIONS, } from "../../../src/react-flow/diagram/autoLayout"; import type { ReactFlowGraph } from "../../../src/react-flow/diagram/diagramBuilder"; import * as core from "../../../src/core"; @@ -188,6 +189,129 @@ describe("autoLayout", () => { expect(elkGraph.children).toHaveLength(1); expect(elkGraph.children?.[0].id).toBe("node1"); }); + + it("applies PARENT_LAYOUT_OPTIONS to nodes with children", () => { + const reactFlowGraph: ReactFlowGraph = { + nodes: [ + { + id: "parent", + position: { x: 0, y: 0 }, + data: {}, + measured: { width: 300, height: 200 }, + }, + { + id: "child", + position: { x: 10, y: 10 }, + data: {}, + parentId: "parent", + measured: { width: 100, height: 50 }, + }, + ] as Node[], + edges: [], + }; + + const elkGraph = buildElkGraphFromReactFlowGraph(reactFlowGraph); + + // Parent node should have layout options and no fixed dimensions + expect(elkGraph.children?.[0].layoutOptions).toBeDefined(); + expect(elkGraph.children?.[0].layoutOptions?.["org.eclipse.elk.padding"]).toBe( + "[top=60,left=20,bottom=20,right=20]", + ); + expect(elkGraph.children?.[0].width).toBeUndefined(); + expect(elkGraph.children?.[0].height).toBeUndefined(); + + // Child node should have fixed dimensions and no layout options + expect(elkGraph.children?.[0].children?.[0].width).toBe(100); + expect(elkGraph.children?.[0].children?.[0].height).toBe(50); + expect(elkGraph.children?.[0].children?.[0].layoutOptions).toBeUndefined(); + }); + + it("places edges at correct hierarchy level - root level", () => { + const reactFlowGraph: ReactFlowGraph = { + nodes: [ + { id: "node1", position: { x: 0, y: 0 }, data: {} }, + { id: "node2", position: { x: 0, y: 0 }, data: {} }, + ] as Node[], + edges: [{ id: "edge1", source: "node1", target: "node2", data: {} }] as Edge[], + }; + + const elkGraph = buildElkGraphFromReactFlowGraph(reactFlowGraph); + + expect(elkGraph.edges).toHaveLength(1); + expect(elkGraph.edges?.[0].id).toBe("edge1"); + }); + + it("places edges at correct hierarchy level - inside parent", () => { + const reactFlowGraph: ReactFlowGraph = { + nodes: [ + { id: "parent", position: { x: 0, y: 0 }, data: {} }, + { id: "child1", position: { x: 0, y: 0 }, data: {}, parentId: "parent" }, + { id: "child2", position: { x: 0, y: 0 }, data: {}, parentId: "parent" }, + ] as Node[], + edges: [{ id: "edge1", source: "child1", target: "child2", data: {} }] as Edge[], + }; + + const elkGraph = buildElkGraphFromReactFlowGraph(reactFlowGraph); + + // Edge should be inside parent, not at root + expect(elkGraph.edges).toHaveLength(0); + expect(elkGraph.children?.[0].edges).toHaveLength(1); + expect(elkGraph.children?.[0].edges?.[0].id).toBe("edge1"); + }); + + it("places edges at lowest common ancestor", () => { + const reactFlowGraph: ReactFlowGraph = { + nodes: [ + { id: "parent1", position: { x: 0, y: 0 }, data: {} }, + { id: "child1", position: { x: 0, y: 0 }, data: {}, parentId: "parent1" }, + { id: "parent2", position: { x: 0, y: 0 }, data: {} }, + { id: "child2", position: { x: 0, y: 0 }, data: {}, parentId: "parent2" }, + ] as Node[], + edges: [{ id: "edge1", source: "child1", target: "child2", data: {} }] as Edge[], + }; + + const elkGraph = buildElkGraphFromReactFlowGraph(reactFlowGraph); + + // Edge connects children from different parents, should be at root + expect(elkGraph.edges).toHaveLength(1); + expect(elkGraph.children?.[0].edges).toBeUndefined(); + expect(elkGraph.children?.[1].edges).toBeUndefined(); + }); + + it("cleans up empty edges arrays", () => { + const reactFlowGraph: ReactFlowGraph = { + nodes: [ + { id: "parent", position: { x: 0, y: 0 }, data: {} }, + { id: "child", position: { x: 0, y: 0 }, data: {}, parentId: "parent" }, + ] as Node[], + edges: [], + }; + + const elkGraph = buildElkGraphFromReactFlowGraph(reactFlowGraph); + + // Nodes should not have empty edges arrays + expect(elkGraph.children?.[0].edges).toBeUndefined(); + expect(elkGraph.children?.[0].children?.[0].edges).toBeUndefined(); + }); + + it("handles multi-level nesting with edges at different levels", () => { + const reactFlowGraph: ReactFlowGraph = { + nodes: [ + { id: "grandparent", position: { x: 0, y: 0 }, data: {} }, + { id: "parent", position: { x: 0, y: 0 }, data: {}, parentId: "grandparent" }, + { id: "child1", position: { x: 0, y: 0 }, data: {}, parentId: "parent" }, + { id: "child2", position: { x: 0, y: 0 }, data: {}, parentId: "parent" }, + ] as Node[], + edges: [{ id: "edge1", source: "child1", target: "child2", data: {} }] as Edge[], + }; + + const elkGraph = buildElkGraphFromReactFlowGraph(reactFlowGraph); + + // Edge should be at parent level (lowest common ancestor) + expect(elkGraph.edges).toHaveLength(0); + expect(elkGraph.children?.[0].edges).toBeUndefined(); + expect(elkGraph.children?.[0].children?.[0].edges).toHaveLength(1); + }); }); describe("matchReactFlowGraphWithElkLayoutedGraph", () => { @@ -465,6 +589,152 @@ describe("autoLayout", () => { { x: 150, y: 100 }, ]); }); + + it("handles edges inside parent nodes - removes wayPoints", () => { + const reactFlowGraph: ReactFlowGraph = { + nodes: [ + { id: "parent", position: { x: 0, y: 0 }, data: {} }, + { id: "child1", position: { x: 0, y: 0 }, data: {}, parentId: "parent" }, + { id: "child2", position: { x: 0, y: 0 }, data: {}, parentId: "parent" }, + ] as Node[], + edges: [{ id: "edge1", source: "child1", target: "child2", data: {} }] as Edge[], + }; + + const layoutedElkGraph: ElkNode = { + id: "root", + children: [ + { + id: "parent", + x: 0, + y: 0, + children: [ + { id: "child1", x: 10, y: 10 }, + { id: "child2", x: 10, y: 70 }, + ], + edges: [ + { + id: "edge1", + sources: ["child1"], + targets: ["child2"], + sections: [ + { + id: "section1", + startPoint: { x: 10, y: 10 }, + endPoint: { x: 10, y: 70 }, + bendPoints: [{ x: 10, y: 40 }], + }, + ], + }, + ], + }, + ], + edges: [], + }; + + const result = matchReactFlowGraphWithElkLayoutedGraph(reactFlowGraph, layoutedElkGraph); + + // wayPoints should be undefined for edges inside parent nodes + expect(result.edges[0].data?.wayPoints).toBeUndefined(); + }); + + it("preserves wayPoints for edges not inside parent nodes", () => { + const reactFlowGraph: ReactFlowGraph = { + nodes: [ + { id: "parent1", position: { x: 0, y: 0 }, data: {} }, + { id: "child1", position: { x: 0, y: 0 }, data: {}, parentId: "parent1" }, + { id: "parent2", position: { x: 0, y: 0 }, data: {} }, + { id: "child2", position: { x: 0, y: 0 }, data: {}, parentId: "parent2" }, + ] as Node[], + edges: [{ id: "edge1", source: "child1", target: "child2", data: {} }] as Edge[], + }; + + const layoutedElkGraph: ElkNode = { + id: "root", + children: [ + { + id: "parent1", + x: 0, + y: 0, + children: [{ id: "child1", x: 10, y: 10 }], + }, + { + id: "parent2", + x: 200, + y: 0, + children: [{ id: "child2", x: 10, y: 10 }], + }, + ], + edges: [ + { + id: "edge1", + sources: ["child1"], + targets: ["child2"], + sections: [ + { + id: "section1", + startPoint: { x: 10, y: 10 }, + endPoint: { x: 210, y: 10 }, + bendPoints: [{ x: 100, y: 10 }], + }, + ], + }, + ], + }; + + const result = matchReactFlowGraphWithElkLayoutedGraph(reactFlowGraph, layoutedElkGraph); + + // wayPoints should be preserved for edges crossing parent boundaries + expect(result.edges[0].data?.wayPoints).toEqual([{ x: 100, y: 10 }]); + }); + + it("preserves other edge data when updating wayPoints", () => { + const reactFlowGraph: ReactFlowGraph = { + nodes: [ + { id: "node1", position: { x: 0, y: 0 }, data: {} }, + { id: "node2", position: { x: 0, y: 0 }, data: {} }, + ] as Node[], + edges: [ + { + id: "edge1", + source: "node1", + target: "node2", + data: { label: "Test", color: "blue", customProp: 123 }, + }, + ] as Edge[], + }; + + const layoutedElkGraph: ElkNode = { + id: "root", + children: [ + { id: "node1", x: 0, y: 0 }, + { id: "node2", x: 200, y: 100 }, + ], + edges: [ + { + id: "edge1", + sources: ["node1"], + targets: ["node2"], + sections: [ + { + id: "section1", + startPoint: { x: 0, y: 0 }, + endPoint: { x: 200, y: 100 }, + bendPoints: [{ x: 100, y: 50 }], + }, + ], + }, + ], + }; + + const result = matchReactFlowGraphWithElkLayoutedGraph(reactFlowGraph, layoutedElkGraph); + + expect(result.edges[0].data).toEqual({ + label: "Test", + color: "blue", + customProp: 123, + wayPoints: [{ x: 100, y: 50 }], + }); + }); }); describe("applyAutoLayout", () => { @@ -651,6 +921,40 @@ describe("autoLayout", () => { expect(result.nodes[2].position).toEqual({ x: 350, y: 50 }); }); + it("passes abort signal to processElkLayout", async () => { + const reactFlowGraph: ReactFlowGraph = { + nodes: [{ id: "node1", position: { x: 0, y: 0 }, data: {} }] as Node[], + edges: [], + }; + + const layoutedElkGraph: ElkNode = { + id: "root", + children: [{ id: "node1", x: 50, y: 100 }], + edges: [], + }; + + vi.mocked(core.processElkLayout).mockResolvedValue(layoutedElkGraph); + + const abortController = new AbortController(); + await applyAutoLayout(reactFlowGraph, abortController.signal); + + expect(core.processElkLayout).toHaveBeenCalledWith( + expect.any(Object), + abortController.signal, + ); + }); + + it("handles processElkLayout rejection gracefully", async () => { + const reactFlowGraph: ReactFlowGraph = { + nodes: [{ id: "node1", position: { x: 10, y: 20 }, data: {} }] as Node[], + edges: [], + }; + + vi.mocked(core.processElkLayout).mockRejectedValue(new Error("Layout failed")); + + await expect(applyAutoLayout(reactFlowGraph)).rejects.toThrow("Layout failed"); + }); + describe("buildElkNodeMap helper", () => { it("handles nested nodes correctly in matchReactFlowGraphWithElkLayoutedGraph", () => { const reactFlowGraph: ReactFlowGraph = { @@ -806,15 +1110,28 @@ describe("autoLayout", () => { describe("ROOT_LAYOUT_OPTIONS", () => { it("contains required ELK layout options", () => { - expect(ROOT_LAYOUT_OPTIONS["elk.algorithm"]).toBe("org.eclipse.elk.layered"); - expect(ROOT_LAYOUT_OPTIONS["elk.direction"]).toBe("DOWN"); - expect(ROOT_LAYOUT_OPTIONS["elk.hierarchyHandling"]).toBe("INCLUDE_CHILDREN"); + expect(ROOT_LAYOUT_OPTIONS["org.eclipse.elk.algorithm"]).toBe("org.eclipse.elk.layered"); + expect(ROOT_LAYOUT_OPTIONS["org.eclipse.elk.direction"]).toBe("DOWN"); + expect(ROOT_LAYOUT_OPTIONS["org.eclipse.elk.hierarchyHandling"]).toBe("INCLUDE_CHILDREN"); }); it("has proper spacing configuration", () => { - expect(ROOT_LAYOUT_OPTIONS["spacing"]).toBe("75"); - expect(ROOT_LAYOUT_OPTIONS["spacing.componentComponent"]).toBe("70"); - expect(ROOT_LAYOUT_OPTIONS["spacing.nodeNodeBetweenLayers"]).toBe("80"); + expect(ROOT_LAYOUT_OPTIONS["org.eclipse.elk.layered.spacing.edgeNode"]).toBe("24"); + expect(ROOT_LAYOUT_OPTIONS["org.eclipse.elk.layered.spacing.componentComponent"]).toBe( + "70", + ); + expect(ROOT_LAYOUT_OPTIONS["org.eclipse.elk.layered.spacing.nodeNodeBetweenLayers"]).toBe( + "80", + ); + }); + }); + + describe("PARENT_LAYOUT_OPTIONS", () => { + it("extends ROOT_LAYOUT_OPTIONS with padding", () => { + expect(PARENT_LAYOUT_OPTIONS["org.eclipse.elk.padding"]).toBe( + "[top=60,left=20,bottom=20,right=20]", + ); + expect(PARENT_LAYOUT_OPTIONS["org.eclipse.elk.algorithm"]).toBe("org.eclipse.elk.layered"); }); }); }); diff --git a/packages/serverless-workflow-diagram-editor/tests/react-flow/diagram/diagramBuilder.test.ts b/packages/serverless-workflow-diagram-editor/tests/react-flow/diagram/diagramBuilder.test.ts index 2c7e0d38..51f9e7cc 100644 --- a/packages/serverless-workflow-diagram-editor/tests/react-flow/diagram/diagramBuilder.test.ts +++ b/packages/serverless-workflow-diagram-editor/tests/react-flow/diagram/diagramBuilder.test.ts @@ -16,7 +16,7 @@ import { describe, it, expect, beforeAll } from "vitest"; import * as RF from "@xyflow/react"; -import { FlatGraphNode, GraphNodeType } from "@serverlessworkflow/sdk"; +import { FlatGraphNode, GraphNodeType, Specification } from "@serverlessworkflow/sdk"; import { getEdgeType, edgeSourceAndTargetExist, @@ -24,6 +24,7 @@ import { getCatchContainerNodeIds, } from "../../../src/react-flow/diagram/diagramBuilder"; import { EdgeTypes } from "../../../src/react-flow/edges/Edges"; +import { CATCH_CONTAINER_NODE_TYPE } from "../../../src/react-flow/nodes/Nodes"; import { parseWorkflow } from "../../../src/core"; import { BASIC_VALID_WORKFLOW_JSON, @@ -279,6 +280,15 @@ describe("diagramBuilder", () => { expect(diagram.nodes).toEqual([]); expect(diagram.edges).toEqual([]); }); + + it("returns ReactFlowGraph with correct structure", () => { + const diagram: DiagramElements = buildDiagramFromWorkflow(BASIC_VALID_WORKFLOW_JSON); + + expect(diagram).toHaveProperty("nodes"); + expect(diagram).toHaveProperty("edges"); + expect(Array.isArray(diagram.nodes)).toBe(true); + expect(Array.isArray(diagram.edges)).toBe(true); + }); }); describe("node properties", () => { @@ -355,6 +365,239 @@ describe("diagramBuilder", () => { const uniqueIds = new Set(edgeIds); expect(uniqueIds.size).toBe(edgeIds.length); }); + + it("sets animated property correctly for error edges", () => { + // Create a workflow with a raise node to generate error edges + const workflowWithRaise = JSON.stringify({ + document: { + dsl: "1.0.3", + name: "workflow-with-raise", + version: "1.0.0", + namespace: "default", + }, + do: [ + { + raiseError: { + raise: { + error: { + type: "https://example.com/errors/test", + status: 500, + }, + }, + }, + }, + ], + }); + + const diagram = buildDiagramFromWorkflow(workflowWithRaise); + const errorEdges = diagram.edges.filter((edge) => edge.type === EdgeTypes.Error); + + errorEdges.forEach((edge) => { + expect(edge.animated).toBe(true); + }); + }); + + it("does not create edges for non-existent nodes", () => { + const diagram: DiagramElements = buildDiagramFromWorkflow(BASIC_VALID_WORKFLOW_JSON_TASKS); + const nodeIdSet = new Set(diagram.nodes.map((node) => node.id)); + + // All edges should reference existing nodes + diagram.edges.forEach((edge) => { + expect(nodeIdSet.has(edge.source)).toBe(true); + expect(nodeIdSet.has(edge.target)).toBe(true); + }); + }); + }); + + describe("parent-child relationships", () => { + it("sets parentId for child nodes", () => { + const workflowWithNesting = JSON.stringify({ + document: { + dsl: "1.0.3", + name: "workflow-with-nesting", + version: "1.0.0", + namespace: "default", + }, + do: [ + { + tryBlock: { + try: [ + { + step1: { + set: { + variable: "nested task", + }, + }, + }, + ], + catch: { + errors: { + with: { + type: "https://example.com/errors/test", + }, + }, + do: [ + { + recover: { + set: { + variable: "recovery", + }, + }, + }, + ], + }, + }, + }, + ], + }); + + const diagram = buildDiagramFromWorkflow(workflowWithNesting); + const childNodes = diagram.nodes.filter( + (node) => node.parentId && node.parentId !== "root", + ); + + expect(childNodes.length).toBeGreaterThan(0); + childNodes.forEach((node) => { + expect(node.parentId).toBeDefined(); + expect(node.extent).toBe("parent"); + }); + }); + + it("does not set parentId for root-level nodes", () => { + const diagram: DiagramElements = buildDiagramFromWorkflow(BASIC_VALID_WORKFLOW_JSON_TASKS); + const rootNodes = diagram.nodes.filter( + (node) => !node.parentId || node.parentId === "root", + ); + + expect(rootNodes.length).toBeGreaterThan(0); + rootNodes.forEach((node) => { + expect(node.extent).toBeUndefined(); + }); + }); + }); + + describe("catch container nodes", () => { + it("creates catch container nodes for catch blocks with children", () => { + const workflowWithCatch = JSON.stringify({ + document: { + dsl: "1.0.3", + name: "workflow-with-catch", + version: "1.0.0", + namespace: "default", + }, + do: [ + { + tryBlock: { + try: [ + { + step1: { + set: { + variable: "task", + }, + }, + }, + ], + catch: { + errors: { + with: { + type: "https://example.com/errors/test", + }, + }, + do: [ + { + recover: { + set: { + variable: "recovery", + }, + }, + }, + ], + }, + }, + }, + ], + }); + + const diagram = buildDiagramFromWorkflow(workflowWithCatch); + const catchContainerNodes = diagram.nodes.filter( + (node) => node.type === CATCH_CONTAINER_NODE_TYPE, + ); + + expect(catchContainerNodes.length).toBeGreaterThan(0); + }); + + it("does not create catch container nodes for leaf catch blocks", () => { + const workflowWithLeafCatch = JSON.stringify({ + document: { + dsl: "1.0.3", + name: "workflow-with-leaf-catch", + version: "1.0.0", + namespace: "default", + }, + do: [ + { + tryBlock: { + try: [ + { + step1: { + set: { + variable: "task", + }, + }, + }, + ], + catch: { + errors: { + with: { + type: "https://example.com/errors/test", + }, + }, + }, + }, + }, + ], + }); + + const diagram = buildDiagramFromWorkflow(workflowWithLeafCatch); + const catchNodes = diagram.nodes.filter((node) => node.type === GraphNodeType.Catch); + + // Leaf catch nodes should use GraphNodeType.Catch, not CATCH_CONTAINER_NODE_TYPE + catchNodes.forEach((node) => { + expect(node.type).toBe(GraphNodeType.Catch); + expect(node.type).not.toBe(CATCH_CONTAINER_NODE_TYPE); + }); + }); + }); + + describe("task data preservation", () => { + it("preserves task data in node data", () => { + const diagram: DiagramElements = buildDiagramFromWorkflow(BASIC_VALID_WORKFLOW_JSON_TASKS); + const taskNodes = diagram.nodes.filter((node) => node.data.task !== undefined); + + taskNodes.forEach((node) => { + expect(node.data.task).toBeDefined(); + expect(typeof node.data.task).toBe("object"); + }); + }); + + it("clones task data to prevent mutations", () => { + const parseResult = parseWorkflow(BASIC_VALID_WORKFLOW_JSON_TASKS); + const diagram1 = buildDiagramElements(parseResult.model); + const diagram2 = buildDiagramElements(parseResult.model); + + const taskNode1 = diagram1.nodes.find((node) => node.data.task !== undefined); + const taskNode2 = diagram2.nodes.find((node) => node.id === taskNode1?.id); + + // Ensure both nodes exist before testing clone behavior + expect(taskNode1).toBeDefined(); + expect(taskNode2).toBeDefined(); + + // Modify one task + (taskNode1!.data.task as Specification.Task).modified = true; + + // The other should not be affected + expect((taskNode2!.data.task as Specification.Task).modified).toBeUndefined(); + }); }); describe("graph structure", () => { @@ -363,6 +606,38 @@ describe("diagramBuilder", () => { expect(diagram.nodes.length).toBeGreaterThan(0); }); + + it("maintains correct node-edge relationships", () => { + const diagram: DiagramElements = buildDiagramFromWorkflow(BASIC_VALID_WORKFLOW_JSON_TASKS); + const nodeIdSet = new Set(diagram.nodes.map((node) => node.id)); + + // Every edge should connect existing nodes + diagram.edges.forEach((edge) => { + expect(nodeIdSet.has(edge.source)).toBe(true); + expect(nodeIdSet.has(edge.target)).toBe(true); + }); + }); + + it("creates connected graph structure", () => { + const diagram: DiagramElements = buildDiagramFromWorkflow(BASIC_VALID_WORKFLOW_JSON_TASKS); + + // Should have at least one edge connecting nodes + expect(diagram.edges.length).toBeGreaterThan(0); + + // All non-terminal nodes should have outgoing edges + const nodesWithOutgoingEdges = new Set(diagram.edges.map((edge) => edge.source)); + const terminalNodeIds = new Set( + diagram.nodes + .filter((node) => node.type === GraphNodeType.End || node.type === GraphNodeType.Exit) + .map((node) => node.id), + ); + + diagram.nodes.forEach((node) => { + if (!terminalNodeIds.has(node.id)) { + expect(nodesWithOutgoingEdges.has(node.id)).toBe(true); + } + }); + }); }); }); @@ -499,5 +774,92 @@ describe("diagramBuilder", () => { expect(result.has("/do/0/TryCatch/catch")).toBe(true); expect(result.has("/do/0/TryCatch")).toBe(false); }); + + it("should handle multiple catch containers at the same level", () => { + const sdkGraph = createFlatGraph( + [ + { + id: "start", + type: GraphNodeType.Start, + } as FlatGraphNode, + { + id: "end", + type: GraphNodeType.End, + } as FlatGraphNode, + { + id: "/do/0/Catch1", + type: GraphNodeType.Catch, + label: "Catch1", + } as FlatGraphNode, + { + id: "/do/0/Catch1/do/0/step1", + type: GraphNodeType.Set, + label: "step1", + parentId: "/do/0/Catch1", + } as FlatGraphNode, + { + id: "/do/0/Catch2", + type: GraphNodeType.Catch, + label: "Catch2", + } as FlatGraphNode, + { + id: "/do/0/Catch2/do/0/step2", + type: GraphNodeType.Set, + label: "step2", + parentId: "/do/0/Catch2", + } as FlatGraphNode, + ], + [], + ); + + const result = getCatchContainerNodeIds(sdkGraph); + + expect(result.has("/do/0/Catch1")).toBe(true); + expect(result.has("/do/0/Catch2")).toBe(true); + expect(result.size).toBe(2); + }); + + it("should return empty set for graph with no nodes", () => { + const sdkGraph = createFlatGraph([], []); + + const result = getCatchContainerNodeIds(sdkGraph); + + expect(result.size).toBe(0); + }); + + it("should handle deeply nested catch containers", () => { + const sdkGraph = createFlatGraph( + [ + { + id: "start", + type: GraphNodeType.Start, + } as FlatGraphNode, + { + id: "/do/0/OuterCatch", + type: GraphNodeType.Catch, + label: "OuterCatch", + } as FlatGraphNode, + { + id: "/do/0/OuterCatch/do/0/InnerCatch", + type: GraphNodeType.Catch, + label: "InnerCatch", + parentId: "/do/0/OuterCatch", + } as FlatGraphNode, + { + id: "/do/0/OuterCatch/do/0/InnerCatch/do/0/step", + type: GraphNodeType.Set, + label: "step", + parentId: "/do/0/OuterCatch/do/0/InnerCatch", + } as FlatGraphNode, + ], + [], + ); + + const result = getCatchContainerNodeIds(sdkGraph); + + expect(result.has("/do/0/OuterCatch")).toBe(true); + expect(result.has("/do/0/OuterCatch/do/0/InnerCatch")).toBe(true); + expect(result.size).toBe(2); + }); }); }); diff --git a/packages/serverless-workflow-diagram-editor/vitest.config.ts b/packages/serverless-workflow-diagram-editor/vitest.config.ts index b5cc3b6e..a868fa99 100644 --- a/packages/serverless-workflow-diagram-editor/vitest.config.ts +++ b/packages/serverless-workflow-diagram-editor/vitest.config.ts @@ -32,10 +32,17 @@ export default defineConfig({ test: { globals: true, environment: "jsdom", - setupFiles: ["./tests/setupTests.ts", "./.storybook/vitest.setup.ts"], - css: true, - include: ["tests/**/*.test.ts", "tests/**/*.test.tsx"], + setupFiles: ["./tests/setupTests.ts"], projects: [ + { + extends: true, + test: { + name: "unit", + css: true, + include: ["tests/**/*.test.ts", "tests/**/*.test.tsx"], + exclude: ["tests/**/*.story.test.ts", "tests/**/*.story.test.tsx"], + }, + }, { extends: true, plugins: [ @@ -51,9 +58,6 @@ export default defineConfig({ provider: playwright({}), instances: [{ browser: "chromium" }], }, - globals: true, - environment: "jsdom", - setupFiles: ["./tests/setupTests.ts", "./.storybook/vitest.setup.ts"], }, }, ],