fix(core): Remove typeorm patches, but still enforce transactions on every migration (#6594)

* revert(core): Remove typeorm patches, but still enforce transactions on every migration

This reverts #6519

* always re-enable foreign keys, and explicitly rollback transaction

package.json

@@ -95,8 +95,7 @@
       "element-ui@2.15.12": "patches/element-ui@2.15.12.patch",
       "typedi@0.10.0": "patches/typedi@0.10.0.patch",
       "@sentry/cli@2.17.0": "patches/@sentry__cli@2.17.0.patch",
-      "pkce-challenge@3.0.0": "patches/pkce-challenge@3.0.0.patch",
-      "typeorm@0.3.12": "patches/typeorm@0.3.12.patch"
+      "pkce-challenge@3.0.0": "patches/pkce-challenge@3.0.0.patch"
     }
   }
 }

packages/cli/src/databases/migrations/sqlite/1652367743993-AddUserSettings.ts

@@ -1,6 +1,8 @@
 import type { MigrationContext, ReversibleMigration } from '@db/types';
 
 export class AddUserSettings1652367743993 implements ReversibleMigration {
+	transaction = false as const;
+
 	async up({ queryRunner, tablePrefix }: MigrationContext) {
 		await queryRunner.query(
 			`CREATE TABLE "temporary_user" ("id" varchar PRIMARY KEY NOT NULL, "email" varchar(255), "firstName" varchar(32), "lastName" varchar(32), "password" varchar, "resetPasswordToken" varchar, "resetPasswordTokenExpiration" integer DEFAULT NULL, "personalizationAnswers" text, "createdAt" datetime(3) NOT NULL DEFAULT (STRFTIME('%Y-%m-%d %H:%M:%f', 'NOW')), "updatedAt" datetime(3) NOT NULL DEFAULT (STRFTIME('%Y-%m-%d %H:%M:%f', 'NOW')), "globalRoleId" integer NOT NULL, "settings" text, CONSTRAINT "FK_${tablePrefix}f0609be844f9200ff4365b1bb3d" FOREIGN KEY ("globalRoleId") REFERENCES "${tablePrefix}role" ("id") ON DELETE NO ACTION ON UPDATE NO ACTION)`,

packages/cli/src/databases/migrations/sqlite/1652905585850-AddAPIKeyColumn.ts

@@ -1,6 +1,8 @@
 import type { MigrationContext, ReversibleMigration } from '@db/types';
 
 export class AddAPIKeyColumn1652905585850 implements ReversibleMigration {
+	transaction = false as const;
+
 	async up({ queryRunner, tablePrefix }: MigrationContext) {
 		await queryRunner.query(
 			`CREATE TABLE "temporary_user" ("id" varchar PRIMARY KEY NOT NULL, "email" varchar(255), "firstName" varchar(32), "lastName" varchar(32), "password" varchar, "resetPasswordToken" varchar, "resetPasswordTokenExpiration" integer DEFAULT NULL, "personalizationAnswers" text, "createdAt" datetime(3) NOT NULL DEFAULT (STRFTIME('%Y-%m-%d %H:%M:%f', 'NOW')), "updatedAt" datetime(3) NOT NULL DEFAULT (STRFTIME('%Y-%m-%d %H:%M:%f', 'NOW')), "globalRoleId" integer NOT NULL, "settings" text, "apiKey" varchar, CONSTRAINT "FK_${tablePrefix}f0609be844f9200ff4365b1bb3d" FOREIGN KEY ("globalRoleId") REFERENCES "${tablePrefix}role" ("id") ON DELETE NO ACTION ON UPDATE NO ACTION)`,

packages/cli/src/databases/migrations/sqlite/1673268682475-DeleteExecutionsWithWorkflows.ts

@@ -1,6 +1,8 @@
 import type { MigrationContext, ReversibleMigration } from '@db/types';
 
 export class DeleteExecutionsWithWorkflows1673268682475 implements ReversibleMigration {
+	transaction = false as const;
+
 	async up({ queryRunner, tablePrefix }: MigrationContext) {
 		const workflowIds = (await queryRunner.query(`
 			SELECT id FROM "${tablePrefix}workflow_entity"

packages/cli/src/databases/migrations/sqlite/1690000000002-MigrateIntegerKeysToString.ts

@@ -1,18 +1,21 @@
 import type { MigrationContext, IrreversibleMigration } from '@db/types';
 
 export class MigrateIntegerKeysToString1690000000002 implements IrreversibleMigration {
+	transaction = false as const;
+
 	async up({ queryRunner, tablePrefix }: MigrationContext) {
 		await queryRunner.query(`
-CREATE TABLE "${tablePrefix}TMP_workflow_entity" ("id" varchar(36) PRIMARY KEY NOT NULL, "name" varchar(128) NOT NULL, "active" boolean NOT NULL, "nodes" text, "connections" text NOT NULL, "createdAt" datetime(3) NOT NULL DEFAULT (STRFTIME('%Y-%m-%d %H:%M:%f', 'NOW')), "updatedAt" datetime(3) NOT NULL DEFAULT (STRFTIME('%Y-%m-%d %H:%M:%f', 'NOW')), "settings" text, "staticData" text, "pinData" text, "versionId" varchar(36), "triggerCount" integer NOT NULL DEFAULT 0);`);
+			CREATE TABLE "${tablePrefix}TMP_workflow_entity" ("id" varchar(36) PRIMARY KEY NOT NULL, "name" varchar(128) NOT NULL, "active" boolean NOT NULL, "nodes" text, "connections" text NOT NULL, "createdAt" datetime(3) NOT NULL DEFAULT (STRFTIME('%Y-%m-%d %H:%M:%f', 'NOW')), "updatedAt" datetime(3) NOT NULL DEFAULT (STRFTIME('%Y-%m-%d %H:%M:%f', 'NOW')), "settings" text, "staticData" text, "pinData" text, "versionId" varchar(36), "triggerCount" integer NOT NULL DEFAULT 0);`);
 		await queryRunner.query(
 			`INSERT INTO "${tablePrefix}TMP_workflow_entity" (id, name, active, nodes, connections, createdAt, updatedAt, settings, staticData, pinData, triggerCount, versionId) SELECT id, name, active, nodes, connections, createdAt, updatedAt, settings, staticData, pinData, triggerCount, versionId FROM "${tablePrefix}workflow_entity";`,
 		);
 		await queryRunner.query(`DROP TABLE "${tablePrefix}workflow_entity";`);
-		await queryRunner.query(`ALTER TABLE "${tablePrefix}TMP_workflow_entity" RENAME TO "${tablePrefix}workflow_entity";
-`);
+		await queryRunner.query(
+			`ALTER TABLE "${tablePrefix}TMP_workflow_entity" RENAME TO "${tablePrefix}workflow_entity"`,
+		);
 
 		await queryRunner.query(`
-CREATE TABLE "${tablePrefix}TMP_tag_entity" ("id" varchar(36) PRIMARY KEY NOT NULL, "name" varchar(24) NOT NULL, "createdAt" datetime(3) NOT NULL DEFAULT (STRFTIME('%Y-%m-%d %H:%M:%f', 'NOW')), "updatedAt" datetime(3) NOT NULL DEFAULT (STRFTIME('%Y-%m-%d %H:%M:%f', 'NOW')));`);
+			CREATE TABLE "${tablePrefix}TMP_tag_entity" ("id" varchar(36) PRIMARY KEY NOT NULL, "name" varchar(24) NOT NULL, "createdAt" datetime(3) NOT NULL DEFAULT (STRFTIME('%Y-%m-%d %H:%M:%f', 'NOW')), "updatedAt" datetime(3) NOT NULL DEFAULT (STRFTIME('%Y-%m-%d %H:%M:%f', 'NOW')));`);
 		await queryRunner.query(
 			`INSERT INTO "${tablePrefix}TMP_tag_entity" SELECT * FROM "${tablePrefix}tag_entity";`,
 		);
@@ -22,7 +25,7 @@ CREATE TABLE "${tablePrefix}TMP_tag_entity" ("id" varchar(36) PRIMARY KEY NOT NU
 		);
 
 		await queryRunner.query(`
-CREATE TABLE "${tablePrefix}TMP_workflows_tags" ("workflowId" varchar(36) NOT NULL, "tagId" integer NOT NULL, CONSTRAINT "FK_${tablePrefix}workflows_tags_workflow_entity" FOREIGN KEY ("workflowId") REFERENCES "${tablePrefix}workflow_entity" ("id") ON DELETE CASCADE ON UPDATE NO ACTION, CONSTRAINT "FK_${tablePrefix}workflows_tags_tag_entity" FOREIGN KEY ("tagId") REFERENCES "${tablePrefix}tag_entity" ("id") ON DELETE CASCADE ON UPDATE NO ACTION, PRIMARY KEY ("workflowId", "tagId"));`);
+			CREATE TABLE "${tablePrefix}TMP_workflows_tags" ("workflowId" varchar(36) NOT NULL, "tagId" integer NOT NULL, CONSTRAINT "FK_${tablePrefix}workflows_tags_workflow_entity" FOREIGN KEY ("workflowId") REFERENCES "${tablePrefix}workflow_entity" ("id") ON DELETE CASCADE ON UPDATE NO ACTION, CONSTRAINT "FK_${tablePrefix}workflows_tags_tag_entity" FOREIGN KEY ("tagId") REFERENCES "${tablePrefix}tag_entity" ("id") ON DELETE CASCADE ON UPDATE NO ACTION, PRIMARY KEY ("workflowId", "tagId"));`);
 		await queryRunner.query(
 			`INSERT INTO "${tablePrefix}TMP_workflows_tags" SELECT * FROM "${tablePrefix}workflows_tags";`,
 		);

packages/cli/src/databases/types.ts

@@ -12,15 +12,19 @@ export interface MigrationContext {
 	migrationName: string;
 }
 
-type MigrationFn = (ctx: MigrationContext) => Promise<void>;
+export type MigrationFn = (ctx: MigrationContext) => Promise<void>;
 
-export interface ReversibleMigration {
+export interface BaseMigration {
 	up: MigrationFn;
+	down?: MigrationFn | never;
+	transaction?: false;
+}
+
+export interface ReversibleMigration extends BaseMigration {
 	down: MigrationFn;
 }
 
-export interface IrreversibleMigration {
-	up: MigrationFn;
+export interface IrreversibleMigration extends BaseMigration {
 	down?: never;
 }
 

packages/cli/src/databases/utils/migrationHelpers.ts

@@ -1,11 +1,10 @@
-/* eslint-disable no-await-in-loop */
 import { readFileSync, rmSync } from 'fs';
 import { UserSettings } from 'n8n-core';
 import type { QueryRunner } from 'typeorm/query-runner/QueryRunner';
 import config from '@/config';
 import { getLogger } from '@/Logger';
 import { inTest } from '@/constants';
-import type { Migration, MigrationContext } from '@db/types';
+import type { BaseMigration, Migration, MigrationContext, MigrationFn } from '@db/types';
 
 const logger = getLogger();
 
@@ -39,30 +38,47 @@ export function loadSurveyFromDisk(): string | null {
 	}
 }
 
-let logFinishTimeout: NodeJS.Timeout;
+let runningMigrations = false;
 
-export function logMigrationStart(migrationName: string, disableLogging = inTest): void {
-	if (disableLogging) return;
+function logMigrationStart(migrationName: string): void {
+	if (inTest) return;
 
-	if (!logFinishTimeout) {
+	if (!runningMigrations) {
 		logger.warn('Migrations in progress, please do NOT stop the process.');
+		runningMigrations = true;
 	}
 
 	logger.debug(`Starting migration ${migrationName}`);
-
-	clearTimeout(logFinishTimeout);
 }
 
-export function logMigrationEnd(migrationName: string, disableLogging = inTest): void {
-	if (disableLogging) return;
+function logMigrationEnd(migrationName: string): void {
+	if (inTest) return;
 
 	logger.debug(`Finished migration ${migrationName}`);
-
-	logFinishTimeout = setTimeout(() => {
-		logger.warn('Migrations finished.');
-	}, 100);
 }
 
+const runDisablingForeignKeys = async (
+	migration: BaseMigration,
+	context: MigrationContext,
+	fn: MigrationFn,
+) => {
+	const { dbType, queryRunner } = context;
+	if (dbType !== 'sqlite') throw new Error('Disabling transactions only available in sqlite');
+	await queryRunner.query('PRAGMA foreign_keys=OFF');
+	await queryRunner.startTransaction();
+	try {
+		await fn.call(migration, context);
+		await queryRunner.commitTransaction();
+	} catch (e) {
+		try {
+			await queryRunner.rollbackTransaction();
+		} catch {}
+		throw e;
+	} finally {
+		await queryRunner.query('PRAGMA foreign_keys=ON');
+	}
+};
+
 export const wrapMigration = (migration: Migration) => {
 	const dbType = config.getEnv('database.type');
 	const dbName = config.getEnv(`database.${dbType === 'mariadb' ? 'mysqldb' : dbType}.database`);
@@ -78,13 +94,23 @@ export const wrapMigration = (migration: Migration) => {
 
 	const { up, down } = migration.prototype;
 	Object.assign(migration.prototype, {
-		async up(queryRunner: QueryRunner) {
+		async up(this: BaseMigration, queryRunner: QueryRunner) {
 			logMigrationStart(migrationName);
-			await up.call(this, { queryRunner, ...context });
+			if (!this.transaction) {
+				await runDisablingForeignKeys(this, { queryRunner, ...context }, up);
+			} else {
+				await up.call(this, { queryRunner, ...context });
+			}
 			logMigrationEnd(migrationName);
 		},
-		async down(queryRunner: QueryRunner) {
-			await down?.call(this, { queryRunner, ...context });
+		async down(this: BaseMigration, queryRunner: QueryRunner) {
+			if (down) {
+				if (!this.transaction) {
+					await runDisablingForeignKeys(this, { queryRunner, ...context }, up);
+				} else {
+					await down.call(this, { queryRunner, ...context });
+				}
+			}
 		},
 	});
 };

patches/typeorm@0.3.12.patch

@@ -1,31 +0,0 @@
-diff --git a/migration/MigrationExecutor.js b/migration/MigrationExecutor.js
-index 5d37b9cf9ca2505242f05160f05ff683e00c1e5d..4a768819f86b8f176bd3b826a649afe54ab39598 100644
---- a/migration/MigrationExecutor.js
-+++ b/migration/MigrationExecutor.js
-@@ -216,15 +216,17 @@ class MigrationExecutor {
-                     // nothing else needs to be done, continue to next migration
-                     continue;
-                 }
-+                await queryRunner.beforeMigration();
-                 if (migration.transaction && !queryRunner.isTransactionActive) {
-                     await queryRunner.startTransaction();
-                     transactionStartedByUs = true;
-                 }
-                 await migration
-                     .instance.up(queryRunner)
--                    .catch((error) => {
-+                    .catch(async (error) => {
-                     // informative log about migration failure
-                     this.connection.logger.logMigration(`Migration "${migration.name}" failed, error: ${error === null || error === void 0 ? void 0 : error.message}`);
-+                    await queryRunner.afterMigration(queryRunner);
-                     throw error;
-                 })
-                     .then(async () => {
-@@ -233,6 +235,7 @@ class MigrationExecutor {
-                     // commit transaction if we started it
-                     if (migration.transaction && transactionStartedByUs)
-                         await queryRunner.commitTransaction();
-+                    await queryRunner.afterMigration(queryRunner);
-                 })
-                     .then(() => {
-                     // informative log about migration success

pnpm-lock.yaml

@@ -38,9 +38,6 @@ patchedDependencies:
   typedi@0.10.0:
     hash: 62r6bc2crgimafeyruodhqlgo4
     path: patches/typedi@0.10.0.patch
-  typeorm@0.3.12:
-    hash: yav7zi22hnry26k2lwg6jcumde
-    path: patches/typeorm@0.3.12.patch
 
 importers:
 
@@ -442,7 +439,7 @@ importers:
         version: 0.10.0(patch_hash=62r6bc2crgimafeyruodhqlgo4)
       typeorm:
         specifier: ^0.3.12
-        version: 0.3.12(patch_hash=yav7zi22hnry26k2lwg6jcumde)(ioredis@5.2.4)(mysql2@2.3.3)(pg@8.8.0)(sqlite3@5.1.6)
+        version: 0.3.12(ioredis@5.2.4)(mysql2@2.3.3)(pg@8.8.0)(sqlite3@5.1.6)
       uuid:
         specifier: ^8.3.2
         version: 8.3.2
@@ -21388,7 +21385,7 @@ packages:
     dev: false
     patched: true
 
-  /typeorm@0.3.12(patch_hash=yav7zi22hnry26k2lwg6jcumde)(ioredis@5.2.4)(mysql2@2.3.3)(pg@8.8.0)(sqlite3@5.1.6):
+  /typeorm@0.3.12(ioredis@5.2.4)(mysql2@2.3.3)(pg@8.8.0)(sqlite3@5.1.6):
     resolution: {integrity: sha512-sYSxBmCf1nJLLTcYtwqZ+lQIRtLPyUoO93rHTOKk9vJCyT4UfRtU7oRsJvfvKP3nnZTD1hzz2SEy2zwPEN6OyA==}
     engines: {node: '>= 12.9.0'}
     hasBin: true
@@ -21470,7 +21467,6 @@ packages:
     transitivePeerDependencies:
       - supports-color
     dev: false
-    patched: true
 
   /typescript@5.1.3:
     resolution: {integrity: sha512-XH627E9vkeqhlZFQuL+UsyAXEnibT0kWR2FWONlr4sTjvxyJYnyefgrkyECLzM5NenmKzRAy2rR/OlYLA1HkZw==}