feat: add new transaction mode to wrap each migration in transaction (#…

…4629)

* Add new mode to wrap each migration in transaction

* Add tests

* feat: fix command line options, refactor MigrationExecutor.executePendingMigrations and fix tests

src/commands/MigrationRevertCommand.ts

@@ -52,9 +52,26 @@ export class MigrationRevertCommand implements yargs.CommandModule {
                 logging: ["query", "error", "schema"]
             });
             connection = await createConnection(connectionOptions);
+
             const options = {
-                transaction: args["t"] === "false" ? false : true
+                transaction: "all" as "all" | "none" | "each",
             };
+
+            switch (args.t) {
+                case "all":
+                    options.transaction = "all";
+                    break;
+                case "none":
+                case "false":
+                    options.transaction = "none";
+                    break;
+                case "each":
+                    options.transaction = "each";
+                    break;
+                default:
+                    // noop
+            }
+
             await connection.undoLastMigration(options);
             await connection.close();
 

src/commands/MigrationRunCommand.ts

@@ -55,8 +55,24 @@ export class MigrationRunCommand implements yargs.CommandModule {
             connection = await createConnection(connectionOptions);
 
             const options = {
-                transaction: args["t"] === "false" ? false : true
+                transaction: "all" as "all" | "none" | "each",
             };
+
+            switch (args.t) {
+                case "all":
+                    options.transaction = "all";
+                    break;
+                case "none":
+                case "false":
+                    options.transaction = "none";
+                    break;
+                case "each":
+                    options.transaction = "each";
+                    break;
+                default:
+                    // noop
+            }
+
             await connection.runMigrations(options);
             await connection.close();
             // exit process if no errors

src/connection/Connection.ts

@@ -278,14 +278,13 @@ export class Connection {
      * Runs all pending migrations.
      * Can be used only after connection to the database is established.
      */
-    async runMigrations(options?: { transaction?: boolean }): Promise<Migration[]> {
+    async runMigrations(options?: { transaction?: "all" | "none" | "each" }): Promise<Migration[]> {
         if (!this.isConnected)
             throw new CannotExecuteNotConnectedError(this.name);
 
         const migrationExecutor = new MigrationExecutor(this);
-        if (options && options.transaction === false) {
-            migrationExecutor.transaction = false;
-        }
+        migrationExecutor.transaction = (options && options.transaction) || "all";
+
         const successMigrations = await migrationExecutor.executePendingMigrations();
         return successMigrations;
     }
@@ -294,15 +293,14 @@ export class Connection {
      * Reverts last executed migration.
      * Can be used only after connection to the database is established.
      */
-    async undoLastMigration(options?: { transaction?: boolean }): Promise<void> {
+    async undoLastMigration(options?: { transaction?: "all" | "none" | "each" }): Promise<void> {
 
         if (!this.isConnected)
             throw new CannotExecuteNotConnectedError(this.name);
 
         const migrationExecutor = new MigrationExecutor(this);
-        if (options && options.transaction === false) {
-            migrationExecutor.transaction = false;
-        }
+        migrationExecutor.transaction = (options && options.transaction) || "all";
+
         await migrationExecutor.undoLastMigration();
     }
 

src/migration/MigrationExecutor.ts

@@ -21,9 +21,12 @@ export class MigrationExecutor {
     // -------------------------------------------------------------------------
 
     /**
-     * Indicates if migrations must be executed in a transaction.
+     * Indicates how migrations should be run in transactions.
+     *   all: all migrations are run in a single transaction
+     *   none: all migrations are run without a transaction
+     *   each: each migration is run in a separate transaction
      */
-    transaction: boolean = true;
+    transaction: "all" | "none" | "each" = "all";
 
     // -------------------------------------------------------------------------
     // Private Properties
@@ -136,17 +139,25 @@ export class MigrationExecutor {
 
         // start transaction if its not started yet
         let transactionStartedByUs = false;
-        if (this.transaction && !queryRunner.isTransactionActive) {
+        if (this.transaction === "all" && !queryRunner.isTransactionActive) {
             await queryRunner.startTransaction();
             transactionStartedByUs = true;
         }
 
         // run all pending migrations in a sequence
         try {
-            await PromiseUtils.runInSequence(pendingMigrations, migration => {
+            await PromiseUtils.runInSequence(pendingMigrations, async migration => {
+                if (this.transaction === "each" && !queryRunner.isTransactionActive) {
+                    await queryRunner.startTransaction();
+                    transactionStartedByUs = true;
+                }
+
                 return migration.instance!.up(queryRunner)
-                    .then(() => { // now when migration is executed we need to insert record about it into the database
-                        return this.insertExecutedMigration(queryRunner, migration);
+                    .then(async () => { // now when migration is executed we need to insert record about it into the database
+                        await this.insertExecutedMigration(queryRunner, migration);
+                        // commit transaction if we started it
+                        if (this.transaction === "each" && transactionStartedByUs)
+                            await queryRunner.commitTransaction();
                     })
                     .then(() => { // informative log about migration success
                         successMigrations.push(migration);
@@ -155,7 +166,7 @@ export class MigrationExecutor {
             });
 
             // commit transaction if we started it
-            if (transactionStartedByUs)
+            if (this.transaction === "all" && transactionStartedByUs)
                 await queryRunner.commitTransaction();
 
         } catch (err) { // rollback transaction if we started it
@@ -216,7 +227,7 @@ export class MigrationExecutor {
 
         // start transaction if its not started yet
         let transactionStartedByUs = false;
-        if (this.transaction && !queryRunner.isTransactionActive) {
+        if ((this.transaction !== "none") && !queryRunner.isTransactionActive) {
             await queryRunner.startTransaction();
             transactionStartedByUs = true;
         }
@@ -400,5 +411,4 @@ export class MigrationExecutor {
         }
 
     }
-
 }

test/github-issues/2693/entity/user.ts

@@ -0,0 +1,8 @@
+import {PrimaryGeneratedColumn} from "../../../../src/decorator/columns/PrimaryGeneratedColumn";
+import {Entity} from "../../../../src/decorator/entity/Entity";
+
+@Entity({name: "users", synchronize: false})
+export class User {
+    @PrimaryGeneratedColumn("uuid")
+    id: number;
+}

test/github-issues/2693/issue-2693.ts

@@ -0,0 +1,30 @@
+import "reflect-metadata";
+import {createTestingConnections, closeTestingConnections, reloadTestingDatabases} from "../../utils/test-utils";
+import {Connection} from "../../../src/connection/Connection";
+import {Migration} from "../../../src/migration/Migration";
+import {QueryFailedError} from "../../../src/error/QueryFailedError";
+
+describe("github issues > #2875 Option to run migrations in 1-transaction-per-migration mode", () => {
+    let connections: Connection[];
+    before(async () => connections = await createTestingConnections({
+        __dirname,
+        schemaCreate: false,
+        dropSchema: true,
+        enabledDrivers: ["postgres"]
+    }));
+    beforeEach(() => reloadTestingDatabases(connections));
+    after(() => closeTestingConnections(connections));
+
+    it("should fail to run all necessary migrations when transaction is all", () => Promise.all(connections.map(async connection => {
+        return connection.runMigrations({ transaction: "all" }).should.be.rejectedWith(QueryFailedError, "relation \"users\" does not exist");
+    })));
+
+    it("should be able to run all necessary migrations when transaction is each", () => Promise.all(connections.map(async connection => {
+        const mymigr: Migration[] = await connection.runMigrations({ transaction: "each" });
+
+        mymigr.length.should.be.equal(3);
+        mymigr[0].name.should.be.equal("CreateUuidExtension0000000000001");
+        mymigr[1].name.should.be.equal("CreateUsers0000000000002");
+        mymigr[2].name.should.be.equal("InsertUser0000000000003");
+    })));
+ });

test/github-issues/2693/migration/0000000000001-CreateUuidExtension.ts

@@ -0,0 +1,12 @@
+import { MigrationInterface } from "../../../../src/migration/MigrationInterface";
+import { QueryRunner } from "../../../../src/query-runner/QueryRunner";
+
+export class CreateUuidExtension0000000000001 implements MigrationInterface {
+    public up(queryRunner: QueryRunner): Promise<any> {
+        return queryRunner.query(`CREATE EXTENSION IF NOT EXISTS "uuid-ossp";`);
+    }
+
+    public down(queryRunner: QueryRunner): Promise<any> {
+        return queryRunner.query("DROP EXTENSION \"uuid-ossp\"");
+    }
+}

test/github-issues/2693/migration/0000000000002-CreateUsers.ts

@@ -0,0 +1,25 @@
+import { MigrationInterface } from "../../../../src/migration/MigrationInterface";
+import { QueryRunner } from "../../../../src/query-runner/QueryRunner";
+import { Table } from "../../../../src/schema-builder/table/Table";
+
+export class CreateUsers0000000000002 implements MigrationInterface {
+    public up(queryRunner: QueryRunner): Promise<any> {
+        return queryRunner.createTable(
+            new Table({
+                name: "users",
+                columns: [
+                    {
+                        name: "id",
+                        type: "uuid",
+                        isPrimary: true,
+                        default: "uuid_generate_v4()"
+                    }
+                ]
+            })
+        );
+    }
+
+    public down(queryRunner: QueryRunner): Promise<any> {
+        return queryRunner.dropTable("users");
+    }
+}

test/github-issues/2693/migration/0000000000003-InsertUser.ts

@@ -0,0 +1,14 @@
+import { MigrationInterface } from "../../../../src/migration/MigrationInterface";
+import { QueryRunner } from "../../../../src/query-runner/QueryRunner";
+import { User } from "../entity/user";
+
+export class InsertUser0000000000003 implements MigrationInterface {
+    public up(queryRunner: QueryRunner): Promise<any> {
+        const userRepo = queryRunner.connection.getRepository<User>(User);
+        return userRepo.save(new User());
+    }
+
+    public down(queryRunner: QueryRunner): Promise<any> {
+        return Promise.resolve();
+    }
+}