From 4e9490a88d4c6a97414c438ea6ae45604c99c3b0 Mon Sep 17 00:00:00 2001 From: ricardo Date: Sat, 30 May 2020 19:03:58 -0400 Subject: [PATCH] :zap: Improvements --- packages/cli/src/ActiveWorkflowRunner.ts | 21 +++++++++++++++---- packages/cli/src/Interfaces.ts | 1 - packages/cli/src/TestWebhooks.ts | 4 +++- .../src/databases/mongodb/WebhookEntity.ts | 12 +++-------- .../src/databases/mysqldb/WebhookEntity.ts | 11 +++------- .../src/databases/postgresdb/WebhookEntity.ts | 11 +++------- .../1587669153312-InitialMigration.ts | 3 ++- .../migrations/1589476000887-WebhookModel.ts | 19 +++++++++++++---- .../cli/src/databases/sqlite/WebhookEntity.ts | 11 +++------- packages/core/src/ActiveWebhooks.ts | 9 +++++++- packages/editor-ui/src/views/NodeView.vue | 5 +++++ 11 files changed, 62 insertions(+), 45 deletions(-) diff --git a/packages/cli/src/ActiveWorkflowRunner.ts b/packages/cli/src/ActiveWorkflowRunner.ts index 4be2326323..a72cb065c7 100644 --- a/packages/cli/src/ActiveWorkflowRunner.ts +++ b/packages/cli/src/ActiveWorkflowRunner.ts @@ -241,13 +241,26 @@ export class ActiveWorkflowRunner { } } catch (error) { - // The workflow was saved with two webhooks with the - // same path/method so delete all webhooks saved + + let errorMessage = ''; await Db.collections.Webhook?.delete({ workflowId: workflow.id }); - // then show error to the user - throw new Error(error.message || error.detail); + // if it's a workflow from the the insert + // TODO check if there is standard error code for deplicate key violation that works + // with all databases + if (error.name === 'QueryFailedError') { + + errorMessage = `The webhook path [${webhook.webhookPath}] and method [${webhook.method}] already exist.`; + + } else if (error.detail) { + // it's a error runnig the webhook methods (checkExists, create) + errorMessage = error.detail; + } else { + errorMessage = error; + } + + throw new Error(errorMessage); } } // Save static data! diff --git a/packages/cli/src/Interfaces.ts b/packages/cli/src/Interfaces.ts index 65b211b2a9..fd56ad3ada 100644 --- a/packages/cli/src/Interfaces.ts +++ b/packages/cli/src/Interfaces.ts @@ -44,7 +44,6 @@ export interface IDatabaseCollections { } export interface IWebhookDb { - id?: number | ObjectID; workflowId: number | string | ObjectID; webhookPath: string; method: string; diff --git a/packages/cli/src/TestWebhooks.ts b/packages/cli/src/TestWebhooks.ts index 45ae624e2b..a20fa76c88 100644 --- a/packages/cli/src/TestWebhooks.ts +++ b/packages/cli/src/TestWebhooks.ts @@ -141,12 +141,14 @@ export class TestWebhooks { let key: string; for (const webhookData of webhooks) { key = this.activeWebhooks!.getWebhookKey(webhookData.httpMethod, webhookData.path); + + await this.activeWebhooks!.add(workflow, webhookData, mode); + this.testWebhookData[key] = { sessionId, timeout, workflowData, }; - await this.activeWebhooks!.add(workflow, webhookData, mode); // Save static data! this.testWebhookData[key].workflowData.staticData = workflow.staticData; diff --git a/packages/cli/src/databases/mongodb/WebhookEntity.ts b/packages/cli/src/databases/mongodb/WebhookEntity.ts index c9952c2c6d..a78fd34ae9 100644 --- a/packages/cli/src/databases/mongodb/WebhookEntity.ts +++ b/packages/cli/src/databases/mongodb/WebhookEntity.ts @@ -1,9 +1,7 @@ import { Column, Entity, - Unique, - ObjectIdColumn, - ObjectID, + PrimaryColumn, } from 'typeorm'; import { @@ -11,19 +9,15 @@ import { } from '../../Interfaces'; @Entity() -@Unique(['webhookPath', 'method']) export class WebhookEntity implements IWebhookDb { - @ObjectIdColumn() - id: ObjectID; - @Column() workflowId: number; - @Column() + @PrimaryColumn() webhookPath: string; - @Column() + @PrimaryColumn() method: string; @Column() diff --git a/packages/cli/src/databases/mysqldb/WebhookEntity.ts b/packages/cli/src/databases/mysqldb/WebhookEntity.ts index df89da4319..a78fd34ae9 100644 --- a/packages/cli/src/databases/mysqldb/WebhookEntity.ts +++ b/packages/cli/src/databases/mysqldb/WebhookEntity.ts @@ -1,8 +1,7 @@ import { Column, Entity, - Unique, - PrimaryGeneratedColumn, + PrimaryColumn, } from 'typeorm'; import { @@ -10,19 +9,15 @@ import { } from '../../Interfaces'; @Entity() -@Unique(['webhookPath', 'method']) export class WebhookEntity implements IWebhookDb { - @PrimaryGeneratedColumn() - id: number; - @Column() workflowId: number; - @Column() + @PrimaryColumn() webhookPath: string; - @Column() + @PrimaryColumn() method: string; @Column() diff --git a/packages/cli/src/databases/postgresdb/WebhookEntity.ts b/packages/cli/src/databases/postgresdb/WebhookEntity.ts index 07c1c88ae8..6e511cde74 100644 --- a/packages/cli/src/databases/postgresdb/WebhookEntity.ts +++ b/packages/cli/src/databases/postgresdb/WebhookEntity.ts @@ -1,8 +1,7 @@ import { Column, Entity, - Unique, - PrimaryGeneratedColumn, + PrimaryColumn, } from 'typeorm'; import { @@ -10,19 +9,15 @@ import { } from '../../'; @Entity() -@Unique(['webhookPath', 'method']) export class WebhookEntity implements IWebhookDb { - @PrimaryGeneratedColumn() - id: number; - @Column() workflowId: number; - @Column() + @PrimaryColumn() webhookPath: string; - @Column() + @PrimaryColumn() method: string; @Column() diff --git a/packages/cli/src/databases/postgresdb/migrations/1587669153312-InitialMigration.ts b/packages/cli/src/databases/postgresdb/migrations/1587669153312-InitialMigration.ts index 555015c10d..5f3798ca66 100644 --- a/packages/cli/src/databases/postgresdb/migrations/1587669153312-InitialMigration.ts +++ b/packages/cli/src/databases/postgresdb/migrations/1587669153312-InitialMigration.ts @@ -1,4 +1,5 @@ -import { MigrationInterface, QueryRunner } from 'typeorm'; +import { + MigrationInterface, QueryRunner } from 'typeorm'; import * as config from '../../../../config'; diff --git a/packages/cli/src/databases/postgresdb/migrations/1589476000887-WebhookModel.ts b/packages/cli/src/databases/postgresdb/migrations/1589476000887-WebhookModel.ts index 0e8ed1e76a..a521d227d0 100644 --- a/packages/cli/src/databases/postgresdb/migrations/1589476000887-WebhookModel.ts +++ b/packages/cli/src/databases/postgresdb/migrations/1589476000887-WebhookModel.ts @@ -1,7 +1,18 @@ -import {MigrationInterface, QueryRunner} from 'typeorm'; +import { + MigrationInterface, + QueryRunner, +} from 'typeorm'; + +import { + IWorkflowDb, + NodeTypes, + WebhookHelpers, + } from '../../..'; + +import { + Workflow, +} from 'n8n-workflow/dist/src/Workflow'; -import { IWorkflowDb, NodeTypes, WebhookHelpers } from '../../..'; -import { Workflow } from 'n8n-workflow/dist/src/Workflow'; import { IWebhookDb, } from '../../../Interfaces'; @@ -18,7 +29,7 @@ export class WebhookModel1589476000887 implements MigrationInterface { tablePrefix = schema + '.' + tablePrefix; } - await queryRunner.query(`CREATE TABLE ${tablePrefix}webhook_entity ("id" SERIAL NOT NULL, "workflowId" integer NOT NULL, "webhookPath" character varying NOT NULL, "method" character varying NOT NULL, "node" character varying NOT NULL, CONSTRAINT "UQ_b21ace2e13596ccd87dc9bf4ea6" UNIQUE ("webhookPath", "method"), CONSTRAINT "PK_202217c8b912cf70b93b1e87256" PRIMARY KEY ("id"))`, undefined); + await queryRunner.query(`CREATE TABLE ${tablePrefix}webhook_entity ("workflowId" integer NOT NULL, "webhookPath" character varying NOT NULL, "method" character varying NOT NULL, "node" character varying NOT NULL, CONSTRAINT "PK_b21ace2e13596ccd87dc9bf4ea6" PRIMARY KEY ("webhookPath", "method"))`, undefined); const workflows = await queryRunner.query(`SELECT * FROM ${tablePrefix}workflow_entity WHERE active=true`) as IWorkflowDb[]; const data: IWebhookDb[] = []; diff --git a/packages/cli/src/databases/sqlite/WebhookEntity.ts b/packages/cli/src/databases/sqlite/WebhookEntity.ts index df89da4319..a78fd34ae9 100644 --- a/packages/cli/src/databases/sqlite/WebhookEntity.ts +++ b/packages/cli/src/databases/sqlite/WebhookEntity.ts @@ -1,8 +1,7 @@ import { Column, Entity, - Unique, - PrimaryGeneratedColumn, + PrimaryColumn, } from 'typeorm'; import { @@ -10,19 +9,15 @@ import { } from '../../Interfaces'; @Entity() -@Unique(['webhookPath', 'method']) export class WebhookEntity implements IWebhookDb { - @PrimaryGeneratedColumn() - id: number; - @Column() workflowId: number; - @Column() + @PrimaryColumn() webhookPath: string; - @Column() + @PrimaryColumn() method: string; @Column() diff --git a/packages/core/src/ActiveWebhooks.ts b/packages/core/src/ActiveWebhooks.ts index 17cf753830..7f6d705ccd 100644 --- a/packages/core/src/ActiveWebhooks.ts +++ b/packages/core/src/ActiveWebhooks.ts @@ -35,13 +35,20 @@ export class ActiveWebhooks { throw new Error('Webhooks can only be added for saved workflows as an id is needed!'); } + const webhookKey = this.getWebhookKey(webhookData.httpMethod, webhookData.path); + + //check that there is not a webhook already registed with that path/method + if (this.webhookUrls[webhookKey] !== undefined) { + throw new Error('There is test wenhook registered on that path'); + } + if (this.workflowWebhooks[webhookData.workflowId] === undefined) { this.workflowWebhooks[webhookData.workflowId] = []; } // Make the webhook available directly because sometimes to create it successfully // it gets called - this.webhookUrls[this.getWebhookKey(webhookData.httpMethod, webhookData.path)] = webhookData; + this.webhookUrls[webhookKey] = webhookData; const webhookExists = await workflow.runWebhookMethod('checkExists', webhookData, NodeExecuteFunctions, mode, this.testWebhooks); if (webhookExists === false) { diff --git a/packages/editor-ui/src/views/NodeView.vue b/packages/editor-ui/src/views/NodeView.vue index 2a86c1be12..65ff1b5267 100644 --- a/packages/editor-ui/src/views/NodeView.vue +++ b/packages/editor-ui/src/views/NodeView.vue @@ -1585,6 +1585,11 @@ export default mixins( console.error(e); // eslint-disable-line no-console } node.parameters = nodeParameters !== null ? nodeParameters : {}; + + // if it's a webhook and the path is empty set the UUID as the default path + if (node.type === 'n8n-nodes-base.webhook' && node.parameters.path === '') { + node.parameters.path = node.webhookPath as string; + } } foundNodeIssues = this.getNodeIssues(nodeType, node);