From cea4cdc8d7d5f7aab27d6e65e00dcbd03ab92753 Mon Sep 17 00:00:00 2001 From: Adrien de Peretti Date: Mon, 14 Oct 2024 16:32:50 +0200 Subject: [PATCH] fix: Link migration descriptor case changes and hash computation (#9560) **What** The module service name case has changed and the hash generation was performed on the non lower cased version of it while after the hash generation everything is based on the lower case version of the generated table name form the service names leading to different hash computation. This pr update the link migration table to adjust the to/from module value with its new value as well as generating the hash from the lower case version of the computed table name Co-authored-by: Carlos R. L. Rodrigues <37986729+carlos-r-l-rodrigues@users.noreply.github.com> --- .../mikro-orm/mikro-orm-create-connection.ts | 31 ++------------ packages/core/utils/src/dal/utils.ts | 33 +++++++++++++++ .../__fixtures__/migrations.ts | 8 ++-- .../__tests__/migrations.spec.ts | 28 ++++++------- .../link-modules/src/migration/index.ts | 42 ++++++++++++------- .../link-modules/src/utils/generate-entity.ts | 4 +- 6 files changed, 84 insertions(+), 62 deletions(-) diff --git a/packages/core/utils/src/dal/mikro-orm/mikro-orm-create-connection.ts b/packages/core/utils/src/dal/mikro-orm/mikro-orm-create-connection.ts index 13c8a67ab6..6630af8ad4 100644 --- a/packages/core/utils/src/dal/mikro-orm/mikro-orm-create-connection.ts +++ b/packages/core/utils/src/dal/mikro-orm/mikro-orm-create-connection.ts @@ -1,40 +1,15 @@ +import { ModuleServiceInitializeOptions } from "@medusajs/types" import { Filter as MikroORMFilter } from "@mikro-orm/core" import { TSMigrationGenerator } from "@mikro-orm/migrations" -import { ModuleServiceInitializeOptions } from "@medusajs/types" import { isString } from "../../common" +import { normalizeMigrationSQL } from "../utils" type FilterDef = Parameters[0] export class CustomTsMigrationGenerator extends TSMigrationGenerator { createStatement(sql: string, padLeft: number): string { if (isString(sql)) { - sql = sql.replace( - /create table (?!if not exists)/g, - "create table if not exists " - ) - sql = sql.replace(/alter table (?!if exists)/g, "alter table if exists ") - sql = sql.replace( - /create index (?!if not exists)/g, - "create index if not exists " - ) - sql = sql.replace(/drop index (?!if exists)/g, "drop index if exists ") - sql = sql.replace( - /create unique index (?!if not exists)/g, - "create unique index if not exists " - ) - sql = sql.replace( - /drop unique index (?!if exists)/g, - "drop unique index if exists " - ) - sql = sql.replace( - /add column (?!if not exists)/g, - "add column if not exists " - ) - sql = sql.replace(/drop column (?!if exists)/g, "drop column if exists ") - sql = sql.replace( - /drop constraint (?!if exists)/g, - "drop constraint if exists " - ) + sql = normalizeMigrationSQL(sql) } return super.createStatement(sql, padLeft) diff --git a/packages/core/utils/src/dal/utils.ts b/packages/core/utils/src/dal/utils.ts index dd4f8c8a60..cc36b3f815 100644 --- a/packages/core/utils/src/dal/utils.ts +++ b/packages/core/utils/src/dal/utils.ts @@ -83,3 +83,36 @@ export function getSoftDeletedCascadedEntitiesIdsMappedBy({ return Object.fromEntries(deletedEntitiesMap) } + +export function normalizeMigrationSQL(sql: string) { + sql = sql.replace( + /create table (?!if not exists)/g, + "create table if not exists " + ) + sql = sql.replace(/alter table (?!if exists)/g, "alter table if exists ") + sql = sql.replace( + /create index (?!if not exists)/g, + "create index if not exists " + ) + sql = sql.replace(/alter index (?!if exists)/g, "alter index if exists ") + sql = sql.replace(/drop index (?!if exists)/g, "drop index if exists ") + sql = sql.replace( + /create unique index (?!if not exists)/g, + "create unique index if not exists " + ) + sql = sql.replace( + /drop unique index (?!if exists)/g, + "drop unique index if exists " + ) + sql = sql.replace( + /add column (?!if not exists)/g, + "add column if not exists " + ) + sql = sql.replace(/drop column (?!if exists)/g, "drop column if exists ") + sql = sql.replace( + /drop constraint (?!if exists)/g, + "drop constraint if exists " + ) + + return sql +} diff --git a/packages/modules/link-modules/integration-tests/__fixtures__/migrations.ts b/packages/modules/link-modules/integration-tests/__fixtures__/migrations.ts index d343900e46..a67877716c 100644 --- a/packages/modules/link-modules/integration-tests/__fixtures__/migrations.ts +++ b/packages/modules/link-modules/integration-tests/__fixtures__/migrations.ts @@ -10,7 +10,7 @@ export const User = model.define("user", { name: model.text(), }) -export const userJoinerConfig = defineJoinerConfig("User", { +export const userJoinerConfig = defineJoinerConfig("user", { models: [User], }) @@ -20,7 +20,7 @@ export class UserService extends MedusaService({ User }) { } } -export const UserModule = Module("User", { +export const UserModule = Module("user", { service: UserService, }) @@ -29,7 +29,7 @@ export const Car = model.define("car", { name: model.text(), }) -export const carJoinerConfig = defineJoinerConfig("Car", { +export const carJoinerConfig = defineJoinerConfig("car", { models: [Car], }) @@ -39,7 +39,7 @@ export class CarService extends MedusaService({ Car }) { } } -export const CarModule = Module("Car", { +export const CarModule = Module("car", { service: CarService, }) diff --git a/packages/modules/link-modules/integration-tests/__tests__/migrations.spec.ts b/packages/modules/link-modules/integration-tests/__tests__/migrations.spec.ts index 9b65a92cd7..29d4464be7 100644 --- a/packages/modules/link-modules/integration-tests/__tests__/migrations.spec.ts +++ b/packages/modules/link-modules/integration-tests/__tests__/migrations.spec.ts @@ -1,17 +1,17 @@ import { MedusaModule } from "@medusajs/framework/modules-sdk" import { ILinkModule, ModuleJoinerConfig } from "@medusajs/framework/types" -import { Modules, defineLink, isObject } from "@medusajs/framework/utils" +import { defineLink, isObject, Modules } from "@medusajs/framework/utils" import { moduleIntegrationTestRunner } from "medusa-test-utils" import { MigrationsExecutionPlanner } from "../../src" import { Car, + carJoinerConfig, CarModule, CustomModuleImplementationContainingAReallyBigNameThatExceedsPosgresLimitToNameATableModule, - User, - UserModule, - carJoinerConfig, longNameJoinerConfig, + User, userJoinerConfig, + UserModule, } from "../__fixtures__/migrations" jest.setTimeout(30000) @@ -63,13 +63,13 @@ moduleIntegrationTestRunner({ expect(actionPlan[0]).toEqual({ action: "create", linkDescriptor: { - fromModule: "User", - toModule: "Car", + fromModule: "user", + toModule: "car", fromModel: "user", toModel: "car", }, tableName: "user_user_car_car", - sql: 'create table "user_user_car_car" ("user_id" varchar(255) not null, "car_id" varchar(255) not null, "id" varchar(255) not null, "created_at" timestamptz(0) not null default CURRENT_TIMESTAMP, "updated_at" timestamptz(0) not null default CURRENT_TIMESTAMP, "deleted_at" timestamptz(0) null, constraint "user_user_car_car_pkey" primary key ("user_id", "car_id"));\ncreate index "IDX_car_id_-8c9667b4" on "user_user_car_car" ("car_id");\ncreate index "IDX_id_-8c9667b4" on "user_user_car_car" ("id");\ncreate index "IDX_user_id_-8c9667b4" on "user_user_car_car" ("user_id");\ncreate index "IDX_deleted_at_-8c9667b4" on "user_user_car_car" ("deleted_at");\n\n', + sql: 'create table if not exists "user_user_car_car" ("user_id" varchar(255) not null, "car_id" varchar(255) not null, "id" varchar(255) not null, "created_at" timestamptz(0) not null default CURRENT_TIMESTAMP, "updated_at" timestamptz(0) not null default CURRENT_TIMESTAMP, "deleted_at" timestamptz(0) null, constraint "user_user_car_car_pkey" primary key ("user_id", "car_id"));\ncreate index if not exists "IDX_car_id_-92128f74" on "user_user_car_car" ("car_id");\ncreate index if not exists "IDX_id_-92128f74" on "user_user_car_car" ("id");\ncreate index if not exists "IDX_user_id_-92128f74" on "user_user_car_car" ("user_id");\ncreate index if not exists "IDX_deleted_at_-92128f74" on "user_user_car_car" ("deleted_at");\n\n', }) /** @@ -108,13 +108,13 @@ moduleIntegrationTestRunner({ expect(actionPlan[0]).toEqual({ action: "update", linkDescriptor: { - fromModule: "User", - toModule: "Car", + fromModule: "user", + toModule: "car", fromModel: "user", toModel: "car", }, tableName: "user_user_car_car", - sql: 'alter table "user_user_car_car" add column "data" jsonb not null;\n\n', + sql: 'alter table if exists "user_user_car_car" add column if not exists "data" jsonb not null;\n\n', }) /** @@ -128,8 +128,8 @@ moduleIntegrationTestRunner({ expect(actionPlan[0]).toEqual({ action: "noop", linkDescriptor: { - fromModule: "User", - toModule: "Car", + fromModule: "user", + toModule: "car", fromModel: "user", toModel: "car", }, @@ -154,9 +154,9 @@ moduleIntegrationTestRunner({ tableName: "user_user_car_car", linkDescriptor: { toModel: "car", - toModule: "Car", + toModule: "car", fromModel: "user", - fromModule: "User", + fromModule: "user", }, }) }) diff --git a/packages/modules/link-modules/src/migration/index.ts b/packages/modules/link-modules/src/migration/index.ts index 962714e2d6..6d5ac9b939 100644 --- a/packages/modules/link-modules/src/migration/index.ts +++ b/packages/modules/link-modules/src/migration/index.ts @@ -7,9 +7,10 @@ import { } from "@medusajs/framework/types" import { + arrayDifference, DALUtils, ModulesSdkUtils, - arrayDifference, + normalizeMigrationSQL, promiseAll, } from "@medusajs/framework/utils" import { EntitySchema, MikroORM } from "@mikro-orm/core" @@ -259,7 +260,7 @@ export class MigrationsExecutionPlanner implements ILinkMigrationsPlanner { action: "create", linkDescriptor, tableName, - sql: await generator.getCreateSchemaSQL(), + sql: normalizeMigrationSQL(await generator.getCreateSchemaSQL()), } } @@ -281,9 +282,11 @@ export class MigrationsExecutionPlanner implements ILinkMigrationsPlanner { }, ]) - const updateSQL = await generator.getUpdateSchemaSQL({ - fromSchema: dbSchema, - }) + const updateSQL = normalizeMigrationSQL( + await generator.getUpdateSchemaSQL({ + fromSchema: dbSchema, + }) + ) /** * Entity is upto-date and hence we do not have to perform @@ -332,24 +335,30 @@ export class MigrationsExecutionPlanner implements ILinkMigrationsPlanner { }[] = [] for (let trackedTable of trackedTables) { - const newTableName = this.#linksEntities.find((entity) => { + const linkEntity = this.#linksEntities.find((entity) => { return ( entity.linkDescriptor.fromModel === trackedTable.link_descriptor.fromModel && entity.linkDescriptor.toModel === trackedTable.link_descriptor.toModel && - entity.linkDescriptor.fromModule === - trackedTable.link_descriptor.fromModule && - entity.linkDescriptor.toModule === - trackedTable.link_descriptor.toModule + entity.linkDescriptor.fromModule.toLowerCase() === + trackedTable.link_descriptor.fromModule.toLowerCase() && + entity.linkDescriptor.toModule.toLowerCase() === + trackedTable.link_descriptor.toModule.toLowerCase() ) - })?.entity.meta.collection + }) + const newTableName = linkEntity?.entity.meta.collection /** * Perform rename */ if (newTableName && trackedTable.table_name !== newTableName) { - await this.renameOldTable(orm, trackedTable.table_name, newTableName) + await this.renameOldTable( + orm, + trackedTable.table_name, + newTableName, + linkEntity.linkDescriptor + ) migratedTables.push({ ...trackedTable, table_name: newTableName, @@ -370,11 +379,16 @@ export class MigrationsExecutionPlanner implements ILinkMigrationsPlanner { protected async renameOldTable( orm: MikroORM, oldName: string, - newName: string + newName: string, + descriptor: PlannerActionLinkDescriptor ) { await orm.em.getDriver().getConnection().execute(` ALTER TABLE "${oldName}" RENAME TO "${newName}"; - UPDATE "${this.tableName}" SET table_name = '${newName}' WHERE table_name = '${oldName}'; + UPDATE "${ + this.tableName + }" SET table_name = '${newName}', link_descriptor = '${JSON.stringify( + descriptor + )}' WHERE table_name = '${oldName}'; `) } diff --git a/packages/modules/link-modules/src/utils/generate-entity.ts b/packages/modules/link-modules/src/utils/generate-entity.ts index c5c0f3391b..595518e607 100644 --- a/packages/modules/link-modules/src/utils/generate-entity.ts +++ b/packages/modules/link-modules/src/utils/generate-entity.ts @@ -3,10 +3,10 @@ import { ModuleJoinerConfig, } from "@medusajs/framework/types" import { - SoftDeletableFilterKey, composeTableName, mikroOrmSoftDeletableFilterOptions, simpleHash, + SoftDeletableFilterKey, } from "@medusajs/framework/utils" import { EntitySchema } from "@mikro-orm/core" @@ -36,7 +36,7 @@ export function generateEntity( primary.foreignKey, foreign.serviceName, foreign.foreignKey - ) + ).toLowerCase() const fields = fieldNames.reduce((acc, curr) => { acc[curr] = {