From 6b06890ffe8260ed5c23369dd80339e211aee61e 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, 6 Sep 2023 13:16:39 +0200 Subject: [PATCH 01/23] ci: Fix master build on Nodejs 20 (no-changelog) (#7119) [`shelljs` is broken on Nodejs 20.6](https://github.com/shelljs/shelljs/issues/1133). Until that is resolved, we should fix the version to 20.5 --- .github/workflows/ci-master.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci-master.yml b/.github/workflows/ci-master.yml index a617ca9153b52..9977f9cd06771 100644 --- a/.github/workflows/ci-master.yml +++ b/.github/workflows/ci-master.yml @@ -13,7 +13,7 @@ jobs: strategy: matrix: - node-version: [18.x, 20.x] + node-version: [18.x, 20.5] steps: - uses: actions/checkout@v3.5.3 @@ -44,7 +44,7 @@ jobs: needs: install-and-build strategy: matrix: - node-version: [18.x, 20.x] + node-version: [18.x, 20.5] steps: - uses: actions/checkout@v3.5.3 with: @@ -82,7 +82,7 @@ jobs: needs: install-and-build strategy: matrix: - node-version: [18.x, 20.x] + node-version: [18.x, 20.5] steps: - uses: actions/checkout@v3.5.3 with: From 36a8e911e6f58d0b87816fae0443c6ce8f5ea45a 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, 6 Sep 2023 16:11:39 +0200 Subject: [PATCH 02/23] fix(Code Node): Disable WASM to address CVE-2023-37903 (#7122) [GH Advisory](https://github.com/advisories/GHSA-g644-9gfx-q4q4) --- packages/nodes-base/nodes/Code/JavaScriptSandbox.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/nodes-base/nodes/Code/JavaScriptSandbox.ts b/packages/nodes-base/nodes/Code/JavaScriptSandbox.ts index e15a37b73ae6a..1b1da7f58d83d 100644 --- a/packages/nodes-base/nodes/Code/JavaScriptSandbox.ts +++ b/packages/nodes-base/nodes/Code/JavaScriptSandbox.ts @@ -42,6 +42,7 @@ export class JavaScriptSandbox extends Sandbox { console: 'redirect', sandbox: context, require: vmResolver, + wasm: false, }); this.vm.on('console.log', (...args: unknown[]) => this.emit('output', ...args)); From 92af1314fe60560cdfb52b3307cc74559ba530a8 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, 6 Sep 2023 16:32:50 +0200 Subject: [PATCH 03/23] fix(Postgres Node): Fix automatic column mapping (#7121) NODE-757 --- .../nodes/Postgres/test/v2/operations.test.ts | 16 ++++++++-------- .../v2/actions/database/insert.operation.ts | 2 +- .../v2/actions/database/update.operation.ts | 2 +- .../v2/actions/database/upsert.operation.ts | 2 +- 4 files changed, 11 insertions(+), 11 deletions(-) diff --git a/packages/nodes-base/nodes/Postgres/test/v2/operations.test.ts b/packages/nodes-base/nodes/Postgres/test/v2/operations.test.ts index 6c7976d181067..bebf8af4875d8 100644 --- a/packages/nodes-base/nodes/Postgres/test/v2/operations.test.ts +++ b/packages/nodes-base/nodes/Postgres/test/v2/operations.test.ts @@ -89,7 +89,7 @@ describe('Test PostgresV2, deleteTable operation', () => { }, ], }, - options: { typeVersion: 2.1 }, + options: { nodeVersion: 2.1 }, }; const nodeOptions = nodeParameters.options as IDataObject; @@ -168,7 +168,7 @@ describe('Test PostgresV2, deleteTable operation', () => { cachedResultName: 'my_table', }, deleteCommand: 'drop', - options: { typeVersion: 2.1 }, + options: { nodeVersion: 2.1 }, }; const nodeOptions = nodeParameters.options as IDataObject; @@ -256,7 +256,7 @@ describe('Test PostgresV2, insert operation', () => { }, ], }, - options: { typeVersion: 2.1 }, + options: { nodeVersion: 2.1 }, }; const columnsInfo: ColumnInfo[] = [ { column_name: 'id', data_type: 'integer', is_nullable: 'NO', udt_name: '' }, @@ -299,7 +299,7 @@ describe('Test PostgresV2, insert operation', () => { mode: 'list', }, dataMode: 'autoMapInputData', - options: { typeVersion: 2.1 }, + options: { nodeVersion: 2.1 }, }; const columnsInfo: ColumnInfo[] = [ { column_name: 'id', data_type: 'integer', is_nullable: 'NO', udt_name: '' }, @@ -509,7 +509,7 @@ describe('Test PostgresV2, update operation', () => { }, options: { outputColumns: ['json', 'foo'], - typeVersion: 2.1, + nodeVersion: 2.1, }, }; const columnsInfo: ColumnInfo[] = [ @@ -566,7 +566,7 @@ describe('Test PostgresV2, update operation', () => { }, dataMode: 'autoMapInputData', columnToMatchOn: 'id', - options: { typeVersion: 2.1 }, + options: { nodeVersion: 2.1 }, }; const columnsInfo: ColumnInfo[] = [ { column_name: 'id', data_type: 'integer', is_nullable: 'NO', udt_name: '' }, @@ -669,7 +669,7 @@ describe('Test PostgresV2, upsert operation', () => { }, options: { outputColumns: ['json'], - typeVersion: 2.1, + nodeVersion: 2.1, }, }; const columnsInfo: ColumnInfo[] = [ @@ -726,7 +726,7 @@ describe('Test PostgresV2, upsert operation', () => { }, dataMode: 'autoMapInputData', columnToMatchOn: 'id', - options: { typeVersion: 2.1 }, + options: { nodeVersion: 2.1 }, }; const columnsInfo: ColumnInfo[] = [ { column_name: 'id', data_type: 'integer', is_nullable: 'NO', udt_name: '' }, diff --git a/packages/nodes-base/nodes/Postgres/v2/actions/database/insert.operation.ts b/packages/nodes-base/nodes/Postgres/v2/actions/database/insert.operation.ts index 29ae34b5c5130..fd4e84552f691 100644 --- a/packages/nodes-base/nodes/Postgres/v2/actions/database/insert.operation.ts +++ b/packages/nodes-base/nodes/Postgres/v2/actions/database/insert.operation.ts @@ -162,7 +162,7 @@ export async function execute( db: PgpDatabase, ): Promise { items = replaceEmptyStringsByNulls(items, nodeOptions.replaceEmptyStrings as boolean); - const nodeVersion = nodeOptions.typeVersion as number; + const nodeVersion = nodeOptions.nodeVersion as number; let schema = this.getNodeParameter('schema', 0, undefined, { extractValue: true, diff --git a/packages/nodes-base/nodes/Postgres/v2/actions/database/update.operation.ts b/packages/nodes-base/nodes/Postgres/v2/actions/database/update.operation.ts index 1261e4905c8df..3a26ab9bf9076 100644 --- a/packages/nodes-base/nodes/Postgres/v2/actions/database/update.operation.ts +++ b/packages/nodes-base/nodes/Postgres/v2/actions/database/update.operation.ts @@ -199,7 +199,7 @@ export async function execute( db: PgpDatabase, ): Promise { items = replaceEmptyStringsByNulls(items, nodeOptions.replaceEmptyStrings as boolean); - const nodeVersion = nodeOptions.typeVersion as number; + const nodeVersion = nodeOptions.nodeVersion as number; let schema = this.getNodeParameter('schema', 0, undefined, { extractValue: true, diff --git a/packages/nodes-base/nodes/Postgres/v2/actions/database/upsert.operation.ts b/packages/nodes-base/nodes/Postgres/v2/actions/database/upsert.operation.ts index b6783ff47ba19..4040798da0dc4 100644 --- a/packages/nodes-base/nodes/Postgres/v2/actions/database/upsert.operation.ts +++ b/packages/nodes-base/nodes/Postgres/v2/actions/database/upsert.operation.ts @@ -198,7 +198,7 @@ export async function execute( db: PgpDatabase, ): Promise { items = replaceEmptyStringsByNulls(items, nodeOptions.replaceEmptyStrings as boolean); - const nodeVersion = nodeOptions.typeVersion as number; + const nodeVersion = nodeOptions.nodeVersion as number; let schema = this.getNodeParameter('schema', 0, undefined, { extractValue: true, From 9af626a1b36edd07b1117fb41c2f02efa566fc10 Mon Sep 17 00:00:00 2001 From: Deborah Date: Thu, 7 Sep 2023 07:49:55 +0100 Subject: [PATCH 04/23] docs: Docs links for TheHive and TheHive5 (#7124) Github issue / Community forum post (link here to close automatically): --- packages/nodes-base/nodes/TheHive/TheHive.node.json | 2 +- .../nodes-base/nodes/TheHiveProject/TheHiveProject.node.json | 4 ++-- .../nodes/TheHiveProject/TheHiveProjectTrigger.node.json | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/nodes-base/nodes/TheHive/TheHive.node.json b/packages/nodes-base/nodes/TheHive/TheHive.node.json index b59545df9c024..7a20277c7573c 100644 --- a/packages/nodes-base/nodes/TheHive/TheHive.node.json +++ b/packages/nodes-base/nodes/TheHive/TheHive.node.json @@ -6,7 +6,7 @@ "resources": { "credentialDocumentation": [ { - "url": "https://docs.n8n.io/credentials/theHive" + "url": "https://docs.n8n.io/integrations/builtin/credentials/thehive/" } ], "primaryDocumentation": [ diff --git a/packages/nodes-base/nodes/TheHiveProject/TheHiveProject.node.json b/packages/nodes-base/nodes/TheHiveProject/TheHiveProject.node.json index 8fe1e4ea76d93..73176eb7df344 100644 --- a/packages/nodes-base/nodes/TheHiveProject/TheHiveProject.node.json +++ b/packages/nodes-base/nodes/TheHiveProject/TheHiveProject.node.json @@ -7,12 +7,12 @@ "resources": { "credentialDocumentation": [ { - "url": "https://docs.n8n.io/credentials/theHive" + "url": "https://docs.n8n.io/integrations/builtin/credentials/thehive5/" } ], "primaryDocumentation": [ { - "url": "https://docs.n8n.io/integrations/builtin/app-nodes/n8n-nodes-base.thehive/" + "url": "https://docs.n8n.io/integrations/builtin/app-nodes/n8n-nodes-base.thehive5/" } ] } diff --git a/packages/nodes-base/nodes/TheHiveProject/TheHiveProjectTrigger.node.json b/packages/nodes-base/nodes/TheHiveProject/TheHiveProjectTrigger.node.json index def7a19826c88..1732f940dd09e 100644 --- a/packages/nodes-base/nodes/TheHiveProject/TheHiveProjectTrigger.node.json +++ b/packages/nodes-base/nodes/TheHiveProject/TheHiveProjectTrigger.node.json @@ -6,7 +6,7 @@ "resources": { "primaryDocumentation": [ { - "url": "https://docs.n8n.io/integrations/builtin/trigger-nodes/n8n-nodes-base.thehivetrigger/" + "url": "https://docs.n8n.io/integrations/builtin/trigger-nodes/n8n-nodes-base.thehive5trigger/" } ] } From 01f875a94d193ba1e709bf6cfe31a3951f3af81a Mon Sep 17 00:00:00 2001 From: MC Naveen <8493007+mcnaveen@users.noreply.github.com> Date: Thu, 7 Sep 2023 12:37:37 +0530 Subject: [PATCH 05/23] feat(Salesforce Node): Add fax field to lead option (#7030) Closes: https://community.n8n.io/t/there-is-no-fax-field-in-the-saleforce-lead-cretion/29829 --- .../nodes/Salesforce/LeadDescription.ts | 30 +++++++++++++++++++ .../nodes/Salesforce/LeadInterface.ts | 2 ++ .../nodes/Salesforce/Salesforce.node.ts | 12 ++++++++ 3 files changed, 44 insertions(+) diff --git a/packages/nodes-base/nodes/Salesforce/LeadDescription.ts b/packages/nodes-base/nodes/Salesforce/LeadDescription.ts index b7a9fc8c4c4d2..4395a48b9ea0b 100644 --- a/packages/nodes-base/nodes/Salesforce/LeadDescription.ts +++ b/packages/nodes-base/nodes/Salesforce/LeadDescription.ts @@ -228,6 +228,13 @@ export const leadFields: INodeProperties[] = [ default: '', description: 'Email address for the lead', }, + { + displayName: 'Fax', + name: 'fax', + type: 'number', + default: '', + description: 'Fax number of the lead', + }, { displayName: 'First Name', name: 'firstname', @@ -243,6 +250,14 @@ export const leadFields: INodeProperties[] = [ description: 'Whether the lead doesn’t want to receive email from Salesforce (true) or does (false). Label is Email Opt Out.', }, + { + displayName: 'Has Opted Out of Fax', + name: 'hasOptedOutOfFax', + type: 'boolean', + default: false, + description: + 'Whether the lead doesn’t want to receive fax from Salesforce (true) or does (false). Label is Email Opt Out.', + }, { displayName: 'Industry', name: 'industry', @@ -497,6 +512,13 @@ export const leadFields: INodeProperties[] = [ default: '', description: 'Email address for the lead', }, + { + displayName: 'Fax', + name: 'fax', + type: 'number', + default: '', + description: 'Fax Number of the lead', + }, { displayName: 'First Name', name: 'firstname', @@ -512,6 +534,14 @@ export const leadFields: INodeProperties[] = [ description: 'Whether the lead doesn’t want to receive email from Salesforce (true) or does (false). Label is Email Opt Out.', }, + { + displayName: 'Has Opted Out of Fax', + name: 'HasOptedOutOfFax', + type: 'boolean', + default: false, + description: + 'Whether the lead doesn’t want to receive fax from Salesforce (true) or does (false). Label is Fax Opt Out.', + }, { displayName: 'Industry', name: 'industry', diff --git a/packages/nodes-base/nodes/Salesforce/LeadInterface.ts b/packages/nodes-base/nodes/Salesforce/LeadInterface.ts index fca3e2be5da70..21881f1b9900e 100644 --- a/packages/nodes-base/nodes/Salesforce/LeadInterface.ts +++ b/packages/nodes-base/nodes/Salesforce/LeadInterface.ts @@ -3,6 +3,7 @@ export interface ILead { Company?: string; LastName?: string; Email?: string; + Fax?: number; City?: string; Phone?: string; State?: string; @@ -25,4 +26,5 @@ export interface ILead { NumberOfEmployees?: number; MobilePhone?: string; HasOptedOutOfEmail?: boolean; + HasOptedOutOfFax?: boolean; } diff --git a/packages/nodes-base/nodes/Salesforce/Salesforce.node.ts b/packages/nodes-base/nodes/Salesforce/Salesforce.node.ts index 380bf0daac32b..168075b9c56e2 100644 --- a/packages/nodes-base/nodes/Salesforce/Salesforce.node.ts +++ b/packages/nodes-base/nodes/Salesforce/Salesforce.node.ts @@ -1084,6 +1084,9 @@ export class Salesforce implements INodeType { if (additionalFields.hasOptedOutOfEmail !== undefined) { body.HasOptedOutOfEmail = additionalFields.hasOptedOutOfEmail as boolean; } + if (additionalFields.hasOptedOutOfFax !== undefined) { + body.HasOptedOutOfFax = additionalFields.hasOptedOutOfFax as boolean; + } if (additionalFields.email !== undefined) { body.Email = additionalFields.email as string; } @@ -1123,6 +1126,9 @@ export class Salesforce implements INodeType { if (additionalFields.industry !== undefined) { body.Industry = additionalFields.industry as string; } + if (additionalFields.fax !== undefined) { + body.Fax = additionalFields.fax as number; + } if (additionalFields.firstname !== undefined) { body.FirstName = additionalFields.firstname as string; } @@ -1190,6 +1196,9 @@ export class Salesforce implements INodeType { if (updateFields.hasOptedOutOfEmail !== undefined) { body.HasOptedOutOfEmail = updateFields.hasOptedOutOfEmail as boolean; } + if (updateFields.hasOptedOutOfFax !== undefined) { + body.hasOptedOutOfFax = updateFields.hasOptedOutOfFax as boolean; + } if (updateFields.lastname !== undefined) { body.LastName = updateFields.lastname as string; } @@ -1238,6 +1247,9 @@ export class Salesforce implements INodeType { if (updateFields.firstname !== undefined) { body.FirstName = updateFields.firstname as string; } + if (updateFields.fax !== undefined) { + body.Fax = updateFields.fax as number; + } if (updateFields.leadSource !== undefined) { body.LeadSource = updateFields.leadSource as string; } From a223734a4a781834bee1a1484dffc47c56e8d50e 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: Thu, 7 Sep 2023 10:25:59 +0200 Subject: [PATCH 06/23] fix(core): Disable Node.js custom inspection to address CVE-2023-37903 (#7125) This seems like a better fix than #7122 --- packages/cli/bin/n8n | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/cli/bin/n8n b/packages/cli/bin/n8n index 1f81419a847ef..c0dc619251812 100755 --- a/packages/cli/bin/n8n +++ b/packages/cli/bin/n8n @@ -32,6 +32,10 @@ if (![18, 20].includes(nodeVersionMajor)) { // Prevent oclif from loading ts-node and typescript process.env.OCLIF_TS_NODE = '0'; +// Disable nodejs custom inspection across the app +const { inspect } = require('util'); +inspect.defaultOptions.customInspect = false; + require('express-async-errors'); require('source-map-support').install(); require('reflect-metadata'); From f07d97fa6ee1efcc635ddaa035988b0d873968b5 Mon Sep 17 00:00:00 2001 From: Jon Date: Thu, 7 Sep 2023 10:42:34 +0100 Subject: [PATCH 07/23] docs: Add email alias to Outlook and GMail (#7127) --- packages/nodes-base/nodes/Google/Gmail/Gmail.node.json | 3 ++- .../nodes/Microsoft/Outlook/MicrosoftOutlook.node.json | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/nodes-base/nodes/Google/Gmail/Gmail.node.json b/packages/nodes-base/nodes/Google/Gmail/Gmail.node.json index 73a72d2fb569f..f55b13a1667db 100644 --- a/packages/nodes-base/nodes/Google/Gmail/Gmail.node.json +++ b/packages/nodes-base/nodes/Google/Gmail/Gmail.node.json @@ -51,5 +51,6 @@ "url": "https://n8n.io/blog/using-automation-to-boost-productivity-in-the-workplace/" } ] - } + }, + "alias": ["email"] } diff --git a/packages/nodes-base/nodes/Microsoft/Outlook/MicrosoftOutlook.node.json b/packages/nodes-base/nodes/Microsoft/Outlook/MicrosoftOutlook.node.json index 4d2d75972feb0..7cb73e35b1f0e 100644 --- a/packages/nodes-base/nodes/Microsoft/Outlook/MicrosoftOutlook.node.json +++ b/packages/nodes-base/nodes/Microsoft/Outlook/MicrosoftOutlook.node.json @@ -14,5 +14,6 @@ "url": "https://docs.n8n.io/integrations/builtin/app-nodes/n8n-nodes-base.microsoftoutlook/" } ] - } + }, + "alias": ["email"] } From 0a35025e5e6669661bdfcc16378453ec9109a347 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: Thu, 7 Sep 2023 14:18:15 +0200 Subject: [PATCH 08/23] fix(Code Node): Upgrade vm2 to address CVE-2023-37466 (#7123) [GH Advisory](https://github.com/advisories/GHSA-cchq-frgv-rjh5) Actual fix [here](https://github.com/n8n-io/vm2/commit/26168e6dfe32fbbb570d1071e211d4891f1ef0d8). --- packages/nodes-base/package.json | 2 +- pnpm-lock.yaml | 20 ++++++++++---------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/packages/nodes-base/package.json b/packages/nodes-base/package.json index 660fa2acac61f..d6d6eb2a5ed58 100644 --- a/packages/nodes-base/package.json +++ b/packages/nodes-base/package.json @@ -814,7 +814,7 @@ }, "dependencies": { "@kafkajs/confluent-schema-registry": "1.0.6", - "@n8n/vm2": "^3.9.19", + "@n8n/vm2": "^3.9.20", "amqplib": "^0.10.3", "aws4": "^1.8.0", "basic-auth": "^2.0.1", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index afa4c212af71b..efcfc7c8cfd50 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1016,8 +1016,8 @@ importers: specifier: 1.0.6 version: 1.0.6 '@n8n/vm2': - specifier: ^3.9.19 - version: 3.9.19 + specifier: ^3.9.20 + version: 3.9.20 amqplib: specifier: ^0.10.3 version: 0.10.3 @@ -4650,8 +4650,8 @@ packages: - '@lezer/common' dev: false - /@n8n/vm2@3.9.19: - resolution: {integrity: sha512-KmrkVqri7VG+GQ2JbJ2/2gyEmVkysxzoJUF+zdtqBZLU8GN9UDwAnjzUOUgpD0dCRd1SUVLqwM4GpjOeca4XZw==} + /@n8n/vm2@3.9.20: + resolution: {integrity: sha512-qk2oJYkuFRVSTxoro4obX/sv/wT1pViZjHh/isjOvFB93D52QIg3TCjMPsHOfHTmkxCKJffjLrUvjIwvWzSMCQ==} engines: {node: '>=18.10', pnpm: '>=8.6.12'} hasBin: true dependencies: @@ -6765,7 +6765,7 @@ packages: ts-dedent: 2.2.0 type-fest: 3.13.1 vue: 3.3.4 - vue-component-type-helpers: 1.8.8 + vue-component-type-helpers: 1.8.10 transitivePeerDependencies: - encoding - supports-color @@ -8430,7 +8430,7 @@ packages: /acorn-globals@7.0.1: resolution: {integrity: sha512-umOSDSDrfHbTNPuNpC2NSnnA3LUrqpevPb4T9jRx4MagXNS0rs+gwiTcAvqCRmsD6utzsrzNt+ebm00SNWiC3Q==} dependencies: - acorn: 8.8.1 + acorn: 8.10.0 acorn-walk: 8.2.0 dev: true @@ -21744,12 +21744,12 @@ packages: vue: 3.3.4 dev: false - /vue-component-type-helpers@1.8.4: - resolution: {integrity: sha512-6bnLkn8O0JJyiFSIF0EfCogzeqNXpnjJ0vW/SZzNHfe6sPx30lTtTXlE5TFs2qhJlAtDFybStVNpL73cPe3OMQ==} + /vue-component-type-helpers@1.8.10: + resolution: {integrity: sha512-FJtmfw2Gn6eQ8kAVNEhw9nYIzWmVQJjdyQRtJXZ7tgXh/FoZhQnZ2KyxR+NuF9U4iZLBvSspeetIpnP9yxxyMw==} dev: true - /vue-component-type-helpers@1.8.8: - resolution: {integrity: sha512-Ohv9HQY92nSbpReC6WhY0X4YkOszHzwUHaaN/lev5tHQLM1AEw+LrLeB2bIGIyKGDU7ZVrncXcv/oBny4rjbYg==} + /vue-component-type-helpers@1.8.4: + resolution: {integrity: sha512-6bnLkn8O0JJyiFSIF0EfCogzeqNXpnjJ0vW/SZzNHfe6sPx30lTtTXlE5TFs2qhJlAtDFybStVNpL73cPe3OMQ==} dev: true /vue-demi@0.14.5(vue@3.3.4): From 7b49cf2a2c750d685af6cff464401f38482dac5a Mon Sep 17 00:00:00 2001 From: Michael Auerswald Date: Thu, 7 Sep 2023 14:44:19 +0200 Subject: [PATCH 09/23] feat(core): Add commands to workers to respond with current state (#7029) This PR adds new endpoints to the REST API: `/orchestration/worker/status` and `/orchestration/worker/id` Currently these just trigger the return of status / ids from the workers via the redis back channel, this still needs to be handled and passed through to the frontend. It also adds the eventbus to each worker, and triggers a reload of those eventbus instances when the configuration changes on the main instances. --- packages/cli/src/AbstractServer.ts | 83 +------ packages/cli/src/Server.ts | 2 + packages/cli/src/commands/BaseCommand.ts | 4 +- packages/cli/src/commands/worker.ts | 222 +++++++++++------- .../cli/src/commands/workerCommandHandler.ts | 82 +++++++ .../controllers/orchestration.controller.ts | 35 +++ .../src/eventbus/EventMessageClasses/index.ts | 10 +- .../MessageEventBus/MessageEventBus.ts | 129 ++++++---- packages/cli/src/requests.ts | 9 + .../cli/src/services/orchestration.service.ts | 172 ++++++++++++++ .../services/redis/RedisServiceCommands.ts | 38 ++- .../redis/RedisServicePubSubSubscriber.ts | 4 +- .../integration/commands/worker.cmd.test.ts | 81 +++++++ .../services/orchestration.service.test.ts | 140 +++++++++++ 14 files changed, 790 insertions(+), 221 deletions(-) create mode 100644 packages/cli/src/commands/workerCommandHandler.ts create mode 100644 packages/cli/src/controllers/orchestration.controller.ts create mode 100644 packages/cli/src/services/orchestration.service.ts create mode 100644 packages/cli/test/integration/commands/worker.cmd.test.ts create mode 100644 packages/cli/test/unit/services/orchestration.service.test.ts diff --git a/packages/cli/src/AbstractServer.ts b/packages/cli/src/AbstractServer.ts index fa321a6724b3d..2e25342bc1fcc 100644 --- a/packages/cli/src/AbstractServer.ts +++ b/packages/cli/src/AbstractServer.ts @@ -4,7 +4,7 @@ import type { Server } from 'http'; import express from 'express'; import compression from 'compression'; import isbot from 'isbot'; -import { jsonParse, LoggerProxy as Logger } from 'n8n-workflow'; +import { LoggerProxy as Logger } from 'n8n-workflow'; import config from '@/config'; import { N8N_VERSION, inDevelopment, inTest } from '@/constants'; @@ -18,16 +18,8 @@ import { rawBodyReader, bodyParser, corsMiddleware } from '@/middlewares'; import { TestWebhooks } from '@/TestWebhooks'; import { WaitingWebhooks } from '@/WaitingWebhooks'; import { webhookRequestHandler } from '@/WebhookHelpers'; -import { RedisService } from '@/services/redis.service'; -import { eventBus } from './eventbus'; -import type { AbstractEventMessageOptions } from './eventbus/EventMessageClasses/AbstractEventMessageOptions'; -import { getEventMessageObjectByType } from './eventbus/EventMessageClasses/Helpers'; -import type { RedisServiceWorkerResponseObject } from './services/redis/RedisServiceCommands'; -import { - EVENT_BUS_REDIS_CHANNEL, - WORKER_RESPONSE_REDIS_CHANNEL, -} from './services/redis/RedisServiceHelper'; import { generateHostInstanceId } from './databases/utils/generators'; +import { OrchestrationService } from './services/orchestration.service'; export abstract class AbstractServer { protected server: Server; @@ -124,78 +116,11 @@ export abstract class AbstractServer { }); if (config.getEnv('executions.mode') === 'queue') { - await this.setupRedis(); + // will start the redis connections + await Container.get(OrchestrationService).init(this.uniqueInstanceId); } } - // This connection is going to be our heartbeat - // IORedis automatically pings redis and tries to reconnect - // We will be using a retryStrategy to control how and when to exit. - // We are also subscribing to the event log channel to receive events from workers - private async setupRedis() { - const redisService = Container.get(RedisService); - const redisSubscriber = await redisService.getPubSubSubscriber(); - - // TODO: these are all proof of concept implementations for the moment - // until worker communication is implemented - // #region proof of concept - await redisSubscriber.subscribeToEventLog(); - await redisSubscriber.subscribeToWorkerResponseChannel(); - redisSubscriber.addMessageHandler( - 'AbstractServerReceiver', - async (channel: string, message: string) => { - // TODO: this is a proof of concept implementation to forward events to the main instance's event bus - // Events are arriving through a pub/sub channel and are forwarded to the eventBus - // In the future, a stream should probably replace this implementation entirely - if (channel === EVENT_BUS_REDIS_CHANNEL) { - const eventData = jsonParse(message); - if (eventData) { - const eventMessage = getEventMessageObjectByType(eventData); - if (eventMessage) { - await eventBus.send(eventMessage); - } - } - } else if (channel === WORKER_RESPONSE_REDIS_CHANNEL) { - // The back channel from the workers as a pub/sub channel - const workerResponse = jsonParse(message); - if (workerResponse) { - // TODO: Handle worker response - console.log('Received worker response', workerResponse); - } - } - }, - ); - // TODO: Leave comments for now as implementation example - // const redisStreamListener = await redisService.getStreamConsumer(); - // void redisStreamListener.listenToStream('teststream'); - // redisStreamListener.addMessageHandler( - // 'MessageLogger', - // async (stream: string, id: string, message: string[]) => { - // // TODO: this is a proof of concept implementation of a stream consumer - // switch (stream) { - // case EVENT_BUS_REDIS_STREAM: - // case COMMAND_REDIS_STREAM: - // case WORKER_RESPONSE_REDIS_STREAM: - // default: - // LoggerProxy.debug( - // `Received message from stream ${stream} with id ${id} and message ${message.join( - // ',', - // )}`, - // ); - // break; - // } - // }, - // ); - - // const redisListReceiver = await redisService.getListReceiver(); - // await redisListReceiver.init(); - - // setInterval(async () => { - // await redisListReceiver.popLatestWorkerResponse(); - // }, 1000); - // #endregion - } - async init(): Promise { const { app, protocol, sslKey, sslCert } = this; diff --git a/packages/cli/src/Server.ts b/packages/cli/src/Server.ts index 3d8f8de2aefa1..3903373c71e35 100644 --- a/packages/cli/src/Server.ts +++ b/packages/cli/src/Server.ts @@ -177,6 +177,7 @@ import { handleMfaDisable, isMfaFeatureEnabled } from './Mfa/helpers'; import { JwtService } from './services/jwt.service'; import { RoleService } from './services/role.service'; import { UserService } from './services/user.service'; +import { OrchestrationController } from './controllers/orchestration.controller'; const exec = promisify(callbackExec); @@ -551,6 +552,7 @@ export class Server extends AbstractServer { Container.get(SourceControlController), Container.get(WorkflowStatisticsController), Container.get(ExternalSecretsController), + Container.get(OrchestrationController), ]; if (isLdapEnabled()) { diff --git a/packages/cli/src/commands/BaseCommand.ts b/packages/cli/src/commands/BaseCommand.ts index 78be7cc23f04c..008e9f4e93435 100644 --- a/packages/cli/src/commands/BaseCommand.ts +++ b/packages/cli/src/commands/BaseCommand.ts @@ -103,12 +103,12 @@ export abstract class BaseCommand extends Command { process.exit(1); } - protected async initBinaryManager() { + async initBinaryManager() { const binaryDataConfig = config.getEnv('binaryDataManager'); await BinaryDataManager.init(binaryDataConfig, true); } - protected async initExternalHooks() { + async initExternalHooks() { this.externalHooks = Container.get(ExternalHooks); await this.externalHooks.init(); } diff --git a/packages/cli/src/commands/worker.ts b/packages/cli/src/commands/worker.ts index dfaf0e34c0c62..320c08aa9c8af 100644 --- a/packages/cli/src/commands/worker.ts +++ b/packages/cli/src/commands/worker.ts @@ -27,6 +27,11 @@ import { generateHostInstanceId } from '@/databases/utils/generators'; import type { ICredentialsOverwrite } from '@/Interfaces'; import { CredentialsOverwrites } from '@/CredentialsOverwrites'; import { rawBodyReader, bodyParser } from '@/middlewares'; +import { eventBus } from '../eventbus'; +import { RedisServicePubSubPublisher } from '../services/redis/RedisServicePubSubPublisher'; +import { RedisServicePubSubSubscriber } from '../services/redis/RedisServicePubSubSubscriber'; +import { EventMessageGeneric } from '../eventbus/EventMessageClasses/EventMessageGeneric'; +import { getWorkerCommandReceivedHandler } from './workerCommandHandler'; export class Worker extends BaseCommand { static description = '\nStarts a n8n worker'; @@ -49,6 +54,10 @@ export class Worker extends BaseCommand { readonly uniqueInstanceId = generateHostInstanceId('worker'); + redisPublisher: RedisServicePubSubPublisher; + + redisSubscriber: RedisServicePubSubSubscriber; + /** * Stop n8n in a graceful way. * Make for example sure that all the webhooks from third party services @@ -240,9 +249,48 @@ export class Worker extends BaseCommand { await this.initBinaryManager(); await this.initExternalHooks(); await this.initExternalSecrets(); + await this.initEventBus(); + await this.initRedis(); + await this.initQueue(); } - async run() { + async initEventBus() { + await eventBus.initialize({ + workerId: this.uniqueInstanceId, + }); + } + + /** + * Initializes the redis connection + * A publishing connection to redis is created to publish events to the event log + * A subscription connection to redis is created to subscribe to commands from the main process + * The subscription connection adds a handler to handle the command messages + */ + async initRedis() { + this.redisPublisher = Container.get(RedisServicePubSubPublisher); + this.redisSubscriber = Container.get(RedisServicePubSubSubscriber); + await this.redisPublisher.init(); + await this.redisPublisher.publishToEventLog( + new EventMessageGeneric({ + eventName: 'n8n.worker.started', + payload: { + workerId: this.uniqueInstanceId, + }, + }), + ); + await this.redisSubscriber.subscribeToCommandChannel(); + this.redisSubscriber.addMessageHandler( + 'WorkerCommandReceivedHandler', + // eslint-disable-next-line @typescript-eslint/no-unsafe-argument + getWorkerCommandReceivedHandler({ + uniqueInstanceId: this.uniqueInstanceId, + redisPublisher: this.redisPublisher, + getRunningJobIds: () => Object.keys(Worker.runningJobs), + }), + ); + } + + async initQueue() { // eslint-disable-next-line @typescript-eslint/no-shadow const { flags } = this.parse(Worker); @@ -255,11 +303,6 @@ export class Worker extends BaseCommand { this.runJob(job, this.nodeTypes), ); - this.logger.info('\nn8n worker is now ready'); - this.logger.info(` * Version: ${N8N_VERSION}`); - this.logger.info(` * Concurrency: ${flags.concurrency}`); - this.logger.info(''); - Worker.jobQueue.on('global:progress', (jobId: JobId, progress) => { // Progress of a job got updated which does get used // to communicate that a job got canceled. @@ -305,105 +348,116 @@ export class Worker extends BaseCommand { throw error; } }); + } - if (config.getEnv('queue.health.active')) { - const port = config.getEnv('queue.health.port'); + async setupHealthMonitor() { + const port = config.getEnv('queue.health.port'); - const app = express(); - app.disable('x-powered-by'); + const app = express(); + app.disable('x-powered-by'); - const server = http.createServer(app); + const server = http.createServer(app); - app.get( - '/healthz', + app.get( + '/healthz', - async (req: express.Request, res: express.Response) => { - LoggerProxy.debug('Health check started!'); + async (req: express.Request, res: express.Response) => { + LoggerProxy.debug('Health check started!'); - const connection = Db.getConnection(); + const connection = Db.getConnection(); - try { - if (!connection.isInitialized) { - // Connection is not active - throw new Error('No active database connection!'); - } - // DB ping - await connection.query('SELECT 1'); - } catch (e) { - LoggerProxy.error('No Database connection!', e as Error); - const error = new ResponseHelper.ServiceUnavailableError('No Database connection!'); - return ResponseHelper.sendErrorResponse(res, error); + try { + if (!connection.isInitialized) { + // Connection is not active + throw new Error('No active database connection!'); } + // DB ping + await connection.query('SELECT 1'); + } catch (e) { + LoggerProxy.error('No Database connection!', e as Error); + const error = new ResponseHelper.ServiceUnavailableError('No Database connection!'); + return ResponseHelper.sendErrorResponse(res, error); + } - // Just to be complete, generally will the worker stop automatically - // if it loses the connection to redis - try { - // Redis ping - await Worker.jobQueue.client.ping(); - } catch (e) { - LoggerProxy.error('No Redis connection!', e as Error); - const error = new ResponseHelper.ServiceUnavailableError('No Redis connection!'); - return ResponseHelper.sendErrorResponse(res, error); - } + // Just to be complete, generally will the worker stop automatically + // if it loses the connection to redis + try { + // Redis ping + await Worker.jobQueue.client.ping(); + } catch (e) { + LoggerProxy.error('No Redis connection!', e as Error); + const error = new ResponseHelper.ServiceUnavailableError('No Redis connection!'); + return ResponseHelper.sendErrorResponse(res, error); + } - // Everything fine - const responseData = { - status: 'ok', - }; + // Everything fine + const responseData = { + status: 'ok', + }; - LoggerProxy.debug('Health check completed successfully!'); + LoggerProxy.debug('Health check completed successfully!'); - ResponseHelper.sendSuccessResponse(res, responseData, true, 200); - }, - ); + ResponseHelper.sendSuccessResponse(res, responseData, true, 200); + }, + ); - let presetCredentialsLoaded = false; - const endpointPresetCredentials = config.getEnv('credentials.overwrite.endpoint'); - if (endpointPresetCredentials !== '') { - // POST endpoint to set preset credentials - app.post( - `/${endpointPresetCredentials}`, - rawBodyReader, - bodyParser, - async (req: express.Request, res: express.Response) => { - if (!presetCredentialsLoaded) { - const body = req.body as ICredentialsOverwrite; - - if (req.contentType !== 'application/json') { - ResponseHelper.sendErrorResponse( - res, - new Error( - 'Body must be a valid JSON, make sure the content-type is application/json', - ), - ); - return; - } - - CredentialsOverwrites().setData(body); - presetCredentialsLoaded = true; - ResponseHelper.sendSuccessResponse(res, { success: true }, true, 200); - } else { + let presetCredentialsLoaded = false; + const endpointPresetCredentials = config.getEnv('credentials.overwrite.endpoint'); + if (endpointPresetCredentials !== '') { + // POST endpoint to set preset credentials + app.post( + `/${endpointPresetCredentials}`, + rawBodyReader, + bodyParser, + async (req: express.Request, res: express.Response) => { + if (!presetCredentialsLoaded) { + const body = req.body as ICredentialsOverwrite; + + if (req.contentType !== 'application/json') { ResponseHelper.sendErrorResponse( res, - new Error('Preset credentials can be set once'), + new Error( + 'Body must be a valid JSON, make sure the content-type is application/json', + ), ); + return; } - }, + + CredentialsOverwrites().setData(body); + presetCredentialsLoaded = true; + ResponseHelper.sendSuccessResponse(res, { success: true }, true, 200); + } else { + ResponseHelper.sendErrorResponse(res, new Error('Preset credentials can be set once')); + } + }, + ); + } + + server.on('error', (error: Error & { code: string }) => { + if (error.code === 'EADDRINUSE') { + this.logger.error( + `n8n's port ${port} is already in use. Do you have the n8n main process running on that port?`, ); + process.exit(1); } + }); - server.on('error', (error: Error & { code: string }) => { - if (error.code === 'EADDRINUSE') { - this.logger.error( - `n8n's port ${port} is already in use. Do you have the n8n main process running on that port?`, - ); - process.exit(1); - } - }); + await new Promise((resolve) => server.listen(port, () => resolve())); + await this.externalHooks.run('worker.ready'); + this.logger.info(`\nn8n worker health check via, port ${port}`); + } + + async run() { + // eslint-disable-next-line @typescript-eslint/no-shadow + const { flags } = this.parse(Worker); - await new Promise((resolve) => server.listen(port, () => resolve())); - await this.externalHooks.run('worker.ready'); - this.logger.info(`\nn8n worker health check via, port ${port}`); + this.logger.info('\nn8n worker is now ready'); + this.logger.info(` * Version: ${N8N_VERSION}`); + this.logger.info(` * Concurrency: ${flags.concurrency}`); + this.logger.info(''); + + if (config.getEnv('queue.health.active')) { + await this.setupHealthMonitor(); } // Make sure that the process does not close diff --git a/packages/cli/src/commands/workerCommandHandler.ts b/packages/cli/src/commands/workerCommandHandler.ts new file mode 100644 index 0000000000000..874ead410c542 --- /dev/null +++ b/packages/cli/src/commands/workerCommandHandler.ts @@ -0,0 +1,82 @@ +import { jsonParse, LoggerProxy } from 'n8n-workflow'; +import { eventBus } from '../eventbus'; +import type { RedisServiceCommandObject } from '@/services/redis/RedisServiceCommands'; +import { COMMAND_REDIS_CHANNEL } from '@/services/redis/RedisServiceHelper'; +import type { RedisServicePubSubPublisher } from '../services/redis/RedisServicePubSubPublisher'; +import * as os from 'os'; + +export function getWorkerCommandReceivedHandler(options: { + uniqueInstanceId: string; + redisPublisher: RedisServicePubSubPublisher; + getRunningJobIds: () => string[]; +}) { + return async (channel: string, messageString: string) => { + if (channel === COMMAND_REDIS_CHANNEL) { + if (!messageString) return; + let message: RedisServiceCommandObject; + try { + message = jsonParse(messageString); + } catch { + LoggerProxy.debug( + `Received invalid message via channel ${COMMAND_REDIS_CHANNEL}: "${messageString}"`, + ); + return; + } + if (message) { + if (message.targets && !message.targets.includes(options.uniqueInstanceId)) { + return; // early return if the message is not for this worker + } + switch (message.command) { + case 'getStatus': + await options.redisPublisher.publishToWorkerChannel({ + workerId: options.uniqueInstanceId, + command: message.command, + payload: { + workerId: options.uniqueInstanceId, + runningJobs: options.getRunningJobIds(), + freeMem: os.freemem(), + totalMem: os.totalmem(), + uptime: process.uptime(), + loadAvg: os.loadavg(), + cpus: os.cpus().map((cpu) => `${cpu.model} - speed: ${cpu.speed}`), + arch: os.arch(), + platform: os.platform(), + hostname: os.hostname(), + net: Object.values(os.networkInterfaces()).flatMap( + (interfaces) => + interfaces?.map((net) => `${net.family} - address: ${net.address}`) ?? '', + ), + }, + }); + break; + case 'getId': + await options.redisPublisher.publishToWorkerChannel({ + workerId: options.uniqueInstanceId, + command: message.command, + }); + break; + case 'restartEventBus': + await eventBus.restart(); + await options.redisPublisher.publishToWorkerChannel({ + workerId: options.uniqueInstanceId, + command: message.command, + payload: { + result: 'success', + }, + }); + break; + case 'stopWorker': + // TODO: implement proper shutdown + // await this.stopProcess(); + break; + default: + LoggerProxy.debug( + // eslint-disable-next-line @typescript-eslint/restrict-template-expressions + `Received unknown command via channel ${COMMAND_REDIS_CHANNEL}: "${message.command}"`, + ); + break; + } + } + } + }; +} diff --git a/packages/cli/src/controllers/orchestration.controller.ts b/packages/cli/src/controllers/orchestration.controller.ts new file mode 100644 index 0000000000000..fd6b68ad4e167 --- /dev/null +++ b/packages/cli/src/controllers/orchestration.controller.ts @@ -0,0 +1,35 @@ +import config from '@/config'; +import { Authorized, Get, RestController } from '@/decorators'; +import { OrchestrationRequest } from '@/requests'; +import { Service } from 'typedi'; +import { OrchestrationService } from '../services/orchestration.service'; + +@Authorized(['global', 'owner']) +@RestController('/orchestration') +@Service() +export class OrchestrationController { + private config = config; + + constructor(private readonly orchestrationService: OrchestrationService) {} + + /** + * These endpoint currently do not return anything, they just trigger the messsage to + * the workers to respond on Redis with their status. + * TODO: these responses need to be forwarded to and handled by the frontend + */ + @Get('/worker/status/:id') + async getWorkersStatus(req: OrchestrationRequest.Get) { + const id = req.params.id; + return this.orchestrationService.getWorkerStatus(id); + } + + @Get('/worker/status') + async getWorkersStatusAll() { + return this.orchestrationService.getWorkerStatus(); + } + + @Get('/worker/ids') + async getWorkerIdsAll() { + return this.orchestrationService.getWorkerIds(); + } +} diff --git a/packages/cli/src/eventbus/EventMessageClasses/index.ts b/packages/cli/src/eventbus/EventMessageClasses/index.ts index 28da7c5eccaa4..c6a0f85bd99ff 100644 --- a/packages/cli/src/eventbus/EventMessageClasses/index.ts +++ b/packages/cli/src/eventbus/EventMessageClasses/index.ts @@ -9,6 +9,7 @@ export const eventNamesWorkflow = [ 'n8n.workflow.failed', 'n8n.workflow.crashed', ] as const; +export const eventNamesGeneric = ['n8n.worker.started', 'n8n.worker.stopped'] as const; export const eventNamesNode = ['n8n.node.started', 'n8n.node.finished'] as const; export const eventNamesAudit = [ 'n8n.audit.user.login.success', @@ -37,14 +38,21 @@ export const eventNamesAudit = [ export type EventNamesWorkflowType = (typeof eventNamesWorkflow)[number]; export type EventNamesAuditType = (typeof eventNamesAudit)[number]; export type EventNamesNodeType = (typeof eventNamesNode)[number]; +export type EventNamesGenericType = (typeof eventNamesGeneric)[number]; export type EventNamesTypes = | EventNamesAuditType | EventNamesWorkflowType | EventNamesNodeType + | EventNamesGenericType | 'n8n.destination.test'; -export const eventNamesAll = [...eventNamesAudit, ...eventNamesWorkflow, ...eventNamesNode]; +export const eventNamesAll = [ + ...eventNamesAudit, + ...eventNamesWorkflow, + ...eventNamesNode, + ...eventNamesGeneric, +]; export type EventMessageTypes = | EventMessageGeneric diff --git a/packages/cli/src/eventbus/MessageEventBus/MessageEventBus.ts b/packages/cli/src/eventbus/MessageEventBus/MessageEventBus.ts index c6d14bf9a3180..43059bf15954b 100644 --- a/packages/cli/src/eventbus/MessageEventBus/MessageEventBus.ts +++ b/packages/cli/src/eventbus/MessageEventBus/MessageEventBus.ts @@ -29,6 +29,7 @@ import { recoverExecutionDataFromEventLogMessages } from './recoverEvents'; import { METRICS_EVENT_NAME } from '../MessageEventBusDestination/Helpers.ee'; import Container from 'typedi'; import { ExecutionRepository, WorkflowRepository } from '@/databases/repositories'; +import { OrchestrationService } from '../../services/orchestration.service'; export type EventMessageReturnMode = 'sent' | 'unsent' | 'all' | 'unfinished'; @@ -37,6 +38,11 @@ export interface MessageWithCallback { confirmCallback: (message: EventMessageTypes, src: EventMessageConfirmSource) => void; } +export interface MessageEventBusInitializeOptions { + skipRecoveryPass?: boolean; + workerId?: string; +} + export class MessageEventBus extends EventEmitter { private static instance: MessageEventBus; @@ -70,7 +76,7 @@ export class MessageEventBus extends EventEmitter { * * Sets `isInitialized` to `true` once finished. */ - async initialize() { + async initialize(options?: MessageEventBusInitializeOptions): Promise { if (this.isInitialized) { return; } @@ -93,64 +99,75 @@ export class MessageEventBus extends EventEmitter { } LoggerProxy.debug('Initializing event writer'); - this.logWriter = await MessageEventBusLogWriter.getInstance(); + if (options?.workerId) { + // only add 'worker' to log file name since the ID changes on every start and we + // would not be able to recover the log files from the previous run not knowing it + const logBaseName = config.getEnv('eventBus.logWriter.logBaseName') + '-worker'; + this.logWriter = await MessageEventBusLogWriter.getInstance({ + logBaseName, + }); + } else { + this.logWriter = await MessageEventBusLogWriter.getInstance(); + } if (!this.logWriter) { LoggerProxy.warn('Could not initialize event writer'); } - // unsent event check: - // - find unsent messages in current event log(s) - // - cycle event logs and start the logging to a fresh file - // - retry sending events - LoggerProxy.debug('Checking for unsent event messages'); - const unsentAndUnfinished = await this.getUnsentAndUnfinishedExecutions(); - LoggerProxy.debug( - `Start logging into ${this.logWriter?.getLogFileName() ?? 'unknown filename'} `, - ); - this.logWriter?.startLogging(); - await this.send(unsentAndUnfinished.unsentMessages); - - const unfinishedExecutionIds = Object.keys(unsentAndUnfinished.unfinishedExecutions); - - if (unfinishedExecutionIds.length > 0) { - LoggerProxy.warn(`Found unfinished executions: ${unfinishedExecutionIds.join(', ')}`); - LoggerProxy.info('This could be due to a crash of an active workflow or a restart of n8n.'); - const activeWorkflows = await Container.get(WorkflowRepository).find({ - where: { active: true }, - select: ['id', 'name'], - }); - if (activeWorkflows.length > 0) { - LoggerProxy.info('Currently active workflows:'); - for (const workflowData of activeWorkflows) { - LoggerProxy.info(` - ${workflowData.name} (ID: ${workflowData.id})`); + if (options?.skipRecoveryPass) { + LoggerProxy.debug('Skipping unsent event check'); + } else { + // unsent event check: + // - find unsent messages in current event log(s) + // - cycle event logs and start the logging to a fresh file + // - retry sending events + LoggerProxy.debug('Checking for unsent event messages'); + const unsentAndUnfinished = await this.getUnsentAndUnfinishedExecutions(); + LoggerProxy.debug( + `Start logging into ${this.logWriter?.getLogFileName() ?? 'unknown filename'} `, + ); + this.logWriter?.startLogging(); + await this.send(unsentAndUnfinished.unsentMessages); + + const unfinishedExecutionIds = Object.keys(unsentAndUnfinished.unfinishedExecutions); + + if (unfinishedExecutionIds.length > 0) { + LoggerProxy.warn(`Found unfinished executions: ${unfinishedExecutionIds.join(', ')}`); + LoggerProxy.info('This could be due to a crash of an active workflow or a restart of n8n.'); + const activeWorkflows = await Container.get(WorkflowRepository).find({ + where: { active: true }, + select: ['id', 'name'], + }); + if (activeWorkflows.length > 0) { + LoggerProxy.info('Currently active workflows:'); + for (const workflowData of activeWorkflows) { + LoggerProxy.info(` - ${workflowData.name} (ID: ${workflowData.id})`); + } } - } - - const recoveryAlreadyAttempted = this.logWriter?.isRecoveryProcessRunning(); - if (recoveryAlreadyAttempted || config.getEnv('eventBus.crashRecoveryMode') === 'simple') { - await Container.get(ExecutionRepository).markAsCrashed(unfinishedExecutionIds); - // if we end up here, it means that the previous recovery process did not finish - // a possible reason would be that recreating the workflow data itself caused e.g an OOM error - // in that case, we do not want to retry the recovery process, but rather mark the executions as crashed - if (recoveryAlreadyAttempted) - LoggerProxy.warn('Skipped recovery process since it previously failed.'); - } else { - // start actual recovery process and write recovery process flag file - this.logWriter?.startRecoveryProcess(); - for (const executionId of unfinishedExecutionIds) { - LoggerProxy.warn(`Attempting to recover execution ${executionId}`); - await recoverExecutionDataFromEventLogMessages( - executionId, - unsentAndUnfinished.unfinishedExecutions[executionId], - true, - ); + const recoveryAlreadyAttempted = this.logWriter?.isRecoveryProcessRunning(); + if (recoveryAlreadyAttempted || config.getEnv('eventBus.crashRecoveryMode') === 'simple') { + await Container.get(ExecutionRepository).markAsCrashed(unfinishedExecutionIds); + // if we end up here, it means that the previous recovery process did not finish + // a possible reason would be that recreating the workflow data itself caused e.g an OOM error + // in that case, we do not want to retry the recovery process, but rather mark the executions as crashed + if (recoveryAlreadyAttempted) + LoggerProxy.warn('Skipped recovery process since it previously failed.'); + } else { + // start actual recovery process and write recovery process flag file + this.logWriter?.startRecoveryProcess(); + for (const executionId of unfinishedExecutionIds) { + LoggerProxy.warn(`Attempting to recover execution ${executionId}`); + await recoverExecutionDataFromEventLogMessages( + executionId, + unsentAndUnfinished.unfinishedExecutions[executionId], + true, + ); + } } + // remove the recovery process flag file + this.logWriter?.endRecoveryProcess(); } - // remove the recovery process flag file - this.logWriter?.endRecoveryProcess(); } - // if configured, run this test every n ms if (config.getEnv('eventBus.checkUnsentInterval') > 0) { if (this.pushIntervalTimer) { @@ -192,6 +209,12 @@ export class MessageEventBus extends EventEmitter { return result; } + async broadcastRestartEventbusAfterDestinationUpdate() { + if (config.getEnv('executions.mode') === 'queue') { + await Container.get(OrchestrationService).restartEventBus(); + } + } + private async trySendingUnsent(msgs?: EventMessageTypes[]) { const unsentMessages = msgs ?? (await this.getEventsUnsent()); if (unsentMessages.length > 0) { @@ -212,9 +235,15 @@ export class MessageEventBus extends EventEmitter { ); await this.destinations[destinationName].close(); } + this.isInitialized = false; LoggerProxy.debug('EventBus shut down.'); } + async restart() { + await this.close(); + await this.initialize({ skipRecoveryPass: true }); + } + async send(msgs: EventMessageTypes | EventMessageTypes[]) { if (!Array.isArray(msgs)) { msgs = [msgs]; diff --git a/packages/cli/src/requests.ts b/packages/cli/src/requests.ts index 7a63bc889ffdc..82f2c6a7a1860 100644 --- a/packages/cli/src/requests.ts +++ b/packages/cli/src/requests.ts @@ -535,3 +535,12 @@ export declare namespace ExternalSecretsRequest { type UpdateProvider = AuthenticatedRequest<{ provider: string }>; } + +// ---------------------------------- +// /orchestration +// ---------------------------------- +// +export declare namespace OrchestrationRequest { + type GetAll = AuthenticatedRequest; + type Get = AuthenticatedRequest<{ id: string }, {}, {}, {}>; +} diff --git a/packages/cli/src/services/orchestration.service.ts b/packages/cli/src/services/orchestration.service.ts new file mode 100644 index 0000000000000..d3f8c7e27eb9f --- /dev/null +++ b/packages/cli/src/services/orchestration.service.ts @@ -0,0 +1,172 @@ +import { Service } from 'typedi'; +import { RedisService } from './redis.service'; +import type { RedisServicePubSubPublisher } from './redis/RedisServicePubSubPublisher'; +import type { RedisServicePubSubSubscriber } from './redis/RedisServicePubSubSubscriber'; +import { LoggerProxy, jsonParse } from 'n8n-workflow'; +import { eventBus } from '../eventbus'; +import type { AbstractEventMessageOptions } from '../eventbus/EventMessageClasses/AbstractEventMessageOptions'; +import { getEventMessageObjectByType } from '../eventbus/EventMessageClasses/Helpers'; +import type { + RedisServiceCommandObject, + RedisServiceWorkerResponseObject, +} from './redis/RedisServiceCommands'; +import { + COMMAND_REDIS_CHANNEL, + EVENT_BUS_REDIS_CHANNEL, + WORKER_RESPONSE_REDIS_CHANNEL, +} from './redis/RedisServiceHelper'; + +@Service() +export class OrchestrationService { + private initialized = false; + + private _uniqueInstanceId = ''; + + get uniqueInstanceId(): string { + return this._uniqueInstanceId; + } + + redisPublisher: RedisServicePubSubPublisher; + + redisSubscriber: RedisServicePubSubSubscriber; + + constructor(readonly redisService: RedisService) {} + + async init(uniqueInstanceId: string) { + this._uniqueInstanceId = uniqueInstanceId; + await this.initPublisher(); + await this.initSubscriber(); + this.initialized = true; + } + + async shutdown() { + await this.redisPublisher?.destroy(); + await this.redisSubscriber?.destroy(); + } + + private async initPublisher() { + this.redisPublisher = await this.redisService.getPubSubPublisher(); + } + + private async initSubscriber() { + this.redisSubscriber = await this.redisService.getPubSubSubscriber(); + + // TODO: these are all proof of concept implementations for the moment + // until worker communication is implemented + // #region proof of concept + await this.redisSubscriber.subscribeToEventLog(); + await this.redisSubscriber.subscribeToWorkerResponseChannel(); + await this.redisSubscriber.subscribeToCommandChannel(); + + this.redisSubscriber.addMessageHandler( + 'OrchestrationMessageReceiver', + async (channel: string, messageString: string) => { + // TODO: this is a proof of concept implementation to forward events to the main instance's event bus + // Events are arriving through a pub/sub channel and are forwarded to the eventBus + // In the future, a stream should probably replace this implementation entirely + if (channel === EVENT_BUS_REDIS_CHANNEL) { + await this.handleEventBusMessage(messageString); + } else if (channel === WORKER_RESPONSE_REDIS_CHANNEL) { + await this.handleWorkerResponseMessage(messageString); + } else if (channel === COMMAND_REDIS_CHANNEL) { + await this.handleCommandMessage(messageString); + } + }, + ); + } + + async handleWorkerResponseMessage(messageString: string) { + const workerResponse = jsonParse(messageString); + if (workerResponse) { + // TODO: Handle worker response + LoggerProxy.debug('Received worker response', workerResponse); + } + return workerResponse; + } + + async handleEventBusMessage(messageString: string) { + const eventData = jsonParse(messageString); + if (eventData) { + const eventMessage = getEventMessageObjectByType(eventData); + if (eventMessage) { + await eventBus.send(eventMessage); + } + } + return eventData; + } + + async handleCommandMessage(messageString: string) { + if (!messageString) return; + let message: RedisServiceCommandObject; + try { + message = jsonParse(messageString); + } catch { + LoggerProxy.debug( + `Received invalid message via channel ${COMMAND_REDIS_CHANNEL}: "${messageString}"`, + ); + return; + } + if (message) { + if ( + message.senderId === this.uniqueInstanceId || + (message.targets && !message.targets.includes(this.uniqueInstanceId)) + ) { + LoggerProxy.debug( + `Skipping command message ${message.command} because it's not for this instance.`, + ); + return message; + } + switch (message.command) { + case 'restartEventBus': + await eventBus.restart(); + break; + } + return message; + } + return; + } + + async getWorkerStatus(id?: string) { + if (!this.initialized) { + throw new Error('OrchestrationService not initialized'); + } + await this.redisPublisher.publishToCommandChannel({ + senderId: this.uniqueInstanceId, + command: 'getStatus', + targets: id ? [id] : undefined, + }); + } + + async getWorkerIds() { + if (!this.initialized) { + throw new Error('OrchestrationService not initialized'); + } + await this.redisPublisher.publishToCommandChannel({ + senderId: this.uniqueInstanceId, + command: 'getId', + }); + } + + // TODO: not implemented yet on worker side + async stopWorker(id?: string) { + if (!this.initialized) { + throw new Error('OrchestrationService not initialized'); + } + await this.redisPublisher.publishToCommandChannel({ + senderId: this.uniqueInstanceId, + command: 'stopWorker', + targets: id ? [id] : undefined, + }); + } + + async restartEventBus(id?: string) { + if (!this.initialized) { + throw new Error('OrchestrationService not initialized'); + } + await this.redisPublisher.publishToCommandChannel({ + senderId: this.uniqueInstanceId, + command: 'restartEventBus', + targets: id ? [id] : undefined, + }); + } +} diff --git a/packages/cli/src/services/redis/RedisServiceCommands.ts b/packages/cli/src/services/redis/RedisServiceCommands.ts index cd70a32d6e853..5796560d4b1e2 100644 --- a/packages/cli/src/services/redis/RedisServiceCommands.ts +++ b/packages/cli/src/services/redis/RedisServiceCommands.ts @@ -1,12 +1,13 @@ -export type RedisServiceCommand = 'getStatus' | 'restartEventBus' | 'stopWorker'; // TODO: add more commands +export type RedisServiceCommand = 'getStatus' | 'getId' | 'restartEventBus' | 'stopWorker'; // TODO: add more commands /** * An object to be sent via Redis pub/sub from the main process to the workers. * @field command: The command to be executed. * @field targets: The targets to execute the command on. Leave empty to execute on all workers or specify worker ids. - * @field args: Optional arguments to be passed to the command. + * @field payload: Optional arguments to be sent with the command. */ type RedisServiceBaseCommand = { + senderId: string; command: RedisServiceCommand; payload?: { [key: string]: string | number | boolean | string[] | number[] | boolean[]; @@ -15,7 +16,38 @@ type RedisServiceBaseCommand = { export type RedisServiceWorkerResponseObject = { workerId: string; -} & RedisServiceBaseCommand; +} & ( + | RedisServiceBaseCommand + | { + command: 'getStatus'; + payload: { + workerId: string; + runningJobs: string[]; + freeMem: number; + totalMem: number; + uptime: number; + loadAvg: number[]; + cpus: string[]; + arch: string; + platform: NodeJS.Platform; + hostname: string; + net: string[]; + }; + } + | { + command: 'getId'; + } + | { + command: 'restartEventBus'; + payload: { + result: 'success' | 'error'; + error?: string; + }; + } + | { + command: 'stopWorker'; + } +); export type RedisServiceCommandObject = { targets?: string[]; diff --git a/packages/cli/src/services/redis/RedisServicePubSubSubscriber.ts b/packages/cli/src/services/redis/RedisServicePubSubSubscriber.ts index cb7b05d41fa68..404544d6f9e93 100644 --- a/packages/cli/src/services/redis/RedisServicePubSubSubscriber.ts +++ b/packages/cli/src/services/redis/RedisServicePubSubSubscriber.ts @@ -23,11 +23,11 @@ export class RedisServicePubSubSubscriber extends RedisServiceBaseReceiver { if (!this.redisClient) { await this.init(); } - await this.redisClient?.subscribe(channel, (error, count: number) => { + await this.redisClient?.subscribe(channel, (error, _count: number) => { if (error) { Logger.error(`Error subscribing to channel ${channel}`); } else { - Logger.debug(`Subscribed ${count.toString()} to eventlog channel`); + Logger.debug(`Subscribed Redis PubSub client to channel: ${channel}`); } }); } diff --git a/packages/cli/test/integration/commands/worker.cmd.test.ts b/packages/cli/test/integration/commands/worker.cmd.test.ts new file mode 100644 index 0000000000000..d860579ee392b --- /dev/null +++ b/packages/cli/test/integration/commands/worker.cmd.test.ts @@ -0,0 +1,81 @@ +import { mockInstance } from '../shared/utils/'; +import { Worker } from '@/commands/worker'; +import * as Config from '@oclif/config'; +import { LoggerProxy } from 'n8n-workflow'; +import { Telemetry } from '@/telemetry'; +import { getLogger } from '@/Logger'; +import { ExternalSecretsManager } from '@/ExternalSecrets/ExternalSecretsManager.ee'; +import { BinaryDataManager } from 'n8n-core'; +import { CacheService } from '@/services/cache.service'; +import { RedisServicePubSubPublisher } from '@/services/redis/RedisServicePubSubPublisher'; +import { RedisServicePubSubSubscriber } from '@/services/redis/RedisServicePubSubSubscriber'; +import { MessageEventBus } from '@/eventbus/MessageEventBus/MessageEventBus'; +import { LoadNodesAndCredentials } from '@/LoadNodesAndCredentials'; +import { CredentialTypes } from '@/CredentialTypes'; +import { NodeTypes } from '@/NodeTypes'; +import { InternalHooks } from '@/InternalHooks'; +import { PostHogClient } from '@/posthog'; +import { RedisService } from '@/services/redis.service'; + +const config: Config.IConfig = new Config.Config({ root: __dirname }); + +beforeAll(async () => { + LoggerProxy.init(getLogger()); + mockInstance(Telemetry); + mockInstance(PostHogClient); + mockInstance(InternalHooks); + mockInstance(CacheService); + mockInstance(ExternalSecretsManager); + mockInstance(BinaryDataManager); + mockInstance(MessageEventBus); + mockInstance(LoadNodesAndCredentials); + mockInstance(CredentialTypes); + mockInstance(NodeTypes); + mockInstance(RedisService); + mockInstance(RedisServicePubSubPublisher); + mockInstance(RedisServicePubSubSubscriber); +}); + +test('worker initializes all its components', async () => { + const worker = new Worker([], config); + + jest.spyOn(worker, 'init'); + jest.spyOn(worker, 'initLicense').mockImplementation(async () => {}); + jest.spyOn(worker, 'initBinaryManager').mockImplementation(async () => {}); + jest.spyOn(worker, 'initExternalHooks').mockImplementation(async () => {}); + jest.spyOn(worker, 'initExternalSecrets').mockImplementation(async () => {}); + jest.spyOn(worker, 'initEventBus').mockImplementation(async () => {}); + jest.spyOn(worker, 'initRedis'); + jest.spyOn(RedisServicePubSubPublisher.prototype, 'init').mockImplementation(async () => {}); + jest + .spyOn(RedisServicePubSubPublisher.prototype, 'publishToEventLog') + .mockImplementation(async () => {}); + jest + .spyOn(RedisServicePubSubSubscriber.prototype, 'subscribeToCommandChannel') + .mockImplementation(async () => {}); + jest + .spyOn(RedisServicePubSubSubscriber.prototype, 'addMessageHandler') + .mockImplementation(async () => {}); + jest.spyOn(worker, 'initQueue').mockImplementation(async () => {}); + + await worker.init(); + + expect(worker.uniqueInstanceId).toBeDefined(); + expect(worker.uniqueInstanceId).toContain('worker'); + expect(worker.uniqueInstanceId.length).toBeGreaterThan(15); + expect(worker.initLicense).toHaveBeenCalled(); + expect(worker.initBinaryManager).toHaveBeenCalled(); + expect(worker.initExternalHooks).toHaveBeenCalled(); + expect(worker.initExternalSecrets).toHaveBeenCalled(); + expect(worker.initEventBus).toHaveBeenCalled(); + expect(worker.initRedis).toHaveBeenCalled(); + expect(worker.redisPublisher).toBeDefined(); + expect(worker.redisPublisher.init).toHaveBeenCalled(); + expect(worker.redisPublisher.publishToEventLog).toHaveBeenCalled(); + expect(worker.redisSubscriber).toBeDefined(); + expect(worker.redisSubscriber.subscribeToCommandChannel).toHaveBeenCalled(); + expect(worker.redisSubscriber.addMessageHandler).toHaveBeenCalled(); + expect(worker.initQueue).toHaveBeenCalled(); + + jest.restoreAllMocks(); +}); diff --git a/packages/cli/test/unit/services/orchestration.service.test.ts b/packages/cli/test/unit/services/orchestration.service.test.ts new file mode 100644 index 0000000000000..18204cea2b81c --- /dev/null +++ b/packages/cli/test/unit/services/orchestration.service.test.ts @@ -0,0 +1,140 @@ +import Container from 'typedi'; +import config from '@/config'; +import { LoggerProxy } from 'n8n-workflow'; +import { getLogger } from '@/Logger'; +import { OrchestrationService } from '@/services/orchestration.service'; +import type { RedisServiceWorkerResponseObject } from '@/services/redis/RedisServiceCommands'; +import { EventMessageWorkflow } from '@/eventbus/EventMessageClasses/EventMessageWorkflow'; +import { eventBus } from '@/eventbus'; +import * as EventHelpers from '@/eventbus/EventMessageClasses/Helpers'; +import { RedisService } from '@/services/redis.service'; +import { mockInstance } from '../../integration/shared/utils'; + +const os = Container.get(OrchestrationService); + +function setDefaultConfig() { + config.set('executions.mode', 'queue'); +} + +const workerRestartEventbusResponse: RedisServiceWorkerResponseObject = { + senderId: 'test', + workerId: 'test', + command: 'restartEventBus', + payload: { + result: 'success', + }, +}; + +const eventBusMessage = new EventMessageWorkflow({ + eventName: 'n8n.workflow.success', + id: 'test', + message: 'test', + payload: { + test: 'test', + }, +}); + +describe('Orchestration Service', () => { + beforeAll(async () => { + mockInstance(RedisService); + LoggerProxy.init(getLogger()); + jest.mock('ioredis', () => { + const Redis = require('ioredis-mock'); + if (typeof Redis === 'object') { + // the first mock is an ioredis shim because ioredis-mock depends on it + // https://github.com/stipsan/ioredis-mock/blob/master/src/index.js#L101-L111 + return { + Command: { _transformer: { argument: {}, reply: {} } }, + }; + } + // second mock for our code + // eslint-disable-next-line @typescript-eslint/no-explicit-any + return function (...args: any) { + return new Redis(args); + }; + }); + jest.mock('../../../src/services/redis/RedisServicePubSubPublisher', () => { + return jest.fn().mockImplementation(() => { + return { + init: jest.fn(), + publishToEventLog: jest.fn(), + publishToWorkerChannel: jest.fn(), + destroy: jest.fn(), + }; + }); + }); + jest.mock('../../../src/services/redis/RedisServicePubSubSubscriber', () => { + return jest.fn().mockImplementation(() => { + return { + subscribeToCommandChannel: jest.fn(), + destroy: jest.fn(), + }; + }); + }); + setDefaultConfig(); + }); + + afterAll(async () => { + jest.mock('../../../src/services/redis/RedisServicePubSubPublisher').restoreAllMocks(); + jest.mock('../../../src/services/redis/RedisServicePubSubSubscriber').restoreAllMocks(); + }); + + test('should initialize', async () => { + await os.init('test-orchestration-service'); + expect(os.redisPublisher).toBeDefined(); + expect(os.redisSubscriber).toBeDefined(); + expect(os.uniqueInstanceId).toBeDefined(); + }); + + test('should handle worker responses', async () => { + const response = await os.handleWorkerResponseMessage( + JSON.stringify(workerRestartEventbusResponse), + ); + expect(response.command).toEqual('restartEventBus'); + }); + + test('should handle event messages', async () => { + const response = await os.handleEventBusMessage(JSON.stringify(eventBusMessage)); + jest.spyOn(eventBus, 'send'); + jest.spyOn(EventHelpers, 'getEventMessageObjectByType'); + expect(eventBus.send).toHaveBeenCalled(); + expect(response.eventName).toEqual('n8n.workflow.success'); + jest.spyOn(eventBus, 'send').mockRestore(); + jest.spyOn(EventHelpers, 'getEventMessageObjectByType').mockRestore(); + }); + + test('should handle command messages from others', async () => { + jest.spyOn(eventBus, 'restart'); + const responseFalseId = await os.handleCommandMessage( + JSON.stringify(workerRestartEventbusResponse), + ); + expect(responseFalseId).toBeDefined(); + expect(responseFalseId!.command).toEqual('restartEventBus'); + expect(responseFalseId!.senderId).toEqual('test'); + expect(eventBus.restart).toHaveBeenCalled(); + jest.spyOn(eventBus, 'restart').mockRestore(); + }); + + test('should reject command messages from iteslf', async () => { + jest.spyOn(eventBus, 'restart'); + const response = await os.handleCommandMessage( + JSON.stringify({ ...workerRestartEventbusResponse, senderId: os.uniqueInstanceId }), + ); + expect(response).toBeDefined(); + expect(response!.command).toEqual('restartEventBus'); + expect(response!.senderId).toEqual(os.uniqueInstanceId); + expect(eventBus.restart).not.toHaveBeenCalled(); + jest.spyOn(eventBus, 'restart').mockRestore(); + }); + + test('should send command messages', async () => { + jest.spyOn(os.redisPublisher, 'publishToCommandChannel'); + await os.getWorkerIds(); + expect(os.redisPublisher.publishToCommandChannel).toHaveBeenCalled(); + jest.spyOn(os.redisPublisher, 'publishToCommandChannel').mockRestore(); + }); + + afterAll(async () => { + await os.shutdown(); + }); +}); From 67aaad15eb55011e9b122254ea21e8339d153d8f 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: Thu, 7 Sep 2023 15:07:32 +0200 Subject: [PATCH 10/23] refactor(core): Use a Set for `deletedProperties` in `AugmentObject` (no-changelog) (#7131) --- packages/workflow/src/AugmentObject.ts | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/packages/workflow/src/AugmentObject.ts b/packages/workflow/src/AugmentObject.ts index 01e6f03ecb49d..708bfd84748a8 100644 --- a/packages/workflow/src/AugmentObject.ts +++ b/packages/workflow/src/AugmentObject.ts @@ -76,11 +76,11 @@ export function augmentObject(data: T): T { if (augmentedObjects.has(data)) return data; const newData = {} as IDataObject; - const deletedProperties: Array = []; + const deletedProperties = new Set(); const proxy = new Proxy(data, { get(target, key: string, receiver): unknown { - if (deletedProperties.indexOf(key) !== -1) { + if (deletedProperties.has(key)) { return undefined; } @@ -107,7 +107,7 @@ export function augmentObject(data: T): T { delete newData[key]; } if (key in target) { - deletedProperties.push(key); + deletedProperties.add(key); } return true; @@ -118,34 +118,33 @@ export function augmentObject(data: T): T { delete newData[key]; } if (key in target) { - deletedProperties.push(key); + deletedProperties.add(key); } return true; } newData[key] = newValue as IDataObject; - const deleteIndex = deletedProperties.indexOf(key); - if (deleteIndex !== -1) { - deletedProperties.splice(deleteIndex, 1); + if (deletedProperties.has(key)) { + deletedProperties.delete(key); } return true; }, has(target, key) { - if (deletedProperties.indexOf(key) !== -1) return false; + if (deletedProperties.has(key)) return false; return Reflect.has(newData, key) || Reflect.has(target, key); }, ownKeys(target) { const originalKeys = Reflect.ownKeys(target); const newKeys = Object.keys(newData); return [...new Set([...originalKeys, ...newKeys])].filter( - (key) => deletedProperties.indexOf(key) === -1, + (key) => !deletedProperties.has(key), ); }, getOwnPropertyDescriptor(target, key) { - if (deletedProperties.indexOf(key) !== -1) return undefined; + if (deletedProperties.has(key)) return undefined; return Object.getOwnPropertyDescriptor(key in newData ? newData : data, key); }, }); From 1d1a022defefc790905cfb8fcb9dd364ffb063bb 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: Thu, 7 Sep 2023 15:58:48 +0200 Subject: [PATCH 11/23] feat(core): Add an option to enable WAL mode for SQLite (#7118) https://www.sqlite.org/wal.html --- packages/cli/src/config/schema.ts | 6 ++++++ packages/cli/src/databases/config.ts | 1 + packages/cli/test/integration/shared/testDb.ts | 3 ++- 3 files changed, 9 insertions(+), 1 deletion(-) diff --git a/packages/cli/src/config/schema.ts b/packages/cli/src/config/schema.ts index 622f740aa7dce..a1df4f5116f79 100644 --- a/packages/cli/src/config/schema.ts +++ b/packages/cli/src/config/schema.ts @@ -164,6 +164,12 @@ export const schema = { default: 'database.sqlite', env: 'DB_SQLITE_DATABASE', }, + enableWAL: { + doc: 'Enable SQLite WAL mode', + format: Boolean, + default: false, + env: 'DB_SQLITE_ENABLE_WAL', + }, executeVacuumOnStartup: { doc: 'Runs VACUUM operation on startup to rebuild the database. Reduces filesize and optimizes indexes. WARNING: This is a long running blocking operation. Will increase start-up time.', format: Boolean, diff --git a/packages/cli/src/databases/config.ts b/packages/cli/src/databases/config.ts index b496ea5bcf6f0..a8e42da625463 100644 --- a/packages/cli/src/databases/config.ts +++ b/packages/cli/src/databases/config.ts @@ -24,6 +24,7 @@ const getDBConnectionOptions = (dbType: DatabaseType) => { UserSettings.getUserN8nFolderPath(), config.getEnv('database.sqlite.database'), ), + enableWAL: config.getEnv('database.sqlite.enableWAL'), } : { database: config.getEnv(`database.${configDBType}.database`), diff --git a/packages/cli/test/integration/shared/testDb.ts b/packages/cli/test/integration/shared/testDb.ts index 3186c3f64bbe2..4a9bd306bcbe4 100644 --- a/packages/cli/test/integration/shared/testDb.ts +++ b/packages/cli/test/integration/shared/testDb.ts @@ -576,7 +576,7 @@ export async function getVariableById(id: string) { * Generate options for an in-memory sqlite database connection, * one per test suite run. */ -export const getSqliteOptions = ({ name }: { name: string }): ConnectionOptions => { +const getSqliteOptions = ({ name }: { name: string }): ConnectionOptions => { return { name, type: 'sqlite', @@ -586,6 +586,7 @@ export const getSqliteOptions = ({ name }: { name: string }): ConnectionOptions migrations: sqliteMigrations, migrationsTableName: 'migrations', migrationsRun: false, + enableWAL: config.getEnv('database.sqlite.enableWAL'), }; }; From 5c6cccd4fabf22b9a0fb6b9a548fe7c36d0d6194 Mon Sep 17 00:00:00 2001 From: Jon Date: Fri, 8 Sep 2023 12:49:16 +0100 Subject: [PATCH 12/23] docs: Add concatenate alias to Split in Batches (#7133) Github issue / Community forum post (link here to close automatically): --- .../nodes-base/nodes/SplitInBatches/SplitInBatches.node.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/nodes-base/nodes/SplitInBatches/SplitInBatches.node.json b/packages/nodes-base/nodes/SplitInBatches/SplitInBatches.node.json index 33cd361076f48..815a3c80b7a6a 100644 --- a/packages/nodes-base/nodes/SplitInBatches/SplitInBatches.node.json +++ b/packages/nodes-base/nodes/SplitInBatches/SplitInBatches.node.json @@ -22,7 +22,7 @@ } ] }, - "alias": ["Loop"], + "alias": ["Loop", "concatenate"], "subcategories": { "Core Nodes": ["Flow"] } From fd800b674b52079eb2572a4d2465774759e9b31d Mon Sep 17 00:00:00 2001 From: Jon Date: Mon, 11 Sep 2023 17:15:52 +0100 Subject: [PATCH 13/23] fix(Zoho CRM Node): Fix issue with Sales Order not updating (#6959) --- .../nodes-base/nodes/Zoho/GenericFunctions.ts | 25 +++++++++++++------ .../nodes-base/nodes/Zoho/ZohoCrm.node.ts | 2 +- 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/packages/nodes-base/nodes/Zoho/GenericFunctions.ts b/packages/nodes-base/nodes/Zoho/GenericFunctions.ts index 954501f67a794..4351ab1c4cc83 100644 --- a/packages/nodes-base/nodes/Zoho/GenericFunctions.ts +++ b/packages/nodes-base/nodes/Zoho/GenericFunctions.ts @@ -30,7 +30,9 @@ import type { export function throwOnErrorStatus( this: IExecuteFunctions | IHookFunctions | ILoadOptionsFunctions, - responseData: { data?: Array<{ status: string; message: string }> }, + responseData: { + data?: Array<{ status: string; message: string }>; + }, ) { if (responseData?.data?.[0].status === 'error') { throw new NodeOperationError(this.getNode(), responseData as Error); @@ -69,14 +71,18 @@ export async function zohoApiRequest( try { const responseData = await this.helpers.requestOAuth2?.call(this, 'zohoOAuth2Api', options); - if (responseData === undefined) return []; - throwOnErrorStatus.call(this, responseData as IDataObject); return responseData; } catch (error) { - throw new NodeApiError(this.getNode(), error as JsonObject); + const args = error.cause?.data + ? { + message: error.cause.data.message || 'The Zoho API returned an error.', + description: JSON.stringify(error.cause.data, null, 2), + } + : undefined; + throw new NodeApiError(this.getNode(), error as JsonObject, args); } } @@ -161,13 +167,18 @@ const omit = (propertyToOmit: string, { [propertyToOmit]: _, ...remainingObject /** * Place a product ID at a nested position in a product details field. */ -export const adjustProductDetails = (productDetails: ProductDetails) => { +export const adjustProductDetails = (productDetails: ProductDetails, operation?: string) => { return productDetails.map((p) => { - return { - ...omit('product', p), + const adjustedProduct = { product: { id: p.id }, quantity: p.quantity || 1, }; + + if (operation === 'upsert') { + return { ...adjustedProduct, ...omit('id', p) }; + } else { + return { ...adjustedProduct, ...omit('product', p) }; + } }); }; diff --git a/packages/nodes-base/nodes/Zoho/ZohoCrm.node.ts b/packages/nodes-base/nodes/Zoho/ZohoCrm.node.ts index 6978b6eb6efb4..62dd144715ffd 100644 --- a/packages/nodes-base/nodes/Zoho/ZohoCrm.node.ts +++ b/packages/nodes-base/nodes/Zoho/ZohoCrm.node.ts @@ -1211,7 +1211,7 @@ export class ZohoCrm implements INodeType { const body: IDataObject = { Account_Name: { id: this.getNodeParameter('accountId', i) }, Subject: this.getNodeParameter('subject', i), - Product_Details: adjustProductDetails(productDetails), + Product_Details: adjustProductDetails(productDetails, 'upsert'), }; const additionalFields = this.getNodeParameter('additionalFields', i); From e51f173608dd79bfe53eb86eeaed976109f74410 Mon Sep 17 00:00:00 2001 From: Csaba Tuncsik Date: Mon, 11 Sep 2023 20:54:03 +0200 Subject: [PATCH 14/23] fix(editor): Update git repo url validation regex (#7151) --- .../src/views/SettingsSourceControl.vue | 3 +- .../__tests__/SettingsSourceControl.test.ts | 45 +++++++++++++++++++ 2 files changed, 47 insertions(+), 1 deletion(-) diff --git a/packages/editor-ui/src/views/SettingsSourceControl.vue b/packages/editor-ui/src/views/SettingsSourceControl.vue index 842ba2c47905d..c6539a83f0e3f 100644 --- a/packages/editor-ui/src/views/SettingsSourceControl.vue +++ b/packages/editor-ui/src/views/SettingsSourceControl.vue @@ -122,7 +122,8 @@ const repoUrlValidationRules: Array = [ { name: 'MATCH_REGEX', config: { - regex: /^(?!https?:\/\/)(?:git|ssh|git@[-\w.]+):(\/\/)?(.*?)(\.git)(\/?|\#[-\d\w._]+?)$/, + regex: + /^git@(?:\[[0-9a-fA-F:]+\]|(?:[a-zA-Z0-9-]+\.)*[a-zA-Z0-9-]+)(?::[0-9]+)*:(?:v[0-9]+\/)?[a-zA-Z0-9_.\-\/]+(\.git)?(?:\/[a-zA-Z0-9_.\-\/]+)*$/, message: locale.baseText('settings.sourceControl.repoUrlInvalid'), }, }, diff --git a/packages/editor-ui/src/views/__tests__/SettingsSourceControl.test.ts b/packages/editor-ui/src/views/__tests__/SettingsSourceControl.test.ts index 8b513aadebf55..631344b0a4b89 100644 --- a/packages/editor-ui/src/views/__tests__/SettingsSourceControl.test.ts +++ b/packages/editor-ui/src/views/__tests__/SettingsSourceControl.test.ts @@ -128,4 +128,49 @@ describe('SettingsSourceControl', () => { expect(queryByTestId('source-control-connected-content')).not.toBeInTheDocument(), ); }, 10000); + + describe('should test repo URLs', () => { + beforeEach(() => { + settingsStore.settings.enterprise[EnterpriseEditionFeature.SourceControl] = true; + }); + + test.each([ + ['git@github.com:user/repository.git', true], + ['git@github.enterprise.com:org-name/repo-name.git', true], + ['git@192.168.1.101:2222:user/repo.git', true], + ['git@github.com:user/repo.git/path/to/subdir', true], + // The opening bracket in curly braces makes sure it is not treated as a special character by the 'user-event' library + ['git@{[}2001:db8:100:f101:210:a4ff:fee3:9566]:user/repo.git', true], + ['git@github.com:org/suborg/repo.git', true], + ['git@github.com:user-name/repo-name.git', true], + ['git@github.com:user_name/repo_name.git', true], + ['git@github.com:user/repository', true], + ['git@github.enterprise.com:org-name/repo-name', true], + ['git@192.168.1.101:2222:user/repo', true], + ['git@ssh.dev.azure.com:v3/User/repo/directory', true], + ['http://github.com/user/repository', false], + ['https://github.com/user/repository', false], + ])('%s', async (url: string, isValid: boolean) => { + await nextTick(); + const { container, queryByText } = renderComponent({ + pinia, + }); + + await waitFor(() => expect(sourceControlStore.preferences.publicKey).not.toEqual('')); + + const repoUrlInput = container.querySelector('input[name="repoUrl"]')!; + + await userEvent.click(repoUrlInput); + await userEvent.type(repoUrlInput, url); + await userEvent.tab(); + + const inputError = expect(queryByText('The Git repository URL is not valid')); + + if (isValid) { + inputError.not.toBeInTheDocument(); + } else { + inputError.toBeInTheDocument(); + } + }); + }); }); From b67a6fc432e71a814944affe131ed5cd519105e1 Mon Sep 17 00:00:00 2001 From: greyliath <57492563+greyliath@users.noreply.github.com> Date: Tue, 12 Sep 2023 09:24:29 +0100 Subject: [PATCH 15/23] docs(editor): Update .round() function in NumberExtensions.ts for clarity (#7150) --- packages/workflow/src/Extensions/NumberExtensions.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/workflow/src/Extensions/NumberExtensions.ts b/packages/workflow/src/Extensions/NumberExtensions.ts index 50eb182f668eb..1cbaf53069301 100644 --- a/packages/workflow/src/Extensions/NumberExtensions.ts +++ b/packages/workflow/src/Extensions/NumberExtensions.ts @@ -88,7 +88,7 @@ format.doc = { round.doc = { name: 'round', description: - 'Returns the value of a number rounded to the nearest whole number. Defaults to 0 decimal places if no argument is given.', + 'Returns the value of a number rounded to the nearest whole number, unless a decimal place is specified. Defaults to 0 decimal places if no argument is given.', returnType: 'number', args: [{ name: 'decimalPlaces?', type: 'number' }], docURL: From 915cfa0f6a0311ca34d2f8eeb471c601473314aa Mon Sep 17 00:00:00 2001 From: Quang-Linh LE Date: Tue, 12 Sep 2023 11:03:33 +0200 Subject: [PATCH 16/23] fix(Google Cloud Firestore Node): Fix empty string interpreted as number (#7136) --- .../nodes/Google/Firebase/CloudFirestore/GenericFunctions.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/nodes-base/nodes/Google/Firebase/CloudFirestore/GenericFunctions.ts b/packages/nodes-base/nodes/Google/Firebase/CloudFirestore/GenericFunctions.ts index 8caf77c2b9018..2191c5bd93206 100644 --- a/packages/nodes-base/nodes/Google/Firebase/CloudFirestore/GenericFunctions.ts +++ b/packages/nodes-base/nodes/Google/Firebase/CloudFirestore/GenericFunctions.ts @@ -83,7 +83,7 @@ export function jsonToDocument(value: string | number | IDataObject | IDataObjec return { booleanValue: value }; } else if (value === null) { return { nullValue: null }; - } else if (!isNaN(value as number)) { + } else if (value !== '' && !isNaN(value as number)) { if (value.toString().indexOf('.') !== -1) { return { doubleValue: value }; } else { From 6e5a4f6a589550a816f421ffa966cfeea3cac64d Mon Sep 17 00:00:00 2001 From: Jon Date: Tue, 12 Sep 2023 17:04:39 +0100 Subject: [PATCH 17/23] fix(HubSpot Node): Fix issue with contact lists not working (#5582) --- .../credentials/HubspotOAuth2Api.credentials.ts | 11 ++++++----- .../nodes/Hubspot/V2/ContactListDescription.ts | 2 +- .../nodes-base/nodes/Hubspot/V2/HubspotV2.node.ts | 7 +++++-- 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/packages/nodes-base/credentials/HubspotOAuth2Api.credentials.ts b/packages/nodes-base/credentials/HubspotOAuth2Api.credentials.ts index 971cc8b1895d3..2158078536e75 100644 --- a/packages/nodes-base/credentials/HubspotOAuth2Api.credentials.ts +++ b/packages/nodes-base/credentials/HubspotOAuth2Api.credentials.ts @@ -1,16 +1,17 @@ import type { ICredentialType, INodeProperties } from 'n8n-workflow'; const scopes = [ - 'crm.schemas.deals.read', - 'crm.objects.owners.read', + 'crm.lists.write', + 'crm.objects.contacts.read', 'crm.objects.contacts.write', - 'crm.objects.companies.write', 'crm.objects.companies.read', + 'crm.objects.companies.write', 'crm.objects.deals.read', - 'crm.schemas.contacts.read', 'crm.objects.deals.write', - 'crm.objects.contacts.read', + 'crm.objects.owners.read', 'crm.schemas.companies.read', + 'crm.schemas.contacts.read', + 'crm.schemas.deals.read', 'forms', 'tickets', ]; diff --git a/packages/nodes-base/nodes/Hubspot/V2/ContactListDescription.ts b/packages/nodes-base/nodes/Hubspot/V2/ContactListDescription.ts index b7c4f3dfaff8b..2f52c06956f56 100644 --- a/packages/nodes-base/nodes/Hubspot/V2/ContactListDescription.ts +++ b/packages/nodes-base/nodes/Hubspot/V2/ContactListDescription.ts @@ -86,7 +86,7 @@ export const contactListFields: INodeProperties[] = [ default: '', }, { - displayName: 'List to Add From', + displayName: 'List to Add To', name: 'listId', type: 'number', required: true, diff --git a/packages/nodes-base/nodes/Hubspot/V2/HubspotV2.node.ts b/packages/nodes-base/nodes/Hubspot/V2/HubspotV2.node.ts index 75db612b6b86d..727aeaf743369 100644 --- a/packages/nodes-base/nodes/Hubspot/V2/HubspotV2.node.ts +++ b/packages/nodes-base/nodes/Hubspot/V2/HubspotV2.node.ts @@ -1152,7 +1152,6 @@ export class HubspotV2 implements INodeType { `/contacts/v1/lists/${listId}/add`, body, ); - returnData.push.apply(returnData, responseData as INodeExecutionData[]); } //https://legacydocs.hubspot.com/docs/methods/lists/remove_contact_from_list if (operation === 'remove') { @@ -1168,8 +1167,12 @@ export class HubspotV2 implements INodeType { `/contacts/v1/lists/${listId}/remove`, body, ); - returnData.push.apply(returnData, responseData as INodeExecutionData[]); } + const executionData = this.helpers.constructExecutionMetaData( + this.helpers.returnJsonArray(responseData as IDataObject[]), + { itemData: { item: 0 } }, + ); + returnData.push(...executionData); } catch (error) { if (this.continueOnFail()) { returnData.push({ json: { error: (error as JsonObject).message } }); From 22edc03cab7b8935d071377270f75c3dabbf43f9 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, 12 Sep 2023 19:57:25 +0200 Subject: [PATCH 18/23] fix(core): Ignore missing user-agent on bot check (no-changelog) (#7153) --- packages/cli/src/AbstractServer.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/cli/src/AbstractServer.ts b/packages/cli/src/AbstractServer.ts index 2e25342bc1fcc..8c3d5b5bbdc5b 100644 --- a/packages/cli/src/AbstractServer.ts +++ b/packages/cli/src/AbstractServer.ts @@ -201,7 +201,7 @@ export abstract class AbstractServer { const checkIfBot = isbot.spawn(['bot']); this.app.use((req, res, next) => { const userAgent = req.headers['user-agent']; - if (!userAgent || checkIfBot(userAgent)) { + if (userAgent && checkIfBot(userAgent)) { Logger.info(`Blocked ${req.method} ${req.url} for "${userAgent}"`); res.status(204).end(); } else next(); From c9b79485cf7d361174aeba175ccb98de7d918693 Mon Sep 17 00:00:00 2001 From: Csaba Tuncsik Date: Wed, 13 Sep 2023 09:00:35 +0200 Subject: [PATCH 19/23] fix(editor): Unbind workflow endpoint events in case of workspace reset (#7129) --- cypress/e2e/7-workflow-actions.cy.ts | 26 ++++++++++++++++++++++- packages/editor-ui/src/views/NodeView.vue | 22 +++++++++++-------- 2 files changed, 38 insertions(+), 10 deletions(-) diff --git a/cypress/e2e/7-workflow-actions.cy.ts b/cypress/e2e/7-workflow-actions.cy.ts index fbba00e27b875..368ba5fc03ce9 100644 --- a/cypress/e2e/7-workflow-actions.cy.ts +++ b/cypress/e2e/7-workflow-actions.cy.ts @@ -3,10 +3,12 @@ import { MANUAL_TRIGGER_NODE_NAME, META_KEY, SCHEDULE_TRIGGER_NODE_NAME, + SET_NODE_NAME, } from '../constants'; import { WorkflowPage as WorkflowPageClass } from '../pages/workflow'; import { WorkflowsPage as WorkflowsPageClass } from '../pages/workflows'; -import { getVisibleDropdown, getVisibleSelect } from '../utils'; +import { getVisibleSelect } from '../utils'; +import { WorkflowExecutionsTab } from "../pages"; const NEW_WORKFLOW_NAME = 'Something else'; const IMPORT_WORKFLOW_URL = @@ -16,6 +18,7 @@ const DUPLICATE_WORKFLOW_TAG = 'Duplicate'; const WorkflowPage = new WorkflowPageClass(); const WorkflowPages = new WorkflowsPageClass(); +const executionsTab = new WorkflowExecutionsTab(); describe('Workflow Actions', () => { beforeEach(() => { @@ -250,4 +253,25 @@ describe('Workflow Actions', () => { duplicateWorkflow(); }); }); + + it('should keep endpoint click working when switching between execution and editor tab', () => { + cy.intercept('GET', '/rest/executions?filter=*').as('getExecutions'); + cy.intercept('GET', '/rest/executions-current?filter=*').as('getCurrentExecutions'); + + WorkflowPage.actions.addInitialNodeToCanvas(MANUAL_TRIGGER_NODE_NAME); + WorkflowPage.actions.addNodeToCanvas(SET_NODE_NAME); + WorkflowPage.actions.saveWorkflowOnButtonClick(); + + WorkflowPage.getters.canvasNodePlusEndpointByName(SET_NODE_NAME).click(); + WorkflowPage.getters.nodeCreatorSearchBar().should('be.visible'); + cy.get('body').type('{esc}'); + + executionsTab.actions.switchToExecutionsTab(); + cy.wait(['@getExecutions', '@getCurrentExecutions']); + cy.wait(500); + executionsTab.actions.switchToEditorTab(); + + WorkflowPage.getters.canvasNodePlusEndpointByName(SET_NODE_NAME).click(); + WorkflowPage.getters.nodeCreatorSearchBar().should('be.visible'); + }); }); diff --git a/packages/editor-ui/src/views/NodeView.vue b/packages/editor-ui/src/views/NodeView.vue index 5af61233578db..8a9b3d9d743ef 100644 --- a/packages/editor-ui/src/views/NodeView.vue +++ b/packages/editor-ui/src/views/NodeView.vue @@ -2531,15 +2531,18 @@ export default defineComponent({ this.instance.unbind(EVENT_CONNECTION_ABORT, this.onConnectionDragAbortDetached); this.instance.unbind(EVENT_CONNECTION_DETACHED, this.onConnectionDragAbortDetached); this.instance.unbind(EVENT_PLUS_ENDPOINT_CLICK, this.onPlusEndpointClick); - - // Get all the endpoints and unbind the events - const elements = this.instance.getManagedElements(); - for (const element of Object.values(elements)) { - const endpoints = element.endpoints; - for (const endpoint of endpoints || []) { - const endpointInstance = endpoint?.endpoint; - if (endpointInstance && endpointInstance.type === N8nPlusEndpointType) { - (endpointInstance as N8nPlusEndpoint).unbindEvents(); + }, + unbindEndpointEventListeners(bind = true) { + if (this.instance) { + // Get all the endpoints and unbind the events + const elements = this.instance.getManagedElements(); + for (const element of Object.values(elements)) { + const endpoints = element.endpoints; + for (const endpoint of endpoints || []) { + const endpointInstance = endpoint?.endpoint; + if (endpointInstance && endpointInstance.type === N8nPlusEndpointType) { + (endpointInstance as N8nPlusEndpoint).unbindEvents(); + } } } } @@ -3575,6 +3578,7 @@ export default defineComponent({ this.nodeCreatorStore.setShowScrim(false); // Reset nodes + this.unbindEndpointEventListeners(); this.deleteEveryEndpoint(); // Make sure that if there is a waiting test-webhook that it gets removed From 67092c0a1bf98ccc5ceadc3d582fac7bff2dc46c Mon Sep 17 00:00:00 2001 From: Omar Ajoue Date: Wed, 13 Sep 2023 09:56:58 +0200 Subject: [PATCH 20/23] fix: Account for nanoid workflow ids for subworkflow execute policy (#7094) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Github issue / Community forum post (link here to close automatically): Since the change to allow workflow IDs to become strings in Nano ID formats, this input broke. This PR allows all characters that comprise workflow IDs. --------- Co-authored-by: Iván Ovejero --- .../src/components/WorkflowSettings.vue | 19 ++- .../__tests__/WorkflowSettings.spec.ts | 152 ++++++++++++++++++ .../src/composables/useExternalHooks.ts | 2 +- 3 files changed, 162 insertions(+), 11 deletions(-) create mode 100644 packages/editor-ui/src/components/__tests__/WorkflowSettings.spec.ts diff --git a/packages/editor-ui/src/components/WorkflowSettings.vue b/packages/editor-ui/src/components/WorkflowSettings.vue index 8461107ac32f3..4a1ee8fe46735 100644 --- a/packages/editor-ui/src/components/WorkflowSettings.vue +++ b/packages/editor-ui/src/components/WorkflowSettings.vue @@ -67,7 +67,7 @@ -
+
{{ $locale.baseText('workflowSettings.callerPolicy') + ':' }} @@ -114,6 +114,7 @@ type="text" v-model="workflowSettings.callerIds" @update:modelValue="onCallerIdsInput" + data-test-id="workflow-caller-policy-workflow-ids" /> @@ -374,13 +375,11 @@ import { import type { WorkflowSettings } from 'n8n-workflow'; import { deepCopy } from 'n8n-workflow'; -import { - useWorkflowsStore, - useSettingsStore, - useRootStore, - useWorkflowsEEStore, - useUsersStore, -} from '@/stores'; +import { useSettingsStore } from '@/stores/settings.store'; +import { useUsersStore } from '@/stores/users.store'; +import { useRootStore } from '@/stores/n8nRoot.store'; +import { useWorkflowsEEStore } from '@/stores/workflows.ee.store'; +import { useWorkflowsStore } from '@/stores/workflows.store'; import { createEventBus } from 'n8n-design-system/utils'; export default defineComponent({ @@ -566,9 +565,9 @@ export default defineComponent({ }, methods: { onCallerIdsInput(str: string) { - this.workflowSettings.callerIds = /^[0-9,\s]+$/.test(str) + this.workflowSettings.callerIds = /^[a-zA-Z0-9,\s]+$/.test(str) ? str - : str.replace(/[^0-9,\s]/g, ''); + : str.replace(/[^a-zA-Z0-9,\s]/g, ''); }, closeDialog() { this.modalBus.emit('close'); diff --git a/packages/editor-ui/src/components/__tests__/WorkflowSettings.spec.ts b/packages/editor-ui/src/components/__tests__/WorkflowSettings.spec.ts new file mode 100644 index 0000000000000..ca12ecbd7ddd0 --- /dev/null +++ b/packages/editor-ui/src/components/__tests__/WorkflowSettings.spec.ts @@ -0,0 +1,152 @@ +import { createPinia, setActivePinia } from 'pinia'; +import WorkflowSettingsVue from '../WorkflowSettings.vue'; + +import { setupServer } from '@/__tests__/server'; +import { afterAll, beforeAll } from 'vitest'; +import { within, fireEvent } from '@testing-library/vue'; + +import { useWorkflowsStore } from '@/stores/workflows.store'; +import { useSettingsStore } from '@/stores/settings.store'; +import { useUIStore } from '@/stores/ui.store'; + +import { createComponentRenderer } from '@/__tests__/render'; +import { EnterpriseEditionFeature, WORKFLOW_SETTINGS_MODAL_KEY } from '@/constants'; + +import { nextTick } from 'vue'; + +let pinia: ReturnType; +let workflowsStore: ReturnType; +let settingsStore: ReturnType; +let uiStore: ReturnType; + +const createComponent = createComponentRenderer(WorkflowSettingsVue, { + global: { + stubs: ['n8n-tooltip'], + }, +}); + +describe('WorkflowSettingsVue', () => { + let server: ReturnType; + beforeAll(() => { + server = setupServer(); + }); + + beforeEach(async () => { + pinia = createPinia(); + setActivePinia(pinia); + + workflowsStore = useWorkflowsStore(); + settingsStore = useSettingsStore(); + uiStore = useUIStore(); + + await settingsStore.getSettings(); + + vi.spyOn(workflowsStore, 'workflowName', 'get').mockReturnValue('Test Workflow'); + vi.spyOn(workflowsStore, 'workflowId', 'get').mockReturnValue('1'); + vi.spyOn(workflowsStore, 'workflow', 'get').mockReturnValue({ + id: '1', + name: 'Test Workflow', + active: true, + nodes: [], + connections: {}, + createdAt: 1, + updatedAt: 1, + versionId: '123', + } as IWorkflowDb); + + uiStore.modals[WORKFLOW_SETTINGS_MODAL_KEY] = { + open: true, + }; + }); + + afterAll(() => { + server.shutdown(); + }); + + it('should render correctly', async () => { + settingsStore.settings.enterprise[EnterpriseEditionFeature.Sharing] = false; + const wrapper = createComponent({ pinia }); + await nextTick(); + expect(wrapper.getByTestId('workflow-settings-dialog')).toBeVisible(); + }); + + it('should not render workflow caller policy when sharing is not enabled', async () => { + settingsStore.settings.enterprise[EnterpriseEditionFeature.Sharing] = false; + const wrapper = createComponent({ pinia }); + + await nextTick(); + + expect( + within(wrapper.getByTestId('workflow-settings-dialog')).queryByTestId( + 'workflow-caller-policy', + ), + ).not.toBeInTheDocument(); + }); + + it('should render workflow caller policy when sharing is enabled', async () => { + settingsStore.settings.enterprise[EnterpriseEditionFeature.Sharing] = true; + const wrapper = createComponent({ pinia }); + + await nextTick(); + + expect(wrapper.getByTestId('workflow-caller-policy')).toBeVisible(); + }); + + it('should render list of workflows field when policy is set to workflowsFromAList', async () => { + settingsStore.settings.enterprise[EnterpriseEditionFeature.Sharing] = true; + const wrapper = createComponent({ pinia }); + + await nextTick(); + + await fireEvent.click(wrapper.getByTestId('workflow-caller-policy')); + console.log(window.document.querySelectorAll('.el-select-dropdown__item')[4].innerHTML); + await fireEvent.click(window.document.querySelectorAll('.el-select-dropdown__item')[4]); + + expect(wrapper.getByTestId('workflow-caller-policy-workflow-ids')).toBeVisible(); + }); + + it('should not remove valid workflow ID characters', async () => { + const validWorkflowList = '1234567890, abcde, efgh, 1234'; + + settingsStore.settings.enterprise[EnterpriseEditionFeature.Sharing] = true; + const wrapper = createComponent({ pinia }); + + await nextTick(); + + await fireEvent.click(wrapper.getByTestId('workflow-caller-policy')); + console.log(window.document.querySelectorAll('.el-select-dropdown__item')[4].innerHTML); + await fireEvent.click(window.document.querySelectorAll('.el-select-dropdown__item')[4]); + + await fireEvent.update( + wrapper.getByTestId('workflow-caller-policy-workflow-ids'), + validWorkflowList, + ); + + expect(wrapper.getByTestId('workflow-caller-policy-workflow-ids')).toHaveValue( + validWorkflowList, + ); + }); + + it('should remove invalid workflow ID characters', async () => { + const invalidWorkflowList = '1234567890@, abc/de, ef*gh, 12%34'; + const cleanedUpWorkflowList = '1234567890, abcde, efgh, 1234'; + + settingsStore.settings.enterprise[EnterpriseEditionFeature.Sharing] = true; + const wrapper = createComponent({ pinia }); + + await nextTick(); + + await fireEvent.click(wrapper.getByTestId('workflow-caller-policy')); + console.log(window.document.querySelectorAll('.el-select-dropdown__item')[4].innerHTML); + await fireEvent.click(window.document.querySelectorAll('.el-select-dropdown__item')[4]); + + await fireEvent.update( + wrapper.getByTestId('workflow-caller-policy-workflow-ids'), + invalidWorkflowList, + ); + + expect(wrapper.getByTestId('workflow-caller-policy-workflow-ids')).toHaveValue( + cleanedUpWorkflowList, + ); + }); +}); diff --git a/packages/editor-ui/src/composables/useExternalHooks.ts b/packages/editor-ui/src/composables/useExternalHooks.ts index e492de00c6950..bc37d97456f62 100644 --- a/packages/editor-ui/src/composables/useExternalHooks.ts +++ b/packages/editor-ui/src/composables/useExternalHooks.ts @@ -1,6 +1,6 @@ import type { IExternalHooks } from '@/Interface'; import type { IDataObject } from 'n8n-workflow'; -import { useWebhooksStore } from '@/stores'; +import { useWebhooksStore } from '@/stores/webhooks.store'; import { runExternalHook } from '@/utils'; export function useExternalHooks(): IExternalHooks { From 217de21605beca57f087921231ae929279071686 Mon Sep 17 00:00:00 2001 From: Csaba Tuncsik Date: Wed, 13 Sep 2023 12:21:26 +0200 Subject: [PATCH 21/23] fix(editor): Tweak hover area of workflow / cred cards (#7108) Context When a user is attempting to interact with a foreground action inside an entity card (workflow, credential, community node, logging destination), they might accidentally open that entity instead of interacting with a foreground action. For these card components, actions are always placed on right side. A/C Area around right "column" of entity cards (workflow, cred, community node, logging destination) should not be a hoverable area (that opens that entity when clicked). This area is roughly highlighted in screen shot below in orange. ![image](https://github.com/n8n-io/n8n/assets/5410822/0916bcd5-e972-4367-a862-41d2086a2334) --- .../src/components/N8nCard/Card.vue | 9 +++- .../src/components/CredentialCard.vue | 54 +++++++++++++------ .../EventDestinationCard.ee.vue | 27 +++++----- .../editor-ui/src/components/WorkflowCard.vue | 39 ++++++++------ 4 files changed, 82 insertions(+), 47 deletions(-) diff --git a/packages/design-system/src/components/N8nCard/Card.vue b/packages/design-system/src/components/N8nCard/Card.vue index 4d15d83a42a0c..a922f782280a8 100644 --- a/packages/design-system/src/components/N8nCard/Card.vue +++ b/packages/design-system/src/components/N8nCard/Card.vue @@ -14,7 +14,7 @@
-
+
@@ -82,7 +82,6 @@ export default defineComponent({ .icon { width: 24px; - height: 24px; display: inline-flex; justify-content: center; align-items: center; @@ -101,4 +100,10 @@ export default defineComponent({ border-color: var(--color-primary); } } + +.append { + display: flex; + align-items: center; + cursor: default; +} diff --git a/packages/editor-ui/src/components/CredentialCard.vue b/packages/editor-ui/src/components/CredentialCard.vue index e96e49ce3416f..80ba4b5498f9a 100644 --- a/packages/editor-ui/src/components/CredentialCard.vue +++ b/packages/editor-ui/src/components/CredentialCard.vue @@ -1,24 +1,26 @@