feat: fix command line options, refactor MigrationExecutor.executePen…

…dingMigrations and fix tests

src/commands/MigrationRevertCommand.ts

@@ -54,19 +54,19 @@ export class MigrationRevertCommand implements yargs.CommandModule {
             connection = await createConnection(connectionOptions);
 
             const options = {
-                transactionMode: "all" as "all" | "none" | "each",
+                transaction: "all" as "all" | "none" | "each",
             };
 
-            switch (argv["t"]) {
+            switch (args.t) {
                 case "all":
-                    options.transactionMode = "all";
+                    options.transaction = "all";
                     break;
                 case "none":
                 case "false":
-                    options.transactionMode = "none";
+                    options.transaction = "none";
                     break;
                 case "each":
-                    options.transactionMode = "each";
+                    options.transaction = "each";
                     break;
                 default:
                     // noop

src/commands/MigrationRunCommand.ts

@@ -55,19 +55,19 @@ export class MigrationRunCommand implements yargs.CommandModule {
             connection = await createConnection(connectionOptions);
 
             const options = {
-                transactionMode: "all" as "all" | "none" | "each",
+                transaction: "all" as "all" | "none" | "each",
             };
 
-            switch (argv["t"]) {
+            switch (args.t) {
                 case "all":
-                    options.transactionMode = "all";
+                    options.transaction = "all";
                     break;
                 case "none":
                 case "false":
-                    options.transactionMode = "none";
+                    options.transaction = "none";
                     break;
                 case "each":
-                    options.transactionMode = "each";
+                    options.transaction = "each";
                     break;
                 default:
                     // noop

src/connection/Connection.ts

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

src/migration/MigrationExecutor.ts

@@ -26,7 +26,7 @@ export class MigrationExecutor {
      *   none: all migrations are run without a transaction
      *   each: each migration is run in a separate transaction
      */
-    transactionMode: "all" | "none" | "each" = "all";
+    transaction: "all" | "none" | "each" = "all";
 
     // -------------------------------------------------------------------------
     // Private Properties
@@ -103,6 +103,9 @@ export class MigrationExecutor {
         // get all user's migrations in the source code
         const allMigrations = this.getMigrations();
 
+        // variable to store all migrations we did successefuly
+        const successMigrations: Migration[] = [];
+
         // find all migrations that needs to be executed
         const pendingMigrations = allMigrations.filter(migration => {
             // check if we already have executed migration
@@ -134,14 +137,55 @@ export class MigrationExecutor {
             this.connection.logger.logSchemaBuild(`${lastTimeExecutedMigration.name} is the last executed migration. It was executed on ${new Date(lastTimeExecutedMigration.timestamp).toString()}.`);
         this.connection.logger.logSchemaBuild(`${pendingMigrations.length} migrations are new migrations that needs to be executed.`);
 
-        switch (this.transactionMode) {
-            case "each":
-                return this.runEachMigrationInSeparateTransaction(queryRunner, pendingMigrations);
-            case "all":
-                return this.runAllMigrationsInSingleTransaction(queryRunner, pendingMigrations);
-            case "none":
-                return this.runAllMigrationsWithoutTransaction(queryRunner, pendingMigrations);
+        // start transaction if its not started yet
+        let transactionStartedByUs = false;
+        if (this.transaction === "all" && !queryRunner.isTransactionActive) {
+            await queryRunner.startTransaction();
+            transactionStartedByUs = true;
         }
+
+        // run all pending migrations in a sequence
+        try {
+            await PromiseUtils.runInSequence(pendingMigrations, async migration => {
+                if (this.transaction === "each" && !queryRunner.isTransactionActive) {
+                    await queryRunner.startTransaction();
+                    transactionStartedByUs = true;
+                }
+
+                return migration.instance!.up(queryRunner)
+                    .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);
+                        this.connection.logger.logSchemaBuild(`Migration ${migration.name} has been executed successfully.`);
+                    });
+            });
+
+            // commit transaction if we started it
+            if (this.transaction === "all" && transactionStartedByUs)
+                await queryRunner.commitTransaction();
+
+        } catch (err) { // rollback transaction if we started it
+            if (transactionStartedByUs) {
+                try { // we throw original error even if rollback thrown an error
+                    await queryRunner.rollbackTransaction();
+                } catch (rollbackError) { }
+            }
+
+            throw err;
+
+        } finally {
+
+            // if query runner was created by us then release it
+            if (!this.queryRunner)
+                await queryRunner.release();
+        }
+        return successMigrations;
+
     }
 
     /**
@@ -183,7 +227,7 @@ export class MigrationExecutor {
 
         // start transaction if its not started yet
         let transactionStartedByUs = false;
-        if ((this.transactionMode !== "none") && !queryRunner.isTransactionActive) {
+        if ((this.transaction !== "none") && !queryRunner.isTransactionActive) {
             await queryRunner.startTransaction();
             transactionStartedByUs = true;
         }
@@ -361,116 +405,4 @@ export class MigrationExecutor {
         }
 
     }
-
-    protected async runEachMigrationInSeparateTransaction(queryRunner: QueryRunner, pendingMigrations: Migration[]) {
-        // variable to store all migrations we did successefuly
-        const successMigrations: Migration[] = [];
-
-        if (queryRunner.isTransactionActive) {
-            throw new Error("Trying to run each migration in separate transaction, but already in one. Try changing this option and run migrations again.");
-        }
-
-        // run all pending migrations in a sequence
-        try {
-            await PromiseUtils.runInSequence(pendingMigrations, async migration => {
-                await queryRunner.startTransaction();
-                return migration.instance!.up(queryRunner)
-                    .then(async () => { // now when migration is executed we need to insert record about it into the database
-                        await this.insertExecutedMigration(queryRunner, migration);
-                        await queryRunner.commitTransaction();
-                    })
-                    .then(() => { // informative log about migration success
-                        successMigrations.push(migration);
-                        this.connection.logger.logSchemaBuild(`Migration ${migration.name} has been executed successfully.`);
-                    });
-            });
-
-        } catch (err) { // rollback transaction if we started it
-            try { // we throw original error even if rollback thrown an error
-                if (queryRunner.isTransactionActive) {
-                    await queryRunner.rollbackTransaction();
-                }
-            } catch (rollbackError) { }
-
-            throw err;
-
-        } finally {
-            // if query runner was created by us then release it
-            if (!this.queryRunner)
-                await queryRunner.release();
-        }
-        return successMigrations;
-    }
-
-    protected async runAllMigrationsInSingleTransaction(queryRunner: QueryRunner, pendingMigrations: Migration[]) {
-        // variable to store all migrations we did successefuly
-        const successMigrations: Migration[] = [];
-
-        // start transaction if its not started yet
-        let transactionStartedByUs = false;
-        if (!queryRunner.isTransactionActive) {
-            await queryRunner.startTransaction();
-            transactionStartedByUs = true;
-        }
-
-        // run all pending migrations in a sequence
-        try {
-            await PromiseUtils.runInSequence(pendingMigrations, migration => {
-                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(() => { // informative log about migration success
-                        successMigrations.push(migration);
-                        this.connection.logger.logSchemaBuild(`Migration ${migration.name} has been executed successfully.`);
-                    });
-            });
-
-            // commit transaction if we started it
-            if (transactionStartedByUs)
-                await queryRunner.commitTransaction();
-
-        } catch (err) { // rollback transaction if we started it
-            if (transactionStartedByUs) {
-                try { // we throw original error even if rollback thrown an error
-                    await queryRunner.rollbackTransaction();
-                } catch (rollbackError) { }
-            }
-
-            throw err;
-
-        } finally {
-
-            // if query runner was created by us then release it
-            if (!this.queryRunner)
-                await queryRunner.release();
-        }
-        return successMigrations;
-    }
-
-    protected async runAllMigrationsWithoutTransaction(queryRunner: QueryRunner, pendingMigrations: Migration[]) {
-        // variable to store all migrations we did successefuly
-        const successMigrations: Migration[] = [];
-
-        // run all pending migrations in a sequence
-        try {
-            await PromiseUtils.runInSequence(pendingMigrations, migration => {
-                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(() => { // informative log about migration success
-                        successMigrations.push(migration);
-                        this.connection.logger.logSchemaBuild(`Migration ${migration.name} has been executed successfully.`);
-                    });
-            });
-        } finally {
-
-            // if query runner was created by us then release it
-            if (!this.queryRunner)
-                await queryRunner.release();
-        }
-        return successMigrations;
-    }
-
 }

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

@@ -2,27 +2,29 @@ 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({
-        migrations: [__dirname + "/migration/*.js"],
+        __dirname,
+        schemaCreate: false,
+        dropSchema: true,
         enabledDrivers: ["postgres"]
     }));
     beforeEach(() => reloadTestingDatabases(connections));
     after(() => closeTestingConnections(connections));
 
-    it("should fail to run all necessary migrations when transactionMode is all", () => Promise.all(connections.map(async connection => {
-        const mymigr: Migration[] = await connection.runMigrations({ transactionMode: "all" });
-
-        mymigr.length.should.be.equal(0);
+    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 transactionMode is each", () => Promise.all(connections.map(async connection => {
-        const mymigr: Migration[] = await connection.runMigrations({ transactionMode: "each" });
-
-        mymigr.length.should.be.equal(2);
-        mymigr[0].name.should.be.equal("CreateUuidExtension1544044606093");
-        mymigr[1].name.should.be.equal("CreateUsers1543965157399");
+    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/1543844606093-CreateUuidExtension.ts → test/github-issues/2693/migration/0000000000001-CreateUuidExtension.ts

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

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

@@ -2,19 +2,24 @@ import { MigrationInterface } from "../../../../src/migration/MigrationInterface
 import { QueryRunner } from "../../../../src/query-runner/QueryRunner";
 import { Table } from "../../../../src/schema-builder/table/Table";
 
-export class CreateUsers1543965157399 implements MigrationInterface {
+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()" },
-            ]
-        })
+            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();
+    }
+}