From bd20d669682d9d3d7633c231cb84bf6bfecf9d39 Mon Sep 17 00:00:00 2001 From: Adrien de Peretti Date: Thu, 29 Aug 2024 17:10:13 +0200 Subject: [PATCH] fix(link-module): Migration planner not closing connection correctly (#8881) RESOLVES CC-405 **What** Fixing mikro orm connection not closed during sync-links --- .../link-modules/src/migration/index.ts | 200 +++++++++--------- 1 file changed, 103 insertions(+), 97 deletions(-) diff --git a/packages/modules/link-modules/src/migration/index.ts b/packages/modules/link-modules/src/migration/index.ts index 8f32c0b91e..5d73347561 100644 --- a/packages/modules/link-modules/src/migration/index.ts +++ b/packages/modules/link-modules/src/migration/index.ts @@ -239,64 +239,64 @@ export class MigrationsExecutionPlanner implements ILinkMigrationsPlanner { const tableName = entity.meta.collection const orm = await this.createORM([entity]) - const generator = orm.getSchemaGenerator() - const platform = orm.em.getPlatform() - const connection = orm.em.getConnection() - const schemaName = this.#dbConfig.schema || "public" - - /** - * If the table name for the entity has not been - * managed by us earlier, then we should create - * it. - */ - if (!trackedLinksTables.includes(tableName)) { - return { - action: "create", - linkDescriptor, - tableName, - sql: await generator.getCreateSchemaSQL(), - } - } - - /** - * Pre-fetching information schema from the database and using that - * as the way to compute the update diff. - * - * @note - * The "loadInformationSchema" mutates the "dbSchema" argument provided - * to it as the first argument. - */ - const dbSchema = new DatabaseSchema(platform, schemaName) - await platform - .getSchemaHelper?.() - ?.loadInformationSchema(dbSchema, connection, [ - { - table_name: tableName, - schema_name: schemaName, - }, - ]) - - const updateSQL = await generator.getUpdateSchemaSQL({ - fromSchema: dbSchema, - }) - - /** - * Entity is upto-date and hence we do not have to perform - * any updates on it. - */ - if (!updateSQL.length) { - return { - action: "noop", - linkDescriptor, - tableName, - } - } - - const usesUnsafeCommands = this.#unsafeSQLCommands.some((fragment) => { - return updateSQL.match(new RegExp(`${fragment}`, "ig")) - }) - try { + const generator = orm.getSchemaGenerator() + const platform = orm.em.getPlatform() + const connection = orm.em.getConnection() + const schemaName = this.#dbConfig.schema || "public" + + /** + * If the table name for the entity has not been + * managed by us earlier, then we should create + * it. + */ + if (!trackedLinksTables.includes(tableName)) { + return { + action: "create", + linkDescriptor, + tableName, + sql: await generator.getCreateSchemaSQL(), + } + } + + /** + * Pre-fetching information schema from the database and using that + * as the way to compute the update diff. + * + * @note + * The "loadInformationSchema" mutates the "dbSchema" argument provided + * to it as the first argument. + */ + const dbSchema = new DatabaseSchema(platform, schemaName) + await platform + .getSchemaHelper?.() + ?.loadInformationSchema(dbSchema, connection, [ + { + table_name: tableName, + schema_name: schemaName, + }, + ]) + + const updateSQL = await generator.getUpdateSchemaSQL({ + fromSchema: dbSchema, + }) + + /** + * Entity is upto-date and hence we do not have to perform + * any updates on it. + */ + if (!updateSQL.length) { + return { + action: "noop", + linkDescriptor, + tableName, + } + } + + const usesUnsafeCommands = this.#unsafeSQLCommands.some((fragment) => { + return updateSQL.match(new RegExp(`${fragment}`, "ig")) + }) + return { action: usesUnsafeCommands ? "notify" : "update", linkDescriptor, @@ -317,48 +317,54 @@ export class MigrationsExecutionPlanner implements ILinkMigrationsPlanner { */ async createPlan() { const orm = await this.createORM() - await this.ensureMigrationsTable(orm) - - const executionActions: LinkMigrationsPlannerAction[] = [] - - await this.ensureMigrationsTableUpToDate(orm) - - const trackedTables = await this.getTrackedLinksTables(orm) - const trackedTablesNames = trackedTables.map(({ table_name }) => table_name) - - /** - * Looping through the new set of entities and generating - * execution plan for them - */ - for (let { entity, linkDescriptor } of this.#linksEntities) { - executionActions.push( - await this.getEntityMigrationPlan( - linkDescriptor, - entity, - trackedTablesNames - ) - ) - } - - const linksTableNames = this.#linksEntities.map( - ({ entity }) => entity.meta.collection - ) - - /** - * Finding the tables to be removed - */ - const tablesToRemove = arrayDifference(trackedTablesNames, linksTableNames) - tablesToRemove.forEach((tableToRemove) => { - executionActions.push({ - action: "delete", - tableName: tableToRemove, - linkDescriptor: trackedTables.find( - ({ table_name }) => tableToRemove === table_name - )!.link_descriptor, - }) - }) try { + await this.ensureMigrationsTable(orm) + + const executionActions: LinkMigrationsPlannerAction[] = [] + + await this.ensureMigrationsTableUpToDate(orm) + + const trackedTables = await this.getTrackedLinksTables(orm) + const trackedTablesNames = trackedTables.map( + ({ table_name }) => table_name + ) + + /** + * Looping through the new set of entities and generating + * execution plan for them + */ + for (let { entity, linkDescriptor } of this.#linksEntities) { + executionActions.push( + await this.getEntityMigrationPlan( + linkDescriptor, + entity, + trackedTablesNames + ) + ) + } + + const linksTableNames = this.#linksEntities.map( + ({ entity }) => entity.meta.collection + ) + + /** + * Finding the tables to be removed + */ + const tablesToRemove = arrayDifference( + trackedTablesNames, + linksTableNames + ) + tablesToRemove.forEach((tableToRemove) => { + executionActions.push({ + action: "delete", + tableName: tableToRemove, + linkDescriptor: trackedTables.find( + ({ table_name }) => tableToRemove === table_name + )!.link_descriptor, + }) + }) + return executionActions } finally { await orm.close(true)