feat(core): reimplement blocking workflow updates on interim changes (#…

…4446) * 📘 Update request type * 📘 Update FE types * ⚡ Adjust store * ⚡ Set received hash * ⚡ Send and load hash * ⚡ Make helper more flexible * 🗃️ Add new field to entity * 🚨 Add check to endpoint * 🧪 Add tests * ⚡ Add `forceSave` flag * 🐛 Fix workflow update failing on new workflow * 🧪 Add more tests * ⚡ Move check to `updateWorkflow()` * ⚡ Refactor to accommodate latest changes * 🧪 Refactor tests to keep them passing * ⚡ Improve syntax
n8n-io · Oct 31, 2022 · 46905fd · 46905fd
1 parent 263e6f3
commit 46905fd
Show file tree

Hide file tree

Showing 12 changed files with 394 additions and 44 deletions.
diff --git a/packages/cli/src/databases/entities/WorkflowEntity.ts b/packages/cli/src/databases/entities/WorkflowEntity.ts
@@ -1,3 +1,4 @@
+import crypto from 'crypto';
 import { Length } from 'class-validator';
 
 import type {
@@ -10,6 +11,9 @@ import type {
 } from 'n8n-workflow';
 
 import {
+	AfterLoad,
+	AfterUpdate,
+	AfterInsert,
 	Column,
 	Entity,
 	Index,
@@ -84,6 +88,30 @@ export class WorkflowEntity extends AbstractEntity implements IWorkflowDb {
 		transformer: sqlite.jsonColumn,
 	})
 	pinData: ISimplifiedPinData;
+
+	/**
+	 * Hash of editable workflow state.
+	 */
+	hash: string;
+
+	@AfterLoad()
+	@AfterUpdate()
+	@AfterInsert()
+	setHash(): void {
+		const { name, active, nodes, connections, settings, staticData, pinData } = this;
+
+		const state = JSON.stringify({
+			name,
+			active,
+			nodes,
+			connections,
+			settings,
+			staticData,
+			pinData,
+		});
+
+		this.hash = crypto.createHash('md5').update(state).digest('hex');
+	}
 }
 
 /**

diff --git a/packages/cli/src/requests.d.ts b/packages/cli/src/requests.d.ts
@@ -48,6 +48,7 @@ export declare namespace WorkflowRequest {
 		settings: IWorkflowSettings;
 		active: boolean;
 		tags: string[];
+		hash: string;
 	}>;
 
 	type Create = AuthenticatedRequest<{}, {}, RequestBody>;
@@ -56,7 +57,7 @@ export declare namespace WorkflowRequest {
 
 	type Delete = Get;
 
-	type Update = AuthenticatedRequest<{ id: string }, {}, RequestBody>;
+	type Update = AuthenticatedRequest<{ id: string }, {}, RequestBody, { forceSave?: string }>;
 
 	type NewName = AuthenticatedRequest<{}, {}, {}, { name?: string }>;
 

diff --git a/packages/cli/src/workflows/workflows.controller.ee.ts b/packages/cli/src/workflows/workflows.controller.ee.ts
@@ -183,6 +183,7 @@ EEWorkflowController.patch(
 	'/:id(\\d+)',
 	ResponseHelper.send(async (req: WorkflowRequest.Update) => {
 		const { id: workflowId } = req.params;
+		const forceSave = req.query.forceSave === 'true';
 
 		const updateData = new WorkflowEntity();
 		const { tags, ...rest } = req.body;
@@ -193,6 +194,7 @@ EEWorkflowController.patch(
 			updateData,
 			workflowId,
 			tags,
+			forceSave,
 		);
 
 		const { id, ...remainder } = updatedWorkflow;

diff --git a/packages/cli/src/workflows/workflows.services.ee.ts b/packages/cli/src/workflows/workflows.services.ee.ts
@@ -121,27 +121,28 @@ export class EEWorkflowsService extends WorkflowsService {
 		workflow: WorkflowEntity,
 		workflowId: string,
 		tags?: string[],
+		forceSave?: boolean,
 	): Promise<WorkflowEntity> {
 		const previousVersion = await EEWorkflowsService.get({ id: parseInt(workflowId, 10) });
 		if (!previousVersion) {
 			throw new ResponseHelper.ResponseError('Workflow not found', undefined, 404);
 		}
 		const allCredentials = await EECredentials.getAll(user);
 		try {
+			// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment, @typescript-eslint/no-unsafe-call
 			workflow = WorkflowHelpers.validateWorkflowCredentialUsage(
 				workflow,
 				previousVersion,
 				allCredentials,
 			);
 		} catch (error) {
-			console.log(error);
 			throw new ResponseHelper.ResponseError(
 				'Invalid workflow credentials - make sure you have access to all credentials and try again.',
 				undefined,
 				400,
 			);
 		}
 
-		return super.updateWorkflow(user, workflow, workflowId, tags);
+		return super.updateWorkflow(user, workflow, workflowId, tags, forceSave);
 	}
 }
diff --git a/packages/cli/src/workflows/workflows.services.ts b/packages/cli/src/workflows/workflows.services.ts
@@ -52,6 +52,7 @@ export class WorkflowsService {
 		workflow: WorkflowEntity,
 		workflowId: string,
 		tags?: string[],
+		forceSave?: boolean,
 	): Promise<WorkflowEntity> {
 		const shared = await Db.collections.SharedWorkflow.findOne({
 			relations: ['workflow'],
@@ -74,6 +75,14 @@ export class WorkflowsService {
 			);
 		}
 
+		if (!forceSave && workflow.hash !== shared.workflow.hash) {
+			throw new ResponseHelper.ResponseError(
+				`Workflow ID ${workflowId} cannot be saved because it was changed by another user.`,
+				undefined,
+				400,
+			);
+		}
+
 		// check credentials for old format
 		await WorkflowHelpers.replaceInvalidCredentials(workflow);
 
@@ -118,7 +127,9 @@ export class WorkflowsService {
 			await validateEntity(workflow);
 		}
 
-		await Db.collections.Workflow.update(workflowId, workflow);
+		const { hash, ...rest } = workflow;
+
+		await Db.collections.Workflow.update(workflowId, rest);
 
 		if (tags && !config.getEnv('workflowTagsDisabled')) {
 			const tablePrefix = config.getEnv('database.tablePrefix');

diff --git a/packages/cli/test/integration/shared/utils.ts b/packages/cli/test/integration/shared/utils.ts
@@ -706,27 +706,24 @@ export const emptyPackage = () => {
 //           workflow
 // ----------------------------------
 
-export function makeWorkflow({
-	withPinData,
-	withCredential,
-}: {
+export function makeWorkflow(options?: {
 	withPinData: boolean;
 	withCredential?: { id: string; name: string };
 }) {
 	const workflow = new WorkflowEntity();
 
 	const node: INode = {
 		id: uuid(),
-		name: 'Spotify',
-		type: 'n8n-nodes-base.spotify',
-		parameters: { resource: 'track', operation: 'get', id: '123' },
+		name: 'Cron',
+		type: 'n8n-nodes-base.cron',
+		parameters: {},
 		typeVersion: 1,
 		position: [740, 240],
 	};
 
-	if (withCredential) {
+	if (options?.withCredential) {
 		node.credentials = {
-			spotifyApi: withCredential,
+			spotifyApi: options.withCredential,
 		};
 	}
 
@@ -735,7 +732,7 @@ export function makeWorkflow({
 	workflow.connections = {};
 	workflow.nodes = [node];
 
-	if (withPinData) {
+	if (options?.withPinData) {
 		workflow.pinData = MOCK_PINDATA;
 	}