diff --git a/README.md b/README.md index 133f9a0..2d8c179 100644 --- a/README.md +++ b/README.md @@ -322,6 +322,42 @@ const flatGraph = buildFlatGraph(workflow); // const flatGraphWithoutPorts = buildFlatGraph(workflow, true); ``` +Task node IDs are pointer-shaped qualified paths built from task names, without `TaskList` array indexes. For +example, a task at `/do/0/step1` has the stable graph ID `/do/step1`. Inserting, removing, or reordering sibling +tasks therefore does not change the IDs of unaffected nodes. Renaming a task or moving it to another task list does +change its ID. Non-root entry and exit ports use a `port-` prefix so their IDs cannot collide with task IDs. Edge +IDs are derived from their endpoint node IDs only, so editing an `if` condition or renaming a `switch` case changes +an edge's `label` but not its identity. + +Each task node also exposes its exact indexed RFC 6901 pointer through `taskReference`. Use that property to +correlate a graph with runtime task references or validation pointers from the same workflow revision: + +```typescript +import { getNodeAtPointer, getNodeByTaskReference } from '@openworkflowspec/sdk'; + +const taskNode = getNodeByTaskReference(graph, '/do/0/step1'); +const invalidTaskNode = getNodeAtPointer(graph, '/do/0/step1/set/variable'); +``` + +`WorkflowValidationError.path` can be passed to `getNodeAtPointer`. For `SchemaValidationError`, pass the +`instancePath` from each entry in `schemaErrors`. + +If you maintain persistent task identities of your own — for example an editor that must keep node identity across +task renames — you can supply them through the `taskId` factory in the build options, accepted by `buildGraph`, +`buildFlatGraph` and `toGraph`: + +```typescript +const graph = buildGraph(workflow, { + taskId: ({ reference, defaultId }) => myIdsByPointer.get(reference) ?? defaultId, +}); +``` + +The factory receives the task, its name, its indexed `reference`, the containing graph's `parentId` and the +`defaultId` the builder would assign; returning `undefined` keeps the default. It is invoked at most once per task +per build, and the returned IDs must be unique — the build throws on duplicates instead of silently merging nodes. +IDs starting with `port-` and the root port IDs are reserved. Derived IDs compose on the custom ID: a renamed +container's ports and children keep default IDs under it (e.g. `my-id/do/child`). + #### Generate a MermaidJS flowchart Generating a [MermaidJS](https://mermaid.js.org/) flowchart can be achieved in two ways: using the `convertToMermaidCode`, the legacy `MermaidDiagram` class, or alternatives: @@ -355,11 +391,11 @@ const mermaidCode = convertToMermaidCode(workflow); /* or */ // const mermaidCode = Classes.Workflow.toMermaidCode(workflow); /* flowchart TD - root-entry-node(( )) - root-exit-node((( ))) - /do/0/step1["step1"] - /do/0/step1 --> root-exit-node - root-entry-node --> /do/0/step1 + n0(( )) + n1((( ))) + n2["step1"] + n2 --> n1 + n0 --> n2 classDef hidden width: 1px, height: 0px, opacity: 0; @@ -368,16 +404,20 @@ classDef hidden width: 1px, height: 0px, opacity: 0; ```mermaid flowchart TD - root-entry-node(( )) - root-exit-node((( ))) - /do/0/step1["step1"] - /do/0/step1 --> root-exit-node - root-entry-node --> /do/0/step1 + n0(( )) + n1((( ))) + n2["step1"] + n2 --> n1 + n0 --> n2 classDef hidden width: 1px, height: 0px, opacity: 0; ``` +Mermaid nodes are declared under renderer-local aliases (`n0`, `n1`, ...) rather than graph node IDs, so the +diagram stays valid whatever characters task names contain. Use the graph API (`buildGraph`, `buildFlatGraph`) +when you need addressable node identities. + You can refer to the Mermaid browser examples for a live demo. The browser samples under [`examples/browser`](./examples/browser) include native ESM examples and legacy UMD-global examples under [`examples/browser/umd`](./examples/browser/umd). ### Building Locally diff --git a/package-lock.json b/package-lock.json index e78f1a2..a701bef 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@openworkflowspec/sdk", - "version": "1.0.3-alpha3", + "version": "1.0.3-alpha5", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@openworkflowspec/sdk", - "version": "1.0.3-alpha3", + "version": "1.0.3-alpha5", "license": "Apache-2.0", "dependencies": { "ajv": "^8.20.0", diff --git a/package.json b/package.json index d80a38d..131d092 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@openworkflowspec/sdk", - "version": "1.0.3-alpha4", + "version": "1.0.3-alpha5", "schemaVersion": "1.0.3", "supportedDslVersions": ">=1.0.0 <=1.0.3", "description": "Typescript SDK for Open Workflow Specification", diff --git a/src/lib/generated/classes/workflow.ts b/src/lib/generated/classes/workflow.ts index 4b62fc1..89aec63 100644 --- a/src/lib/generated/classes/workflow.ts +++ b/src/lib/generated/classes/workflow.ts @@ -33,7 +33,7 @@ import { getLifecycleHooks } from '../../lifecycle-hooks'; import { validate } from '../../validation'; import { isObject } from '../../utils'; import * as yaml from 'js-yaml'; -import { buildGraph, Graph } from '../../graph-builder'; +import { buildGraph, Graph, GraphBuildOptions } from '../../graph-builder'; import { convertToMermaidCode } from '../../mermaid-converter'; /** @@ -114,8 +114,8 @@ export class Workflow extends ObjectHydrator { return yaml.dump(normalized); } - static toGraph(model: Partial): Graph { - return buildGraph(model as unknown as WorkflowIntersection); + static toGraph(model: Partial, options?: GraphBuildOptions): Graph { + return buildGraph(model as unknown as WorkflowIntersection, options); } static toMermaidCode(model: Partial): string { @@ -134,10 +134,11 @@ export class Workflow extends ObjectHydrator { /** * Creates a directed graph representation of the workflow + * @param options The options used to customize how the graph is built, e.g. to provide custom node ids * @returns A directed graph of the workflow */ - toGraph(): Graph { - return Workflow.toGraph(this as unknown as WorkflowIntersection); + toGraph(options?: GraphBuildOptions): Graph { + return Workflow.toGraph(this as unknown as WorkflowIntersection, options); } /** @@ -169,9 +170,10 @@ export const _Workflow = Workflow as WorkflowConstructor & { /** * Creates a directed graph representation of the provided workflow * @param workflow The workflow to convert + * @param options The options used to customize how the graph is built, e.g. to provide custom node ids * @returns A directed graph of the provided workflow */ - toGraph(workflow: Partial): Graph; + toGraph(workflow: Partial, options?: GraphBuildOptions): Graph; /** * Generates the MermaidJS code corresponding to the provided workflow diff --git a/src/lib/graph-builder.ts b/src/lib/graph-builder.ts index 02212e5..e91b538 100644 --- a/src/lib/graph-builder.ts +++ b/src/lib/graph-builder.ts @@ -15,14 +15,15 @@ import { WaitTask, Workflow, } from './generated/definitions/specification'; +import { appendJsonPointerSegment } from './utils'; const entrySuffix = '-entry-node'; const exitSuffix = '-exit-node'; const rootId = 'root'; +const portPrefix = 'port-'; const doReference = '/do'; -const forReference = '/for'; const catchReference = '/catch'; const branchReference = '/fork/branches'; const tryReference = '/try'; @@ -73,6 +74,8 @@ export type GraphNode = GraphElement & { parent?: Graph; /** The related task */ task?: Task; + /** The task's indexed RFC 6901 JSON Pointer within the workflow document. */ + taskReference?: string; }; /** @@ -121,6 +124,37 @@ export type FlatGraph = FlatGraphNode & { exitNode?: GraphNode; }; +/** + * The information provided to a custom task id factory when resolving a task's node id. + */ +export type TaskIdFactoryContext = { + /** The task definition. */ + task: Task; + /** The task name. */ + name: string; + /** The task's indexed RFC 6901 JSON Pointer within the workflow document, e.g. `/do/0/init`. */ + reference: string; + /** The id of the graph the node will be added to (`root` for top-level tasks). */ + parentId: string; + /** The id the builder would assign by default, e.g. `/do/init`. */ + defaultId: string; +}; + +/** + * Options used to customize how a graph is built. + */ +export type GraphBuildOptions = { + /** + * Returns a custom node id for a task, or undefined to keep the default id. + * The factory is invoked at most once per task per build: the first resolution is cached + * and reused wherever the task is reached again. Returned ids must be unique across the + * whole graph, must not start with `port-` and must not be a root port id; the build + * throws otherwise. Derived ids (ports, try/catch subgraphs, child task lists) compose + * on the returned id, so children of a renamed container keep default ids under it. + */ + taskId?: (context: TaskIdFactoryContext) => string | undefined; +}; + /** * Context information used when processing tasks in a workflow graph. */ @@ -129,14 +163,24 @@ type TaskContext = { graph: Graph; /** The reference of the task list. */ taskListReference: string; + /** The index-free, pointer-shaped path of the task list. */ + taskListId: string; /** The list of sibling tasks. */ taskList: Map; /** The current task name. */ taskName?: string; /** The current reference. */ taskReference: string; + /** The current stable graph node id. */ + taskId: string; /** The ids of edges already visited */ knownEdges: GraphEdge[]; + /** The graph build options, if any. */ + options?: GraphBuildOptions; + /** The resolved node ids, mapped by task reference. */ + taskIdsByReference: Map; + /** The task references, mapped by resolved node id. */ + taskReferencesById: Map; }; /** @@ -192,6 +236,7 @@ function initGraph( task: Task | undefined = undefined, label: string | undefined = undefined, parent: Graph | undefined = undefined, + taskReference: string | undefined = undefined, ): Graph { const graph: Graph = { id, @@ -199,17 +244,18 @@ function initGraph( type, parent, task, + taskReference, nodes: [], edges: [], }; const entryNode: GraphNode = { type: id === rootId ? GraphNodeType.Start : GraphNodeType.Entry, - id: `${id}${entrySuffix}`, + id: id === rootId ? `${id}${entrySuffix}` : `${portPrefix}${id}${entrySuffix}`, parent: graph, }; const exitNode: GraphNode = { type: id === rootId ? GraphNodeType.End : GraphNodeType.Exit, - id: `${id}${exitSuffix}`, + id: id === rootId ? `${id}${exitSuffix}` : `${portPrefix}${id}${exitSuffix}`, parent: graph, }; graph.entryNode = entryNode; @@ -281,6 +327,52 @@ function getEndNode(graph: Graph): GraphNode { return rootGraph.exitNode; } +/** + * Resolves the node id of a task, delegating to the custom task id factory when one is provided. + * The first resolution per task reference is cached and reused: `buildTaskNode` dedups revisited + * tasks by id, so a task must resolve to the same id however it is reached. + * @param context The context the task is resolved in + * @param parent The graph the task's node will be added to + * @param task The task definition + * @param name The task name + * @param reference The task's indexed RFC 6901 JSON Pointer within the workflow document + * @param defaultId The id the builder assigns by default + * @returns The resolved node id + */ +function resolveTaskId( + context: TaskContext, + parent: Graph, + task: Task, + name: string, + reference: string, + defaultId: string, +): string { + const cached = context.taskIdsByReference.get(reference); + if (cached) return cached; + const customId = context.options?.taskId?.({ task, name, reference, parentId: parent.id, defaultId }); + if (customId != null) { + if (typeof customId !== 'string' || !customId.length) { + throw new Error(`The task id factory returned an invalid id for task '${reference}'`); + } + if ( + customId.startsWith(portPrefix) || + customId === rootId || + customId === `${rootId}${entrySuffix}` || + customId === `${rootId}${exitSuffix}` + ) { + throw new Error(`The task id factory returned the reserved id '${customId}' for task '${reference}'`); + } + } + const id = customId ?? defaultId; + const knownReference = context.taskReferencesById.get(id); + if (knownReference !== undefined && knownReference !== reference) { + throw new Error(`The task id '${id}' is assigned to both tasks '${knownReference}' and '${reference}'`); + } + context.taskIdsByReference.set(reference, id); + context.taskReferencesById.set(id, reference); + return id; +} + /** * Builds the provided transition from the source node * @param sourceNode The node to build the transition from @@ -290,9 +382,21 @@ function getEndNode(graph: Graph): GraphNode { function buildTransition(sourceNode: GraphNode | Graph, transition: TransitionInfo, context: TaskContext) { const exitAnchor = (sourceNode as Graph).exitNode || sourceNode; if (transition.index != -1) { + const taskReference = appendJsonPointerSegment( + appendJsonPointerSegment(context.taskListReference, transition.index), + transition.name, + ); const targetNode = buildTaskNode({ ...context, - taskReference: `${context.taskListReference}/${transition.index}/${transition.name}`, + taskId: resolveTaskId( + context, + context.graph, + transition.task!, + transition.name, + taskReference, + appendJsonPointerSegment(context.taskListId, transition.name), + ), + taskReference, taskName: transition.name, }); buildEdge( @@ -340,7 +444,7 @@ function buildTransitions(sourceNode: GraphNode | Graph, context: TaskContext) { * @returns A graph or node for the provided context */ function buildTaskNode(context: TaskContext): GraphNode | Graph { - const existingNode = context.graph.nodes.find((node) => node.id === context.taskReference); + const existingNode = context.graph.nodes.find((node) => node.id === context.taskId); if (existingNode) return existingNode; const task = context.taskList.get(context.taskName!); if (!task) throw new Error(`Unabled to find the task '${context.taskName}' in the current context`); @@ -368,9 +472,10 @@ function buildTaskNode(context: TaskContext): GraphNode | Graph { function buildGenericTaskNode(task: Task, type: GraphNodeType, context: TaskContext): GraphNode { const node: GraphNode = { task, + taskReference: context.taskReference, type, parent: context.graph, - id: context.taskReference, + id: context.taskId, label: context.taskName, }; context.graph.nodes.push(node); @@ -396,10 +501,18 @@ function buildCallTaskNode(task: CallTask, context: TaskContext): GraphNode { * @returns A graph for the provided task */ function buildDoTaskNode(task: DoTask, context: TaskContext): Graph { - const subgraph: Graph = initGraph(GraphNodeType.Do, context.taskReference, task, context.taskName, context.graph); + const subgraph: Graph = initGraph( + GraphNodeType.Do, + context.taskId, + task, + context.taskName, + context.graph, + context.taskReference, + ); const doContext: TaskContext = { ...context, graph: subgraph, + taskListId: context.taskId + doReference, taskListReference: context.taskReference + doReference, taskList: mapTasks(task.do), taskName: undefined, @@ -428,11 +541,19 @@ function buildEmitTaskNode(task: EmitTask, context: TaskContext): GraphNode { * @returns A graph for the provided task */ function buildForTaskNode(task: ForTask, context: TaskContext): Graph { - const subgraph: Graph = initGraph(GraphNodeType.For, context.taskReference, task, context.taskName, context.graph); + const subgraph: Graph = initGraph( + GraphNodeType.For, + context.taskId, + task, + context.taskName, + context.graph, + context.taskReference, + ); const forContext: TaskContext = { ...context, graph: subgraph, - taskListReference: subgraph.id + forReference + doReference, + taskListId: context.taskId + doReference, + taskListReference: context.taskReference + doReference, taskList: mapTasks(task.do), taskName: undefined, }; @@ -449,17 +570,36 @@ function buildForTaskNode(task: ForTask, context: TaskContext): Graph { * @returns A graph for the provided task */ function buildForkTaskNode(task: ForkTask, context: TaskContext): Graph { - const subgraph: Graph = initGraph(GraphNodeType.Fork, context.taskReference, task, context.taskName, context.graph); + const subgraph: Graph = initGraph( + GraphNodeType.Fork, + context.taskId, + task, + context.taskName, + context.graph, + context.taskReference, + ); for (let i = 0, c = task.fork?.branches.length || 0; i < c; i++) { const branchItem = task.fork?.branches[i]; if (!branchItem) continue; - const [branchName] = Object.entries(branchItem)[0]; + const [branchName, branchTask] = Object.entries(branchItem)[0]; + const branchListId = `${context.taskId}${branchReference}`; + const branchListReference = `${context.taskReference}${branchReference}`; + const branchTaskReference = appendJsonPointerSegment(appendJsonPointerSegment(branchListReference, i), branchName); const branchContext: TaskContext = { ...context, graph: subgraph, - taskListReference: `${context.taskReference}${branchReference}`, + taskListId: branchListId, + taskListReference: branchListReference, taskList: mapTasks([branchItem]), - taskReference: `${context.taskReference}${branchReference}/${i}/${branchName}`, + taskId: resolveTaskId( + context, + subgraph, + branchTask, + branchName, + branchTaskReference, + appendJsonPointerSegment(branchListId, branchName), + ), + taskReference: branchTaskReference, taskName: branchName, }; const branchNode = buildTaskNode(branchContext); @@ -547,14 +687,15 @@ function buildSwitchTaskNode(task: SwitchTask, context: TaskContext): GraphNode function buildTryCatchTaskNode(task: TryTask, context: TaskContext): Graph { const containerSubgraph: Graph = initGraph( GraphNodeType.TryCatch, - context.taskReference, + context.taskId, task, context.taskName, context.graph, + context.taskReference, ); const trySubgraph: Graph = initGraph( GraphNodeType.Try, - context.taskReference + tryReference, + context.taskId + tryReference, task, context.taskName + ' (try)', containerSubgraph, @@ -565,7 +706,8 @@ function buildTryCatchTaskNode(task: TryTask, context: TaskContext): Graph { const tryContext: TaskContext = { ...context, graph: trySubgraph, - taskListReference: trySubgraph.id, + taskListId: context.taskId + tryReference, + taskListReference: context.taskReference + tryReference, taskList: mapTasks(task.try), taskName: undefined, }; @@ -576,7 +718,7 @@ function buildTryCatchTaskNode(task: TryTask, context: TaskContext): Graph { task, type: GraphNodeType.Catch, parent: containerSubgraph, - id: context.taskReference + catchReference, + id: context.taskId + catchReference, label: context.taskName + ' (catch)', }; containerSubgraph.nodes.push(catchNode); @@ -587,7 +729,7 @@ function buildTryCatchTaskNode(task: TryTask, context: TaskContext): Graph { } else { const catchSubgraph: Graph = initGraph( GraphNodeType.Catch, - context.taskReference + catchReference + doReference, + context.taskId + catchReference + doReference, task, context.taskName + ' (catch)', containerSubgraph, @@ -598,7 +740,8 @@ function buildTryCatchTaskNode(task: TryTask, context: TaskContext): Graph { const catchContext: TaskContext = { ...context, graph: catchSubgraph, - taskListReference: catchSubgraph.id, + taskListId: context.taskId + catchReference + doReference, + taskListReference: context.taskReference + catchReference + doReference, taskList: mapTasks(task.catch.do), taskName: undefined, }; @@ -634,13 +777,12 @@ function buildEdge(graph: Graph, knownEdges: GraphEdge[], source: GraphNode, tar if (edge) { if (label && !edge.label?.includes(label)) { edge.label = edge.label + (edge.label ? ' / ' : '') + label; - edge.id = `${source.id}-${target.id}-${edge.label}`; } return edge; } const newEdge: GraphEdge = { label, - id: `${source.id}-${target.id}${label ? `-${label}` : ''}`, + id: `${source.id}-${target.id}`, sourceId: source.id, targetId: target.id, }; @@ -654,45 +796,45 @@ function buildEdge(graph: Graph, knownEdges: GraphEdge[], source: GraphNode, tar * @param edges */ export const remapEdges = (edges: GraphEdge[]): GraphEdge[] => { - let remappedEdges = [...edges.map((e) => ({ ...e }))]; - const leadsToPort = (edge: GraphEdge) => - edge.targetId !== 'root-exit-node' && - (edge.targetId.endsWith('-entry-node') || edge.targetId.endsWith('-exit-node')); - const isPortToPort = (edge: GraphEdge) => - edge.sourceId !== 'root-entry-node' && - edge.targetId !== 'root-exit-node' && - (edge.sourceId.endsWith('-entry-node') || edge.sourceId.endsWith('-exit-node')); - const edgesLeadingToPort = edges.filter((e) => leadsToPort(e) && !isPortToPort(e)); - const remap = (lead: GraphEdge, tails: GraphEdge[]) => { - tails.forEach((tail) => { - remappedEdges = remappedEdges.filter((e) => e.id !== tail.id); - if (!leadsToPort(tail)) { - const sourceId = lead.sourceId; - const targetId = tail.targetId; - const label = `${lead.label}${lead.label && tail.label ? ' / ' : ''}${tail.label}`; - const id = `${sourceId}-${targetId}${label ? `-${label}` : ''}`; - const newEdge: GraphEdge = { - id, - sourceId, - targetId, - label, - }; - remappedEdges.push(newEdge); - } else { - remap( - lead, - edges.filter((e) => e.sourceId === tail.targetId), - ); + const isInnerPort = (id: string) => + id.startsWith(portPrefix) && (id.endsWith(entrySuffix) || id.endsWith(exitSuffix)); + const outgoing = edges.reduce((bySource, edge) => { + const sourceEdges = bySource.get(edge.sourceId) ?? []; + sourceEdges.push(edge); + bySource.set(edge.sourceId, sourceEdges); + return bySource; + }, new Map()); + const remappedEdges: GraphEdge[] = []; + const appendLabel = (current: string, next: string | undefined) => + `${current}${current && next ? ' / ' : ''}${next ?? ''}`; + const addEdge = (sourceId: string, targetId: string, label: string) => { + const existing = remappedEdges.find((edge) => edge.sourceId === sourceId && edge.targetId === targetId); + if (existing) { + if (label && !existing.label?.includes(label)) { + existing.label = appendLabel(existing.label ?? '', label); } + return; + } + remappedEdges.push({ + id: `${sourceId}-${targetId}`, + sourceId, + targetId, + label, }); }; - edgesLeadingToPort.forEach((lead) => { - remappedEdges = remappedEdges.filter((e) => e.id !== lead.id); - remap( - lead, - edges.filter((e) => e.sourceId === lead.targetId), - ); - }); + const follow = (sourceId: string, edge: GraphEdge, label: string, visited: Set) => { + if (visited.has(edge)) return; + const nextVisited = new Set(visited).add(edge); + const nextLabel = appendLabel(label, edge.label); + if (!isInnerPort(edge.targetId)) { + addEdge(sourceId, edge.targetId, nextLabel); + return; + } + (outgoing.get(edge.targetId) ?? []).forEach((next) => follow(sourceId, next, nextLabel, nextVisited)); + }; + edges + .filter((edge) => !isInnerPort(edge.sourceId)) + .forEach((edge) => follow(edge.sourceId, edge, '', new Set())); return remappedEdges; }; @@ -717,6 +859,7 @@ export const flattenNodes = (node: Graph | GraphNode): FlatGraphNode[] => [ label: node.label, type: node.type, task: node.task, + taskReference: node.taskReference, parentId: node.parent?.id, }, ...((node as Graph).nodes || []).flatMap(flattenNodes), @@ -742,22 +885,44 @@ export function flattenGraph(graph: Graph, removePorts: boolean = false): FlatGr }; } +/** + * Asserts that all node ids in the provided graph are unique. + * Guards against custom task ids colliding with derived ids (ports, try/catch subgraphs). + * @param graph The graph to check + */ +function assertUniqueNodeIds(graph: Graph): void { + const knownIds = new Set(); + graph.nodes.flatMap(flattenNodes).forEach(({ id }) => { + if (knownIds.has(id)) { + throw new Error(`Duplicate node id '${id}' produced with the provided task id factory`); + } + knownIds.add(id); + }); +} + /** * Constructs a graph representation based on the given workflow. * * @param workflow The workflow to be converted into a graph structure. + * @param options The options used to customize how the graph is built. * @returns A graph representation of the workflow. */ -export function buildGraph(workflow: Workflow): Graph { +export function buildGraph(workflow: Workflow, options?: GraphBuildOptions): Graph { const graph = initGraph(GraphNodeType.Root); if (!graph.entryNode) throw new Error('The root graph should have an entry node.'); buildTransitions(graph.entryNode, { graph, + taskListId: doReference, taskListReference: doReference, taskList: mapTasks(workflow.do), + taskId: doReference, taskReference: doReference, knownEdges: [], + options, + taskIdsByReference: new Map(), + taskReferencesById: new Map(), }); + if (options?.taskId) assertUniqueNodeIds(graph); return graph; } @@ -766,9 +931,62 @@ export function buildGraph(workflow: Workflow): Graph { * * @param workflow The workflow to be converted into a flattened graph structure. * @param removePorts A boolean indicating whether the port nodes should be removed. + * @param options The options used to customize how the graph is built. * @returns A flattened graph representation of the workflow. */ -export function buildFlatGraph(workflow: Workflow, removePorts: boolean = false): FlatGraph { - const graph = buildGraph(workflow); +export function buildFlatGraph( + workflow: Workflow, + removePorts: boolean = false, + options?: GraphBuildOptions, +): FlatGraph { + const graph = buildGraph(workflow, options); return flattenGraph(graph, removePorts); } + +/** + * Returns all nodes in a hierarchical or flat graph without copying them. + * @param graph The graph to traverse + * @returns The root and all descendant nodes + */ +function getGraphNodes(graph: Graph | FlatGraph): Array { + const nodes: Array = []; + const visit = (node: GraphNode | FlatGraphNode | Graph | FlatGraph) => { + nodes.push(node); + const children = (node as Graph | FlatGraph).nodes; + if (Array.isArray(children)) children.forEach(visit); + }; + visit(graph); + return nodes; +} + +/** + * Gets the node whose task reference exactly matches the supplied indexed RFC 6901 JSON Pointer. + * @param graph The hierarchical or flat graph to search + * @param taskReference The task reference to find + * @returns The matching node, if any + */ +export function getNodeByTaskReference( + graph: Graph | FlatGraph, + taskReference: string, +): GraphNode | FlatGraphNode | undefined { + return getGraphNodes(graph).find((node) => node.taskReference === taskReference); +} + +/** + * Gets the deepest task node whose task reference contains the supplied workflow pointer. + * DSL validation errors expose their pointer through `WorkflowValidationError.path`; schema validation + * errors expose pointers through each `SchemaValidationError.schemaErrors[].instancePath`. + * Pointers above task granularity (e.g. a `TaskItem` at `/do/0` or a task list at `/do`) have no + * owning task node and resolve to `undefined`. + * @param graph The hierarchical or flat graph to search + * @param pointer The indexed RFC 6901 JSON Pointer to locate + * @returns The nearest owning task node, if any + */ +export function getNodeAtPointer(graph: Graph | FlatGraph, pointer: string): GraphNode | FlatGraphNode | undefined { + return getGraphNodes(graph) + .filter( + (node): node is (GraphNode | FlatGraphNode) & { taskReference: string } => + !!node.taskReference && (pointer === node.taskReference || pointer.startsWith(`${node.taskReference}/`)), + ) + .sort((left, right) => right.taskReference.length - left.taskReference.length)[0]; +} diff --git a/src/lib/mermaid-converter.ts b/src/lib/mermaid-converter.ts index 3769443..98f254a 100644 --- a/src/lib/mermaid-converter.ts +++ b/src/lib/mermaid-converter.ts @@ -1,6 +1,11 @@ import { Workflow } from './generated/definitions/specification'; import { buildFlatGraph, FlatGraph, FlatGraphNode, GraphEdge, GraphNodeType } from './graph-builder'; +/** + * Resolves the renderer-local alias of a node id. + */ +type AliasResolver = (id: string) => string; + /** * Adds indentation to each line of the provided code * @param code The code to indent @@ -12,34 +17,64 @@ const indent = (code: string) => .map((line) => ` ${line}`) .join('\n'); +/** + * Escapes Mermaid-significant characters in a displayed label + * @param label The label to escape + * @returns The escaped label + */ +const escapeLabel = (label: string): string => label.replace(/"/g, '#quot;'); + +/** + * Builds a resolver mapping node ids to renderer-local aliases (`n0`, `n1`, ...), so arbitrary + * task names cannot break the Mermaid syntax. Aliases are assigned in node declaration order, + * making them deterministic for a given graph. + * @param root The flattened graph to alias + * @returns The alias resolver + */ +function buildAliases(root: FlatGraph): AliasResolver { + const aliases = new Map(); + const alias: AliasResolver = (id) => { + let value = aliases.get(id); + if (!value) { + value = `n${aliases.size}`; + aliases.set(id, value); + } + return value; + }; + root.nodes.forEach((node) => alias(node.id)); + return alias; +} + /** * Converts a graph to Mermaid code * @param root The root graph + * @param alias The node id alias resolver * @param subgraphNode The graph to convert * @returns The converted graph */ -function convertGraphToCode(root: FlatGraph, subgraphNode?: FlatGraphNode): string { +function convertGraphToCode(root: FlatGraph, alias: AliasResolver, subgraphNode?: FlatGraphNode): string { const nodes = !subgraphNode ? root.nodes : root.nodes.filter((n) => n.parentId === subgraphNode.id); const edges = !subgraphNode ? root.edges : []; - const code = `${!subgraphNode ? 'flowchart TD' : `subgraph ${subgraphNode.id} ["${subgraphNode.label || subgraphNode.id}"]`} -${indent(nodes.map((node) => convertNodeToCode(root, node)).join('\n'))} -${indent(edges.map((edge) => convertEdgeToCode(edge)).join('\n'))} + const code = `${!subgraphNode ? 'flowchart TD' : `subgraph ${alias(subgraphNode.id)} ["${escapeLabel(subgraphNode.label || subgraphNode.id)}"]`} +${indent(nodes.map((node) => convertNodeToCode(root, alias, node)).join('\n'))} +${indent(edges.map((edge) => convertEdgeToCode(edge, alias)).join('\n'))} ${!subgraphNode ? '' : 'end'}`; return code; } /** * Converts a node to Mermaid code + * @param root The root graph + * @param alias The node id alias resolver * @param node The node to convert - * @param graph The root graph * @returns The converted node */ -function convertNodeToCode(root: FlatGraph, node: FlatGraphNode): string { +function convertNodeToCode(root: FlatGraph, alias: AliasResolver, node: FlatGraphNode): string { let code = ''; if (root.nodes.filter((n) => n.parentId === node.id).length) { - code = convertGraphToCode(root, node); + code = convertGraphToCode(root, alias, node); } else { - code = node.id; + code = alias(node.id); switch (node.type) { case GraphNodeType.Entry: // shouldn't exist in a simplified graph case GraphNodeType.Exit: @@ -52,7 +87,7 @@ function convertNodeToCode(root: FlatGraph, node: FlatGraphNode): string { code += '((( )))'; // alt '@{ shape: dbl-circ, label: " "}'; break; default: - code += `["${node.label || ' '}"]`; // alt `@{ label: "${node.label}" }` + code += `["${escapeLabel(node.label || ' ')}"]`; // alt `@{ label: "${node.label}" }` } } return code; @@ -61,25 +96,29 @@ function convertNodeToCode(root: FlatGraph, node: FlatGraphNode): string { /** * Converts an edge to Mermaid code * @param edge The edge to convert + * @param alias The node id alias resolver * @returns The converted edge */ -function convertEdgeToCode(edge: GraphEdge): string { +function convertEdgeToCode(edge: GraphEdge, alias: AliasResolver): string { const ignoreEndArrow = - !edge.targetId.startsWith('root') && + edge.targetId.startsWith('port-') && (edge.targetId.endsWith('-entry-node') || edge.targetId.endsWith('-exit-node')); - const code = `${edge.sourceId} ${edge.label ? `--"${edge.label}"` : ''}--${ignoreEndArrow ? '-' : '>'} ${edge.targetId}`; + const code = `${alias(edge.sourceId)} ${edge.label ? `--"${escapeLabel(edge.label)}"` : ''}--${ignoreEndArrow ? '-' : '>'} ${alias(edge.targetId)}`; return code; } /** - * Converts the provided workflow to Mermaid code + * Converts the provided workflow to Mermaid code. + * Nodes are rendered under renderer-local aliases (`n0`, `n1`, ...) rather than their graph ids, + * so the output stays valid whatever characters task names contain. * @param workflow The workflow to convert * @returns The Mermaid diagram */ export function convertToMermaidCode(workflow: Workflow): string { const graph = buildFlatGraph(workflow, true); + const alias = buildAliases(graph); return ( - convertGraphToCode(graph) + + convertGraphToCode(graph, alias) + ` classDef hidden width: 1px, height: 1px;` // should be "classDef hidden display: none;" but it can induce a Mermaid bug - https://github.com/mermaid-js/mermaid/issues/6452 diff --git a/src/lib/utils.ts b/src/lib/utils.ts index d270c38..516fe10 100644 --- a/src/lib/utils.ts +++ b/src/lib/utils.ts @@ -32,6 +32,23 @@ export const isObject = (value: T): value is T & object => { */ export const deepCopy = (obj: T): T => JSON.parse(JSON.stringify(obj)); +/** + * Escapes a JSON Pointer segment according to RFC 6901. + * @param segment The raw property name, record key, or array index + * @returns The escaped segment + */ +export const escapeJsonPointerSegment = (segment: string | number): string => + typeof segment === 'number' ? String(segment) : segment.replace(/~/g, '~0').replace(/\//g, '~1'); + +/** + * Appends a segment to an RFC 6901 JSON Pointer or pointer-shaped path. + * @param base The base pointer or path + * @param segment The segment to append + * @returns The extended pointer or path + */ +export const appendJsonPointerSegment = (base: string, segment: string | number): string => + `${base}/${escapeJsonPointerSegment(segment)}`; + /** * Checks the provided array is an array * @param arr diff --git a/src/lib/validation.ts b/src/lib/validation.ts index 8d920d6..bff9d02 100644 --- a/src/lib/validation.ts +++ b/src/lib/validation.ts @@ -19,7 +19,7 @@ import Ajv, { ValidateFunction } from 'ajv/dist/2020.js'; import addFormats from 'ajv-formats'; import workflowSchema from './generated/schema/workflow.json'; import { ChildType, childTypes, validationPointers } from './generated/validation'; -import { deepCopy, isObject } from './utils'; +import { appendJsonPointerSegment, deepCopy, isObject } from './utils'; import { getLifecycleHooks } from './lifecycle-hooks'; import { Specification } from './generated/definitions'; import { DslValidationError, SchemaValidationError, WorkflowValidationError } from './errors'; @@ -42,22 +42,6 @@ const validators: Map = new Map `~0`, `/` -> `~1`); numbers are array indices. - * @param segment The raw property name, record key or array index - * @returns The escaped segment - */ -const escapeSegment = (segment: string | number): string => - typeof segment === 'number' ? String(segment) : segment.replace(/~/g, '~0').replace(/\//g, '~1'); - -/** - * Appends a segment to a RFC 6901 JSON Pointer. - * @param base The base pointer ('' for the root) - * @param segment The segment to append - * @returns The extended pointer - */ -const appendPointer = (base: string, segment: string | number): string => `${base}/${escapeSegment(segment)}`; - /** * Renders a JSON Pointer for display in an error message, labelling the root explicitly. * @param path The RFC 6901 JSON Pointer ('' for the root) @@ -177,7 +161,7 @@ const validateDescendants = ( if (childNode == null) { continue; } - const childPath = segments.reduce(appendPointer, path); + const childPath = segments.reduce(appendJsonPointerSegment, path); const hooks = getLifecycleHooks(child.type); withValidationContext(child.type, childPath, () => { hooks?.preValidation?.(childNode, workflow); diff --git a/tests/graph/build-options.spec.ts b/tests/graph/build-options.spec.ts new file mode 100644 index 0000000..3463b8e --- /dev/null +++ b/tests/graph/build-options.spec.ts @@ -0,0 +1,147 @@ +import { buildFlatGraph, buildGraph, TaskIdFactoryContext } from '../../src/lib/graph-builder'; +import { Workflow } from '../../src/lib/generated/definitions/specification'; +import { Classes } from '../../src/lib/generated/classes'; + +const document = { + dsl: '1.0.3', + namespace: 'test', + name: 'build-options', + version: '0.1.0', +}; + +const workflowWith = (tasks: Workflow['do']): Workflow => ({ document, do: tasks }); + +describe('Graph build options', () => { + it('applies custom task ids and composes derived ids under them', () => { + const workflow = workflowWith([ + { wrapper: { do: [{ child: { set: { value: true } } }] } }, + { other: { set: { value: false } } }, + ]); + const flatGraph = buildFlatGraph(workflow, false, { + taskId: ({ reference }) => (reference === '/do/0/wrapper' ? 'node-1' : undefined), + }); + const ids = flatGraph.nodes.map((node) => node.id); + + expect(ids).toContain('node-1'); + expect(ids).toContain('port-node-1-entry-node'); + expect(ids).toContain('port-node-1-exit-node'); + expect(ids).toContain('node-1/do/child'); + expect(ids).toContain('/do/other'); + expect(ids).not.toContain('/do/wrapper'); + }); + + it('provides the task, name, reference, parent id and default id to the factory', () => { + const innerTask = { set: { value: 1 } }; + const outerTask = { do: [{ inner: innerTask }] }; + const workflow = workflowWith([{ outer: outerTask }]); + const calls: TaskIdFactoryContext[] = []; + buildGraph(workflow, { + taskId: (factoryContext) => { + calls.push(factoryContext); + return undefined; + }, + }); + + const outerCall = calls.find((call) => call.name === 'outer'); + expect(outerCall?.task).toBe(outerTask); + expect(outerCall?.reference).toBe('/do/0/outer'); + expect(outerCall?.defaultId).toBe('/do/outer'); + expect(outerCall?.parentId).toBe('root'); + const innerCall = calls.find((call) => call.name === 'inner'); + expect(innerCall?.task).toBe(innerTask); + expect(innerCall?.reference).toBe('/do/0/outer/do/0/inner'); + expect(innerCall?.defaultId).toBe('/do/outer/do/inner'); + expect(innerCall?.parentId).toBe('/do/outer'); + }); + + it('provides fork branches to the factory like any other task', () => { + const workflow = workflowWith([ + { + split: { + fork: { branches: [{ left: { set: { value: 'left' } } }, { right: { set: { value: 'right' } } }] }, + }, + }, + ]); + const calls: TaskIdFactoryContext[] = []; + buildGraph(workflow, { + taskId: (factoryContext) => { + calls.push(factoryContext); + return undefined; + }, + }); + + const rightCall = calls.find((call) => call.name === 'right'); + expect(rightCall?.reference).toBe('/do/0/split/fork/branches/1/right'); + expect(rightCall?.defaultId).toBe('/do/split/fork/branches/right'); + expect(rightCall?.parentId).toBe('/do/split'); + }); + + it('throws when two tasks resolve to the same id', () => { + const workflow = workflowWith([{ a: { set: { value: 'a' } } }, { b: { set: { value: 'b' } } }]); + expect(() => buildGraph(workflow, { taskId: () => 'same' })).toThrow( + "The task id 'same' is assigned to both tasks '/do/0/a' and '/do/1/b'", + ); + }); + + it('resolves each task at most once, keeping ids stable for tasks reached through multiple transitions', () => { + const workflow = workflowWith([ + { + decide: { + switch: [{ first: { when: '${ .x == 1 }', then: 'done' } }, { otherwise: { then: 'done' } }], + }, + }, + { done: { set: { value: true } } }, + ]); + let counter = 0; + const graph = buildGraph(workflow, { taskId: ({ name }) => `${name}#${++counter}` }); + const taskNodeIds = graph.nodes.filter((node) => node.task).map((node) => node.id); + + expect(counter).toBe(2); + expect(taskNodeIds).toEqual(['decide#1', 'done#2']); + }); + + it('rejects reserved and invalid factory results', () => { + const workflow = workflowWith([{ a: { set: { value: 'a' } } }]); + expect(() => buildGraph(workflow, { taskId: () => 'port-a' })).toThrow( + "The task id factory returned the reserved id 'port-a' for task '/do/0/a'", + ); + expect(() => buildGraph(workflow, { taskId: () => 'root' })).toThrow( + "The task id factory returned the reserved id 'root' for task '/do/0/a'", + ); + expect(() => buildGraph(workflow, { taskId: () => '' })).toThrow( + "The task id factory returned an invalid id for task '/do/0/a'", + ); + }); + + it('throws when a custom id collides with a derived subgraph id', () => { + const workflow = workflowWith([ + { guarded: { try: [{ attempt: { set: { value: 'try' } } }], catch: {} } }, + { other: { set: { value: 'other' } } }, + ]); + const customIds = new Map([ + ['/do/0/guarded', 'X'], + ['/do/1/other', 'X/try'], + ]); + expect(() => buildGraph(workflow, { taskId: ({ reference }) => customIds.get(reference) })).toThrow( + "Duplicate node id 'X/try' produced with the provided task id factory", + ); + }); + + it('honors the factory through the generated Workflow class', () => { + const workflow = Classes.Workflow.deserialize(` +document: + dsl: '1.0.3' + namespace: test + name: build-options + version: '0.1.0' +do: + - initialize: + set: + foo: bar`); + const staticGraph = Classes.Workflow.toGraph(workflow, { taskId: () => 'custom-init' }); + const instanceGraph = workflow.toGraph({ taskId: () => 'custom-init' }); + + expect(staticGraph.nodes.some((node) => node.id === 'custom-init')).toBe(true); + expect(instanceGraph.nodes.some((node) => node.id === 'custom-init')).toBe(true); + }); +}); diff --git a/tests/graph/graph.spec.ts b/tests/graph/graph.spec.ts index d773747..ed35ccc 100644 --- a/tests/graph/graph.spec.ts +++ b/tests/graph/graph.spec.ts @@ -17,15 +17,13 @@ do: const graph = buildGraph(workflow); expect(graph).toBeDefined(); - const initializeNode = graph.nodes.find((n) => n.id === '/do/0/initialize'); + const initializeNode = graph.nodes.find((n) => n.id === '/do/initialize'); expect(initializeNode).toBeDefined(); expect(initializeNode?.type).toBe(GraphNodeType.Set); expect(initializeNode?.label).toBe('initialize'); - expect( - graph.edges.find((e) => e.sourceId === 'root-entry-node' && e.targetId === '/do/0/initialize'), - ).toBeDefined(); - expect(graph.edges.find((e) => e.sourceId === '/do/0/initialize' && e.targetId === 'root-exit-node')).toBeDefined(); + expect(graph.edges.find((e) => e.sourceId === 'root-entry-node' && e.targetId === '/do/initialize')).toBeDefined(); + expect(graph.edges.find((e) => e.sourceId === '/do/initialize' && e.targetId === 'root-exit-node')).toBeDefined(); expect(graph.nodes.length).toBe(3); // start --> initialize --> end expect(graph.edges.length).toBe(2); @@ -45,14 +43,12 @@ do: const graph = workflow.toGraph(); expect(graph).toBeDefined(); - const initializeNode = graph.nodes.find((n) => n.id === '/do/0/initialize'); + const initializeNode = graph.nodes.find((n) => n.id === '/do/initialize'); expect(initializeNode).toBeDefined(); expect(initializeNode?.type).toBe(GraphNodeType.Set); - expect( - graph.edges.find((e) => e.sourceId === 'root-entry-node' && e.targetId === '/do/0/initialize'), - ).toBeDefined(); - expect(graph.edges.find((e) => e.sourceId === '/do/0/initialize' && e.targetId === 'root-exit-node')).toBeDefined(); + expect(graph.edges.find((e) => e.sourceId === 'root-entry-node' && e.targetId === '/do/initialize')).toBeDefined(); + expect(graph.edges.find((e) => e.sourceId === '/do/initialize' && e.targetId === 'root-exit-node')).toBeDefined(); expect(graph.nodes.length).toBe(3); // start --> initialize --> end expect(graph.edges.length).toBe(2); @@ -79,14 +75,12 @@ do: const graph = Classes.Workflow.toGraph(workflow); expect(graph).toBeDefined(); - const initializeNode = graph.nodes.find((n) => n.id === '/do/0/initialize'); + const initializeNode = graph.nodes.find((n) => n.id === '/do/initialize'); expect(initializeNode).toBeDefined(); expect(initializeNode?.type).toBe(GraphNodeType.Set); - expect( - graph.edges.find((e) => e.sourceId === 'root-entry-node' && e.targetId === '/do/0/initialize'), - ).toBeDefined(); - expect(graph.edges.find((e) => e.sourceId === '/do/0/initialize' && e.targetId === 'root-exit-node')).toBeDefined(); + expect(graph.edges.find((e) => e.sourceId === 'root-entry-node' && e.targetId === '/do/initialize')).toBeDefined(); + expect(graph.edges.find((e) => e.sourceId === '/do/initialize' && e.targetId === 'root-exit-node')).toBeDefined(); expect(graph.nodes.length).toBe(3); // start --> initialize --> end expect(graph.edges.length).toBe(2); @@ -112,17 +106,17 @@ do: const graph = buildGraph(workflow); expect(graph).toBeDefined(); - const step1 = graph.nodes.find((n) => n.id === '/do/0/step1'); - const step2 = graph.nodes.find((n) => n.id === '/do/1/step2'); - const step3 = graph.nodes.find((n) => n.id === '/do/2/step3'); + const step1 = graph.nodes.find((n) => n.id === '/do/step1'); + const step2 = graph.nodes.find((n) => n.id === '/do/step2'); + const step3 = graph.nodes.find((n) => n.id === '/do/step3'); expect(step1?.type).toBe(GraphNodeType.Set); expect(step2?.type).toBe(GraphNodeType.Set); expect(step3?.type).toBe(GraphNodeType.Set); - expect(graph.edges.find((e) => e.sourceId === 'root-entry-node' && e.targetId === '/do/0/step1')).toBeDefined(); - expect(graph.edges.find((e) => e.sourceId === '/do/0/step1' && e.targetId === '/do/1/step2')).toBeDefined(); - expect(graph.edges.find((e) => e.sourceId === '/do/1/step2' && e.targetId === '/do/2/step3')).toBeDefined(); - expect(graph.edges.find((e) => e.sourceId === '/do/2/step3' && e.targetId === 'root-exit-node')).toBeDefined(); + expect(graph.edges.find((e) => e.sourceId === 'root-entry-node' && e.targetId === '/do/step1')).toBeDefined(); + expect(graph.edges.find((e) => e.sourceId === '/do/step1' && e.targetId === '/do/step2')).toBeDefined(); + expect(graph.edges.find((e) => e.sourceId === '/do/step2' && e.targetId === '/do/step3')).toBeDefined(); + expect(graph.edges.find((e) => e.sourceId === '/do/step3' && e.targetId === 'root-exit-node')).toBeDefined(); expect(graph.nodes.length).toBe(5); // start --> step1 --> step2 --> step3 --> end expect(graph.edges.length).toBe(4); @@ -143,12 +137,12 @@ do: const graph = buildGraph(workflow); expect(graph).toBeDefined(); - const initializeNode = graph.nodes.find((n) => n.id === '/do/0/initialize'); + const initializeNode = graph.nodes.find((n) => n.id === '/do/initialize'); expect(initializeNode?.type).toBe(GraphNodeType.Set); // Conditional edge: start -- (if expression) --> initialize const conditionalEdge = graph.edges.find( - (e) => e.sourceId === 'root-entry-node' && e.targetId === '/do/0/initialize', + (e) => e.sourceId === 'root-entry-node' && e.targetId === '/do/initialize', ); expect(conditionalEdge?.label).toBe('${ input.data == true }'); @@ -156,7 +150,7 @@ do: expect(graph.edges.find((e) => e.sourceId === 'root-entry-node' && e.targetId === 'root-exit-node')).toBeDefined(); // Completion edge: initialize -> end - expect(graph.edges.find((e) => e.sourceId === '/do/0/initialize' && e.targetId === 'root-exit-node')).toBeDefined(); + expect(graph.edges.find((e) => e.sourceId === '/do/initialize' && e.targetId === 'root-exit-node')).toBeDefined(); expect(graph.nodes.length).toBe(3); // start --> initialize --> end expect(graph.edges.length).toBe(3); // -----------------> @@ -188,31 +182,31 @@ do: const graph = buildGraph(workflow); expect(graph).toBeDefined(); - const forSubgraph = graph.nodes.find((node) => node.id === '/do/0/checkup') as Graph; + const forSubgraph = graph.nodes.find((node) => node.id === '/do/checkup') as Graph; expect(forSubgraph).toBeDefined(); expect(forSubgraph.type).toBe(GraphNodeType.For); // Root edges wire the For subgraph's entry/exit ports into the root pipeline. expect( - graph.edges.find((e) => e.sourceId === 'root-entry-node' && e.targetId === '/do/0/checkup-entry-node'), + graph.edges.find((e) => e.sourceId === 'root-entry-node' && e.targetId === 'port-/do/checkup-entry-node'), ).toBeDefined(); expect( - graph.edges.find((e) => e.sourceId === '/do/0/checkup-exit-node' && e.targetId === 'root-exit-node'), + graph.edges.find((e) => e.sourceId === 'port-/do/checkup-exit-node' && e.targetId === 'root-exit-node'), ).toBeDefined(); expect(graph.nodes.length).toBe(3); // start --> checkup --> end expect(graph.edges.length).toBe(2); // Subgraph internals: entry -> waitForCheckup -> exit - const waitNode = forSubgraph.nodes.find((n) => n.id === '/do/0/checkup/for/do/0/waitForCheckup'); + const waitNode = forSubgraph.nodes.find((n) => n.id === '/do/checkup/do/waitForCheckup'); expect(waitNode?.type).toBe(GraphNodeType.Listen); expect( forSubgraph.edges.find( - (e) => e.sourceId === '/do/0/checkup-entry-node' && e.targetId === '/do/0/checkup/for/do/0/waitForCheckup', + (e) => e.sourceId === 'port-/do/checkup-entry-node' && e.targetId === '/do/checkup/do/waitForCheckup', ), ).toBeDefined(); expect( forSubgraph.edges.find( - (e) => e.sourceId === '/do/0/checkup/for/do/0/waitForCheckup' && e.targetId === '/do/0/checkup-exit-node', + (e) => e.sourceId === '/do/checkup/do/waitForCheckup' && e.targetId === 'port-/do/checkup-exit-node', ), ).toBeDefined(); expect(forSubgraph.nodes.length).toBe(3); // entry --> waitForCheckup --> exit @@ -257,7 +251,7 @@ do: foo: bar`); const graph = buildGraph(workflow); const decideToDoneEdges = graph.edges.filter( - (edge) => edge.sourceId === '/do/0/decide' && edge.targetId === '/do/1/done', + (edge) => edge.sourceId === '/do/decide' && edge.targetId === '/do/done', ); expect(decideToDoneEdges).toHaveLength(1); @@ -283,14 +277,14 @@ do: const graph = buildGraph(workflow); expect(graph).toBeDefined(); - const taskA = graph.nodes.find((n) => n.id === '/do/0/taskA'); - const taskB = graph.nodes.find((n) => n.id === '/do/1/taskB'); + const taskA = graph.nodes.find((n) => n.id === '/do/taskA'); + const taskB = graph.nodes.find((n) => n.id === '/do/taskB'); expect(taskA?.type).toBe(GraphNodeType.Set); expect(taskB?.type).toBe(GraphNodeType.Set); - expect(graph.edges.find((e) => e.sourceId === 'root-entry-node' && e.targetId === '/do/0/taskA')).toBeDefined(); - expect(graph.edges.find((e) => e.sourceId === '/do/0/taskA' && e.targetId === '/do/1/taskB')).toBeDefined(); - expect(graph.edges.find((e) => e.sourceId === '/do/1/taskB' && e.targetId === '/do/0/taskA')).toBeDefined(); + expect(graph.edges.find((e) => e.sourceId === 'root-entry-node' && e.targetId === '/do/taskA')).toBeDefined(); + expect(graph.edges.find((e) => e.sourceId === '/do/taskA' && e.targetId === '/do/taskB')).toBeDefined(); + expect(graph.edges.find((e) => e.sourceId === '/do/taskB' && e.targetId === '/do/taskA')).toBeDefined(); expect(graph.nodes.length).toBe(4); // start, taskA, taskB, end expect(graph.edges.length).toBe(3); // start->taskA, taskA->taskB, taskB->taskA @@ -311,11 +305,11 @@ do: const graph = buildGraph(workflow); expect(graph).toBeDefined(); - const loopTask = graph.nodes.find((n) => n.id === '/do/0/loopTask'); + const loopTask = graph.nodes.find((n) => n.id === '/do/loopTask'); expect(loopTask?.type).toBe(GraphNodeType.Set); - expect(graph.edges.find((e) => e.sourceId === 'root-entry-node' && e.targetId === '/do/0/loopTask')).toBeDefined(); - expect(graph.edges.find((e) => e.sourceId === '/do/0/loopTask' && e.targetId === '/do/0/loopTask')).toBeDefined(); + expect(graph.edges.find((e) => e.sourceId === 'root-entry-node' && e.targetId === '/do/loopTask')).toBeDefined(); + expect(graph.edges.find((e) => e.sourceId === '/do/loopTask' && e.targetId === '/do/loopTask')).toBeDefined(); expect(graph.nodes.length).toBe(3); // start, loopTask, end expect(graph.edges.length).toBe(2); // start->loopTask, loopTask->loopTask @@ -344,27 +338,31 @@ do: const graph = buildGraph(workflow); expect(graph).toBeDefined(); - const devSubgraph = graph.nodes.find((n) => n.id === '/do/0/awaitForDevWork') as Graph; - const qaSubgraph = graph.nodes.find((n) => n.id === '/do/1/awaitDetailsFromQA') as Graph; + const devSubgraph = graph.nodes.find((n) => n.id === '/do/awaitForDevWork') as Graph; + const qaSubgraph = graph.nodes.find((n) => n.id === '/do/awaitDetailsFromQA') as Graph; expect(devSubgraph?.type).toBe(GraphNodeType.Do); expect(qaSubgraph?.type).toBe(GraphNodeType.Do); - expect(devSubgraph.nodes.find((n) => n.id === '/do/0/awaitForDevWork/do/0/notifyDev')).toBeDefined(); - expect(qaSubgraph.nodes.find((n) => n.id === '/do/1/awaitDetailsFromQA/do/0/notifyQA')).toBeDefined(); + expect(devSubgraph.nodes.find((n) => n.id === '/do/awaitForDevWork/do/notifyDev')).toBeDefined(); + expect(qaSubgraph.nodes.find((n) => n.id === '/do/awaitDetailsFromQA/do/notifyQA')).toBeDefined(); // Entry edge into the first subgraph expect( - graph.edges.find((e) => e.sourceId === 'root-entry-node' && e.targetId === '/do/0/awaitForDevWork-entry-node'), + graph.edges.find((e) => e.sourceId === 'root-entry-node' && e.targetId === 'port-/do/awaitForDevWork-entry-node'), ).toBeDefined(); // Forward edge from dev -> qa (subgraph exit to subgraph entry) expect( graph.edges.find( - (e) => e.sourceId === '/do/0/awaitForDevWork-exit-node' && e.targetId === '/do/1/awaitDetailsFromQA-entry-node', + (e) => + e.sourceId === 'port-/do/awaitForDevWork-exit-node' && + e.targetId === 'port-/do/awaitDetailsFromQA-entry-node', ), ).toBeDefined(); // Back edge: qa -> dev expect( graph.edges.find( - (e) => e.sourceId === '/do/1/awaitDetailsFromQA-exit-node' && e.targetId === '/do/0/awaitForDevWork-entry-node', + (e) => + e.sourceId === 'port-/do/awaitDetailsFromQA-exit-node' && + e.targetId === 'port-/do/awaitForDevWork-entry-node', ), ).toBeDefined(); @@ -426,15 +424,13 @@ do: endpoint: https://example.com/orders/1`); const graph = buildGraph(workflow); - const callNode = graph.nodes.find((n) => n.id === '/do/0/fetchOrder'); + const callNode = graph.nodes.find((n) => n.id === '/do/fetchOrder'); expect(callNode).toBeDefined(); expect(callNode?.type).toBe(GraphNodeType.Call); expect(callNode?.label).toBe('fetchOrder'); - expect( - graph.edges.find((e) => e.sourceId === 'root-entry-node' && e.targetId === '/do/0/fetchOrder'), - ).toBeDefined(); - expect(graph.edges.find((e) => e.sourceId === '/do/0/fetchOrder' && e.targetId === 'root-exit-node')).toBeDefined(); + expect(graph.edges.find((e) => e.sourceId === 'root-entry-node' && e.targetId === '/do/fetchOrder')).toBeDefined(); + expect(graph.edges.find((e) => e.sourceId === '/do/fetchOrder' && e.targetId === 'root-exit-node')).toBeDefined(); expect(graph.nodes.length).toBe(3); // start --> fetchOrder --> end expect(graph.edges.length).toBe(2); @@ -456,12 +452,12 @@ do: type: com.example.order.placed.v1`); const graph = buildGraph(workflow); - const emitNode = graph.nodes.find((n) => n.id === '/do/0/notify'); + const emitNode = graph.nodes.find((n) => n.id === '/do/notify'); expect(emitNode).toBeDefined(); expect(emitNode?.type).toBe(GraphNodeType.Emit); - expect(graph.edges.find((e) => e.sourceId === 'root-entry-node' && e.targetId === '/do/0/notify')).toBeDefined(); - expect(graph.edges.find((e) => e.sourceId === '/do/0/notify' && e.targetId === 'root-exit-node')).toBeDefined(); + expect(graph.edges.find((e) => e.sourceId === 'root-entry-node' && e.targetId === '/do/notify')).toBeDefined(); + expect(graph.edges.find((e) => e.sourceId === '/do/notify' && e.targetId === 'root-exit-node')).toBeDefined(); expect(graph.nodes.length).toBe(3); expect(graph.edges.length).toBe(2); @@ -486,34 +482,34 @@ do: b: 2`); const graph = buildGraph(workflow); - const forkSubgraph = graph.nodes.find((n) => n.id === '/do/0/split') as Graph; + const forkSubgraph = graph.nodes.find((n) => n.id === '/do/split') as Graph; expect(forkSubgraph).toBeDefined(); expect(forkSubgraph.type).toBe(GraphNodeType.Fork); - const branchA = forkSubgraph.nodes.find((n) => n.id === '/do/0/split/fork/branches/0/branchA'); - const branchB = forkSubgraph.nodes.find((n) => n.id === '/do/0/split/fork/branches/1/branchB'); + const branchA = forkSubgraph.nodes.find((n) => n.id === '/do/split/fork/branches/branchA'); + const branchB = forkSubgraph.nodes.find((n) => n.id === '/do/split/fork/branches/branchB'); expect(branchA?.type).toBe(GraphNodeType.Set); expect(branchB?.type).toBe(GraphNodeType.Set); // Each branch fans out from the fork subgraph entry and joins its exit. expect( forkSubgraph.edges.find( - (e) => e.sourceId === '/do/0/split-entry-node' && e.targetId === '/do/0/split/fork/branches/0/branchA', + (e) => e.sourceId === 'port-/do/split-entry-node' && e.targetId === '/do/split/fork/branches/branchA', ), ).toBeDefined(); expect( forkSubgraph.edges.find( - (e) => e.sourceId === '/do/0/split-entry-node' && e.targetId === '/do/0/split/fork/branches/1/branchB', + (e) => e.sourceId === 'port-/do/split-entry-node' && e.targetId === '/do/split/fork/branches/branchB', ), ).toBeDefined(); expect( forkSubgraph.edges.find( - (e) => e.sourceId === '/do/0/split/fork/branches/0/branchA' && e.targetId === '/do/0/split-exit-node', + (e) => e.sourceId === '/do/split/fork/branches/branchA' && e.targetId === 'port-/do/split-exit-node', ), ).toBeDefined(); expect( forkSubgraph.edges.find( - (e) => e.sourceId === '/do/0/split/fork/branches/1/branchB' && e.targetId === '/do/0/split-exit-node', + (e) => e.sourceId === '/do/split/fork/branches/branchB' && e.targetId === 'port-/do/split-exit-node', ), ).toBeDefined(); @@ -539,16 +535,14 @@ do: type: com.example.order.placed.v1`); const graph = buildGraph(workflow); - const listenNode = graph.nodes.find((n) => n.id === '/do/0/waitForOrder'); + const listenNode = graph.nodes.find((n) => n.id === '/do/waitForOrder'); expect(listenNode).toBeDefined(); expect(listenNode?.type).toBe(GraphNodeType.Listen); expect( - graph.edges.find((e) => e.sourceId === 'root-entry-node' && e.targetId === '/do/0/waitForOrder'), - ).toBeDefined(); - expect( - graph.edges.find((e) => e.sourceId === '/do/0/waitForOrder' && e.targetId === 'root-exit-node'), + graph.edges.find((e) => e.sourceId === 'root-entry-node' && e.targetId === '/do/waitForOrder'), ).toBeDefined(); + expect(graph.edges.find((e) => e.sourceId === '/do/waitForOrder' && e.targetId === 'root-exit-node')).toBeDefined(); expect(graph.nodes.length).toBe(3); expect(graph.edges.length).toBe(2); @@ -570,12 +564,12 @@ do: title: Validation Error`); const graph = buildGraph(workflow); - const raiseNode = graph.nodes.find((n) => n.id === '/do/0/fail'); + const raiseNode = graph.nodes.find((n) => n.id === '/do/fail'); expect(raiseNode).toBeDefined(); expect(raiseNode?.type).toBe(GraphNodeType.Raise); - expect(graph.edges.find((e) => e.sourceId === 'root-entry-node' && e.targetId === '/do/0/fail')).toBeDefined(); - expect(graph.edges.find((e) => e.sourceId === '/do/0/fail' && e.targetId === 'root-exit-node')).toBeDefined(); + expect(graph.edges.find((e) => e.sourceId === 'root-entry-node' && e.targetId === '/do/fail')).toBeDefined(); + expect(graph.edges.find((e) => e.sourceId === '/do/fail' && e.targetId === 'root-exit-node')).toBeDefined(); expect(graph.nodes.length).toBe(3); expect(graph.edges.length).toBe(2); @@ -595,12 +589,12 @@ do: command: echo "hello"`); const graph = buildGraph(workflow); - const runNode = graph.nodes.find((n) => n.id === '/do/0/runScript'); + const runNode = graph.nodes.find((n) => n.id === '/do/runScript'); expect(runNode).toBeDefined(); expect(runNode?.type).toBe(GraphNodeType.Run); - expect(graph.edges.find((e) => e.sourceId === 'root-entry-node' && e.targetId === '/do/0/runScript')).toBeDefined(); - expect(graph.edges.find((e) => e.sourceId === '/do/0/runScript' && e.targetId === 'root-exit-node')).toBeDefined(); + expect(graph.edges.find((e) => e.sourceId === 'root-entry-node' && e.targetId === '/do/runScript')).toBeDefined(); + expect(graph.edges.find((e) => e.sourceId === '/do/runScript' && e.targetId === 'root-exit-node')).toBeDefined(); expect(graph.nodes.length).toBe(3); expect(graph.edges.length).toBe(2); @@ -619,12 +613,12 @@ do: seconds: 5`); const graph = buildGraph(workflow); - const waitNode = graph.nodes.find((n) => n.id === '/do/0/pause'); + const waitNode = graph.nodes.find((n) => n.id === '/do/pause'); expect(waitNode).toBeDefined(); expect(waitNode?.type).toBe(GraphNodeType.Wait); - expect(graph.edges.find((e) => e.sourceId === 'root-entry-node' && e.targetId === '/do/0/pause')).toBeDefined(); - expect(graph.edges.find((e) => e.sourceId === '/do/0/pause' && e.targetId === 'root-exit-node')).toBeDefined(); + expect(graph.edges.find((e) => e.sourceId === 'root-entry-node' && e.targetId === '/do/pause')).toBeDefined(); + expect(graph.edges.find((e) => e.sourceId === '/do/pause' && e.targetId === 'root-exit-node')).toBeDefined(); expect(graph.nodes.length).toBe(3); expect(graph.edges.length).toBe(2); @@ -648,26 +642,24 @@ do: b: 2`); const graph = buildGraph(workflow); - const doSubgraph = graph.nodes.find((n) => n.id === '/do/0/group') as Graph; + const doSubgraph = graph.nodes.find((n) => n.id === '/do/group') as Graph; expect(doSubgraph).toBeDefined(); expect(doSubgraph.type).toBe(GraphNodeType.Do); - const inner1 = doSubgraph.nodes.find((n) => n.id === '/do/0/group/do/0/inner1'); - const inner2 = doSubgraph.nodes.find((n) => n.id === '/do/0/group/do/1/inner2'); + const inner1 = doSubgraph.nodes.find((n) => n.id === '/do/group/do/inner1'); + const inner2 = doSubgraph.nodes.find((n) => n.id === '/do/group/do/inner2'); expect(inner1?.type).toBe(GraphNodeType.Set); expect(inner2?.type).toBe(GraphNodeType.Set); // entry -> inner1 -> inner2 -> exit expect( - doSubgraph.edges.find((e) => e.sourceId === '/do/0/group-entry-node' && e.targetId === '/do/0/group/do/0/inner1'), + doSubgraph.edges.find((e) => e.sourceId === 'port-/do/group-entry-node' && e.targetId === '/do/group/do/inner1'), ).toBeDefined(); expect( - doSubgraph.edges.find( - (e) => e.sourceId === '/do/0/group/do/0/inner1' && e.targetId === '/do/0/group/do/1/inner2', - ), + doSubgraph.edges.find((e) => e.sourceId === '/do/group/do/inner1' && e.targetId === '/do/group/do/inner2'), ).toBeDefined(); expect( - doSubgraph.edges.find((e) => e.sourceId === '/do/0/group/do/1/inner2' && e.targetId === '/do/0/group-exit-node'), + doSubgraph.edges.find((e) => e.sourceId === '/do/group/do/inner2' && e.targetId === 'port-/do/group-exit-node'), ).toBeDefined(); // entry, exit, inner1, inner2 @@ -761,14 +753,14 @@ do: const decideNode = graph.nodes.find((node) => node.label === 'decide'); expect(decideNode?.type).toBe(GraphNodeType.Switch); - const matchedEdge = graph.edges.find((e) => e.sourceId === '/do/0/decide' && e.targetId === '/do/1/matchedStep'); - const defaultEdge = graph.edges.find((e) => e.sourceId === '/do/0/decide' && e.targetId === '/do/2/defaultStep'); + const matchedEdge = graph.edges.find((e) => e.sourceId === '/do/decide' && e.targetId === '/do/matchedStep'); + const defaultEdge = graph.edges.find((e) => e.sourceId === '/do/decide' && e.targetId === '/do/defaultStep'); expect(matchedEdge?.label).toBe('matched'); expect(defaultEdge?.label).toBe('default'); // Because the switch has a default case, there must be no implicit fall-through edge from decide. const fallthrough = graph.edges.filter( - (e) => e.sourceId === '/do/0/decide' && e.targetId !== '/do/1/matchedStep' && e.targetId !== '/do/2/defaultStep', + (e) => e.sourceId === '/do/decide' && e.targetId !== '/do/matchedStep' && e.targetId !== '/do/defaultStep', ); expect(fallthrough).toHaveLength(0); }); @@ -789,7 +781,7 @@ do: set: never: true`); const graph = buildGraph(workflow); - const exitEdge = graph.edges.find((e) => e.sourceId === '/do/0/bail' && e.targetId === 'root-exit-node'); + const exitEdge = graph.edges.find((e) => e.sourceId === '/do/bail' && e.targetId === 'root-exit-node'); expect(exitEdge).toBeDefined(); }); }); diff --git a/tests/graph/node-identity.spec.ts b/tests/graph/node-identity.spec.ts new file mode 100644 index 0000000..3c8fb95 --- /dev/null +++ b/tests/graph/node-identity.spec.ts @@ -0,0 +1,184 @@ +import { + buildFlatGraph, + buildGraph, + flattenGraph, + getNodeAtPointer, + getNodeByTaskReference, + Graph, + GraphNode, + GraphNodeType, +} from '../../src/lib/graph-builder'; +import { Workflow } from '../../src/lib/generated/definitions/specification'; + +const document = { + dsl: '1.0.3', + namespace: 'test', + name: 'node-identity', + version: '0.1.0', +}; + +const workflowWith = (tasks: Workflow['do']): Workflow => ({ document, do: tasks }); + +const collectNodes = (graph: Graph): GraphNode[] => { + const nodes: GraphNode[] = [graph]; + graph.nodes.forEach((node) => { + nodes.push(node); + if (Array.isArray((node as Graph).nodes)) nodes.push(...collectNodes(node as Graph).slice(1)); + }); + return nodes; +}; + +const resolvePointer = (value: unknown, pointer: string): unknown => + pointer + .split('/') + .slice(1) + .map((segment) => segment.replace(/~1/g, '/').replace(/~0/g, '~')) + .reduce((current, segment) => (current as Record)[segment], value); + +describe('Graph node identity', () => { + it('keeps task and unaffected edge ids stable when siblings are inserted or removed', () => { + const base = workflowWith([ + { a: { set: { value: 'a' } } }, + { b: { set: { value: 'b' } } }, + { c: { set: { value: 'c' } } }, + ]); + const inserted = workflowWith([ + { added: { set: { value: 'new' } } }, + { a: { set: { value: 'a' } } }, + { b: { set: { value: 'b' } } }, + { c: { set: { value: 'c' } } }, + ]); + const removed = workflowWith([{ a: { set: { value: 'a' } } }, { c: { set: { value: 'c' } } }]); + + const baseGraph = buildGraph(base); + const insertedGraph = buildGraph(inserted); + const removedGraph = buildGraph(removed); + + expect(baseGraph.nodes.filter((node) => node.task).map((node) => node.id)).toEqual(['/do/a', '/do/b', '/do/c']); + expect(insertedGraph.nodes.filter((node) => node.task).map((node) => node.id)).toEqual([ + '/do/added', + '/do/a', + '/do/b', + '/do/c', + ]); + expect(removedGraph.nodes.filter((node) => node.task).map((node) => node.id)).toEqual(['/do/a', '/do/c']); + + const baseEdge = baseGraph.edges.find((edge) => edge.sourceId === '/do/a' && edge.targetId === '/do/b'); + const insertedEdge = insertedGraph.edges.find((edge) => edge.sourceId === '/do/a' && edge.targetId === '/do/b'); + expect(insertedEdge?.id).toBe(baseEdge?.id); + }); + + it('keeps edge ids stable when a transition label changes', () => { + const withCondition = (expression: string): Workflow => + workflowWith([{ a: { set: { value: 'a' } } }, { b: { set: { value: 'b' }, if: expression } }]); + const firstGraph = buildGraph(withCondition('${ .count > 1 }')); + const secondGraph = buildGraph(withCondition('${ .count > 2 }')); + const firstEdge = firstGraph.edges.find((edge) => edge.sourceId === '/do/a' && edge.targetId === '/do/b'); + const secondEdge = secondGraph.edges.find((edge) => edge.sourceId === '/do/a' && edge.targetId === '/do/b'); + + expect(firstEdge?.label).toBe('${ .count > 1 }'); + expect(secondEdge?.label).toBe('${ .count > 2 }'); + expect(secondEdge?.id).toBe(firstEdge?.id); + }); + + it('assigns document-faithful references only to actual task nodes', () => { + const workflow = workflowWith([ + { + loop: { + for: { each: 'item', in: '.items' }, + do: [{ child: { set: { value: true } } }], + }, + }, + { + split: { + fork: { branches: [{ left: { set: { value: 'left' } } }, { right: { set: { value: 'right' } } }] }, + }, + }, + { + guarded: { + try: [{ attempt: { set: { value: 'try' } } }], + catch: { do: [{ recover: { set: { value: 'catch' } } }] }, + }, + }, + ]); + + const graph = buildGraph(workflow); + const nodes = collectNodes(graph); + const referencedNodes = nodes.filter((node) => node.taskReference); + + referencedNodes.forEach((node) => expect(resolvePointer(workflow, node.taskReference!)).toBe(node.task)); + expect(referencedNodes.map((node) => node.taskReference)).toContain('/do/0/loop/do/0/child'); + + nodes + .filter((node) => + [ + GraphNodeType.Root, + GraphNodeType.Start, + GraphNodeType.End, + GraphNodeType.Entry, + GraphNodeType.Exit, + GraphNodeType.Try, + GraphNodeType.Catch, + ].includes(node.type), + ) + .forEach((node) => expect(node.taskReference).toBeUndefined()); + }); + + it('looks up exact task references and the deepest task owning a workflow pointer', () => { + const workflow = workflowWith([{ a: { set: { value: true } } }]); + const graph = buildGraph(workflow); + const flatGraph = flattenGraph(graph); + + expect(getNodeByTaskReference(graph, '/do/0/a')?.id).toBe('/do/a'); + expect(getNodeByTaskReference(flatGraph, '/do/0/a')?.id).toBe('/do/a'); + expect(getNodeByTaskReference(graph, '/do/0/missing')).toBeUndefined(); + expect(getNodeAtPointer(graph, '/do/0/a/set/value')?.id).toBe('/do/a'); + expect(getNodeAtPointer(flatGraph, '/do/0/a/set/value')?.id).toBe('/do/a'); + expect(getNodeAtPointer(graph, '/do/0/ab/set/value')).toBeUndefined(); + expect(getNodeAtPointer(graph, '/do')).toBeUndefined(); + }); + + it('escapes slash and tilde characters in ids and task references', () => { + const workflow = workflowWith([{ 'a/b~c': { set: { value: true } } }]); + const graph = buildGraph(workflow); + const node = graph.nodes.find((candidate) => candidate.task); + + expect(node?.id).toBe('/do/a~1b~0c'); + expect(node?.taskReference).toBe('/do/0/a~1b~0c'); + expect(resolvePointer(workflow, node!.taskReference!)).toBe(node?.task); + expect(getNodeByTaskReference(graph, '/do/0/a~1b~0c')).toBe(node); + }); + + it('qualifies identical task names by their containing task list', () => { + const workflow = workflowWith([ + { first: { do: [{ notify: { set: { source: 'first' } } }] } }, + { second: { do: [{ notify: { set: { source: 'second' } } }] } }, + ]); + const ids = buildFlatGraph(workflow).nodes.map((node) => node.id); + + expect(ids).toContain('/do/first/do/notify'); + expect(ids).toContain('/do/second/do/notify'); + }); + + it('keeps non-root port ids distinct from valid task ids with port-like names', () => { + const workflow = workflowWith([ + { x: { do: [{ inner: { set: { value: true } } }] } }, + { 'x-entry-node': { set: { value: false } } }, + ]); + const graph = buildGraph(workflow); + const flatGraph = flattenGraph(graph); + const simplifiedGraph = buildFlatGraph(workflow, true); + const ids = flatGraph.nodes.map((node) => node.id); + + expect(new Set(ids).size).toBe(ids.length); + expect(ids).toContain('port-/do/x-entry-node'); + expect(ids).toContain('/do/x-entry-node'); + expect( + graph.edges.some((edge) => edge.sourceId === 'port-/do/x-exit-node' && edge.targetId === '/do/x-entry-node'), + ).toBe(true); + expect(simplifiedGraph.nodes.some((node) => node.id === '/do/x-entry-node')).toBe(true); + expect( + simplifiedGraph.edges.some((edge) => edge.sourceId === '/do/x/do/inner' && edge.targetId === '/do/x-entry-node'), + ).toBe(true); + }); +}); diff --git a/tests/graph/simplified-graph.spec.ts b/tests/graph/simplified-graph.spec.ts index a2b6c8c..73ee779 100644 --- a/tests/graph/simplified-graph.spec.ts +++ b/tests/graph/simplified-graph.spec.ts @@ -27,14 +27,12 @@ do: // start, initialize, end expect(graph.nodes.find((n) => n.type === GraphNodeType.Start)).toBeDefined(); expect(graph.nodes.find((n) => n.type === GraphNodeType.End)).toBeDefined(); - const initializeNode = graph.nodes.find((n) => n.id === '/do/0/initialize'); + const initializeNode = graph.nodes.find((n) => n.id === '/do/initialize'); expect(initializeNode?.type).toBe(GraphNodeType.Set); // start -> initialize -> end - expect( - graph.edges.find((e) => e.sourceId === 'root-entry-node' && e.targetId === '/do/0/initialize'), - ).toBeDefined(); - expect(graph.edges.find((e) => e.sourceId === '/do/0/initialize' && e.targetId === 'root-exit-node')).toBeDefined(); + expect(graph.edges.find((e) => e.sourceId === 'root-entry-node' && e.targetId === '/do/initialize')).toBeDefined(); + expect(graph.edges.find((e) => e.sourceId === '/do/initialize' && e.targetId === 'root-exit-node')).toBeDefined(); expect(graph.nodes.length).toBe(3); // start --> initialize --> end expect(graph.edges.length).toBe(2); @@ -69,20 +67,16 @@ do: // start, checkup (For subgraph node), waitForCheckup, end expect(graph.nodes.find((n) => n.type === GraphNodeType.Start)).toBeDefined(); expect(graph.nodes.find((n) => n.type === GraphNodeType.End)).toBeDefined(); - expect(graph.nodes.find((n) => n.id === '/do/0/checkup')?.type).toBe(GraphNodeType.For); - expect(graph.nodes.find((n) => n.id === '/do/0/checkup/for/do/0/waitForCheckup')?.type).toBe(GraphNodeType.Listen); + expect(graph.nodes.find((n) => n.id === '/do/checkup')?.type).toBe(GraphNodeType.For); + expect(graph.nodes.find((n) => n.id === '/do/checkup/do/waitForCheckup')?.type).toBe(GraphNodeType.Listen); // After remapping, the inner entry/exit ports are collapsed, so: // root-entry -> waitForCheckup, waitForCheckup -> root-exit expect( - graph.edges.find( - (e) => e.sourceId === 'root-entry-node' && e.targetId === '/do/0/checkup/for/do/0/waitForCheckup', - ), + graph.edges.find((e) => e.sourceId === 'root-entry-node' && e.targetId === '/do/checkup/do/waitForCheckup'), ).toBeDefined(); expect( - graph.edges.find( - (e) => e.sourceId === '/do/0/checkup/for/do/0/waitForCheckup' && e.targetId === 'root-exit-node', - ), + graph.edges.find((e) => e.sourceId === '/do/checkup/do/waitForCheckup' && e.targetId === 'root-exit-node'), ).toBeDefined(); expect(graph.nodes.length).toBe(4); // start --[--> waitForCheckup --]--> end @@ -182,7 +176,7 @@ do: const waitNode = nodes.find((n) => n.label === 'waitForCheckup'); expect(waitNode).toBeDefined(); // Nested node's parentId should point at its enclosing subgraph. - expect(waitNode?.parentId).toBe('/do/0/checkup'); + expect(waitNode?.parentId).toBe('/do/checkup'); // Root's direct children should have 'root' as parentId. expect(nodes.find((n) => n.type === GraphNodeType.Start)?.parentId).toBe('root'); expect(nodes.find((n) => n.type === GraphNodeType.End)?.parentId).toBe('root'); @@ -222,10 +216,10 @@ do: // end up with exactly 2 direct edges: root-entry -> waitForCheckup, waitForCheckup -> root-exit. expect(remapped.length).toBe(2); expect( - remapped.some((e) => e.sourceId === 'root-entry-node' && e.targetId === '/do/0/checkup/for/do/0/waitForCheckup'), + remapped.some((e) => e.sourceId === 'root-entry-node' && e.targetId === '/do/checkup/do/waitForCheckup'), ).toBe(true); expect( - remapped.some((e) => e.sourceId === '/do/0/checkup/for/do/0/waitForCheckup' && e.targetId === 'root-exit-node'), + remapped.some((e) => e.sourceId === '/do/checkup/do/waitForCheckup' && e.targetId === 'root-exit-node'), ).toBe(true); }); diff --git a/tests/mermaid/mermaid.spec.ts b/tests/mermaid/mermaid.spec.ts index 234b262..6497dc9 100644 --- a/tests/mermaid/mermaid.spec.ts +++ b/tests/mermaid/mermaid.spec.ts @@ -17,11 +17,11 @@ do: const mermaidCode = convertToMermaidCode(workflow).trim(); expect(mermaidCode).toBe( `flowchart TD - root-entry-node(( )) - root-exit-node((( ))) - /do/0/initialize["initialize"] - /do/0/initialize --> root-exit-node - root-entry-node --> /do/0/initialize + n0(( )) + n1((( ))) + n2["initialize"] + n2 --> n1 + n0 --> n2 classDef hidden width: 1px, height: 1px;`.trim(), @@ -42,11 +42,11 @@ do: const mermaidCode = workflow.toMermaidCode().trim(); expect(mermaidCode).toBe( `flowchart TD - root-entry-node(( )) - root-exit-node((( ))) - /do/0/initialize["initialize"] - /do/0/initialize --> root-exit-node - root-entry-node --> /do/0/initialize + n0(( )) + n1((( ))) + n2["initialize"] + n2 --> n1 + n0 --> n2 classDef hidden width: 1px, height: 1px;`.trim(), @@ -74,11 +74,11 @@ classDef hidden width: 1px, height: 1px;`.trim(), const mermaidCode = Classes.Workflow.toMermaidCode(workflow).trim(); expect(mermaidCode).toBe( `flowchart TD - root-entry-node(( )) - root-exit-node((( ))) - /do/0/initialize["initialize"] - /do/0/initialize --> root-exit-node - root-entry-node --> /do/0/initialize + n0(( )) + n1((( ))) + n2["initialize"] + n2 --> n1 + n0 --> n2 classDef hidden width: 1px, height: 1px;`.trim(), @@ -99,11 +99,11 @@ do: const mermaidCode = new MermaidDiagram(workflow).sourceCode().trim(); expect(mermaidCode).toBe( `flowchart TD - root-entry-node(( )) - root-exit-node((( ))) - /do/0/initialize["initialize"] - /do/0/initialize --> root-exit-node - root-entry-node --> /do/0/initialize + n0(( )) + n1((( ))) + n2["initialize"] + n2 --> n1 + n0 --> n2 classDef hidden width: 1px, height: 1px;`.trim(), @@ -125,12 +125,12 @@ do: const mermaidCode = convertToMermaidCode(workflow).trim(); expect(mermaidCode).toBe( `flowchart TD - root-entry-node(( )) - root-exit-node((( ))) - /do/0/initialize["initialize"] - /do/0/initialize --> root-exit-node - root-entry-node --"\${ input.data == true }"--> /do/0/initialize - root-entry-node --> root-exit-node + n0(( )) + n1((( ))) + n2["initialize"] + n2 --> n1 + n0 --"\${ input.data == true }"--> n2 + n0 --> n1 classDef hidden width: 1px, height: 1px;`.trim(), @@ -163,15 +163,47 @@ do: const mermaidCode = convertToMermaidCode(workflow).trim(); expect(mermaidCode).toBe( `flowchart TD - root-entry-node(( )) - root-exit-node((( ))) - subgraph /do/0/checkup ["checkup"] - /do/0/checkup/for/do/0/waitForCheckup["waitForCheckup"] + n0(( )) + n1((( ))) + subgraph n2 ["checkup"] + n3["waitForCheckup"] end - /do/0/checkup/for/do/0/waitForCheckup["waitForCheckup"] - root-entry-node --> /do/0/checkup/for/do/0/waitForCheckup - /do/0/checkup/for/do/0/waitForCheckup --> root-exit-node + n3["waitForCheckup"] + n0 --> n3 + n3 --> n1 + + +classDef hidden width: 1px, height: 1px;`.trim(), + ); + }); + + it('should keep the diagram valid when task names contain Mermaid-significant characters', () => { + const workflow = { + document: { + dsl: '1.0.3', + name: 'special-characters', + version: '1.0.0', + namespace: 'test', + }, + do: [ + { + 'say "hello world" (loudly)': { + set: { + foo: 'bar', + }, + }, + }, + ], + } as Specification.Workflow; + const mermaidCode = Classes.Workflow.toMermaidCode(workflow).trim(); + expect(mermaidCode).toBe( + `flowchart TD + n0(( )) + n1((( ))) + n2["say #quot;hello world#quot; (loudly)"] + n2 --> n1 + n0 --> n2 classDef hidden width: 1px, height: 1px;`.trim(), diff --git a/tools/4_generate-classes.ts b/tools/4_generate-classes.ts index 3281b0c..028f4e8 100644 --- a/tools/4_generate-classes.ts +++ b/tools/4_generate-classes.ts @@ -49,7 +49,7 @@ ${hydrationResult.code ? `import { isObject } from '../../utils';` : ''} ${ name === 'Workflow' ? `import * as yaml from 'js-yaml'; -import { buildGraph, Graph } from '../../graph-builder'; +import { buildGraph, Graph, GraphBuildOptions } from '../../graph-builder'; import { convertToMermaidCode } from '../../mermaid-converter';` : '' } @@ -132,8 +132,8 @@ export class ${name} extends ${baseClass ? '_' + baseClass : `ObjectHydrator): Graph { - return buildGraph(model as unknown as WorkflowIntersection); + static toGraph(model: Partial, options?: GraphBuildOptions): Graph { + return buildGraph(model as unknown as WorkflowIntersection, options); } static toMermaidCode(model: Partial): string { @@ -152,10 +152,11 @@ export class ${name} extends ${baseClass ? '_' + baseClass : `ObjectHydrator): Graph; + toGraph(workflow: Partial, options?: GraphBuildOptions): Graph; /** * Generates the MermaidJS code corresponding to the provided workflow