Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(): Improve internal repository delete algo #11601

Merged
merged 9 commits into from
Feb 25, 2025
9 changes: 9 additions & 0 deletions .changeset/purple-donkeys-learn.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
"@medusajs/inventory": patch
"@medusajs/link-modules": patch
"@medusajs/product": patch
"@medusajs/types": patch
"@medusajs/utils": patch
---

chore(): Improve internal repository delete algo
7 changes: 5 additions & 2 deletions packages/core/types/src/dal/repository-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,10 @@ export interface RepositoryService<T = any> extends BaseRepositoryService {
context?: Context
): Promise<InferRepositoryReturnType<T>[]>

delete(idsOrPKs: FindOptions<T>["where"], context?: Context): Promise<void>
delete(
idsOrPKs: FindOptions<T>["where"],
context?: Context
): Promise<string[]>

/**
* Soft delete entities and cascade to related entities if configured.
Expand Down Expand Up @@ -127,7 +130,7 @@ export interface TreeRepositoryService<T = any> extends BaseRepositoryService {
context?: Context
): Promise<InferRepositoryReturnType<T>[]>

delete(ids: string[], context?: Context): Promise<void>
delete(ids: string[], context?: Context): Promise<string[]>
}

/**
Expand Down
10 changes: 5 additions & 5 deletions packages/core/types/src/modules-sdk/medusa-internal-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,16 +64,16 @@ export interface IMedusaInternalService<
sharedContext?: Context
): Promise<InferEntityType<TEntity>[]>

delete(idOrSelector: string, sharedContext?: Context): Promise<void>
delete(idOrSelector: string[], sharedContext?: Context): Promise<void>
delete(idOrSelector: object, sharedContext?: Context): Promise<void>
delete(idOrSelector: object[], sharedContext?: Context): Promise<void>
delete(idOrSelector: string, sharedContext?: Context): Promise<string[]>
delete(idOrSelector: string[], sharedContext?: Context): Promise<string[]>
delete(idOrSelector: object, sharedContext?: Context): Promise<string[]>
delete(idOrSelector: object[], sharedContext?: Context): Promise<string[]>
delete(
idOrSelector: {
selector: FilterQuery<any> | BaseFilterable<FilterQuery<any>>
},
sharedContext?: Context
): Promise<void>
): Promise<string[]>

softDelete(
idsOrFilter:
Expand Down
37 changes: 32 additions & 5 deletions packages/core/utils/src/dal/mikro-orm/mikro-orm-repository.ts
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,10 @@ export class MikroOrmBaseRepository<const T extends object = object>
throw new Error("Method not implemented.")
}

delete(idsOrPKs: FindOptions<T>["where"], context?: Context): Promise<void> {
delete(
idsOrPKs: FindOptions<T>["where"],
context?: Context
): Promise<string[]> {
throw new Error("Method not implemented.")
}

Expand Down Expand Up @@ -285,7 +288,7 @@ export class MikroOrmBaseTreeRepository<
throw new Error("Method not implemented.")
}

delete(ids: string[], context?: Context): Promise<void> {
delete(ids: string[], context?: Context): Promise<string[]> {
throw new Error("Method not implemented.")
}
}
Expand All @@ -301,6 +304,10 @@ export function mikroOrmBaseRepositoryFactory<const T extends object>(

class MikroOrmAbstractBaseRepository_ extends MikroOrmBaseRepository<T> {
entity = mikroOrmEntity
tableName = (
(mikroOrmEntity as unknown as EntitySchema).meta ??
(mikroOrmEntity as EntityClass<any>).prototype.__meta
).collection

// @ts-ignore
constructor(...args: any[]) {
Expand Down Expand Up @@ -428,9 +435,29 @@ export function mikroOrmBaseRepositoryFactory<const T extends object>(
async delete(
filters: FindOptions<T>["where"],
context?: Context
): Promise<void> {
const manager = this.getActiveManager<EntityManager>(context)
await manager.nativeDelete<T>(this.entity, filters)
): Promise<string[]> {
const manager = this.getActiveManager<SqlEntityManager>(context)

const whereSqlInfo = manager
.createQueryBuilder(this.entity.name, this.tableName)
.where(filters)
.getKnexQuery()
.toSQL()

const where = [
whereSqlInfo.sql.split("where ")[1],
whereSqlInfo.bindings,
] as [string, any[]]

return await (manager.getTransactionContext() ?? manager.getKnex())
.queryBuilder()
.from(this.tableName)
.delete()
.where(manager.getKnex().raw(...where))
.returning("id")
.then((rows: { id: string }[]) => {
return rows.map((row: { id: string }) => row.id)
})
}

async find(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -267,12 +267,7 @@ describe("Internal Module Service Factory", () => {
})

it("should delete entities successfully", async () => {
const entitiesToDelete = [{ id: "1", name: "Item" }]
containerMock[modelRepositoryName].find.mockResolvedValueOnce(
entitiesToDelete
)

await instance.delete({ selector: {} })
await instance.delete({ selector: { id: "1" } })
expect(containerMock[modelRepositoryName].delete).toHaveBeenCalledWith(
{
$or: [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,21 +29,21 @@ describe("Abstract Module Service Factory", () => {
mainModelMockService: {
retrieve: jest.fn().mockResolvedValue({ id: "1", name: "Item" }),
list: jest.fn().mockResolvedValue([{ id: "1", name: "Item" }]),
delete: jest.fn().mockResolvedValue(undefined),
delete: jest.fn().mockResolvedValue([]),
softDelete: jest.fn().mockResolvedValue([[], {}]),
restore: jest.fn().mockResolvedValue([[], {}]),
},
otherModelMock1Service: {
retrieve: jest.fn().mockResolvedValue({ id: "1", name: "Item" }),
list: jest.fn().mockResolvedValue([{ id: "1", name: "Item" }]),
delete: jest.fn().mockResolvedValue(undefined),
delete: jest.fn().mockResolvedValue([]),
softDelete: jest.fn().mockResolvedValue([[], {}]),
restore: jest.fn().mockResolvedValue([[], {}]),
},
otherModelMock2Service: {
retrieve: jest.fn().mockResolvedValue({ id: "1", name: "Item" }),
list: jest.fn().mockResolvedValue([{ id: "1", name: "Item" }]),
delete: jest.fn().mockResolvedValue(undefined),
delete: jest.fn().mockResolvedValue([]),
softDelete: jest.fn().mockResolvedValue([[], {}]),
restore: jest.fn().mockResolvedValue([[], {}]),
},
Expand Down
56 changes: 13 additions & 43 deletions packages/core/utils/src/modules-sdk/medusa-internal-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -367,16 +367,16 @@ export function MedusaInternalService<
)
}

delete(idOrSelector: string, sharedContext?: Context): Promise<void>
delete(idOrSelector: string[], sharedContext?: Context): Promise<void>
delete(idOrSelector: object, sharedContext?: Context): Promise<void>
delete(idOrSelector: object[], sharedContext?: Context): Promise<void>
delete(idOrSelector: string, sharedContext?: Context): Promise<string[]>
delete(idOrSelector: string[], sharedContext?: Context): Promise<string[]>
delete(idOrSelector: object, sharedContext?: Context): Promise<string[]>
delete(idOrSelector: object[], sharedContext?: Context): Promise<string[]>
delete(
idOrSelector: {
selector: FilterQuery<any> | BaseFilterable<FilterQuery<any>>
},
sharedContext?: Context
): Promise<void>
): Promise<string[]>

@InjectTransactionManager(propertyRepositoryName)
async delete(
Expand All @@ -389,12 +389,12 @@ export function MedusaInternalService<
selector: FilterQuery<any> | BaseFilterable<FilterQuery<any>>
},
@MedusaContext() sharedContext: Context = {}
): Promise<void> {
): Promise<string[]> {
if (
!isDefined(idOrSelector) ||
(Array.isArray(idOrSelector) && !idOrSelector.length)
) {
return
return []
}

const primaryKeys = AbstractService_.retrievePrimaryKeys(model)
Expand All @@ -420,21 +420,7 @@ export function MedusaInternalService<
}

if (isObject(idOrSelector) && "selector" in idOrSelector) {
const entitiesToDelete = await this.list(
idOrSelector.selector as FilterQuery<any>,
{
select: primaryKeys,
},
sharedContext
)

for (const entity of entitiesToDelete) {
const criteria = {}
primaryKeys.forEach((key) => {
criteria[key] = entity[key]
})
deleteCriteria.$or.push(criteria)
}
deleteCriteria.$or.push(idOrSelector.selector)
} else {
const primaryKeysValues = Array.isArray(idOrSelector)
? idOrSelector
Expand All @@ -451,34 +437,18 @@ export function MedusaInternalService<
criteria[primaryKeys[0]] = primaryKeyValue
}

// TODO: Revisit
/*primaryKeys.forEach((key) => {
/!*if (
isObject(primaryKeyValue) &&
!isDefined(primaryKeyValue[key]) &&
// primaryKeys.length > 1
) {
throw new MedusaError(
MedusaError.Types.INVALID_DATA,
`Composite key must contain all primary key fields: ${primaryKeys.join(
", "
)}. Found: ${Object.keys(primaryKeyValue)}`
)
}*!/

criteria[key] = isObject(primaryKeyValue)
? primaryKeyValue[key]
: primaryKeyValue
})*/
return criteria
})
}

