Skip to content

Commit e152cfe

Browse files
authored
fix(core): All migrations should run in a transaction (n8n-io#6519)
1 parent ddfb24b commit e152cfe

10 files changed

+48
-45
lines changed

package.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,8 @@
9595
"element-ui@2.15.12": "patches/element-ui@2.15.12.patch",
9696
"typedi@0.10.0": "patches/typedi@0.10.0.patch",
9797
"@sentry/cli@2.17.0": "patches/@sentry__cli@2.17.0.patch",
98-
"pkce-challenge@3.0.0": "patches/pkce-challenge@3.0.0.patch"
98+
"pkce-challenge@3.0.0": "patches/pkce-challenge@3.0.0.patch",
99+
"typeorm@0.3.12": "patches/typeorm@0.3.12.patch"
99100
}
100101
}
101102
}

packages/cli/src/databases/migrations/mysqldb/1690000000001-MigrateIntegerKeysToString.ts

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
1-
import type { MigrationContext, ReversibleMigration } from '@db/types';
1+
import type { MigrationContext, IrreversibleMigration } from '@db/types';
22
import config from '@/config';
33

44
const COLLATION_57 = 'utf8mb4_general_ci';
55
const COLLATION_80 = 'utf8mb4_0900_ai_ci';
66

7-
export class MigrateIntegerKeysToString1690000000001 implements ReversibleMigration {
7+
export class MigrateIntegerKeysToString1690000000001 implements IrreversibleMigration {
88
async up({ queryRunner, tablePrefix }: MigrationContext) {
99
const databaseType = config.get('database.type');
1010
let collation: string;
@@ -272,7 +272,4 @@ export class MigrateIntegerKeysToString1690000000001 implements ReversibleMigrat
272272
);
273273
await queryRunner.query(`ALTER TABLE ${tablePrefix}variables DROP COLUMN \`tmp_id\`;`);
274274
}
275-
276-
// eslint-disable-next-line @typescript-eslint/no-empty-function, @typescript-eslint/no-unused-vars
277-
async down({ queryRunner, tablePrefix }: MigrationContext) {}
278275
}

packages/cli/src/databases/migrations/postgresdb/1690000000000-MigrateIntegerKeysToString.ts

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
/* eslint-disable n8n-local-rules/no-unneeded-backticks */
2-
import type { MigrationContext, ReversibleMigration } from '@db/types';
2+
import type { MigrationContext, IrreversibleMigration } from '@db/types';
33

