From d254b2ddba3fba4fb60e1076a6bc4e7d425bbd30 Mon Sep 17 00:00:00 2001 From: Adrien de Peretti Date: Wed, 26 Feb 2025 19:01:36 +0100 Subject: [PATCH] chore(): Improve cascade soft deletetion/restoration and update (#11618) **What** - Fix soft deletion and restoration emitted events - Improve soft deleted/restore algorithm - Fix big number field handling null value during partial hydration from mikro orm --- .changeset/odd-pears-poke.md | 5 + .../src/dal/mikro-orm/big-number-field.ts | 58 +++--- .../src/dal/mikro-orm/mikro-orm-repository.ts | 43 ++--- .../core/utils/src/dal/mikro-orm/utils.ts | 177 +++++++++++++----- packages/core/utils/src/dal/utils.ts | 54 ------ .../modules-sdk/medusa-internal-service.ts | 17 +- .../utils/src/modules-sdk/medusa-service.ts | 82 ++++---- .../product/data/create-product.ts | 2 +- .../product-module-service/products.spec.ts | 101 +++++++++- 9 files changed, 337 insertions(+), 202 deletions(-) create mode 100644 .changeset/odd-pears-poke.md diff --git a/.changeset/odd-pears-poke.md b/.changeset/odd-pears-poke.md new file mode 100644 index 0000000000..3e6535f2f5 --- /dev/null +++ b/.changeset/odd-pears-poke.md @@ -0,0 +1,5 @@ +--- +"@medusajs/utils": patch +--- + +chore(): Improve cascade soft deletetion and update diff --git a/packages/core/utils/src/dal/mikro-orm/big-number-field.ts b/packages/core/utils/src/dal/mikro-orm/big-number-field.ts index e3012fbcde..70e534c339 100644 --- a/packages/core/utils/src/dal/mikro-orm/big-number-field.ts +++ b/packages/core/utils/src/dal/mikro-orm/big-number-field.ts @@ -29,34 +29,42 @@ export function MikroOrmBigNumberProperty( this.__helper.__data[rawColumnName] = null this[rawColumnName] = null } else { - let bigNumber: BigNumber + // When mikro orm create and hydrate the entity with partial selection it can happen + // that null value is being passed. + if (!options?.nullable && value === null) { + this.__helper.__data[columnName] = undefined + this.__helper.__data[rawColumnName] = undefined + this[rawColumnName] = undefined + } else { + let bigNumber: BigNumber - try { - if (value instanceof BigNumber) { - bigNumber = value - } else if (this[rawColumnName]) { - const precision = this[rawColumnName].precision - bigNumber = new BigNumber(value, { - precision, - }) - } else { - bigNumber = new BigNumber(value) + try { + if (value instanceof BigNumber) { + bigNumber = value + } else if (this[rawColumnName]) { + const precision = this[rawColumnName].precision + bigNumber = new BigNumber(value, { + precision, + }) + } else { + bigNumber = new BigNumber(value) + } + } catch (e) { + throw new Error(`Cannot set value ${value} for ${columnName}.`) } - } catch (e) { - throw new Error(`Cannot set value ${value} for ${columnName}.`) + + const raw = bigNumber.raw! + raw.value = trimZeros(raw.value as string) + + // Note: this.__helper isn't present when directly working with the entity + // Adding this in optionally for it not to break. + if (isDefined(this.__helper)) { + this.__helper.__data[columnName] = bigNumber.numeric + this.__helper.__data[rawColumnName] = raw + } + + this[rawColumnName] = raw } - - const raw = bigNumber.raw! - raw.value = trimZeros(raw.value as string) - - // Note: this.__helper isn't present when directly working with the entity - // Adding this in optionally for it not to break. - if (isDefined(this.__helper)) { - this.__helper.__data[columnName] = bigNumber.numeric - this.__helper.__data[rawColumnName] = raw - } - - this[rawColumnName] = raw } // Note: this.__helper isn't present when directly working with the entity diff --git a/packages/core/utils/src/dal/mikro-orm/mikro-orm-repository.ts b/packages/core/utils/src/dal/mikro-orm/mikro-orm-repository.ts index df6201d717..aed2610c7c 100644 --- a/packages/core/utils/src/dal/mikro-orm/mikro-orm-repository.ts +++ b/packages/core/utils/src/dal/mikro-orm/mikro-orm-repository.ts @@ -31,10 +31,7 @@ import { } from "../../common" import { toMikroORMEntity } from "../../dml" import { buildQuery } from "../../modules-sdk/build-query" -import { - getSoftDeletedCascadedEntitiesIdsMappedBy, - transactionWrapper, -} from "../utils" +import { transactionWrapper } from "../utils" import { dbErrorMapper } from "./db-error-mapper" import { mikroOrmSerializer } from "./mikro-orm-serializer" import { mikroOrmUpdateDeletedAtRecursively } from "./utils" @@ -215,17 +212,13 @@ export class MikroOrmBaseRepository const date = new Date() const manager = this.getActiveManager(sharedContext) - await mikroOrmUpdateDeletedAtRecursively( + const softDeletedEntitiesMap = await mikroOrmUpdateDeletedAtRecursively( manager, entities as any[], date ) - const softDeletedEntitiesMap = getSoftDeletedCascadedEntitiesIdsMappedBy({ - entities, - }) - - return [entities, softDeletedEntitiesMap] + return [entities, Object.fromEntries(softDeletedEntitiesMap)] } async restore( @@ -239,14 +232,13 @@ export class MikroOrmBaseRepository const entities = await this.find(query, sharedContext) const manager = this.getActiveManager(sharedContext) - await mikroOrmUpdateDeletedAtRecursively(manager, entities as any[], null) + const softDeletedEntitiesMap = await mikroOrmUpdateDeletedAtRecursively( + manager, + entities as any[], + null + ) - const softDeletedEntitiesMap = getSoftDeletedCascadedEntitiesIdsMappedBy({ - entities, - restored: true, - }) - - return [entities, softDeletedEntitiesMap] + return [entities, Object.fromEntries(softDeletedEntitiesMap)] } } @@ -398,38 +390,41 @@ export function mikroOrmBaseRepositoryFactory( descriptor, ] of collectionsToRemoveAllFrom) { await promiseAll( - data.map(async ({ entity }) => { + data.flatMap(async ({ entity }) => { if (!descriptor.mappedBy) { return await entity[collectionToRemoveAllFrom].init() } + const promises: Promise[] = [] await entity[collectionToRemoveAllFrom].init() const items = entity[collectionToRemoveAllFrom] for (const item of items) { - await item[descriptor.mappedBy!].init() + promises.push(item[descriptor.mappedBy!].init()) } + + return promises }) ) } } async update( - data: { entity; update }[], + data: { entity: any; update: any }[], context?: Context ): Promise[]> { const manager = this.getActiveManager(context) await this.initManyToManyToDetachAllItemsIfNeeded(data, context) - data.map((_, index) => { - manager.assign(data[index].entity, data[index].update, { + data.forEach(({ entity, update }) => { + manager.assign(entity, update, { mergeObjectProperties: true, }) - manager.persist(data[index].entity) + manager.persist(entity) }) - return data.map((d) => d.entity) + return data.map((d) => d.entity) as InferRepositoryReturnType[] } async delete( diff --git a/packages/core/utils/src/dal/mikro-orm/utils.ts b/packages/core/utils/src/dal/mikro-orm/utils.ts index 8900cbb02a..a34f9162b0 100644 --- a/packages/core/utils/src/dal/mikro-orm/utils.ts +++ b/packages/core/utils/src/dal/mikro-orm/utils.ts @@ -1,7 +1,8 @@ -import { Collection, EntityMetadata, FindOptions, wrap } from "@mikro-orm/core" +import { EntityMetadata, FindOptions } from "@mikro-orm/core" import { SqlEntityManager } from "@mikro-orm/postgresql" -import { buildQuery } from "../../modules-sdk/build-query" +import { promiseAll } from "../../common" import { isString } from "../../common/is-string" +import { buildQuery } from "../../modules-sdk/build-query" function detectCircularDependency( manager: SqlEntityManager, @@ -60,47 +61,80 @@ function detectCircularDependency( async function performCascadingSoftDeletion( manager: SqlEntityManager, entity: T & { id: string; deleted_at?: string | Date | null }, - value: Date | null + value: Date | null, + softDeletedEntitiesMap: Map< + string, + (T & { id: string; deleted_at?: string | Date | null })[] + > = new Map() ) { if (!("deleted_at" in entity)) return entity.deleted_at = value - const entityName = entity.constructor.name + const softDeletedEntityMapItem = softDeletedEntitiesMap.get( + entity.constructor.name + ) + if (!softDeletedEntityMapItem) { + softDeletedEntitiesMap.set(entity.constructor.name, [entity]) + } else { + softDeletedEntityMapItem.push(entity) + } - const relations = manager.getDriver().getMetadata().get(entityName).relations + const entityName = entity.constructor.name + const entityMetadata = manager.getDriver().getMetadata().get(entityName) + const relations = entityMetadata.relations const relationsToCascade = relations.filter((relation) => relation.cascade?.includes("soft-remove" as any) ) + // If there are no relations to cascade, just persist the entity and return + if (!relationsToCascade.length) { + manager.persist(entity) + return + } + + // Fetch the entity with all cascading relations in a single query + const relationNames = relationsToCascade.map((r) => r.name) + + const query = buildQuery( + { + id: entity.id, + }, + { + select: [ + "id", + "deleted_at", + ...relationNames.flatMap((r) => [`${r}.id`, `${r}.deleted_at`]), + ], + relations: relationNames, + withDeleted: true, + } + ) + + const entityWithRelations = await manager.findOne(entityName, query.where, { + ...query.options, + populateFilter: { + withDeleted: true, + }, + } as FindOptions) + + if (!entityWithRelations) { + manager.persist(entity) + return + } + + // Create a map to group related entities by their type + const relatedEntitiesByType = new Map< + string, + T & { id: string; deleted_at?: string | Date | null }[] + >() + + // Collect all related entities by type for (const relation of relationsToCascade) { - let entityRelation = entity[relation.name] + const entityRelation = entityWithRelations[relation.name] - // Handle optional relationships - if (relation.nullable && !entityRelation) { - continue - } - - const retrieveEntity = async () => { - const query = buildQuery( - { - id: entity.id, - }, - { - relations: [relation.name], - withDeleted: true, - } - ) - return await manager.findOne( - entity.constructor.name, - query.where, - query.options as FindOptions - ) - } - - entityRelation = await retrieveEntity() - entityRelation = entityRelation[relation.name] + // Skip if relation is null or undefined if (!entityRelation) { continue } @@ -109,45 +143,86 @@ async function performCascadingSoftDeletion( let relationEntities: any[] = [] if (isCollection) { - if (!(entityRelation as Collection).isInitialized()) { - entityRelation = await retrieveEntity() - entityRelation = entityRelation[relation.name] - } relationEntities = entityRelation.getItems() } else { - const wrappedEntity = wrap(entityRelation) - - let initializedEntityRelation = entityRelation - if (!wrappedEntity.isInitialized()) { - initializedEntityRelation = await wrap(entityRelation).init() - } - - relationEntities = [initializedEntityRelation] + relationEntities = [entityRelation] } if (!relationEntities.length) { continue } - await mikroOrmUpdateDeletedAtRecursively(manager, relationEntities, value) + // Add to the map of entities by type + if (!relatedEntitiesByType.has(relation.type)) { + relatedEntitiesByType.set(relation.type, [] as any) + } + relatedEntitiesByType.get(relation.type)!.push(...relationEntities) } - await manager.persist(entity) + // Process each type of related entity in batch + const promises: Promise[] = [] + for (const [, entities] of relatedEntitiesByType.entries()) { + if (entities.length === 0) continue + + // Process cascading relations for these entities + promises.push( + ...entities.map((entity) => + performCascadingSoftDeletion( + manager, + entity as any, + value, + softDeletedEntitiesMap + ) + ) + ) + } + + await promiseAll(promises) + + manager.persist(entity) } +/** + * Updates the deleted_at field for all entities in the given array and their + * cascaded relations and returns a map of entity IDs to their corresponding + * entity types. + * + * @param manager - The Mikro ORM manager instance. + * @param entities - An array of entities to update. + * @param value - The value to set for the deleted_at field. + * @returns A map of entity IDs to their corresponding entity types. + */ export const mikroOrmUpdateDeletedAtRecursively = async < T extends object = any >( manager: SqlEntityManager, entities: (T & { id: string; deleted_at?: string | Date | null })[], value: Date | null -) => { +): Promise< + Map +> => { + const softDeletedEntitiesMap = new Map< + string, + (T & { id: string; deleted_at?: string | Date | null })[] + >() + + if (!entities.length) return softDeletedEntitiesMap + + const entityMetadata = manager + .getDriver() + .getMetadata() + .get(entities[0].constructor.name) + detectCircularDependency(manager, entityMetadata) + + // Process each entity type for (const entity of entities) { - const entityMetadata = manager - .getDriver() - .getMetadata() - .get(entity.constructor.name) - detectCircularDependency(manager, entityMetadata) - await performCascadingSoftDeletion(manager, entity, value) + await performCascadingSoftDeletion( + manager, + entity, + value, + softDeletedEntitiesMap + ) } + + return softDeletedEntitiesMap } diff --git a/packages/core/utils/src/dal/utils.ts b/packages/core/utils/src/dal/utils.ts index cc36b3f815..425d858c2d 100644 --- a/packages/core/utils/src/dal/utils.ts +++ b/packages/core/utils/src/dal/utils.ts @@ -1,5 +1,3 @@ -import { isObject } from "../common" - export async function transactionWrapper( manager: any, task: (transactionManager: any) => Promise, @@ -32,58 +30,6 @@ export async function transactionWrapper( return await transactionMethod.bind(manager)(task, options) } -/** - * Can be used to create a new Object that collect the entities - * based on the columnLookup. This is useful when you want to soft delete entities and return - * an object where the keys are the entities name and the values are the entities - * that were soft deleted. - * - * @param entities - * @param deletedEntitiesMap - * @param getEntityName - */ -export function getSoftDeletedCascadedEntitiesIdsMappedBy({ - entities, - deletedEntitiesMap, - getEntityName, - restored, -}: { - entities: any[] - deletedEntitiesMap?: Map - getEntityName?: (entity: any) => string - restored?: boolean -}): Record { - deletedEntitiesMap ??= new Map() - getEntityName ??= (entity) => entity.constructor.name - - for (const entity of entities) { - const entityName = getEntityName(entity) - const shouldSkip = !!deletedEntitiesMap - .get(entityName) - ?.some((e) => e.id === entity.id) - - if ((restored ? !!entity.deleted_at : !entity.deleted_at) || shouldSkip) { - continue - } - - const values = deletedEntitiesMap.get(entityName) ?? [] - values.push(entity) - deletedEntitiesMap.set(entityName, values) - - Object.values(entity).forEach((propValue: any) => { - if (propValue != null && isObject(propValue[0])) { - getSoftDeletedCascadedEntitiesIdsMappedBy({ - entities: propValue, - deletedEntitiesMap, - getEntityName, - }) - } - }) - } - - return Object.fromEntries(deletedEntitiesMap) -} - export function normalizeMigrationSQL(sql: string) { sql = sql.replace( /create table (?!if not exists)/g, diff --git a/packages/core/utils/src/modules-sdk/medusa-internal-service.ts b/packages/core/utils/src/modules-sdk/medusa-internal-service.ts index 5809ddce1e..0a3e64f81a 100644 --- a/packages/core/utils/src/modules-sdk/medusa-internal-service.ts +++ b/packages/core/utils/src/modules-sdk/medusa-internal-service.ts @@ -259,7 +259,7 @@ export function MedusaInternalService< const primaryKeys = AbstractService_.retrievePrimaryKeys(model) const inputArray = Array.isArray(input) ? input : [input] - const toUpdateData: { entity; update }[] = [] + const toUpdateData: { entity: TEntity; update: Partial }[] = [] // Only used when we receive data and no selector const keySelectorForDataOnly: any = { @@ -353,10 +353,17 @@ export function MedusaInternalService< // Manage metadata if needed toUpdateData.forEach(({ entity, update }) => { - if (isPresent(update.metadata)) { - entity.metadata = update.metadata = mergeMetadata( - entity.metadata ?? {}, - update.metadata + const update_ = update as (typeof toUpdateData)[number]["update"] & { + metadata: Record + } + const entity_ = entity as InferEntityType & { + metadata?: Record + } + + if (isPresent(update_.metadata)) { + entity_.metadata = update_.metadata = mergeMetadata( + entity_.metadata ?? {}, + update_.metadata ) } }) diff --git a/packages/core/utils/src/modules-sdk/medusa-service.ts b/packages/core/utils/src/modules-sdk/medusa-service.ts index ac3d7aa2d3..bb0b7cea3b 100644 --- a/packages/core/utils/src/modules-sdk/medusa-service.ts +++ b/packages/core/utils/src/modules-sdk/medusa-service.ts @@ -137,6 +137,37 @@ export function MedusaService< ? ModelConfigurationsToConfigTemplate : ModelsConfig > { + function emitSoftDeleteRestoreEvents( + this: AbstractModuleService_, + klassPrototype: any, + cascadedModelsMap: Record, + action: string, + sharedContext: Context + ) { + const joinerConfig = ( + typeof this.__joinerConfig === "function" + ? this.__joinerConfig() + : this.__joinerConfig + ) as ModuleJoinerConfig + + const emittedEntities = new Set() + + Object.entries(cascadedModelsMap).forEach(([linkableKey, ids]) => { + const entity = joinerConfig.linkableKeys?.[linkableKey]! + if (entity && !emittedEntities.has(entity)) { + emittedEntities.add(entity) + const linkableKeyEntity = camelToSnakeCase(entity).toLowerCase() + + klassPrototype.aggregatedEvents.bind(this)({ + action: action, + object: linkableKeyEntity, + data: { id: ids }, + context: sharedContext, + }) + } + }) + } + const buildAndAssignMethodImpl = function ( klassPrototype: any, method: string, @@ -321,27 +352,11 @@ export function MedusaService< ) if (mappedCascadedModelsMap) { - const joinerConfig = ( - typeof this.__joinerConfig === "function" - ? this.__joinerConfig() - : this.__joinerConfig - ) as ModuleJoinerConfig - - Object.entries(mappedCascadedModelsMap).forEach( - ([linkableKey, ids]) => { - const entity = joinerConfig.linkableKeys?.[linkableKey]! - if (entity) { - const linkableKeyEntity = - camelToSnakeCase(entity).toLowerCase() - - klassPrototype.aggregatedEvents.bind(this)({ - action: CommonEvents.DELETED, - object: linkableKeyEntity, - data: { id: ids }, - context: sharedContext, - }) - } - } + emitSoftDeleteRestoreEvents.bind(this)( + klassPrototype, + mappedCascadedModelsMap, + CommonEvents.DELETED, + sharedContext ) } @@ -378,26 +393,11 @@ export function MedusaService< ) if (mappedCascadedModelsMap) { - const joinerConfig = ( - typeof this.__joinerConfig === "function" - ? this.__joinerConfig() - : this.__joinerConfig - ) as ModuleJoinerConfig - - Object.entries(mappedCascadedModelsMap).forEach( - ([linkableKey, ids]) => { - const entity = joinerConfig.linkableKeys?.[linkableKey]! - if (entity) { - const linkableKeyEntity = - camelToSnakeCase(entity).toLowerCase() - klassPrototype.aggregatedEvents.bind(this)({ - action: CommonEvents.CREATED, - object: linkableKeyEntity, - data: { id: ids }, - context: sharedContext, - }) - } - } + emitSoftDeleteRestoreEvents.bind(this)( + klassPrototype, + mappedCascadedModelsMap, + CommonEvents.CREATED, + sharedContext ) } diff --git a/packages/modules/product/integration-tests/__fixtures__/product/data/create-product.ts b/packages/modules/product/integration-tests/__fixtures__/product/data/create-product.ts index d2b92935bc..7dc77b1908 100644 --- a/packages/modules/product/integration-tests/__fixtures__/product/data/create-product.ts +++ b/packages/modules/product/integration-tests/__fixtures__/product/data/create-product.ts @@ -52,7 +52,7 @@ export const buildProductAndRelationsData = ({ options, variants, collection_id, -}: Partial) => { +}: Partial & { tags?: { value: string }[] }) => { const defaultOptionTitle = "test-option" const defaultOptionValue = "test-value" diff --git a/packages/modules/product/integration-tests/__tests__/product-module-service/products.spec.ts b/packages/modules/product/integration-tests/__tests__/product-module-service/products.spec.ts index 5ad8f0a839..22fe12f8bd 100644 --- a/packages/modules/product/integration-tests/__tests__/product-module-service/products.spec.ts +++ b/packages/modules/product/integration-tests/__tests__/product-module-service/products.spec.ts @@ -30,7 +30,7 @@ import { createTypes, } from "../../__fixtures__/product" -jest.setTimeout(3000000) +jest.setTimeout(300000) moduleIntegrationTestRunner({ moduleName: Modules.PRODUCT, @@ -975,6 +975,85 @@ moduleIntegrationTestRunner({ const data = buildProductAndRelationsData({ images, thumbnail: images[0].url, + options: [ + { title: "size", values: ["large", "small"] }, + { title: "color", values: ["red", "blue"] }, + { title: "material", values: ["cotton", "polyester"] }, + ], + variants: [ + { + title: "Large Red Cotton", + sku: "LRG-RED-CTN", + options: { + size: "large", + color: "red", + material: "cotton", + }, + }, + { + title: "Large Red Polyester", + sku: "LRG-RED-PLY", + options: { + size: "large", + color: "red", + material: "polyester", + }, + }, + { + title: "Large Blue Cotton", + sku: "LRG-BLU-CTN", + options: { + size: "large", + color: "blue", + material: "cotton", + }, + }, + { + title: "Large Blue Polyester", + sku: "LRG-BLU-PLY", + options: { + size: "large", + color: "blue", + material: "polyester", + }, + }, + { + title: "Small Red Cotton", + sku: "SML-RED-CTN", + options: { + size: "small", + color: "red", + material: "cotton", + }, + }, + { + title: "Small Red Polyester", + sku: "SML-RED-PLY", + options: { + size: "small", + color: "red", + material: "polyester", + }, + }, + { + title: "Small Blue Cotton", + sku: "SML-BLU-CTN", + options: { + size: "small", + color: "blue", + material: "cotton", + }, + }, + { + title: "Small Blue Polyester", + sku: "SML-BLU-PLY", + options: { + size: "small", + color: "blue", + material: "polyester", + }, + }, + ], }) const products = await service.createProducts([data]) @@ -1074,6 +1153,26 @@ moduleIntegrationTestRunner({ source: Modules.PRODUCT, action: CommonEvents.DELETED, }), + composeMessage(ProductEvents.PRODUCT_VARIANT_DELETED, { + data: { id: [products[0].variants[0].id] }, + object: "product_variant", + source: Modules.PRODUCT, + action: CommonEvents.DELETED, + }), + composeMessage(ProductEvents.PRODUCT_OPTION_DELETED, { + data: { id: [products[0].options[0].id] }, + object: "product_option", + source: Modules.PRODUCT, + action: CommonEvents.DELETED, + }), + composeMessage(ProductEvents.PRODUCT_OPTION_VALUE_DELETED, { + data: { + id: [products[0].options[0].values[0].id], + }, + object: "product_option_value", + source: Modules.PRODUCT, + action: CommonEvents.DELETED, + }), ], { internal: true,