From 1c52bf9362e2f6782ed952851d4b9ab148f6e18b Mon Sep 17 00:00:00 2001 From: Alex Grozav Date: Tue, 10 Dec 2024 15:35:51 +0200 Subject: [PATCH 01/10] fix(editor): Prepare e2e suite for inline-expression-editor for new canvas (no-changelog) (#12110) Co-authored-by: Oleg Ivaniv --- cypress/e2e/11-inline-expression-editor.cy.ts | 2 +- cypress/e2e/13-pinning.cy.ts | 2 +- cypress/pages/workflow.ts | 11 ++++++----- cypress/types.ts | 5 +++++ 4 files changed, 13 insertions(+), 7 deletions(-) diff --git a/cypress/e2e/11-inline-expression-editor.cy.ts b/cypress/e2e/11-inline-expression-editor.cy.ts index 945c62821b7a7..a762135a65fc6 100644 --- a/cypress/e2e/11-inline-expression-editor.cy.ts +++ b/cypress/e2e/11-inline-expression-editor.cy.ts @@ -129,7 +129,7 @@ describe('Inline expression editor', () => { // Run workflow ndv.actions.close(); - WorkflowPage.actions.executeNode('No Operation'); + WorkflowPage.actions.executeNode('No Operation', { anchor: 'topLeft' }); WorkflowPage.actions.openNode('Hacker News'); WorkflowPage.actions.openInlineExpressionEditor(); diff --git a/cypress/e2e/13-pinning.cy.ts b/cypress/e2e/13-pinning.cy.ts index 4f48fa45290dc..2d3351f8aad3b 100644 --- a/cypress/e2e/13-pinning.cy.ts +++ b/cypress/e2e/13-pinning.cy.ts @@ -112,7 +112,7 @@ describe('Data pinning', () => { it('Should be able to pin data from canvas (context menu or shortcut)', () => { workflowPage.actions.addInitialNodeToCanvas('Schedule Trigger'); workflowPage.actions.addNodeToCanvas(EDIT_FIELDS_SET_NODE_NAME); - workflowPage.actions.openContextMenu(EDIT_FIELDS_SET_NODE_NAME, 'overflow-button'); + workflowPage.actions.openContextMenu(EDIT_FIELDS_SET_NODE_NAME, { method: 'overflow-button' }); workflowPage.getters .contextMenuAction('toggle_pin') .parent() diff --git a/cypress/pages/workflow.ts b/cypress/pages/workflow.ts index ee90fa55e8043..8707783ca1275 100644 --- a/cypress/pages/workflow.ts +++ b/cypress/pages/workflow.ts @@ -1,6 +1,7 @@ import { BasePage } from './base'; import { NodeCreator } from './features/node-creator'; import { META_KEY } from '../constants'; +import type { OpenContextMenuOptions } from '../types'; import { getVisibleSelect } from '../utils'; import { getUniqueWorkflowName, isCanvasV2 } from '../utils/workflowUtils'; @@ -272,14 +273,14 @@ export class WorkflowPage extends BasePage { }, openContextMenu: ( nodeTypeName?: string, - method: 'right-click' | 'overflow-button' = 'right-click', + { method = 'right-click', anchor = 'center' }: OpenContextMenuOptions = {}, ) => { const target = nodeTypeName ? this.getters.canvasNodeByName(nodeTypeName) : this.getters.nodeViewBackground(); if (method === 'right-click') { - target.rightclick(nodeTypeName ? 'center' : 'topLeft', { force: true }); + target.rightclick(nodeTypeName ? anchor : 'topLeft', { force: true }); } else { target.realHover(); target.find('[data-test-id="overflow-node-button"]').click({ force: true }); @@ -296,8 +297,8 @@ export class WorkflowPage extends BasePage { this.actions.openContextMenu(nodeTypeName); this.actions.contextMenuAction('delete'); }, - executeNode: (nodeTypeName: string) => { - this.actions.openContextMenu(nodeTypeName); + executeNode: (nodeTypeName: string, options?: OpenContextMenuOptions) => { + this.actions.openContextMenu(nodeTypeName, options); this.actions.contextMenuAction('execute'); }, addStickyFromContextMenu: () => { @@ -324,7 +325,7 @@ export class WorkflowPage extends BasePage { this.actions.contextMenuAction('toggle_pin'); }, openNodeFromContextMenu: (nodeTypeName: string) => { - this.actions.openContextMenu(nodeTypeName, 'overflow-button'); + this.actions.openContextMenu(nodeTypeName, { method: 'overflow-button' }); this.actions.contextMenuAction('open'); }, selectAllFromContextMenu: () => { diff --git a/cypress/types.ts b/cypress/types.ts index 6186c4201d405..63f2ddb99e703 100644 --- a/cypress/types.ts +++ b/cypress/types.ts @@ -22,3 +22,8 @@ export interface ExecutionResponse { results: Execution[]; }; } + +export type OpenContextMenuOptions = { + method?: 'right-click' | 'overflow-button'; + anchor?: 'topRight' | 'topLeft' | 'center' | 'bottomRight' | 'bottomLeft'; +}; From 2d36b42798a7a4424bc0d040dcc0abf57599de5a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E0=A4=95=E0=A4=BE=E0=A4=B0=E0=A4=A4=E0=A5=8B=E0=A4=AB?= =?UTF-8?q?=E0=A5=8D=E0=A4=AB=E0=A5=87=E0=A4=B2=E0=A4=B8=E0=A5=8D=E0=A4=95?= =?UTF-8?q?=E0=A5=8D=E0=A4=B0=E0=A4=BF=E0=A4=AA=E0=A5=8D=E0=A4=9F=E2=84=A2?= Date: Tue, 10 Dec 2024 14:48:39 +0100 Subject: [PATCH 02/10] fix(core): Fix supportedNodes for non-lazy loaded community packages (no-changelog) (#11329) --- packages/@n8n/nodes-langchain/package.json | 5 +- .../src/__tests__/credential-types.test.ts | 140 +++- .../src/__tests__/credentials-helper.test.ts | 76 +- packages/cli/src/__tests__/node-types.test.ts | 185 +++-- packages/cli/src/credential-types.ts | 28 +- .../installed-packages.repository.ts | 2 +- .../cli/src/load-nodes-and-credentials.ts | 46 +- packages/cli/src/node-types.ts | 57 +- .../active-workflow-manager.test.ts | 49 +- .../integration/public-api/workflows.test.ts | 22 +- .../shared/utils/community-nodes.ts | 2 +- .../test/integration/shared/utils/index.ts | 23 +- packages/core/bin/generate-known | 99 --- .../{generate-ui-types => generate-metadata} | 44 +- packages/core/package.json | 3 +- packages/core/src/DirectoryLoader.ts | 322 +++++--- packages/core/src/errors/index.ts | 2 + .../unrecognized-credential-type.error.ts | 9 + .../errors/unrecognized-node-type.error.ts | 4 +- packages/core/src/index.ts | 1 - packages/core/test/ClassLoader.test.ts | 52 ++ packages/core/test/DirectoryLoader.test.ts | 781 ++++++++++++++++++ packages/core/test/helpers/index.ts | 7 +- packages/nodes-base/package.json | 5 +- .../scripts/validate-load-options-methods.js | 2 +- packages/nodes-base/test/nodes/Helpers.ts | 12 +- packages/workflow/src/Interfaces.ts | 1 - packages/workflow/src/NodeHelpers.ts | 38 - 28 files changed, 1433 insertions(+), 584 deletions(-) delete mode 100755 packages/core/bin/generate-known rename packages/core/bin/{generate-ui-types => generate-metadata} (59%) create mode 100644 packages/core/src/errors/unrecognized-credential-type.error.ts rename packages/{cli => core}/src/errors/unrecognized-node-type.error.ts (55%) create mode 100644 packages/core/test/ClassLoader.test.ts create mode 100644 packages/core/test/DirectoryLoader.test.ts diff --git a/packages/@n8n/nodes-langchain/package.json b/packages/@n8n/nodes-langchain/package.json index f086f2e82f617..6164f91894f08 100644 --- a/packages/@n8n/nodes-langchain/package.json +++ b/packages/@n8n/nodes-langchain/package.json @@ -7,13 +7,12 @@ "clean": "rimraf dist .turbo", "dev": "pnpm run watch", "typecheck": "tsc --noEmit", - "build": "tsc -p tsconfig.build.json && pnpm n8n-copy-icons && pnpm build:metadata", - "build:metadata": "pnpm n8n-generate-known && pnpm n8n-generate-ui-types", + "build": "tsc -p tsconfig.build.json && pnpm n8n-copy-icons && pnpm n8n-generate-metadata", "format": "biome format --write .", "format:check": "biome ci .", "lint": "eslint nodes credentials --quiet", "lintfix": "eslint nodes credentials --fix", - "watch": "tsc-watch -p tsconfig.build.json --onCompilationComplete \"tsc-alias -p tsconfig.build.json\" --onSuccess \"pnpm n8n-generate-ui-types\"", + "watch": "tsc-watch -p tsconfig.build.json --onCompilationComplete \"tsc-alias -p tsconfig.build.json\" --onSuccess \"pnpm n8n-generate-metadata\"", "test": "jest", "test:dev": "jest --watch" }, diff --git a/packages/cli/src/__tests__/credential-types.test.ts b/packages/cli/src/__tests__/credential-types.test.ts index 0ccb5ab771838..82780d114cac4 100644 --- a/packages/cli/src/__tests__/credential-types.test.ts +++ b/packages/cli/src/__tests__/credential-types.test.ts @@ -1,41 +1,121 @@ -import { Container } from 'typedi'; +import { mock } from 'jest-mock-extended'; +import { UnrecognizedCredentialTypeError } from 'n8n-core'; +import type { ICredentialType, LoadedClass } from 'n8n-workflow'; import { CredentialTypes } from '@/credential-types'; -import { LoadNodesAndCredentials } from '@/load-nodes-and-credentials'; -import { mockInstance } from '@test/mocking'; +import type { LoadNodesAndCredentials } from '@/load-nodes-and-credentials'; describe('CredentialTypes', () => { - const mockNodesAndCredentials = mockInstance(LoadNodesAndCredentials, { - loadedCredentials: { - fakeFirstCredential: { - type: { - name: 'fakeFirstCredential', - displayName: 'Fake First Credential', - properties: [], - }, - sourcePath: '', - }, - fakeSecondCredential: { - type: { - name: 'fakeSecondCredential', - displayName: 'Fake Second Credential', - properties: [], - }, - sourcePath: '', - }, - }, + const loadNodesAndCredentials = mock(); + + const credentialTypes = new CredentialTypes(loadNodesAndCredentials); + + const testCredential: LoadedClass = { + sourcePath: '', + type: mock(), + }; + + loadNodesAndCredentials.getCredential.mockImplementation((credentialType) => { + if (credentialType === 'testCredential') return testCredential; + throw new UnrecognizedCredentialTypeError(credentialType); + }); + + beforeEach(() => { + jest.clearAllMocks(); + }); + + describe('getByName', () => { + test('Should throw error when calling invalid credential name', () => { + expect(() => credentialTypes.getByName('unknownCredential')).toThrowError('c'); + }); + + test('Should return correct credential type for valid name', () => { + expect(credentialTypes.getByName('testCredential')).toStrictEqual(testCredential.type); + }); + }); + + describe('recognizes', () => { + test('Should recognize credential type that exists in knownCredentials', () => { + const credentialTypes = new CredentialTypes( + mock({ + loadedCredentials: {}, + knownCredentials: { testCredential: mock({ supportedNodes: [] }) }, + }), + ); + + expect(credentialTypes.recognizes('testCredential')).toBe(true); + }); + + test('Should recognize credential type that exists in loadedCredentials', () => { + const credentialTypes = new CredentialTypes( + mock({ + loadedCredentials: { testCredential }, + knownCredentials: {}, + }), + ); + + expect(credentialTypes.recognizes('testCredential')).toBe(true); + }); + + test('Should not recognize unknown credential type', () => { + expect(credentialTypes.recognizes('unknownCredential')).toBe(false); + }); }); - const credentialTypes = Container.get(CredentialTypes); + describe('getSupportedNodes', () => { + test('Should return supported nodes for known credential type', () => { + const supportedNodes = ['node1', 'node2']; + const credentialTypes = new CredentialTypes( + mock({ + knownCredentials: { testCredential: mock({ supportedNodes }) }, + }), + ); + + expect(credentialTypes.getSupportedNodes('testCredential')).toEqual(supportedNodes); + }); - test('Should throw error when calling invalid credential name', () => { - expect(() => credentialTypes.getByName('fakeThirdCredential')).toThrowError(); + test('Should return empty array for unknown credential type supported nodes', () => { + expect(credentialTypes.getSupportedNodes('unknownCredential')).toBeEmptyArray(); + }); }); - test('Should return correct credential type for valid name', () => { - const mockedCredentialTypes = mockNodesAndCredentials.loadedCredentials; - expect(credentialTypes.getByName('fakeFirstCredential')).toStrictEqual( - mockedCredentialTypes.fakeFirstCredential.type, - ); + describe('getParentTypes', () => { + test('Should return parent types for credential type with extends', () => { + const credentialTypes = new CredentialTypes( + mock({ + knownCredentials: { + childType: { extends: ['parentType1', 'parentType2'] }, + parentType1: { extends: ['grandparentType'] }, + parentType2: { extends: [] }, + grandparentType: { extends: [] }, + }, + }), + ); + + const parentTypes = credentialTypes.getParentTypes('childType'); + expect(parentTypes).toContain('parentType1'); + expect(parentTypes).toContain('parentType2'); + expect(parentTypes).toContain('grandparentType'); + }); + + test('Should return empty array for credential type without extends', () => { + const credentialTypes = new CredentialTypes( + mock({ + knownCredentials: { testCredential: { extends: [] } }, + }), + ); + + expect(credentialTypes.getParentTypes('testCredential')).toBeEmptyArray(); + }); + + test('Should return empty array for unknown credential type parent types', () => { + const credentialTypes = new CredentialTypes( + mock({ + knownCredentials: {}, + }), + ); + + expect(credentialTypes.getParentTypes('unknownCredential')).toBeEmptyArray(); + }); }); }); diff --git a/packages/cli/src/__tests__/credentials-helper.test.ts b/packages/cli/src/__tests__/credentials-helper.test.ts index 62cab968e4570..7deffcd2295e1 100644 --- a/packages/cli/src/__tests__/credentials-helper.test.ts +++ b/packages/cli/src/__tests__/credentials-helper.test.ts @@ -1,3 +1,4 @@ +import { mock } from 'jest-mock-extended'; import type { IAuthenticateGeneric, ICredentialDataDecryptedObject, @@ -5,59 +6,25 @@ import type { IHttpRequestOptions, INode, INodeProperties, + INodeTypes, } from 'n8n-workflow'; -import { NodeConnectionType, deepCopy } from 'n8n-workflow'; -import { Workflow } from 'n8n-workflow'; -import Container from 'typedi'; +import { deepCopy, Workflow } from 'n8n-workflow'; +import { CredentialTypes } from '@/credential-types'; import { CredentialsHelper } from '@/credentials-helper'; -import { CredentialsRepository } from '@/databases/repositories/credentials.repository'; -import { SharedCredentialsRepository } from '@/databases/repositories/shared-credentials.repository'; -import { LoadNodesAndCredentials } from '@/load-nodes-and-credentials'; -import { NodeTypes } from '@/node-types'; -import { mockInstance } from '@test/mocking'; +import type { LoadNodesAndCredentials } from '@/load-nodes-and-credentials'; describe('CredentialsHelper', () => { - mockInstance(CredentialsRepository); - mockInstance(SharedCredentialsRepository); - const mockNodesAndCredentials = mockInstance(LoadNodesAndCredentials, { - loadedNodes: { - 'test.set': { - sourcePath: '', - type: { - description: { - displayName: 'Set', - name: 'set', - group: ['input'], - version: 1, - description: 'Sets a value', - defaults: { - name: 'Set', - color: '#0000FF', - }, - inputs: [NodeConnectionType.Main], - outputs: [NodeConnectionType.Main], - properties: [ - { - displayName: 'Value1', - name: 'value1', - type: 'string', - default: 'default-value1', - }, - { - displayName: 'Value2', - name: 'value2', - type: 'string', - default: 'default-value2', - }, - ], - }, - }, - }, - }, - }); + const nodeTypes = mock(); + const mockNodesAndCredentials = mock(); - const nodeTypes = mockInstance(NodeTypes); + const credentialsHelper = new CredentialsHelper( + new CredentialTypes(mockNodesAndCredentials), + mock(), + mock(), + mock(), + mock(), + ); describe('authenticate', () => { const tests: Array<{ @@ -272,19 +239,16 @@ describe('CredentialsHelper', () => { for (const testData of tests) { test(testData.description, async () => { - //@ts-expect-error `loadedCredentials` is a getter and we are replacing it here with a property - mockNodesAndCredentials.loadedCredentials = { - [testData.input.credentialType.name]: { - type: testData.input.credentialType, - sourcePath: '', - }, - }; + const { credentialType } = testData.input; - const credentialsHelper = Container.get(CredentialsHelper); + mockNodesAndCredentials.getCredential.calledWith(credentialType.name).mockReturnValue({ + type: credentialType, + sourcePath: '', + }); const result = await credentialsHelper.authenticate( testData.input.credentials, - testData.input.credentialType.name, + credentialType.name, deepCopy(incomingRequestOptions), workflow, node, diff --git a/packages/cli/src/__tests__/node-types.test.ts b/packages/cli/src/__tests__/node-types.test.ts index 11e2c5ba2b5f8..6b1984a79f0c0 100644 --- a/packages/cli/src/__tests__/node-types.test.ts +++ b/packages/cli/src/__tests__/node-types.test.ts @@ -1,95 +1,111 @@ import { mock } from 'jest-mock-extended'; -import type { INodeType, IVersionedNodeType } from 'n8n-workflow'; +import { UnrecognizedNodeTypeError } from 'n8n-core'; +import type { + LoadedClass, + INodeType, + IVersionedNodeType, + INodeTypeDescription, +} from 'n8n-workflow'; import type { LoadNodesAndCredentials } from '@/load-nodes-and-credentials'; - -import { NodeTypes } from '../node-types'; +import { NodeTypes } from '@/node-types'; describe('NodeTypes', () => { - let nodeTypes: NodeTypes; const loadNodesAndCredentials = mock(); + const nodeTypes: NodeTypes = new NodeTypes(loadNodesAndCredentials); + + const nonVersionedNode: LoadedClass = { + sourcePath: '', + type: { + description: mock({ + name: 'n8n-nodes-base.nonVersioned', + usableAsTool: undefined, + }), + }, + }; + const v1Node = mock(); + const v2Node = mock(); + const versionedNode: LoadedClass = { + sourcePath: '', + type: { + description: mock({ + name: 'n8n-nodes-base.versioned', + }), + currentVersion: 2, + nodeVersions: { + 1: v1Node, + 2: v2Node, + }, + getNodeType(version) { + if (version === 1) return v1Node; + return v2Node; + }, + }, + }; + const toolSupportingNode: LoadedClass = { + sourcePath: '', + type: { + description: mock({ + name: 'n8n-nodes-base.testNode', + displayName: 'TestNode', + usableAsTool: true, + properties: [], + }), + }, + }; + + loadNodesAndCredentials.getNode.mockImplementation((fullNodeType) => { + const [packageName, nodeType] = fullNodeType.split('.'); + if (nodeType === 'nonVersioned') return nonVersionedNode; + if (nodeType === 'versioned') return versionedNode; + if (nodeType === 'testNode') return toolSupportingNode; + throw new UnrecognizedNodeTypeError(packageName, nodeType); + }); + beforeEach(() => { jest.clearAllMocks(); - nodeTypes = new NodeTypes(loadNodesAndCredentials); + }); + + describe('getByName', () => { + it('should return node type when it exists', () => { + const result = nodeTypes.getByName('n8n-nodes-base.nonVersioned'); + expect(result).toBe(nonVersionedNode.type); + }); }); describe('getByNameAndVersion', () => { - const nodeTypeName = 'n8n-nodes-base.testNode'; + it('should throw an error if the package does not exist', () => { + expect(() => nodeTypes.getByNameAndVersion('invalid-package.unknownNode')).toThrow( + 'Unrecognized node type: invalid-package.unknownNode', + ); + }); it('should throw an error if the node-type does not exist', () => { - const nodeTypeName = 'unknownNode'; - - // @ts-expect-error overwriting a readonly property - loadNodesAndCredentials.loadedNodes = {}; - // @ts-expect-error overwriting a readonly property - loadNodesAndCredentials.knownNodes = {}; - - expect(() => nodeTypes.getByNameAndVersion(nodeTypeName)).toThrow( - 'Unrecognized node type: unknownNode', + expect(() => nodeTypes.getByNameAndVersion('n8n-nodes-base.unknownNode')).toThrow( + 'Unrecognized node type: n8n-nodes-base.unknownNode', ); }); it('should return a regular node-type without version', () => { - const nodeType = mock(); - - // @ts-expect-error overwriting a readonly property - loadNodesAndCredentials.loadedNodes = { - [nodeTypeName]: { type: nodeType }, - }; - - const result = nodeTypes.getByNameAndVersion(nodeTypeName); - - expect(result).toEqual(nodeType); + const result = nodeTypes.getByNameAndVersion('n8n-nodes-base.nonVersioned'); + expect(result).toBe(nonVersionedNode.type); }); it('should return a regular node-type with version', () => { - const nodeTypeV1 = mock(); - const nodeType = mock({ - nodeVersions: { 1: nodeTypeV1 }, - getNodeType: () => nodeTypeV1, - }); - - // @ts-expect-error overwriting a readonly property - loadNodesAndCredentials.loadedNodes = { - [nodeTypeName]: { type: nodeType }, - }; - - const result = nodeTypes.getByNameAndVersion(nodeTypeName); - - expect(result).toEqual(nodeTypeV1); + const result = nodeTypes.getByNameAndVersion('n8n-nodes-base.versioned'); + expect(result).toBe(v2Node); }); it('should throw when a node-type is requested as tool, but does not support being used as one', () => { - const nodeType = mock(); - - // @ts-expect-error overwriting a readonly property - loadNodesAndCredentials.loadedNodes = { - [nodeTypeName]: { type: nodeType }, - }; - - expect(() => nodeTypes.getByNameAndVersion(`${nodeTypeName}Tool`)).toThrow( + expect(() => nodeTypes.getByNameAndVersion('n8n-nodes-base.nonVersionedTool')).toThrow( 'Node cannot be used as a tool', ); }); it('should return the tool node-type when requested as tool', () => { - const nodeType = mock(); - // @ts-expect-error can't use a mock here - nodeType.description = { - name: nodeTypeName, - displayName: 'TestNode', - usableAsTool: true, - properties: [], - }; - - // @ts-expect-error overwriting a readonly property - loadNodesAndCredentials.loadedNodes = { - [nodeTypeName]: { type: nodeType }, - }; - - const result = nodeTypes.getByNameAndVersion(`${nodeTypeName}Tool`); - expect(result).not.toEqual(nodeType); + const result = nodeTypes.getByNameAndVersion('n8n-nodes-base.testNodeTool'); + expect(result).not.toEqual(toolSupportingNode); expect(result.description.name).toEqual('n8n-nodes-base.testNodeTool'); expect(result.description.displayName).toEqual('TestNode Tool'); expect(result.description.codex?.categories).toContain('AI'); @@ -97,4 +113,47 @@ describe('NodeTypes', () => { expect(result.description.outputs).toEqual(['ai_tool']); }); }); + + describe('getWithSourcePath', () => { + it('should return description and source path for existing node', () => { + const result = nodeTypes.getWithSourcePath('n8n-nodes-base.nonVersioned', 1); + expect(result).toHaveProperty('description'); + expect(result).toHaveProperty('sourcePath'); + expect(result.sourcePath).toBe(nonVersionedNode.sourcePath); + }); + + it('should throw error for non-existent node', () => { + expect(() => nodeTypes.getWithSourcePath('n8n-nodes-base.nonExistent', 1)).toThrow( + 'Unrecognized node type: n8n-nodes-base.nonExistent', + ); + }); + }); + + describe('getKnownTypes', () => { + it('should return known node types', () => { + // @ts-expect-error readonly property + loadNodesAndCredentials.knownNodes = ['n8n-nodes-base.nonVersioned']; + const result = nodeTypes.getKnownTypes(); + expect(result).toEqual(['n8n-nodes-base.nonVersioned']); + }); + }); + + describe('getNodeTypeDescriptions', () => { + it('should return descriptions for valid node types', () => { + const nodeTypes = new NodeTypes(loadNodesAndCredentials); + const result = nodeTypes.getNodeTypeDescriptions([ + { name: 'n8n-nodes-base.nonVersioned', version: 1 }, + ]); + + expect(result).toHaveLength(1); + expect(result[0].name).toBe('n8n-nodes-base.nonVersioned'); + }); + + it('should throw error for invalid node type', () => { + const nodeTypes = new NodeTypes(loadNodesAndCredentials); + expect(() => + nodeTypes.getNodeTypeDescriptions([{ name: 'n8n-nodes-base.nonExistent', version: 1 }]), + ).toThrow('Unrecognized node type: n8n-nodes-base.nonExistent'); + }); + }); }); diff --git a/packages/cli/src/credential-types.ts b/packages/cli/src/credential-types.ts index 24e2d9f2bac4f..a6d3f29eb0035 100644 --- a/packages/cli/src/credential-types.ts +++ b/packages/cli/src/credential-types.ts @@ -1,13 +1,6 @@ -import { loadClassInIsolation } from 'n8n-core'; -import { - ApplicationError, - type ICredentialType, - type ICredentialTypes, - type LoadedClass, -} from 'n8n-workflow'; +import type { ICredentialType, ICredentialTypes } from 'n8n-workflow'; import { Service } from 'typedi'; -import { RESPONSE_ERROR_MESSAGES } from '@/constants'; import { LoadNodesAndCredentials } from '@/load-nodes-and-credentials'; @Service() @@ -20,7 +13,7 @@ export class CredentialTypes implements ICredentialTypes { } getByName(credentialType: string): ICredentialType { - return this.getCredential(credentialType).type; + return this.loadNodesAndCredentials.getCredential(credentialType).type; } getSupportedNodes(type: string): string[] { @@ -39,21 +32,4 @@ export class CredentialTypes implements ICredentialTypes { } return extendsArr; } - - private getCredential(type: string): LoadedClass { - const { loadedCredentials, knownCredentials } = this.loadNodesAndCredentials; - if (type in loadedCredentials) { - return loadedCredentials[type]; - } - - if (type in knownCredentials) { - const { className, sourcePath } = knownCredentials[type]; - const loaded: ICredentialType = loadClassInIsolation(sourcePath, className); - loadedCredentials[type] = { sourcePath, type: loaded }; - return loadedCredentials[type]; - } - throw new ApplicationError(RESPONSE_ERROR_MESSAGES.NO_CREDENTIAL, { - tags: { credentialType: type }, - }); - } } diff --git a/packages/cli/src/databases/repositories/installed-packages.repository.ts b/packages/cli/src/databases/repositories/installed-packages.repository.ts index 50f9e7ad4fa2c..77faf9681747b 100644 --- a/packages/cli/src/databases/repositories/installed-packages.repository.ts +++ b/packages/cli/src/databases/repositories/installed-packages.repository.ts @@ -35,7 +35,7 @@ export class InstalledPackagesRepository extends Repository { for (const loadedNode of loadedNodes) { const installedNode = this.installedNodesRepository.create({ name: nodeTypes[loadedNode.name].type.description.displayName, - type: loadedNode.name, + type: `${packageName}.${loadedNode.name}`, latestVersion: loadedNode.version, package: { packageName }, }); diff --git a/packages/cli/src/load-nodes-and-credentials.ts b/packages/cli/src/load-nodes-and-credentials.ts index 22273fb89462d..0e38affd0fcbf 100644 --- a/packages/cli/src/load-nodes-and-credentials.ts +++ b/packages/cli/src/load-nodes-and-credentials.ts @@ -8,6 +8,8 @@ import { CustomDirectoryLoader, PackageDirectoryLoader, LazyPackageDirectoryLoader, + UnrecognizedCredentialTypeError, + UnrecognizedNodeTypeError, } from 'n8n-core'; import type { KnownNodesAndCredentials, @@ -15,6 +17,10 @@ import type { INodeTypeDescription, INodeTypeData, ICredentialTypeData, + LoadedClass, + ICredentialType, + INodeType, + IVersionedNodeType, } from 'n8n-workflow'; import { NodeHelpers, ApplicationError, ErrorReporterProxy as ErrorReporter } from 'n8n-workflow'; import path from 'path'; @@ -307,13 +313,18 @@ export class LoadNodesAndCredentials { for (const loader of Object.values(this.loaders)) { // list of node & credential types that will be sent to the frontend - const { known, types, directory } = loader; - this.types.nodes = this.types.nodes.concat(types.nodes); + const { known, types, directory, packageName } = loader; + this.types.nodes = this.types.nodes.concat( + types.nodes.map(({ name, ...rest }) => ({ + ...rest, + name: `${packageName}.${name}`, + })), + ); this.types.credentials = this.types.credentials.concat(types.credentials); // Nodes and credentials that have been loaded immediately for (const nodeTypeName in loader.nodeTypes) { - this.loaded.nodes[nodeTypeName] = loader.nodeTypes[nodeTypeName]; + this.loaded.nodes[`${packageName}.${nodeTypeName}`] = loader.nodeTypes[nodeTypeName]; } for (const credentialTypeName in loader.credentialTypes) { @@ -322,7 +333,7 @@ export class LoadNodesAndCredentials { for (const type in known.nodes) { const { className, sourcePath } = known.nodes[type]; - this.known.nodes[type] = { + this.known.nodes[`${packageName}.${type}`] = { className, sourcePath: path.join(directory, sourcePath), }; @@ -356,6 +367,33 @@ export class LoadNodesAndCredentials { } } + getNode(fullNodeType: string): LoadedClass { + const [packageName, nodeType] = fullNodeType.split('.'); + const { loaders } = this; + const loader = loaders[packageName]; + if (!loader) { + throw new UnrecognizedNodeTypeError(packageName, nodeType); + } + return loader.getNode(nodeType); + } + + getCredential(credentialType: string): LoadedClass { + const { loadedCredentials } = this; + + for (const loader of Object.values(this.loaders)) { + if (credentialType in loader.known.credentials) { + const loaded = loader.getCredential(credentialType); + loadedCredentials[credentialType] = loaded; + } + } + + if (credentialType in loadedCredentials) { + return loadedCredentials[credentialType]; + } + + throw new UnrecognizedCredentialTypeError(credentialType); + } + async setupHotReload() { const { default: debounce } = await import('lodash/debounce'); // eslint-disable-next-line import/no-extraneous-dependencies diff --git a/packages/cli/src/node-types.ts b/packages/cli/src/node-types.ts index 553aedd620e2e..72584ac5022cf 100644 --- a/packages/cli/src/node-types.ts +++ b/packages/cli/src/node-types.ts @@ -1,26 +1,16 @@ import type { NeededNodeType } from '@n8n/task-runner'; import type { Dirent } from 'fs'; import { readdir } from 'fs/promises'; -import { loadClassInIsolation } from 'n8n-core'; -import type { - INodeType, - INodeTypeDescription, - INodeTypes, - IVersionedNodeType, - LoadedClass, -} from 'n8n-workflow'; +import type { INodeType, INodeTypeDescription, INodeTypes, IVersionedNodeType } from 'n8n-workflow'; import { ApplicationError, NodeHelpers } from 'n8n-workflow'; import { join, dirname } from 'path'; import { Service } from 'typedi'; -import { UnrecognizedNodeTypeError } from './errors/unrecognized-node-type.error'; import { LoadNodesAndCredentials } from './load-nodes-and-credentials'; @Service() export class NodeTypes implements INodeTypes { - constructor(private loadNodesAndCredentials: LoadNodesAndCredentials) { - loadNodesAndCredentials.addPostProcessor(async () => this.applySpecialNodeParameters()); - } + constructor(private loadNodesAndCredentials: LoadNodesAndCredentials) {} /** * Variant of `getByNameAndVersion` that includes the node's source path, used to locate a node's translations. @@ -29,19 +19,14 @@ export class NodeTypes implements INodeTypes { nodeTypeName: string, version: number, ): { description: INodeTypeDescription } & { sourcePath: string } { - const nodeType = this.getNode(nodeTypeName); - - if (!nodeType) { - throw new ApplicationError('Unknown node type', { tags: { nodeTypeName } }); - } - + const nodeType = this.loadNodesAndCredentials.getNode(nodeTypeName); const { description } = NodeHelpers.getVersionedNodeType(nodeType.type, version); return { description: { ...description }, sourcePath: nodeType.sourcePath }; } getByName(nodeType: string): INodeType | IVersionedNodeType { - return this.getNode(nodeType).type; + return this.loadNodesAndCredentials.getNode(nodeType).type; } getByNameAndVersion(nodeType: string, version?: number): INodeType { @@ -52,7 +37,7 @@ export class NodeTypes implements INodeTypes { nodeType = nodeType.replace(/Tool$/, ''); } - const node = this.getNode(nodeType); + const node = this.loadNodesAndCredentials.getNode(nodeType); const versionedNodeType = NodeHelpers.getVersionedNodeType(node.type, version); if (!toolRequested) return versionedNodeType; @@ -79,36 +64,10 @@ export class NodeTypes implements INodeTypes { return tool; } - /* Some nodeTypes need to get special parameters applied like the polling nodes the polling times */ - applySpecialNodeParameters() { - for (const nodeTypeData of Object.values(this.loadNodesAndCredentials.loadedNodes)) { - const nodeType = NodeHelpers.getVersionedNodeType(nodeTypeData.type); - NodeHelpers.applySpecialNodeParameters(nodeType); - } - } - getKnownTypes() { return this.loadNodesAndCredentials.knownNodes; } - private getNode(type: string): LoadedClass { - const { loadedNodes, knownNodes } = this.loadNodesAndCredentials; - if (type in loadedNodes) { - return loadedNodes[type]; - } - - if (type in knownNodes) { - const { className, sourcePath } = knownNodes[type]; - const loaded: INodeType = loadClassInIsolation(sourcePath, className); - NodeHelpers.applySpecialNodeParameters(loaded); - - loadedNodes[type] = { sourcePath, type: loaded }; - return loadedNodes[type]; - } - - throw new UnrecognizedNodeTypeError(type); - } - async getNodeTranslationPath({ nodeSourcePath, longNodeType, @@ -153,14 +112,12 @@ export class NodeTypes implements INodeTypes { getNodeTypeDescriptions(nodeTypes: NeededNodeType[]): INodeTypeDescription[] { return nodeTypes.map(({ name: nodeTypeName, version: nodeTypeVersion }) => { - const nodeType = this.getNode(nodeTypeName); - - if (!nodeType) throw new ApplicationError(`Unknown node type: ${nodeTypeName}`); - + const nodeType = this.loadNodesAndCredentials.getNode(nodeTypeName); const { description } = NodeHelpers.getVersionedNodeType(nodeType.type, nodeTypeVersion); const descriptionCopy = { ...description }; + // TODO: do we still need this? descriptionCopy.name = descriptionCopy.name.startsWith('n8n-nodes') ? descriptionCopy.name : `n8n-nodes-base.${descriptionCopy.name}`; // nodes-base nodes are unprefixed diff --git a/packages/cli/test/integration/active-workflow-manager.test.ts b/packages/cli/test/integration/active-workflow-manager.test.ts index 8ea790ade7209..24b6b5469a225 100644 --- a/packages/cli/test/integration/active-workflow-manager.test.ts +++ b/packages/cli/test/integration/active-workflow-manager.test.ts @@ -1,6 +1,6 @@ import { mock } from 'jest-mock-extended'; import type { InstanceSettings } from 'n8n-core'; -import { NodeApiError, NodeOperationError, Workflow } from 'n8n-workflow'; +import { NodeApiError, Workflow } from 'n8n-workflow'; import type { IWebhookData, WorkflowActivateMode } from 'n8n-workflow'; import { Container } from 'typedi'; @@ -10,7 +10,6 @@ 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 { LoadNodesAndCredentials } from '@/load-nodes-and-credentials'; import { NodeTypes } from '@/node-types'; import { Push } from '@/push'; import { SecretsHelper } from '@/secrets-helpers'; @@ -22,6 +21,7 @@ import { WorkflowService } from '@/workflows/workflow.service'; import { createOwner } from './shared/db/users'; import { createWorkflow } from './shared/db/workflows'; import * as testDb from './shared/test-db'; +import * as utils from './shared/utils/'; import { mockInstance } from '../shared/mocking'; mockInstance(ActiveExecutions); @@ -30,21 +30,6 @@ mockInstance(SecretsHelper); mockInstance(ExecutionService); mockInstance(WorkflowService); -const loader = mockInstance(LoadNodesAndCredentials); - -Object.assign(loader.loadedNodes, { - 'n8n-nodes-base.scheduleTrigger': { - type: { - description: { - displayName: 'Schedule Trigger', - name: 'scheduleTrigger', - properties: [], - }, - trigger: async () => {}, - }, - }, -}); - const webhookService = mockInstance(WebhookService); const externalHooks = mockInstance(ExternalHooks); @@ -58,15 +43,17 @@ beforeAll(async () => { activeWorkflowManager = Container.get(ActiveWorkflowManager); + await utils.initNodeTypes(); + const owner = await createOwner(); createActiveWorkflow = async () => await createWorkflow({ active: true }, owner); createInactiveWorkflow = async () => await createWorkflow({ active: false }, owner); }); afterEach(async () => { - await testDb.truncate(['Workflow', 'Webhook']); await activeWorkflowManager.removeAll(); - jest.restoreAllMocks(); + await testDb.truncate(['Workflow', 'Webhook']); + jest.clearAllMocks(); }); afterAll(async () => { @@ -206,22 +193,22 @@ describe('remove()', () => { }); describe('executeErrorWorkflow()', () => { - it('should delegate to `WorkflowExecuteAdditionalData`', async () => { - const dbWorkflow = await createActiveWorkflow(); - const [node] = dbWorkflow.nodes; + // it('should delegate to `WorkflowExecuteAdditionalData`', async () => { + // const dbWorkflow = await createActiveWorkflow(); + // const [node] = dbWorkflow.nodes; - const executeSpy = jest.spyOn(AdditionalData, 'executeErrorWorkflow'); + // const executeSpy = jest.spyOn(AdditionalData, 'executeErrorWorkflow'); - await activeWorkflowManager.init(); + // await activeWorkflowManager.init(); - activeWorkflowManager.executeErrorWorkflow( - new NodeOperationError(node, 'Something went wrong'), - dbWorkflow, - 'trigger', - ); + // activeWorkflowManager.executeErrorWorkflow( + // new NodeOperationError(node, 'Something went wrong'), + // dbWorkflow, + // 'trigger', + // ); - expect(executeSpy).toHaveBeenCalledTimes(1); - }); + // expect(executeSpy).toHaveBeenCalledTimes(1); + // }); it('should be called on failure to activate due to 401', async () => { const dbWorkflow = await createActiveWorkflow(); diff --git a/packages/cli/test/integration/public-api/workflows.test.ts b/packages/cli/test/integration/public-api/workflows.test.ts index 687b29da6af8d..5425455aca790 100644 --- a/packages/cli/test/integration/public-api/workflows.test.ts +++ b/packages/cli/test/integration/public-api/workflows.test.ts @@ -528,8 +528,28 @@ describe('POST /workflows/:id/activate', () => { expect(response.statusCode).toBe(404); }); + test('should fail due to trying to activate a workflow without any nodes', async () => { + const workflow = await createWorkflow({ nodes: [] }, owner); + const response = await authOwnerAgent.post(`/workflows/${workflow.id}/activate`); + expect(response.statusCode).toBe(400); + }); + test('should fail due to trying to activate a workflow without a trigger', async () => { - const workflow = await createWorkflow({}, owner); + const workflow = await createWorkflow( + { + nodes: [ + { + id: 'uuid-1234', + name: 'Start', + parameters: {}, + position: [-20, 260], + type: 'n8n-nodes-base.start', + typeVersion: 1, + }, + ], + }, + owner, + ); const response = await authOwnerAgent.post(`/workflows/${workflow.id}/activate`); expect(response.statusCode).toBe(400); }); diff --git a/packages/cli/test/integration/shared/utils/community-nodes.ts b/packages/cli/test/integration/shared/utils/community-nodes.ts index 7734f707620fa..d29a9361be31e 100644 --- a/packages/cli/test/integration/shared/utils/community-nodes.ts +++ b/packages/cli/test/integration/shared/utils/community-nodes.ts @@ -22,7 +22,7 @@ export const mockNode = (packageName: string) => { return Container.get(InstalledNodesRepository).create({ name: nodeName, - type: nodeName, + type: `${packageName}.${nodeName}`, latestVersion: COMMUNITY_NODE_VERSION.CURRENT, package: { packageName }, }); diff --git a/packages/cli/test/integration/shared/utils/index.ts b/packages/cli/test/integration/shared/utils/index.ts index 78de2c1b25361..d8225a415dd7d 100644 --- a/packages/cli/test/integration/shared/utils/index.ts +++ b/packages/cli/test/integration/shared/utils/index.ts @@ -1,10 +1,12 @@ -import { BinaryDataService } from 'n8n-core'; +import { mock } from 'jest-mock-extended'; +import { BinaryDataService, UnrecognizedNodeTypeError, type DirectoryLoader } from 'n8n-core'; import { Ftp } from 'n8n-nodes-base/credentials/Ftp.credentials'; import { GithubApi } from 'n8n-nodes-base/credentials/GithubApi.credentials'; import { Cron } from 'n8n-nodes-base/nodes/Cron/Cron.node'; +import { ScheduleTrigger } from 'n8n-nodes-base/nodes/Schedule/ScheduleTrigger.node'; import { Set } from 'n8n-nodes-base/nodes/Set/Set.node'; import { Start } from 'n8n-nodes-base/nodes/Start/Start.node'; -import { type INode } from 'n8n-workflow'; +import type { INodeTypeData, INode } from 'n8n-workflow'; import type request from 'supertest'; import { Container } from 'typedi'; import { v4 as uuid } from 'uuid'; @@ -62,7 +64,8 @@ export async function initCredentialsTypes(): Promise { * Initialize node types. */ export async function initNodeTypes() { - Container.get(LoadNodesAndCredentials).loaded.nodes = { + ScheduleTrigger.prototype.trigger = async () => ({}); + const nodes: INodeTypeData = { 'n8n-nodes-base.start': { type: new Start(), sourcePath: '', @@ -75,7 +78,21 @@ export async function initNodeTypes() { type: new Set(), sourcePath: '', }, + 'n8n-nodes-base.scheduleTrigger': { + type: new ScheduleTrigger(), + sourcePath: '', + }, }; + const loader = mock(); + loader.getNode.mockImplementation((nodeType) => { + const node = nodes[`n8n-nodes-base.${nodeType}`]; + if (!node) throw new UnrecognizedNodeTypeError('n8n-nodes-base', nodeType); + return node; + }); + + const loadNodesAndCredentials = Container.get(LoadNodesAndCredentials); + loadNodesAndCredentials.loaders = { 'n8n-nodes-base': loader }; + loadNodesAndCredentials.loaded.nodes = nodes; } /** diff --git a/packages/core/bin/generate-known b/packages/core/bin/generate-known deleted file mode 100755 index 3784f15fb1f8f..0000000000000 --- a/packages/core/bin/generate-known +++ /dev/null @@ -1,99 +0,0 @@ -#!/usr/bin/env node - -const path = require('path'); -const glob = require('fast-glob'); -const uniq = require('lodash/uniq'); -const { LoggerProxy, getCredentialsForNode } = require('n8n-workflow'); -const { packageDir, writeJSON } = require('./common'); -const { loadClassInIsolation } = require('../dist/ClassLoader'); - -LoggerProxy.init(console); - -const loadClass = (sourcePath) => { - try { - const [className] = path.parse(sourcePath).name.split('.'); - const filePath = path.resolve(packageDir, sourcePath); - const instance = loadClassInIsolation(filePath, className); - return { instance, sourcePath, className }; - } catch (e) { - LoggerProxy.warn(`Failed to load ${sourcePath}: ${e.message}`); - } -}; - -const generateKnownNodes = async () => { - const nodeClasses = glob - .sync('dist/nodes/**/*.node.js', { cwd: packageDir }) - .map(loadClass) - // Ignore node versions - .filter((nodeClass) => nodeClass && !/[vV]\d.node\.js$/.test(nodeClass.sourcePath)); - - const nodes = {}; - const nodesByCredential = {}; - - for (const { className, sourcePath, instance } of nodeClasses) { - const nodeName = instance.description.name; - nodes[nodeName] = { className, sourcePath }; - - for (const credential of getCredentialsForNode(instance)) { - if (!nodesByCredential[credential.name]) { - nodesByCredential[credential.name] = []; - } - - nodesByCredential[credential.name].push(nodeName); - } - } - - LoggerProxy.info(`Detected ${Object.keys(nodes).length} nodes`); - await writeJSON('known/nodes.json', nodes); - return { nodes, nodesByCredential }; -}; - -const generateKnownCredentials = async (nodesByCredential) => { - const credentialClasses = glob - .sync(`dist/credentials/**/*.credentials.js`, { cwd: packageDir }) - .map(loadClass) - .filter((data) => !!data); - - for (const { instance } of credentialClasses) { - if (Array.isArray(instance.extends)) { - for (const extendedCredential of instance.extends) { - nodesByCredential[extendedCredential] = [ - ...(nodesByCredential[extendedCredential] ?? []), - ...(nodesByCredential[instance.name] ?? []), - ]; - } - } - } - - const credentials = credentialClasses.reduce( - (credentials, { className, sourcePath, instance }) => { - const credentialName = instance.name; - const credential = { - className, - sourcePath, - }; - - if (Array.isArray(instance.extends)) { - credential.extends = instance.extends; - } - - if (nodesByCredential[credentialName]) { - credential.supportedNodes = Array.from(new Set(nodesByCredential[credentialName])); - } - - credentials[credentialName] = credential; - - return credentials; - }, - {}, - ); - - LoggerProxy.info(`Detected ${Object.keys(credentials).length} credentials`); - await writeJSON('known/credentials.json', credentials); - return credentials; -}; - -(async () => { - const { nodesByCredential } = await generateKnownNodes(); - await generateKnownCredentials(nodesByCredential); -})(); diff --git a/packages/core/bin/generate-ui-types b/packages/core/bin/generate-metadata similarity index 59% rename from packages/core/bin/generate-ui-types rename to packages/core/bin/generate-metadata index f73ca87a15eca..18dbca687a2b3 100755 --- a/packages/core/bin/generate-ui-types +++ b/packages/core/bin/generate-metadata @@ -1,6 +1,6 @@ #!/usr/bin/env node -const { LoggerProxy, NodeHelpers } = require('n8n-workflow'); +const { LoggerProxy } = require('n8n-workflow'); const { PackageDirectoryLoader } = require('../dist/DirectoryLoader'); const { packageDir, writeJSON } = require('./common'); @@ -33,7 +33,7 @@ function findReferencedMethods(obj, refs = {}, latestName = '') { const loaderNodeTypes = Object.values(loader.nodeTypes); const definedMethods = loaderNodeTypes.reduce((acc, cur) => { - NodeHelpers.getVersionedNodeTypeAll(cur.type).forEach((type) => { + loader.getVersionedNodeTypeAll(cur.type).forEach((type) => { const methods = type.description?.__loadOptionsMethods; if (!methods) return; @@ -52,51 +52,21 @@ function findReferencedMethods(obj, refs = {}, latestName = '') { }, {}); const nodeTypes = loaderNodeTypes - .map((data) => { - const nodeType = NodeHelpers.getVersionedNodeType(data.type); - NodeHelpers.applySpecialNodeParameters(nodeType); - return data.type; - }) + .map(({ type }) => type) .flatMap((nodeType) => - NodeHelpers.getVersionedNodeTypeAll(nodeType).map((item) => { + loader.getVersionedNodeTypeAll(nodeType).map((item) => { const { __loadOptionsMethods, ...rest } = item.description; return rest; }), ); const knownCredentials = loader.known.credentials; - const credentialTypes = Object.values(loader.credentialTypes).map((data) => { - const credentialType = data.type; - const supportedNodes = knownCredentials[credentialType.name].supportedNodes ?? []; - if (supportedNodes.length > 0 && credentialType.httpRequestNode) { - credentialType.httpRequestNode.hidden = true; - } - - credentialType.supportedNodes = supportedNodes; - - if (!credentialType.iconUrl && !credentialType.icon) { - for (const supportedNode of supportedNodes) { - const nodeType = loader.nodeTypes[supportedNode]?.type.description; - - if (!nodeType) continue; - if (nodeType.icon) { - credentialType.icon = nodeType.icon; - credentialType.iconColor = nodeType.iconColor; - break; - } - if (nodeType.iconUrl) { - credentialType.iconUrl = nodeType.iconUrl; - break; - } - } - } - - return credentialType; - }); - + const credentialTypes = Object.values(loader.credentialTypes).map(({ type }) => type); const referencedMethods = findReferencedMethods(nodeTypes); await Promise.all([ + writeJSON('known/nodes.json', loader.known.nodes), + writeJSON('known/credentials.json', loader.known.credentials), writeJSON('types/credentials.json', credentialTypes), writeJSON('types/nodes.json', nodeTypes), writeJSON('methods/defined.json', definedMethods), diff --git a/packages/core/package.json b/packages/core/package.json index d334e118f1c5a..e0bfe88e6dec8 100644 --- a/packages/core/package.json +++ b/packages/core/package.json @@ -6,9 +6,8 @@ "types": "dist/index.d.ts", "bin": { "n8n-copy-icons": "./bin/copy-icons", - "n8n-generate-known": "./bin/generate-known", "n8n-generate-translations": "./bin/generate-translations", - "n8n-generate-ui-types": "./bin/generate-ui-types" + "n8n-generate-metadata": "./bin/generate-metadata" }, "scripts": { "clean": "rimraf dist .turbo", diff --git a/packages/core/src/DirectoryLoader.ts b/packages/core/src/DirectoryLoader.ts index b0e77125a7cc3..d73fd1608026d 100644 --- a/packages/core/src/DirectoryLoader.ts +++ b/packages/core/src/DirectoryLoader.ts @@ -1,9 +1,11 @@ import glob from 'fast-glob'; +import uniqBy from 'lodash/uniqBy'; import type { CodexData, DocumentationLink, ICredentialType, ICredentialTypeData, + INodeCredentialDescription, INodeType, INodeTypeBaseDescription, INodeTypeData, @@ -12,19 +14,15 @@ import type { IVersionedNodeType, KnownNodesAndCredentials, } from 'n8n-workflow'; -import { - ApplicationError, - LoggerProxy as Logger, - getCredentialsForNode, - getVersionedNodeTypeAll, - jsonParse, -} from 'n8n-workflow'; +import { ApplicationError, LoggerProxy as Logger, NodeHelpers, jsonParse } from 'n8n-workflow'; import { readFileSync } from 'node:fs'; import { readFile } from 'node:fs/promises'; import * as path from 'path'; import { loadClassInIsolation } from './ClassLoader'; import { CUSTOM_NODES_CATEGORY } from './Constants'; +import { UnrecognizedCredentialTypeError } from './errors/unrecognized-credential-type.error'; +import { UnrecognizedNodeTypeError } from './errors/unrecognized-node-type.error'; import type { n8n } from './Interfaces'; function toJSON(this: ICredentialType) { @@ -34,11 +32,25 @@ function toJSON(this: ICredentialType) { }; } +type Codex = { + categories: string[]; + subcategories: { [subcategory: string]: string[] }; + resources: { + primaryDocumentation: DocumentationLink[]; + credentialDocumentation: DocumentationLink[]; + }; + alias: string[]; +}; + export type Types = { nodes: INodeTypeBaseDescription[]; credentials: ICredentialType[]; }; +/** + * Base class for loading n8n nodes and credentials from a directory. + * Handles the common functionality for resolving paths, loading classes, and managing node and credential types. + */ export abstract class DirectoryLoader { isLazyLoaded = false; @@ -58,7 +70,7 @@ export abstract class DirectoryLoader { // Stores the different versions with their individual descriptions types: Types = { nodes: [], credentials: [] }; - protected nodesByCredential: Record = {}; + readonly nodesByCredential: Record = {}; constructor( readonly directory: string, @@ -82,35 +94,40 @@ export abstract class DirectoryLoader { return path.resolve(this.directory, file); } - protected loadNodeFromFile(nodeName: string, filePath: string) { - let tempNode: INodeType | IVersionedNodeType; - let nodeVersion = 1; - const isCustom = this.packageName === 'CUSTOM'; - + private loadClass(sourcePath: string) { + const filePath = this.resolvePath(sourcePath); + const [className] = path.parse(sourcePath).name.split('.'); try { - tempNode = loadClassInIsolation(filePath, nodeName); - this.addCodex({ node: tempNode, filePath, isCustom }); + return loadClassInIsolation(filePath, className); } catch (error) { - Logger.error( - `Error loading node "${nodeName}" from: "${filePath}" - ${(error as Error).message}`, - ); - throw error; + throw error instanceof TypeError + ? new ApplicationError( + 'Class could not be found. Please check if the class is named correctly.', + { extra: { className } }, + ) + : error; } + } - const fullNodeName = `${this.packageName}.${tempNode.description.name}`; + /** Loads a nodes class from a file, fixes icons, and augments the codex */ + loadNodeFromFile(filePath: string) { + const tempNode = this.loadClass(filePath); + this.addCodex(tempNode, filePath); - if (this.includeNodes.length && !this.includeNodes.includes(fullNodeName)) { + const nodeType = tempNode.description.name; + const fullNodeType = `${this.packageName}.${nodeType}`; + + if (this.includeNodes.length && !this.includeNodes.includes(fullNodeType)) { return; } - if (this.excludeNodes.includes(fullNodeName)) { + if (this.excludeNodes.includes(fullNodeType)) { return; } - tempNode.description.name = fullNodeName; - this.fixIconPaths(tempNode.description, filePath); + let nodeVersion = 1; if ('nodeVersions' in tempNode) { for (const versionNode of Object.values(tempNode.nodeVersions)) { this.fixIconPaths(versionNode.description, filePath); @@ -118,85 +135,93 @@ export abstract class DirectoryLoader { for (const version of Object.values(tempNode.nodeVersions)) { this.addLoadOptionsMethods(version); + NodeHelpers.applySpecialNodeParameters(version); } const currentVersionNode = tempNode.nodeVersions[tempNode.currentVersion]; - this.addCodex({ node: currentVersionNode, filePath, isCustom }); + this.addCodex(currentVersionNode, filePath); nodeVersion = tempNode.currentVersion; if (currentVersionNode.hasOwnProperty('executeSingle')) { throw new ApplicationError( '"executeSingle" has been removed. Please update the code of this node to use "execute" instead.', - { extra: { nodeName: `${this.packageName}.${nodeName}` } }, + { extra: { nodeType: fullNodeType } }, ); } } else { this.addLoadOptionsMethods(tempNode); - // Short renaming to avoid type issues + NodeHelpers.applySpecialNodeParameters(tempNode); + // Short renaming to avoid type issues nodeVersion = Array.isArray(tempNode.description.version) ? tempNode.description.version.slice(-1)[0] : tempNode.description.version; } - this.known.nodes[fullNodeName] = { - className: nodeName, + this.known.nodes[nodeType] = { + className: tempNode.constructor.name, sourcePath: filePath, }; - this.nodeTypes[fullNodeName] = { + this.nodeTypes[nodeType] = { type: tempNode, sourcePath: filePath, }; this.loadedNodes.push({ - name: fullNodeName, + name: nodeType, version: nodeVersion, }); - getVersionedNodeTypeAll(tempNode).forEach(({ description }) => { + this.getVersionedNodeTypeAll(tempNode).forEach(({ description }) => { this.types.nodes.push(description); }); - for (const credential of getCredentialsForNode(tempNode)) { + for (const credential of this.getCredentialsForNode(tempNode)) { if (!this.nodesByCredential[credential.name]) { this.nodesByCredential[credential.name] = []; } - this.nodesByCredential[credential.name].push(fullNodeName); + this.nodesByCredential[credential.name].push(nodeType); } } - protected loadCredentialFromFile(credentialClassName: string, filePath: string): void { - let tempCredential: ICredentialType; - try { - tempCredential = loadClassInIsolation(filePath, credentialClassName); - - // Add serializer method "toJSON" to the class so that authenticate method (if defined) - // gets mapped to the authenticate attribute before it is sent to the client. - // The authenticate property is used by the client to decide whether or not to - // include the credential type in the predefined credentials (HTTP node) - Object.assign(tempCredential, { toJSON }); + getNode(nodeType: string) { + const { + nodeTypes, + known: { nodes: knownNodes }, + } = this; + if (!(nodeType in nodeTypes) && nodeType in knownNodes) { + const { sourcePath } = knownNodes[nodeType]; + this.loadNodeFromFile(sourcePath); + } - this.fixIconPaths(tempCredential, filePath); - } catch (e) { - if (e instanceof TypeError) { - throw new ApplicationError( - 'Class could not be found. Please check if the class is named correctly.', - { extra: { credentialClassName } }, - ); - } else { - throw e; - } + if (nodeType in nodeTypes) { + return nodeTypes[nodeType]; } - this.known.credentials[tempCredential.name] = { - className: credentialClassName, + throw new UnrecognizedNodeTypeError(this.packageName, nodeType); + } + + /** Loads a credential class from a file, and fixes icons */ + loadCredentialFromFile(filePath: string): void { + const tempCredential = this.loadClass(filePath); + // Add serializer method "toJSON" to the class so that authenticate method (if defined) + // gets mapped to the authenticate attribute before it is sent to the client. + // The authenticate property is used by the client to decide whether or not to + // include the credential type in the predefined credentials (HTTP node) + Object.assign(tempCredential, { toJSON }); + + this.fixIconPaths(tempCredential, filePath); + + const credentialType = tempCredential.name; + this.known.credentials[credentialType] = { + className: tempCredential.constructor.name, sourcePath: filePath, extends: tempCredential.extends, - supportedNodes: this.nodesByCredential[tempCredential.name], + supportedNodes: this.nodesByCredential[credentialType], }; - this.credentialTypes[tempCredential.name] = { + this.credentialTypes[credentialType] = { type: tempCredential, sourcePath: filePath, }; @@ -204,40 +229,79 @@ export abstract class DirectoryLoader { this.types.credentials.push(tempCredential); } + getCredential(credentialType: string) { + const { + credentialTypes, + known: { credentials: knownCredentials }, + } = this; + if (!(credentialType in credentialTypes) && credentialType in knownCredentials) { + const { sourcePath } = knownCredentials[credentialType]; + this.loadCredentialFromFile(sourcePath); + } + + if (credentialType in credentialTypes) { + return credentialTypes[credentialType]; + } + + throw new UnrecognizedCredentialTypeError(credentialType); + } + + /** + * Returns an array of credential descriptions that are supported by a node. + * For versioned nodes, combines and deduplicates credentials from all versions. + */ + getCredentialsForNode(object: IVersionedNodeType | INodeType): INodeCredentialDescription[] { + if ('nodeVersions' in object) { + const credentials = Object.values(object.nodeVersions).flatMap( + ({ description }) => description.credentials ?? [], + ); + return uniqBy(credentials, 'name'); + } + return object.description.credentials ?? []; + } + + /** + * Returns an array of all versions of a node type. + * For non-versioned nodes, returns an array with just that node. + * For versioned nodes, returns all available versions. + */ + getVersionedNodeTypeAll(object: IVersionedNodeType | INodeType): INodeType[] { + if ('nodeVersions' in object) { + const nodeVersions = Object.values(object.nodeVersions).map((element) => { + element.description.name = object.description.name; + element.description.codex = object.description.codex; + return element; + }); + return uniqBy(nodeVersions.reverse(), (node) => { + const { version } = node.description; + return Array.isArray(version) ? version.join(',') : version.toString(); + }); + } + return [object]; + } + /** * Retrieves `categories`, `subcategories` and alias (if defined) * from the codex data for the node at the given file path. */ private getCodex(filePath: string): CodexData { - type Codex = { - categories: string[]; - subcategories: { [subcategory: string]: string[] }; - resources: { - primaryDocumentation: DocumentationLink[]; - credentialDocumentation: DocumentationLink[]; - }; - alias: string[]; - }; - - const codexFilePath = `${filePath}on`; // .js to .json + const codexFilePath = this.resolvePath(`${filePath}on`); // .js to .json const { categories, subcategories, - resources: allResources, + resources: { primaryDocumentation, credentialDocumentation }, alias, } = module.require(codexFilePath) as Codex; - const resources = { - primaryDocumentation: allResources.primaryDocumentation, - credentialDocumentation: allResources.credentialDocumentation, - }; - return { ...(categories && { categories }), ...(subcategories && { subcategories }), - ...(resources && { resources }), ...(alias && { alias }), + resources: { + primaryDocumentation, + credentialDocumentation, + }, }; } @@ -245,15 +309,8 @@ export abstract class DirectoryLoader { * Adds a node codex `categories` and `subcategories` (if defined) * to a node description `codex` property. */ - private addCodex({ - node, - filePath, - isCustom, - }: { - node: INodeType | IVersionedNodeType; - filePath: string; - isCustom: boolean; - }) { + private addCodex(node: INodeType | IVersionedNodeType, filePath: string) { + const isCustom = this.packageName === 'CUSTOM'; try { let codex; @@ -273,7 +330,7 @@ export abstract class DirectoryLoader { node.description.codex = codex; } catch { - Logger.debug(`No codex available for: ${filePath.split('/').pop() ?? ''}`); + Logger.debug(`No codex available for: ${node.description.name}`); if (isCustom) { node.description.codex = { @@ -291,8 +348,7 @@ export abstract class DirectoryLoader { private getIconPath(icon: string, filePath: string) { const iconPath = path.join(path.dirname(filePath), icon.replace('file:', '')); - const relativePath = path.relative(this.directory, iconPath); - return `icons/${this.packageName}/${relativePath}`; + return `icons/${this.packageName}/${iconPath}`; } private fixIconPaths( @@ -305,14 +361,14 @@ export abstract class DirectoryLoader { if (typeof icon === 'string') { if (icon.startsWith('file:')) { obj.iconUrl = this.getIconPath(icon, filePath); - delete obj.icon; + obj.icon = undefined; } } else if (icon.light.startsWith('file:') && icon.dark.startsWith('file:')) { obj.iconUrl = { light: this.getIconPath(icon.light, filePath), dark: this.getIconPath(icon.dark, filePath), }; - delete obj.icon; + obj.icon = undefined; } } } @@ -331,8 +387,7 @@ export class CustomDirectoryLoader extends DirectoryLoader { }); for (const nodePath of nodes) { - const [fileName] = path.parse(nodePath).name.split('.'); - this.loadNodeFromFile(fileName, nodePath); + this.loadNodeFromFile(nodePath); } const credentials = await glob('**/*.credentials.js', { @@ -341,8 +396,7 @@ export class CustomDirectoryLoader extends DirectoryLoader { }); for (const credentialPath of credentials) { - const [fileName] = path.parse(credentialPath).name.split('.'); - this.loadCredentialFromFile(fileName, credentialPath); + this.loadCredentialFromFile(credentialPath); } } } @@ -363,33 +417,55 @@ export class PackageDirectoryLoader extends DirectoryLoader { const { nodes, credentials } = n8n; if (Array.isArray(nodes)) { - for (const node of nodes) { - const filePath = this.resolvePath(node); - const [nodeName] = path.parse(node).name.split('.'); - - this.loadNodeFromFile(nodeName, filePath); + for (const nodePath of nodes) { + this.loadNodeFromFile(nodePath); } } if (Array.isArray(credentials)) { - for (const credential of credentials) { - const filePath = this.resolvePath(credential); - const [credentialName] = path.parse(credential).name.split('.'); - - this.loadCredentialFromFile(credentialName, filePath); + for (const credentialPath of credentials) { + this.loadCredentialFromFile(credentialPath); } } + this.inferSupportedNodes(); + Logger.debug(`Loaded all credentials and nodes from ${this.packageName}`, { credentials: credentials?.length ?? 0, nodes: nodes?.length ?? 0, }); } - protected readJSONSync(file: string): T { - const filePath = this.resolvePath(file); - const fileString = readFileSync(filePath, 'utf8'); + private inferSupportedNodes() { + const knownCredentials = this.known.credentials; + for (const { type: credentialType } of Object.values(this.credentialTypes)) { + const supportedNodes = knownCredentials[credentialType.name].supportedNodes ?? []; + if (supportedNodes.length > 0 && credentialType.httpRequestNode) { + credentialType.httpRequestNode.hidden = true; + } + + credentialType.supportedNodes = supportedNodes; + if (!credentialType.iconUrl && !credentialType.icon) { + for (const supportedNode of supportedNodes) { + const nodeDescription = this.nodeTypes[supportedNode]?.type.description; + + if (!nodeDescription) continue; + if (nodeDescription.icon) { + credentialType.icon = nodeDescription.icon; + credentialType.iconColor = nodeDescription.iconColor; + break; + } + if (nodeDescription.iconUrl) { + credentialType.iconUrl = nodeDescription.iconUrl; + break; + } + } + } + } + } + + private parseJSON(fileString: string, filePath: string): T { try { return jsonParse(fileString); } catch (error) { @@ -397,15 +473,16 @@ export class PackageDirectoryLoader extends DirectoryLoader { } } + protected readJSONSync(file: string): T { + const filePath = this.resolvePath(file); + const fileString = readFileSync(filePath, 'utf8'); + return this.parseJSON(fileString, filePath); + } + protected async readJSON(file: string): Promise { const filePath = this.resolvePath(file); const fileString = await readFile(filePath, 'utf8'); - - try { - return jsonParse(fileString); - } catch (error) { - throw new ApplicationError('Failed to parse JSON', { extra: { filePath } }); - } + return this.parseJSON(fileString, filePath); } } @@ -415,10 +492,7 @@ export class PackageDirectoryLoader extends DirectoryLoader { export class LazyPackageDirectoryLoader extends PackageDirectoryLoader { override async loadAll() { try { - const knownNodes: typeof this.known.nodes = await this.readJSON('dist/known/nodes.json'); - for (const nodeName in knownNodes) { - this.known.nodes[`${this.packageName}.${nodeName}`] = knownNodes[nodeName]; - } + this.known.nodes = await this.readJSON('dist/known/nodes.json'); this.known.credentials = await this.readJSON('dist/known/credentials.json'); this.types.nodes = await this.readJSON('dist/types/nodes.json'); @@ -426,9 +500,10 @@ export class LazyPackageDirectoryLoader extends PackageDirectoryLoader { if (this.includeNodes.length) { const allowedNodes: typeof this.known.nodes = {}; - for (const nodeName of this.includeNodes) { - if (nodeName in this.known.nodes) { - allowedNodes[nodeName] = this.known.nodes[nodeName]; + for (const fullNodeType of this.includeNodes) { + const [packageName, nodeType] = fullNodeType.split('.'); + if (packageName === this.packageName && nodeType in this.known.nodes) { + allowedNodes[nodeType] = this.known.nodes[nodeType]; } } this.known.nodes = allowedNodes; @@ -439,8 +514,11 @@ export class LazyPackageDirectoryLoader extends PackageDirectoryLoader { } if (this.excludeNodes.length) { - for (const nodeName of this.excludeNodes) { - delete this.known.nodes[nodeName]; + for (const fullNodeType of this.excludeNodes) { + const [packageName, nodeType] = fullNodeType.split('.'); + if (packageName === this.packageName) { + delete this.known.nodes[nodeType]; + } } this.types.nodes = this.types.nodes.filter( diff --git a/packages/core/src/errors/index.ts b/packages/core/src/errors/index.ts index c280ecb30dcba..38cd481c257dd 100644 --- a/packages/core/src/errors/index.ts +++ b/packages/core/src/errors/index.ts @@ -3,3 +3,5 @@ export { DisallowedFilepathError } from './disallowed-filepath.error'; export { InvalidModeError } from './invalid-mode.error'; export { InvalidManagerError } from './invalid-manager.error'; export { InvalidExecutionMetadataError } from './invalid-execution-metadata.error'; +export { UnrecognizedCredentialTypeError } from './unrecognized-credential-type.error'; +export { UnrecognizedNodeTypeError } from './unrecognized-node-type.error'; diff --git a/packages/core/src/errors/unrecognized-credential-type.error.ts b/packages/core/src/errors/unrecognized-credential-type.error.ts new file mode 100644 index 0000000000000..c60b576bca244 --- /dev/null +++ b/packages/core/src/errors/unrecognized-credential-type.error.ts @@ -0,0 +1,9 @@ +import { ApplicationError } from 'n8n-workflow'; + +export class UnrecognizedCredentialTypeError extends ApplicationError { + severity = 'warning'; + + constructor(credentialType: string) { + super(`Unrecognized credential type: ${credentialType}`); + } +} diff --git a/packages/cli/src/errors/unrecognized-node-type.error.ts b/packages/core/src/errors/unrecognized-node-type.error.ts similarity index 55% rename from packages/cli/src/errors/unrecognized-node-type.error.ts rename to packages/core/src/errors/unrecognized-node-type.error.ts index 1ca5281de5913..6ca5e34e40d05 100644 --- a/packages/cli/src/errors/unrecognized-node-type.error.ts +++ b/packages/core/src/errors/unrecognized-node-type.error.ts @@ -3,7 +3,7 @@ import { ApplicationError } from 'n8n-workflow'; export class UnrecognizedNodeTypeError extends ApplicationError { severity = 'warning'; - constructor(nodeType: string) { - super(`Unrecognized node type: ${nodeType}".`); + constructor(packageName: string, nodeType: string) { + super(`Unrecognized node type: ${packageName}.${nodeType}`); } } diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 9c141867dee1d..4b3e479018948 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -5,7 +5,6 @@ export * from './ActiveWorkflows'; export * from './BinaryData/BinaryData.service'; export * from './BinaryData/types'; export { Cipher } from './Cipher'; -export * from './ClassLoader'; export * from './Constants'; export * from './Credentials'; export * from './DirectoryLoader'; diff --git a/packages/core/test/ClassLoader.test.ts b/packages/core/test/ClassLoader.test.ts new file mode 100644 index 0000000000000..95275726625bf --- /dev/null +++ b/packages/core/test/ClassLoader.test.ts @@ -0,0 +1,52 @@ +import vm from 'vm'; + +import { loadClassInIsolation } from '@/ClassLoader'; + +describe('ClassLoader', () => { + const filePath = '/path/to/TestClass.js'; + const className = 'TestClass'; + + class TestClass { + getValue(): string { + return 'test value'; + } + } + + jest.spyOn(vm, 'createContext').mockReturnValue({}); + + const runInContext = jest.fn().mockImplementation(() => new TestClass()); + const scriptSpy = jest.spyOn(vm, 'Script').mockImplementation(function (this: vm.Script) { + this.runInContext = runInContext; + return this; + }); + + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('should create script with correct require statement', () => { + const instance = loadClassInIsolation(filePath, className); + + expect(scriptSpy).toHaveBeenCalledWith(`new (require('${filePath}').${className})()`); + expect(instance.getValue()).toBe('test value'); + }); + + it('should handle Windows-style paths', () => { + const originalPlatform = process.platform; + Object.defineProperty(process, 'platform', { value: 'win32' }); + + loadClassInIsolation('/path\\to\\TestClass.js', 'TestClass'); + + expect(scriptSpy).toHaveBeenCalledWith(`new (require('${filePath}').${className})()`); + + Object.defineProperty(process, 'platform', { value: originalPlatform }); + }); + + it('should throw error when script execution fails', () => { + runInContext.mockImplementationOnce(() => { + throw new Error('Script execution failed'); + }); + + expect(() => loadClassInIsolation(filePath, className)).toThrow('Script execution failed'); + }); +}); diff --git a/packages/core/test/DirectoryLoader.test.ts b/packages/core/test/DirectoryLoader.test.ts new file mode 100644 index 0000000000000..01a8c8d34a3e9 --- /dev/null +++ b/packages/core/test/DirectoryLoader.test.ts @@ -0,0 +1,781 @@ +import { mock } from 'jest-mock-extended'; +import type { + ICredentialType, + INodeType, + INodeTypeDescription, + IVersionedNodeType, +} from 'n8n-workflow'; +import { deepCopy } from 'n8n-workflow'; +import fs from 'node:fs'; +import fsPromises from 'node:fs/promises'; + +jest.mock('node:fs'); +jest.mock('node:fs/promises'); +const mockFs = mock(); +const mockFsPromises = mock(); +fs.readFileSync = mockFs.readFileSync; +fsPromises.readFile = mockFsPromises.readFile; + +jest.mock('fast-glob', () => async (pattern: string) => { + return pattern.endsWith('.node.js') + ? ['dist/Node1/Node1.node.js', 'dist/Node2/Node2.node.js'] + : ['dist/Credential1.js']; +}); + +import * as classLoader from '@/ClassLoader'; +import { + CustomDirectoryLoader, + PackageDirectoryLoader, + LazyPackageDirectoryLoader, +} from '@/DirectoryLoader'; + +describe('DirectoryLoader', () => { + const directory = '/not/a/real/path'; + const packageJson = JSON.stringify({ + name: 'n8n-nodes-testing', + n8n: { + credentials: ['dist/Credential1.js'], + nodes: ['dist/Node1/Node1.node.js', 'dist/Node2/Node2.node.js'], + }, + }); + + const createNode = (name: string, credential?: string) => + mock({ + description: { + name, + version: 1, + icon: `file:${name}.svg`, + iconUrl: undefined, + credentials: credential ? [{ name: credential }] : [], + properties: [], + }, + }); + + const createCredential = (name: string) => + mock({ + name, + icon: `file:${name}.svg`, + iconUrl: undefined, + extends: undefined, + properties: [], + }); + + let mockCredential1: ICredentialType, mockNode1: INodeType, mockNode2: INodeType; + + beforeEach(() => { + mockCredential1 = createCredential('credential1'); + mockNode1 = createNode('node1', 'credential1'); + mockNode2 = createNode('node2'); + jest.clearAllMocks(); + }); + + //@ts-expect-error overwrite a readonly property + classLoader.loadClassInIsolation = jest.fn((_: string, className: string) => { + if (className === 'Node1') return mockNode1; + if (className === 'Node2') return mockNode2; + if (className === 'Credential1') return mockCredential1; + throw new Error(`${className} is invalid`); + }); + + describe('CustomDirectoryLoader', () => { + it('should load custom nodes and credentials', async () => { + const loader = new CustomDirectoryLoader(directory); + expect(loader.packageName).toEqual('CUSTOM'); + + await loader.loadAll(); + + expect(loader.isLazyLoaded).toBe(false); + expect(mockFsPromises.readFile).not.toHaveBeenCalled(); + expect(classLoader.loadClassInIsolation).toHaveBeenCalledTimes(3); + + expect(loader.nodesByCredential).toEqual({ credential1: ['node1'] }); + expect(loader.credentialTypes).toEqual({ + credential1: { sourcePath: 'dist/Credential1.js', type: mockCredential1 }, + }); + expect(loader.nodeTypes).toEqual({ + node1: { sourcePath: 'dist/Node1/Node1.node.js', type: mockNode1 }, + node2: { sourcePath: 'dist/Node2/Node2.node.js', type: mockNode2 }, + }); + expect(mockCredential1.iconUrl).toBe('icons/CUSTOM/dist/credential1.svg'); + expect(mockNode1.description.iconUrl).toBe('icons/CUSTOM/dist/Node1/node1.svg'); + expect(mockNode2.description.iconUrl).toBe('icons/CUSTOM/dist/Node2/node2.svg'); + + expect(mockFs.readFileSync).not.toHaveBeenCalled(); + }); + }); + + describe('PackageDirectoryLoader', () => { + it('should load nodes and credentials from an installed package', async () => { + mockFs.readFileSync.calledWith(`${directory}/package.json`).mockReturnValue(packageJson); + + const loader = new PackageDirectoryLoader(directory); + expect(loader.packageName).toEqual('n8n-nodes-testing'); + + await loader.loadAll(); + + expect(loader.isLazyLoaded).toBe(false); + expect(mockFsPromises.readFile).not.toHaveBeenCalled(); + expect(classLoader.loadClassInIsolation).toHaveBeenCalledTimes(3); + + expect(loader.nodesByCredential).toEqual({ credential1: ['node1'] }); + expect(loader.credentialTypes).toEqual({ + credential1: { sourcePath: 'dist/Credential1.js', type: mockCredential1 }, + }); + expect(loader.nodeTypes).toEqual({ + node1: { sourcePath: 'dist/Node1/Node1.node.js', type: mockNode1 }, + node2: { sourcePath: 'dist/Node2/Node2.node.js', type: mockNode2 }, + }); + expect(mockCredential1.iconUrl).toBe('icons/n8n-nodes-testing/dist/credential1.svg'); + expect(mockNode1.description.iconUrl).toBe('icons/n8n-nodes-testing/dist/Node1/node1.svg'); + expect(mockNode2.description.iconUrl).toBe('icons/n8n-nodes-testing/dist/Node2/node2.svg'); + }); + + it('should throw error when package.json is missing', async () => { + mockFs.readFileSync.mockImplementationOnce(() => { + throw new Error('ENOENT'); + }); + + expect(() => new PackageDirectoryLoader(directory)).toThrow(); + }); + + it('should throw error when package.json is invalid', async () => { + mockFs.readFileSync.calledWith(`${directory}/package.json`).mockReturnValue('invalid json'); + + expect(() => new PackageDirectoryLoader(directory)).toThrow('Failed to parse JSON'); + }); + + it('should do nothing if package.json has no n8n field', async () => { + mockFs.readFileSync.calledWith(`${directory}/package.json`).mockReturnValue( + JSON.stringify({ + name: 'n8n-nodes-testing', + }), + ); + + const loader = new PackageDirectoryLoader(directory); + await loader.loadAll(); + + expect(loader.nodeTypes).toEqual({}); + expect(loader.credentialTypes).toEqual({}); + expect(classLoader.loadClassInIsolation).not.toHaveBeenCalled(); + }); + + it('should hide httpRequestNode property when credential has supported nodes', async () => { + mockFs.readFileSync.calledWith(`${directory}/package.json`).mockReturnValue(packageJson); + mockCredential1.httpRequestNode = mock({ hidden: false }); + + const loader = new PackageDirectoryLoader(directory); + await loader.loadAll(); + + expect(mockCredential1.httpRequestNode?.hidden).toBe(true); + }); + + it('should not modify httpRequestNode when credential has no supported nodes', async () => { + mockFs.readFileSync.calledWith(`${directory}/package.json`).mockReturnValue(packageJson); + mockCredential1.httpRequestNode = mock({ hidden: false }); + mockNode1.description.credentials = []; + + const loader = new PackageDirectoryLoader(directory); + await loader.loadAll(); + + expect(mockCredential1.httpRequestNode?.hidden).toBe(false); + }); + + it('should inherit iconUrl from supported node when credential has no icon', async () => { + mockFs.readFileSync.calledWith(`${directory}/package.json`).mockReturnValue(packageJson); + mockCredential1.icon = undefined; + + const loader = new PackageDirectoryLoader(directory); + await loader.loadAll(); + + expect(mockCredential1.supportedNodes).toEqual(['node1']); + expect(mockCredential1.iconUrl).toBe(mockNode1.description.iconUrl); + }); + }); + + describe('LazyPackageDirectoryLoader', () => { + it('should skip loading nodes and credentials from a lazy-loadable package', async () => { + mockFs.readFileSync.calledWith(`${directory}/package.json`).mockReturnValue(packageJson); + mockFsPromises.readFile.mockResolvedValue('[]'); + + const loader = new LazyPackageDirectoryLoader(directory); + expect(loader.packageName).toEqual('n8n-nodes-testing'); + + await loader.loadAll(); + + expect(loader.isLazyLoaded).toBe(true); + expect(mockFsPromises.readFile).toHaveBeenCalledTimes(4); + expect(classLoader.loadClassInIsolation).not.toHaveBeenCalled(); + }); + + it('should fall back to non-lazy loading if any json file fails to parse', async () => { + mockFs.readFileSync.calledWith(`${directory}/package.json`).mockReturnValue(packageJson); + mockFsPromises.readFile.mockRejectedValue(new Error('Failed to read file')); + + const loader = new LazyPackageDirectoryLoader(directory); + await loader.loadAll(); + + expect(loader.isLazyLoaded).toBe(false); + expect(mockFsPromises.readFile).toHaveBeenCalled(); + expect(classLoader.loadClassInIsolation).toHaveBeenCalledTimes(3); + }); + + it('should only load included nodes when includeNodes is set', async () => { + mockFs.readFileSync.calledWith(`${directory}/package.json`).mockReturnValue(packageJson); + + mockFsPromises.readFile.mockImplementation(async (path) => { + if (typeof path !== 'string') throw new Error('Invalid path'); + + if (path.endsWith('known/nodes.json')) { + return JSON.stringify({ + node1: { className: 'Node1', sourcePath: 'dist/Node1/Node1.node.js' }, + node2: { className: 'Node2', sourcePath: 'dist/Node2/Node2.node.js' }, + }); + } + if (path.endsWith('known/credentials.json')) { + return JSON.stringify({}); + } + if (path.endsWith('types/nodes.json')) { + return JSON.stringify([ + { name: 'n8n-nodes-testing.node1' }, + { name: 'n8n-nodes-testing.node2' }, + ]); + } + if (path.endsWith('types/credentials.json')) { + return JSON.stringify([]); + } + throw new Error('File not found'); + }); + + const loader = new LazyPackageDirectoryLoader(directory, [], ['n8n-nodes-testing.node1']); + await loader.loadAll(); + + expect(loader.isLazyLoaded).toBe(true); + expect(loader.known.nodes).toEqual({ + node1: { className: 'Node1', sourcePath: 'dist/Node1/Node1.node.js' }, + }); + expect(loader.types.nodes).toHaveLength(1); + expect(loader.types.nodes[0].name).toBe('n8n-nodes-testing.node1'); + expect(classLoader.loadClassInIsolation).not.toHaveBeenCalled(); + }); + + it('should load no nodes when includeNodes does not match any nodes', async () => { + mockFs.readFileSync.calledWith(`${directory}/package.json`).mockReturnValue(packageJson); + + mockFsPromises.readFile.mockImplementation(async (path) => { + if (typeof path !== 'string') throw new Error('Invalid path'); + + if (path.endsWith('known/nodes.json')) { + return JSON.stringify({ + node1: { className: 'Node1', sourcePath: 'dist/Node1/Node1.node.js' }, + node2: { className: 'Node2', sourcePath: 'dist/Node2/Node2.node.js' }, + }); + } + if (path.endsWith('known/credentials.json')) { + return JSON.stringify({}); + } + if (path.endsWith('types/nodes.json')) { + return JSON.stringify([ + { name: 'n8n-nodes-testing.node1' }, + { name: 'n8n-nodes-testing.node2' }, + ]); + } + if (path.endsWith('types/credentials.json')) { + return JSON.stringify([]); + } + throw new Error('File not found'); + }); + + const loader = new LazyPackageDirectoryLoader( + directory, + [], + ['n8n-nodes-testing.nonexistent'], + ); + await loader.loadAll(); + + expect(loader.isLazyLoaded).toBe(true); + expect(loader.known.nodes).toEqual({}); + expect(loader.types.nodes).toHaveLength(0); + expect(classLoader.loadClassInIsolation).not.toHaveBeenCalled(); + }); + + it('should exclude specified nodes when excludeNodes is set', async () => { + mockFs.readFileSync.calledWith(`${directory}/package.json`).mockReturnValue(packageJson); + + mockFsPromises.readFile.mockImplementation(async (path) => { + if (typeof path !== 'string') throw new Error('Invalid path'); + + if (path.endsWith('known/nodes.json')) { + return JSON.stringify({ + node1: { className: 'Node1', sourcePath: 'dist/Node1/Node1.node.js' }, + node2: { className: 'Node2', sourcePath: 'dist/Node2/Node2.node.js' }, + }); + } + if (path.endsWith('known/credentials.json')) { + return JSON.stringify({}); + } + if (path.endsWith('types/nodes.json')) { + return JSON.stringify([ + { name: 'n8n-nodes-testing.node1' }, + { name: 'n8n-nodes-testing.node2' }, + ]); + } + if (path.endsWith('types/credentials.json')) { + return JSON.stringify([]); + } + throw new Error('File not found'); + }); + + const loader = new LazyPackageDirectoryLoader(directory, ['n8n-nodes-testing.node1']); + await loader.loadAll(); + + expect(loader.isLazyLoaded).toBe(true); + expect(loader.known.nodes).toEqual({ + node2: { className: 'Node2', sourcePath: 'dist/Node2/Node2.node.js' }, + }); + expect(loader.types.nodes).toHaveLength(1); + expect(loader.types.nodes[0].name).toBe('n8n-nodes-testing.node2'); + expect(classLoader.loadClassInIsolation).not.toHaveBeenCalled(); + }); + }); + + describe('reset()', () => { + it('should reset all properties to their initial state', async () => { + mockFs.readFileSync.calledWith(`${directory}/package.json`).mockReturnValue(packageJson); + + const loader = new PackageDirectoryLoader(directory); + await loader.loadAll(); + + // Verify loader has loaded data + expect(loader.nodeTypes).not.toEqual({}); + expect(loader.credentialTypes).not.toEqual({}); + expect(loader.types.nodes.length).toBeGreaterThan(0); + expect(loader.types.credentials.length).toBeGreaterThan(0); + expect(loader.loadedNodes.length).toBeGreaterThan(0); + expect(Object.keys(loader.known.nodes).length).toBeGreaterThan(0); + expect(Object.keys(loader.known.credentials).length).toBeGreaterThan(0); + + // Reset the loader + loader.reset(); + + // Verify all properties are reset + expect(loader.nodeTypes).toEqual({}); + expect(loader.credentialTypes).toEqual({}); + expect(loader.types.nodes).toEqual([]); + expect(loader.types.credentials).toEqual([]); + expect(loader.loadedNodes).toEqual([]); + expect(loader.known.nodes).toEqual({}); + expect(loader.known.credentials).toEqual({}); + }); + }); + + describe('getVersionedNodeTypeAll', () => { + it('should return array with single node for non-versioned node', () => { + const loader = new CustomDirectoryLoader(directory); + const node = createNode('node1'); + + const result = loader.getVersionedNodeTypeAll(node); + + expect(result).toHaveLength(1); + expect(result[0]).toBe(node); + }); + + it('should return all versions of a versioned node', () => { + const loader = new CustomDirectoryLoader(directory); + const nodeV1 = createNode('test'); + const nodeV2 = createNode('test'); + nodeV1.description.version = 1; + nodeV2.description.version = 2; + + const versionedNode = mock({ + description: { name: 'test', codex: {} }, + currentVersion: 2, + nodeVersions: { + 1: nodeV1, + 2: nodeV2, + }, + }); + + const result = loader.getVersionedNodeTypeAll(versionedNode); + + expect(result).toHaveLength(2); + expect(result).toEqual([nodeV2, nodeV1]); + expect(result[0].description.name).toBe('test'); + expect(result[1].description.name).toBe('test'); + }); + }); + + describe('getCredentialsForNode', () => { + it('should return empty array if node has no credentials', () => { + const loader = new CustomDirectoryLoader(directory); + const node = createNode('node1'); + + const result = loader.getCredentialsForNode(node); + + expect(Array.isArray(result)).toBe(true); + expect(result.length).toEqual(0); + }); + + it('should return credentials for non-versioned node', () => { + const loader = new CustomDirectoryLoader(directory); + const node = createNode('node1', 'testCred'); + + const result = loader.getCredentialsForNode(node); + + expect(result).toHaveLength(1); + expect(result[0].name).toBe('testCred'); + }); + + it('should return unique credentials from all versions of a versioned node', () => { + const loader = new CustomDirectoryLoader(directory); + const nodeV1 = createNode('test', 'cred1'); + const nodeV2 = createNode('test', 'cred2'); + + const versionedNode = mock({ + description: { name: 'test' }, + currentVersion: 2, + nodeVersions: { + 1: nodeV1, + 2: nodeV2, + }, + }); + + const result = loader.getCredentialsForNode(versionedNode); + + expect(result).toHaveLength(2); + expect(result[0].name).toBe('cred1'); + expect(result[1].name).toBe('cred2'); + }); + + it('should remove duplicate credentials from different versions', () => { + const loader = new CustomDirectoryLoader(directory); + const nodeV1 = createNode('test', 'cred1'); + const nodeV2 = createNode('test', 'cred1'); // Same credential + + const versionedNode = mock({ + description: { name: 'test' }, + currentVersion: 2, + nodeVersions: { + 1: nodeV1, + 2: nodeV2, + }, + }); + + const result = loader.getCredentialsForNode(versionedNode); + + expect(result).toHaveLength(1); + expect(result[0].name).toBe('cred1'); + }); + }); + + describe('loadCredentialFromFile', () => { + it('should load credential and store it correctly', () => { + const loader = new CustomDirectoryLoader(directory); + const filePath = 'dist/Credential1.js'; + + loader.loadCredentialFromFile(filePath); + + expect(loader.credentialTypes).toEqual({ + credential1: { + type: mockCredential1, + sourcePath: filePath, + }, + }); + + expect(loader.known.credentials).toEqual({ + credential1: { + className: mockCredential1.constructor.name, + sourcePath: filePath, + extends: undefined, + supportedNodes: undefined, + }, + }); + + expect(loader.types.credentials).toEqual([mockCredential1]); + }); + + it('should update credential icon paths', () => { + const loader = new CustomDirectoryLoader(directory); + const filePath = 'dist/Credential1.js'; + + const credWithIcon = createCredential('credentialWithIcon'); + credWithIcon.icon = { + light: 'file:light.svg', + dark: 'file:dark.svg', + }; + + jest.spyOn(classLoader, 'loadClassInIsolation').mockReturnValueOnce(credWithIcon); + + loader.loadCredentialFromFile(filePath); + + expect(credWithIcon.iconUrl).toEqual({ + light: 'icons/CUSTOM/dist/light.svg', + dark: 'icons/CUSTOM/dist/dark.svg', + }); + expect(credWithIcon.icon).toBeUndefined(); + }); + + it('should add toJSON method to credential type', () => { + const loader = new CustomDirectoryLoader(directory); + const filePath = 'dist/Credential1.js'; + + const credWithAuth = createCredential('credWithAuth'); + credWithAuth.authenticate = jest.fn(); + + jest.spyOn(classLoader, 'loadClassInIsolation').mockReturnValueOnce(credWithAuth); + + loader.loadCredentialFromFile(filePath); + + const serialized = deepCopy(credWithAuth); + expect(serialized.authenticate).toEqual({}); + }); + + it('should store credential extends and supported nodes info', () => { + const loader = new CustomDirectoryLoader(directory); + const filePath = 'dist/Credential1.js'; + + const extendingCred = createCredential('extendingCred'); + extendingCred.extends = ['baseCredential']; + + jest.spyOn(classLoader, 'loadClassInIsolation').mockReturnValueOnce(extendingCred); + + // Set up nodesByCredential before loading + loader.nodesByCredential.extendingCred = ['node1', 'node2']; + + loader.loadCredentialFromFile(filePath); + + expect(loader.known.credentials.extendingCred).toEqual({ + className: extendingCred.constructor.name, + sourcePath: filePath, + extends: ['baseCredential'], + supportedNodes: ['node1', 'node2'], + }); + }); + + it('should throw error if credential class cannot be loaded', () => { + const loader = new CustomDirectoryLoader(directory); + const filePath = 'dist/InvalidCred.js'; + + jest.spyOn(classLoader, 'loadClassInIsolation').mockImplementationOnce(() => { + throw new TypeError('Class not found'); + }); + + expect(() => loader.loadCredentialFromFile(filePath)).toThrow('Class could not be found'); + }); + }); + + describe('getCredential', () => { + it('should return existing loaded credential type', () => { + const loader = new CustomDirectoryLoader(directory); + const filePath = 'dist/Credential1.js'; + + loader.loadCredentialFromFile(filePath); + + const result = loader.getCredential('credential1'); + expect(result).toEqual({ + type: mockCredential1, + sourcePath: filePath, + }); + }); + + it('should load credential from known credentials if not already loaded', () => { + const loader = new CustomDirectoryLoader(directory); + const filePath = 'dist/Credential1.js'; + + // Setup known credentials without loading + loader.known.credentials.credential1 = { + className: 'Credential1', + sourcePath: filePath, + }; + + const result = loader.getCredential('credential1'); + + expect(result).toEqual({ + type: mockCredential1, + sourcePath: filePath, + }); + expect(classLoader.loadClassInIsolation).toHaveBeenCalledWith( + expect.stringContaining(filePath), + 'Credential1', + ); + }); + + it('should throw UnrecognizedCredentialTypeError if credential type is not found', () => { + const loader = new CustomDirectoryLoader(directory); + + expect(() => loader.getCredential('nonexistent')).toThrow( + 'Unrecognized credential type: nonexistent', + ); + }); + }); + + describe('loadNodeFromFile', () => { + it('should load node and store it correctly', () => { + const loader = new CustomDirectoryLoader(directory); + const filePath = 'dist/Node1/Node1.node.js'; + + loader.loadNodeFromFile(filePath); + + expect(loader.nodeTypes).toEqual({ + node1: { + type: mockNode1, + sourcePath: filePath, + }, + }); + + expect(loader.known.nodes).toEqual({ + node1: { + className: mockNode1.constructor.name, + sourcePath: filePath, + }, + }); + + expect(loader.types.nodes).toEqual([mockNode1.description]); + expect(loader.loadedNodes).toEqual([{ name: 'node1', version: 1 }]); + }); + + it('should update node icon paths', () => { + const loader = new CustomDirectoryLoader(directory); + const filePath = 'dist/Node1/Node1.node.js'; + + const nodeWithIcon = createNode('nodeWithIcon'); + nodeWithIcon.description.icon = { + light: 'file:light.svg', + dark: 'file:dark.svg', + }; + + jest.spyOn(classLoader, 'loadClassInIsolation').mockReturnValueOnce(nodeWithIcon); + + loader.loadNodeFromFile(filePath); + + expect(nodeWithIcon.description.iconUrl).toEqual({ + light: 'icons/CUSTOM/dist/Node1/light.svg', + dark: 'icons/CUSTOM/dist/Node1/dark.svg', + }); + expect(nodeWithIcon.description.icon).toBeUndefined(); + }); + + it('should skip node if included in excludeNodes', () => { + const loader = new CustomDirectoryLoader(directory, ['CUSTOM.node1']); + const filePath = 'dist/Node1/Node1.node.js'; + + loader.loadNodeFromFile(filePath); + + expect(loader.nodeTypes).toEqual({}); + expect(loader.known.nodes).toEqual({}); + expect(loader.types.nodes).toEqual([]); + expect(loader.loadedNodes).toEqual([]); + }); + + it('should skip node if not in includeNodes', () => { + const loader = new CustomDirectoryLoader(directory, [], ['CUSTOM.other']); + const filePath = 'dist/Node1/Node1.node.js'; + + loader.loadNodeFromFile(filePath); + + expect(loader.nodeTypes).toEqual({}); + expect(loader.known.nodes).toEqual({}); + expect(loader.types.nodes).toEqual([]); + expect(loader.loadedNodes).toEqual([]); + }); + + it('should handle versioned nodes correctly', () => { + const loader = new CustomDirectoryLoader(directory); + const filePath = 'dist/Node1/Node1.node.js'; + + const nodeV1 = createNode('test'); + const nodeV2 = createNode('test'); + nodeV1.description.version = 1; + nodeV2.description.version = 2; + + const versionedNode = mock({ + description: { name: 'test', codex: {}, iconUrl: undefined, icon: undefined }, + currentVersion: 2, + nodeVersions: { + 1: nodeV1, + 2: nodeV2, + }, + }); + + jest.spyOn(classLoader, 'loadClassInIsolation').mockReturnValueOnce(versionedNode); + + loader.loadNodeFromFile(filePath); + + expect(loader.loadedNodes).toEqual([{ name: 'test', version: 2 }]); + + const nodes = loader.types.nodes as INodeTypeDescription[]; + expect(nodes).toHaveLength(2); + expect(nodes[0]?.version).toBe(2); + expect(nodes[1]?.version).toBe(1); + }); + + it('should store credential associations correctly', () => { + const loader = new CustomDirectoryLoader(directory); + const filePath = 'dist/Node1/Node1.node.js'; + + const nodeWithCreds = createNode('testNode', 'testCred'); + jest.spyOn(classLoader, 'loadClassInIsolation').mockReturnValueOnce(nodeWithCreds); + + loader.loadNodeFromFile(filePath); + + expect(loader.nodesByCredential).toEqual({ + testCred: ['testNode'], + }); + }); + + it('should throw error if node class cannot be loaded', () => { + const loader = new CustomDirectoryLoader(directory); + const filePath = 'dist/InvalidNode/InvalidNode.node.js'; + + jest.spyOn(classLoader, 'loadClassInIsolation').mockImplementationOnce(() => { + throw new TypeError('Class not found'); + }); + + expect(() => loader.loadNodeFromFile(filePath)).toThrow('Class could not be found'); + }); + }); + + describe('getNode', () => { + it('should return existing loaded node type', () => { + const loader = new CustomDirectoryLoader(directory); + const filePath = 'dist/Node1/Node1.node.js'; + + loader.loadNodeFromFile(filePath); + + const result = loader.getNode('node1'); + expect(result).toEqual({ + type: mockNode1, + sourcePath: filePath, + }); + }); + + it('should load node from known nodes if not already loaded', () => { + const loader = new CustomDirectoryLoader(directory); + const filePath = 'dist/Node1/Node1.node.js'; + + // Setup known nodes without loading + loader.known.nodes.node1 = { + className: 'Node1', + sourcePath: filePath, + }; + + const result = loader.getNode('node1'); + + expect(result).toEqual({ + type: mockNode1, + sourcePath: filePath, + }); + expect(classLoader.loadClassInIsolation).toHaveBeenCalledWith( + expect.stringContaining(filePath), + 'Node1', + ); + }); + + it('should throw UnrecognizedNodeTypeError if node type is not found', () => { + const loader = new CustomDirectoryLoader(directory); + + expect(() => loader.getNode('nonexistent')).toThrow( + 'Unrecognized node type: CUSTOM.nonexistent', + ); + }); + }); +}); diff --git a/packages/core/test/helpers/index.ts b/packages/core/test/helpers/index.ts index 5f0858ea413ae..2ced1a423b29c 100644 --- a/packages/core/test/helpers/index.ts +++ b/packages/core/test/helpers/index.ts @@ -17,6 +17,8 @@ import type { import { ApplicationError, NodeHelpers, WorkflowHooks } from 'n8n-workflow'; import path from 'path'; +import { UnrecognizedNodeTypeError } from '@/errors'; + import { predefinedNodesTypes } from './constants'; const BASE_DIR = path.resolve(__dirname, '../../..'); @@ -102,12 +104,9 @@ export function getNodeTypes(testData: WorkflowTestData[] | WorkflowTestData) { ); for (const nodeName of nodeNames) { - if (!nodeName.startsWith('n8n-nodes-base.')) { - throw new ApplicationError('Unknown node type', { tags: { nodeType: nodeName } }); - } const loadInfo = knownNodes[nodeName.replace('n8n-nodes-base.', '')]; if (!loadInfo) { - throw new ApplicationError('Unknown node type', { tags: { nodeType: nodeName } }); + throw new UnrecognizedNodeTypeError('n8n-nodes-base', nodeName); } const sourcePath = loadInfo.sourcePath.replace(/^dist\//, './').replace(/\.js$/, '.ts'); const nodeSourcePath = path.join(BASE_DIR, 'nodes-base', sourcePath); diff --git a/packages/nodes-base/package.json b/packages/nodes-base/package.json index 0c6a5acb9152f..290b34f4fd749 100644 --- a/packages/nodes-base/package.json +++ b/packages/nodes-base/package.json @@ -7,13 +7,12 @@ "clean": "rimraf dist .turbo", "dev": "pnpm watch", "typecheck": "tsc --noEmit", - "build": "tsc -p tsconfig.build.json && tsc-alias -p tsconfig.build.json && pnpm n8n-copy-icons && pnpm n8n-generate-translations && pnpm build:metadata", - "build:metadata": "pnpm n8n-generate-known && pnpm n8n-generate-ui-types", + "build": "tsc -p tsconfig.build.json && tsc-alias -p tsconfig.build.json && pnpm n8n-copy-icons && pnpm n8n-generate-translations && pnpm n8n-generate-metadata", "format": "biome format --write .", "format:check": "biome ci .", "lint": "eslint . --quiet && node ./scripts/validate-load-options-methods.js", "lintfix": "eslint . --fix", - "watch": "tsc-watch -p tsconfig.build.json --onCompilationComplete \"tsc-alias -p tsconfig.build.json\" --onSuccess \"pnpm n8n-generate-ui-types\"", + "watch": "tsc-watch -p tsconfig.build.json --onCompilationComplete \"tsc-alias -p tsconfig.build.json\" --onSuccess \"pnpm n8n-generate-metadata\"", "test": "jest" }, "files": [ diff --git a/packages/nodes-base/scripts/validate-load-options-methods.js b/packages/nodes-base/scripts/validate-load-options-methods.js index 9fa11a78222d8..b043365b93cd2 100644 --- a/packages/nodes-base/scripts/validate-load-options-methods.js +++ b/packages/nodes-base/scripts/validate-load-options-methods.js @@ -6,7 +6,7 @@ try { definedMethods = require('../dist/methods/defined.json'); } catch (error) { console.error( - 'Failed to find methods to validate. Please run `npm run n8n-generate-ui-types` first.', + 'Failed to find methods to validate. Please run `npm run n8n-generate-metadata` first.', ); process.exit(1); } diff --git a/packages/nodes-base/test/nodes/Helpers.ts b/packages/nodes-base/test/nodes/Helpers.ts index 98e8ebf9182c9..829f21e3037e6 100644 --- a/packages/nodes-base/test/nodes/Helpers.ts +++ b/packages/nodes-base/test/nodes/Helpers.ts @@ -4,7 +4,12 @@ import { tmpdir } from 'os'; import nock from 'nock'; import { isEmpty } from 'lodash'; import { get } from 'lodash'; -import { BinaryDataService, Credentials, constructExecutionMetaData } from 'n8n-core'; +import { + BinaryDataService, + Credentials, + UnrecognizedNodeTypeError, + constructExecutionMetaData, +} from 'n8n-core'; import { Container } from 'typedi'; import { mock } from 'jest-mock-extended'; import type { @@ -251,12 +256,9 @@ export function setup(testData: WorkflowTestData[] | WorkflowTestData) { const nodeNames = nodes.map((n) => n.type); for (const nodeName of nodeNames) { - if (!nodeName.startsWith('n8n-nodes-base.')) { - throw new ApplicationError(`Unknown node type: ${nodeName}`, { level: 'warning' }); - } const loadInfo = knownNodes[nodeName.replace('n8n-nodes-base.', '')]; if (!loadInfo) { - throw new ApplicationError(`Unknown node type: ${nodeName}`, { level: 'warning' }); + throw new UnrecognizedNodeTypeError('n8n-nodes-base', nodeName); } const sourcePath = loadInfo.sourcePath.replace(/^dist\//, './').replace(/\.js$/, '.ts'); const nodeSourcePath = path.join(baseDir, sourcePath); diff --git a/packages/workflow/src/Interfaces.ts b/packages/workflow/src/Interfaces.ts index 81dbe231b0c97..b5bca9c5c375e 100644 --- a/packages/workflow/src/Interfaces.ts +++ b/packages/workflow/src/Interfaces.ts @@ -2015,7 +2015,6 @@ export interface IWebhookDescription { responseData?: WebhookResponseData | string; restartWebhook?: boolean; isForm?: boolean; - hasLifecycleMethods?: boolean; // set automatically by generate-ui-types ndvHideUrl?: string | boolean; // If true the webhook will not be displayed in the editor ndvHideMethod?: string | boolean; // If true the method will not be displayed in the editor } diff --git a/packages/workflow/src/NodeHelpers.ts b/packages/workflow/src/NodeHelpers.ts index 97e63c96d8a6a..5f2ad7ec9606e 100644 --- a/packages/workflow/src/NodeHelpers.ts +++ b/packages/workflow/src/NodeHelpers.ts @@ -1,14 +1,10 @@ /* eslint-disable @typescript-eslint/no-unsafe-argument */ - /* eslint-disable @typescript-eslint/no-use-before-define */ /* eslint-disable @typescript-eslint/no-unsafe-assignment */ /* eslint-disable @typescript-eslint/prefer-nullish-coalescing */ - /* eslint-disable prefer-spread */ - import get from 'lodash/get'; import isEqual from 'lodash/isEqual'; -import uniqBy from 'lodash/uniqBy'; import { SINGLE_EXECUTION_NODES } from './Constants'; import { ApplicationError } from './errors/application.error'; @@ -1972,40 +1968,6 @@ export function getVersionedNodeType( return object; } -export function getVersionedNodeTypeAll(object: IVersionedNodeType | INodeType): INodeType[] { - if ('nodeVersions' in object) { - return uniqBy( - Object.values(object.nodeVersions) - .map((element) => { - element.description.name = object.description.name; - element.description.codex = object.description.codex; - return element; - }) - .reverse(), - (node) => { - const { version } = node.description; - return Array.isArray(version) ? version.join(',') : version.toString(); - }, - ); - } - return [object]; -} - -export function getCredentialsForNode( - object: IVersionedNodeType | INodeType, -): INodeCredentialDescription[] { - if ('nodeVersions' in object) { - return uniqBy( - Object.values(object.nodeVersions).flatMap( - (version) => version.description.credentials ?? [], - ), - 'name', - ); - } - - return object.description.credentials ?? []; -} - export function isSingleExecution(type: string, parameters: INodeParameters): boolean { const singleExecutionCase = SINGLE_EXECUTION_NODES[type]; From eb7d5934ef8bc6e999d6de4c0b8025ce175df5dd Mon Sep 17 00:00:00 2001 From: Jon Date: Tue, 10 Dec 2024 14:33:09 +0000 Subject: [PATCH 03/10] fix(FTP Node): Fix issue with creating folders on rename (#9340) --- packages/nodes-base/nodes/Ftp/Ftp.node.ts | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/packages/nodes-base/nodes/Ftp/Ftp.node.ts b/packages/nodes-base/nodes/Ftp/Ftp.node.ts index 99d9e4b77da8a..d9c8a94b42abc 100644 --- a/packages/nodes-base/nodes/Ftp/Ftp.node.ts +++ b/packages/nodes-base/nodes/Ftp/Ftp.node.ts @@ -752,10 +752,20 @@ export class Ftp implements INodeType { if (operation === 'rename') { const oldPath = this.getNodeParameter('oldPath', i) as string; - const newPath = this.getNodeParameter('newPath', i) as string; + const options = this.getNodeParameter('options', i); - await ftp!.rename(oldPath, newPath); + try { + await ftp!.rename(oldPath, newPath); + } catch (error) { + if ([451, 550].includes(error.code) && options.createDirectories) { + const dirPath = newPath.replace(basename(newPath), ''); + await ftp!.mkdir(dirPath, true); + await ftp!.rename(oldPath, newPath); + } else { + throw new NodeApiError(this.getNode(), error as JsonObject); + } + } const executionData = this.helpers.constructExecutionMetaData( [{ json: { success: true } }], { itemData: { item: i } }, From b37e5142b24f955477e33998a43ba070e4d0765b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Tue, 10 Dec 2024 16:38:24 +0100 Subject: [PATCH 04/10] refactor(core): Encapsulate manual execution flow in `WorkflowRunner` (#12135) --- packages/cli/src/workflow-runner.ts | 171 +++++++++++++++------------- 1 file changed, 90 insertions(+), 81 deletions(-) diff --git a/packages/cli/src/workflow-runner.ts b/packages/cli/src/workflow-runner.ts index 19ae201a8da3a..43beee1d052ff 100644 --- a/packages/cli/src/workflow-runner.ts +++ b/packages/cli/src/workflow-runner.ts @@ -20,6 +20,7 @@ import type { WorkflowHooks, IWorkflowExecutionDataProcess, IRunExecutionData, + IWorkflowExecuteAdditionalData, } from 'n8n-workflow'; import { ErrorReporterProxy as ErrorReporter, @@ -295,88 +296,8 @@ export class WorkflowRunner { data.executionData, ); workflowExecution = workflowExecute.processRunExecutionData(workflow); - } else if (data.triggerToStartFrom?.data && data.startNodes && !data.destinationNode) { - this.logger.debug( - `Execution ID ${executionId} had triggerToStartFrom. Starting from that trigger.`, - { executionId }, - ); - const startNodes = data.startNodes.map((data) => { - const node = workflow.getNode(data.name); - a.ok(node, `Could not find a node named "${data.name}" in the workflow.`); - return node; - }); - const runData = { [data.triggerToStartFrom.name]: [data.triggerToStartFrom.data] }; - - const { nodeExecutionStack, waitingExecution, waitingExecutionSource } = - recreateNodeExecutionStack( - filterDisabledNodes(DirectedGraph.fromWorkflow(workflow)), - new Set(startNodes), - runData, - data.pinData ?? {}, - ); - const executionData: IRunExecutionData = { - resultData: { runData, pinData }, - executionData: { - contextData: {}, - metadata: {}, - nodeExecutionStack, - waitingExecution, - waitingExecutionSource, - }, - }; - - const workflowExecute = new WorkflowExecute(additionalData, 'manual', executionData); - workflowExecution = workflowExecute.processRunExecutionData(workflow); - } else if ( - data.runData === undefined || - data.startNodes === undefined || - data.startNodes.length === 0 - ) { - // Full Execution - // TODO: When the old partial execution logic is removed this block can - // be removed and the previous one can be merged into - // `workflowExecute.runPartialWorkflow2`. - // Partial executions then require either a destination node from which - // everything else can be derived, or a triggerToStartFrom with - // triggerData. - this.logger.debug(`Execution ID ${executionId} will run executing all nodes.`, { - executionId, - }); - // Execute all nodes - - const startNode = WorkflowHelpers.getExecutionStartNode(data, workflow); - - // Can execute without webhook so go on - const workflowExecute = new WorkflowExecute(additionalData, data.executionMode); - workflowExecution = workflowExecute.run( - workflow, - startNode, - data.destinationNode, - data.pinData, - ); } else { - // Partial Execution - this.logger.debug(`Execution ID ${executionId} is a partial execution.`, { executionId }); - // Execute only the nodes between start and destination nodes - const workflowExecute = new WorkflowExecute(additionalData, data.executionMode); - - if (data.partialExecutionVersion === '1') { - workflowExecution = workflowExecute.runPartialWorkflow2( - workflow, - data.runData, - data.pinData, - data.dirtyNodeNames, - data.destinationNode, - ); - } else { - workflowExecution = workflowExecute.runPartialWorkflow( - workflow, - data.runData, - data.startNodes, - data.destinationNode, - data.pinData, - ); - } + workflowExecution = this.runManually(data, workflow, additionalData, executionId, pinData); } this.activeExecutions.attachWorkflowExecution(executionId, workflowExecution); @@ -539,4 +460,92 @@ export class WorkflowRunner { this.activeExecutions.attachWorkflowExecution(executionId, workflowExecution); } + + // eslint-disable-next-line @typescript-eslint/promise-function-async + runManually( + data: IWorkflowExecutionDataProcess, + workflow: Workflow, + additionalData: IWorkflowExecuteAdditionalData, + executionId: string, + pinData?: IPinData, + ) { + if (data.triggerToStartFrom?.data && data.startNodes && !data.destinationNode) { + this.logger.debug( + `Execution ID ${executionId} had triggerToStartFrom. Starting from that trigger.`, + { executionId }, + ); + const startNodes = data.startNodes.map((data) => { + const node = workflow.getNode(data.name); + a.ok(node, `Could not find a node named "${data.name}" in the workflow.`); + return node; + }); + const runData = { [data.triggerToStartFrom.name]: [data.triggerToStartFrom.data] }; + + const { nodeExecutionStack, waitingExecution, waitingExecutionSource } = + recreateNodeExecutionStack( + filterDisabledNodes(DirectedGraph.fromWorkflow(workflow)), + new Set(startNodes), + runData, + data.pinData ?? {}, + ); + const executionData: IRunExecutionData = { + resultData: { runData, pinData }, + executionData: { + contextData: {}, + metadata: {}, + nodeExecutionStack, + waitingExecution, + waitingExecutionSource, + }, + }; + + const workflowExecute = new WorkflowExecute(additionalData, 'manual', executionData); + return workflowExecute.processRunExecutionData(workflow); + } else if ( + data.runData === undefined || + data.startNodes === undefined || + data.startNodes.length === 0 + ) { + // Full Execution + // TODO: When the old partial execution logic is removed this block can + // be removed and the previous one can be merged into + // `workflowExecute.runPartialWorkflow2`. + // Partial executions then require either a destination node from which + // everything else can be derived, or a triggerToStartFrom with + // triggerData. + this.logger.debug(`Execution ID ${executionId} will run executing all nodes.`, { + executionId, + }); + // Execute all nodes + + const startNode = WorkflowHelpers.getExecutionStartNode(data, workflow); + + // Can execute without webhook so go on + const workflowExecute = new WorkflowExecute(additionalData, data.executionMode); + return workflowExecute.run(workflow, startNode, data.destinationNode, data.pinData); + } else { + // Partial Execution + this.logger.debug(`Execution ID ${executionId} is a partial execution.`, { executionId }); + // Execute only the nodes between start and destination nodes + const workflowExecute = new WorkflowExecute(additionalData, data.executionMode); + + if (data.partialExecutionVersion === '1') { + return workflowExecute.runPartialWorkflow2( + workflow, + data.runData, + data.pinData, + data.dirtyNodeNames, + data.destinationNode, + ); + } else { + return workflowExecute.runPartialWorkflow( + workflow, + data.runData, + data.startNodes, + data.destinationNode, + data.pinData, + ); + } + } + } } From ed359586c88a7662f4d94d58c5a87cf91d027ab9 Mon Sep 17 00:00:00 2001 From: Jon Date: Tue, 10 Dec 2024 15:58:19 +0000 Subject: [PATCH 05/10] feat(Redis Node): Add support for continue on fail / error output branch (#11714) --- packages/nodes-base/nodes/Redis/Redis.node.ts | 51 ++++++++++++++----- 1 file changed, 37 insertions(+), 14 deletions(-) diff --git a/packages/nodes-base/nodes/Redis/Redis.node.ts b/packages/nodes-base/nodes/Redis/Redis.node.ts index 59d1582fdb309..ccd826eb892eb 100644 --- a/packages/nodes-base/nodes/Redis/Redis.node.ts +++ b/packages/nodes-base/nodes/Redis/Redis.node.ts @@ -4,7 +4,7 @@ import type { INodeType, INodeTypeDescription, } from 'n8n-workflow'; -import { NodeConnectionType } from 'n8n-workflow'; +import { NodeConnectionType, NodeOperationError } from 'n8n-workflow'; import set from 'lodash/set'; @@ -521,17 +521,30 @@ export class Redis implements INodeType { const operation = this.getNodeParameter('operation', 0); const returnItems: INodeExecutionData[] = []; - try { - if (operation === 'info') { + if (operation === 'info') { + try { const result = await client.info(); returnItems.push({ json: convertInfoToObject(result) }); - } else if ( - ['delete', 'get', 'keys', 'set', 'incr', 'publish', 'push', 'pop'].includes(operation) - ) { - const items = this.getInputData(); + } catch (error) { + if (this.continueOnFail()) { + returnItems.push({ + json: { + error: error.message, + }, + }); + } else { + await client.quit(); + throw new NodeOperationError(this.getNode(), error); + } + } + } else if ( + ['delete', 'get', 'keys', 'set', 'incr', 'publish', 'push', 'pop'].includes(operation) + ) { + const items = this.getInputData(); - let item: INodeExecutionData; - for (let itemIndex = 0; itemIndex < items.length; itemIndex++) { + let item: INodeExecutionData; + for (let itemIndex = 0; itemIndex < items.length; itemIndex++) { + try { item = { json: {}, pairedItem: { item: itemIndex } }; if (operation === 'delete') { @@ -625,14 +638,24 @@ export class Redis implements INodeType { } returnItems.push(item); } + } catch (error) { + if (this.continueOnFail()) { + returnItems.push({ + json: { + error: error.message, + }, + pairedItem: { + item: itemIndex, + }, + }); + continue; + } + await client.quit(); + throw new NodeOperationError(this.getNode(), error, { itemIndex }); } } - } catch (error) { - throw error; - } finally { - await client.quit(); } - + await client.quit(); return [returnItems]; } } From 0468945c99f083577c4cc71f671b4b950f6aeb86 Mon Sep 17 00:00:00 2001 From: Csaba Tuncsik Date: Wed, 11 Dec 2024 10:11:51 +0100 Subject: [PATCH 06/10] fix(editor): Render sanitized HTML content in toast messages (#12139) --- .../editor-ui/src/components/NpsSurvey.vue | 1 - .../src/components/ParameterInputFull.vue | 1 - .../src/composables/useExecutionDebugging.ts | 2 +- .../src/composables/usePushConnection.test.ts | 1 - .../src/composables/usePushConnection.ts | 1 - .../src/composables/useToast.test.ts | 97 +++++++++++++------ .../editor-ui/src/composables/useToast.ts | 12 +-- .../src/composables/useWorkflowHelpers.ts | 1 - .../editor-ui/src/stores/settings.store.ts | 1 - packages/editor-ui/src/utils/htmlUtils.ts | 4 +- packages/editor-ui/src/views/NodeView.v2.vue | 2 - packages/editor-ui/src/views/NodeView.vue | 4 +- 12 files changed, 71 insertions(+), 56 deletions(-) diff --git a/packages/editor-ui/src/components/NpsSurvey.vue b/packages/editor-ui/src/components/NpsSurvey.vue index 818f7483f8acf..ce792ba20de0b 100644 --- a/packages/editor-ui/src/components/NpsSurvey.vue +++ b/packages/editor-ui/src/components/NpsSurvey.vue @@ -96,7 +96,6 @@ async function send() { message: Number(form.value.value) >= 8 ? i18n.baseText('prompts.npsSurvey.reviewUs') : '', type: 'success', duration: 15000, - dangerouslyUseHTMLString: true, }); setTimeout(() => { diff --git a/packages/editor-ui/src/components/ParameterInputFull.vue b/packages/editor-ui/src/components/ParameterInputFull.vue index 84b697f9da42f..332011b9ea743 100644 --- a/packages/editor-ui/src/components/ParameterInputFull.vue +++ b/packages/editor-ui/src/components/ParameterInputFull.vue @@ -167,7 +167,6 @@ function onDrop(newParamValue: string) { title: i18n.baseText('dataMapping.success.title'), message: i18n.baseText('dataMapping.success.moreInfo'), type: 'success', - dangerouslyUseHTMLString: true, }); ndvStore.setMappingOnboarded(); diff --git a/packages/editor-ui/src/composables/useExecutionDebugging.ts b/packages/editor-ui/src/composables/useExecutionDebugging.ts index 632eac195b863..19bb660d38e5a 100644 --- a/packages/editor-ui/src/composables/useExecutionDebugging.ts +++ b/packages/editor-ui/src/composables/useExecutionDebugging.ts @@ -76,7 +76,7 @@ export const useExecutionDebugging = () => { type: 'warning', confirmButtonText: i18n.baseText('nodeView.confirmMessage.debug.confirmButtonText'), cancelButtonText: i18n.baseText('nodeView.confirmMessage.debug.cancelButtonText'), - dangerouslyUseHTMLString: true, + customClass: 'matching-pinned-nodes-confirmation', }, ); diff --git a/packages/editor-ui/src/composables/usePushConnection.test.ts b/packages/editor-ui/src/composables/usePushConnection.test.ts index c2901249d63c4..63592afd795af 100644 --- a/packages/editor-ui/src/composables/usePushConnection.test.ts +++ b/packages/editor-ui/src/composables/usePushConnection.test.ts @@ -202,7 +202,6 @@ describe('usePushConnection()', () => { title: 'Problem in node ‘Last Node‘', type: 'error', duration: 0, - dangerouslyUseHTMLString: true, }); expect(result).toBeTruthy(); diff --git a/packages/editor-ui/src/composables/usePushConnection.ts b/packages/editor-ui/src/composables/usePushConnection.ts index 86ffbf358baec..6305ba499c673 100644 --- a/packages/editor-ui/src/composables/usePushConnection.ts +++ b/packages/editor-ui/src/composables/usePushConnection.ts @@ -397,7 +397,6 @@ export function usePushConnection({ router }: { router: ReturnType { - const original = await vi.importActual('element-plus'); - return { - ...original, - ElNotification: vi.fn(), - ElTooltip: vi.fn(), - }; -}); describe('useToast', () => { let toast: ReturnType; beforeEach(() => { - setActivePinia(createPinia()); + document.body.innerHTML = '
'; + createTestingPinia(); toast = useToast(); }); - afterEach(() => { - vi.restoreAllMocks(); - }); - - it('should show a message', () => { + it('should show a message', async () => { const messageData = { message: 'Test message', title: 'Test title' }; toast.showMessage(messageData); - expect(Notification).toHaveBeenCalledWith( - expect.objectContaining({ - message: 'Test message', - title: 'Test title', - }), - ); + await waitFor(() => { + expect(screen.getByRole('alert')).toBeVisible(); + expect( + within(screen.getByRole('alert')).getByRole('heading', { level: 2 }), + ).toHaveTextContent('Test title'); + expect(screen.getByRole('alert')).toContainHTML('

Test message

'); + }); }); - it('should sanitize message and title', () => { + it('should sanitize message and title', async () => { const messageData = { message: '', title: '', }; + toast.showMessage(messageData); + + await waitFor(() => { + expect(screen.getByRole('alert')).toBeVisible(); + expect( + within(screen.getByRole('alert')).getByRole('heading', { level: 2 }), + ).toHaveTextContent('alert("xss")'); + expect(screen.getByRole('alert')).toContainHTML('

alert("xss")

'); + }); + }); + + it('should sanitize but keep valid, allowed HTML tags', async () => { + const messageData = { + message: + 'Refresh to see the latest status.
More info or go to the Usage and plan settings page.', + title: 'Title', + }; + + toast.showMessage(messageData); + + await waitFor(() => { + expect(screen.getByRole('alert')).toBeVisible(); + expect( + within(screen.getByRole('alert')).getByRole('heading', { level: 2 }), + ).toHaveTextContent('Title'); + expect( + within(screen.getByRole('alert')).getByRole('heading', { level: 2 }).querySelectorAll('*'), + ).toHaveLength(0); + expect(screen.getByRole('alert')).toContainHTML( + 'Refresh to see the latest status.
More info or go to the Usage and plan settings page.', + ); + }); + }); + + it('should render component as message, sanitized as well', async () => { + const messageData = { + message: h( + defineComponent({ + template: '

Test content

', + }), + ), + }; toast.showMessage(messageData); - expect(Notification).toHaveBeenCalledWith( - expect.objectContaining({ - message: 'alert("xss")', - title: 'alert("xss")', - }), - ); + await waitFor(() => { + expect(screen.getByRole('alert')).toBeVisible(); + expect( + within(screen.getByRole('alert')).queryByRole('heading', { level: 2 }), + ).toHaveTextContent(''); + expect( + within(screen.getByRole('alert')).getByRole('heading', { level: 2 }).querySelectorAll('*'), + ).toHaveLength(0); + expect(screen.getByRole('alert')).toContainHTML('

Test content

'); + }); }); }); diff --git a/packages/editor-ui/src/composables/useToast.ts b/packages/editor-ui/src/composables/useToast.ts index 365a72e3fe043..764215cf70802 100644 --- a/packages/editor-ui/src/composables/useToast.ts +++ b/packages/editor-ui/src/composables/useToast.ts @@ -33,7 +33,7 @@ export function useToast() { const canvasStore = useCanvasStore(); const messageDefaults: Partial> = { - dangerouslyUseHTMLString: false, + dangerouslyUseHTMLString: true, position: 'bottom-right', zIndex: APP_Z_INDEXES.TOASTS, // above NDV and modal overlays offset: settingsStore.isAiAssistantEnabled || workflowsStore.isChatPanelOpen ? 64 : 0, @@ -82,7 +82,6 @@ export function useToast() { customClass?: string; closeOnClick?: boolean; type?: MessageBoxState['type']; - dangerouslyUseHTMLString?: boolean; }) { // eslint-disable-next-line prefer-const let notification: NotificationHandle; @@ -107,7 +106,6 @@ export function useToast() { duration: config.duration, customClass: config.customClass, type: config.type, - dangerouslyUseHTMLString: config.dangerouslyUseHTMLString ?? true, }); return notification; @@ -145,7 +143,6 @@ export function useToast() { ${collapsableDetails(error)}`, type: 'error', duration: 0, - dangerouslyUseHTMLString: true, }, false, ); @@ -165,12 +162,6 @@ export function useToast() { }); } - function showAlert(config: NotificationOptions): NotificationHandle { - return Notification({ - ...config, - }); - } - function causedByCredential(message: string | undefined) { if (!message || typeof message !== 'string') return false; @@ -209,7 +200,6 @@ export function useToast() { showMessage, showToast, showError, - showAlert, clearAllStickyNotifications, showNotificationForViews, }; diff --git a/packages/editor-ui/src/composables/useWorkflowHelpers.ts b/packages/editor-ui/src/composables/useWorkflowHelpers.ts index ca934cf2cf7bd..789540a5bcf20 100644 --- a/packages/editor-ui/src/composables/useWorkflowHelpers.ts +++ b/packages/editor-ui/src/composables/useWorkflowHelpers.ts @@ -882,7 +882,6 @@ export function useWorkflowHelpers(options: { router: ReturnType { message: i18n.baseText('startupError.message'), type: 'error', duration: 0, - dangerouslyUseHTMLString: true, }); throw e; diff --git a/packages/editor-ui/src/utils/htmlUtils.ts b/packages/editor-ui/src/utils/htmlUtils.ts index cf4bb85c6c0f3..f1d6119adbe80 100644 --- a/packages/editor-ui/src/utils/htmlUtils.ts +++ b/packages/editor-ui/src/utils/htmlUtils.ts @@ -18,8 +18,8 @@ export function sanitizeHtml(dirtyHtml: string) { } if (ALLOWED_HTML_ATTRIBUTES.includes(name) || name.startsWith('data-')) { - // href is allowed but we need to sanitize certain protocols - if (name === 'href' && !value.match(/^https?:\/\//gm)) { + // href is allowed but we allow only https and relative URLs + if (name === 'href' && !value.match(/^https?:\/\//gm) && !value.startsWith('/')) { return ''; } return `${name}="${escapeAttrValue(value)}"`; diff --git a/packages/editor-ui/src/views/NodeView.v2.vue b/packages/editor-ui/src/views/NodeView.v2.vue index 4f58b522942b1..e8a4c8b67f434 100644 --- a/packages/editor-ui/src/views/NodeView.v2.vue +++ b/packages/editor-ui/src/views/NodeView.v2.vue @@ -587,7 +587,6 @@ async function onClipboardPaste(plainTextData: string): Promise { cancelButtonText: i18n.baseText( 'nodeView.confirmMessage.onClipboardPasteEvent.cancelButtonText', ), - dangerouslyUseHTMLString: true, }, ); @@ -1368,7 +1367,6 @@ function checkIfEditingIsAllowed(): boolean { : 'readOnly.showMessage.executions.message', ), type: 'info', - dangerouslyUseHTMLString: true, }) as unknown as { visible: boolean }; return false; diff --git a/packages/editor-ui/src/views/NodeView.vue b/packages/editor-ui/src/views/NodeView.vue index f802cc3cd1cb8..b200903eace8a 100644 --- a/packages/editor-ui/src/views/NodeView.vue +++ b/packages/editor-ui/src/views/NodeView.vue @@ -825,7 +825,7 @@ export default defineComponent({ : 'readOnly.showMessage.executions.message', ), type: 'info', - dangerouslyUseHTMLString: true, + onClose: () => { this.readOnlyNotification = null; }, @@ -934,7 +934,6 @@ export default defineComponent({ // Close the creator panel if user clicked on the link if (this.createNodeActive) notice.close(); }, 0), - dangerouslyUseHTMLString: true, }); }, async clearExecutionData() { @@ -1827,7 +1826,6 @@ export default defineComponent({ cancelButtonText: this.i18n.baseText( 'nodeView.confirmMessage.onClipboardPasteEvent.cancelButtonText', ), - dangerouslyUseHTMLString: true, }, ); From e6985f79dbdc87a35bcc0cad7a8c1e3b4ab2f830 Mon Sep 17 00:00:00 2001 From: Alex Grozav Date: Wed, 11 Dec 2024 12:12:47 +0200 Subject: [PATCH 07/10] test: Update `20-workflow-executions` e2e tests for new canvas (#12136) --- cypress/pages/workflow-executions-tab.ts | 6 ++++++ cypress/pages/workflow.ts | 2 +- packages/editor-ui/src/views/NodeView.v2.vue | 8 ++++++++ 3 files changed, 15 insertions(+), 1 deletion(-) diff --git a/cypress/pages/workflow-executions-tab.ts b/cypress/pages/workflow-executions-tab.ts index 5e8c36c055552..be022e6cdf789 100644 --- a/cypress/pages/workflow-executions-tab.ts +++ b/cypress/pages/workflow-executions-tab.ts @@ -30,6 +30,12 @@ export class WorkflowExecutionsTab extends BasePage { actions = { toggleNodeEnabled: (nodeName: string) => { + cy.ifCanvasVersion( + () => {}, + () => { + cy.get('body').click(); // Cancel selection if it exists + }, + ); workflowPage.getters.canvasNodeByName(nodeName).click(); cy.get('body').type('d', { force: true }); }, diff --git a/cypress/pages/workflow.ts b/cypress/pages/workflow.ts index 8707783ca1275..86af76189d64a 100644 --- a/cypress/pages/workflow.ts +++ b/cypress/pages/workflow.ts @@ -97,7 +97,7 @@ export class WorkflowPage extends BasePage { disabledNodes: () => cy.ifCanvasVersion( () => cy.get('.node-box.disabled'), - () => cy.get('[data-test-id="canvas-trigger-node"][class*="disabled"]'), + () => cy.get('[data-test-id*="node"][class*="disabled"]'), ), selectedNodes: () => cy.ifCanvasVersion( diff --git a/packages/editor-ui/src/views/NodeView.v2.vue b/packages/editor-ui/src/views/NodeView.v2.vue index e8a4c8b67f434..65a141295e96f 100644 --- a/packages/editor-ui/src/views/NodeView.v2.vue +++ b/packages/editor-ui/src/views/NodeView.v2.vue @@ -1485,6 +1485,13 @@ function unregisterCustomActions() { unregisterCustomAction('showNodeCreator'); } +function showAddFirstStepIfEnabled() { + if (uiStore.addFirstStepOnLoad) { + void onOpenNodeCreatorForTriggerNodes(NODE_CREATOR_OPEN_SOURCES.TRIGGER_PLACEHOLDER_BUTTON); + uiStore.addFirstStepOnLoad = false; + } +} + /** * Routing */ @@ -1547,6 +1554,7 @@ onMounted(() => { onActivated(async () => { addUndoRedoEventBindings(); + showAddFirstStepIfEnabled(); }); onDeactivated(() => { From ec54333f7875d9f374c40110191223f8169c05a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E0=A4=95=E0=A4=BE=E0=A4=B0=E0=A4=A4=E0=A5=8B=E0=A4=AB?= =?UTF-8?q?=E0=A5=8D=E0=A4=AB=E0=A5=87=E0=A4=B2=E0=A4=B8=E0=A5=8D=E0=A4=95?= =?UTF-8?q?=E0=A5=8D=E0=A4=B0=E0=A4=BF=E0=A4=AA=E0=A5=8D=E0=A4=9F=E2=84=A2?= Date: Wed, 11 Dec 2024 11:26:27 +0100 Subject: [PATCH 08/10] refactor(core): Refactor execution contexts to reduce code duplication, and improve type-safety (no-changelog) (#12138) --- .../agents/Agent/agents/ToolsAgent/execute.ts | 4 +- .../src/workflow-execute-additional-data.ts | 6 - packages/core/src/Agent/index.ts | 61 ---- packages/core/src/NodeExecuteFunctions.ts | 286 +++++------------- packages/core/src/WorkflowExecute.ts | 4 +- .../__tests__/execute-context.test.ts | 22 +- .../__tests__/execute-single-context.test.ts | 20 +- .../__tests__/supply-data-context.test.ts | 24 +- .../base-execute-context.ts | 23 +- .../node-execution-context/execute-context.ts | 114 ++----- .../execute-single-context.ts | 27 +- .../supply-data-context.ts | 159 ++++++++-- .../node-execution-context/webhook-context.ts | 7 +- packages/workflow/src/Interfaces.ts | 13 +- packages/workflow/src/Workflow.ts | 4 +- 15 files changed, 327 insertions(+), 447 deletions(-) delete mode 100644 packages/core/src/Agent/index.ts diff --git a/packages/@n8n/nodes-langchain/nodes/agents/Agent/agents/ToolsAgent/execute.ts b/packages/@n8n/nodes-langchain/nodes/agents/Agent/agents/ToolsAgent/execute.ts index 74d68199618f2..9c92a6336174e 100644 --- a/packages/@n8n/nodes-langchain/nodes/agents/Agent/agents/ToolsAgent/execute.ts +++ b/packages/@n8n/nodes-langchain/nodes/agents/Agent/agents/ToolsAgent/execute.ts @@ -33,7 +33,7 @@ function getOutputParserSchema(outputParser: N8nOutputParser): ZodObject data.mimeType.startsWith('image/')) @@ -260,7 +260,7 @@ export async function toolsAgentExecute(this: IExecuteFunctions): Promise diff --git a/packages/core/src/Agent/index.ts b/packages/core/src/Agent/index.ts deleted file mode 100644 index ed842d99eee28..0000000000000 --- a/packages/core/src/Agent/index.ts +++ /dev/null @@ -1,61 +0,0 @@ -import type { - IExecuteFunctions, - Workflow, - IRunExecutionData, - INodeExecutionData, - ITaskDataConnections, - INode, - IWorkflowExecuteAdditionalData, - WorkflowExecuteMode, - INodeParameters, - IExecuteData, - IDataObject, - Result, -} from 'n8n-workflow'; -import { createEnvProviderState } from 'n8n-workflow'; - -export const createAgentStartJob = ( - additionalData: IWorkflowExecuteAdditionalData, - inputData: ITaskDataConnections, - node: INode, - workflow: Workflow, - runExecutionData: IRunExecutionData, - runIndex: number, - activeNodeName: string, - connectionInputData: INodeExecutionData[], - siblingParameters: INodeParameters, - mode: WorkflowExecuteMode, - executeData?: IExecuteData, - defaultReturnRunIndex?: number, - selfData?: IDataObject, - contextNodeName?: string, -): IExecuteFunctions['startJob'] => { - return async function startJob( - this: IExecuteFunctions, - jobType: string, - settings: unknown, - itemIndex: number, - ): Promise> { - return await additionalData.startAgentJob( - additionalData, - jobType, - settings, - this, - inputData, - node, - workflow, - runExecutionData, - runIndex, - itemIndex, - activeNodeName, - connectionInputData, - siblingParameters, - mode, - createEnvProviderState(), - executeData, - defaultReturnRunIndex, - selfData, - contextNodeName, - ); - }; -}; diff --git a/packages/core/src/NodeExecuteFunctions.ts b/packages/core/src/NodeExecuteFunctions.ts index 4400609fd8cb2..a07674eecdcae 100644 --- a/packages/core/src/NodeExecuteFunctions.ts +++ b/packages/core/src/NodeExecuteFunctions.ts @@ -54,9 +54,7 @@ import type { IPollFunctions, IRequestOptions, IRunExecutionData, - ITaskData, ITaskDataConnections, - ITaskMetadata, ITriggerFunctions, IWebhookData, IWebhookDescription, @@ -2021,113 +2019,6 @@ export function getWebhookDescription( return undefined; } -// TODO: Change options to an object -export const addExecutionDataFunctions = async ( - type: 'input' | 'output', - nodeName: string, - data: INodeExecutionData[][] | ExecutionBaseError, - runExecutionData: IRunExecutionData, - connectionType: NodeConnectionType, - additionalData: IWorkflowExecuteAdditionalData, - sourceNodeName: string, - sourceNodeRunIndex: number, - currentNodeRunIndex: number, - metadata?: ITaskMetadata, -): Promise => { - if (connectionType === NodeConnectionType.Main) { - throw new ApplicationError('Setting type is not supported for main connection', { - extra: { type }, - }); - } - - let taskData: ITaskData | undefined; - if (type === 'input') { - taskData = { - startTime: new Date().getTime(), - executionTime: 0, - executionStatus: 'running', - source: [null], - }; - } else { - // At the moment we expect that there is always an input sent before the output - taskData = get( - runExecutionData, - ['resultData', 'runData', nodeName, currentNodeRunIndex], - undefined, - ); - if (taskData === undefined) { - return; - } - taskData.metadata = metadata; - } - taskData = taskData!; - - if (data instanceof Error) { - taskData.executionStatus = 'error'; - taskData.error = data; - } else { - if (type === 'output') { - taskData.executionStatus = 'success'; - } - taskData.data = { - [connectionType]: data, - } as ITaskDataConnections; - } - - if (type === 'input') { - if (!(data instanceof Error)) { - taskData.inputOverride = { - [connectionType]: data, - } as ITaskDataConnections; - } - - if (!runExecutionData.resultData.runData.hasOwnProperty(nodeName)) { - runExecutionData.resultData.runData[nodeName] = []; - } - - runExecutionData.resultData.runData[nodeName][currentNodeRunIndex] = taskData; - if (additionalData.sendDataToUI) { - additionalData.sendDataToUI('nodeExecuteBefore', { - executionId: additionalData.executionId, - nodeName, - }); - } - } else { - // Outputs - taskData.executionTime = new Date().getTime() - taskData.startTime; - - if (additionalData.sendDataToUI) { - additionalData.sendDataToUI('nodeExecuteAfter', { - executionId: additionalData.executionId, - nodeName, - data: taskData, - }); - } - - if (get(runExecutionData, 'executionData.metadata', undefined) === undefined) { - runExecutionData.executionData!.metadata = {}; - } - - let sourceTaskData = get(runExecutionData, ['executionData', 'metadata', sourceNodeName]); - - if (!sourceTaskData) { - runExecutionData.executionData!.metadata[sourceNodeName] = []; - sourceTaskData = runExecutionData.executionData!.metadata[sourceNodeName]; - } - - if (!sourceTaskData[sourceNodeRunIndex]) { - sourceTaskData[sourceNodeRunIndex] = { - subRun: [], - }; - } - - sourceTaskData[sourceNodeRunIndex]!.subRun!.push({ - node: nodeName, - runIndex: currentNodeRunIndex, - }); - } -}; - export async function getInputConnectionData( this: IAllExecuteFunctions, workflow: Workflow, @@ -2139,7 +2030,7 @@ export async function getInputConnectionData( executeData: IExecuteData, mode: WorkflowExecuteMode, closeFunctions: CloseFunction[], - inputName: NodeConnectionType, + connectionType: NodeConnectionType, itemIndex: number, abortSignal?: AbortSignal, ): Promise { @@ -2150,14 +2041,14 @@ export async function getInputConnectionData( let inputConfiguration = inputs.find((input) => { if (typeof input === 'string') { - return input === inputName; + return input === connectionType; } - return input.type === inputName; + return input.type === connectionType; }); if (inputConfiguration === undefined) { throw new ApplicationError('Node does not have input of type', { - extra: { nodeName: node.name, inputName }, + extra: { nodeName: node.name, connectionType }, }); } @@ -2167,114 +2058,103 @@ export async function getInputConnectionData( } as INodeInputConfiguration; } - const parentNodes = workflow.getParentNodes(node.name, inputName, 1); - if (parentNodes.length === 0) { + const connectedNodes = workflow + .getParentNodes(node.name, connectionType, 1) + .map((nodeName) => workflow.getNode(nodeName) as INode) + .filter((connectedNode) => connectedNode.disabled !== true); + + if (connectedNodes.length === 0) { if (inputConfiguration.required) { throw new NodeOperationError( node, - `A ${inputConfiguration?.displayName ?? inputName} sub-node must be connected`, + `A ${inputConfiguration?.displayName ?? connectionType} sub-node must be connected and enabled`, ); } return inputConfiguration.maxConnections === 1 ? undefined : []; } - const constParentNodes = parentNodes - .map((nodeName) => { - return workflow.getNode(nodeName) as INode; - }) - .filter((connectedNode) => connectedNode.disabled !== true) - .map(async (connectedNode) => { - const nodeType = workflow.nodeTypes.getByNameAndVersion( - connectedNode.type, - connectedNode.typeVersion, - ); - const context = new SupplyDataContext( - workflow, - connectedNode, - additionalData, - mode, - runExecutionData, - runIndex, - connectionInputData, - inputData, - executeData, - closeFunctions, - abortSignal, - ); - - if (!nodeType.supplyData) { - if (nodeType.description.outputs.includes(NodeConnectionType.AiTool)) { - nodeType.supplyData = async function (this: ISupplyDataFunctions) { - return createNodeAsTool(this, nodeType, this.getNode().parameters); - }; - } else { - throw new ApplicationError('Node does not have a `supplyData` method defined', { - extra: { nodeName: connectedNode.name }, - }); - } - } - - try { - const response = await nodeType.supplyData.call(context, itemIndex); - if (response.closeFunction) { - closeFunctions.push(response.closeFunction); - } - return response; - } catch (error) { - // Propagate errors from sub-nodes - if (error.functionality === 'configuration-node') throw error; - if (!(error instanceof ExecutionBaseError)) { - error = new NodeOperationError(connectedNode, error, { - itemIndex, - }); - } + if ( + inputConfiguration.maxConnections !== undefined && + connectedNodes.length > inputConfiguration.maxConnections + ) { + throw new NodeOperationError( + node, + `Only ${inputConfiguration.maxConnections} ${connectionType} sub-nodes are/is allowed to be connected`, + ); + } - let currentNodeRunIndex = 0; - if (runExecutionData.resultData.runData.hasOwnProperty(node.name)) { - currentNodeRunIndex = runExecutionData.resultData.runData[node.name].length; - } + const constParentNodes = connectedNodes.map(async (connectedNode) => { + const nodeType = workflow.nodeTypes.getByNameAndVersion( + connectedNode.type, + connectedNode.typeVersion, + ); + const context = new SupplyDataContext( + workflow, + connectedNode, + additionalData, + mode, + runExecutionData, + runIndex, + connectionInputData, + inputData, + executeData, + closeFunctions, + abortSignal, + ); - // Display the error on the node which is causing it - await addExecutionDataFunctions( - 'input', - connectedNode.name, - error, - runExecutionData, - inputName, - additionalData, - node.name, - runIndex, - currentNodeRunIndex, - ); + if (!nodeType.supplyData) { + if (nodeType.description.outputs.includes(NodeConnectionType.AiTool)) { + nodeType.supplyData = async function (this: ISupplyDataFunctions) { + return createNodeAsTool(this, nodeType, this.getNode().parameters); + }; + } else { + throw new ApplicationError('Node does not have a `supplyData` method defined', { + extra: { nodeName: connectedNode.name }, + }); + } + } - // Display on the calling node which node has the error - throw new NodeOperationError(connectedNode, `Error in sub-node ${connectedNode.name}`, { + try { + const response = await nodeType.supplyData.call(context, itemIndex); + if (response.closeFunction) { + closeFunctions.push(response.closeFunction); + } + return response; + } catch (error) { + // Propagate errors from sub-nodes + if (error.functionality === 'configuration-node') throw error; + if (!(error instanceof ExecutionBaseError)) { + error = new NodeOperationError(connectedNode, error, { itemIndex, - functionality: 'configuration-node', - description: error.message, }); } - }); + + let currentNodeRunIndex = 0; + if (runExecutionData.resultData.runData.hasOwnProperty(node.name)) { + currentNodeRunIndex = runExecutionData.resultData.runData[node.name].length; + } + + // Display the error on the node which is causing it + await context.addExecutionDataFunctions( + 'input', + error, + connectionType, + node.name, + currentNodeRunIndex, + ); + + // Display on the calling node which node has the error + throw new NodeOperationError(connectedNode, `Error in sub-node ${connectedNode.name}`, { + itemIndex, + functionality: 'configuration-node', + description: error.message, + }); + } + }); // Validate the inputs const nodes = await Promise.all(constParentNodes); - if (inputConfiguration.required && nodes.length === 0) { - throw new NodeOperationError( - node, - `A ${inputConfiguration?.displayName ?? inputName} sub-node must be connected`, - ); - } - if ( - inputConfiguration.maxConnections !== undefined && - nodes.length > inputConfiguration.maxConnections - ) { - throw new NodeOperationError( - node, - `Only ${inputConfiguration.maxConnections} ${inputName} sub-nodes are/is allowed to be connected`, - ); - } - return inputConfiguration.maxConnections === 1 ? (nodes || [])[0]?.response : nodes.map((node) => node.response); diff --git a/packages/core/src/WorkflowExecute.ts b/packages/core/src/WorkflowExecute.ts index 2990d61cf8626..27244fafc3d8c 100644 --- a/packages/core/src/WorkflowExecute.ts +++ b/packages/core/src/WorkflowExecute.ts @@ -1018,8 +1018,8 @@ export class WorkflowExecute { // Update the pairedItem information on items const newTaskDataConnections: ITaskDataConnections = {}; - for (const inputName of Object.keys(executionData.data)) { - newTaskDataConnections[inputName] = executionData.data[inputName].map( + for (const connectionType of Object.keys(executionData.data)) { + newTaskDataConnections[connectionType] = executionData.data[connectionType].map( (input, inputIndex) => { if (input === null) { return input; diff --git a/packages/core/src/node-execution-context/__tests__/execute-context.test.ts b/packages/core/src/node-execution-context/__tests__/execute-context.test.ts index 723bab24f3e47..a888a5a7ffc0b 100644 --- a/packages/core/src/node-execution-context/__tests__/execute-context.test.ts +++ b/packages/core/src/node-execution-context/__tests__/execute-context.test.ts @@ -14,7 +14,7 @@ import type { INodeTypes, ICredentialDataDecryptedObject, } from 'n8n-workflow'; -import { ApplicationError, ExpressionError } from 'n8n-workflow'; +import { ApplicationError, ExpressionError, NodeConnectionType } from 'n8n-workflow'; import { describeCommonTests } from './shared-tests'; import { ExecuteContext } from '../execute-context'; @@ -92,33 +92,39 @@ describe('ExecuteContext', () => { describe('getInputData', () => { const inputIndex = 0; - const inputName = 'main'; + const connectionType = NodeConnectionType.Main; afterEach(() => { - inputData[inputName] = [[{ json: { test: 'data' } }]]; + inputData[connectionType] = [[{ json: { test: 'data' } }]]; }); it('should return the input data correctly', () => { const expectedData = [{ json: { test: 'data' } }]; - expect(executeContext.getInputData(inputIndex, inputName)).toEqual(expectedData); + expect(executeContext.getInputData(inputIndex, connectionType)).toEqual(expectedData); }); it('should return an empty array if the input name does not exist', () => { - const inputName = 'nonExistent'; - expect(executeContext.getInputData(inputIndex, inputName)).toEqual([]); + const connectionType = 'nonExistent'; + expect(executeContext.getInputData(inputIndex, connectionType as NodeConnectionType)).toEqual( + [], + ); }); it('should throw an error if the input index is out of range', () => { const inputIndex = 2; - expect(() => executeContext.getInputData(inputIndex, inputName)).toThrow(ApplicationError); + expect(() => executeContext.getInputData(inputIndex, connectionType)).toThrow( + ApplicationError, + ); }); it('should throw an error if the input index was not set', () => { inputData.main[inputIndex] = null; - expect(() => executeContext.getInputData(inputIndex, inputName)).toThrow(ApplicationError); + expect(() => executeContext.getInputData(inputIndex, connectionType)).toThrow( + ApplicationError, + ); }); }); diff --git a/packages/core/src/node-execution-context/__tests__/execute-single-context.test.ts b/packages/core/src/node-execution-context/__tests__/execute-single-context.test.ts index e62c2b0f4632d..6c1b9f108955c 100644 --- a/packages/core/src/node-execution-context/__tests__/execute-single-context.test.ts +++ b/packages/core/src/node-execution-context/__tests__/execute-single-context.test.ts @@ -14,7 +14,7 @@ import type { INodeTypes, ICredentialDataDecryptedObject, } from 'n8n-workflow'; -import { ApplicationError } from 'n8n-workflow'; +import { ApplicationError, NodeConnectionType } from 'n8n-workflow'; import { describeCommonTests } from './shared-tests'; import { ExecuteSingleContext } from '../execute-single-context'; @@ -91,29 +91,31 @@ describe('ExecuteSingleContext', () => { describe('getInputData', () => { const inputIndex = 0; - const inputName = 'main'; + const connectionType = NodeConnectionType.Main; afterEach(() => { - inputData[inputName] = [[{ json: { test: 'data' } }]]; + inputData[connectionType] = [[{ json: { test: 'data' } }]]; }); it('should return the input data correctly', () => { const expectedData = { json: { test: 'data' } }; - expect(executeSingleContext.getInputData(inputIndex, inputName)).toEqual(expectedData); + expect(executeSingleContext.getInputData(inputIndex, connectionType)).toEqual(expectedData); }); it('should return an empty object if the input name does not exist', () => { - const inputName = 'nonExistent'; + const connectionType = 'nonExistent'; const expectedData = { json: {} }; - expect(executeSingleContext.getInputData(inputIndex, inputName)).toEqual(expectedData); + expect( + executeSingleContext.getInputData(inputIndex, connectionType as NodeConnectionType), + ).toEqual(expectedData); }); it('should throw an error if the input index is out of range', () => { const inputIndex = 1; - expect(() => executeSingleContext.getInputData(inputIndex, inputName)).toThrow( + expect(() => executeSingleContext.getInputData(inputIndex, connectionType)).toThrow( ApplicationError, ); }); @@ -121,7 +123,7 @@ describe('ExecuteSingleContext', () => { it('should throw an error if the input index was not set', () => { inputData.main[inputIndex] = null; - expect(() => executeSingleContext.getInputData(inputIndex, inputName)).toThrow( + expect(() => executeSingleContext.getInputData(inputIndex, connectionType)).toThrow( ApplicationError, ); }); @@ -129,7 +131,7 @@ describe('ExecuteSingleContext', () => { it('should throw an error if the value of input with given index was not set', () => { delete inputData.main[inputIndex]![itemIndex]; - expect(() => executeSingleContext.getInputData(inputIndex, inputName)).toThrow( + expect(() => executeSingleContext.getInputData(inputIndex, connectionType)).toThrow( ApplicationError, ); }); diff --git a/packages/core/src/node-execution-context/__tests__/supply-data-context.test.ts b/packages/core/src/node-execution-context/__tests__/supply-data-context.test.ts index 6c5a3849dda22..d3ebddc75cc96 100644 --- a/packages/core/src/node-execution-context/__tests__/supply-data-context.test.ts +++ b/packages/core/src/node-execution-context/__tests__/supply-data-context.test.ts @@ -14,7 +14,7 @@ import type { INodeTypes, ICredentialDataDecryptedObject, } from 'n8n-workflow'; -import { ApplicationError } from 'n8n-workflow'; +import { ApplicationError, NodeConnectionType } from 'n8n-workflow'; import { describeCommonTests } from './shared-tests'; import { SupplyDataContext } from '../supply-data-context'; @@ -56,7 +56,8 @@ describe('SupplyDataContext', () => { const mode: WorkflowExecuteMode = 'manual'; const runExecutionData = mock(); const connectionInputData: INodeExecutionData[] = []; - const inputData: ITaskDataConnections = { main: [[{ json: { test: 'data' } }]] }; + const connectionType = NodeConnectionType.Main; + const inputData: ITaskDataConnections = { [connectionType]: [[{ json: { test: 'data' } }]] }; const executeData = mock(); const runIndex = 0; const closeFn = jest.fn(); @@ -91,33 +92,38 @@ describe('SupplyDataContext', () => { describe('getInputData', () => { const inputIndex = 0; - const inputName = 'main'; afterEach(() => { - inputData[inputName] = [[{ json: { test: 'data' } }]]; + inputData[connectionType] = [[{ json: { test: 'data' } }]]; }); it('should return the input data correctly', () => { const expectedData = [{ json: { test: 'data' } }]; - expect(supplyDataContext.getInputData(inputIndex, inputName)).toEqual(expectedData); + expect(supplyDataContext.getInputData(inputIndex, connectionType)).toEqual(expectedData); }); it('should return an empty array if the input name does not exist', () => { - const inputName = 'nonExistent'; - expect(supplyDataContext.getInputData(inputIndex, inputName)).toEqual([]); + const connectionType = 'nonExistent'; + expect( + supplyDataContext.getInputData(inputIndex, connectionType as NodeConnectionType), + ).toEqual([]); }); it('should throw an error if the input index is out of range', () => { const inputIndex = 2; - expect(() => supplyDataContext.getInputData(inputIndex, inputName)).toThrow(ApplicationError); + expect(() => supplyDataContext.getInputData(inputIndex, connectionType)).toThrow( + ApplicationError, + ); }); it('should throw an error if the input index was not set', () => { inputData.main[inputIndex] = null; - expect(() => supplyDataContext.getInputData(inputIndex, inputName)).toThrow(ApplicationError); + expect(() => supplyDataContext.getInputData(inputIndex, connectionType)).toThrow( + ApplicationError, + ); }); }); diff --git a/packages/core/src/node-execution-context/base-execute-context.ts b/packages/core/src/node-execution-context/base-execute-context.ts index 0794a263b019a..4f186e559708c 100644 --- a/packages/core/src/node-execution-context/base-execute-context.ts +++ b/packages/core/src/node-execution-context/base-execute-context.ts @@ -21,6 +21,7 @@ import type { IWorkflowDataProxyData, ISourceData, AiEvent, + NodeConnectionType, } from 'n8n-workflow'; import { ApplicationError, NodeHelpers, WAIT_INDEFINITELY, WorkflowDataProxy } from 'n8n-workflow'; import { Container } from 'typedi'; @@ -137,6 +138,24 @@ export class BaseExecuteContext extends NodeExecutionContext { return { ...result, data }; } + protected getInputItems(inputIndex: number, connectionType: NodeConnectionType) { + const inputData = this.inputData[connectionType]; + if (inputData.length < inputIndex) { + throw new ApplicationError('Could not get input with given index', { + extra: { inputIndex, connectionType }, + }); + } + + const allItems = inputData[inputIndex] as INodeExecutionData[] | null | undefined; + if (allItems === null) { + throw new ApplicationError('Input index was not set', { + extra: { inputIndex, connectionType }, + }); + } + + return allItems; + } + getNodeInputs(): INodeInputConfiguration[] { const nodeType = this.workflow.nodeTypes.getByNameAndVersion( this.node.type, @@ -157,12 +176,12 @@ export class BaseExecuteContext extends NodeExecutionContext { ); } - getInputSourceData(inputIndex = 0, inputName = 'main'): ISourceData { + getInputSourceData(inputIndex = 0, connectionType = 'main'): ISourceData { if (this.executeData?.source === null) { // Should never happen as n8n sets it automatically throw new ApplicationError('Source data is missing'); } - return this.executeData.source[inputName][inputIndex]!; + return this.executeData.source[connectionType][inputIndex]!; } getWorkflowDataProxy(itemIndex: number): IWorkflowDataProxyData { diff --git a/packages/core/src/node-execution-context/execute-context.ts b/packages/core/src/node-execution-context/execute-context.ts index c587fb2168614..2424697c6fd87 100644 --- a/packages/core/src/node-execution-context/execute-context.ts +++ b/packages/core/src/node-execution-context/execute-context.ts @@ -1,7 +1,6 @@ import type { CallbackManager, CloseFunction, - ExecutionBaseError, IExecuteData, IExecuteFunctions, IExecuteResponsePromiseData, @@ -10,15 +9,18 @@ import type { INodeExecutionData, IRunExecutionData, ITaskDataConnections, - ITaskMetadata, IWorkflowExecuteAdditionalData, - NodeConnectionType, + Result, Workflow, WorkflowExecuteMode, } from 'n8n-workflow'; -import { ApplicationError, createDeferredPromise } from 'n8n-workflow'; +import { + ApplicationError, + createDeferredPromise, + createEnvProviderState, + NodeConnectionType, +} from 'n8n-workflow'; -import { createAgentStartJob } from '@/Agent'; // eslint-disable-next-line import/no-cycle import { returnJsonArray, @@ -26,7 +28,6 @@ import { normalizeItems, constructExecutionMetaData, getInputConnectionData, - addExecutionDataFunctions, assertBinaryData, getBinaryDataBuffer, copyBinaryFile, @@ -46,8 +47,6 @@ export class ExecuteContext extends BaseExecuteContext implements IExecuteFuncti readonly getNodeParameter: IExecuteFunctions['getNodeParameter']; - readonly startJob: IExecuteFunctions['startJob']; - constructor( workflow: Workflow, node: INode, @@ -122,23 +121,37 @@ export class ExecuteContext extends BaseExecuteContext implements IExecuteFuncti fallbackValue, options, )) as IExecuteFunctions['getNodeParameter']; + } - this.startJob = createAgentStartJob( + async startJob( + jobType: string, + settings: unknown, + itemIndex: number, + ): Promise> { + return await this.additionalData.startAgentJob( this.additionalData, + jobType, + settings, + this, this.inputData, this.node, this.workflow, this.runExecutionData, this.runIndex, + itemIndex, this.node.name, this.connectionInputData, {}, this.mode, + createEnvProviderState(), this.executeData, ); } - async getInputConnectionData(inputName: NodeConnectionType, itemIndex: number): Promise { + async getInputConnectionData( + connectionType: NodeConnectionType, + itemIndex: number, + ): Promise { return await getInputConnectionData.call( this, this.workflow, @@ -150,33 +163,18 @@ export class ExecuteContext extends BaseExecuteContext implements IExecuteFuncti this.executeData, this.mode, this.closeFunctions, - inputName, + connectionType, itemIndex, this.abortSignal, ); } - getInputData(inputIndex = 0, inputName = 'main') { - if (!this.inputData.hasOwnProperty(inputName)) { + getInputData(inputIndex = 0, connectionType = NodeConnectionType.Main) { + if (!this.inputData.hasOwnProperty(connectionType)) { // Return empty array because else it would throw error when nothing is connected to input return []; } - - const inputData = this.inputData[inputName]; - // TODO: Check if nodeType has input with that index defined - if (inputData.length < inputIndex) { - throw new ApplicationError('Could not get input with given index', { - extra: { inputIndex, inputName }, - }); - } - - if (inputData[inputIndex] === null) { - throw new ApplicationError('Value of input was not set', { - extra: { inputIndex, inputName }, - }); - } - - return inputData[inputIndex]; + return super.getInputItems(inputIndex, connectionType) ?? []; } logNodeOutput(...args: unknown[]): void { @@ -194,60 +192,14 @@ export class ExecuteContext extends BaseExecuteContext implements IExecuteFuncti await this.additionalData.hooks?.executeHookFunctions('sendResponse', [response]); } - addInputData( - connectionType: NodeConnectionType, - data: INodeExecutionData[][] | ExecutionBaseError, - ): { index: number } { - const nodeName = this.node.name; - let currentNodeRunIndex = 0; - if (this.runExecutionData.resultData.runData.hasOwnProperty(nodeName)) { - currentNodeRunIndex = this.runExecutionData.resultData.runData[nodeName].length; - } - - void addExecutionDataFunctions( - 'input', - nodeName, - data, - this.runExecutionData, - connectionType, - this.additionalData, - nodeName, - this.runIndex, - currentNodeRunIndex, - ).catch((error) => { - this.logger.warn( - // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access - `There was a problem logging input data of node "${nodeName}": ${error.message}`, - ); - }); - - return { index: currentNodeRunIndex }; + /** @deprecated use ISupplyDataFunctions.addInputData */ + addInputData(): { index: number } { + throw new ApplicationError('addInputData should not be called on IExecuteFunctions'); } - addOutputData( - connectionType: NodeConnectionType, - currentNodeRunIndex: number, - data: INodeExecutionData[][] | ExecutionBaseError, - metadata?: ITaskMetadata, - ): void { - const nodeName = this.node.name; - addExecutionDataFunctions( - 'output', - nodeName, - data, - this.runExecutionData, - connectionType, - this.additionalData, - nodeName, - this.runIndex, - currentNodeRunIndex, - metadata, - ).catch((error) => { - this.logger.warn( - // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access - `There was a problem logging output data of node "${nodeName}": ${error.message}`, - ); - }); + /** @deprecated use ISupplyDataFunctions.addOutputData */ + addOutputData(): void { + throw new ApplicationError('addOutputData should not be called on IExecuteFunctions'); } getParentCallbackManager(): CallbackManager | undefined { diff --git a/packages/core/src/node-execution-context/execute-single-context.ts b/packages/core/src/node-execution-context/execute-single-context.ts index 91c7fcf683030..cb46ea9c91e33 100644 --- a/packages/core/src/node-execution-context/execute-single-context.ts +++ b/packages/core/src/node-execution-context/execute-single-context.ts @@ -11,7 +11,7 @@ import type { ITaskDataConnections, IExecuteData, } from 'n8n-workflow'; -import { ApplicationError, createDeferredPromise } from 'n8n-workflow'; +import { ApplicationError, createDeferredPromise, NodeConnectionType } from 'n8n-workflow'; // eslint-disable-next-line import/no-cycle import { @@ -76,31 +76,18 @@ export class ExecuteSingleContext extends BaseExecuteContext implements IExecute return super.evaluateExpression(expression, itemIndex); } - getInputData(inputIndex = 0, inputName = 'main') { - if (!this.inputData.hasOwnProperty(inputName)) { + getInputData(inputIndex = 0, connectionType = NodeConnectionType.Main) { + if (!this.inputData.hasOwnProperty(connectionType)) { // Return empty array because else it would throw error when nothing is connected to input return { json: {} }; } - // TODO: Check if nodeType has input with that index defined - if (this.inputData[inputName].length < inputIndex) { - throw new ApplicationError('Could not get input index', { - extra: { inputIndex, inputName }, - }); - } - - const allItems = this.inputData[inputName][inputIndex]; - - if (allItems === null || allItems === undefined) { - throw new ApplicationError('Input index was not set', { - extra: { inputIndex, inputName }, - }); - } + const allItems = super.getInputItems(inputIndex, connectionType); - const data = allItems[this.itemIndex]; - if (data === null || data === undefined) { + const data = allItems?.[this.itemIndex]; + if (data === undefined) { throw new ApplicationError('Value of input with given index was not set', { - extra: { inputIndex, inputName, itemIndex: this.itemIndex }, + extra: { inputIndex, connectionType, itemIndex: this.itemIndex }, }); } diff --git a/packages/core/src/node-execution-context/supply-data-context.ts b/packages/core/src/node-execution-context/supply-data-context.ts index bab9d111089e9..0155b9d85e857 100644 --- a/packages/core/src/node-execution-context/supply-data-context.ts +++ b/packages/core/src/node-execution-context/supply-data-context.ts @@ -1,19 +1,21 @@ +import get from 'lodash/get'; import type { CloseFunction, + ExecutionBaseError, IExecuteData, IGetNodeParameterOptions, INode, INodeExecutionData, IRunExecutionData, ISupplyDataFunctions, + ITaskData, ITaskDataConnections, ITaskMetadata, IWorkflowExecuteAdditionalData, - NodeConnectionType, Workflow, WorkflowExecuteMode, } from 'n8n-workflow'; -import { ApplicationError, createDeferredPromise } from 'n8n-workflow'; +import { ApplicationError, NodeConnectionType, createDeferredPromise } from 'n8n-workflow'; // eslint-disable-next-line import/no-cycle import { @@ -29,7 +31,6 @@ import { normalizeItems, returnJsonArray, getInputConnectionData, - addExecutionDataFunctions, } from '@/NodeExecuteFunctions'; import { BaseExecuteContext } from './base-execute-context'; @@ -104,7 +105,10 @@ export class SupplyDataContext extends BaseExecuteContext implements ISupplyData )) as ISupplyDataFunctions['getNodeParameter']; } - async getInputConnectionData(inputName: NodeConnectionType, itemIndex: number): Promise { + async getInputConnectionData( + connectionType: NodeConnectionType, + itemIndex: number, + ): Promise { return await getInputConnectionData.call( this, this.workflow, @@ -116,34 +120,21 @@ export class SupplyDataContext extends BaseExecuteContext implements ISupplyData this.executeData, this.mode, this.closeFunctions, - inputName, + connectionType, itemIndex, this.abortSignal, ); } - getInputData(inputIndex = 0, inputName = 'main') { - if (!this.inputData.hasOwnProperty(inputName)) { + getInputData(inputIndex = 0, connectionType = NodeConnectionType.Main) { + if (!this.inputData.hasOwnProperty(connectionType)) { // Return empty array because else it would throw error when nothing is connected to input return []; } - - // TODO: Check if nodeType has input with that index defined - if (this.inputData[inputName].length < inputIndex) { - throw new ApplicationError('Could not get input with given index', { - extra: { inputIndex, inputName }, - }); - } - - if (this.inputData[inputName][inputIndex] === null) { - throw new ApplicationError('Value of input was not set', { - extra: { inputIndex, inputName }, - }); - } - - return this.inputData[inputName][inputIndex]; + return super.getInputItems(inputIndex, connectionType) ?? []; } + /** @deprecated create a context object with inputData for every runIndex */ addInputData( connectionType: NodeConnectionType, data: INodeExecutionData[][], @@ -154,15 +145,11 @@ export class SupplyDataContext extends BaseExecuteContext implements ISupplyData currentNodeRunIndex = this.runExecutionData.resultData.runData[nodeName].length; } - addExecutionDataFunctions( + this.addExecutionDataFunctions( 'input', - nodeName, data, - this.runExecutionData, connectionType, - this.additionalData, nodeName, - this.runIndex, currentNodeRunIndex, ).catch((error) => { this.logger.warn( @@ -176,6 +163,7 @@ export class SupplyDataContext extends BaseExecuteContext implements ISupplyData return { index: currentNodeRunIndex }; } + /** @deprecated Switch to WorkflowExecute to store output on runExecutionData.resultData.runData */ addOutputData( connectionType: NodeConnectionType, currentNodeRunIndex: number, @@ -183,15 +171,11 @@ export class SupplyDataContext extends BaseExecuteContext implements ISupplyData metadata?: ITaskMetadata, ): void { const nodeName = this.node.name; - addExecutionDataFunctions( + this.addExecutionDataFunctions( 'output', - nodeName, data, - this.runExecutionData, connectionType, - this.additionalData, nodeName, - this.runIndex, currentNodeRunIndex, metadata, ).catch((error) => { @@ -203,4 +187,115 @@ export class SupplyDataContext extends BaseExecuteContext implements ISupplyData ); }); } + + async addExecutionDataFunctions( + type: 'input' | 'output', + data: INodeExecutionData[][] | ExecutionBaseError, + connectionType: NodeConnectionType, + sourceNodeName: string, + currentNodeRunIndex: number, + metadata?: ITaskMetadata, + ): Promise { + if (connectionType === NodeConnectionType.Main) { + throw new ApplicationError('Setting type is not supported for main connection', { + extra: { type }, + }); + } + + const { + additionalData, + runExecutionData, + runIndex: sourceNodeRunIndex, + node: { name: nodeName }, + } = this; + + let taskData: ITaskData | undefined; + if (type === 'input') { + taskData = { + startTime: new Date().getTime(), + executionTime: 0, + executionStatus: 'running', + source: [null], + }; + } else { + // At the moment we expect that there is always an input sent before the output + taskData = get( + runExecutionData, + ['resultData', 'runData', nodeName, currentNodeRunIndex], + undefined, + ); + if (taskData === undefined) { + return; + } + taskData.metadata = metadata; + } + taskData = taskData!; + + if (data instanceof Error) { + taskData.executionStatus = 'error'; + taskData.error = data; + } else { + if (type === 'output') { + taskData.executionStatus = 'success'; + } + taskData.data = { + [connectionType]: data, + } as ITaskDataConnections; + } + + if (type === 'input') { + if (!(data instanceof Error)) { + this.inputData[connectionType] = data; + // TODO: remove inputOverride + taskData.inputOverride = { + [connectionType]: data, + } as ITaskDataConnections; + } + + if (!runExecutionData.resultData.runData.hasOwnProperty(nodeName)) { + runExecutionData.resultData.runData[nodeName] = []; + } + + runExecutionData.resultData.runData[nodeName][currentNodeRunIndex] = taskData; + if (additionalData.sendDataToUI) { + additionalData.sendDataToUI('nodeExecuteBefore', { + executionId: additionalData.executionId, + nodeName, + }); + } + } else { + // Outputs + taskData.executionTime = new Date().getTime() - taskData.startTime; + + if (additionalData.sendDataToUI) { + additionalData.sendDataToUI('nodeExecuteAfter', { + executionId: additionalData.executionId, + nodeName, + data: taskData, + }); + } + + if (get(runExecutionData, 'executionData.metadata', undefined) === undefined) { + runExecutionData.executionData!.metadata = {}; + } + + let sourceTaskData = runExecutionData.executionData?.metadata?.[sourceNodeName]; + + if (!sourceTaskData) { + runExecutionData.executionData!.metadata[sourceNodeName] = []; + sourceTaskData = runExecutionData.executionData!.metadata[sourceNodeName]; + } + + if (!sourceTaskData[sourceNodeRunIndex]) { + sourceTaskData[sourceNodeRunIndex] = { + subRun: [], + }; + } + + sourceTaskData[sourceNodeRunIndex].subRun!.push({ + node: nodeName, + runIndex: currentNodeRunIndex, + }); + } + } } diff --git a/packages/core/src/node-execution-context/webhook-context.ts b/packages/core/src/node-execution-context/webhook-context.ts index e1dae9c1debfe..8f55b26097bbc 100644 --- a/packages/core/src/node-execution-context/webhook-context.ts +++ b/packages/core/src/node-execution-context/webhook-context.ts @@ -138,7 +138,10 @@ export class WebhookContext extends NodeExecutionContext implements IWebhookFunc return this.webhookData.webhookDescription.name; } - async getInputConnectionData(inputName: NodeConnectionType, itemIndex: number): Promise { + async getInputConnectionData( + connectionType: NodeConnectionType, + itemIndex: number, + ): Promise { // To be able to use expressions like "$json.sessionId" set the // body data the webhook received to what is normally used for // incoming node data. @@ -170,7 +173,7 @@ export class WebhookContext extends NodeExecutionContext implements IWebhookFunc executeData, this.mode, this.closeFunctions, - inputName, + connectionType, itemIndex, ); } diff --git a/packages/workflow/src/Interfaces.ts b/packages/workflow/src/Interfaces.ts index b5bca9c5c375e..f88db21ae71dc 100644 --- a/packages/workflow/src/Interfaces.ts +++ b/packages/workflow/src/Interfaces.ts @@ -943,7 +943,7 @@ type BaseExecutionFunctions = FunctionsBaseWithRequiredKeys<'getMode'> & { getContext(type: ContextType): IContextObject; getExecuteData(): IExecuteData; getWorkflowDataProxy(itemIndex: number): IWorkflowDataProxyData; - getInputSourceData(inputIndex?: number, inputName?: string): ISourceData; + getInputSourceData(inputIndex?: number, connectionType?: NodeConnectionType): ISourceData; getExecutionCancelSignal(): AbortSignal | undefined; onExecutionCancellation(handler: () => unknown): void; logAiEvent(eventName: AiEvent, msg?: string | undefined): void; @@ -962,11 +962,11 @@ export type IExecuteFunctions = ExecuteFunctions.GetNodeParameterFn & }, ): Promise; getInputConnectionData( - inputName: NodeConnectionType, + connectionType: NodeConnectionType, itemIndex: number, inputIndex?: number, ): Promise; - getInputData(inputIndex?: number, inputName?: string): INodeExecutionData[]; + getInputData(inputIndex?: number, connectionType?: NodeConnectionType): INodeExecutionData[]; getNodeInputs(): INodeInputConfiguration[]; getNodeOutputs(): INodeOutputConfiguration[]; putExecutionToWait(waitTill: Date): Promise; @@ -1013,7 +1013,7 @@ export type IExecuteFunctions = ExecuteFunctions.GetNodeParameterFn & }; export interface IExecuteSingleFunctions extends BaseExecutionFunctions { - getInputData(inputIndex?: number, inputName?: string): INodeExecutionData; + getInputData(inputIndex?: number, connectionType?: NodeConnectionType): INodeExecutionData; getItemIndex(): number; getNodeParameter( parameterName: string, @@ -1127,7 +1127,7 @@ export interface IWebhookFunctions extends FunctionsBaseWithRequiredKeys<'getMod getBodyData(): IDataObject; getHeaderData(): IncomingHttpHeaders; getInputConnectionData( - inputName: NodeConnectionType, + connectionType: NodeConnectionType, itemIndex: number, inputIndex?: number, ): Promise; @@ -2372,9 +2372,6 @@ export interface IWorkflowExecuteAdditionalData { mode: WorkflowExecuteMode, envProviderState: EnvProviderState, executeData?: IExecuteData, - defaultReturnRunIndex?: number, - selfData?: IDataObject, - contextNodeName?: string, ): Promise>; } diff --git a/packages/workflow/src/Workflow.ts b/packages/workflow/src/Workflow.ts index b2eb597e2ed27..19f0e14e7ff3f 100644 --- a/packages/workflow/src/Workflow.ts +++ b/packages/workflow/src/Workflow.ts @@ -1357,8 +1357,8 @@ export class Workflow { if (node.executeOnce === true) { // If node should be executed only once so use only the first input item const newInputData: ITaskDataConnections = {}; - for (const inputName of Object.keys(inputData)) { - newInputData[inputName] = inputData[inputName].map((input) => { + for (const connectionType of Object.keys(inputData)) { + newInputData[connectionType] = inputData[connectionType].map((input) => { // eslint-disable-next-line @typescript-eslint/prefer-optional-chain return input && input.slice(0, 1); }); From f4c252341985fe03927a2fd5d60ba846ec3dfc77 Mon Sep 17 00:00:00 2001 From: Michael Kret <88898367+michael-radency@users.noreply.github.com> Date: Wed, 11 Dec 2024 13:52:40 +0200 Subject: [PATCH 09/10] fix(n8n Form Node): Completion page display if EXECUTIONS_DATA_SAVE_ON_SUCCESS=none (#11869) --- .../form-trigger-completion.handlebars | 3 +++ .../__tests__/node-execution-context.test.ts | 4 ++-- .../node-execution-context.ts | 2 ++ packages/nodes-base/nodes/Form/Form.node.ts | 13 ++++++++++-- .../nodes/Form/test/Form.node.test.ts | 21 ++++++++++++++++--- .../Form/test/FormTriggerV2.node.test.ts | 1 + packages/workflow/src/Interfaces.ts | 1 + 7 files changed, 38 insertions(+), 7 deletions(-) diff --git a/packages/cli/templates/form-trigger-completion.handlebars b/packages/cli/templates/form-trigger-completion.handlebars index 49520fd14c1f5..a15855d371752 100644 --- a/packages/cli/templates/form-trigger-completion.handlebars +++ b/packages/cli/templates/form-trigger-completion.handlebars @@ -69,6 +69,9 @@ {{/if}} + diff --git a/packages/core/src/node-execution-context/__tests__/node-execution-context.test.ts b/packages/core/src/node-execution-context/__tests__/node-execution-context.test.ts index 95ed62337e559..02318739849c8 100644 --- a/packages/core/src/node-execution-context/__tests__/node-execution-context.test.ts +++ b/packages/core/src/node-execution-context/__tests__/node-execution-context.test.ts @@ -79,7 +79,7 @@ describe('NodeExecutionContext', () => { const result = testContext.getChildNodes('Test Node'); - expect(result).toEqual([ + expect(result).toMatchObject([ { name: 'Child Node 1', type: 'testType1', typeVersion: 1 }, { name: 'Child Node 2', type: 'testType2', typeVersion: 2 }, ]); @@ -98,7 +98,7 @@ describe('NodeExecutionContext', () => { const result = testContext.getParentNodes('Test Node'); - expect(result).toEqual([ + expect(result).toMatchObject([ { name: 'Parent Node 1', type: 'testType1', typeVersion: 1 }, { name: 'Parent Node 2', type: 'testType2', typeVersion: 2 }, ]); diff --git a/packages/core/src/node-execution-context/node-execution-context.ts b/packages/core/src/node-execution-context/node-execution-context.ts index c4bb0739fbc4a..158b06d02ea76 100644 --- a/packages/core/src/node-execution-context/node-execution-context.ts +++ b/packages/core/src/node-execution-context/node-execution-context.ts @@ -86,6 +86,7 @@ export abstract class NodeExecutionContext implements Omit node.type === FORM_NODE_TYPE || node.type === WAIT_NODE_TYPE, + (node) => !node.disabled && (node.type === FORM_NODE_TYPE || node.type === WAIT_NODE_TYPE), ); if (hasNextPage) { @@ -426,6 +432,9 @@ export class Form extends Node { }; staticData[id] = config; + + const waitTill = new Date(WAIT_INDEFINITELY); + await context.putExecutionToWait(waitTill); } return [context.getInputData()]; diff --git a/packages/nodes-base/nodes/Form/test/Form.node.test.ts b/packages/nodes-base/nodes/Form/test/Form.node.test.ts index ecd24d8ca54b3..8b1a24abcca09 100644 --- a/packages/nodes-base/nodes/Form/test/Form.node.test.ts +++ b/packages/nodes-base/nodes/Form/test/Form.node.test.ts @@ -94,7 +94,12 @@ describe('Form Node', () => { ); mockWebhookFunctions.getRequestObject.mockReturnValue({ method: 'GET' } as Request); mockWebhookFunctions.getParentNodes.mockReturnValue([ - { type: 'n8n-nodes-base.formTrigger', name: 'Form Trigger', typeVersion: 2.1 }, + { + type: 'n8n-nodes-base.formTrigger', + name: 'Form Trigger', + typeVersion: 2.1, + disabled: false, + }, ]); mockWebhookFunctions.evaluateExpression.mockReturnValue('test'); mockWebhookFunctions.getNode.mockReturnValue(mock()); @@ -122,7 +127,12 @@ describe('Form Node', () => { it('should return form data for POST request', async () => { mockWebhookFunctions.getRequestObject.mockReturnValue({ method: 'POST' } as Request); mockWebhookFunctions.getParentNodes.mockReturnValue([ - { type: 'n8n-nodes-base.formTrigger', name: 'Form Trigger', typeVersion: 2.1 }, + { + type: 'n8n-nodes-base.formTrigger', + name: 'Form Trigger', + typeVersion: 2.1, + disabled: false, + }, ]); mockWebhookFunctions.evaluateExpression.mockReturnValue('test'); mockWebhookFunctions.getNode.mockReturnValue(mock()); @@ -174,7 +184,12 @@ describe('Form Node', () => { return {}; }); mockWebhookFunctions.getParentNodes.mockReturnValue([ - { type: 'n8n-nodes-base.formTrigger', name: 'Form Trigger', typeVersion: 2.1 }, + { + type: 'n8n-nodes-base.formTrigger', + name: 'Form Trigger', + typeVersion: 2.1, + disabled: false, + }, ]); mockWebhookFunctions.evaluateExpression.mockReturnValue('test'); diff --git a/packages/nodes-base/nodes/Form/test/FormTriggerV2.node.test.ts b/packages/nodes-base/nodes/Form/test/FormTriggerV2.node.test.ts index d3c96783c5ae2..63dbfd4986a4e 100644 --- a/packages/nodes-base/nodes/Form/test/FormTriggerV2.node.test.ts +++ b/packages/nodes-base/nodes/Form/test/FormTriggerV2.node.test.ts @@ -201,6 +201,7 @@ describe('FormTrigger', () => { name: 'Test Respond To Webhook', type: 'n8n-nodes-base.respondToWebhook', typeVersion: 1, + disabled: false, }, ], }), diff --git a/packages/workflow/src/Interfaces.ts b/packages/workflow/src/Interfaces.ts index f88db21ae71dc..77325caf0363d 100644 --- a/packages/workflow/src/Interfaces.ts +++ b/packages/workflow/src/Interfaces.ts @@ -903,6 +903,7 @@ export type NodeTypeAndVersion = { name: string; type: string; typeVersion: number; + disabled: boolean; }; export interface FunctionsBase { From c572c0648ca5b644b222157b3cabac9c05704a84 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E0=A4=95=E0=A4=BE=E0=A4=B0=E0=A4=A4=E0=A5=8B=E0=A4=AB?= =?UTF-8?q?=E0=A5=8D=E0=A4=AB=E0=A5=87=E0=A4=B2=E0=A4=B8=E0=A5=8D=E0=A4=95?= =?UTF-8?q?=E0=A5=8D=E0=A4=B0=E0=A4=BF=E0=A4=AA=E0=A5=8D=E0=A4=9F=E2=84=A2?= Date: Wed, 11 Dec 2024 13:47:13 +0100 Subject: [PATCH 10/10] fix(core): Fix support for multiple invocation of AI tools (#12141) Co-authored-by: Oleg Ivaniv --- packages/core/src/CreateNodeAsTool.ts | 73 +++--- packages/core/src/NodeExecuteFunctions.ts | 141 +++++------ .../__tests__/supply-data-context.test.ts | 1 + .../base-execute-context.ts | 9 +- .../supply-data-context.ts | 3 +- packages/core/test/CreateNodeAsTool.test.ts | 183 +++++++-------- packages/workflow/src/WorkflowDataProxy.ts | 19 +- .../workflow/test/WorkflowDataProxy.test.ts | 67 +++++- .../from_ai_multiple_items_run.json | 221 ++++++++++++++++++ .../from_ai_multiple_items_workflow.json | 112 +++++++++ 10 files changed, 604 insertions(+), 225 deletions(-) create mode 100644 packages/workflow/test/fixtures/WorkflowDataProxy/from_ai_multiple_items_run.json create mode 100644 packages/workflow/test/fixtures/WorkflowDataProxy/from_ai_multiple_items_workflow.json diff --git a/packages/core/src/CreateNodeAsTool.ts b/packages/core/src/CreateNodeAsTool.ts index c569f943ce272..8e0656ce69f18 100644 --- a/packages/core/src/CreateNodeAsTool.ts +++ b/packages/core/src/CreateNodeAsTool.ts @@ -1,9 +1,11 @@ import { DynamicStructuredTool } from '@langchain/core/tools'; import type { IExecuteFunctions, + INode, INodeParameters, INodeType, ISupplyDataFunctions, + ITaskDataConnections, } from 'n8n-workflow'; import { jsonParse, NodeConnectionType, NodeOperationError } from 'n8n-workflow'; import { z } from 'zod'; @@ -16,6 +18,12 @@ interface FromAIArgument { defaultValue?: string | number | boolean | Record; } +type ParserOptions = { + node: INode; + nodeType: INodeType; + contextFactory: (runIndex: number, inputData: ITaskDataConnections) => ISupplyDataFunctions; +}; + /** * AIParametersParser * @@ -23,15 +31,12 @@ interface FromAIArgument { * generating Zod schemas, and creating LangChain tools. */ class AIParametersParser { - private ctx: ISupplyDataFunctions; + private runIndex = 0; /** * Constructs an instance of AIParametersParser. - * @param ctx The execution context. */ - constructor(ctx: ISupplyDataFunctions) { - this.ctx = ctx; - } + constructor(private readonly options: ParserOptions) {} /** * Generates a Zod schema based on the provided FromAIArgument placeholder. @@ -162,14 +167,14 @@ class AIParametersParser { } catch (error) { // If parsing fails, throw an ApplicationError with details throw new NodeOperationError( - this.ctx.getNode(), + this.options.node, `Failed to parse $fromAI arguments: ${argsString}: ${error}`, ); } } else { // Log an error if parentheses are unbalanced throw new NodeOperationError( - this.ctx.getNode(), + this.options.node, `Unbalanced parentheses while parsing $fromAI call: ${str.slice(startIndex)}`, ); } @@ -254,7 +259,7 @@ class AIParametersParser { const type = cleanArgs?.[2] || 'string'; if (!['string', 'number', 'boolean', 'json'].includes(type.toLowerCase())) { - throw new NodeOperationError(this.ctx.getNode(), `Invalid type: ${type}`); + throw new NodeOperationError(this.options.node, `Invalid type: ${type}`); } return { @@ -315,13 +320,12 @@ class AIParametersParser { /** * Creates a DynamicStructuredTool from a node. - * @param node The node type. - * @param nodeParameters The parameters of the node. * @returns A DynamicStructuredTool instance. */ - public createTool(node: INodeType, nodeParameters: INodeParameters): DynamicStructuredTool { + public createTool(): DynamicStructuredTool { + const { node, nodeType } = this.options; const collectedArguments: FromAIArgument[] = []; - this.traverseNodeParameters(nodeParameters, collectedArguments); + this.traverseNodeParameters(node.parameters, collectedArguments); // Validate each collected argument const nameValidationRegex = /^[a-zA-Z0-9_-]{1,64}$/; @@ -331,7 +335,7 @@ class AIParametersParser { const isEmptyError = 'You must specify a key when using $fromAI()'; const isInvalidError = `Parameter key \`${argument.key}\` is invalid`; const error = new Error(argument.key.length === 0 ? isEmptyError : isInvalidError); - throw new NodeOperationError(this.ctx.getNode(), error, { + throw new NodeOperationError(node, error, { description: 'Invalid parameter key, must be between 1 and 64 characters long and only contain letters, numbers, underscores, and hyphens', }); @@ -348,7 +352,7 @@ class AIParametersParser { ) { // If not, throw an error for inconsistent duplicate keys throw new NodeOperationError( - this.ctx.getNode(), + node, `Duplicate key '${argument.key}' found with different description or type`, { description: @@ -378,37 +382,38 @@ class AIParametersParser { }, {}); const schema = z.object(schemaObj).required(); - const description = this.getDescription(node, nodeParameters); - const nodeName = this.ctx.getNode().name.replace(/ /g, '_'); - const name = nodeName || node.description.name; + const description = this.getDescription(nodeType, node.parameters); + const nodeName = node.name.replace(/ /g, '_'); + const name = nodeName || nodeType.description.name; const tool = new DynamicStructuredTool({ name, description, schema, - func: async (functionArgs: z.infer) => { - const { index } = this.ctx.addInputData(NodeConnectionType.AiTool, [ - [{ json: functionArgs }], - ]); + func: async (toolArgs: z.infer) => { + const context = this.options.contextFactory(this.runIndex, {}); + context.addInputData(NodeConnectionType.AiTool, [[{ json: toolArgs }]]); try { // Execute the node with the proxied context - const result = await node.execute?.bind(this.ctx as IExecuteFunctions)(); + const result = await nodeType.execute?.call(context as IExecuteFunctions); // Process and map the results const mappedResults = result?.[0]?.flatMap((item) => item.json); // Add output data to the context - this.ctx.addOutputData(NodeConnectionType.AiTool, index, [ + context.addOutputData(NodeConnectionType.AiTool, this.runIndex, [ [{ json: { response: mappedResults } }], ]); // Return the stringified results return JSON.stringify(mappedResults); } catch (error) { - const nodeError = new NodeOperationError(this.ctx.getNode(), error as Error); - this.ctx.addOutputData(NodeConnectionType.AiTool, index, nodeError); + const nodeError = new NodeOperationError(this.options.node, error as Error); + context.addOutputData(NodeConnectionType.AiTool, this.runIndex, nodeError); return 'Error during node execution: ' + nodeError.description; + } finally { + this.runIndex++; } }, }); @@ -421,20 +426,8 @@ class AIParametersParser { * Converts node into LangChain tool by analyzing node parameters, * identifying placeholders using the $fromAI function, and generating a Zod schema. It then creates * a DynamicStructuredTool that can be used in LangChain workflows. - * - * @param ctx The execution context. - * @param node The node type. - * @param nodeParameters The parameters of the node. - * @returns An object containing the DynamicStructuredTool instance. */ -export function createNodeAsTool( - ctx: ISupplyDataFunctions, - node: INodeType, - nodeParameters: INodeParameters, -) { - const parser = new AIParametersParser(ctx); - - return { - response: parser.createTool(node, nodeParameters), - }; +export function createNodeAsTool(options: ParserOptions) { + const parser = new AIParametersParser(options); + return { response: parser.createTool() }; } diff --git a/packages/core/src/NodeExecuteFunctions.ts b/packages/core/src/NodeExecuteFunctions.ts index a07674eecdcae..4967617c9be8a 100644 --- a/packages/core/src/NodeExecuteFunctions.ts +++ b/packages/core/src/NodeExecuteFunctions.ts @@ -77,9 +77,9 @@ import type { DeduplicationScope, DeduplicationItemTypes, ICheckProcessedContextData, - ISupplyDataFunctions, WebhookType, SchedulingFunctions, + SupplyData, } from 'n8n-workflow'; import { NodeConnectionType, @@ -2023,9 +2023,9 @@ export async function getInputConnectionData( this: IAllExecuteFunctions, workflow: Workflow, runExecutionData: IRunExecutionData, - runIndex: number, + parentRunIndex: number, connectionInputData: INodeExecutionData[], - inputData: ITaskDataConnections, + parentInputData: ITaskDataConnections, additionalData: IWorkflowExecuteAdditionalData, executeData: IExecuteData, mode: WorkflowExecuteMode, @@ -2034,10 +2034,13 @@ export async function getInputConnectionData( itemIndex: number, abortSignal?: AbortSignal, ): Promise { - const node = this.getNode(); - const nodeType = workflow.nodeTypes.getByNameAndVersion(node.type, node.typeVersion); + const parentNode = this.getNode(); + const parentNodeType = workflow.nodeTypes.getByNameAndVersion( + parentNode.type, + parentNode.typeVersion, + ); - const inputs = NodeHelpers.getNodeInputs(workflow, node, nodeType.description); + const inputs = NodeHelpers.getNodeInputs(workflow, parentNode, parentNodeType.description); let inputConfiguration = inputs.find((input) => { if (typeof input === 'string') { @@ -2048,7 +2051,7 @@ export async function getInputConnectionData( if (inputConfiguration === undefined) { throw new ApplicationError('Node does not have input of type', { - extra: { nodeName: node.name, connectionType }, + extra: { nodeName: parentNode.name, connectionType }, }); } @@ -2059,14 +2062,14 @@ export async function getInputConnectionData( } const connectedNodes = workflow - .getParentNodes(node.name, connectionType, 1) + .getParentNodes(parentNode.name, connectionType, 1) .map((nodeName) => workflow.getNode(nodeName) as INode) .filter((connectedNode) => connectedNode.disabled !== true); if (connectedNodes.length === 0) { if (inputConfiguration.required) { throw new NodeOperationError( - node, + parentNode, `A ${inputConfiguration?.displayName ?? connectionType} sub-node must be connected and enabled`, ); } @@ -2078,82 +2081,86 @@ export async function getInputConnectionData( connectedNodes.length > inputConfiguration.maxConnections ) { throw new NodeOperationError( - node, + parentNode, `Only ${inputConfiguration.maxConnections} ${connectionType} sub-nodes are/is allowed to be connected`, ); } - const constParentNodes = connectedNodes.map(async (connectedNode) => { - const nodeType = workflow.nodeTypes.getByNameAndVersion( + const nodes: SupplyData[] = []; + for (const connectedNode of connectedNodes) { + const connectedNodeType = workflow.nodeTypes.getByNameAndVersion( connectedNode.type, connectedNode.typeVersion, ); - const context = new SupplyDataContext( - workflow, - connectedNode, - additionalData, - mode, - runExecutionData, - runIndex, - connectionInputData, - inputData, - executeData, - closeFunctions, - abortSignal, - ); + const contextFactory = (runIndex: number, inputData: ITaskDataConnections) => + new SupplyDataContext( + workflow, + connectedNode, + additionalData, + mode, + runExecutionData, + runIndex, + connectionInputData, + inputData, + connectionType, + executeData, + closeFunctions, + abortSignal, + ); - if (!nodeType.supplyData) { - if (nodeType.description.outputs.includes(NodeConnectionType.AiTool)) { - nodeType.supplyData = async function (this: ISupplyDataFunctions) { - return createNodeAsTool(this, nodeType, this.getNode().parameters); - }; + if (!connectedNodeType.supplyData) { + if (connectedNodeType.description.outputs.includes(NodeConnectionType.AiTool)) { + const supplyData = createNodeAsTool({ + node: connectedNode, + nodeType: connectedNodeType, + contextFactory, + }); + nodes.push(supplyData); } else { throw new ApplicationError('Node does not have a `supplyData` method defined', { extra: { nodeName: connectedNode.name }, }); } - } + } else { + const context = contextFactory(parentRunIndex, parentInputData); + try { + const supplyData = await connectedNodeType.supplyData.call(context, itemIndex); + if (supplyData.closeFunction) { + closeFunctions.push(supplyData.closeFunction); + } + nodes.push(supplyData); + } catch (error) { + // Propagate errors from sub-nodes + if (error.functionality === 'configuration-node') throw error; + if (!(error instanceof ExecutionBaseError)) { + error = new NodeOperationError(connectedNode, error, { + itemIndex, + }); + } - try { - const response = await nodeType.supplyData.call(context, itemIndex); - if (response.closeFunction) { - closeFunctions.push(response.closeFunction); - } - return response; - } catch (error) { - // Propagate errors from sub-nodes - if (error.functionality === 'configuration-node') throw error; - if (!(error instanceof ExecutionBaseError)) { - error = new NodeOperationError(connectedNode, error, { + let currentNodeRunIndex = 0; + if (runExecutionData.resultData.runData.hasOwnProperty(parentNode.name)) { + currentNodeRunIndex = runExecutionData.resultData.runData[parentNode.name].length; + } + + // Display the error on the node which is causing it + await context.addExecutionDataFunctions( + 'input', + error, + connectionType, + parentNode.name, + currentNodeRunIndex, + ); + + // Display on the calling node which node has the error + throw new NodeOperationError(connectedNode, `Error in sub-node ${connectedNode.name}`, { itemIndex, + functionality: 'configuration-node', + description: error.message, }); } - - let currentNodeRunIndex = 0; - if (runExecutionData.resultData.runData.hasOwnProperty(node.name)) { - currentNodeRunIndex = runExecutionData.resultData.runData[node.name].length; - } - - // Display the error on the node which is causing it - await context.addExecutionDataFunctions( - 'input', - error, - connectionType, - node.name, - currentNodeRunIndex, - ); - - // Display on the calling node which node has the error - throw new NodeOperationError(connectedNode, `Error in sub-node ${connectedNode.name}`, { - itemIndex, - functionality: 'configuration-node', - description: error.message, - }); } - }); - - // Validate the inputs - const nodes = await Promise.all(constParentNodes); + } return inputConfiguration.maxConnections === 1 ? (nodes || [])[0]?.response diff --git a/packages/core/src/node-execution-context/__tests__/supply-data-context.test.ts b/packages/core/src/node-execution-context/__tests__/supply-data-context.test.ts index d3ebddc75cc96..99ee41c6fda3a 100644 --- a/packages/core/src/node-execution-context/__tests__/supply-data-context.test.ts +++ b/packages/core/src/node-execution-context/__tests__/supply-data-context.test.ts @@ -72,6 +72,7 @@ describe('SupplyDataContext', () => { runIndex, connectionInputData, inputData, + connectionType, executeData, [closeFn], abortSignal, diff --git a/packages/core/src/node-execution-context/base-execute-context.ts b/packages/core/src/node-execution-context/base-execute-context.ts index 4f186e559708c..8ecc658579400 100644 --- a/packages/core/src/node-execution-context/base-execute-context.ts +++ b/packages/core/src/node-execution-context/base-execute-context.ts @@ -21,9 +21,14 @@ import type { IWorkflowDataProxyData, ISourceData, AiEvent, +} from 'n8n-workflow'; +import { + ApplicationError, + NodeHelpers, NodeConnectionType, + WAIT_INDEFINITELY, + WorkflowDataProxy, } from 'n8n-workflow'; -import { ApplicationError, NodeHelpers, WAIT_INDEFINITELY, WorkflowDataProxy } from 'n8n-workflow'; import { Container } from 'typedi'; import { BinaryDataService } from '@/BinaryData/BinaryData.service'; @@ -176,7 +181,7 @@ export class BaseExecuteContext extends NodeExecutionContext { ); } - getInputSourceData(inputIndex = 0, connectionType = 'main'): ISourceData { + getInputSourceData(inputIndex = 0, connectionType = NodeConnectionType.Main): ISourceData { if (this.executeData?.source === null) { // Should never happen as n8n sets it automatically throw new ApplicationError('Source data is missing'); diff --git a/packages/core/src/node-execution-context/supply-data-context.ts b/packages/core/src/node-execution-context/supply-data-context.ts index 0155b9d85e857..0ca059d048fa8 100644 --- a/packages/core/src/node-execution-context/supply-data-context.ts +++ b/packages/core/src/node-execution-context/supply-data-context.ts @@ -49,6 +49,7 @@ export class SupplyDataContext extends BaseExecuteContext implements ISupplyData runIndex: number, connectionInputData: INodeExecutionData[], inputData: ITaskDataConnections, + private readonly connectionType: NodeConnectionType, executeData: IExecuteData, private readonly closeFunctions: CloseFunction[], abortSignal?: AbortSignal, @@ -126,7 +127,7 @@ export class SupplyDataContext extends BaseExecuteContext implements ISupplyData ); } - getInputData(inputIndex = 0, connectionType = NodeConnectionType.Main) { + getInputData(inputIndex = 0, connectionType = this.connectionType) { if (!this.inputData.hasOwnProperty(connectionType)) { // Return empty array because else it would throw error when nothing is connected to input return []; diff --git a/packages/core/test/CreateNodeAsTool.test.ts b/packages/core/test/CreateNodeAsTool.test.ts index 5c485b983718f..2f6dd313229e6 100644 --- a/packages/core/test/CreateNodeAsTool.test.ts +++ b/packages/core/test/CreateNodeAsTool.test.ts @@ -1,4 +1,5 @@ -import type { IExecuteFunctions, INodeParameters, INodeType } from 'n8n-workflow'; +import { mock } from 'jest-mock-extended'; +import type { INodeType, ISupplyDataFunctions, INode } from 'n8n-workflow'; import { NodeConnectionType, NodeOperationError } from 'n8n-workflow'; import { z } from 'zod'; @@ -14,28 +15,29 @@ jest.mock('@langchain/core/tools', () => ({ })); describe('createNodeAsTool', () => { - let mockCtx: IExecuteFunctions; - let mockNode: INodeType; - let mockNodeParameters: INodeParameters; + const context = mock({ + getNodeParameter: jest.fn(), + addInputData: jest.fn(), + addOutputData: jest.fn(), + getNode: jest.fn(), + }); + const contextFactory = () => context; + const nodeType = mock({ + description: { + name: 'TestNode', + description: 'Test node description', + }, + }); + const node = mock({ name: 'Test_Node' }); + const options = { node, nodeType, contextFactory }; beforeEach(() => { - // Setup mock objects - mockCtx = { - getNodeParameter: jest.fn(), - addInputData: jest.fn().mockReturnValue({ index: 0 }), - addOutputData: jest.fn(), - getNode: jest.fn().mockReturnValue({ name: 'Test_Node' }), - } as unknown as IExecuteFunctions; - - mockNode = { - description: { - name: 'TestNode', - description: 'Test node description', - }, - execute: jest.fn().mockResolvedValue([[{ json: { result: 'test' } }]]), - } as unknown as INodeType; + jest.clearAllMocks(); + (context.addInputData as jest.Mock).mockReturnValue({ index: 0 }); + (context.getNode as jest.Mock).mockReturnValue(node); + (nodeType.execute as jest.Mock).mockResolvedValue([[{ json: { result: 'test' } }]]); - mockNodeParameters = { + node.parameters = { param1: "={{$fromAI('param1', 'Test parameter', 'string') }}", param2: 'static value', nestedParam: { @@ -45,13 +47,11 @@ describe('createNodeAsTool', () => { resource: 'testResource', operation: 'testOperation', }; - - jest.clearAllMocks(); }); describe('Tool Creation and Basic Properties', () => { it('should create a DynamicStructuredTool with correct properties', () => { - const tool = createNodeAsTool(mockCtx, mockNode, mockNodeParameters).response; + const tool = createNodeAsTool(options).response; expect(tool).toBeDefined(); expect(tool.name).toBe('Test_Node'); @@ -62,10 +62,10 @@ describe('createNodeAsTool', () => { }); it('should use toolDescription if provided', () => { - mockNodeParameters.descriptionType = 'manual'; - mockNodeParameters.toolDescription = 'Custom tool description'; + node.parameters.descriptionType = 'manual'; + node.parameters.toolDescription = 'Custom tool description'; - const tool = createNodeAsTool(mockCtx, mockNode, mockNodeParameters).response; + const tool = createNodeAsTool(options).response; expect(tool.description).toBe('Custom tool description'); }); @@ -73,7 +73,7 @@ describe('createNodeAsTool', () => { describe('Schema Creation and Parameter Handling', () => { it('should create a schema based on fromAI arguments in nodeParameters', () => { - const tool = createNodeAsTool(mockCtx, mockNode, mockNodeParameters).response; + const tool = createNodeAsTool(options).response; expect(tool.schema).toBeDefined(); expect(tool.schema.shape).toHaveProperty('param1'); @@ -82,14 +82,14 @@ describe('createNodeAsTool', () => { }); it('should handle fromAI arguments correctly', () => { - const tool = createNodeAsTool(mockCtx, mockNode, mockNodeParameters).response; + const tool = createNodeAsTool(options).response; expect(tool.schema.shape.param1).toBeInstanceOf(z.ZodString); expect(tool.schema.shape.subparam).toBeInstanceOf(z.ZodString); }); it('should handle default values correctly', () => { - mockNodeParameters = { + node.parameters = { paramWithDefault: "={{ $fromAI('paramWithDefault', 'Parameter with default', 'string', 'default value') }}", numberWithDefault: @@ -98,7 +98,7 @@ describe('createNodeAsTool', () => { "={{ $fromAI('booleanWithDefault', 'Boolean with default', 'boolean', true) }}", }; - const tool = createNodeAsTool(mockCtx, mockNode, mockNodeParameters).response; + const tool = createNodeAsTool(options).response; expect(tool.schema.shape.paramWithDefault.description).toBe('Parameter with default'); expect(tool.schema.shape.numberWithDefault.description).toBe('Number with default'); @@ -106,7 +106,7 @@ describe('createNodeAsTool', () => { }); it('should handle nested parameters correctly', () => { - mockNodeParameters = { + node.parameters = { topLevel: "={{ $fromAI('topLevel', 'Top level parameter', 'string') }}", nested: { level1: "={{ $fromAI('level1', 'Nested level 1', 'string') }}", @@ -116,7 +116,7 @@ describe('createNodeAsTool', () => { }, }; - const tool = createNodeAsTool(mockCtx, mockNode, mockNodeParameters).response; + const tool = createNodeAsTool(options).response; expect(tool.schema.shape.topLevel).toBeInstanceOf(z.ZodString); expect(tool.schema.shape.level1).toBeInstanceOf(z.ZodString); @@ -124,14 +124,14 @@ describe('createNodeAsTool', () => { }); it('should handle array parameters correctly', () => { - mockNodeParameters = { + node.parameters = { arrayParam: [ "={{ $fromAI('item1', 'First item', 'string') }}", "={{ $fromAI('item2', 'Second item', 'number') }}", ], }; - const tool = createNodeAsTool(mockCtx, mockNode, mockNodeParameters).response; + const tool = createNodeAsTool(options).response; expect(tool.schema.shape.item1).toBeInstanceOf(z.ZodString); expect(tool.schema.shape.item2).toBeInstanceOf(z.ZodNumber); @@ -140,13 +140,13 @@ describe('createNodeAsTool', () => { describe('Error Handling and Edge Cases', () => { it('should handle error during node execution', async () => { - mockNode.execute = jest.fn().mockRejectedValue(new Error('Execution failed')); - const tool = createNodeAsTool(mockCtx, mockNode, mockNodeParameters).response; + nodeType.execute = jest.fn().mockRejectedValue(new Error('Execution failed')); + const tool = createNodeAsTool(options).response; const result = await tool.func({ param1: 'test value' }); expect(result).toContain('Error during node execution:'); - expect(mockCtx.addOutputData).toHaveBeenCalledWith( + expect(context.addOutputData).toHaveBeenCalledWith( NodeConnectionType.AiTool, 0, expect.any(NodeOperationError), @@ -154,31 +154,27 @@ describe('createNodeAsTool', () => { }); it('should throw an error for invalid parameter names', () => { - mockNodeParameters.invalidParam = "$fromAI('invalid param', 'Invalid parameter', 'string')"; + node.parameters.invalidParam = "$fromAI('invalid param', 'Invalid parameter', 'string')"; - expect(() => createNodeAsTool(mockCtx, mockNode, mockNodeParameters)).toThrow( - 'Parameter key `invalid param` is invalid', - ); + expect(() => createNodeAsTool(options)).toThrow('Parameter key `invalid param` is invalid'); }); it('should throw an error for $fromAI calls with unsupported types', () => { - mockNodeParameters = { + node.parameters = { invalidTypeParam: "={{ $fromAI('invalidType', 'Param with unsupported type', 'unsupportedType') }}", }; - expect(() => createNodeAsTool(mockCtx, mockNode, mockNodeParameters)).toThrow( - 'Invalid type: unsupportedType', - ); + expect(() => createNodeAsTool(options)).toThrow('Invalid type: unsupportedType'); }); it('should handle empty parameters and parameters with no fromAI calls', () => { - mockNodeParameters = { + node.parameters = { param1: 'static value 1', param2: 'static value 2', }; - const tool = createNodeAsTool(mockCtx, mockNode, mockNodeParameters).response; + const tool = createNodeAsTool(options).response; expect(tool.schema.shape).toEqual({}); }); @@ -186,13 +182,13 @@ describe('createNodeAsTool', () => { describe('Parameter Name and Description Handling', () => { it('should accept parameter names with underscores and hyphens', () => { - mockNodeParameters = { + node.parameters = { validName1: "={{ $fromAI('param_name-1', 'Valid name with underscore and hyphen', 'string') }}", validName2: "={{ $fromAI('param_name_2', 'Another valid name', 'number') }}", }; - const tool = createNodeAsTool(mockCtx, mockNode, mockNodeParameters).response; + const tool = createNodeAsTool(options).response; expect(tool.schema.shape['param_name-1']).toBeInstanceOf(z.ZodString); expect(tool.schema.shape['param_name-1'].description).toBe( @@ -204,22 +200,20 @@ describe('createNodeAsTool', () => { }); it('should throw an error for parameter names with invalid special characters', () => { - mockNodeParameters = { + node.parameters = { invalidNameParam: "={{ $fromAI('param@name!', 'Invalid name with special characters', 'string') }}", }; - expect(() => createNodeAsTool(mockCtx, mockNode, mockNodeParameters)).toThrow( - 'Parameter key `param@name!` is invalid', - ); + expect(() => createNodeAsTool(options)).toThrow('Parameter key `param@name!` is invalid'); }); it('should throw an error for empty parameter name', () => { - mockNodeParameters = { + node.parameters = { invalidNameParam: "={{ $fromAI('', 'Invalid name with special characters', 'string') }}", }; - expect(() => createNodeAsTool(mockCtx, mockNode, mockNodeParameters)).toThrow( + expect(() => createNodeAsTool(options)).toThrow( 'You must specify a key when using $fromAI()', ); }); @@ -227,50 +221,51 @@ describe('createNodeAsTool', () => { it('should handle parameter names with exact and exceeding character limits', () => { const longName = 'a'.repeat(64); const tooLongName = 'a'.repeat(65); - mockNodeParameters = { + node.parameters = { longNameParam: `={{ $fromAI('${longName}', 'Param with 64 character name', 'string') }}`, }; - const tool = createNodeAsTool(mockCtx, mockNode, mockNodeParameters).response; + const tool = createNodeAsTool(options).response; expect(tool.schema.shape[longName]).toBeInstanceOf(z.ZodString); expect(tool.schema.shape[longName].description).toBe('Param with 64 character name'); - expect(() => - createNodeAsTool(mockCtx, mockNode, { - tooLongNameParam: `={{ $fromAI('${tooLongName}', 'Param with 65 character name', 'string') }}`, - }), - ).toThrow(`Parameter key \`${tooLongName}\` is invalid`); + node.parameters = { + tooLongNameParam: `={{ $fromAI('${tooLongName}', 'Param with 65 character name', 'string') }}`, + }; + expect(() => createNodeAsTool(options)).toThrow( + `Parameter key \`${tooLongName}\` is invalid`, + ); }); it('should handle $fromAI calls with empty description', () => { - mockNodeParameters = { + node.parameters = { emptyDescriptionParam: "={{ $fromAI('emptyDescription', '', 'number') }}", }; - const tool = createNodeAsTool(mockCtx, mockNode, mockNodeParameters).response; + const tool = createNodeAsTool(options).response; expect(tool.schema.shape.emptyDescription).toBeInstanceOf(z.ZodNumber); expect(tool.schema.shape.emptyDescription.description).toBeUndefined(); }); it('should throw an error for calls with the same parameter but different descriptions', () => { - mockNodeParameters = { + node.parameters = { duplicateParam1: "={{ $fromAI('duplicate', 'First duplicate', 'string') }}", duplicateParam2: "={{ $fromAI('duplicate', 'Second duplicate', 'number') }}", }; - expect(() => createNodeAsTool(mockCtx, mockNode, mockNodeParameters)).toThrow( + expect(() => createNodeAsTool(options)).toThrow( "Duplicate key 'duplicate' found with different description or type", ); }); it('should throw an error for calls with the same parameter but different types', () => { - mockNodeParameters = { + node.parameters = { duplicateParam1: "={{ $fromAI('duplicate', 'First duplicate', 'string') }}", duplicateParam2: "={{ $fromAI('duplicate', 'First duplicate', 'number') }}", }; - expect(() => createNodeAsTool(mockCtx, mockNode, mockNodeParameters)).toThrow( + expect(() => createNodeAsTool(options)).toThrow( "Duplicate key 'duplicate' found with different description or type", ); }); @@ -278,7 +273,7 @@ describe('createNodeAsTool', () => { describe('Complex Parsing Scenarios', () => { it('should correctly parse $fromAI calls with varying spaces, capitalization, and within template literals', () => { - mockNodeParameters = { + node.parameters = { varyingSpacing1: "={{$fromAI('param1','Description1','string')}}", varyingSpacing2: "={{ $fromAI ( 'param2' , 'Description2' , 'number' ) }}", varyingSpacing3: "={{ $FROMai('param3', 'Description3', 'boolean') }}", @@ -288,7 +283,7 @@ describe('createNodeAsTool', () => { "={{ `Value is: ${$fromAI('templatedParam', 'Templated param description', 'string')}` }}", }; - const tool = createNodeAsTool(mockCtx, mockNode, mockNodeParameters).response; + const tool = createNodeAsTool(options).response; expect(tool.schema.shape.param1).toBeInstanceOf(z.ZodString); expect(tool.schema.shape.param1.description).toBe('Description1'); @@ -307,12 +302,12 @@ describe('createNodeAsTool', () => { }); it('should correctly parse multiple $fromAI calls interleaved with regular text', () => { - mockNodeParameters = { + node.parameters = { interleavedParams: "={{ 'Start ' + $fromAI('param1', 'First param', 'string') + ' Middle ' + $fromAI('param2', 'Second param', 'number') + ' End' }}", }; - const tool = createNodeAsTool(mockCtx, mockNode, mockNodeParameters).response; + const tool = createNodeAsTool(options).response; expect(tool.schema.shape.param1).toBeInstanceOf(z.ZodString); expect(tool.schema.shape.param1.description).toBe('First param'); @@ -322,12 +317,12 @@ describe('createNodeAsTool', () => { }); it('should correctly parse $fromAI calls with complex JSON default values', () => { - mockNodeParameters = { + node.parameters = { complexJsonDefault: '={{ $fromAI(\'complexJson\', \'Param with complex JSON default\', \'json\', \'{"nested": {"key": "value"}, "array": [1, 2, 3]}\') }}', }; - const tool = createNodeAsTool(mockCtx, mockNode, mockNodeParameters).response; + const tool = createNodeAsTool(options).response; expect(tool.schema.shape.complexJson._def.innerType).toBeInstanceOf(z.ZodRecord); expect(tool.schema.shape.complexJson.description).toBe('Param with complex JSON default'); @@ -338,7 +333,7 @@ describe('createNodeAsTool', () => { }); it('should ignore $fromAI calls embedded in non-string node parameters', () => { - mockNodeParameters = { + node.parameters = { numberParam: 42, booleanParam: false, objectParam: { @@ -355,7 +350,7 @@ describe('createNodeAsTool', () => { ], }; - const tool = createNodeAsTool(mockCtx, mockNode, mockNodeParameters).response; + const tool = createNodeAsTool(options).response; expect(tool.schema.shape.innerParam).toBeInstanceOf(z.ZodString); expect(tool.schema.shape.innerParam.description).toBe('Inner param'); @@ -373,48 +368,48 @@ describe('createNodeAsTool', () => { describe('Escaping and Special Characters', () => { it('should handle escaped single quotes in parameter names and descriptions', () => { - mockNodeParameters = { + node.parameters = { escapedQuotesParam: "={{ $fromAI('paramName', 'Description with \\'escaped\\' quotes', 'string') }}", }; - const tool = createNodeAsTool(mockCtx, mockNode, mockNodeParameters).response; + const tool = createNodeAsTool(options).response; expect(tool.schema.shape.paramName).toBeInstanceOf(z.ZodString); expect(tool.schema.shape.paramName.description).toBe("Description with 'escaped' quotes"); }); it('should handle escaped double quotes in parameter names and descriptions', () => { - mockNodeParameters = { + node.parameters = { escapedQuotesParam: '={{ $fromAI("paramName", "Description with \\"escaped\\" quotes", "string") }}', }; - const tool = createNodeAsTool(mockCtx, mockNode, mockNodeParameters).response; + const tool = createNodeAsTool(options).response; expect(tool.schema.shape.paramName).toBeInstanceOf(z.ZodString); expect(tool.schema.shape.paramName.description).toBe('Description with "escaped" quotes'); }); it('should handle escaped backslashes in parameter names and descriptions', () => { - mockNodeParameters = { + node.parameters = { escapedBackslashesParam: "={{ $fromAI('paramName', 'Description with \\\\ backslashes', 'string') }}", }; - const tool = createNodeAsTool(mockCtx, mockNode, mockNodeParameters).response; + const tool = createNodeAsTool(options).response; expect(tool.schema.shape.paramName).toBeInstanceOf(z.ZodString); expect(tool.schema.shape.paramName.description).toBe('Description with \\ backslashes'); }); it('should handle mixed escaped characters in parameter names and descriptions', () => { - mockNodeParameters = { + node.parameters = { mixedEscapesParam: '={{ $fromAI(`paramName`, \'Description with \\\'mixed" characters\', "number") }}', }; - const tool = createNodeAsTool(mockCtx, mockNode, mockNodeParameters).response; + const tool = createNodeAsTool(options).response; expect(tool.schema.shape.paramName).toBeInstanceOf(z.ZodNumber); expect(tool.schema.shape.paramName.description).toBe('Description with \'mixed" characters'); @@ -423,12 +418,12 @@ describe('createNodeAsTool', () => { describe('Edge Cases and Limitations', () => { it('should ignore excess arguments in $fromAI calls beyond the fourth argument', () => { - mockNodeParameters = { + node.parameters = { excessArgsParam: "={{ $fromAI('excessArgs', 'Param with excess arguments', 'string', 'default', 'extraArg1', 'extraArg2') }}", }; - const tool = createNodeAsTool(mockCtx, mockNode, mockNodeParameters).response; + const tool = createNodeAsTool(options).response; expect(tool.schema.shape.excessArgs._def.innerType).toBeInstanceOf(z.ZodString); expect(tool.schema.shape.excessArgs.description).toBe('Param with excess arguments'); @@ -436,12 +431,12 @@ describe('createNodeAsTool', () => { }); it('should correctly parse $fromAI calls with nested parentheses', () => { - mockNodeParameters = { + node.parameters = { nestedParenthesesParam: "={{ $fromAI('paramWithNested', 'Description with ((nested)) parentheses', 'string') }}", }; - const tool = createNodeAsTool(mockCtx, mockNode, mockNodeParameters).response; + const tool = createNodeAsTool(options).response; expect(tool.schema.shape.paramWithNested).toBeInstanceOf(z.ZodString); expect(tool.schema.shape.paramWithNested.description).toBe( @@ -451,24 +446,24 @@ describe('createNodeAsTool', () => { it('should handle $fromAI calls with very long descriptions', () => { const longDescription = 'A'.repeat(1000); - mockNodeParameters = { + node.parameters = { longParam: `={{ $fromAI('longParam', '${longDescription}', 'string') }}`, }; - const tool = createNodeAsTool(mockCtx, mockNode, mockNodeParameters).response; + const tool = createNodeAsTool(options).response; expect(tool.schema.shape.longParam).toBeInstanceOf(z.ZodString); expect(tool.schema.shape.longParam.description).toBe(longDescription); }); it('should handle $fromAI calls with only some parameters', () => { - mockNodeParameters = { + node.parameters = { partialParam1: "={{ $fromAI('partial1') }}", partialParam2: "={{ $fromAI('partial2', 'Description only') }}", partialParam3: "={{ $fromAI('partial3', '', 'number') }}", }; - const tool = createNodeAsTool(mockCtx, mockNode, mockNodeParameters).response; + const tool = createNodeAsTool(options).response; expect(tool.schema.shape.partial1).toBeInstanceOf(z.ZodString); expect(tool.schema.shape.partial2).toBeInstanceOf(z.ZodString); @@ -478,11 +473,11 @@ describe('createNodeAsTool', () => { describe('Unicode and Internationalization', () => { it('should handle $fromAI calls with unicode characters', () => { - mockNodeParameters = { + node.parameters = { unicodeParam: "={{ $fromAI('unicodeParam', '🌈 Unicode parameter 你好', 'string') }}", }; - const tool = createNodeAsTool(mockCtx, mockNode, mockNodeParameters).response; + const tool = createNodeAsTool(options).response; expect(tool.schema.shape.unicodeParam).toBeInstanceOf(z.ZodString); expect(tool.schema.shape.unicodeParam.description).toBe('🌈 Unicode parameter 你好'); diff --git a/packages/workflow/src/WorkflowDataProxy.ts b/packages/workflow/src/WorkflowDataProxy.ts index ea167f50b31fb..d94d9220d0908 100644 --- a/packages/workflow/src/WorkflowDataProxy.ts +++ b/packages/workflow/src/WorkflowDataProxy.ts @@ -945,10 +945,11 @@ export class WorkflowDataProxy { _type: string = 'string', defaultValue?: unknown, ) => { + const { itemIndex, runIndex } = that; if (!name || name === '') { throw new ExpressionError("Add a key, e.g. $fromAI('placeholder_name')", { - runIndex: that.runIndex, - itemIndex: that.itemIndex, + runIndex, + itemIndex, }); } const nameValidationRegex = /^[a-zA-Z0-9_-]{0,64}$/; @@ -956,20 +957,20 @@ export class WorkflowDataProxy { throw new ExpressionError( 'Invalid parameter key, must be between 1 and 64 characters long and only contain lowercase letters, uppercase letters, numbers, underscores, and hyphens', { - runIndex: that.runIndex, - itemIndex: that.itemIndex, + runIndex, + itemIndex, }, ); } + const inputData = + that.runExecutionData?.resultData.runData[that.activeNodeName]?.[runIndex].inputOverride; const placeholdersDataInputData = - that.runExecutionData?.resultData.runData[that.activeNodeName]?.[0].inputOverride?.[ - NodeConnectionType.AiTool - ]?.[0]?.[0].json; + inputData?.[NodeConnectionType.AiTool]?.[0]?.[itemIndex].json; if (Boolean(!placeholdersDataInputData)) { throw new ExpressionError('No execution data available', { - runIndex: that.runIndex, - itemIndex: that.itemIndex, + runIndex, + itemIndex, type: 'no_execution_data', }); } diff --git a/packages/workflow/test/WorkflowDataProxy.test.ts b/packages/workflow/test/WorkflowDataProxy.test.ts index 89b0751321543..8c41b0571cb47 100644 --- a/packages/workflow/test/WorkflowDataProxy.test.ts +++ b/packages/workflow/test/WorkflowDataProxy.test.ts @@ -1,11 +1,12 @@ import { ExpressionError } from '@/errors/expression.error'; -import type { - IExecuteData, - INode, - IPinData, - IRun, - IWorkflowBase, - WorkflowExecuteMode, +import { + NodeConnectionType, + type IExecuteData, + type INode, + type IPinData, + type IRun, + type IWorkflowBase, + type WorkflowExecuteMode, } from '@/Interfaces'; import { Workflow } from '@/Workflow'; import { WorkflowDataProxy } from '@/WorkflowDataProxy'; @@ -26,10 +27,15 @@ const getProxyFromFixture = ( run: IRun | null, activeNode: string, mode?: WorkflowExecuteMode, - opts?: { throwOnMissingExecutionData: boolean }, + opts?: { + throwOnMissingExecutionData: boolean; + connectionType?: NodeConnectionType; + runIndex?: number; + }, ) => { - const taskData = run?.data.resultData.runData[activeNode]?.[0]; - const lastNodeConnectionInputData = taskData?.data?.main[0]; + const taskData = run?.data.resultData.runData[activeNode]?.[opts?.runIndex ?? 0]; + const lastNodeConnectionInputData = + taskData?.data?.[opts?.connectionType ?? NodeConnectionType.Main]?.[0]; let executeData: IExecuteData | undefined; @@ -38,7 +44,7 @@ const getProxyFromFixture = ( data: taskData.data!, node: workflow.nodes.find((node) => node.name === activeNode) as INode, source: { - main: taskData.source, + [opts?.connectionType ?? NodeConnectionType.Main]: taskData.source, }, }; } @@ -64,7 +70,7 @@ const getProxyFromFixture = ( pinData, }), run?.data ?? null, - 0, + opts?.runIndex ?? 0, 0, activeNode, lastNodeConnectionInputData ?? [], @@ -443,4 +449,41 @@ describe('WorkflowDataProxy', () => { }); }); }); + + describe('$fromAI', () => { + const fixture = loadFixture('from_ai_multiple_items'); + const getFromAIProxy = (runIndex = 0) => + getProxyFromFixture(fixture.workflow, fixture.run, 'Google Sheets1', 'manual', { + connectionType: NodeConnectionType.AiTool, + throwOnMissingExecutionData: false, + runIndex, + }); + + test('Retrieves values for first item', () => { + expect(getFromAIProxy().$fromAI('full_name')).toEqual('Mr. Input 1'); + expect(getFromAIProxy().$fromAI('email')).toEqual('input1@n8n.io'); + }); + + test('Retrieves values for second item', () => { + expect(getFromAIProxy(1).$fromAI('full_name')).toEqual('Mr. Input 2'); + expect(getFromAIProxy(1).$fromAI('email')).toEqual('input2@n8n.io'); + }); + + test('Case variants: $fromAi and $fromai', () => { + expect(getFromAIProxy().$fromAi('full_name')).toEqual('Mr. Input 1'); + expect(getFromAIProxy().$fromai('email')).toEqual('input1@n8n.io'); + }); + + test('Returns default value when key not found', () => { + expect( + getFromAIProxy().$fromAI('non_existent_key', 'description', 'string', 'default_value'), + ).toEqual('default_value'); + }); + + test('Throws an error when a key is invalid (e.g. empty string)', () => { + expect(() => getFromAIProxy().$fromAI('')).toThrow(ExpressionError); + expect(() => getFromAIProxy().$fromAI('invalid key')).toThrow(ExpressionError); + expect(() => getFromAIProxy().$fromAI('invalid!')).toThrow(ExpressionError); + }); + }); }); diff --git a/packages/workflow/test/fixtures/WorkflowDataProxy/from_ai_multiple_items_run.json b/packages/workflow/test/fixtures/WorkflowDataProxy/from_ai_multiple_items_run.json new file mode 100644 index 0000000000000..563e52f15a38b --- /dev/null +++ b/packages/workflow/test/fixtures/WorkflowDataProxy/from_ai_multiple_items_run.json @@ -0,0 +1,221 @@ +{ + "data": { + "startData": {}, + "resultData": { + "runData": { + "When clicking ‘Test workflow’": [ + { + "hints": [], + "startTime": 1733478795595, + "executionTime": 0, + "source": [], + "executionStatus": "success", + "data": { + "main": [ + [ + { + "json": {}, + "pairedItem": { + "item": 0 + } + } + ] + ] + } + } + ], + "Code": [ + { + "hints": [ + { + "message": "To make sure expressions after this node work, return the input items that produced each output item. More info", + "location": "outputPane" + } + ], + "startTime": 1733478795595, + "executionTime": 2, + "source": [ + { + "previousNode": "When clicking ‘Test workflow’" + } + ], + "executionStatus": "success", + "data": { + "main": [ + [ + { + "json": { + "full_name": "Mr. Input 1", + "email": "input1@n8n.io" + }, + "pairedItem": { + "item": 0 + } + }, + { + "json": { + "full_name": "Mr. Input 2", + "email": "input2@n8n.io" + }, + "pairedItem": { + "item": 0 + } + } + ] + ] + } + } + ], + "Google Sheets1": [ + { + "startTime": 1733478796468, + "executionTime": 1417, + "executionStatus": "success", + "source": [null], + "data": { + "ai_tool": [ + [ + { + "json": { + "response": [ + { + "full name": "Mr. Input 1", + "email": "input1@n8n.io" + }, + {}, + {} + ] + } + } + ] + ] + }, + "inputOverride": { + "ai_tool": [ + [ + { + "json": { + "full_name": "Mr. Input 1", + "email": "input1@n8n.io" + } + } + ] + ] + }, + "metadata": { + "subRun": [ + { + "node": "Google Sheets1", + "runIndex": 0 + }, + { + "node": "Google Sheets1", + "runIndex": 1 + } + ] + } + }, + { + "startTime": 1733478799915, + "executionTime": 1271, + "executionStatus": "success", + "source": [null], + "data": { + "ai_tool": [ + [ + { + "json": { + "response": [ + { + "full name": "Mr. Input 1", + "email": "input1@n8n.io" + }, + {}, + {} + ] + } + } + ] + ] + }, + "inputOverride": { + "ai_tool": [ + [ + { + "json": { + "full_name": "Mr. Input 2", + "email": "input2@n8n.io" + } + } + ] + ] + } + } + ], + "Agent single list with multiple tool calls": [ + { + "hints": [], + "startTime": 1733478795597, + "executionTime": 9157, + "source": [ + { + "previousNode": "Code" + } + ], + "executionStatus": "success", + "data": { + "main": [ + [ + { + "json": { + "output": "The user \"Mr. Input 1\" with the email \"input1@n8n.io\" has been successfully added to your Users sheet." + }, + "pairedItem": { + "item": 0 + } + }, + { + "json": { + "output": "The user \"Mr. Input 2\" with the email \"input2@n8n.io\" has been successfully added to your Users sheet." + }, + "pairedItem": { + "item": 1 + } + } + ] + ] + } + } + ] + }, + "pinData": {}, + "lastNodeExecuted": "Agent single list with multiple tool calls" + }, + "executionData": { + "contextData": {}, + "nodeExecutionStack": [], + "metadata": { + "Google Sheets1": [ + { + "subRun": [ + { + "node": "Google Sheets1", + "runIndex": 0 + }, + { + "node": "Google Sheets1", + "runIndex": 1 + } + ] + } + ] + }, + "waitingExecution": {}, + "waitingExecutionSource": {} + } + }, + "mode": "manual", + "startedAt": "2024-02-08T15:45:18.848Z", + "stoppedAt": "2024-02-08T15:45:18.862Z", + "status": "running" +} diff --git a/packages/workflow/test/fixtures/WorkflowDataProxy/from_ai_multiple_items_workflow.json b/packages/workflow/test/fixtures/WorkflowDataProxy/from_ai_multiple_items_workflow.json new file mode 100644 index 0000000000000..9f3f75970e1f7 --- /dev/null +++ b/packages/workflow/test/fixtures/WorkflowDataProxy/from_ai_multiple_items_workflow.json @@ -0,0 +1,112 @@ +{ + "id": "8d7lUG8IdEyvIUim", + "name": "Multiple items tool", + "active": false, + "nodes": [ + { + "parameters": { + "mode": "runOnceForAllItems", + "language": "javaScript", + "jsCode": "return [\n { \"full_name\": \"Mr. Input 1\", \"email\": \"input1@n8n.io\" }, \n { \"full_name\": \"Mr. Input 2\", \"email\": \"input2@n8n.io\" }\n]", + "notice": "" + }, + "id": "cb19a188-12ae-4d46-86df-4a2044ec3346", + "name": "Code", + "type": "n8n-nodes-base.code", + "typeVersion": 2, + "position": [-160, 480] + }, + { + "parameters": { "notice": "", "model": "gpt-4o-mini", "options": {} }, + "id": "c448b6b4-9e11-4044-96e5-f4138534ae52", + "name": "OpenAI Chat Model1", + "type": "@n8n/n8n-nodes-langchain.lmChatOpenAi", + "typeVersion": 1, + "position": [40, 700] + }, + { + "parameters": { + "descriptionType": "manual", + "toolDescription": "Add row to Users sheet", + "authentication": "oAuth2", + "resource": "sheet", + "operation": "append", + "columns": { + "mappingMode": "defineBelow", + "value": { + "full name": "={{ $fromAI('full_name') }}", + "email": "={{ $fromAI('email') }}" + }, + "matchingColumns": [], + "schema": [ + { + "id": "full name", + "displayName": "full name", + "required": false, + "defaultMatch": false, + "display": true, + "type": "string", + "canBeUsedToMatch": true + }, + { + "id": "email", + "displayName": "email", + "required": false, + "defaultMatch": false, + "display": true, + "type": "string", + "canBeUsedToMatch": true + } + ] + }, + "options": { "useAppend": true } + }, + "id": "d8b40267-9397-45b6-8a64-ee7e8f9eb8a8", + "name": "Google Sheets1", + "type": "n8n-nodes-base.googleSheetsTool", + "typeVersion": 4.5, + "position": [240, 700] + }, + { + "parameters": { + "notice_tip": "", + "agent": "toolsAgent", + "promptType": "define", + "text": "=Add this user to my Users sheet:\n{{ $json.toJsonString() }}", + "hasOutputParser": false, + "options": {}, + "credentials": "" + }, + "id": "0d6c1bd7-cc91-4571-8fdb-c875a1af44c7", + "name": "Agent single list with multiple tool calls", + "type": "@n8n/n8n-nodes-langchain.agent", + "typeVersion": 1.7, + "position": [40, 480] + } + ], + "connections": { + "When clicking ‘Test workflow’": { "main": [[{ "node": "Code", "type": "main", "index": 0 }]] }, + "Code": { + "main": [ + [{ "node": "Agent single list with multiple tool calls", "type": "main", "index": 0 }] + ] + }, + "OpenAI Chat Model1": { + "ai_languageModel": [ + [ + { + "node": "Agent single list with multiple tool calls", + "type": "ai_languageModel", + "index": 0 + } + ] + ] + }, + "Google Sheets1": { + "ai_tool": [ + [{ "node": "Agent single list with multiple tool calls", "type": "ai_tool", "index": 0 }] + ] + } + }, + "pinData": {} +}