fix: dropIndex now works when providing a tableIndex without name (#8937

)
typeorm · Aug 24, 2022 · de8aaac · de8aaac
1 parent 64674e6
commit de8aaac
Show file tree

Hide file tree

Showing 12 changed files with 119 additions and 54 deletions.
diff --git a/src/driver/aurora-mysql/AuroraMysqlQueryRunner.ts b/src/driver/aurora-mysql/AuroraMysqlQueryRunner.ts
@@ -1792,12 +1792,7 @@ export class AuroraMysqlQueryRunner
             : await this.getCachedTable(tableOrName)
 
         // new index may be passed without name. In this case we generate index name manually.
-        if (!index.name)
-            index.name = this.connection.namingStrategy.indexName(
-                table,
-                index.columnNames,
-                index.where,
-            )
+        if (!index.name) index.name = this.generateIndexName(table, index)
 
         const up = this.createIndexSql(table, index)
         const down = this.dropIndexSql(table, index)
@@ -1836,6 +1831,9 @@ export class AuroraMysqlQueryRunner
                 `Supplied index ${indexOrName} was not found in table ${table.name}`,
             )
 
+        // old index may be passed without name. In this case we generate index name manually.
+        if (!index.name) index.name = this.generateIndexName(table, index)
+
         const up = this.dropIndexSql(table, index)
         const down = this.createIndexSql(table, index)
         await this.executeQueries(up, down)

diff --git a/src/driver/cockroachdb/CockroachQueryRunner.ts b/src/driver/cockroachdb/CockroachQueryRunner.ts
@@ -2348,12 +2348,7 @@ export class CockroachQueryRunner
             : await this.getCachedTable(tableOrName)
 
         // new index may be passed without name. In this case we generate index name manually.
-        if (!index.name)
-            index.name = this.connection.namingStrategy.indexName(
-                table,
-                index.columnNames,
-                index.where,
-            )
+        if (!index.name) index.name = this.generateIndexName(table, index)
 
         // CockroachDB stores unique indices and UNIQUE constraints
         if (index.isUnique) {
@@ -2405,6 +2400,9 @@ export class CockroachQueryRunner
                 `Supplied index ${indexOrName} was not found in table ${table.name}`,
             )
 
+        // old index may be passed without name. In this case we generate index name manually.
+        if (!index.name) index.name = this.generateIndexName(table, index)
+
         const up = this.dropIndexSql(table, index)
         const down = this.createIndexSql(table, index)
         await this.executeQueries(up, down)

