refactor(core): Move execution engine code out of n8n-workflow (no-changelog) (#12147)

This commit is contained in:
कारतोफ्फेलस्क्रिप्ट™
2024-12-12 13:54:44 +01:00
committed by GitHub
parent 73f0c4cca9
commit 5a055ed526
44 changed files with 1995 additions and 1795 deletions

View File

@@ -0,0 +1,125 @@
import { mock } from 'jest-mock-extended';
import type { InstanceSettings } from 'n8n-core';
import type {
WorkflowParameters,
INode,
INodeType,
INodeTypeDescription,
WorkflowActivateMode,
} from 'n8n-workflow';
import { Workflow } from 'n8n-workflow';
import { ActiveWorkflowManager } from '@/active-workflow-manager';
import type { NodeTypes } from '@/node-types';
describe('ActiveWorkflowManager', () => {
let activeWorkflowManager: ActiveWorkflowManager;
const instanceSettings = mock<InstanceSettings>();
const nodeTypes = mock<NodeTypes>();
beforeEach(() => {
jest.clearAllMocks();
activeWorkflowManager = new ActiveWorkflowManager(
mock(),
mock(),
mock(),
mock(),
mock(),
nodeTypes,
mock(),
mock(),
mock(),
mock(),
mock(),
mock(),
mock(),
mock(),
instanceSettings,
mock(),
);
});
describe('checkIfWorkflowCanBeActivated', () => {
const disabledNode = mock<INode>({ type: 'triggerNode', disabled: true });
const unknownNode = mock<INode>({ type: 'unknownNode' });
const noTriggersNode = mock<INode>({ type: 'noTriggersNode' });
const pollNode = mock<INode>({ type: 'pollNode' });
const triggerNode = mock<INode>({ type: 'triggerNode' });
const webhookNode = mock<INode>({ type: 'webhookNode' });
nodeTypes.getByNameAndVersion.mockImplementation((type) => {
// TODO: getByNameAndVersion signature needs to be updated to allow returning undefined
if (type === 'unknownNode') return undefined as unknown as INodeType;
const partial: Partial<INodeType> = {
poll: undefined,
trigger: undefined,
webhook: undefined,
description: mock<INodeTypeDescription>({
properties: [],
}),
};
if (type === 'pollNode') partial.poll = jest.fn();
if (type === 'triggerNode') partial.trigger = jest.fn();
if (type === 'webhookNode') partial.webhook = jest.fn();
return mock(partial);
});
test.each([
['should skip disabled nodes', disabledNode, [], false],
['should skip nodes marked as ignored', triggerNode, ['triggerNode'], false],
['should skip unknown nodes', unknownNode, [], false],
['should skip nodes with no trigger method', noTriggersNode, [], false],
['should activate if poll method exists', pollNode, [], true],
['should activate if trigger method exists', triggerNode, [], true],
['should activate if webhook method exists', webhookNode, [], true],
])('%s', async (_, node, ignoredNodes, expected) => {
const workflow = new Workflow(mock<WorkflowParameters>({ nodeTypes, nodes: [node] }));
const canBeActivated = activeWorkflowManager.checkIfWorkflowCanBeActivated(
workflow,
ignoredNodes,
);
expect(canBeActivated).toBe(expected);
});
});
describe('shouldAddWebhooks', () => {
describe('if leader', () => {
beforeAll(() => {
Object.assign(instanceSettings, { isLeader: true, isFollower: false });
});
test('should return `true` for `init`', () => {
// ensure webhooks are populated on init: https://github.com/n8n-io/n8n/pull/8830
const result = activeWorkflowManager.shouldAddWebhooks('init');
expect(result).toBe(true);
});
test('should return `false` for `leadershipChange`', () => {
const result = activeWorkflowManager.shouldAddWebhooks('leadershipChange');
expect(result).toBe(false);
});
test('should return `true` for `update` or `activate`', () => {
const modes = ['update', 'activate'] as WorkflowActivateMode[];
for (const mode of modes) {
const result = activeWorkflowManager.shouldAddWebhooks(mode);
expect(result).toBe(true);
}
});
});
describe('if follower', () => {
beforeAll(() => {
Object.assign(instanceSettings, { isLeader: false, isFollower: true });
});
test('should return `false` for `update` or `activate`', () => {
const modes = ['update', 'activate'] as WorkflowActivateMode[];
for (const mode of modes) {
const result = activeWorkflowManager.shouldAddWebhooks(mode);
expect(result).toBe(false);
}
});
});
});
});

View File

@@ -1,5 +1,7 @@
import { mock } from 'jest-mock-extended';
import type { DirectoryLoader } from 'n8n-core';
import type { INodeProperties, INodeTypeDescription } from 'n8n-workflow';
import { NodeConnectionType } from 'n8n-workflow';
import { LoadNodesAndCredentials } from '../load-nodes-and-credentials';
@@ -34,4 +36,179 @@ describe('LoadNodesAndCredentials', () => {
expect(result).toBeUndefined();
});
});
describe('convertNodeToAiTool', () => {
const instance = new LoadNodesAndCredentials(mock(), mock(), mock(), mock());
let fullNodeWrapper: { description: INodeTypeDescription };
beforeEach(() => {
fullNodeWrapper = {
description: {
displayName: 'Test Node',
name: 'testNode',
group: ['test'],
description: 'A test node',
version: 1,
defaults: {},
inputs: [NodeConnectionType.Main],
outputs: [NodeConnectionType.Main],
properties: [],
},
};
});
it('should modify the name and displayName correctly', () => {
const result = instance.convertNodeToAiTool(fullNodeWrapper);
expect(result.description.name).toBe('testNodeTool');
expect(result.description.displayName).toBe('Test Node Tool');
});
it('should update inputs and outputs', () => {
const result = instance.convertNodeToAiTool(fullNodeWrapper);
expect(result.description.inputs).toEqual([]);
expect(result.description.outputs).toEqual([NodeConnectionType.AiTool]);
});
it('should remove the usableAsTool property', () => {
fullNodeWrapper.description.usableAsTool = true;
const result = instance.convertNodeToAiTool(fullNodeWrapper);
expect(result.description.usableAsTool).toBeUndefined();
});
it("should add toolDescription property if it doesn't exist", () => {
const result = instance.convertNodeToAiTool(fullNodeWrapper);
const toolDescriptionProp = result.description.properties.find(
(prop) => prop.name === 'toolDescription',
);
expect(toolDescriptionProp).toBeDefined();
expect(toolDescriptionProp?.type).toBe('string');
expect(toolDescriptionProp?.default).toBe(fullNodeWrapper.description.description);
});
it('should set codex categories correctly', () => {
const result = instance.convertNodeToAiTool(fullNodeWrapper);
expect(result.description.codex).toEqual({
categories: ['AI'],
subcategories: {
AI: ['Tools'],
Tools: ['Other Tools'],
},
resources: {},
});
});
it('should preserve existing properties', () => {
const existingProp: INodeProperties = {
displayName: 'Existing Prop',
name: 'existingProp',
type: 'string',
default: 'test',
};
fullNodeWrapper.description.properties = [existingProp];
const result = instance.convertNodeToAiTool(fullNodeWrapper);
expect(result.description.properties).toHaveLength(3); // Existing prop + toolDescription + notice
expect(result.description.properties).toContainEqual(existingProp);
});
it('should handle nodes with resource property', () => {
const resourceProp: INodeProperties = {
displayName: 'Resource',
name: 'resource',
type: 'options',
options: [{ name: 'User', value: 'user' }],
default: 'user',
};
fullNodeWrapper.description.properties = [resourceProp];
const result = instance.convertNodeToAiTool(fullNodeWrapper);
expect(result.description.properties[1].name).toBe('descriptionType');
expect(result.description.properties[2].name).toBe('toolDescription');
expect(result.description.properties[3]).toEqual(resourceProp);
});
it('should handle nodes with operation property', () => {
const operationProp: INodeProperties = {
displayName: 'Operation',
name: 'operation',
type: 'options',
options: [{ name: 'Create', value: 'create' }],
default: 'create',
};
fullNodeWrapper.description.properties = [operationProp];
const result = instance.convertNodeToAiTool(fullNodeWrapper);
expect(result.description.properties[1].name).toBe('descriptionType');
expect(result.description.properties[2].name).toBe('toolDescription');
expect(result.description.properties[3]).toEqual(operationProp);
});
it('should handle nodes with both resource and operation properties', () => {
const resourceProp: INodeProperties = {
displayName: 'Resource',
name: 'resource',
type: 'options',
options: [{ name: 'User', value: 'user' }],
default: 'user',
};
const operationProp: INodeProperties = {
displayName: 'Operation',
name: 'operation',
type: 'options',
options: [{ name: 'Create', value: 'create' }],
default: 'create',
};
fullNodeWrapper.description.properties = [resourceProp, operationProp];
const result = instance.convertNodeToAiTool(fullNodeWrapper);
expect(result.description.properties[1].name).toBe('descriptionType');
expect(result.description.properties[2].name).toBe('toolDescription');
expect(result.description.properties[3]).toEqual(resourceProp);
expect(result.description.properties[4]).toEqual(operationProp);
});
it('should handle nodes with empty properties', () => {
fullNodeWrapper.description.properties = [];
const result = instance.convertNodeToAiTool(fullNodeWrapper);
expect(result.description.properties).toHaveLength(2);
expect(result.description.properties[1].name).toBe('toolDescription');
});
it('should handle nodes with existing codex property', () => {
fullNodeWrapper.description.codex = {
categories: ['Existing'],
subcategories: {
Existing: ['Category'],
},
resources: {
primaryDocumentation: [{ url: 'https://example.com' }],
},
};
const result = instance.convertNodeToAiTool(fullNodeWrapper);
expect(result.description.codex).toEqual({
categories: ['AI'],
subcategories: {
AI: ['Tools'],
Tools: ['Other Tools'],
},
resources: {
primaryDocumentation: [{ url: 'https://example.com' }],
},
});
});
it('should handle nodes with very long names', () => {
fullNodeWrapper.description.name = 'veryLongNodeNameThatExceedsNormalLimits'.repeat(10);
fullNodeWrapper.description.displayName =
'Very Long Node Name That Exceeds Normal Limits'.repeat(10);
const result = instance.convertNodeToAiTool(fullNodeWrapper);
expect(result.description.name.endsWith('Tool')).toBe(true);
expect(result.description.displayName.endsWith('Tool')).toBe(true);
});
it('should handle nodes with special characters in name and displayName', () => {
fullNodeWrapper.description.name = 'special@#$%Node';
fullNodeWrapper.description.displayName = 'Special @#$% Node';
const result = instance.convertNodeToAiTool(fullNodeWrapper);
expect(result.description.name).toBe('special@#$%NodeTool');
expect(result.description.displayName).toBe('Special @#$% Node Tool');
});
});
});

