diff --git a/packages/core/src/execution-engine/partial-execution-utils/__tests__/find-trigger-for-partial-execution.test.ts b/packages/core/src/execution-engine/partial-execution-utils/__tests__/find-trigger-for-partial-execution.test.ts new file mode 100644 index 0000000000..b9d1ae9eb9 --- /dev/null +++ b/packages/core/src/execution-engine/partial-execution-utils/__tests__/find-trigger-for-partial-execution.test.ts @@ -0,0 +1,217 @@ +import { mock } from 'jest-mock-extended'; +import type { IConnections, INode, INodeType, INodeTypes, IPinData } from 'n8n-workflow'; +import { Workflow } from 'n8n-workflow'; + +import { toIConnections } from './helpers'; +import { findTriggerForPartialExecution } from '../find-trigger-for-partial-execution'; + +describe('findTriggerForPartialExecution', () => { + const nodeTypes = mock(); + + const createMockWorkflow = (nodes: INode[], connections: IConnections, pinData?: IPinData) => + new Workflow({ + active: false, + nodes, + connections, + nodeTypes, + pinData, + }); + + const createNode = (name: string, type: string, disabled = false) => + mock({ name, type, disabled }); + const manualTriggerNode = createNode('ManualTrigger', 'n8n-nodes-base.manualTrigger'); + const disabledTriggerNode = createNode('DisabledTrigger', 'n8n-nodes-base.manualTrigger', true); + const pinnedTrigger = createNode('PinnedTrigger', 'n8n-nodes-base.manualTrigger'); + const setNode = createNode('Set', 'n8n-nodes-base.set'); + const noOpNode = createNode('No Operation', 'n8n-nodes-base.noOp'); + const webhookNode = createNode('Webhook', 'n8n-nodes-base.webhook'); + const webhookNode1 = createNode('Webhook1', 'n8n-nodes-base.webhook'); + + beforeEach(() => { + nodeTypes.getByNameAndVersion.mockImplementation((type) => { + const isTrigger = type.endsWith('Trigger') || type.endsWith('webhook'); + return mock({ + description: { + group: isTrigger ? ['trigger'] : [], + properties: [], + }, + }); + }); + }); + + const testGroups: Record< + string, + Array<{ + description: string; + nodes: INode[]; + connections: Array<{ to: INode; from: INode }>; + destinationNodeName: string; + pinData?: IPinData; + expectedTrigger?: INode; + }> + > = { + 'Single trigger node': [ + { + description: 'should return the destination node if it is a trigger', + nodes: [manualTriggerNode], + connections: [], + destinationNodeName: manualTriggerNode.name, + expectedTrigger: manualTriggerNode, + }, + { + description: 'should return a parent trigger node for a non-trigger destination', + nodes: [manualTriggerNode, setNode], + connections: [{ from: manualTriggerNode, to: setNode }], + destinationNodeName: setNode.name, + expectedTrigger: manualTriggerNode, + }, + ], + 'Multiple trigger nodes': [ + { + description: 'should prioritize webhook nodes when multiple parent triggers exist', + nodes: [webhookNode, manualTriggerNode, setNode], + connections: [ + { from: webhookNode, to: setNode }, + { from: manualTriggerNode, to: setNode }, + ], + destinationNodeName: setNode.name, + expectedTrigger: webhookNode, + }, + { + description: 'should handle multiple webhook triggers', + nodes: [webhookNode, webhookNode1, setNode], + connections: [ + { from: webhookNode, to: setNode }, + { from: webhookNode1, to: setNode }, + ], + destinationNodeName: setNode.name, + expectedTrigger: webhookNode1, + }, + { + description: 'should prioritize webhook node, even if it is further up', + nodes: [manualTriggerNode, setNode, noOpNode, webhookNode], + connections: [ + { from: manualTriggerNode, to: setNode }, + { from: setNode, to: noOpNode }, + { from: webhookNode, to: noOpNode }, + ], + destinationNodeName: noOpNode.name, + expectedTrigger: webhookNode, + }, + { + description: 'should ignore disabled parent trigger nodes', + nodes: [disabledTriggerNode, manualTriggerNode, setNode], + connections: [ + { from: disabledTriggerNode, to: setNode }, + { from: manualTriggerNode, to: setNode }, + ], + destinationNodeName: setNode.name, + expectedTrigger: manualTriggerNode, + }, + ], + 'No trigger nodes': [ + { + description: 'should return undefined when no valid parent triggers found', + nodes: [setNode, noOpNode], + connections: [{ from: setNode, to: noOpNode }], + destinationNodeName: noOpNode.name, + expectedTrigger: undefined, + }, + ], + 'Trigger node with pinned data': [ + { + description: 'should prioritize pinned trigger nodes', + nodes: [pinnedTrigger, manualTriggerNode, setNode], + connections: [ + { from: pinnedTrigger, to: setNode }, + { from: manualTriggerNode, to: setNode }, + ], + destinationNodeName: setNode.name, + pinData: { [pinnedTrigger.name]: [{ json: { test: true } }] }, + expectedTrigger: pinnedTrigger, + }, + { + description: 'should prioritize pinned webhook triggers', + nodes: [pinnedTrigger, manualTriggerNode, webhookNode, setNode], + connections: [ + { from: pinnedTrigger, to: setNode }, + { from: webhookNode, to: setNode }, + { from: manualTriggerNode, to: setNode }, + ], + destinationNodeName: setNode.name, + pinData: { + [pinnedTrigger.name]: [{ json: { test: true } }], + [webhookNode.name]: [{ json: { test: true } }], + }, + expectedTrigger: webhookNode, + }, + { + description: 'should prioritize the first connected pinned webhook triggers', + nodes: [webhookNode, webhookNode1, pinnedTrigger, manualTriggerNode, setNode], + connections: [ + { from: pinnedTrigger, to: setNode }, + { from: webhookNode, to: setNode }, + { from: webhookNode1, to: setNode }, + { from: manualTriggerNode, to: setNode }, + ], + destinationNodeName: setNode.name, + pinData: { + [pinnedTrigger.name]: [{ json: { test: true } }], + [webhookNode.name]: [{ json: { test: true } }], + [webhookNode1.name]: [{ json: { test: true } }], + }, + expectedTrigger: webhookNode, + }, + { + description: 'should prioritize the first connected pinned webhook triggers (reverse)', + nodes: [webhookNode1, webhookNode, pinnedTrigger, manualTriggerNode, setNode], + connections: [ + { from: pinnedTrigger, to: setNode }, + { from: webhookNode1, to: setNode }, + { from: webhookNode, to: setNode }, + { from: manualTriggerNode, to: setNode }, + ], + destinationNodeName: setNode.name, + pinData: { + [pinnedTrigger.name]: [{ json: { test: true } }], + [webhookNode.name]: [{ json: { test: true } }], + [webhookNode1.name]: [{ json: { test: true } }], + }, + expectedTrigger: webhookNode1, + }, + ], + }; + + for (const [group, tests] of Object.entries(testGroups)) { + describe(group, () => { + test.each(tests)( + '$description', + ({ nodes, connections, destinationNodeName, expectedTrigger, pinData }) => { + const workflow = createMockWorkflow(nodes, toIConnections(connections), pinData); + expect(findTriggerForPartialExecution(workflow, destinationNodeName)).toBe( + expectedTrigger, + ); + }, + ); + }); + } + + describe('Error and Edge Case Handling', () => { + it('should handle non-existent destination node gracefully', () => { + const workflow = createMockWorkflow([], {}); + expect(findTriggerForPartialExecution(workflow, 'NonExistentNode')).toBeUndefined(); + }); + + it('should handle empty workflow', () => { + const workflow = createMockWorkflow([], {}); + expect(findTriggerForPartialExecution(workflow, '')).toBeUndefined(); + }); + + it('should handle workflow with no connections', () => { + const workflow = createMockWorkflow([manualTriggerNode], {}); + expect(findTriggerForPartialExecution(workflow, manualTriggerNode.name)).toBe( + manualTriggerNode, + ); + }); + }); +}); diff --git a/packages/core/src/execution-engine/partial-execution-utils/find-trigger-for-partial-execution.ts b/packages/core/src/execution-engine/partial-execution-utils/find-trigger-for-partial-execution.ts index 977e99c107..a788d958c9 100644 --- a/packages/core/src/execution-engine/partial-execution-utils/find-trigger-for-partial-execution.ts +++ b/packages/core/src/execution-engine/partial-execution-utils/find-trigger-for-partial-execution.ts @@ -1,5 +1,7 @@ import * as assert from 'assert/strict'; -import type { INode, Workflow } from 'n8n-workflow'; +import type { INode, INodeType, Workflow } from 'n8n-workflow'; + +const isTriggerNode = (nodeType: INodeType) => nodeType.description.group.includes('trigger'); function findAllParentTriggers(workflow: Workflow, destinationNodeName: string) { const parentNodes = workflow @@ -17,35 +19,50 @@ function findAllParentTriggers(workflow: Workflow, destinationNodeName: string) }; }) .filter((value) => value !== null) - .filter(({ nodeType }) => nodeType.description.group.includes('trigger')) + .filter(({ nodeType }) => isTriggerNode(nodeType)) .map(({ node }) => node); return parentNodes; } -// TODO: write unit tests for this // TODO: rewrite this using DirectedGraph instead of workflow. export function findTriggerForPartialExecution( workflow: Workflow, destinationNodeName: string, ): INode | undefined { + // First, check if the destination node itself is a trigger + const destinationNode = workflow.getNode(destinationNodeName); + if (!destinationNode) return; + + const destinationNodeType = workflow.nodeTypes.getByNameAndVersion( + destinationNode.type, + destinationNode.typeVersion, + ); + + if (isTriggerNode(destinationNodeType) && !destinationNode.disabled) { + return destinationNode; + } + + // Since the destination node wasn't a trigger, we try to find a parent node that's a trigger const parentTriggers = findAllParentTriggers(workflow, destinationNodeName).filter( (trigger) => !trigger.disabled, ); + + // Prioritize webhook triggers with pinned-data const pinnedTriggers = parentTriggers // TODO: add the other filters here from `findAllPinnedActivators`, see // copy below. .filter((trigger) => workflow.pinData?.[trigger.name]) - // TODO: Make this sorting more predictable // Put nodes which names end with 'webhook' first, while also reversing the // order they had in the original array. - .sort((n) => (n.type.endsWith('webhook') ? -1 : 1)); - + .sort((a, b) => (a.type.endsWith('webhook') ? -1 : b.type.endsWith('webhook') ? 1 : 0)); if (pinnedTriggers.length) { return pinnedTriggers[0]; - } else { - return parentTriggers[0]; } + + // Prioritize webhook triggers over other parent triggers + const webhookTriggers = parentTriggers.filter((trigger) => trigger.type.endsWith('webhook')); + return webhookTriggers.length > 0 ? webhookTriggers[0] : parentTriggers[0]; } //function findAllPinnedActivators(workflow: Workflow, pinData?: IPinData) {