From b2c8168514b23671080e6d384e381e997fbaa11e Mon Sep 17 00:00:00 2001 From: Matteias Collet Date: Sun, 14 Nov 2021 17:02:42 +0100 Subject: [PATCH] fix: create typeorm_metadata table when running migrations (#4956) * fix: create typeorm_metatable when running migrations * remove redundant check * create typeorm_metadata table in MigrationExecutor * remove redundant call to createMetadataTableIfNecessary * move create metadata table logic * add missing test (#4956) * fix migration name * pass query runner instance to metadata creation * do not create metadata table in log() Avoid creating the metatable when you don't intend to modify the database, such as when generating migrations --- src/migration/MigrationExecutor.ts | 11 ++- src/schema-builder/RdbmsSchemaBuilder.ts | 27 ++++--- test/github-issues/4956/entities/Foo.ts | 12 +++ test/github-issues/4956/entities/FooView.ts | 13 ++++ test/github-issues/4956/issue-4956.ts | 78 +++++++++++++++++++ .../github-issues/4956/migrations/WithView.ts | 28 +++++++ .../4956/migrations/WithoutView.ts | 15 ++++ 7 files changed, 169 insertions(+), 15 deletions(-) create mode 100644 test/github-issues/4956/entities/Foo.ts create mode 100644 test/github-issues/4956/entities/FooView.ts create mode 100644 test/github-issues/4956/issue-4956.ts create mode 100644 test/github-issues/4956/migrations/WithView.ts create mode 100644 test/github-issues/4956/migrations/WithoutView.ts diff --git a/src/migration/MigrationExecutor.ts b/src/migration/MigrationExecutor.ts index ae810d9ea9..c3700a7b0d 100644 --- a/src/migration/MigrationExecutor.ts +++ b/src/migration/MigrationExecutor.ts @@ -5,9 +5,10 @@ import {ObjectLiteral} from "../common/ObjectLiteral"; import {QueryRunner} from "../query-runner/QueryRunner"; import {SqlServerDriver} from "../driver/sqlserver/SqlServerDriver"; import {MssqlParameter} from "../driver/sqlserver/MssqlParameter"; +import {RdbmsSchemaBuilder} from "../schema-builder/RdbmsSchemaBuilder"; import {MongoDriver} from "../driver/mongodb/MongoDriver"; import {MongoQueryRunner} from "../driver/mongodb/MongoQueryRunner"; -import { TypeORMError } from "../error"; +import {TypeORMError} from "../error"; /** * Executes migrations: runs pending and reverts previously executed migrations. @@ -157,6 +158,14 @@ export class MigrationExecutor { const queryRunner = this.queryRunner || this.connection.createQueryRunner(); // create migrations table if its not created yet await this.createMigrationsTableIfNotExist(queryRunner); + + // create the typeorm_metadata table if necessary + const schemaBuilder = this.connection.driver.createSchemaBuilder(); + + if (schemaBuilder instanceof RdbmsSchemaBuilder) { + await schemaBuilder.createMetadataTableIfNecessary(queryRunner); + } + // get all migrations that are executed and saved in the database const executedMigrations = await this.loadExecutedMigrations(queryRunner); diff --git a/src/schema-builder/RdbmsSchemaBuilder.ts b/src/schema-builder/RdbmsSchemaBuilder.ts index a84581bd59..ea95dc3b72 100644 --- a/src/schema-builder/RdbmsSchemaBuilder.ts +++ b/src/schema-builder/RdbmsSchemaBuilder.ts @@ -17,7 +17,7 @@ import {TableUnique} from "./table/TableUnique"; import {TableCheck} from "./table/TableCheck"; import {TableExclusion} from "./table/TableExclusion"; import {View} from "./view/View"; -import { ViewUtils } from "./util/ViewUtils"; +import {ViewUtils} from "./util/ViewUtils"; import {AuroraDataApiDriver} from "../driver/aurora-data-api/AuroraDataApiDriver"; /** @@ -77,10 +77,7 @@ export class RdbmsSchemaBuilder implements SchemaBuilder { } try { - if (this.viewEntityToSyncMetadatas.length > 0 || (this.connection.driver instanceof PostgresDriver && this.connection.driver.isGeneratedColumnsSupported)) { - await this.createTypeormMetadataTable(); - } - + await this.createMetadataTableIfNecessary(this.queryRunner); // Flush the queryrunner table & view cache const tablePaths = this.entityToSyncMetadatas.map(metadata => this.getTablePath(metadata)); @@ -111,19 +108,21 @@ export class RdbmsSchemaBuilder implements SchemaBuilder { } } + /** + * If the schema contains views, create the typeorm_metadata table if it doesn't exist yet + */ + async createMetadataTableIfNecessary(queryRunner: QueryRunner): Promise { + if (this.viewEntityToSyncMetadatas.length > 0 || (this.connection.driver instanceof PostgresDriver && this.connection.driver.isGeneratedColumnsSupported)) { + await this.createTypeormMetadataTable(queryRunner); + } + } + /** * Returns sql queries to be executed by schema builder. */ async log(): Promise { this.queryRunner = this.connection.createQueryRunner(); try { - - // TODO: typeorm_metadata table needs only for Views for now. - // Remove condition or add new conditions if necessary (for CHECK constraints for example). - if (this.viewEntityToSyncMetadatas.length > 0) { - await this.createTypeormMetadataTable(); - } - // Flush the queryrunner table & view cache const tablePaths = this.entityToSyncMetadatas.map(metadata => this.getTablePath(metadata)); await this.queryRunner.getTables(tablePaths); @@ -831,12 +830,12 @@ export class RdbmsSchemaBuilder implements SchemaBuilder { /** * Creates typeorm service table for storing user defined Views. */ - protected async createTypeormMetadataTable() { + protected async createTypeormMetadataTable(queryRunner: QueryRunner) { const schema = this.currentSchema; const database = this.currentDatabase; const typeormMetadataTable = this.connection.driver.buildTableName("typeorm_metadata", schema, database); - await this.queryRunner.createTable(new Table( + await queryRunner.createTable(new Table( { database: database, schema: schema, diff --git a/test/github-issues/4956/entities/Foo.ts b/test/github-issues/4956/entities/Foo.ts new file mode 100644 index 0000000000..1f6ac1f9fc --- /dev/null +++ b/test/github-issues/4956/entities/Foo.ts @@ -0,0 +1,12 @@ +import { PrimaryGeneratedColumn, UpdateDateColumn } from "../../../../src"; + +import { Entity } from "../../../../src/decorator/entity/Entity"; + +@Entity("foo") +export class Foo { + @PrimaryGeneratedColumn({ name: "id" }) + id: number; + + @UpdateDateColumn({ name: "updated_at" }) + updatedAt: Date; +} diff --git a/test/github-issues/4956/entities/FooView.ts b/test/github-issues/4956/entities/FooView.ts new file mode 100644 index 0000000000..bfe5821f4d --- /dev/null +++ b/test/github-issues/4956/entities/FooView.ts @@ -0,0 +1,13 @@ +import { Connection, ViewColumn, ViewEntity } from "../../../../src"; + +import { Foo } from "./Foo"; + +@ViewEntity({ + name: "foo_view", + expression: (connection: Connection) => + connection.createQueryBuilder(Foo, "foo").select(`foo.updatedAt`), +}) +export class FooView { + @ViewColumn() + updatedAt: Date; +} diff --git a/test/github-issues/4956/issue-4956.ts b/test/github-issues/4956/issue-4956.ts new file mode 100644 index 0000000000..95e5661920 --- /dev/null +++ b/test/github-issues/4956/issue-4956.ts @@ -0,0 +1,78 @@ +import "reflect-metadata"; + +import { + closeTestingConnections, + createTestingConnections, +} from "../../utils/test-utils"; + +import { Connection } from "../../../src/connection/Connection"; +import { afterEach } from "mocha"; +import { expect } from "chai"; + +describe("github issues > #4956 create typeorm_metatable when running migrations.", () => { + let connections: Connection[]; + + afterEach(async () => { + await closeTestingConnections(connections); + }); + + it("should create typeorm_metadata table when running migrations with views", async () => { + connections = await createTestingConnections({ + entities: [__dirname + "/entities/*{.js,.ts}"], + migrations: [__dirname + "/migrations/WithView{.js,.ts}"], + enabledDrivers: ["mysql", "mariadb"], + schemaCreate: false, + dropSchema: true, + }); + + await Promise.all( + connections.map(async (connection) => { + const queryRunner = connection.createQueryRunner(); + const typeormMetadataTableName = "typeorm_metadata"; + + const hasMetadataTable = await queryRunner.hasTable( + typeormMetadataTableName + ); + + expect(hasMetadataTable).to.be.false; + + await connection.runMigrations(); + + const hasPostMigrationMetadataTable = + await queryRunner.hasTable(typeormMetadataTableName); + + expect(hasPostMigrationMetadataTable).to.be.true; + }) + ); + }); + + it("should not create typeorm_metadata table when running migrations if there are no views", async () => { + connections = await createTestingConnections({ + entities: [__dirname + "/entities/Foo{.js,.ts}"], + migrations: [__dirname + "/migrations/WithoutView{.js,.ts}"], + enabledDrivers: ["mysql", "mariadb"], + schemaCreate: false, + dropSchema: true, + }); + + await Promise.all( + connections.map(async (connection) => { + const queryRunner = connection.createQueryRunner(); + const typeormMetadataTableName = "typeorm_metadata"; + + const hasMetadataTable = await queryRunner.hasTable( + typeormMetadataTableName + ); + + expect(hasMetadataTable).to.be.false; + + await connection.runMigrations(); + + const hasPostMigrationMetadataTable = + await queryRunner.hasTable(typeormMetadataTableName); + + expect(hasPostMigrationMetadataTable).to.be.false; + }) + ); + }); +}); diff --git a/test/github-issues/4956/migrations/WithView.ts b/test/github-issues/4956/migrations/WithView.ts new file mode 100644 index 0000000000..38e01c3410 --- /dev/null +++ b/test/github-issues/4956/migrations/WithView.ts @@ -0,0 +1,28 @@ +import { MigrationInterface, QueryRunner } from "../../../../src"; + +export class WithView1623518107000 implements MigrationInterface { + name = "WithView1623518107000"; + + public async up(queryRunner: QueryRunner): Promise { + await queryRunner.query( + "CREATE TABLE `foo` (`id` int NOT NULL AUTO_INCREMENT, `updated_at` datetime(6) NOT NULL DEFAULT CURRENT_TIMESTAMP(6) ON UPDATE CURRENT_TIMESTAMP(6), PRIMARY KEY (`id`)) ENGINE=InnoDB" + ); + await queryRunner.query( + "CREATE VIEW `foo_view` AS SELECT updated_at FROM `foo`" + ); + await queryRunner.query( + "INSERT INTO `typeorm_metadata`(`type`, `schema`, `name`, `value`) VALUES (?, ?, ?, ?)", + ["VIEW", null, "foo_view", "SELECT `updated_at` FROM `foo`"] + ); + } + + public async down(queryRunner: QueryRunner): Promise { + await queryRunner.query( + "DELETE FROM `typeorm_metadata` WHERE `type` = 'VIEW' AND `schema` = ? AND `name` = ?", + [null, "foo_view"] + ); + + await queryRunner.query("DROP VIEW `foo_view`"); + await queryRunner.query("DROP Table `foo`"); + } +} diff --git a/test/github-issues/4956/migrations/WithoutView.ts b/test/github-issues/4956/migrations/WithoutView.ts new file mode 100644 index 0000000000..47b5ac2319 --- /dev/null +++ b/test/github-issues/4956/migrations/WithoutView.ts @@ -0,0 +1,15 @@ +import { MigrationInterface, QueryRunner } from "../../../../src"; + +export class WithoutView1623518107000 implements MigrationInterface { + name = "WithoutView1623518107000"; + + public async up(queryRunner: QueryRunner): Promise { + await queryRunner.query( + "CREATE TABLE `foo` (`id` int NOT NULL AUTO_INCREMENT, `updated_at` datetime(6) NOT NULL DEFAULT CURRENT_TIMESTAMP(6) ON UPDATE CURRENT_TIMESTAMP(6), PRIMARY KEY (`id`)) ENGINE=InnoDB" + ); + } + + public async down(queryRunner: QueryRunner): Promise { + await queryRunner.query("DROP Table `foo`"); + } +}