View File

@@ -7,7 +7,7 @@ import type {
INodeTypeDescription,
} from 'n8n-workflow';
import type { LoadNodesAndCredentials } from '@/load-nodes-and-credentials';
import { LoadNodesAndCredentials } from '@/load-nodes-and-credentials';
import { NodeTypes } from '@/node-types';
describe('NodeTypes', () => {
@@ -104,6 +104,9 @@ describe('NodeTypes', () => {
});
it('should return the tool node-type when requested as tool', () => {
// @ts-expect-error don't mock convertNodeToAiTool for now
loadNodesAndCredentials.convertNodeToAiTool =
LoadNodesAndCredentials.prototype.convertNodeToAiTool;
const result = nodeTypes.getByNameAndVersion('n8n-nodes-base.testNodeTool');
expect(result).not.toEqual(toolSupportingNode);
expect(result.description.name).toEqual('n8n-nodes-base.testNodeTool');

View File

@@ -1,10 +1,8 @@
/* eslint-disable @typescript-eslint/no-unsafe-member-access */
import {
ActiveWorkflows,
ErrorReporter,
InstanceSettings,
NodeExecuteFunctions,
PollContext,
TriggerContext,
} from 'n8n-core';
@@ -186,12 +184,7 @@ export class ActiveWorkflowManager {
try {
// TODO: this should happen in a transaction, that way we don't need to manually remove this in `catch`
await this.webhookService.storeWebhook(webhook);
await workflow.createWebhookIfNotExists(
webhookData,
NodeExecuteFunctions,
mode,
activation,
);
await this.webhookService.createWebhookIfNotExists(workflow, webhookData, mode, activation);
} catch (error) {
if (activation === 'init' && error.name === 'QueryFailedError') {
// n8n does not remove the registered webhooks on exit.
@@ -261,7 +254,7 @@ export class ActiveWorkflowManager {
const webhooks = WebhookHelpers.getWorkflowWebhooks(workflow, additionalData, undefined, true);
for (const webhookData of webhooks) {
await workflow.deleteWebhook(webhookData, NodeExecuteFunctions, mode, 'update');
await this.webhookService.deleteWebhook(workflow, webhookData, mode, 'update');
}
await this.workflowStaticDataService.saveStaticData(workflow);
@@ -557,7 +550,7 @@ export class ActiveWorkflowManager {
settings: dbWorkflow.settings,
});
const canBeActivated = workflow.checkIfWorkflowCanBeActivated(STARTING_NODES);
const canBeActivated = this.checkIfWorkflowCanBeActivated(workflow, STARTING_NODES);
if (!canBeActivated) {
throw new WorkflowActivationError(
@@ -601,6 +594,48 @@ export class ActiveWorkflowManager {
return shouldDisplayActivationMessage;
}
/**
* A workflow can only be activated if it has a node which has either triggers
* or webhooks defined.
*
* @param {string[]} [ignoreNodeTypes] Node-types to ignore in the check
*/
checkIfWorkflowCanBeActivated(workflow: Workflow, ignoreNodeTypes?: string[]): boolean {
let node: INode;
let nodeType: INodeType | undefined;
for (const nodeName of Object.keys(workflow.nodes)) {
node = workflow.nodes[nodeName];
if (node.disabled === true) {
// Deactivated nodes can not trigger a run so ignore
continue;
}
if (ignoreNodeTypes !== undefined && ignoreNodeTypes.includes(node.type)) {
continue;
}
nodeType = this.nodeTypes.getByNameAndVersion(node.type, node.typeVersion);
if (nodeType === undefined) {
// Type is not known so check is not possible
continue;
}
if (
nodeType.poll !== undefined ||
nodeType.trigger !== undefined ||
nodeType.webhook !== undefined
) {
// Is a trigger node. So workflow can be activated.
return true;
}
}
return false;
}
/**
* Count all triggers in the workflow, excluding Manual Trigger.
*/

View File

@@ -22,8 +22,9 @@ import type {
ICredentialType,
INodeType,
IVersionedNodeType,
INodeProperties,
} from 'n8n-workflow';
import { NodeHelpers, ApplicationError } from 'n8n-workflow';
import { ApplicationError, NodeConnectionType } from 'n8n-workflow';
import path from 'path';
import picocolors from 'picocolors';
import { Container, Service } from 'typedi';
@@ -293,7 +294,7 @@ export class LoadNodesAndCredentials {
for (const usableNode of usableNodes) {
const description: INodeTypeBaseDescription | INodeTypeDescription =
structuredClone(usableNode);
const wrapped = NodeHelpers.convertNodeToAiTool({ description }).description;
const wrapped = this.convertNodeToAiTool({ description }).description;
this.types.nodes.push(wrapped);
this.known.nodes[wrapped.name] = structuredClone(this.known.nodes[usableNode.name]);
@@ -396,6 +397,101 @@ export class LoadNodesAndCredentials {
throw new UnrecognizedCredentialTypeError(credentialType);
}
/**
* Modifies the description of the passed in object, such that it can be used
* as an AI Agent Tool.
* Returns the modified item (not copied)
*/
convertNodeToAiTool<
T extends object & { description: INodeTypeDescription | INodeTypeBaseDescription },
>(item: T): T {
// quick helper function for type-guard down below
function isFullDescription(obj: unknown): obj is INodeTypeDescription {
return typeof obj === 'object' && obj !== null && 'properties' in obj;
}
if (isFullDescription(item.description)) {
item.description.name += 'Tool';
item.description.inputs = [];
item.description.outputs = [NodeConnectionType.AiTool];
item.description.displayName += ' Tool';
delete item.description.usableAsTool;
const hasResource = item.description.properties.some((prop) => prop.name === 'resource');
const hasOperation = item.description.properties.some((prop) => prop.name === 'operation');
if (!item.description.properties.map((prop) => prop.name).includes('toolDescription')) {
const descriptionType: INodeProperties = {
displayName: 'Tool Description',
name: 'descriptionType',
type: 'options',
noDataExpression: true,
options: [
{
name: 'Set Automatically',
value: 'auto',
description: 'Automatically set based on resource and operation',
},
{
name: 'Set Manually',
value: 'manual',
description: 'Manually set the description',
},
],
default: 'auto',
};
const descProp: INodeProperties = {
displayName: 'Description',
name: 'toolDescription',
type: 'string',
default: item.description.description,
required: true,
typeOptions: { rows: 2 },
description:
'Explain to the LLM what this tool does, a good, specific description would allow LLMs to produce expected results much more often',
placeholder: `e.g. ${item.description.description}`,
};
const noticeProp: INodeProperties = {
displayName:
"Use the expression {{ $fromAI('placeholder_name') }} for any data to be filled by the model",
name: 'notice',
type: 'notice',
default: '',
};
item.description.properties.unshift(descProp);
// If node has resource or operation we can determine pre-populate tool description based on it
// so we add the descriptionType property as the first property
if (hasResource || hasOperation) {
item.description.properties.unshift(descriptionType);
descProp.displayOptions = {
show: {
descriptionType: ['manual'],
},
};
}
item.description.properties.unshift(noticeProp);
}
}
const resources = item.description.codex?.resources ?? {};
item.description.codex = {
categories: ['AI'],
subcategories: {
AI: ['Tools'],
Tools: ['Other Tools'],
},
resources,
};
return item;
}
async setupHotReload() {
const { default: debounce } = await import('lodash/debounce');
// eslint-disable-next-line import/no-extraneous-dependencies

View File

@@ -59,7 +59,7 @@ export class NodeTypes implements INodeTypes {
const clonedNode = Object.create(versionedNodeType, {
description: { value: clonedDescription },
}) as INodeType;
const tool = NodeHelpers.convertNodeToAiTool(clonedNode);
const tool = this.loadNodesAndCredentials.convertNodeToAiTool(clonedNode);
loadedNodes[nodeType + 'Tool'] = { sourcePath: '', type: tool };
return tool;
}

View File

@@ -4,7 +4,7 @@
/* eslint-disable @typescript-eslint/no-unsafe-return */
/* eslint-disable @typescript-eslint/no-unsafe-call */
import get from 'lodash/get';
import { ErrorReporter, NodeExecuteFunctions } from 'n8n-core';
import { ErrorReporter, NodeExecuteFunctions, RoutingNode } from 'n8n-core';
import type {
ICredentialsDecrypted,
ICredentialTestFunction,
@@ -23,13 +23,7 @@ import type {
ICredentialTestFunctions,
IDataObject,
} from 'n8n-workflow';
import {
VersionedNodeType,
NodeHelpers,
RoutingNode,
Workflow,
ApplicationError,
} from 'n8n-workflow';
import { VersionedNodeType, NodeHelpers, Workflow, ApplicationError } from 'n8n-workflow';
import { Service } from 'typedi';
import { CredentialTypes } from '@/credential-types';
@@ -312,7 +306,6 @@ export class CredentialsTester {
runIndex,
nodeTypeCopy,
{ node, data: {}, source: null },
NodeExecuteFunctions,
credentialsDecrypted,
);
} catch (error) {

View File

@@ -1,4 +1,4 @@
import { LoadOptionsContext, NodeExecuteFunctions } from 'n8n-core';
import { LoadOptionsContext, RoutingNode } from 'n8n-core';
import type {
ILoadOptions,
ILoadOptionsFunctions,
@@ -18,7 +18,7 @@ import type {
NodeParameterValueType,
IDataObject,
} from 'n8n-workflow';
import { Workflow, RoutingNode, ApplicationError } from 'n8n-workflow';
import { Workflow, ApplicationError } from 'n8n-workflow';
import { Service } from 'typedi';
import { NodeTypes } from '@/node-types';
@@ -105,13 +105,11 @@ export class DynamicNodeParametersService {
main: [[{ json: {} }]],
};
const optionsData = await routingNode.runNode(
inputData,
runIndex,
tempNode,
{ node, source: null, data: {} },
NodeExecuteFunctions,
);
const optionsData = await routingNode.runNode(inputData, runIndex, tempNode, {
node,
source: null,
data: {},
});
if (optionsData?.length === 0) {
return [];

View File

@@ -18,6 +18,7 @@ import type {
} from '@/webhooks/test-webhook-registrations.service';
import { TestWebhooks } from '@/webhooks/test-webhooks';
import * as WebhookHelpers from '@/webhooks/webhook-helpers';
import type { WebhookService } from '@/webhooks/webhook.service';
import type { WebhookRequest } from '@/webhooks/webhook.types';
import * as AdditionalData from '@/workflow-execute-additional-data';
@@ -38,13 +39,20 @@ const webhook = mock<IWebhookData>({
userId,
});
const registrations = mock<TestWebhookRegistrationsService>();
let testWebhooks: TestWebhooks;
describe('TestWebhooks', () => {
const registrations = mock<TestWebhookRegistrationsService>();
const webhookService = mock<WebhookService>();
const testWebhooks = new TestWebhooks(
mock(),
mock(),
registrations,
mock(),
mock(),
webhookService,
);
beforeAll(() => {
testWebhooks = new TestWebhooks(mock(), mock(), registrations, mock(), mock());
jest.useFakeTimers();
});
@@ -68,7 +76,7 @@ describe('TestWebhooks', () => {
const needsWebhook = await testWebhooks.needsWebhook(args);
const [registerOrder] = registrations.register.mock.invocationCallOrder;
const [createOrder] = workflow.createWebhookIfNotExists.mock.invocationCallOrder;
const [createOrder] = webhookService.createWebhookIfNotExists.mock.invocationCallOrder;
expect(registerOrder).toBeLessThan(createOrder);
expect(needsWebhook).toBe(true);
@@ -132,11 +140,11 @@ describe('TestWebhooks', () => {
// ASSERT
const [registerOrder] = registrations.register.mock.invocationCallOrder;
const [createOrder] = workflow.createWebhookIfNotExists.mock.invocationCallOrder;
const [createOrder] = webhookService.createWebhookIfNotExists.mock.invocationCallOrder;
expect(registerOrder).toBeLessThan(createOrder);
expect(registrations.register.mock.calls[0][0].webhook.node).toBe(webhook2.node);
expect(workflow.createWebhookIfNotExists.mock.calls[0][0].node).toBe(webhook2.node);
expect(webhookService.createWebhookIfNotExists.mock.calls[0][1].node).toBe(webhook2.node);
expect(needsWebhook).toBe(true);
});
});

View File

@@ -6,7 +6,7 @@ import { WaitingForms } from '@/webhooks/waiting-forms';
describe('WaitingForms', () => {
const executionRepository = mock<ExecutionRepository>();
const waitingWebhooks = new WaitingForms(mock(), mock(), executionRepository);
const waitingWebhooks = new WaitingForms(mock(), mock(), executionRepository, mock());
beforeEach(() => {
jest.restoreAllMocks();

View File

@@ -10,7 +10,7 @@ import type { WaitingWebhookRequest } from '@/webhooks/webhook.types';
describe('WaitingWebhooks', () => {
const executionRepository = mock<ExecutionRepository>();
const waitingWebhooks = new WaitingWebhooks(mock(), mock(), executionRepository);
const waitingWebhooks = new WaitingWebhooks(mock(), mock(), executionRepository, mock());
beforeEach(() => {
jest.restoreAllMocks();

View File

@@ -1,11 +1,14 @@
import { mock } from 'jest-mock-extended';
import type { INode, INodeType, IWebhookData, IWorkflowExecuteAdditionalData } from 'n8n-workflow';
import { Workflow } from 'n8n-workflow';
import { v4 as uuid } from 'uuid';
import config from '@/config';
import { WebhookEntity } from '@/databases/entities/webhook-entity';
import { WebhookRepository } from '@/databases/repositories/webhook.repository';
import { CacheService } from '@/services/cache/cache.service';
import type { WebhookRepository } from '@/databases/repositories/webhook.repository';
import type { NodeTypes } from '@/node-types';
import type { CacheService } from '@/services/cache/cache.service';
import { WebhookService } from '@/webhooks/webhook.service';
import { mockInstance } from '@test/mocking';
const createWebhook = (method: string, path: string, webhookId?: string, pathSegments?: number) =>
Object.assign(new WebhookEntity(), {
@@ -16,9 +19,11 @@ const createWebhook = (method: string, path: string, webhookId?: string, pathSeg
}) as WebhookEntity;
describe('WebhookService', () => {
const webhookRepository = mockInstance(WebhookRepository);
const cacheService = mockInstance(CacheService);
const webhookService = new WebhookService(webhookRepository, cacheService);
const webhookRepository = mock<WebhookRepository>();
const cacheService = mock<CacheService>();
const nodeTypes = mock<NodeTypes>();
const webhookService = new WebhookService(mock(), webhookRepository, cacheService, nodeTypes);
const additionalData = mock<IWorkflowExecuteAdditionalData>();
beforeEach(() => {
config.load(config.default);
@@ -188,4 +193,171 @@ describe('WebhookService', () => {
expect(webhookRepository.upsert).toHaveBeenCalledWith(mockWebhook, ['method', 'webhookPath']);
});
});
describe('getNodeWebhooks()', () => {
const workflow = new Workflow({
id: 'test-workflow',
nodes: [],
connections: {},
active: true,
nodeTypes,
});
test('should return empty array if node is disabled', async () => {
const node = { disabled: true } as INode;
const webhooks = webhookService.getNodeWebhooks(workflow, node, additionalData);
expect(webhooks).toEqual([]);
});
test('should return webhooks for node with webhook definitions', async () => {
const node = {
name: 'Webhook',
type: 'n8n-nodes-base.webhook',
disabled: false,
} as INode;
const nodeType = {
description: {
webhooks: [
{
name: 'default',
httpMethod: 'GET',
path: '/webhook',
isFullPath: false,
restartWebhook: false,
},
],
},
} as INodeType;
nodeTypes.getByNameAndVersion.mockReturnValue(nodeType);
const webhooks = webhookService.getNodeWebhooks(workflow, node, additionalData);
expect(webhooks).toHaveLength(1);
expect(webhooks[0]).toMatchObject({
httpMethod: 'GET',
node: 'Webhook',
workflowId: 'test-workflow',
});
});
});
describe('createWebhookIfNotExists()', () => {
const workflow = new Workflow({
id: 'test-workflow',
nodes: [
mock<INode>({
name: 'Webhook',
type: 'n8n-nodes-base.webhook',
typeVersion: 1,
parameters: {},
}),
],
connections: {},
active: false,
nodeTypes,
});
const webhookData = mock<IWebhookData>({
node: 'Webhook',
webhookDescription: {
name: 'default',
httpMethod: 'GET',
path: '/webhook',
},
});
const defaultWebhookMethods = {
checkExists: jest.fn(),
create: jest.fn(),
};
const nodeType = mock<INodeType>({
webhookMethods: { default: defaultWebhookMethods },
});
test('should create webhook if it does not exist', async () => {
defaultWebhookMethods.checkExists.mockResolvedValue(false);
defaultWebhookMethods.create.mockResolvedValue(true);
nodeTypes.getByNameAndVersion.mockReturnValue(nodeType);
await webhookService.createWebhookIfNotExists(workflow, webhookData, 'trigger', 'init');
expect(defaultWebhookMethods.checkExists).toHaveBeenCalled();
expect(defaultWebhookMethods.create).toHaveBeenCalled();
});
test('should not create webhook if it already exists', async () => {
defaultWebhookMethods.checkExists.mockResolvedValue(true);
nodeTypes.getByNameAndVersion.mockReturnValue(nodeType);
await webhookService.createWebhookIfNotExists(workflow, webhookData, 'trigger', 'init');
expect(defaultWebhookMethods.checkExists).toHaveBeenCalled();
expect(defaultWebhookMethods.create).not.toHaveBeenCalled();
});
test('should handle case when webhook methods are not defined', async () => {
nodeTypes.getByNameAndVersion.mockReturnValue({} as INodeType);
await webhookService.createWebhookIfNotExists(workflow, webhookData, 'trigger', 'init');
// Test passes if no error is thrown when webhook methods are undefined
});
});
describe('deleteWebhook()', () => {
test('should call runWebhookMethod with delete', async () => {
const workflow = mock<Workflow>();
const webhookData = mock<IWebhookData>();
const runWebhookMethodSpy = jest.spyOn(webhookService as any, 'runWebhookMethod');
await webhookService.deleteWebhook(workflow, webhookData, 'trigger', 'init');
expect(runWebhookMethodSpy).toHaveBeenCalledWith(
'delete',
workflow,
webhookData,
'trigger',
'init',
);
});
});
describe('runWebhook()', () => {
const workflow = mock<Workflow>();
const webhookData = mock<IWebhookData>();
const node = mock<INode>();
const responseData = { workflowData: [] };
test('should throw error if node does not have webhooks', async () => {
const nodeType = {} as INodeType;
nodeTypes.getByNameAndVersion.mockReturnValue(nodeType);
await expect(
webhookService.runWebhook(workflow, webhookData, node, additionalData, 'trigger', null),
).rejects.toThrow('Node does not have any webhooks defined');
});
test('should execute webhook and return response data', async () => {
const nodeType = mock<INodeType>({
webhook: jest.fn().mockResolvedValue(responseData),
});
nodeTypes.getByNameAndVersion.mockReturnValue(nodeType);
const result = await webhookService.runWebhook(
workflow,
webhookData,
node,
additionalData,
'trigger',
null,
);
expect(result).toEqual(responseData);
expect(nodeType.webhook).toHaveBeenCalled();
});
});
});

View File

@@ -1,5 +1,5 @@
import type { Response } from 'express';
import { Workflow, NodeHelpers, CHAT_TRIGGER_NODE_TYPE } from 'n8n-workflow';
import { Workflow, CHAT_TRIGGER_NODE_TYPE } from 'n8n-workflow';
import type { INode, IWebhookData, IHttpRequestMethods } from 'n8n-workflow';
import { Service } from 'typedi';
@@ -114,11 +114,9 @@ export class LiveWebhooks implements IWebhookManager {
const additionalData = await WorkflowExecuteAdditionalData.getBase();
const webhookData = NodeHelpers.getNodeWebhooks(
workflow,
workflow.getNode(webhook.node) as INode,
additionalData,
).find((w) => w.httpMethod === httpMethod && w.path === webhook.webhookPath) as IWebhookData;
const webhookData = this.webhookService
.getNodeWebhooks(workflow, workflow.getNode(webhook.node) as INode, additionalData)
.find((w) => w.httpMethod === httpMethod && w.path === webhook.webhookPath) as IWebhookData;
// Get the node which has the webhook defined to know where to start from and to
// get additional data

View File

@@ -1,5 +1,4 @@
import type express from 'express';
import * as NodeExecuteFunctions from 'n8n-core';
import { InstanceSettings } from 'n8n-core';
import { WebhookPathTakenError, Workflow } from 'n8n-workflow';
import type {
@@ -25,6 +24,7 @@ import * as WebhookHelpers from '@/webhooks/webhook-helpers';
import * as WorkflowExecuteAdditionalData from '@/workflow-execute-additional-data';
import type { WorkflowRequest } from '@/workflows/workflow.request';
import { WebhookService } from './webhook.service';
import type {
IWebhookResponseCallbackData,
IWebhookManager,
@@ -44,6 +44,7 @@ export class TestWebhooks implements IWebhookManager {
private readonly registrations: TestWebhookRegistrationsService,
private readonly instanceSettings: InstanceSettings,
private readonly publisher: Publisher,
private readonly webhookService: WebhookService,
) {}
private timeouts: { [webhookKey: string]: NodeJS.Timeout } = {};
@@ -314,7 +315,7 @@ export class TestWebhooks implements IWebhookManager {
*/
await this.registrations.register(registration);
await workflow.createWebhookIfNotExists(webhook, NodeExecuteFunctions, 'manual', 'manual');
await this.webhookService.createWebhookIfNotExists(workflow, webhook, 'manual', 'manual');
cacheableWebhook.staticData = workflow.staticData;
@@ -431,7 +432,7 @@ export class TestWebhooks implements IWebhookManager {
if (staticData) workflow.staticData = staticData;
await workflow.deleteWebhook(webhook, NodeExecuteFunctions, 'internal', 'update');
await this.webhookService.deleteWebhook(workflow, webhook, 'internal', 'update');
}
await this.registrations.deregisterAll();

View File

@@ -3,7 +3,6 @@ import {
FORM_NODE_TYPE,
type INodes,
type IWorkflowBase,
NodeHelpers,
SEND_AND_WAIT_OPERATION,
WAIT_NODE_TYPE,
Workflow,
@@ -19,6 +18,7 @@ import { NodeTypes } from '@/node-types';
import * as WebhookHelpers from '@/webhooks/webhook-helpers';
import * as WorkflowExecuteAdditionalData from '@/workflow-execute-additional-data';
import { WebhookService } from './webhook.service';
import type {
IWebhookResponseCallbackData,
IWebhookManager,
@@ -38,6 +38,7 @@ export class WaitingWebhooks implements IWebhookManager {
protected readonly logger: Logger,
protected readonly nodeTypes: NodeTypes,
private readonly executionRepository: ExecutionRepository,
private readonly webhookService: WebhookService,
) {}
// TODO: implement `getWebhookMethods` for CORS support
@@ -164,17 +165,15 @@ export class WaitingWebhooks implements IWebhookManager {
}
const additionalData = await WorkflowExecuteAdditionalData.getBase();
const webhookData = NodeHelpers.getNodeWebhooks(
workflow,
workflowStartNode,
additionalData,
).find(
(webhook) =>
webhook.httpMethod === req.method &&
webhook.path === (suffix ?? '') &&
webhook.webhookDescription.restartWebhook === true &&
(webhook.webhookDescription.isForm || false) === this.includeForms,
);
const webhookData = this.webhookService
.getNodeWebhooks(workflow, workflowStartNode, additionalData)
.find(
(webhook) =>
webhook.httpMethod === req.method &&
webhook.path === (suffix ?? '') &&
webhook.webhookDescription.restartWebhook === true &&
(webhook.webhookDescription.isForm || false) === this.includeForms,
);
if (webhookData === undefined) {
// If no data got found it means that the execution can not be started via a webhook.

View File

@@ -9,7 +9,7 @@
import { GlobalConfig } from '@n8n/config';
import type express from 'express';
import get from 'lodash/get';
import { BinaryDataService, ErrorReporter, NodeExecuteFunctions } from 'n8n-core';
import { BinaryDataService, ErrorReporter } from 'n8n-core';
import type {
IBinaryData,
IBinaryKeyData,
@@ -35,7 +35,6 @@ import {
createDeferredPromise,
ExecutionCancelledError,
FORM_NODE_TYPE,
NodeHelpers,
NodeOperationError,
} from 'n8n-workflow';
import { finished } from 'stream/promises';
@@ -57,6 +56,7 @@ import * as WorkflowExecuteAdditionalData from '@/workflow-execute-additional-da
import * as WorkflowHelpers from '@/workflow-helpers';
import { WorkflowRunner } from '@/workflow-runner';
import { WebhookService } from './webhook.service';
import type { IWebhookResponseCallbackData, WebhookRequest } from './webhook.types';
/**
@@ -88,7 +88,12 @@ export function getWorkflowWebhooks(
}
returnData.push.apply(
returnData,
NodeHelpers.getNodeWebhooks(workflow, node, additionalData, ignoreRestartWebhooks),
Container.get(WebhookService).getNodeWebhooks(
workflow,
node,
additionalData,
ignoreRestartWebhooks,
),
);
}
@@ -254,11 +259,11 @@ export async function executeWebhook(
}
try {
webhookResultData = await workflow.runWebhook(
webhookResultData = await Container.get(WebhookService).runWebhook(
workflow,
webhookData,
workflowStartNode,
additionalData,
NodeExecuteFunctions,
executionMode,
runExecutionData ?? null,
);

View File

@@ -1,8 +1,23 @@
import type { IHttpRequestMethods } from 'n8n-workflow';
import { HookContext, WebhookContext } from 'n8n-core';
import { ApplicationError, Node, NodeHelpers } from 'n8n-workflow';
import type {
IHttpRequestMethods,
INode,
IRunExecutionData,
IWebhookData,
IWebhookResponseData,
IWorkflowExecuteAdditionalData,
WebhookSetupMethodNames,
Workflow,
WorkflowActivateMode,
WorkflowExecuteMode,
} from 'n8n-workflow';
import { Service } from 'typedi';
import type { WebhookEntity } from '@/databases/entities/webhook-entity';
import { WebhookRepository } from '@/databases/repositories/webhook.repository';
import { Logger } from '@/logging/logger.service';
import { NodeTypes } from '@/node-types';
import { CacheService } from '@/services/cache/cache.service';
type Method = NonNullable<IHttpRequestMethods>;
@@ -10,8 +25,10 @@ type Method = NonNullable<IHttpRequestMethods>;
@Service()
export class WebhookService {
constructor(
private webhookRepository: WebhookRepository,
private cacheService: CacheService,
private readonly logger: Logger,
private readonly webhookRepository: WebhookRepository,
private readonly cacheService: CacheService,
private readonly nodeTypes: NodeTypes,
) {}
async populateCache() {
@@ -118,4 +135,210 @@ export class WebhookService {
.find({ select: ['method'], where: { webhookPath: path } })
.then((rows) => rows.map((r) => r.method));
}
/**
* Returns all the webhooks which should be created for the give node
*/
getNodeWebhooks(
workflow: Workflow,
node: INode,
additionalData: IWorkflowExecuteAdditionalData,
ignoreRestartWebhooks = false,
): IWebhookData[] {
if (node.disabled === true) {
// Node is disabled so webhooks will also not be enabled
return [];
}
const nodeType = this.nodeTypes.getByNameAndVersion(node.type, node.typeVersion);
if (nodeType.description.webhooks === undefined) {
// Node does not have any webhooks so return
return [];
}
const workflowId = workflow.id || '__UNSAVED__';
const mode = 'internal';
const returnData: IWebhookData[] = [];
for (const webhookDescription of nodeType.description.webhooks) {
if (ignoreRestartWebhooks && webhookDescription.restartWebhook === true) {
continue;
}
let nodeWebhookPath = workflow.expression.getSimpleParameterValue(
node,
webhookDescription.path,
mode,
{},
);
if (nodeWebhookPath === undefined) {
this.logger.error(
`No webhook path could be found for node "${node.name}" in workflow "${workflowId}".`,
);
continue;
}
nodeWebhookPath = nodeWebhookPath.toString();
if (nodeWebhookPath.startsWith('/')) {
nodeWebhookPath = nodeWebhookPath.slice(1);
}
if (nodeWebhookPath.endsWith('/')) {
nodeWebhookPath = nodeWebhookPath.slice(0, -1);
}
const isFullPath: boolean = workflow.expression.getSimpleParameterValue(
node,
webhookDescription.isFullPath,
'internal',
{},
undefined,
false,
) as boolean;
const restartWebhook: boolean = workflow.expression.getSimpleParameterValue(
node,
webhookDescription.restartWebhook,
'internal',
{},
undefined,
false,
) as boolean;
const path = NodeHelpers.getNodeWebhookPath(
workflowId,
node,
nodeWebhookPath,
isFullPath,
restartWebhook,
);
const webhookMethods = workflow.expression.getSimpleParameterValue(
node,
webhookDescription.httpMethod,
mode,
{},
undefined,
'GET',
);
if (webhookMethods === undefined) {
this.logger.error(
`The webhook "${path}" for node "${node.name}" in workflow "${workflowId}" could not be added because the httpMethod is not defined.`,
);
continue;
}
let webhookId: string | undefined;
if ((path.startsWith(':') || path.includes('/:')) && node.webhookId) {
webhookId = node.webhookId;
}
String(webhookMethods)
.split(',')
.forEach((httpMethod) => {
if (!httpMethod) return;
returnData.push({
httpMethod: httpMethod.trim() as IHttpRequestMethods,
node: node.name,
path,
webhookDescription,
workflowId,
workflowExecuteAdditionalData: additionalData,
webhookId,
});
});
}
return returnData;
}
async createWebhookIfNotExists(
workflow: Workflow,
webhookData: IWebhookData,
mode: WorkflowExecuteMode,
activation: WorkflowActivateMode,
): Promise<void> {
const webhookExists = await this.runWebhookMethod(
'checkExists',
workflow,
webhookData,
mode,
activation,
);
if (!webhookExists) {
// If webhook does not exist yet create it
await this.runWebhookMethod('create', workflow, webhookData, mode, activation);
}
}
async deleteWebhook(
workflow: Workflow,
webhookData: IWebhookData,
mode: WorkflowExecuteMode,
activation: WorkflowActivateMode,
) {
await this.runWebhookMethod('delete', workflow, webhookData, mode, activation);
}
private async runWebhookMethod(
method: WebhookSetupMethodNames,
workflow: Workflow,
webhookData: IWebhookData,
mode: WorkflowExecuteMode,
activation: WorkflowActivateMode,
): Promise<boolean | undefined> {
const node = workflow.getNode(webhookData.node);
if (!node) return;
const nodeType = this.nodeTypes.getByNameAndVersion(node.type, node.typeVersion);
const webhookFn = nodeType.webhookMethods?.[webhookData.webhookDescription.name]?.[method];
if (webhookFn === undefined) return;
const context = new HookContext(
workflow,
node,
webhookData.workflowExecuteAdditionalData,
mode,
activation,
webhookData,
);
return (await webhookFn.call(context)) as boolean;
}
/**
* Executes the webhook data to see what it should return and if the
* workflow should be started or not
*/
async runWebhook(
workflow: Workflow,
webhookData: IWebhookData,
node: INode,
additionalData: IWorkflowExecuteAdditionalData,
mode: WorkflowExecuteMode,
runExecutionData: IRunExecutionData | null,
): Promise<IWebhookResponseData> {
const nodeType = this.nodeTypes.getByNameAndVersion(node.type, node.typeVersion);
if (nodeType.webhook === undefined) {
throw new ApplicationError('Node does not have any webhooks defined', {
extra: { nodeName: node.name },
});
}
const context = new WebhookContext(
workflow,
node,
additionalData,
mode,
webhookData,
[],
runExecutionData ?? null,
);
return nodeType instanceof Node
? await nodeType.webhook(context)
: ((await nodeType.webhook.call(context)) as IWebhookResponseData);
}
}

View File

@@ -1,5 +1,4 @@
import { mock } from 'jest-mock-extended';
import type { InstanceSettings } from 'n8n-core';
import { NodeApiError, Workflow } from 'n8n-workflow';
import type { IWebhookData, WorkflowActivateMode } from 'n8n-workflow';
import { Container } from 'typedi';
@@ -10,6 +9,7 @@ import type { WebhookEntity } from '@/databases/entities/webhook-entity';
import type { WorkflowEntity } from '@/databases/entities/workflow-entity';
import { ExecutionService } from '@/executions/execution.service';
import { ExternalHooks } from '@/external-hooks';
import { Logger } from '@/logging/logger.service';
import { NodeTypes } from '@/node-types';
import { Push } from '@/push';
import { SecretsHelper } from '@/secrets-helpers';
@@ -25,6 +25,7 @@ import * as utils from './shared/utils/';
import { mockInstance } from '../shared/mocking';
mockInstance(ActiveExecutions);
mockInstance(Logger);
mockInstance(Push);
mockInstance(SecretsHelper);
mockInstance(ExecutionService);
@@ -85,7 +86,7 @@ describe('init()', () => {
await Promise.all([createActiveWorkflow(), createActiveWorkflow()]);
const checkSpy = jest
.spyOn(Workflow.prototype, 'checkIfWorkflowCanBeActivated')
.spyOn(activeWorkflowManager, 'checkIfWorkflowCanBeActivated')
.mockReturnValue(true);
await activeWorkflowManager.init();
@@ -166,7 +167,6 @@ describe('remove()', () => {
it('should remove all webhooks of a workflow from external service', async () => {
const dbWorkflow = await createActiveWorkflow();
const deleteWebhookSpy = jest.spyOn(Workflow.prototype, 'deleteWebhook');
jest
.spyOn(WebhookHelpers, 'getWorkflowWebhooks')
.mockReturnValue([mock<IWebhookData>({ path: 'some-path' })]);
@@ -174,7 +174,7 @@ describe('remove()', () => {
await activeWorkflowManager.init();
await activeWorkflowManager.remove(dbWorkflow.id);
expect(deleteWebhookSpy).toHaveBeenCalledTimes(1);
expect(webhookService.deleteWebhook).toHaveBeenCalledTimes(1);
});
it('should stop running triggers and pollers', async () => {
@@ -258,82 +258,11 @@ describe('addWebhooks()', () => {
const [node] = dbWorkflow.nodes;
jest.spyOn(Workflow.prototype, 'getNode').mockReturnValue(node);
jest.spyOn(Workflow.prototype, 'checkIfWorkflowCanBeActivated').mockReturnValue(true);
jest.spyOn(Workflow.prototype, 'createWebhookIfNotExists').mockResolvedValue(undefined);
jest.spyOn(activeWorkflowManager, 'checkIfWorkflowCanBeActivated').mockReturnValue(true);
webhookService.createWebhookIfNotExists.mockResolvedValue(undefined);
await activeWorkflowManager.addWebhooks(workflow, additionalData, 'trigger', 'init');
expect(webhookService.storeWebhook).toHaveBeenCalledTimes(1);
});
});
describe('shouldAddWebhooks', () => {
describe('if leader', () => {
const activeWorkflowManager = new ActiveWorkflowManager(
mock(),
mock(),
mock(),
mock(),
mock(),
mock(),
mock(),
mock(),
mock(),
mock(),
mock(),
mock(),
mock(),
mock(),
mock<InstanceSettings>({ isLeader: true, isFollower: false }),
mock(),
);
test('should return `true` for `init`', () => {
// ensure webhooks are populated on init: https://github.com/n8n-io/n8n/pull/8830
const result = activeWorkflowManager.shouldAddWebhooks('init');
expect(result).toBe(true);
});
test('should return `false` for `leadershipChange`', () => {
const result = activeWorkflowManager.shouldAddWebhooks('leadershipChange');
expect(result).toBe(false);
});
test('should return `true` for `update` or `activate`', () => {
const modes = ['update', 'activate'] as WorkflowActivateMode[];
for (const mode of modes) {
const result = activeWorkflowManager.shouldAddWebhooks(mode);
expect(result).toBe(true);
}
});
});
describe('if follower', () => {
const activeWorkflowManager = new ActiveWorkflowManager(
mock(),
mock(),
mock(),
mock(),
mock(),
mock(),
mock(),
mock(),
mock(),
mock(),
mock(),
mock(),
mock(),
mock(),
mock<InstanceSettings>({ isLeader: false, isFollower: true }),
mock(),
);
test('should return `false` for `update` or `activate`', () => {
const modes = ['update', 'activate'] as WorkflowActivateMode[];
for (const mode of modes) {
const result = activeWorkflowManager.shouldAddWebhooks(mode);
expect(result).toBe(false);
}
});
});
});