4-
export class MigrateIntegerKeysToString1690000000000 implements ReversibleMigration {
4+
export class MigrateIntegerKeysToString1690000000000 implements IrreversibleMigration {
55
async up({ queryRunner, tablePrefix }: MigrationContext) {
66
await queryRunner.query(
77
`ALTER TABLE ${tablePrefix}workflow_entity RENAME COLUMN id to tmp_id;`,
@@ -260,7 +260,4 @@ export class MigrateIntegerKeysToString1690000000000 implements ReversibleMigrat
260260
await queryRunner.query(`ALTER TABLE ${tablePrefix}variables DROP COLUMN tmp_id;`);
261261
await queryRunner.query(`ALTER TABLE ${tablePrefix}variables ADD PRIMARY KEY (id);`);
262262
}
263-
264-
// eslint-disable-next-line @typescript-eslint/no-empty-function, @typescript-eslint/no-unused-vars
265-
async down({ queryRunner, tablePrefix }: MigrationContext) {}
266263
}

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

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,7 @@
11
import type { MigrationContext, ReversibleMigration } from '@db/types';
22

33
export class AddUserSettings1652367743993 implements ReversibleMigration {
4-
transaction = false as const;
5-
64
async up({ queryRunner, tablePrefix }: MigrationContext) {
7-
await queryRunner.query('PRAGMA foreign_keys=OFF');
8-
95
await queryRunner.query(
106
`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)`,
117
);
@@ -18,8 +14,6 @@ export class AddUserSettings1652367743993 implements ReversibleMigration {
1814
await queryRunner.query(
1915
`CREATE UNIQUE INDEX "UQ_${tablePrefix}e12875dfb3b1d92d7d7c5377e2" ON "${tablePrefix}user" ("email")`,
2016
);
21-
22-
await queryRunner.query('PRAGMA foreign_keys=ON');
2317
}
2418

2519
async down({ queryRunner, tablePrefix }: MigrationContext) {

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

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,7 @@
11
import type { MigrationContext, ReversibleMigration } from '@db/types';
22

33
export class AddAPIKeyColumn1652905585850 implements ReversibleMigration {
4-
transaction = false as const;
5-
64
async up({ queryRunner, tablePrefix }: MigrationContext) {
7-
await queryRunner.query('PRAGMA foreign_keys=OFF');
8-
95
await queryRunner.query(
106
`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)`,
117
);
@@ -21,8 +17,6 @@ export class AddAPIKeyColumn1652905585850 implements ReversibleMigration {
2117
await queryRunner.query(
2218
`CREATE UNIQUE INDEX "UQ_${tablePrefix}ie0zomxves9w3p774drfrkxtj5" ON "${tablePrefix}user" ("apiKey")`,
2319
);
24-
25-
await queryRunner.query('PRAGMA foreign_keys=ON');
2620
}
2721

2822
async down({ queryRunner, tablePrefix }: MigrationContext) {

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

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
import type { MigrationContext, ReversibleMigration } from '@db/types';
22

33
export class DeleteExecutionsWithWorkflows1673268682475 implements ReversibleMigration {
4-
transaction = false as const;
5-
64
async up({ queryRunner, tablePrefix }: MigrationContext) {
75
const workflowIds = (await queryRunner.query(`
86
SELECT id FROM "${tablePrefix}workflow_entity"
@@ -18,8 +16,6 @@ export class DeleteExecutionsWithWorkflows1673268682475 implements ReversibleMig
1816
}`,
1917
);
2018

21-
await queryRunner.query('PRAGMA foreign_keys=OFF');
22-
2319
await queryRunner.query(`DROP TABLE IF EXISTS "${tablePrefix}temporary_execution_entity"`);
2420
await queryRunner.query(
2521
`CREATE TABLE "${tablePrefix}temporary_execution_entity" (
@@ -53,8 +49,6 @@ export class DeleteExecutionsWithWorkflows1673268682475 implements ReversibleMig
5349
await queryRunner.query(
5450
`CREATE INDEX "IDX_${tablePrefix}ca4a71b47f28ac6ea88293a8e2" ON "${tablePrefix}execution_entity" ("waitTill")`,
5551
);
56-
57-
await queryRunner.query('PRAGMA foreign_keys=ON');
5852
}
5953

6054
async down({ queryRunner, tablePrefix }: MigrationContext) {

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

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,7 @@
1-
import type { MigrationContext, ReversibleMigration } from '@db/types';
2-
3-
export class MigrateIntegerKeysToString1690000000002 implements ReversibleMigration {
4-
transaction = false as const;
1+
import type { MigrationContext, IrreversibleMigration } from '@db/types';
52

3+
export class MigrateIntegerKeysToString1690000000002 implements IrreversibleMigration {
64
async up({ queryRunner, tablePrefix }: MigrationContext) {
7-
await queryRunner.query('PRAGMA foreign_keys=OFF');
8-
await queryRunner.startTransaction();
95
await queryRunner.query(`
106
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);`);
117
await queryRunner.query(
@@ -174,12 +170,8 @@ CREATE TABLE "${tablePrefix}TMP_workflows_tags" ("workflowId" varchar(36) NOT NU
174170
await queryRunner.query(
175171
`ALTER TABLE "${tablePrefix}TMP_variables" RENAME TO "${tablePrefix}variables";`,
176172
);
177-
await queryRunner.query(`CREATE UNIQUE INDEX "idx_${tablePrefix}variables_key" ON "${tablePrefix}variables" ("key");
178-
`);
179-
await queryRunner.commitTransaction();
180-
await queryRunner.query('PRAGMA foreign_keys=ON');
173+
await queryRunner.query(
174+
`CREATE UNIQUE INDEX "idx_${tablePrefix}variables_key" ON "${tablePrefix}variables" ("key")`,
175+
);
181176
}
182-
183-
// eslint-disable-next-line @typescript-eslint/no-empty-function, @typescript-eslint/no-unused-vars
184-
async down({ queryRunner, tablePrefix }: MigrationContext) {}
185177
}

packages/cli/src/databases/types.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ type MigrationFn = (ctx: MigrationContext) => Promise<void>;
1717
export interface ReversibleMigration {
1818
up: MigrationFn;
1919
down: MigrationFn;
20-
transaction?: false;
2120
}
2221

2322
export interface IrreversibleMigration {

patches/typeorm@0.3.12.patch

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
diff --git a/migration/MigrationExecutor.js b/migration/MigrationExecutor.js
2+
index 5d37b9cf9ca2505242f05160f05ff683e00c1e5d..4a768819f86b8f176bd3b826a649afe54ab39598 100644
3+
--- a/migration/MigrationExecutor.js
4+
+++ b/migration/MigrationExecutor.js
5+
@@ -216,15 +216,17 @@ class MigrationExecutor {
6+
// nothing else needs to be done, continue to next migration
7+
continue;
8+
}
9+
+ await queryRunner.beforeMigration();
10+
if (migration.transaction && !queryRunner.isTransactionActive) {
11+
await queryRunner.startTransaction();
12+
transactionStartedByUs = true;
13+
}
14+
await migration
15+
.instance.up(queryRunner)
16+
- .catch((error) => {
17+
+ .catch(async (error) => {
18+
// informative log about migration failure
19+
this.connection.logger.logMigration(`Migration "${migration.name}" failed, error: ${error === null || error === void 0 ? void 0 : error.message}`);
20+
+ await queryRunner.afterMigration(queryRunner);
21+
throw error;
22+
})
23+
.then(async () => {
24+
@@ -233,6 +235,7 @@ class MigrationExecutor {
25+
// commit transaction if we started it
26+
if (migration.transaction && transactionStartedByUs)
27+
await queryRunner.commitTransaction();
28+
+ await queryRunner.afterMigration(queryRunner);
29+
})
30+
.then(() => {
31+
// informative log about migration success

pnpm-lock.yaml

Lines changed: 6 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)