fix: throwing error on duplicate migration names #4701 (#4704)

* fix: skip duplicated pending migrations

Make the behavior the same whether duplicated migrations applied in one step or not.

Closes: #4701

* style: revert unnecessary formatting

* fix: throw exception when duplicate migrations

src/migration/MigrationExecutor.ts

@@ -339,10 +339,20 @@ export class MigrationExecutor {
             return new Migration(undefined, migrationTimestamp, migrationClassName, migration);
         });
 
+        this.checkForDuplicateMigrations(migrations);
+
         // sort them by timestamp
         return migrations.sort((a, b) => a.timestamp - b.timestamp);
     }
 
+    protected checkForDuplicateMigrations(migrations: Migration[]) {
+        const migrationNames = migrations.map(migration => migration.name);
+        const duplicates = Array.from(new Set(migrationNames.filter((migrationName, index) => migrationNames.indexOf(migrationName) < index)));
+        if (duplicates.length > 0) {
+            throw Error(`Duplicate migrations: ${duplicates.join(", ")}`);
+        }
+    }
+
     /**
      * Finds the latest migration (sorts by timestamp) in the given array of migrations.
      */

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

@@ -0,0 +1,24 @@
+import "reflect-metadata";
+import {expect} from "chai";
+import {
+    createTestingConnections,
+    closeTestingConnections,
+    reloadTestingDatabases
+} from "../../utils/test-utils";
+import { Connection } from "../../../src/connection/Connection";
+
+describe("github issues > #4701 Duplicate migrations are executed.", () => {
+    let connections: Connection[];
+    before(async () => connections = await createTestingConnections({
+        migrations: [__dirname + "/migration/*.js"],
+        enabledDrivers: ["postgres"],
+        schemaCreate: true,
+        dropSchema: true
+    }));
+    beforeEach(() => reloadTestingDatabases(connections));
+    after(() => closeTestingConnections(connections));
+
+    it("should throw error if there're duplicate migrations", () => Promise.all(connections.map(async connection => {
+        await expect(connection.runMigrations()).to.be.rejectedWith(Error, "Duplicate migrations: ExampleMigrationOne1567759789051");
+    })));
+});

test/github-issues/4701/migration/1567759789051-ExampleMigration.ts

@@ -0,0 +1,7 @@
+import { MigrationInterface } from "../../../../src/migration/MigrationInterface";
+import { QueryRunner } from "../../../../src/query-runner/QueryRunner";
+
+export class ExampleMigrationOne1567759789051 implements MigrationInterface {
+    public async up(queryRunner: QueryRunner): Promise<any> {}
+    public async down(queryRunner: QueryRunner): Promise<any> {}
+}

test/github-issues/4701/migration/1567759832591-ExampleMigrationTwo.ts

@@ -0,0 +1,7 @@
+import { MigrationInterface } from "../../../../src/migration/MigrationInterface";
+import { QueryRunner } from "../../../../src/query-runner/QueryRunner";
+
+export class ExampleMigrationOne1567759789051 implements MigrationInterface {
+    public async up(queryRunner: QueryRunner): Promise<any> {}
+    public async down(queryRunner: QueryRunner): Promise<any> {}
+}

test/github-issues/4701/migration/1571426391120-ExampleMigrationThree.ts

@@ -0,0 +1,7 @@
+import { MigrationInterface } from "../../../../src/migration/MigrationInterface";
+import { QueryRunner } from "../../../../src/query-runner/QueryRunner";
+
+export class ExampleMigrationThree1571426391120 implements MigrationInterface {
+    public async up(queryRunner: QueryRunner): Promise<any> {}
+    public async down(queryRunner: QueryRunner): Promise<any> {}
+}