if (!deleteCriteria.$or.length) {
return
return []
}

await this[propertyRepositoryName].delete(deleteCriteria, sharedContext)
return await this[propertyRepositoryName].delete(
deleteCriteria,
sharedContext
)
}

@InjectTransactionManager(propertyRepositoryName)
Expand Down
8 changes: 3 additions & 5 deletions packages/core/utils/src/modules-sdk/medusa-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -277,18 +277,16 @@ export function MedusaService<
? primaryKeyValues
: [primaryKeyValues]

await this.__container__[serviceRegistrationName].delete(
const ids = await this.__container__[serviceRegistrationName].delete(
primaryKeyValues_,
sharedContext
)

primaryKeyValues_.map((primaryKeyValue) =>
ids.map((id) =>
klassPrototype.aggregatedEvents.bind(this)({
action: CommonEvents.DELETED,
object: camelToSnakeCase(modelName).toLowerCase(),
data: isString(primaryKeyValue)
? { id: primaryKeyValue }
: primaryKeyValue,
data: isString(id) ? { id: id } : id,
context: sharedContext,
})
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -547,7 +547,7 @@ export default class InventoryModuleService
return
}

return await this.inventoryLevelService_.delete(inventoryLevel.id, context)
await this.inventoryLevelService_.delete(inventoryLevel.id, context)
}

// @ts-ignore
Expand Down
5 changes: 2 additions & 3 deletions packages/modules/link-modules/src/repositories/link.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,15 @@ export function getLinkRepository(model: EntitySchema) {
this.joinerConfig_ = joinerConfig
}

async delete(data: any, context: Context = {}): Promise<void> {
async delete(data: any, context: Context = {}): Promise<string[]> {
const filter = {}
for (const key in data) {
filter[key] = {
$in: Array.isArray(data[key]) ? data[key] : [data[key]],
}
}

const manager = this.getActiveManager<SqlEntityManager>(context)
await manager.nativeDelete(model, data, {})
return await super.delete(filter, context)
}

async create(data: object[], context: Context = {}): Promise<object[]> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1312,12 +1312,15 @@ moduleIntegrationTestRunner<IProductModuleService>({
relations: ["images"],
})

const retrievedProductAgain = await service.retrieveProduct(product.id, {
relations: ["images"],
})
const retrievedProductAgain = await service.retrieveProduct(
product.id,
{
relations: ["images"],
}
)

expect(retrievedProduct.images).toEqual(retrievedProductAgain.images)

expect(retrievedProduct.images).toEqual(
Array.from({ length: 1000 }, (_, i) =>
expect.objectContaining({
Expand All @@ -1332,7 +1335,9 @@ moduleIntegrationTestRunner<IProductModuleService>({
// Explicitly verify sequential order
retrievedProduct.images.forEach((img, idx) => {
if (idx > 0) {
expect(img.rank).toBeGreaterThan(retrievedProduct.images[idx - 1].rank)
expect(img.rank).toBeGreaterThan(
retrievedProduct.images[idx - 1].rank
)
}
})
})
Expand Down
2 changes: 1 addition & 1 deletion packages/modules/product/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
"resolve:aliases": "tsc --showConfig -p tsconfig.json > tsconfig.resolved.json && tsc-alias -p tsconfig.resolved.json && rimraf tsconfig.resolved.json",
"build": "rimraf dist && tsc --build && npm run resolve:aliases",
"test": "jest --runInBand --bail --forceExit -- src/**/__tests__/**/*.ts",
"test:integration": "jest --forceExit",
"test:integration": "jest --forceExit -- integration-tests/__tests__/**/*.ts",
"migration:initial": " MIKRO_ORM_CLI_CONFIG=./mikro-orm.config.dev.ts medusa-mikro-orm migration:create --initial",
"migration:create": " MIKRO_ORM_CLI_CONFIG=./mikro-orm.config.dev.ts medusa-mikro-orm migration:create",
"migration:up": " MIKRO_ORM_CLI_CONFIG=./mikro-orm.config.dev.ts medusa-mikro-orm migration:up",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -272,10 +272,11 @@ export class ProductCategoryRepository extends DALUtils.MikroOrmBaseTreeReposito
return [this.sortCategoriesByRank(categoriesTree), count]
}

async delete(ids: string[], context: Context = {}): Promise<void> {
async delete(ids: string[], context: Context = {}): Promise<string[]> {
const manager = super.getActiveManager<SqlEntityManager>(context)
await this.baseDelete(ids, context)
await manager.nativeDelete(ProductCategory.name, { id: ids }, {})
return ids
}

async softDelete(
Expand Down
4 changes: 2 additions & 2 deletions packages/modules/product/src/services/product-category.ts
Original file line number Diff line number Diff line change
Expand Up @@ -163,8 +163,8 @@ export default class ProductCategoryService {
async delete(
ids: string[],
@MedusaContext() sharedContext: Context = {}
): Promise<void> {
await this.productCategoryRepository_.delete(ids, sharedContext)
): Promise<string[]> {
return await this.productCategoryRepository_.delete(ids, sharedContext)
}

@InjectTransactionManager("productCategoryRepository_")
Expand Down
Loading