refactor: Reactivate workflow locking (#4770)

* feat: Reenable workflow locking Co-authored-by: freyamade <[email protected]> Co-authored-by: Csaba Tuncsik <[email protected]>
n8n-io · Dec 6, 2022 · 4813da5 · 4813da5
1 parent 915f144
commit 4813da5
Show file tree

Hide file tree

Showing 19 changed files with 249 additions and 118 deletions.
diff --git a/packages/cli/src/ResponseHelper.ts b/packages/cli/src/ResponseHelper.ts
@@ -41,8 +41,8 @@ abstract class ResponseError extends Error {
 }
 
 export class BadRequestError extends ResponseError {
-	constructor(message: string) {
-		super(message, 400);
+	constructor(message: string, errorCode?: number) {
+		super(message, 400, errorCode);
 	}
 }
 

diff --git a/packages/cli/src/databases/entities/WorkflowEntity.ts b/packages/cli/src/databases/entities/WorkflowEntity.ts
@@ -1,4 +1,3 @@
-import crypto from 'crypto';
 import { Length } from 'class-validator';
 
 import type {
@@ -11,9 +10,6 @@ import type {
 } from 'n8n-workflow';
 
 import {
-	AfterLoad,
-	AfterUpdate,
-	AfterInsert,
 	Column,
 	Entity,
 	Index,
@@ -29,7 +25,6 @@ import { SharedWorkflow } from './SharedWorkflow';
 import { objectRetriever, sqlite } from '../utils/transformers';
 import { AbstractEntity, jsonColumnType } from './AbstractEntity';
 import type { IWorkflowDb } from '@/Interfaces';
-import { alphabetizeKeys } from '@/utils';
 
 @Entity()
 export class WorkflowEntity extends AbstractEntity implements IWorkflowDb {
@@ -90,32 +85,8 @@ export class WorkflowEntity extends AbstractEntity implements IWorkflowDb {
 	})
 	pinData: ISimplifiedPinData;
 
-	/**
-	 * Hash of editable workflow state.
-	 */
-	hash: string;
-
-	@AfterLoad()
-	@AfterUpdate()
-	@AfterInsert()
-	setHash(): void {
-		const { name, active, nodes, connections, settings, staticData, pinData } = this;
-
-		// Workflow listing page does not request the `connections` column, so we can use this for `undefined` to avoid generating hashes for all the workflows.
-		if (!connections) return;
-
-		const state = JSON.stringify({
-			name,
-			active,
-			nodes: nodes ? nodes.map(alphabetizeKeys) : [],
-			connections,
-			settings,
-			staticData,
-			pinData,
-		});
-
-		this.hash = crypto.createHash('md5').update(state).digest('hex');
-	}
+	@Column({ length: 36 })
+	versionId: string;
 }
 
 /**

diff --git a/packages/cli/src/databases/migrations/mysqldb/1669739707125-AddWorkflowVersionIdColumn.ts b/packages/cli/src/databases/migrations/mysqldb/1669739707125-AddWorkflowVersionIdColumn.ts
@@ -0,0 +1,45 @@
+import { MigrationInterface, QueryRunner } from 'typeorm';
+import { logMigrationEnd, logMigrationStart } from '@db/utils/migrationHelpers';
+import config from '@/config';
+import { v4 as uuidv4 } from 'uuid';
+
+export class AddWorkflowVersionIdColumn1669739707125 implements MigrationInterface {
+	name = 'AddWorkflowVersionIdColumn1669739707125';
+
+	async up(queryRunner: QueryRunner): Promise<void> {
+		logMigrationStart(this.name);
+
+		const tablePrefix = config.getEnv('database.tablePrefix');
+
+		await queryRunner.query(
+			`ALTER TABLE ${tablePrefix}workflow_entity ADD COLUMN versionId CHAR(36)`,
+		);
+
+		const workflowIds: Array<{ id: number }> = await queryRunner.query(`
+			SELECT id
+			FROM ${tablePrefix}workflow_entity
+		`);
+
+		workflowIds.map(({ id }) => {
+			const [updateQuery, updateParams] = queryRunner.connection.driver.escapeQueryWithParameters(
+				`
+					UPDATE ${tablePrefix}workflow_entity
+					SET versionId = :versionId
+					WHERE id = '${id}'
+				`,
+				{ versionId: uuidv4() },
+				{},
+			);
+
+			return queryRunner.query(updateQuery, updateParams);
+		});
+
+		logMigrationEnd(this.name);
+	}
+
+	async down(queryRunner: QueryRunner) {
+		const tablePrefix = config.getEnv('database.tablePrefix');
+
+		await queryRunner.query(`ALTER TABLE ${tablePrefix}workflow_entity DROP COLUMN versionId`);
+	}
+}
diff --git a/packages/cli/src/databases/migrations/mysqldb/index.ts b/packages/cli/src/databases/migrations/mysqldb/index.ts
@@ -23,6 +23,7 @@ import { CreateCredentialsUserRole1660062385367 } from './1660062385367-CreateCr
 import { CreateWorkflowsEditorRole1663755770894 } from './1663755770894-CreateWorkflowsEditorRole';
 import { CreateCredentialUsageTable1665484192213 } from './1665484192213-CreateCredentialUsageTable';
 import { RemoveCredentialUsageTable1665754637026 } from './1665754637026-RemoveCredentialUsageTable';
+import { AddWorkflowVersionIdColumn1669739707125 } from './1669739707125-AddWorkflowVersionIdColumn';
 
 export const mysqlMigrations = [
 	InitialMigration1588157391238,
@@ -50,4 +51,5 @@ export const mysqlMigrations = [
 	CreateWorkflowsEditorRole1663755770894,
 	CreateCredentialUsageTable1665484192213,
 	RemoveCredentialUsageTable1665754637026,
+	AddWorkflowVersionIdColumn1669739707125,
 ];
diff --git a/packages/cli/src/databases/migrations/postgresdb/1669739707126-AddWorkflowVersionIdColumn.ts b/packages/cli/src/databases/migrations/postgresdb/1669739707126-AddWorkflowVersionIdColumn.ts
@@ -0,0 +1,44 @@
+import { MigrationInterface, QueryRunner } from 'typeorm';
+import { getTablePrefix, logMigrationEnd, logMigrationStart } from '@db/utils/migrationHelpers';
+import config from '@/config';
+import { v4 as uuidv4 } from 'uuid';
+
+export class AddWorkflowVersionIdColumn1669739707126 implements MigrationInterface {
+	name = 'AddWorkflowVersionIdColumn1669739707126';
+
+	async up(queryRunner: QueryRunner) {
+		logMigrationStart(this.name);
+
+		const tablePrefix = getTablePrefix();
+		await queryRunner.query(
+			`ALTER TABLE ${tablePrefix}workflow_entity ADD COLUMN "versionId" CHAR(36)`,
+		);
+
+		const workflowIds: Array<{ id: number }> = await queryRunner.query(`
+			SELECT id
+			FROM ${tablePrefix}workflow_entity
+		`);
+
+		workflowIds.map(({ id }) => {
+			const [updateQuery, updateParams] = queryRunner.connection.driver.escapeQueryWithParameters(
+				`
+					UPDATE ${tablePrefix}workflow_entity
+					SET "versionId" = :versionId
+					WHERE id = '${id}'
+				`,
+				{ versionId: uuidv4() },
+				{},
+			);
+
+			return queryRunner.query(updateQuery, updateParams);
+		});
+
+		logMigrationEnd(this.name);
+	}
+
+	async down(queryRunner: QueryRunner) {
+		const tablePrefix = config.getEnv('database.tablePrefix');
+
+		await queryRunner.query(`ALTER TABLE ${tablePrefix}workflow_entity DROP COLUMN "versionId"`);
+	}
+}
diff --git a/packages/cli/src/databases/migrations/postgresdb/index.ts b/packages/cli/src/databases/migrations/postgresdb/index.ts
@@ -21,6 +21,7 @@ import { CreateCredentialsUserRole1660062385367 } from './1660062385367-CreateCr
 import { CreateWorkflowsEditorRole1663755770893 } from './1663755770893-CreateWorkflowsEditorRole';
 import { CreateCredentialUsageTable1665484192212 } from './1665484192212-CreateCredentialUsageTable';
 import { RemoveCredentialUsageTable1665754637025 } from './1665754637025-RemoveCredentialUsageTable';
+import { AddWorkflowVersionIdColumn1669739707126 } from './1669739707126-AddWorkflowVersionIdColumn';
 
 export const postgresMigrations = [
 	InitialMigration1587669153312,
@@ -46,4 +47,5 @@ export const postgresMigrations = [
 	CreateWorkflowsEditorRole1663755770893,
 	CreateCredentialUsageTable1665484192212,
 	RemoveCredentialUsageTable1665754637025,
+	AddWorkflowVersionIdColumn1669739707126,
 ];
diff --git a/packages/cli/src/databases/migrations/sqlite/1669739707124-AddWorkflowVersionIdColumn.ts b/packages/cli/src/databases/migrations/sqlite/1669739707124-AddWorkflowVersionIdColumn.ts
@@ -0,0 +1,47 @@
+import { MigrationInterface, QueryRunner } from 'typeorm';
+import { logMigrationEnd, logMigrationStart } from '@db/utils/migrationHelpers';
+import config from '@/config';
+import { v4 as uuidv4 } from 'uuid';
+
+export class AddWorkflowVersionIdColumn1669739707124 implements MigrationInterface {
+	name = 'AddWorkflowVersionIdColumn1669739707124';
+
+	async up(queryRunner: QueryRunner) {
+		logMigrationStart(this.name);
+
+		const tablePrefix = config.getEnv('database.tablePrefix');
+
+		await queryRunner.query(
+			`ALTER TABLE \`${tablePrefix}workflow_entity\` ADD COLUMN "versionId" char(36)`,
+		);
+
+		const workflowIds: Array<{ id: number }> = await queryRunner.query(`
+			SELECT id
+			FROM "${tablePrefix}workflow_entity"
+		`);
+
+		workflowIds.map(({ id }) => {
+			const [updateQuery, updateParams] = queryRunner.connection.driver.escapeQueryWithParameters(
+				`
+					UPDATE "${tablePrefix}workflow_entity"
+					SET versionId = :versionId
+					WHERE id = '${id}'
+				`,
+				{ versionId: uuidv4() },
+				{},
+			);
+
+			return queryRunner.query(updateQuery, updateParams);
+		});
+
+		logMigrationEnd(this.name);
+	}
+
+	async down(queryRunner: QueryRunner) {
+		const tablePrefix = config.getEnv('database.tablePrefix');
+
+		await queryRunner.query(
+			`ALTER TABLE \`${tablePrefix}workflow_entity\` DROP COLUMN "versionId"`,
+		);
+	}
+}
diff --git a/packages/cli/src/databases/migrations/sqlite/index.ts b/packages/cli/src/databases/migrations/sqlite/index.ts
@@ -20,6 +20,7 @@ import { CreateCredentialsUserRole1660062385367 } from './1660062385367-CreateCr
 import { CreateWorkflowsEditorRole1663755770892 } from './1663755770892-CreateWorkflowsUserRole';
 import { CreateCredentialUsageTable1665484192211 } from './1665484192211-CreateCredentialUsageTable';
 import { RemoveCredentialUsageTable1665754637024 } from './1665754637024-RemoveCredentialUsageTable';
+import { AddWorkflowVersionIdColumn1669739707124 } from './1669739707124-AddWorkflowVersionIdColumn';
 
 const sqliteMigrations = [
 	InitialMigration1588102412422,
@@ -44,6 +45,7 @@ const sqliteMigrations = [
 	CreateWorkflowsEditorRole1663755770892,
 	CreateCredentialUsageTable1665484192211,
 	RemoveCredentialUsageTable1665754637024,
+	AddWorkflowVersionIdColumn1669739707124,
 ];
 
 export { sqliteMigrations };
diff --git a/packages/cli/src/workflows/workflows.controller.ee.ts b/packages/cli/src/workflows/workflows.controller.ee.ts
@@ -1,4 +1,5 @@
 import express from 'express';
+import { v4 as uuid } from 'uuid';
 import * as Db from '@/Db';
 import { InternalHooksManager } from '@/InternalHooksManager';
 import * as ResponseHelper from '@/ResponseHelper';
@@ -112,6 +113,8 @@ EEWorkflowController.post(
 
 		Object.assign(newWorkflow, req.body);
 
+		newWorkflow.versionId = uuid();
+
 		await validateEntity(newWorkflow);
 
 		await externalHooks.run('workflow.create', [newWorkflow]);
@@ -213,7 +216,7 @@ EEWorkflowController.patch(
 	'/:id(\\d+)',
 	ResponseHelper.send(async (req: WorkflowRequest.Update) => {
 		const { id: workflowId } = req.params;
-		// const forceSave = req.query.forceSave === 'true'; // disabled temporarily - tests were also disabled
+		const forceSave = req.query.forceSave === 'true';
 
 		const updateData = new WorkflowEntity();
 		const { tags, ...rest } = req.body;
@@ -226,7 +229,7 @@ EEWorkflowController.patch(
 			safeWorkflow,
 			workflowId,
 			tags,
-			true,
+			forceSave,
 		);
 
 		const { id, ...remainder } = updatedWorkflow;

diff --git a/packages/cli/src/workflows/workflows.controller.ts b/packages/cli/src/workflows/workflows.controller.ts
@@ -1,6 +1,7 @@
 /* eslint-disable no-param-reassign */
 
 import express from 'express';
+import { v4 as uuid } from 'uuid';
 import { LoggerProxy } from 'n8n-workflow';
 
 import axios from 'axios';
@@ -52,6 +53,8 @@ workflowsController.post(
 
 		Object.assign(newWorkflow, req.body);
 
+		newWorkflow.versionId = uuid();
+
 		await validateEntity(newWorkflow);
 
 		await externalHooks.run('workflow.create', [newWorkflow]);

diff --git a/packages/cli/src/workflows/workflows.services.ts b/packages/cli/src/workflows/workflows.services.ts
@@ -2,6 +2,7 @@ import { validate as jsonSchemaValidate } from 'jsonschema';
 import { INode, IPinData, JsonObject, jsonParse, LoggerProxy, Workflow } from 'n8n-workflow';
 import { FindManyOptions, FindOneOptions, In, ObjectLiteral } from 'typeorm';
 import pick from 'lodash.pick';
+import { v4 as uuid } from 'uuid';
 import * as ActiveWorkflowRunner from '@/ActiveWorkflowRunner';
 import * as Db from '@/Db';
 import { InternalHooksManager } from '@/InternalHooksManager';
@@ -172,7 +173,7 @@ export class WorkflowsService {
 		}
 
 		const query: FindManyOptions<WorkflowEntity> = {
-			select: isSharingEnabled ? [...fields, 'nodes'] : fields,
+			select: isSharingEnabled ? [...fields, 'nodes', 'versionId'] : fields,
 			relations,
 			where: {
 				id: In(sharedWorkflowIds),
@@ -220,12 +221,28 @@ export class WorkflowsService {
 			);
 		}
 
-		if (!forceSave && workflow.hash !== '' && workflow.hash !== shared.workflow.hash) {
+		if (
+			!forceSave &&
+			workflow.versionId !== '' &&
+			workflow.versionId !== shared.workflow.versionId
+		) {
 			throw new ResponseHelper.BadRequestError(
 				'Your most recent changes may be lost, because someone else just updated this workflow. Open this workflow in a new tab to see those new updates.',
+				100,
 			);
 		}
 
+		// Update the workflow's version
+		workflow.versionId = uuid();
+
+		LoggerProxy.verbose(
+			`Updating versionId for workflow ${workflowId} for user ${user.id} after saving`,
+			{
+				previousVersionId: shared.workflow.versionId,
+				newVersionId: workflow.versionId,
+			},
+		);
+
 		// check credentials for old format
 		await WorkflowHelpers.replaceInvalidCredentials(workflow);
 
@@ -280,6 +297,7 @@ export class WorkflowsService {
 				'settings',
 				'staticData',
 				'pinData',
+				'versionId',
 			]),
 		);