feat: add support to string array on dropColumns (#7654)

* feat: add support to string array on dropColumns * Update drop-column.ts removed `skip` Co-authored-by: AlexMesser <dmzt08@gmail.com>
typeorm · Jul 31, 2021 · 91d5b2f · 91d5b2f
1 parent cd71f62
commit 91d5b2f
Show file tree

Hide file tree

Showing 13 changed files with 108 additions and 65 deletions.
diff --git a/docs/migrations.md b/docs/migrations.md
@@ -540,11 +540,11 @@ Drops a column in the table.
 ---
 
 ```ts
-dropColumns(table: Table|string, columns: TableColumn[]): Promise<void>
+dropColumns(table: Table|string, columns: TableColumn[]|string[]): Promise<void>
 ```
 
 - `table` - Table object or name
-- `columns` - array of TableColumn objects to be dropped
+- `columns` - array of TableColumn objects or column names to be dropped 
 
 Drops a columns in the table.
 

diff --git a/docs/zh_CN/migrations.md b/docs/zh_CN/migrations.md
@@ -507,7 +507,7 @@ dropColumn(table: Table|string, column: TableColumn|string): Promise<void>
 ---
 
 ```ts
-dropColumns(table: Table|string, columns: TableColumn[]): Promise<void>
+dropColumns(table: Table|string, columns: TableColumn[]|string[]): Promise<void>
 ```
 
 - `table` - 表对象或名称

diff --git a/src/driver/aurora-data-api/AuroraDataApiQueryRunner.ts b/src/driver/aurora-data-api/AuroraDataApiQueryRunner.ts
@@ -838,7 +838,7 @@ export class AuroraDataApiQueryRunner extends BaseQueryRunner implements QueryRu
     /**
      * Drops the columns in the table.
      */
-    async dropColumns(tableOrName: Table|string, columns: TableColumn[]): Promise<void> {
+    async dropColumns(tableOrName: Table|string, columns: TableColumn[]|string[]): Promise<void> {
         for (const column of columns) {
             await this.dropColumn(tableOrName, column);
         }

diff --git a/src/driver/cockroachdb/CockroachQueryRunner.ts b/src/driver/cockroachdb/CockroachQueryRunner.ts
@@ -1006,7 +1006,7 @@ export class CockroachQueryRunner extends BaseQueryRunner implements QueryRunner
     /**
      * Drops the columns in the table.
      */
-    async dropColumns(tableOrName: Table|string, columns: TableColumn[]): Promise<void> {
+    async dropColumns(tableOrName: Table|string, columns: TableColumn[]|string[]): Promise<void> {
         for (const column of columns) {
             await this.dropColumn(tableOrName, column);
         }

diff --git a/src/driver/mongodb/MongoQueryRunner.ts b/src/driver/mongodb/MongoQueryRunner.ts
@@ -657,7 +657,7 @@ export class MongoQueryRunner implements QueryRunner {
     /**
      * Drops the columns in the table.
      */
-    async dropColumns(tableOrName: Table | string, columns: TableColumn[]): Promise<void> {
+    async dropColumns(tableOrName: Table | string, columns: TableColumn[] | string[]): Promise<void> {
         throw new TypeORMError(`Schema update queries are not supported by MongoDB driver.`);
     }
 

diff --git a/src/driver/mysql/MysqlQueryRunner.ts b/src/driver/mysql/MysqlQueryRunner.ts
@@ -892,7 +892,7 @@ export class MysqlQueryRunner extends BaseQueryRunner implements QueryRunner {
     /**
      * Drops the columns in the table.
      */
-    async dropColumns(tableOrName: Table|string, columns: TableColumn[]): Promise<void> {
+    async dropColumns(tableOrName: Table|string, columns: TableColumn[]|string[]): Promise<void> {
         for (const column of columns) {
             await this.dropColumn(tableOrName, column);
         }

diff --git a/src/driver/oracle/OracleQueryRunner.ts b/src/driver/oracle/OracleQueryRunner.ts
@@ -883,7 +883,7 @@ export class OracleQueryRunner extends BaseQueryRunner implements QueryRunner {
     /**
      * Drops the columns in the table.
      */
-    async dropColumns(tableOrName: Table|string, columns: TableColumn[]): Promise<void> {
+    async dropColumns(tableOrName: Table|string, columns: TableColumn[]|string[]): Promise<void> {
         for (const column of columns) {
             await this.dropColumn(tableOrName, column);
         }

diff --git a/src/driver/postgres/PostgresQueryRunner.ts b/src/driver/postgres/PostgresQueryRunner.ts
@@ -1110,7 +1110,7 @@ export class PostgresQueryRunner extends BaseQueryRunner implements QueryRunner
     /**
      * Drops the columns in the table.
      */
-    async dropColumns(tableOrName: Table|string, columns: TableColumn[]): Promise<void> {
+    async dropColumns(tableOrName: Table|string, columns: TableColumn[]|string[]): Promise<void> {
         for (const column of columns) {
             await this.dropColumn(tableOrName, column);
         }

diff --git a/src/driver/sap/SapQueryRunner.ts b/src/driver/sap/SapQueryRunner.ts
@@ -1065,7 +1065,7 @@ export class SapQueryRunner extends BaseQueryRunner implements QueryRunner {
     /**
      * Drops the columns in the table.
      */
-    async dropColumns(tableOrName: Table|string, columns: TableColumn[]): Promise<void> {
+    async dropColumns(tableOrName: Table|string, columns: TableColumn[]|string[]): Promise<void> {
         for (const column of columns) {
             await this.dropColumn(tableOrName, column);
         }

diff --git a/src/driver/sqlite-abstract/AbstractSqliteQueryRunner.ts b/src/driver/sqlite-abstract/AbstractSqliteQueryRunner.ts
@@ -458,27 +458,23 @@ export abstract class AbstractSqliteQueryRunner extends BaseQueryRunner implemen
     /**
      * Drops the columns in the table.
      */
-    async dropColumns(tableOrName: Table|string, columns: TableColumn[]): Promise<void> {
+    async dropColumns(tableOrName: Table|string, columns: TableColumn[]|string[]): Promise<void> {
         const table = tableOrName instanceof Table ? tableOrName : await this.getCachedTable(tableOrName);
 
         // clone original table and remove column and its constraints from cloned table
         const changedTable = table.clone();
-        columns.forEach(column => {
-            changedTable.removeColumn(column);
-            changedTable.findColumnUniques(column).forEach(unique => changedTable.removeUniqueConstraint(unique));
-            changedTable.findColumnIndices(column).forEach(index => changedTable.removeIndex(index));
-            changedTable.findColumnForeignKeys(column).forEach(fk => changedTable.removeForeignKey(fk));
+        columns.forEach((column: TableColumn|string) => {
+            const columnInstance = column instanceof TableColumn ? column : table.findColumnByName(column);
+            if (!columnInstance)
+                throw new Error(`Column "${column}" was not found in table "${table.name}"`);
+
+            changedTable.removeColumn(columnInstance);
+            changedTable.findColumnUniques(columnInstance).forEach(unique => changedTable.removeUniqueConstraint(unique));
+            changedTable.findColumnIndices(columnInstance).forEach(index => changedTable.removeIndex(index));
+            changedTable.findColumnForeignKeys(columnInstance).forEach(fk => changedTable.removeForeignKey(fk));
         });
 
         await this.recreateTable(changedTable, table);
-
-        // remove column and its constraints from original table.
-        columns.forEach(column => {
-            table.removeColumn(column);
-            table.findColumnUniques(column).forEach(unique => table.removeUniqueConstraint(unique));
-            table.findColumnIndices(column).forEach(index => table.removeIndex(index));
-            table.findColumnForeignKeys(column).forEach(fk => table.removeForeignKey(fk));
-        });
     }
 
     /**

diff --git a/src/driver/sqlserver/SqlServerQueryRunner.ts b/src/driver/sqlserver/SqlServerQueryRunner.ts
@@ -1103,7 +1103,7 @@ export class SqlServerQueryRunner extends BaseQueryRunner implements QueryRunner
     /**
      * Drops the columns in the table.
      */
-    async dropColumns(tableOrName: Table|string, columns: TableColumn[]): Promise<void> {
+    async dropColumns(tableOrName: Table|string, columns: TableColumn[]|string[]): Promise<void> {
         for (const column of columns) {
             await this.dropColumn(tableOrName, column);
         }

diff --git a/src/query-runner/QueryRunner.ts b/src/query-runner/QueryRunner.ts
@@ -259,7 +259,7 @@ export interface QueryRunner {
     /**
      * Drops columns in the table.
      */
-    dropColumns(table: Table|string, columns: TableColumn[]): Promise<void>;
+    dropColumns(table: Table|string, columns: TableColumn[]|string[]): Promise<void>;
 
     /**
      * Creates a new primary key.

diff --git a/test/functional/query-runner/drop-column.ts b/test/functional/query-runner/drop-column.ts
@@ -16,47 +16,94 @@ describe("query runner > drop column", () => {
     });
     after(() => closeTestingConnections(connections));
 
-    it("should correctly drop column and revert drop", () => Promise.all(connections.map(async connection => {
-
-        const queryRunner = connection.createQueryRunner();
-
-        let table = await queryRunner.getTable("post");
-        const idColumn = table!.findColumnByName("id")!;
-        const nameColumn = table!.findColumnByName("name")!;
-        const versionColumn = table!.findColumnByName("version")!;
-        idColumn!.should.be.exist;
-        nameColumn!.should.be.exist;
-        versionColumn!.should.be.exist;
-
-        // better-sqlite3 seems not able to create a check constraint on a non-existing column
-        if (connection.name === "better-sqlite3") {
-            await queryRunner.dropCheckConstraints(table!, table!.checks);
-        }
-
-        // In Sqlite 'dropColumns' method is more optimal than 'dropColumn', because it recreate table just once,
-        // without all removed columns. In other drivers it's no difference between these methods, because 'dropColumns'
-        // calls 'dropColumn' method for each removed column.
-        // CockroachDB does not support changing pk.
-        if (connection.driver instanceof CockroachDriver) {
-            await queryRunner.dropColumns(table!, [nameColumn, versionColumn]);
-        } else {
-            await queryRunner.dropColumns(table!, [idColumn, nameColumn, versionColumn]);
-        }
-
-        table = await queryRunner.getTable("post");
-        expect(table!.findColumnByName("name")).to.be.undefined;
-        expect(table!.findColumnByName("version")).to.be.undefined;
-        if (!(connection.driver instanceof CockroachDriver))
-            expect(table!.findColumnByName("id")).to.be.undefined;
+    describe("when columns are instances of TableColumn", () => {
+        it("should correctly drop column and revert drop", () => Promise.all(connections.map(async connection => {
+
+            const queryRunner = connection.createQueryRunner();
+
+            let table = await queryRunner.getTable("post");
+            const idColumn = table!.findColumnByName("id")!;
+            const nameColumn = table!.findColumnByName("name")!;
+            const versionColumn = table!.findColumnByName("version")!;
+            idColumn!.should.be.exist;
+            nameColumn!.should.be.exist;
+            versionColumn!.should.be.exist;
+
+            // better-sqlite3 seems not able to create a check constraint on a non-existing column
+            if (connection.name === "better-sqlite3") {
+                await queryRunner.dropCheckConstraints(table!, table!.checks);
+            }
+
+            // In Sqlite 'dropColumns' method is more optimal than 'dropColumn', because it recreate table just once,
+            // without all removed columns. In other drivers it's no difference between these methods, because 'dropColumns'
+            // calls 'dropColumn' method for each removed column.
+            // CockroachDB does not support changing pk.
+            if (connection.driver instanceof CockroachDriver) {
+                await queryRunner.dropColumns(table!, [nameColumn, versionColumn]);
+            } else {
+                await queryRunner.dropColumns(table!, [idColumn, nameColumn, versionColumn]);
+            }
+
+            table = await queryRunner.getTable("post");
+            expect(table!.findColumnByName("name")).to.be.undefined;
+            expect(table!.findColumnByName("version")).to.be.undefined;
+            if (!(connection.driver instanceof CockroachDriver))
+                expect(table!.findColumnByName("id")).to.be.undefined;
+
+            await queryRunner.executeMemoryDownSql();
+
+            table = await queryRunner.getTable("post");
+            table!.findColumnByName("id")!.should.be.exist;
+            table!.findColumnByName("name")!.should.be.exist;
+            table!.findColumnByName("version")!.should.be.exist;
+
+            await queryRunner.release();
+        })));
+    });
 
-        await queryRunner.executeMemoryDownSql();
+    describe("when columns are strings", () => {
+        it("should correctly drop column and revert drop", () => Promise.all(connections.map(async connection => {
 
-        table = await queryRunner.getTable("post");
-        table!.findColumnByName("id")!.should.be.exist;
-        table!.findColumnByName("name")!.should.be.exist;
-        table!.findColumnByName("version")!.should.be.exist;
+            const queryRunner = connection.createQueryRunner();
+
+            let table = await queryRunner.getTable("post");
+            const idColumn = table!.findColumnByName("id")!;
+            const nameColumn = table!.findColumnByName("name")!;
+            const versionColumn = table!.findColumnByName("version")!;
+            idColumn!.should.be.exist;
+            nameColumn!.should.be.exist;
+            versionColumn!.should.be.exist;
+
+            // better-sqlite3 seems not able to create a check constraint on a non-existing column
+            if (connection.name === "better-sqlite3") {
+                await queryRunner.dropCheckConstraints(table!, table!.checks);
+            }
+
+            // In Sqlite 'dropColumns' method is more optimal than 'dropColumn', because it recreate table just once,
+            // without all removed columns. In other drivers it's no difference between these methods, because 'dropColumns'
+            // calls 'dropColumn' method for each removed column.
+            // CockroachDB does not support changing pk.
+            if (connection.driver instanceof CockroachDriver) {
+                await queryRunner.dropColumns(table!, ["name", "version"]);
+            } else {
+                await queryRunner.dropColumns(table!, ["id", "name", "version"]);
+            }
+
+            table = await queryRunner.getTable("post");
+            expect(table!.findColumnByName("name")).to.be.undefined;
+            expect(table!.findColumnByName("version")).to.be.undefined;
+            if (!(connection.driver instanceof CockroachDriver))
+                expect(table!.findColumnByName("id")).to.be.undefined;
+
+            await queryRunner.executeMemoryDownSql();
 
-        await queryRunner.release();
-    })));
+            table = await queryRunner.getTable("post");
+            table!.findColumnByName("id")!.should.be.exist;
+            table!.findColumnByName("name")!.should.be.exist;
+            table!.findColumnByName("version")!.should.be.exist;
+
+            await queryRunner.release();
+        })));
+    });
 
 });