diff --git a/src/components/shared/ReactFlow/FlowCanvas/utils/addAndConnectNode.test.ts b/src/components/shared/ReactFlow/FlowCanvas/utils/addAndConnectNode.test.ts new file mode 100644 index 000000000..bb9ab26dc --- /dev/null +++ b/src/components/shared/ReactFlow/FlowCanvas/utils/addAndConnectNode.test.ts @@ -0,0 +1,327 @@ +import type { Handle, Position } from "@xyflow/react"; +import { beforeEach, describe, expect, test, vi } from "vitest"; + +import type { NodeManager } from "@/nodeManager"; +import { + type ComponentReference, + type ComponentSpec, + isGraphImplementation, +} from "@/utils/componentSpec"; + +import { addAndConnectNode } from "./addAndConnectNode"; +import { handleConnection } from "./handleConnection"; + +// Mock the dependencies +vi.mock("./addTask", () => ({ + default: vi.fn((_taskType, taskSpec, _position, componentSpec) => { + const newTaskId = `new-task-${Date.now()}`; + return { + ...componentSpec, + implementation: { + ...componentSpec.implementation, + graph: { + ...componentSpec.implementation.graph, + tasks: { + ...componentSpec.implementation.graph.tasks, + [newTaskId]: taskSpec, + }, + }, + }, + }; + }), +})); + +vi.mock("./handleConnection", () => ({ + handleConnection: vi.fn((graph) => graph), // Just return the graph unchanged for testing +})); + +describe("addAndConnectNode", () => { + const mockNodeManager: NodeManager = { + getHandleInfo: vi.fn(), + getNodeId: vi.fn(), + getHandleNodeId: vi.fn(), + } as any; + + const mockComponentRef: ComponentReference = { + spec: { + name: "TestComponent", + inputs: [{ name: "input1", type: "string" }], + outputs: [{ name: "output1", type: "string" }], + implementation: { + container: { image: "test", command: ["echo"] }, + }, + }, + }; + + const mockComponentSpec: ComponentSpec = { + name: "Pipeline", + inputs: [{ name: "pipelineInput", type: "string" }], + outputs: [{ name: "pipelineOutput", type: "string" }], + implementation: { + graph: { + tasks: { + "existing-task": { + annotations: {}, + componentRef: { + spec: { + name: "ExistingTask", + inputs: [{ name: "taskInput", type: "string" }], + outputs: [{ name: "taskOutput", type: "string" }], + implementation: { + container: { image: "existing", command: ["run"] }, + }, + }, + }, + }, + }, + }, + }, + }; + + const createMockHandle = ( + id: string | null | undefined, + nodeId: string, + ): Handle => ({ + id, + nodeId, + x: 0, + y: 0, + position: "top" as Position, + type: "source", + width: 10, + height: 10, + }); + + beforeEach(() => { + vi.clearAllMocks(); + }); + + test("should return unchanged spec when implementation is not a graph", () => { + const nonGraphSpec: ComponentSpec = { + name: "NonGraph", + implementation: { + container: { image: "test", command: ["echo"] }, + }, + }; + + const result = addAndConnectNode({ + componentRef: mockComponentRef, + fromHandle: createMockHandle("handle1", "node1"), + position: { x: 100, y: 100 }, + componentSpec: nonGraphSpec, + nodeManager: mockNodeManager, + }); + + expect(result).toBe(nonGraphSpec); + }); + + test("should return unchanged spec when fromHandle has no id", () => { + const result = addAndConnectNode({ + componentRef: mockComponentRef, + fromHandle: createMockHandle(null, "node1"), + position: { x: 100, y: 100 }, + componentSpec: mockComponentSpec, + nodeManager: mockNodeManager, + }); + + expect(result).toBe(mockComponentSpec); + }); + + test("should return unchanged spec when fromHandle has empty string id", () => { + const result = addAndConnectNode({ + componentRef: mockComponentRef, + fromHandle: createMockHandle("", "node1"), + position: { x: 100, y: 100 }, + componentSpec: mockComponentSpec, + nodeManager: mockNodeManager, + }); + + expect(result).toBe(mockComponentSpec); + }); + + test("should return unchanged spec when fromHandle has undefined id", () => { + const result = addAndConnectNode({ + componentRef: mockComponentRef, + fromHandle: createMockHandle(undefined, "node1"), + position: { x: 100, y: 100 }, + componentSpec: mockComponentSpec, + nodeManager: mockNodeManager, + }); + + expect(result).toBe(mockComponentSpec); + }); + + test("should return unchanged spec when handle info is invalid", () => { + vi.mocked(mockNodeManager.getHandleInfo).mockReturnValue(undefined); + + const result = addAndConnectNode({ + componentRef: mockComponentRef, + fromHandle: createMockHandle("handle1", "node1"), + position: { x: 100, y: 100 }, + componentSpec: mockComponentSpec, + nodeManager: mockNodeManager, + }); + + expect(result).toBe(mockComponentSpec); + }); + + test("should return unchanged spec when handle type is invalid", () => { + vi.mocked(mockNodeManager.getHandleInfo).mockReturnValue({ + handleType: "invalidType" as any, + parentRefId: "task1", + handleName: "output1", + }); + + const result = addAndConnectNode({ + componentRef: mockComponentRef, + fromHandle: createMockHandle("handle1", "node1"), + position: { x: 100, y: 100 }, + componentSpec: mockComponentSpec, + nodeManager: mockNodeManager, + }); + + expect(result).toBe(mockComponentSpec); + }); + + test("should add task and attempt connection from output handle", () => { + vi.mocked(mockNodeManager.getHandleInfo).mockReturnValue({ + handleType: "handle-out", + parentRefId: "existing-task", + handleName: "taskOutput", + }); + vi.mocked(mockNodeManager.getNodeId).mockReturnValue("new-node-id"); + vi.mocked(mockNodeManager.getHandleNodeId).mockReturnValue("new-handle-id"); + + const result = addAndConnectNode({ + componentRef: mockComponentRef, + fromHandle: createMockHandle("from-handle", "from-node"), + position: { x: 100, y: 100 }, + componentSpec: mockComponentSpec, + nodeManager: mockNodeManager, + }); + + // Should have added a new task + const graphSpec = + isGraphImplementation(result.implementation) && + result.implementation.graph; + if (!graphSpec) { + throw new Error("Resulting implementation is not a graph"); + } + const tasks = graphSpec?.tasks; + expect(Object.keys(tasks)).toHaveLength(2); + + // Verify handleConnection was called + expect(handleConnection).toHaveBeenCalled(); + }); + + test("should add task and attempt connection from input handle", () => { + vi.mocked(mockNodeManager.getHandleInfo).mockReturnValue({ + handleType: "handle-in", + parentRefId: "existing-task", + handleName: "taskInput", + }); + vi.mocked(mockNodeManager.getNodeId).mockReturnValue("new-node-id"); + vi.mocked(mockNodeManager.getHandleNodeId).mockReturnValue("new-handle-id"); + + const result = addAndConnectNode({ + componentRef: mockComponentRef, + fromHandle: createMockHandle("from-handle", "from-node"), + position: { x: 100, y: 100 }, + componentSpec: mockComponentSpec, + nodeManager: mockNodeManager, + }); + + // Should have added a new task + const graphSpec = + isGraphImplementation(result.implementation) && + result.implementation.graph; + if (!graphSpec) { + throw new Error("Resulting implementation is not a graph"); + } + const tasks = graphSpec?.tasks; + expect(Object.keys(tasks)).toHaveLength(2); + + // Verify handleConnection was called + expect(handleConnection).toHaveBeenCalled(); + }); + + test("should add task but skip connection when no compatible handle found", () => { + vi.mocked(mockNodeManager.getHandleInfo).mockReturnValue({ + handleType: "handle-out", + parentRefId: "existing-task", + handleName: "taskOutput", + }); + + const incompatibleComponentRef: ComponentReference = { + spec: { + name: "IncompatibleComponent", + inputs: [{ name: "input1", type: "number" }], + outputs: [{ name: "output1", type: "boolean" }], + implementation: { + container: { image: "test", command: ["echo"] }, + }, + }, + }; + + const result = addAndConnectNode({ + componentRef: incompatibleComponentRef, + fromHandle: createMockHandle("from-handle", "from-node"), + position: { x: 100, y: 100 }, + componentSpec: mockComponentSpec, + nodeManager: mockNodeManager, + }); + + // Should still add the task even without connection + const graphSpec = + isGraphImplementation(result.implementation) && + result.implementation.graph; + if (!graphSpec) { + throw new Error("Resulting implementation is not a graph"); + } + const tasks = graphSpec?.tasks; + expect(Object.keys(tasks)).toHaveLength(2); + + // Should not call handleConnection when no compatible handle + expect(handleConnection).not.toHaveBeenCalled(); + }); + + test("should handle pipeline-level input/output handles", () => { + vi.mocked(mockNodeManager.getHandleInfo).mockReturnValue({ + handleType: "handle-in", + parentRefId: "pipeline", // Pipeline-level handle + handleName: "pipelineInput", + }); + vi.mocked(mockNodeManager.getNodeId).mockReturnValue("new-node-id"); + vi.mocked(mockNodeManager.getHandleNodeId).mockReturnValue("new-handle-id"); + + const result = addAndConnectNode({ + componentRef: mockComponentRef, + fromHandle: createMockHandle("from-handle", "from-node"), + position: { x: 100, y: 100 }, + componentSpec: mockComponentSpec, + nodeManager: mockNodeManager, + }); + + // Should have added a new task + const graphSpec = + isGraphImplementation(result.implementation) && + result.implementation.graph; + if (!graphSpec) { + throw new Error("Resulting implementation is not a graph"); + } + const tasks = graphSpec?.tasks; + expect(Object.keys(tasks)).toHaveLength(2); + }); + + test("should return unchanged spec when fromHandle is null", () => { + const result = addAndConnectNode({ + componentRef: mockComponentRef, + fromHandle: null, + position: { x: 100, y: 100 }, + componentSpec: mockComponentSpec, + nodeManager: mockNodeManager, + }); + + expect(result).toBe(mockComponentSpec); + }); +}); diff --git a/src/components/shared/ReactFlow/FlowCanvas/utils/addAndConnectNode.ts b/src/components/shared/ReactFlow/FlowCanvas/utils/addAndConnectNode.ts index b73772b50..083ae3337 100644 --- a/src/components/shared/ReactFlow/FlowCanvas/utils/addAndConnectNode.ts +++ b/src/components/shared/ReactFlow/FlowCanvas/utils/addAndConnectNode.ts @@ -1,6 +1,6 @@ import type { Connection, Handle } from "@xyflow/react"; -import type { NodeManager, NodeType } from "@/nodeManager"; +import type { HandleInfo, NodeManager, NodeType } from "@/nodeManager"; import { type ComponentReference, type ComponentSpec, @@ -21,6 +21,15 @@ type AddAndConnectNodeParams = { nodeManager: NodeManager; }; +/* + Add a new task node to the graph and connect it to an existing handle + - ComponentRef: The component reference for the new task + - fromHandle: The handle from which the connection originates (input or output handle) + - position: The position to place the new task node + - componentSpec: The current component specification containing the graph + - nodeManager: The NodeManager instance for managing node IDs +*/ + export function addAndConnectNode({ componentRef, fromHandle, @@ -28,58 +37,27 @@ export function addAndConnectNode({ componentSpec, nodeManager, }: AddAndConnectNodeParams): ComponentSpec { - // 1. Add the new node - const taskSpec: TaskSpec = { - annotations: {}, - componentRef: { ...componentRef }, - }; - - if (!isGraphImplementation(componentSpec.implementation)) { + if (!isGraphImplementation(componentSpec.implementation) || !fromHandle?.id) { return componentSpec; } - const oldGraphSpec = componentSpec.implementation.graph; - - if (!fromHandle?.id) { - return componentSpec; - } - - const fromNodeId = fromHandle.nodeId; - const fromNodeType = nodeManager.getNodeType(fromNodeId); - const fromTaskId = nodeManager.getRefId(fromNodeId); - - if (!fromTaskId) { - return componentSpec; - } - - let fromHandleType: NodeType | undefined; - let fromHandleName: string | undefined; - - if (fromNodeType === "task") { - const fromHandleInfo = nodeManager.getHandleInfo(fromHandle.id); - fromHandleName = fromHandleInfo?.handleName; - fromHandleType = nodeManager.getNodeType(fromHandle.id); - } else if (fromNodeType === "input") { - fromHandleType = "handle-out"; - fromHandleName = fromTaskId; - } else if (fromNodeType === "output") { - fromHandleType = "handle-in"; - fromHandleName = fromTaskId; - } else { - return componentSpec; - } - - if (!fromHandleName) { - return componentSpec; - } + const handleInfo = nodeManager.getHandleInfo(fromHandle.id); + const fromHandleType = handleInfo?.handleType; if ( + !handleInfo || !fromHandleType || - (fromHandleType !== "handle-in" && fromHandleType !== "handle-out") + !["handle-in", "handle-out"].includes(fromHandleType) ) { return componentSpec; } + // 1. Create new task + const newTaskSpec: TaskSpec = { + annotations: {}, + componentRef: { ...componentRef }, + }; + const adjustedPosition = fromHandleType === "handle-in" ? { ...position, x: position.x - DEFAULT_NODE_DIMENSIONS.w } @@ -87,114 +65,117 @@ export function addAndConnectNode({ const newComponentSpec = addTask( "task", - taskSpec, + newTaskSpec, adjustedPosition, componentSpec, ); - // 2. Find the new node if (!isGraphImplementation(newComponentSpec.implementation)) { - return newComponentSpec; - } - - const graphSpec = newComponentSpec.implementation.graph; - - const newTaskId = Object.keys(graphSpec.tasks).find( - (key) => !(key in oldGraphSpec.tasks), - ); - - if (!newTaskId) { - return newComponentSpec; + return componentSpec; } - const newNodeId = nodeManager.getNodeId(newTaskId, "task"); - - // 3. Determine the connection data type and find the first matching handle on the new node - let fromComponentSpec: ComponentSpec | undefined; + const oldTasks = componentSpec.implementation.graph.tasks; + const newTasks = newComponentSpec.implementation.graph.tasks; + const newTaskId = Object.keys(newTasks).find((key) => !(key in oldTasks)); - if (fromNodeType === "task") { - // Get spec from task - const fromTaskSpec = graphSpec.tasks[fromTaskId]; - fromComponentSpec = fromTaskSpec?.componentRef.spec; - } else { - // For IO nodes, get spec from component spec - fromComponentSpec = componentSpec; - } + if (!newTaskId) return newComponentSpec; - let connectionType: TypeSpecType | undefined; - if (fromHandleType === "handle-in") { - connectionType = fromComponentSpec?.inputs?.find( - (io) => io.name === fromHandleName, - )?.type; - } else if (fromHandleType === "handle-out") { - connectionType = fromComponentSpec?.outputs?.find( - (io) => io.name === fromHandleName, - )?.type; - } + // 2. Find the first matching handle on the new task + const connectionType = getConnectionType( + handleInfo, + fromHandleType, + componentSpec, + newTasks, + ); + if (!connectionType) return newComponentSpec; - // Find the first matching handle on the new node const toHandleType = fromHandleType === "handle-in" ? "handle-out" : "handle-in"; + const toHandleName = findCompatibleHandle( + componentRef, + toHandleType, + connectionType, + ); - const inputHandleName = componentRef.spec?.inputs?.find( - (io) => io.type === connectionType, - )?.name; - - const outputHandleName = componentRef.spec?.outputs?.find( - (io) => io.type === connectionType, - )?.name; - - const toHandleName = - toHandleType === "handle-in" ? inputHandleName : outputHandleName; - - if (!toHandleName) { - return newComponentSpec; - } + if (!toHandleName) return newComponentSpec; + // 3. Create connection + const newNodeId = nodeManager.getNodeId(newTaskId, "task"); const targetHandleId = nodeManager.getHandleNodeId( newTaskId, toHandleName, toHandleType, ); + const isReversed = + fromHandleType === "handle-in" && toHandleType === "handle-out"; + + const connection: Connection = isReversed + ? { + source: newNodeId, + sourceHandle: targetHandleId, + target: fromHandle.nodeId, + targetHandle: fromHandle.id, + } + : { + source: fromHandle.nodeId, + sourceHandle: fromHandle.id, + target: newNodeId, + targetHandle: targetHandleId, + }; + + const updatedGraphSpec = handleConnection( + newComponentSpec.implementation.graph, + connection, + nodeManager, + ); - // 4. Build a Connection object and use handleConnection to add the edge - if (targetHandleId) { - const fromNodeId = fromHandle.nodeId; - const fromHandleId = fromHandle.id; - - const isReversedConnection = - fromHandleType === "handle-in" && toHandleType === "handle-out"; - - const connection: Connection = isReversedConnection - ? // Drawing from an input handle to a new output handle - { - source: newNodeId, - sourceHandle: targetHandleId, - target: fromNodeId, - targetHandle: fromHandleId, - } - : // Drawing from an output handle to a new input handle - { - source: fromNodeId, - sourceHandle: fromHandleId, - target: newNodeId, - targetHandle: targetHandleId, - }; - - const updatedGraphSpec = handleConnection( - graphSpec, - connection, - nodeManager, - ); - - return { - ...newComponentSpec, - implementation: { - ...newComponentSpec.implementation, - graph: updatedGraphSpec, - }, - }; + return { + ...newComponentSpec, + implementation: { + ...newComponentSpec.implementation, + graph: updatedGraphSpec, + }, + }; +} + +function getConnectionType( + handleInfo: HandleInfo, + handleType: NodeType, + componentSpec: ComponentSpec, + tasks: Record, +): TypeSpecType | undefined { + let targetSpec: ComponentSpec | undefined; + + const taskSpec = tasks[handleInfo.parentRefId]; + + if (taskSpec) { + targetSpec = taskSpec.componentRef.spec; + } else { + targetSpec = componentSpec; + } + + if (!targetSpec) return undefined; + + if (handleType === "handle-in") { + return targetSpec.inputs?.find((io) => io.name === handleInfo.handleName) + ?.type; + } else { + return targetSpec.outputs?.find((io) => io.name === handleInfo.handleName) + ?.type; } +} - return newComponentSpec; +function findCompatibleHandle( + componentRef: ComponentReference, + handleType: "handle-in" | "handle-out", + connectionType: TypeSpecType, +): string | undefined { + const spec = componentRef.spec; + if (!spec) return undefined; + + if (handleType === "handle-in") { + return spec.inputs?.find((io) => io.type === connectionType)?.name; + } else { + return spec.outputs?.find((io) => io.type === connectionType)?.name; + } }