From 27e02da6ee457c86f60f3751d9c96076a057f689 Mon Sep 17 00:00:00 2001 From: James Ward Date: Tue, 24 Aug 2021 14:49:07 -0700 Subject: [PATCH] fix: use full path for table lookups (#8097) The changes made as part of #7575 fixed some issues and introduced others. The problem became that the table name would be compared against the EntityMetadata tablePath. This would fail in some cases because while the EntityMetadata would specify database, it'd be the "default"/current database & would be omitted from the table name. To work around that this PR introduces the database & schema on the table objects as well as a fully-qualified table path property which will always include all of them for comparing against EntityMetadata. --- src/driver/Driver.ts | 25 +- src/driver/mysql/MysqlDriver.ts | 95 ++++-- src/driver/mysql/MysqlQueryRunner.ts | 35 +- src/driver/postgres/PostgresDriver.ts | 130 ++++--- src/driver/postgres/PostgresQueryRunner.ts | 33 +- .../sqlite-abstract/AbstractSqliteDriver.ts | 53 +-- .../AbstractSqliteQueryRunner.ts | 14 + src/metadata/EntityMetadata.ts | 11 +- src/query-runner/BaseQueryRunner.ts | 73 +++- src/query-runner/QueryRunner.ts | 10 + src/schema-builder/RdbmsSchemaBuilder.ts | 72 ++-- src/schema-builder/options/ViewOptions.ts | 10 + src/schema-builder/table/Table.ts | 27 +- src/schema-builder/table/TableForeignKey.ts | 16 + src/schema-builder/view/View.ts | 16 + .../custom-db-and-schema-sync.ts | 316 ++++++++++++++---- test/github-issues/7867/entity/Example.ts | 10 + test/github-issues/7867/issue-7867.ts | 82 +++++ 18 files changed, 753 insertions(+), 275 deletions(-) create mode 100644 test/github-issues/7867/entity/Example.ts create mode 100644 test/github-issues/7867/issue-7867.ts diff --git a/src/driver/Driver.ts b/src/driver/Driver.ts index 3a82419006..8fbf0f707c 100644 --- a/src/driver/Driver.ts +++ b/src/driver/Driver.ts @@ -14,6 +14,9 @@ import {Expression} from "../expression-builder/Expression"; import {ColumnMode} from "../metadata-args/types/ColumnMode"; import {ColumnOptions} from "../decorator/options/ColumnOptions"; import {DriverConnectionOptions} from "../data-source/DriverConnectionOptions"; +import {Table} from "../schema-builder/table/Table"; +import {View} from "../schema-builder/view/View"; +import {TableForeignKey} from "../schema-builder/table/TableForeignKey"; /** * Driver organizes TypeORM communication with specific database management system. @@ -31,12 +34,17 @@ export interface Driver { options: DriverConnectionOptions; /** - * Master database used to perform all write queries. + * Database name used to perform all write queries. * * todo: probably move into query runner. */ database?: string; + /** + * Schema name used to perform all write queries. + */ + schema?: string; + /** * Indicates if replication is enabled. */ @@ -115,12 +123,6 @@ export interface Driver { */ createRepository?(): Repository; - /** - * Replaces parameters in the given sql with special escaping character - * and an array of parameter names to be passed to a query. - */ - escapeQueryWithParameters(sql: string, parameters: ObjectLiteral, nativeParameters: ObjectLiteral): [string, any[]]; - /** * Escapes a table name, column name or an alias. * @@ -130,12 +132,15 @@ export interface Driver { /** * Build full table path with database name, schema name and table name. - * E.g. "myDB"."mySchema"."myTable" - * - * TODO: Rename to buildTablePath + * E.g. myDB.mySchema.myTable */ buildTableName(tableName: string, schema?: string, database?: string): string; + /** + * Parse a target table name or other types and return a normalized table definition. + */ + parseTableName(target: EntityMetadata | Table | View | TableForeignKey | string): { tableName: string, schema?: string, database?: string }; + /** * Build full schema path with database name and schema name. * E.g. "myDB"."mySchema" diff --git a/src/driver/mysql/MysqlDriver.ts b/src/driver/mysql/MysqlDriver.ts index 6e16967ff8..b0c26bfda0 100644 --- a/src/driver/mysql/MysqlDriver.ts +++ b/src/driver/mysql/MysqlDriver.ts @@ -26,6 +26,9 @@ import {Expression, ExpressionBuilder} from "../../expression-builder/Expression import {Fn} from "../../expression-builder/expression/Function"; import {CurrentTimestamp} from "../../expression-builder/expression/datetime/CurrentTimestamp"; import {DefaultExpressionBuildInterface} from "../../expression-builder/DefaultExpressionBuildInterface"; +import {Table} from "../../schema-builder/table/Table"; +import {View} from "../../schema-builder/view/View"; +import {TableForeignKey} from "../../schema-builder/table/TableForeignKey"; /** * Organizes communication with MySQL DBMS. @@ -101,9 +104,15 @@ export class MysqlDriver implements Driver { /** * Master database used to perform all write queries. + * Database name used to perform all write queries. */ database?: string; + /** + * Schema name used to performn all write queries. + */ + schema?: string; + /** * Indicates if replication is enabled. */ @@ -349,7 +358,7 @@ export class MysqlDriver implements Driver { // load mysql package this.loadDependencies(); - this.database = this.options.replication ? this.options.replication.master.database : this.options.database; + this.database = DriverUtils.buildDriverOptions(this.options.replication ? this.options.replication.master : this.options).database; // validate options to make sure everything is set // todo: revisit validation with replication in mind @@ -382,6 +391,14 @@ export class MysqlDriver implements Driver { } else { this.pool = await this.createPool(this.createConnectionOptions(this.options, this.options)); } + + if (this.database === undefined) { + const queryRunner = await this.createQueryRunner("master"); + + this.database = await queryRunner.getCurrentDatabase(); + + await queryRunner.release(); + } } /** @@ -420,35 +437,6 @@ export class MysqlDriver implements Driver { return new MysqlQueryRunner(this, mode); } - /** - * Replaces parameters in the given sql with special escaping character - * and an array of parameter names to be passed to a query. - */ - escapeQueryWithParameters(sql: string, parameters: ObjectLiteral, nativeParameters: ObjectLiteral): [string, any[]] { - const escapedParameters: any[] = Object.keys(nativeParameters).map(key => nativeParameters[key]); - if (!parameters || !Object.keys(parameters).length) - return [sql, escapedParameters]; - - const keys = Object.keys(parameters).map(parameter => "(:(\\.\\.\\.)?" + parameter + "\\b)").join("|"); - sql = sql.replace(new RegExp(keys, "g"), (key: string) => { - let value: any; - if (key.substr(0, 4) === ":...") { - value = parameters[key.substr(4)]; - } else { - value = parameters[key.substr(1)]; - } - - if (value instanceof Function) { - return value(); - - } else { - escapedParameters.push(value); - return "?"; - } - }); // todo: make replace only in value statements, otherwise problems - return [sql, escapedParameters]; - } - /** * Escapes a column name. */ @@ -464,6 +452,53 @@ export class MysqlDriver implements Driver { return database ? `${database}.${tableName}` : tableName; } + /** + * Parse a target table name or other types and return a normalized table definition. + */ + parseTableName(target: EntityMetadata | Table | View | TableForeignKey | string): { database?: string, schema?: string, tableName: string } { + const driverDatabase = this.database; + const driverSchema = undefined; + + if (target instanceof Table || target instanceof View) { + const parsed = this.parseTableName(target.name); + + return { + database: target.database || parsed.database || driverDatabase, + schema: target.schema || parsed.schema || driverSchema, + tableName: parsed.tableName + }; + } + + if (target instanceof TableForeignKey) { + const parsed = this.parseTableName(target.referencedTableName); + + return { + database: target.referencedDatabase || parsed.database || driverDatabase, + schema: target.referencedSchema || parsed.schema || driverSchema, + tableName: parsed.tableName + }; + } + + if (target instanceof EntityMetadata) { + // EntityMetadata tableName is never a path + + return { + database: target.database || driverDatabase, + schema: target.schema || driverSchema, + tableName: target.tableName + }; + + } + + const parts = target.split("."); + + return { + database: (parts.length > 1 ? parts[0] : undefined) || driverDatabase, + schema: driverSchema, + tableName: parts.length > 1 ? parts[1] : parts[0] + }; + } + /** * Wraps given value in any additional expressions required based on its column type and metadata. */ diff --git a/src/driver/mysql/MysqlQueryRunner.ts b/src/driver/mysql/MysqlQueryRunner.ts index cccc744c43..e301d1e742 100644 --- a/src/driver/mysql/MysqlQueryRunner.ts +++ b/src/driver/mysql/MysqlQueryRunner.ts @@ -242,6 +242,14 @@ export class MysqlQueryRunner extends BaseQueryRunner implements QueryRunner { return result.length > 0; } + /** + * Loads currently using database + */ + async getCurrentDatabase(): Promise { + const query = await this.query(`SELECT DATABASE() AS \`db_name\``); + return query[0]["db_name"]; + } + /** * Checks if schema with the given name exist. */ @@ -249,6 +257,14 @@ export class MysqlQueryRunner extends BaseQueryRunner implements QueryRunner { throw new Error(`MySql driver does not support table schemas`); } + /** + * Loads currently using database schema + */ + async getCurrentSchema(): Promise { + const query = await this.query(`SELECT SCHEMA() AS \`schema_name\``); + return query[0]["schema_name"]; + } + /** * Checks if table with the given name exist in the database. */ @@ -1160,14 +1176,6 @@ export class MysqlQueryRunner extends BaseQueryRunner implements QueryRunner { // Protected Methods // ------------------------------------------------------------------------- - /** - * Returns current database. - */ - protected async getCurrentDatabase(): Promise { - const currentDBQuery = await this.query(`SELECT DATABASE() AS \`db_name\``); - return currentDBQuery[0]["db_name"]; - } - protected async loadViews(viewNames: string[]): Promise { const hasTable = await this.hasTable(this.getTypeormMetadataTableName()); if (!hasTable) return []; @@ -1808,9 +1816,14 @@ export class MysqlQueryRunner extends BaseQueryRunner implements QueryRunner { /** * Escapes given table or view path. */ - protected escapePath(target: Table|View|string, disableEscape?: boolean): string { - const tableName = target instanceof Table || target instanceof View ? target.name : target; - return tableName.split(".").map(i => disableEscape ? i : `\`${i}\``).join("."); + protected escapePath(target: Table|View|string): string { + const { database, tableName } = this.driver.parseTableName(target); + + if (database && database !== this.driver.database) { + return `\`${database}\`.\`${tableName}\``; + } + + return `\`${tableName}\``; } /** diff --git a/src/driver/postgres/PostgresDriver.ts b/src/driver/postgres/PostgresDriver.ts index d191b0472b..1993c13548 100644 --- a/src/driver/postgres/PostgresDriver.ts +++ b/src/driver/postgres/PostgresDriver.ts @@ -24,6 +24,10 @@ import {LockNotSupportedOnGivenDriverError} from "../../error/LockNotSupportedOn import {Expression} from "../../expression-builder/Expression"; import {Fn} from "../../expression-builder/expression/Function"; import {Cast} from "../../expression-builder/expression/misc/Cast"; +import {DriverUtils} from "../DriverUtils"; +import {Table} from "../../schema-builder/table/Table"; +import {View} from "../../schema-builder/view/View"; +import {TableForeignKey} from "../../schema-builder/table/TableForeignKey"; /** * Organizes communication with PostgreSQL DBMS. @@ -118,10 +122,25 @@ export class PostgresDriver implements Driver { options: PostgresDataSourceOptions; /** - * Master database used to perform all write queries. + * Database name used to perform all write queries. */ database?: string; + /** + * Schema name used to perform all write queries. + */ + schema?: string; + + /** + * Schema that's used internally by Postgres for object resolution. + * + * Because we never set this we have to track it in separately from the `schema` so + * we know when we have to specify the full schema or not. + * + * In most cases this will be `public`. + */ + searchSchema?: string; + /** * Indicates if replication is enabled. */ @@ -317,6 +336,9 @@ export class PostgresDriver implements Driver { // load postgres package this.loadDependencies(); + this.database = DriverUtils.buildDriverOptions(this.options.replication ? this.options.replication.master : this.options).database; + this.schema = DriverUtils.buildDriverOptions(this.options).schema; + // Object.assign(this.options, DriverUtils.buildDriverOptions(data-source.options)); // todo: do it better way // validate options to make sure everything is set // todo: revisit validation with replication in mind @@ -338,17 +360,31 @@ export class PostgresDriver implements Driver { * either create a pool and create data-source when needed. */ async connect(): Promise { - if (this.options.replication) { this.slaves = await Promise.all(this.options.replication.slaves.map(slave => { return this.createPool(this.options, slave); })); this.master = await this.createPool(this.options, this.options.replication.master); - this.database = this.options.replication.master.database; - } else { this.master = await this.createPool(this.options, this.options); - this.database = this.options.database; + } + + if (!this.database || !this.searchSchema) { + const queryRunner = await this.createQueryRunner("master"); + + if (!this.database) { + this.database = await queryRunner.getCurrentDatabase(); + } + + if (!this.searchSchema) { + this.searchSchema = await queryRunner.getCurrentSchema(); + } + + await queryRunner.release(); + } + + if (!this.schema) { + this.schema = this.searchSchema; } } @@ -678,43 +714,6 @@ export class PostgresDriver implements Driver { return value; } - /** - * Replaces parameters in the given sql with special escaping character - * and an array of parameter names to be passed to a query. - */ - escapeQueryWithParameters(sql: string, parameters: ObjectLiteral, nativeParameters: ObjectLiteral): [string, any[]] { - const builtParameters: any[] = Object.keys(nativeParameters).map(key => nativeParameters[key]); - if (!parameters || !Object.keys(parameters).length) - return [sql, builtParameters]; - - const keys = Object.keys(parameters).map(parameter => "(:(\\.\\.\\.)?" + parameter + "\\b)").join("|"); - sql = sql.replace(new RegExp(keys, "g"), (key: string): string => { - let value: any; - let isArray = false; - if (key.substr(0, 4) === ":...") { - isArray = true; - value = parameters[key.substr(4)]; - } else { - value = parameters[key.substr(1)]; - } - - if (isArray) { - return value.map((v: any) => { - builtParameters.push(v); - return "$" + builtParameters.length; - }).join(", "); - - } else if (value instanceof Function) { - return value(); - - } else { - builtParameters.push(value); - return "$" + builtParameters.length; - } - }); // todo: make replace only in value statements, otherwise problems - return [sql, builtParameters]; - } - /** * Escapes a column name. */ @@ -731,11 +730,50 @@ export class PostgresDriver implements Driver { } /** - * Build full schema path with database name and schema name. - * E.g. "myDB"."mySchema" + * Parse a target table name or other types and return a normalized table definition. */ - buildSchemaPath(schema?: string): string | undefined { - return schema; + parseTableName(target: EntityMetadata | Table | View | TableForeignKey | string): { database?: string, schema?: string, tableName: string } { + const driverDatabase = this.database; + const driverSchema = this.schema; + + if (target instanceof Table || target instanceof View) { + const parsed = this.parseTableName(target.name); + + return { + database: target.database || parsed.database || driverDatabase, + schema: target.schema || parsed.schema || driverSchema, + tableName: parsed.tableName + }; + } + + if (target instanceof TableForeignKey) { + const parsed = this.parseTableName(target.referencedTableName); + + return { + database: target.referencedDatabase || parsed.database || driverDatabase, + schema: target.referencedSchema || parsed.schema || driverSchema, + tableName: parsed.tableName + }; + } + + if (target instanceof EntityMetadata) { + // EntityMetadata tableName is never a path + + return { + database: target.database || driverDatabase, + schema: target.schema || driverSchema, + tableName: target.tableName + } + + } + + const parts = target.split(".") + + return { + database: driverDatabase, + schema: (parts.length > 1 ? parts[0] : undefined) || driverSchema, + tableName: parts.length > 1 ? parts[1] : parts[0], + }; } /** diff --git a/src/driver/postgres/PostgresQueryRunner.ts b/src/driver/postgres/PostgresQueryRunner.ts index a160eab47d..af76a6021b 100644 --- a/src/driver/postgres/PostgresQueryRunner.ts +++ b/src/driver/postgres/PostgresQueryRunner.ts @@ -259,6 +259,14 @@ export class PostgresQueryRunner extends BaseQueryRunner implements QueryRunner return false; } + /** + * Loads currently using database + */ + async getCurrentDatabase(): Promise { + const query = await this.query(`SELECT * FROM current_database()`); + return query[0]["current_database"]; + } + /** * Checks if schema with the given name exist. */ @@ -267,6 +275,14 @@ export class PostgresQueryRunner extends BaseQueryRunner implements QueryRunner return result.length > 0; } + /** + * Loads currently using database schema + */ + async getCurrentSchema(): Promise { + const query = await this.query(`SELECT * FROM current_schema()`); + return query[0]["current_schema"]; + } + /** * Checks if table with the given name exist in the database. */ @@ -1351,8 +1367,7 @@ export class PostgresQueryRunner extends BaseQueryRunner implements QueryRunner const hasTable = await this.hasTable(this.getTypeormMetadataTableName()); if (!hasTable) return []; - const currentSchemaQuery = await this.query(`SELECT * FROM current_schema()`); - const currentSchema = currentSchemaQuery[0]["current_schema"]; + const currentSchema = await this.getCurrentSchema(); const viewsCondition = viewNames.map(viewName => { let [schema, name] = viewName.split("."); @@ -1384,8 +1399,7 @@ export class PostgresQueryRunner extends BaseQueryRunner implements QueryRunner if (!tableNames || !tableNames.length) return []; - const currentSchemaQuery = await this.query(`SELECT * FROM current_schema()`); - const currentSchema = currentSchemaQuery[0]["current_schema"]; + const currentSchema = await this.getCurrentSchema(); const tablesCondition = tableNames.map(tableName => { let [schema, name] = tableName.split("."); @@ -1496,9 +1510,13 @@ export class PostgresQueryRunner extends BaseQueryRunner implements QueryRunner return Promise.all(dbTables.map(async dbTable => { const table = new Table(); - const getSchemaFromKey = (dbObject: any, key: string) => dbObject[key] === currentSchema && !this.driver.options.schema ? undefined : dbObject[key]; + const getSchemaFromKey = (dbObject: any, key: string) => { + return dbObject[key] === currentSchema && (!this.driver.options.schema || this.driver.options.schema === currentSchema) + ? undefined + : dbObject[key] + }; // We do not need to join schema name, when database is by default. - // In this case we need local variable `tableFullName` for below comparision. + // In this case we need local variable `tableFullName` for below comparison. const schema = getSchemaFromKey(dbTable, "table_schema"); table.name = this.driver.buildTableName(dbTable["table_name"], schema); const tableFullName = this.driver.buildTableName(dbTable["table_name"], dbTable["table_schema"]); @@ -1827,8 +1845,7 @@ export class PostgresQueryRunner extends BaseQueryRunner implements QueryRunner } protected async insertViewDefinitionSql(view: View): Promise { - const currentSchemaQuery = await this.query(`SELECT * FROM current_schema()`); - const currentSchema = currentSchemaQuery[0]["current_schema"]; + const currentSchema = await this.getCurrentSchema(); const splittedName = view.name.split("."); let schema = this.driver.options.schema || currentSchema; let name = view.name; diff --git a/src/driver/sqlite-abstract/AbstractSqliteDriver.ts b/src/driver/sqlite-abstract/AbstractSqliteDriver.ts index 5fdc3b8bcb..6129e76d08 100644 --- a/src/driver/sqlite-abstract/AbstractSqliteDriver.ts +++ b/src/driver/sqlite-abstract/AbstractSqliteDriver.ts @@ -73,9 +73,15 @@ export abstract class AbstractSqliteDriver implements Driver { /** * Master database used to perform all write queries. + * Database name used to perform all write queries. */ database?: string; + /** + * Schema name used to performn all write queries. + */ + schema?: string; + /** * Indicates if replication is enabled. */ @@ -362,53 +368,6 @@ export abstract class AbstractSqliteDriver implements Driver { return value; } - /** - * Replaces parameters in the given sql with special escaping character - * and an array of parameter names to be passed to a query. - */ - escapeQueryWithParameters(sql: string, parameters: ObjectLiteral, nativeParameters: ObjectLiteral): [string, any[]] { - const builtParameters: any[] = Object.keys(nativeParameters).map(key => { - // Mapping boolean values to their numeric representation - if (typeof nativeParameters[key] === "boolean") { - return nativeParameters[key] === true ? 1 : 0; - } - - return nativeParameters[key]; - }); - - if (!parameters || !Object.keys(parameters).length) - return [sql, builtParameters]; - - const keys = Object.keys(parameters).map(parameter => "(:(\\.\\.\\.)?" + parameter + "\\b)").join("|"); - sql = sql.replace(new RegExp(keys, "g"), (key: string): string => { - let value: any; - let isArray = false; - if (key.substr(0, 4) === ":...") { - isArray = true; - value = parameters[key.substr(4)]; - } else { - value = parameters[key.substr(1)]; - } - - if (isArray) { - return value.map((v: any) => { - builtParameters.push(v); - return "?"; - // return "$" + builtParameters.length; - }).join(", "); - - } else if (value instanceof Function) { - return value(); - - } else { - builtParameters.push(value); - return "?"; - // return "$" + builtParameters.length; - } - }); // todo: make replace only in value statements, otherwise problems - return [sql, builtParameters]; - } - /** * Escapes a column name. */ diff --git a/src/driver/sqlite-abstract/AbstractSqliteQueryRunner.ts b/src/driver/sqlite-abstract/AbstractSqliteQueryRunner.ts index 692df83f09..dbf509bde9 100644 --- a/src/driver/sqlite-abstract/AbstractSqliteQueryRunner.ts +++ b/src/driver/sqlite-abstract/AbstractSqliteQueryRunner.ts @@ -140,6 +140,13 @@ export abstract class AbstractSqliteQueryRunner extends BaseQueryRunner implemen return false; } + /** + * Loads currently using database + */ + async getCurrentDatabase(): Promise { + return undefined; + } + /** * Checks if schema with the given name exist. */ @@ -147,6 +154,13 @@ export abstract class AbstractSqliteQueryRunner extends BaseQueryRunner implemen throw new Error(`This driver does not support table schemas`); } + /** + * Loads currently using database schema + */ + async getCurrentSchema(): Promise { + return undefined; + } + /** * Checks if table with the given name exist in the database. */ diff --git a/src/metadata/EntityMetadata.ts b/src/metadata/EntityMetadata.ts index 716f67cb42..55e3c2fdb9 100644 --- a/src/metadata/EntityMetadata.ts +++ b/src/metadata/EntityMetadata.ts @@ -105,12 +105,6 @@ export class EntityMetadata { */ tablePath: string; - /** - * Entity schema path. Contains database name and schema name. - * E.g. myDB.mySchema - */ - schemaPath?: string; - /** * Indicates if schema will be synchronized for this entity or not. */ @@ -565,8 +559,8 @@ export class EntityMetadata { this.database = this.tableMetadataArgs.database; if (this.tableMetadataArgs.schema) { this.schema = this.tableMetadataArgs.schema; - } else { - this.schema = (this.connection.options as PostgresDataSourceOptions).schema; + } else if (this.connection.options?.hasOwnProperty("schema")) { + this.schema = (this.connection.options as any).schema; } this.givenTableName = this.tableMetadataArgs.name; this.synchronize = this.tableMetadataArgs.synchronize !== false; @@ -576,7 +570,6 @@ export class EntityMetadata { this.expression = this.tableMetadataArgs.expression; this.withoutRowid = this.tableMetadataArgs.withoutRowid === true; this.tablePath = this.connection.driver.buildTableName(this.tableName, this.schema, this.database); - this.schemaPath = this.connection.driver.buildSchemaPath?.(this.schema, this.database); this.orderBy = this.tableMetadataArgs.orderBy; this.isJunction = this.tableMetadataArgs.type === "junction"; diff --git a/src/query-runner/BaseQueryRunner.ts b/src/query-runner/BaseQueryRunner.ts index 9e36226fc6..fcf2e3cd9e 100644 --- a/src/query-runner/BaseQueryRunner.ts +++ b/src/query-runner/BaseQueryRunner.ts @@ -10,6 +10,8 @@ import {ReplicationMode} from "../driver/types/ReplicationMode"; import {DeleteResult} from "../query-builder/result/DeleteResult"; import {InsertResult} from "../query-builder/result/InsertResult"; import {UpdateResult} from "../query-builder/result/UpdateResult"; +import { TableForeignKey } from "../schema-builder/table/TableForeignKey"; +import { EntityMetadata } from "../metadata/EntityMetadata"; export abstract class BaseQueryRunner { @@ -80,6 +82,8 @@ export abstract class BaseQueryRunner { */ protected mode: ReplicationMode; + private cachedTablePaths: Record = {}; + // ------------------------------------------------------------------------- // Public Abstract Methods // ------------------------------------------------------------------------- @@ -114,9 +118,9 @@ export abstract class BaseQueryRunner { // Protected Abstract Methods // ------------------------------------------------------------------------- - protected abstract loadTables(tablePaths: string[]): Promise; + protected abstract loadTables(tablePaths?: string[]): Promise; - protected abstract loadViews(tablePaths: string[]): Promise; + protected abstract loadViews(tablePaths?: string[]): Promise; // ------------------------------------------------------------------------- // Public Methods @@ -133,7 +137,13 @@ export abstract class BaseQueryRunner { /** * Loads all tables (with given names) from the database. */ - async getTables(tableNames: string[]): Promise { + async getTables(tableNames?: string[]): Promise { + if (!tableNames) { + // Don't cache in this case. + // This is the new case & isn't used anywhere else anyway. + return await this.loadTables(tableNames); + } + this.loadedTables = await this.loadTables(tableNames); return this.loadedTables; } @@ -149,7 +159,7 @@ export abstract class BaseQueryRunner { /** * Loads given view's data from the database. */ - async getViews(viewPaths: string[]): Promise { + async getViews(viewPaths?: string[]): Promise { this.loadedViews = await this.loadViews(viewPaths); return this.loadedViews; } @@ -231,13 +241,29 @@ export abstract class BaseQueryRunner { * Gets table from previously loaded tables, otherwise loads it from database. */ protected async getCachedTable(tableName: string): Promise { - const table = this.loadedTables.find(table => table.name === tableName); - if (table) return table; + if (tableName in this.cachedTablePaths) { + const tablePath = this.cachedTablePaths[tableName]; + const table = this.loadedTables.find(table => this.getTablePath(table) === tablePath); + + if (table) { + return table; + } + } const foundTables = await this.loadTables([tableName]); + if (foundTables.length > 0) { - this.loadedTables.push(foundTables[0]); - return foundTables[0]; + const foundTablePath = this.getTablePath(foundTables[0]); + + const cachedTable = this.loadedTables.find((table) => this.getTablePath(table) === foundTablePath); + + if (!cachedTable) { + this.cachedTablePaths[tableName] = this.getTablePath(foundTables[0]); + this.loadedTables.push(foundTables[0]); + return foundTables[0]; + } else { + return cachedTable; + } } else { throw new Error(`Table "${tableName}" does not exist.`); } @@ -247,8 +273,19 @@ export abstract class BaseQueryRunner { * Replaces loaded table with given changed table. */ protected replaceCachedTable(table: Table, changedTable: Table): void { - const foundTable = this.loadedTables.find(loadedTable => loadedTable.name === table.name); + const oldTablePath = this.getTablePath(table); + const foundTable = this.loadedTables.find(loadedTable => this.getTablePath(loadedTable) === oldTablePath); + + // Clean up the lookup cache.. + for (const [key, cachedPath] of Object.entries(this.cachedTablePaths)) { + if (cachedPath === oldTablePath) { + this.cachedTablePaths[key] = this.getTablePath(changedTable); + } + } + if (foundTable) { + foundTable.database = changedTable.database; + foundTable.schema = changedTable.schema; foundTable.name = changedTable.name; foundTable.columns = changedTable.columns; foundTable.indices = changedTable.indices; @@ -260,6 +297,16 @@ export abstract class BaseQueryRunner { } } + protected getTablePath(target: EntityMetadata | Table | View | TableForeignKey | string): string { + const parsed = this.connection.driver.parseTableName(target); + + return this.connection.driver.buildTableName( + parsed.tableName, + parsed.schema, + parsed.database + ); + } + protected getTypeormMetadataTableName(): string { const options = this.connection.driver.options; return this.connection.driver.buildTableName("typeorm_metadata", options.schema, options.database); @@ -320,8 +367,12 @@ export abstract class BaseQueryRunner { if (this.connection.hasMetadata(table.name)) { const metadata = this.connection.getMetadata(table.name); const columnMetadata = metadata.findColumnWithDatabaseName(column.name); - if (columnMetadata && columnMetadata.length) - return false; + + if (columnMetadata) { + const columnMetadataLength = this.connection.driver.getColumnLength(columnMetadata); + if (columnMetadataLength) + return false; + } } if (this.connection.driver.dataTypeDefaults diff --git a/src/query-runner/QueryRunner.ts b/src/query-runner/QueryRunner.ts index 8af1240f05..f719e15d6f 100644 --- a/src/query-runner/QueryRunner.ts +++ b/src/query-runner/QueryRunner.ts @@ -158,11 +158,21 @@ export interface QueryRunner { */ hasDatabase(database: string): Promise; + /** + * Loads currently using database + */ + getCurrentDatabase(): Promise; + /** * Checks if a schema with the given name exist. */ hasSchema(schema: string): Promise; + /** + * Loads currently using database schema + */ + getCurrentSchema(): Promise; + /** * Checks if a table with the given name exist. */ diff --git a/src/schema-builder/RdbmsSchemaBuilder.ts b/src/schema-builder/RdbmsSchemaBuilder.ts index 2ff406aecf..f429503321 100644 --- a/src/schema-builder/RdbmsSchemaBuilder.ts +++ b/src/schema-builder/RdbmsSchemaBuilder.ts @@ -1,4 +1,3 @@ -import {PostgresDataSourceOptions} from "../driver/postgres/PostgresDataSourceOptions"; import {Table} from "./table/Table"; import {TableColumn} from "./table/TableColumn"; import {TableForeignKey} from "./table/TableForeignKey"; @@ -6,17 +5,16 @@ import {TableIndex} from "./table/TableIndex"; import {QueryRunner} from "../query-runner/QueryRunner"; import {ColumnMetadata} from "../metadata/ColumnMetadata"; import {EntityMetadata} from "../metadata/EntityMetadata"; -import {DataSource} from "../data-source/DataSource"; import {SchemaBuilder} from "./SchemaBuilder"; import {SqlInMemory} from "../driver/SqlInMemory"; import {TableUtils} from "./util/TableUtils"; import {TableColumnOptions} from "./options/TableColumnOptions"; -import {PostgresDriver} from "../driver/postgres/PostgresDriver"; import {TableUnique} from "./table/TableUnique"; import {TableCheck} from "./table/TableCheck"; import {TableExclusion} from "./table/TableExclusion"; import {View} from "./view/View"; -import { ForeignKeyMetadata } from "../metadata/ForeignKeyMetadata"; +import {ForeignKeyMetadata} from "../metadata/ForeignKeyMetadata"; +import {DataSource} from "../data-source/DataSource"; /** * Creates complete tables schemas in the database based on the entity metadatas. @@ -43,6 +41,10 @@ export class RdbmsSchemaBuilder implements SchemaBuilder { */ protected queryRunner: QueryRunner; + private currentDatabase?: string; + + private currentSchema?: string; + // ------------------------------------------------------------------------- // Constructor // ------------------------------------------------------------------------- @@ -59,28 +61,37 @@ export class RdbmsSchemaBuilder implements SchemaBuilder { */ async build(): Promise { this.queryRunner = this.source.createQueryRunner(); - await this.queryRunner.startTransaction(); + + // this.connection.driver.database || this.currentDatabase; + this.currentDatabase = this.source.driver.database; + this.currentSchema = this.source.driver.schema; + + const isUsingTransactions = this.source.options.migrationsTransactionMode !== "none"; + if (isUsingTransactions) await this.queryRunner.startTransaction(); + try { - const tablePaths = this.entityToSyncMetadatas.map(metadata => metadata.tablePath); - // 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); await this.queryRunner.getViews([]); + await this.executeSchemaSyncOperationsInProperOrder(); // if cache is enabled then perform cache-synchronization as well if (this.source.queryResultCache) await this.source.queryResultCache.synchronize(this.queryRunner); - await this.queryRunner.commitTransaction(); - + if (isUsingTransactions) await this.queryRunner.commitTransaction(); } catch (error) { + try { // we throw original error even if rollback thrown an error - await this.queryRunner.rollbackTransaction(); + if (isUsingTransactions) await this.queryRunner.rollbackTransaction(); } catch (rollbackError) { } throw error; + } finally { await this.queryRunner.release(); } @@ -92,13 +103,17 @@ export class RdbmsSchemaBuilder implements SchemaBuilder { async log(): Promise { this.queryRunner = this.source.createQueryRunner(); try { - const tablePaths = this.entityToSyncMetadatas.map(metadata => metadata.tablePath); + // 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); await this.queryRunner.getViews([]); + this.queryRunner.enableSqlMemory(); await this.executeSchemaSyncOperationsInProperOrder(); @@ -164,12 +179,9 @@ export class RdbmsSchemaBuilder implements SchemaBuilder { protected async dropOldViews(): Promise { for (const view of this.queryRunner.loadedViews) { const existViewMetadata = this.viewEntityToSyncMetadatas.some(metadata => { - const database = metadata.database && metadata.database !== this.source.driver.database ? metadata.database : undefined; - const schema = metadata.schema ?? (this.source.driver as unknown as PostgresDriver).options.schema; - const fullViewName = this.source.driver.buildTableName(metadata.tableName, schema, database); const viewExpression = typeof view.expression === "string" ? view.expression.trim() : view.expression(this.source).getQuery(); const metadataExpression = typeof metadata.expression === "string" ? metadata.expression.trim() : metadata.expression!(this.source).getQuery(); - return view.name === fullViewName && viewExpression === metadataExpression; + return this.getTablePath(view) === this.getTablePath(metadata) && viewExpression === metadataExpression; }); if (existViewMetadata) continue; @@ -331,13 +343,7 @@ export class RdbmsSchemaBuilder implements SchemaBuilder { protected async createNewTables(): Promise { for (const metadata of this.entityToSyncMetadatas) { // check if table does not exist yet - const existTable = this.queryRunner.loadedTables.some(table => { - const database = metadata.database && metadata.database !== this.source.driver.database ? metadata.database : undefined; - const schema = metadata.schema ?? (this.source.driver as unknown as PostgresDriver).options.schema; - const fullTableName = this.source.driver.buildTableName(metadata.tableName, schema, database); - - return table.name === fullTableName; - }); + const existTable = this.queryRunner.loadedTables.find(table => this.getTablePath(table) === this.getTablePath(metadata)); if (existTable) continue; this.source.logger.logSchemaBuild(`creating a new table: ${metadata.tablePath}`); @@ -506,12 +512,9 @@ export class RdbmsSchemaBuilder implements SchemaBuilder { for (const metadata of this.viewEntityToSyncMetadatas) { // check if view does not exist yet const existView = this.queryRunner.loadedViews.some(view => { - const database = metadata.database && metadata.database !== this.source.driver.database ? metadata.database : undefined; - const schema = metadata.schema ?? (this.source.driver as unknown as PostgresDriver).options.schema; - const fullViewName = this.source.driver.buildTableName(metadata.tableName, schema, database); const viewExpression = typeof view.expression === "string" ? view.expression.trim() : view.expression(this.source).getQuery(); const metadataExpression = typeof metadata.expression === "string" ? metadata.expression.trim() : metadata.expression!(this.source).getQuery(); - return view.name === fullViewName && viewExpression === metadataExpression; + return this.getTablePath(view) === this.getTablePath(metadata) && viewExpression === metadataExpression; }); if (existView) continue; @@ -656,8 +659,9 @@ export class RdbmsSchemaBuilder implements SchemaBuilder { * Creates typeorm service table for storing user defined Views. */ protected async createTypeormMetadataTable() { - const options = this.source.driver.options; - const typeormMetadataTable = this.source.driver.buildTableName("typeorm_metadata", options.schema, options.database); + const schema = this.currentSchema; + const database = this.currentDatabase; + const typeormMetadataTable = this.source.driver.buildTableName("typeorm_metadata", schema, database); await this.queryRunner.createTable(new Table( { @@ -698,6 +702,16 @@ export class RdbmsSchemaBuilder implements SchemaBuilder { ), true); } + private getTablePath(target: EntityMetadata | Table | View | TableForeignKey | string): string { + const parsed = this.source.driver.parseTableName(target); + + return this.source.driver.buildTableName( + parsed.tableName, + parsed.schema || this.currentSchema, + parsed.database || this.currentDatabase + ); + } + } function foreignKeysMatch( diff --git a/src/schema-builder/options/ViewOptions.ts b/src/schema-builder/options/ViewOptions.ts index f93a4bf240..f81a2dfe50 100644 --- a/src/schema-builder/options/ViewOptions.ts +++ b/src/schema-builder/options/ViewOptions.ts @@ -9,6 +9,16 @@ export interface ViewOptions { // Public Properties // ------------------------------------------------------------------------- + /** + * Database name that this table resides in if it applies. + */ + database?: string; + + /** + * Schema name that this table resides in if it applies. + */ + schema?: string; + /** * View name. */ diff --git a/src/schema-builder/table/Table.ts b/src/schema-builder/table/Table.ts index 820c9ca979..7985cb8e5b 100644 --- a/src/schema-builder/table/Table.ts +++ b/src/schema-builder/table/Table.ts @@ -19,8 +19,19 @@ export class Table { // ------------------------------------------------------------------------- /** - * Contains database name, schema name and table name. - * E.g. "myDB"."mySchema"."myTable" + * Database name that this table resides in if it applies. + */ + database?: string; + + /** + * Schema name that this table resides in if it applies. + */ + schema?: string; + + /** + * May contain database name, schema name and table name, unless they're the current database. + * + * E.g. myDB.mySchema.myTable */ name: string; @@ -72,6 +83,10 @@ export class Table { constructor(options?: TableOptions) { if (options) { + this.database = options.database; + + this.schema = options.schema; + this.name = options.name; if (options.columns) @@ -81,7 +96,11 @@ export class Table { this.indices = options.indices.map(index => new TableIndex(index)); if (options.foreignKeys) - this.foreignKeys = options.foreignKeys.map(foreignKey => new TableForeignKey(foreignKey)); + this.foreignKeys = options.foreignKeys.map(foreignKey => new TableForeignKey({ + ...foreignKey, + referencedDatabase: foreignKey?.referencedDatabase || options.database, + referencedSchema: foreignKey?.referencedSchema || options.schema, + })); if (options.uniques) this.uniques = options.uniques.map(unique => new TableUnique(unique)); @@ -116,6 +135,8 @@ export class Table { */ clone(): Table { return new Table({ + schema: this.schema, + database: this.database, name: this.name, columns: this.columns.map(column => column.clone()), indices: this.indices.map(constraint => constraint.clone()), diff --git a/src/schema-builder/table/TableForeignKey.ts b/src/schema-builder/table/TableForeignKey.ts index 9272e40487..56ea0ac779 100644 --- a/src/schema-builder/table/TableForeignKey.ts +++ b/src/schema-builder/table/TableForeignKey.ts @@ -20,6 +20,16 @@ export class TableForeignKey { */ columnNames: string[] = []; + /** + * Database of Table referenced in the foreign key. + */ + referencedDatabase?: string; + + /** + * Database of Table referenced in the foreign key. + */ + referencedSchema?: string; + /** * Table referenced in the foreign key. */ @@ -56,6 +66,8 @@ export class TableForeignKey { this.name = options.name; this.columnNames = options.columnNames; this.referencedColumnNames = options.referencedColumnNames; + this.referencedDatabase = options.referencedDatabase; + this.referencedSchema = options.referencedSchema; this.referencedTableName = options.referencedTableName; this.onDelete = options.onDelete; this.onUpdate = options.onUpdate; @@ -74,6 +86,8 @@ export class TableForeignKey { name: this.name, columnNames: [...this.columnNames], referencedColumnNames: [...this.referencedColumnNames], + referencedDatabase: this.referencedDatabase, + referencedSchema: this.referencedSchema, referencedTableName: this.referencedTableName, onDelete: this.onDelete, onUpdate: this.onUpdate, @@ -93,6 +107,8 @@ export class TableForeignKey { name: metadata.name, columnNames: metadata.columnNames, referencedColumnNames: metadata.referencedColumnNames, + referencedDatabase: metadata.referencedEntityMetadata.database, + referencedSchema: metadata.referencedEntityMetadata.schema, referencedTableName: metadata.referencedTablePath, onDelete: metadata.onDelete, onUpdate: metadata.onUpdate, diff --git a/src/schema-builder/view/View.ts b/src/schema-builder/view/View.ts index c0c5679031..ee9ee87d1f 100644 --- a/src/schema-builder/view/View.ts +++ b/src/schema-builder/view/View.ts @@ -10,6 +10,16 @@ export class View { // Public Properties // ------------------------------------------------------------------------- + /** + * Database name that this view resides in if it applies. + */ + database?: string; + + /** + * Schema name that this view resides in if it applies. + */ + schema?: string; + /** * Contains database name, schema name and table name. * E.g. "myDB"."mySchema"."myTable" @@ -33,6 +43,8 @@ export class View { constructor(options?: ViewOptions) { if (options) { + this.database = options.database; + this.schema = options.schema; this.name = options.name; this.expression = options.expression; this.materialized = !!options.materialized; @@ -48,6 +60,8 @@ export class View { */ clone(): View { return new View({ + database: this.database, + schema: this.schema, name: this.name, expression: this.expression, materialized: this.materialized, @@ -63,6 +77,8 @@ export class View { */ static create(entityMetadata: EntityMetadata, driver: Driver): View { const options: ViewOptions = { + database: entityMetadata.database, + schema: entityMetadata.schema, name: driver.buildTableName(entityMetadata.tableName, entityMetadata.schema, entityMetadata.database), expression: entityMetadata.expression!, materialized: entityMetadata.tableMetadataArgs.materialized diff --git a/test/functional/schema-builder/custom-db-and-schema-sync.ts b/test/functional/schema-builder/custom-db-and-schema-sync.ts index d8d2a81b69..7bf29c2e67 100644 --- a/test/functional/schema-builder/custom-db-and-schema-sync.ts +++ b/test/functional/schema-builder/custom-db-and-schema-sync.ts @@ -1,107 +1,281 @@ import "reflect-metadata"; import {Connection} from "../../../src"; -import {MysqlDriver} from "../../../src/driver/mysql/MysqlDriver"; -import {PostgresDriver} from "../../../src/driver/postgres/PostgresDriver"; -import {SapDriver} from "../../../src/driver/sap/SapDriver"; -import {SqlServerDriver} from "../../../src/driver/sqlserver/SqlServerDriver"; import {ForeignKeyMetadata} from "../../../src/metadata/ForeignKeyMetadata"; import {closeTestingConnections, createTestingConnections, reloadTestingDatabases} from "../../utils/test-utils"; +import { Album } from "./entity/Album"; +import { Photo } from "./entity/Photo"; describe("schema builder > custom-db-and-schema-sync", () => { - - let connections: Connection[]; - before(async () => { - connections = await createTestingConnections({ - entities: [__dirname + "/entity/*{.js,.ts}"], - enabledDrivers: ["mysql", "mssql", "postgres", "sap"], - dropSchema: true, + describe("custom database", () => { + let connections: Connection[]; + before(async () => { + connections = await createTestingConnections({ + entities: [ Album, Photo ], + enabledDrivers: ["mysql"], + dropSchema: true, + }); }); - }); - beforeEach(() => reloadTestingDatabases(connections)); - after(() => closeTestingConnections(connections)); + beforeEach(() => reloadTestingDatabases(connections)); + after(() => closeTestingConnections(connections)); - it("should correctly sync tables with custom schema and database", () => Promise.all(connections.map(async connection => { - const queryRunner = connection.createQueryRunner(); - const photoMetadata = connection.getMetadata("photo"); - const albumMetadata = connection.getMetadata("album"); + it("should correctly sync tables with custom schema and database", () => Promise.all(connections.map(async connection => { + const queryRunner = connection.createQueryRunner(); + const photoMetadata = connection.getMetadata("photo"); + const albumMetadata = connection.getMetadata("album"); - // create tables - photoMetadata.synchronize = true; - albumMetadata.synchronize = true; + // create tables + photoMetadata.synchronize = true; + albumMetadata.synchronize = true; - if (connection.driver instanceof SqlServerDriver) { photoMetadata.database = "secondDB"; - photoMetadata.schema = "photo-schema"; - photoMetadata.tablePath = "secondDB.photo-schema.photo"; - photoMetadata.schemaPath = "secondDB.photo-schema"; + photoMetadata.tablePath = "secondDB.photo"; albumMetadata.database = "secondDB"; - albumMetadata.schema = "album-schema"; - albumMetadata.tablePath = "secondDB.album-schema.album"; - albumMetadata.schemaPath = "secondDB.album-schema"; + albumMetadata.tablePath = "secondDB.album"; await queryRunner.createDatabase(photoMetadata.database, true); - await queryRunner.createSchema(photoMetadata.schemaPath, true); - await queryRunner.createSchema(albumMetadata.schemaPath, true); - } else if (connection.driver instanceof PostgresDriver || connection.driver instanceof SapDriver) { + await connection.synchronize(); + + // create foreign key + let albumTable = await queryRunner.getTable(albumMetadata.tablePath); + let photoTable = await queryRunner.getTable(photoMetadata.tablePath); + albumTable!.should.be.exist; + photoTable!.should.be.exist; + + const columns = photoMetadata.columns.filter(column => column.propertyName === "albumId"); + const referencedColumns = albumMetadata.columns.filter(column => column.propertyName === "id"); + const fkMetadata = new ForeignKeyMetadata({ + entityMetadata: photoMetadata, + referencedEntityMetadata: albumMetadata, + columns: columns, + referencedColumns: referencedColumns, + namingStrategy: connection.namingStrategy + }); + photoMetadata.foreignKeys.push(fkMetadata); + + await connection.synchronize(); + + photoTable = await queryRunner.getTable(photoMetadata.tablePath); + photoTable!.foreignKeys.length.should.be.equal(1); + + // drop foreign key + photoMetadata.foreignKeys = []; + await connection.synchronize(); + + // drop tables manually, because they will not synchronize automatically + await queryRunner.dropTable(photoMetadata.tablePath, true, false); + await queryRunner.dropTable(albumMetadata.tablePath, true, false); + + // drop created database + await queryRunner.dropDatabase("secondDB", true); + + await queryRunner.release(); + + }))); + }) + + describe("custom schema", () => { + let connections: Connection[]; + before(async () => { + connections = await createTestingConnections({ + enabledDrivers: [ "postgres", "sap" ], + entities: [ Album, Photo ], + dropSchema: true, + }); + }); + beforeEach(() => reloadTestingDatabases(connections)); + after(() => closeTestingConnections(connections)); + + it("should correctly sync tables with custom schema", () => Promise.all(connections.map(async connection => { + const queryRunner = connection.createQueryRunner(); + const photoMetadata = connection.getMetadata("photo"); + const albumMetadata = connection.getMetadata("album"); + + // create tables + photoMetadata.synchronize = true; + albumMetadata.synchronize = true; + photoMetadata.schema = "photo-schema"; photoMetadata.tablePath = "photo-schema.photo"; - photoMetadata.schemaPath = "photo-schema"; albumMetadata.schema = "album-schema"; albumMetadata.tablePath = "album-schema.album"; - albumMetadata.schemaPath = "album-schema"; - await queryRunner.createSchema(photoMetadata.schemaPath, true); - await queryRunner.createSchema(albumMetadata.schemaPath, true); - } else if (connection.driver instanceof MysqlDriver) { + await queryRunner.createSchema(photoMetadata.schema, true); + await queryRunner.createSchema(albumMetadata.schema, true); + + await connection.synchronize(); + + // create foreign key + let albumTable = await queryRunner.getTable(albumMetadata.tablePath); + let photoTable = await queryRunner.getTable(photoMetadata.tablePath); + albumTable!.should.be.exist; + photoTable!.should.be.exist; + + const columns = photoMetadata.columns.filter(column => column.propertyName === "albumId"); + const referencedColumns = albumMetadata.columns.filter(column => column.propertyName === "id"); + const fkMetadata = new ForeignKeyMetadata({ + entityMetadata: photoMetadata, + referencedEntityMetadata: albumMetadata, + columns: columns, + referencedColumns: referencedColumns, + namingStrategy: connection.namingStrategy + }); + photoMetadata.foreignKeys.push(fkMetadata); + + await connection.synchronize(); + + photoTable = await queryRunner.getTable(photoMetadata.tablePath); + photoTable!.foreignKeys.length.should.be.equal(1); + + // drop foreign key + photoMetadata.foreignKeys = []; + await connection.synchronize(); + + // drop tables manually, because they will not synchronize automatically + await queryRunner.dropTable(photoMetadata.tablePath, true, false); + await queryRunner.dropTable(albumMetadata.tablePath, true, false); + + // drop created database + await queryRunner.dropDatabase("secondDB", true); + + await queryRunner.release(); + + }))); + + it("should correctly sync tables with `public` schema", () => Promise.all(connections.map(async connection => { + const queryRunner = connection.createQueryRunner(); + const photoMetadata = connection.getMetadata("photo"); + const albumMetadata = connection.getMetadata("album"); + + // create tables + photoMetadata.synchronize = true; + albumMetadata.synchronize = true; + + photoMetadata.schema = "public"; + photoMetadata.tablePath = "photo"; + + albumMetadata.schema = "public"; + albumMetadata.tablePath = "album"; + + await queryRunner.createSchema(photoMetadata.schema, true); + await queryRunner.createSchema(albumMetadata.schema, true); + + await connection.synchronize(); + + // create foreign key + let albumTable = await queryRunner.getTable(albumMetadata.tablePath); + let photoTable = await queryRunner.getTable(photoMetadata.tablePath); + + albumTable!.should.be.exist; + photoTable!.should.be.exist; + + photoTable!.foreignKeys.length.should.be.equal(0); + + const columns = photoMetadata.columns.filter(column => column.propertyName === "albumId"); + const referencedColumns = albumMetadata.columns.filter(column => column.propertyName === "id"); + const fkMetadata = new ForeignKeyMetadata({ + entityMetadata: photoMetadata, + referencedEntityMetadata: albumMetadata, + columns: columns, + referencedColumns: referencedColumns, + namingStrategy: connection.namingStrategy + }); + + photoMetadata.foreignKeys.push(fkMetadata); + await connection.synchronize(); + + photoTable = await queryRunner.getTable(photoMetadata.tablePath); + photoTable!.foreignKeys.length.should.be.equal(1); + + // drop foreign key + photoMetadata.foreignKeys = []; + await connection.synchronize(); + + // drop tables manually, because they will not synchronize automatically + await queryRunner.dropTable(photoMetadata.tablePath, true, false); + await queryRunner.dropTable(albumMetadata.tablePath, true, false); + + // drop created database + await queryRunner.dropDatabase("secondDB", true); + + await queryRunner.release(); + }))); + }) + + describe("custom database and schema", () => { + let connections: Connection[]; + before(async () => { + connections = await createTestingConnections({ + entities: [ Album, Photo ], + enabledDrivers: [ "mssql" ], + dropSchema: true, + }); + }); + beforeEach(() => reloadTestingDatabases(connections)); + after(() => closeTestingConnections(connections)); + + it("should correctly sync tables with custom schema and database", () => Promise.all(connections.map(async connection => { + const queryRunner = connection.createQueryRunner(); + const photoMetadata = connection.getMetadata("photo"); + const albumMetadata = connection.getMetadata("album"); + + // create tables + photoMetadata.synchronize = true; + albumMetadata.synchronize = true; + photoMetadata.database = "secondDB"; - photoMetadata.tablePath = "secondDB.photo"; + photoMetadata.schema = "photo-schema"; + photoMetadata.tablePath = "secondDB.photo-schema.photo"; + const photoMetadataSchemaPath = "secondDB.photo-schema"; albumMetadata.database = "secondDB"; - albumMetadata.tablePath = "secondDB.album"; + albumMetadata.schema = "album-schema"; + albumMetadata.tablePath = "secondDB.album-schema.album"; + const albumMetadataSchemaPath = "secondDB.album-schema"; await queryRunner.createDatabase(photoMetadata.database, true); - } - - await connection.synchronize(); - - // create foreign key - let albumTable = await queryRunner.getTable(albumMetadata.tablePath); - let photoTable = await queryRunner.getTable(photoMetadata.tablePath); - albumTable!.should.be.exist; - photoTable!.should.be.exist; - - const columns = photoMetadata.columns.filter(column => column.propertyName === "albumId"); - const referencedColumns = albumMetadata.columns.filter(column => column.propertyName === "id"); - const fkMetadata = new ForeignKeyMetadata({ - entityMetadata: photoMetadata, - referencedEntityMetadata: albumMetadata, - columns: columns, - referencedColumns: referencedColumns, - namingStrategy: connection.namingStrategy - }); - photoMetadata.foreignKeys.push(fkMetadata); + await queryRunner.createSchema(photoMetadataSchemaPath, true); + await queryRunner.createSchema(albumMetadataSchemaPath, true); + + await connection.synchronize(); + + // create foreign key + let albumTable = await queryRunner.getTable(albumMetadata.tablePath); + let photoTable = await queryRunner.getTable(photoMetadata.tablePath); + albumTable!.should.be.exist; + photoTable!.should.be.exist; - await connection.synchronize(); + const columns = photoMetadata.columns.filter(column => column.propertyName === "albumId"); + const referencedColumns = albumMetadata.columns.filter(column => column.propertyName === "id"); + const fkMetadata = new ForeignKeyMetadata({ + entityMetadata: photoMetadata, + referencedEntityMetadata: albumMetadata, + columns: columns, + referencedColumns: referencedColumns, + namingStrategy: connection.namingStrategy + }); + photoMetadata.foreignKeys.push(fkMetadata); - photoTable = await queryRunner.getTable(photoMetadata.tablePath); - photoTable!.foreignKeys.length.should.be.equal(1); + await connection.synchronize(); - // drop foreign key - photoMetadata.foreignKeys = []; - await connection.synchronize(); + photoTable = await queryRunner.getTable(photoMetadata.tablePath); + photoTable!.foreignKeys.length.should.be.equal(1); - // drop tables manually, because they will not synchronize automatically - await queryRunner.dropTable(photoMetadata.tablePath, true, false); - await queryRunner.dropTable(albumMetadata.tablePath, true, false); + // drop foreign key + photoMetadata.foreignKeys = []; + await connection.synchronize(); - // drop created database - await queryRunner.dropDatabase("secondDB", true); + // drop tables manually, because they will not synchronize automatically + await queryRunner.dropTable(photoMetadata.tablePath, true, false); + await queryRunner.dropTable(albumMetadata.tablePath, true, false); - await queryRunner.release(); + // drop created database + await queryRunner.dropDatabase("secondDB", true); - }))); + await queryRunner.release(); + }))); + }) }); diff --git a/test/github-issues/7867/entity/Example.ts b/test/github-issues/7867/entity/Example.ts new file mode 100644 index 0000000000..00b946666d --- /dev/null +++ b/test/github-issues/7867/entity/Example.ts @@ -0,0 +1,10 @@ +import { Column, Entity, PrimaryGeneratedColumn } from "../../../../src"; + +@Entity() +export class Example { + @PrimaryGeneratedColumn() + id: number; + + @Column() + name: string; +} diff --git a/test/github-issues/7867/issue-7867.ts b/test/github-issues/7867/issue-7867.ts new file mode 100644 index 0000000000..0f7f4fd7e9 --- /dev/null +++ b/test/github-issues/7867/issue-7867.ts @@ -0,0 +1,82 @@ +import "reflect-metadata"; +import { Connection } from "../../../src"; +import { closeTestingConnections, createTestingConnections, reloadTestingDatabases } from "../../utils/test-utils"; +import { Example } from "./entity/Example"; +import { expect } from "chai"; + +describe("github issues > #7867 Column not renamed when schema/database is set", () => { + + describe('schema is set', () => { + let connections: Connection[]; + before(async () => { + connections = await createTestingConnections({ + entities: [ Example ], + schemaCreate: true, + dropSchema: true, + driverSpecific: { + schema: "public" + }, + enabledDrivers: [ "postgres" ], + }); + }); + beforeEach(() => reloadTestingDatabases(connections)); + after(() => closeTestingConnections(connections)); + + it("should correctly change column name", () => Promise.all(connections.map(async connection => { + const postMetadata = connection.getMetadata(Example); + const nameColumn = postMetadata.findColumnWithPropertyName("name")!; + nameColumn.propertyName = "title"; + nameColumn.build(connection); + + await connection.synchronize(); + + const queryRunner = connection.createQueryRunner(); + const postTable = await queryRunner.getTable("example"); + await queryRunner.release(); + + expect(postTable!.findColumnByName("name")).to.be.undefined; + postTable!.findColumnByName("title")!.should.be.exist; + + // revert changes + nameColumn.propertyName = "name"; + nameColumn.build(connection); + }))); + }); + + describe('database is set', () => { + let connections: Connection[]; + before(async () => { + connections = await createTestingConnections({ + entities: [ Example ], + schemaCreate: true, + dropSchema: true, + driverSpecific: { + database: "test" + }, + enabledDrivers: [ "mysql" ], + }); + }); + beforeEach(() => reloadTestingDatabases(connections)); + after(() => closeTestingConnections(connections)); + + it("should correctly change column name", () => Promise.all(connections.map(async connection => { + const postMetadata = connection.getMetadata(Example); + const nameColumn = postMetadata.findColumnWithPropertyName("name")!; + nameColumn.propertyName = "title"; + nameColumn.build(connection); + + await connection.synchronize(); + + const queryRunner = connection.createQueryRunner(); + const postTable = await queryRunner.getTable("example"); + await queryRunner.release(); + + expect(postTable!.findColumnByName("name")).to.be.undefined; + postTable!.findColumnByName("title")!.should.be.exist; + + // revert changes + nameColumn.propertyName = "name"; + nameColumn.build(connection); + }))); + }); +});