diff --git a/src/driver/mysql/MysqlQueryRunner.ts b/src/driver/mysql/MysqlQueryRunner.ts
@@ -2099,12 +2099,7 @@ export class MysqlQueryRunner extends BaseQueryRunner implements QueryRunner {
             : await this.getCachedTable(tableOrName)
 
         // new index may be passed without name. In this case we generate index name manually.
-        if (!index.name)
-            index.name = this.connection.namingStrategy.indexName(
-                table,
-                index.columnNames,
-                index.where,
-            )
+        if (!index.name) index.name = this.generateIndexName(table, index)
 
         const up = this.createIndexSql(table, index)
         const down = this.dropIndexSql(table, index)
@@ -2143,6 +2138,9 @@ export class MysqlQueryRunner extends BaseQueryRunner implements QueryRunner {
                 `Supplied index ${indexOrName} was not found in table ${table.name}`,
             )
 
+        // old index may be passed without name. In this case we generate index name manually.
+        if (!index.name) index.name = this.generateIndexName(table, index)
+
         const up = this.dropIndexSql(table, index)
         const down = this.createIndexSql(table, index)
         await this.executeQueries(up, down)

diff --git a/src/driver/oracle/OracleQueryRunner.ts b/src/driver/oracle/OracleQueryRunner.ts
@@ -2109,12 +2109,7 @@ export class OracleQueryRunner extends BaseQueryRunner implements QueryRunner {
             : await this.getCachedTable(tableOrName)
 
         // new index may be passed without name. In this case we generate index name manually.
-        if (!index.name)
-            index.name = this.connection.namingStrategy.indexName(
-                table,
-                index.columnNames,
-                index.where,
-            )
+        if (!index.name) index.name = this.generateIndexName(table, index)
 
         const up = this.createIndexSql(table, index)
         const down = this.dropIndexSql(index)
@@ -2152,6 +2147,8 @@ export class OracleQueryRunner extends BaseQueryRunner implements QueryRunner {
             throw new TypeORMError(
                 `Supplied index ${indexOrName} was not found in table ${table.name}`,
             )
+        // old index may be passed without name. In this case we generate index name manually.
+        if (!index.name) index.name = this.generateIndexName(table, index)
 
         const up = this.dropIndexSql(index)
         const down = this.createIndexSql(table, index)

diff --git a/src/driver/postgres/PostgresQueryRunner.ts b/src/driver/postgres/PostgresQueryRunner.ts
@@ -2899,12 +2899,7 @@ export class PostgresQueryRunner
             : await this.getCachedTable(tableOrName)
 
         // new index may be passed without name. In this case we generate index name manually.
-        if (!index.name)
-            index.name = this.connection.namingStrategy.indexName(
-                table,
-                index.columnNames,
-                index.where,
-            )
+        if (!index.name) index.name = this.generateIndexName(table, index)
 
         const up = this.createIndexSql(table, index)
         const down = this.dropIndexSql(table, index)
@@ -2941,6 +2936,8 @@ export class PostgresQueryRunner
             throw new TypeORMError(
                 `Supplied index ${indexOrName} was not found in table ${table.name}`,
             )
+        // old index may be passed without name. In this case we generate index name manually.
+        if (!index.name) index.name = this.generateIndexName(table, index)
 
         const up = this.dropIndexSql(table, index)
         const down = this.createIndexSql(table, index)

diff --git a/src/driver/sap/SapQueryRunner.ts b/src/driver/sap/SapQueryRunner.ts
@@ -2283,12 +2283,7 @@ export class SapQueryRunner extends BaseQueryRunner implements QueryRunner {
             : await this.getCachedTable(tableOrName)
 
         // new index may be passed without name. In this case we generate index name manually.
-        if (!index.name)
-            index.name = this.connection.namingStrategy.indexName(
-                table,
-                index.columnNames,
-                index.where,
-            )
+        if (!index.name) index.name = this.generateIndexName(table, index)
 
         const up = this.createIndexSql(table, index)
         const down = this.dropIndexSql(table, index)
@@ -2327,6 +2322,9 @@ export class SapQueryRunner extends BaseQueryRunner implements QueryRunner {
                 `Supplied index ${indexOrName} was not found in table ${table.name}`,
             )
 
+        // old index may be passed without name. In this case we generate index name manually.
+        if (!index.name) index.name = this.generateIndexName(table, index)
+
         const up = this.dropIndexSql(table, index)
         const down = this.createIndexSql(table, index)
         await this.executeQueries(up, down)

diff --git a/src/driver/spanner/SpannerQueryRunner.ts b/src/driver/spanner/SpannerQueryRunner.ts
@@ -1333,12 +1333,7 @@ export class SpannerQueryRunner extends BaseQueryRunner implements QueryRunner {
                 : await this.getCachedTable(tableOrName)
 
         // new index may be passed without name. In this case we generate index name manually.
-        if (!index.name)
-            index.name = this.connection.namingStrategy.indexName(
-                table,
-                index.columnNames,
-                index.where,
-            )
+        if (!index.name) index.name = this.generateIndexName(table, index)
 
         const up = this.createIndexSql(table, index)
         const down = this.dropIndexSql(table, index)
@@ -1378,6 +1373,9 @@ export class SpannerQueryRunner extends BaseQueryRunner implements QueryRunner {
                 `Supplied index ${indexOrName} was not found in table ${table.name}`,
             )
 
+        // new index may be passed without name. In this case we generate index name manually.
+        if (!index.name) index.name = this.generateIndexName(table, index)
+
         const up = this.dropIndexSql(table, index)
         const down = this.createIndexSql(table, index)
         await this.executeQueries(up, down)

diff --git a/src/driver/sqlite-abstract/AbstractSqliteQueryRunner.ts b/src/driver/sqlite-abstract/AbstractSqliteQueryRunner.ts
@@ -1089,12 +1089,7 @@ export abstract class AbstractSqliteQueryRunner
             : await this.getCachedTable(tableOrName)
 
         // new index may be passed without name. In this case we generate index name manually.
-        if (!index.name)
-            index.name = this.connection.namingStrategy.indexName(
-                table,
-                index.columnNames,
-                index.where,
-            )
+        if (!index.name) index.name = this.generateIndexName(table, index)
 
         const up = this.createIndexSql(table, index)
         const down = this.dropIndexSql(index)
@@ -1133,6 +1128,9 @@ export abstract class AbstractSqliteQueryRunner
                 `Supplied index ${indexOrName} was not found in table ${table.name}`,
             )
 
+        // old index may be passed without name. In this case we generate index name manually.
+        if (!index.name) index.name = this.generateIndexName(table, index)
+
         const up = this.dropIndexSql(index)
         const down = this.createIndexSql(table, index)
         await this.executeQueries(up, down)

diff --git a/src/driver/sqlserver/SqlServerQueryRunner.ts b/src/driver/sqlserver/SqlServerQueryRunner.ts
@@ -2484,12 +2484,7 @@ export class SqlServerQueryRunner
             : await this.getCachedTable(tableOrName)
 
         // new index may be passed without name. In this case we generate index name manually.
-        if (!index.name)
-            index.name = this.connection.namingStrategy.indexName(
-                table,
-                index.columnNames,
-                index.where,
-            )
+        if (!index.name) index.name = this.generateIndexName(table, index)
 
         const up = this.createIndexSql(table, index)
         const down = this.dropIndexSql(table, index)
@@ -2528,6 +2523,9 @@ export class SqlServerQueryRunner
                 `Supplied index was not found in table ${table.name}`,
             )
 
+        // old index may be passed without name. In this case we generate index name manually.
+        if (!index.name) index.name = this.generateIndexName(table, index)
+
         const up = this.dropIndexSql(table, index)
         const down = this.createIndexSql(table, index)
         await this.executeQueries(up, down)

diff --git a/src/query-runner/BaseQueryRunner.ts b/src/query-runner/BaseQueryRunner.ts
@@ -2,6 +2,7 @@ import { PostgresConnectionOptions } from "../driver/postgres/PostgresConnection
 import { Query } from "../driver/Query"
 import { SqlInMemory } from "../driver/SqlInMemory"
 import { SqlServerConnectionOptions } from "../driver/sqlserver/SqlServerConnectionOptions"
+import { TableIndex } from "../schema-builder/table/TableIndex"
 import { View } from "../schema-builder/view/View"
 import { DataSource } from "../data-source/DataSource"
 import { Table } from "../schema-builder/table/Table"
@@ -650,4 +651,16 @@ export abstract class BaseQueryRunner {
             await this.query(query, parameters)
         }
     }
+
+    /**
+     * Generated an index name for a table and index
+     */
+    protected generateIndexName(table: Table, index: TableIndex): string {
+        // new index may be passed without name. In this case we generate index name manually.
+        return this.connection.namingStrategy.indexName(
+            table,
+            index.columnNames,
+            index.where,
+        )
+    }
 }
diff --git a/test/github-issues/8936/entity/User.ts b/test/github-issues/8936/entity/User.ts
@@ -0,0 +1,16 @@
+import { Column, Entity, PrimaryColumn } from "../../../../src"
+
+@Entity()
+export class User {
+    @PrimaryColumn({ nullable: false })
+    id: number
+
+    @Column()
+    firstName: string
+
+    @Column()
+    lastName: string
+
+    @Column()
+    github: string
+}
diff --git a/test/github-issues/8936/issue-8936.ts b/test/github-issues/8936/issue-8936.ts
@@ -0,0 +1,56 @@
+import "../../utils/test-setup"
+import {
+    QueryFailedError,
+    QueryRunner,
+    Repository,
+    TableIndex,
+} from "../../../src"
+import {
+    createTestingConnections,
+    closeTestingConnections,
+    reloadTestingDatabases,
+} from "../../utils/test-utils"
+import { Connection } from "../../../src/connection/Connection"
+import { User } from "./entity/User"
+
+describe("github issues > #8936 DropIndex with a TableIndex without name is not working", () => {
+    let connections: Connection[]
+
+    const tableIndex: TableIndex = new TableIndex({
+        columnNames: ["firstName", "lastName"],
+        isUnique: true,
+    })
+
+    before(
+        async () =>
+            (connections = await createTestingConnections({
+                entities: [__dirname + "/entity/*{.js,.ts}"],
+                schemaCreate: true,
+                dropSchema: true,
+            })),
+    )
+    beforeEach(() => reloadTestingDatabases(connections))
+    after(() => closeTestingConnections(connections))
+
+    it("should drop the index as expected", () => {
+        // Create a clone because the createIndex will set the name
+        const dropTableIndex: TableIndex = tableIndex.clone()
+
+        return Promise.all(
+            connections.map(async (connection) => {
+                const queryRunner: QueryRunner = connection.createQueryRunner()
+                const userRepository: Repository<User> =
+                    connection.getRepository(User)
+                const tableName: string = userRepository.metadata.tableName
+
+                // Create the index so it exists when we delete it
+                await queryRunner.createIndex(tableName, tableIndex)
+
+                // Drop the index expecting it not to raise QueryFailed
+                await queryRunner
+                    .dropIndex(tableName, dropTableIndex)
+                    .should.not.be.rejectedWith(QueryFailedError)
+            }),
+        )
+    })
+})