Skip to content

Commit 41054a3

Browse files
authored
fix(product): Deep update data retrieval bottleneck (medusajs#12538)
* fix(product): Deep update data retrieval bottleneck * Create shiny-spiders-matter.md * fix(product): Deep update data retrieval bottleneck
1 parent ebe5cc7 commit 41054a3

File tree

3 files changed

+111
-39
lines changed

3 files changed

+111
-39
lines changed

.changeset/shiny-spiders-matter.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@medusajs/product": patch
3+
---
4+
5+
fix(product): Deep update data retrieval bottleneck

packages/modules/product/integration-tests/__tests__/product-module-service/products.spec.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,6 @@ import {
3333
jest.setTimeout(300000)
3434

3535
moduleIntegrationTestRunner<IProductModuleService>({
36-
debug: true,
3736
moduleName: Modules.PRODUCT,
3837
injectedDependencies: {
3938
[Modules.EVENT_BUS]: new MockEventBusService(),

packages/modules/product/src/repositories/product.ts

Lines changed: 106 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,14 @@
11
import { Product, ProductOption } from "@models"
22

33
import { Context, DAL, InferEntityType } from "@medusajs/framework/types"
4-
import { buildQuery, DALUtils } from "@medusajs/framework/utils"
4+
import {
5+
arrayDifference,
6+
buildQuery,
7+
DALUtils,
8+
MedusaError,
9+
} from "@medusajs/framework/utils"
510
import { SqlEntityManager, wrap } from "@mikro-orm/postgresql"
611

7-
// eslint-disable-next-line max-len
812
export class ProductRepository extends DALUtils.mikroOrmBaseRepositoryFactory(
913
Product
1014
) {
@@ -13,35 +17,102 @@ export class ProductRepository extends DALUtils.mikroOrmBaseRepositoryFactory(
1317
super(...arguments)
1418
}
1519

20+
/**
21+
* Identify the relations to load for the given update.
22+
* @param update
23+
* @returns
24+
*/
25+
static #getProductDeepUpdateRelationsToLoad(
26+
productsToUpdate: any[]
27+
): string[] {
28+
const relationsToLoad = new Set<string>()
29+
productsToUpdate.forEach((productToUpdate) => {
30+
if (productToUpdate.options) {
31+
relationsToLoad.add("options")
32+
relationsToLoad.add("options.values")
33+
}
34+
if (productToUpdate.variants) {
35+
relationsToLoad.add("options")
36+
relationsToLoad.add("options.values")
37+
relationsToLoad.add("variants")
38+
relationsToLoad.add("variants.options")
39+
relationsToLoad.add("variants.options.option")
40+
}
41+
if (productToUpdate.tags) relationsToLoad.add("tags")
42+
if (productToUpdate.categories) relationsToLoad.add("categories")
43+
if (productToUpdate.images) relationsToLoad.add("images")
44+
if (productToUpdate.collection) relationsToLoad.add("collection")
45+
if (productToUpdate.type) relationsToLoad.add("type")
46+
})
47+
return Array.from(relationsToLoad)
48+
}
49+
50+
// We should probably fix the column types in the database to avoid this
51+
// It would also match the types in ProductVariant, which are already numbers
52+
static #correctUpdateDTOTypes(productToUpdate: {
53+
weight?: string | number
54+
length?: string | number
55+
height?: string | number
56+
width?: string | number
57+
}) {
58+
productToUpdate.weight = productToUpdate.weight?.toString()
59+
productToUpdate.length = productToUpdate.length?.toString()
60+
productToUpdate.height = productToUpdate.height?.toString()
61+
productToUpdate.width = productToUpdate.width?.toString()
62+
}
63+
1664
async deepUpdate(
17-
updates: any[],
65+
productsToUpdate: ({ id: string } & any)[],
1866
validateVariantOptions: (
1967
variants: any[],
2068
options: InferEntityType<typeof ProductOption>[]
2169
) => void,
2270
context: Context = {}
2371
): Promise<InferEntityType<typeof Product>[]> {
24-
updates.forEach((update) => this.correctUpdateDTOTypes(update))
25-
26-
const products = await this.find(
27-
buildQuery({ id: updates.map((p) => p.id) }, { relations: ["*"] }),
28-
context
72+
const productIdsToUpdate: string[] = []
73+
productsToUpdate.forEach((productToUpdate) => {
74+
ProductRepository.#correctUpdateDTOTypes(productToUpdate)
75+
productIdsToUpdate.push(productToUpdate.id)
76+
})
77+
78+
const relationsToLoad =
79+
ProductRepository.#getProductDeepUpdateRelationsToLoad(productsToUpdate)
80+
81+
const findOptions = buildQuery(
82+
{ id: productIdsToUpdate },
83+
{
84+
relations: relationsToLoad,
85+
take: productsToUpdate.length,
86+
}
2987
)
88+
89+
const products = await this.find(findOptions, context)
3090
const productsMap = new Map(products.map((p) => [p.id, p]))
3191

32-
for (const update of updates) {
33-
const product = productsMap.get(update.id)!
92+
const productIds = Array.from(productsMap.keys())
93+
const productsNotFound = arrayDifference(productIdsToUpdate, productIds)
94+
95+
if (productsNotFound.length > 0) {
96+
throw new MedusaError(
97+
MedusaError.Types.NOT_FOUND,
98+
`Unable to update the products with ids: ${productsNotFound.join(", ")}`
99+
)
100+
}
101+
102+
for (const productToUpdate of productsToUpdate) {
103+
const product = productsMap.get(productToUpdate.id)!
104+
const wrappedProduct = wrap(product)
34105

35106
// Assign the options first, so they'll be available for the variants loop below
36-
if (update.options) {
37-
wrap(product).assign({ options: update.options })
38-
delete update.options // already assigned above, so no longer necessary
107+
if (productToUpdate.options) {
108+
wrappedProduct.assign({ options: productToUpdate.options })
109+
delete productToUpdate.options // already assigned above, so no longer necessary
39110
}
40111

41-
if (update.variants) {
42-
validateVariantOptions(update.variants, product.options)
112+
if (productToUpdate.variants) {
113+
validateVariantOptions(productToUpdate.variants, product.options)
43114

44-
update.variants.forEach((variant: any) => {
115+
productToUpdate.variants.forEach((variant: any) => {
45116
if (variant.options) {
46117
variant.options = Object.entries(variant.options).map(
47118
([key, value]) => {
@@ -58,37 +129,34 @@ export class ProductRepository extends DALUtils.mikroOrmBaseRepositoryFactory(
58129
})
59130
}
60131

61-
if (update.tags) {
62-
update.tags = update.tags.map((t: { id: string }) => t.id)
132+
if (productToUpdate.tags) {
133+
productToUpdate.tags = productToUpdate.tags.map(
134+
(t: { id: string }) => t.id
135+
)
63136
}
64-
if (update.categories) {
65-
update.categories = update.categories.map((c: { id: string }) => c.id)
137+
if (productToUpdate.categories) {
138+
productToUpdate.categories = productToUpdate.categories.map(
139+
(c: { id: string }) => c.id
140+
)
66141
}
67-
if (update.images) {
68-
update.images = update.images.map((image: any, index: number) => ({
69-
...image,
70-
rank: index,
71-
}))
142+
if (productToUpdate.images) {
143+
productToUpdate.images = productToUpdate.images.map(
144+
(image: any, index: number) => ({
145+
...image,
146+
rank: index,
147+
})
148+
)
72149
}
73150

74-
wrap(product!).assign(update)
151+
wrappedProduct.assign(productToUpdate)
75152
}
76153

77154
// Doing this to ensure updates are returned in the same order they were provided,
78155
// since some core flows rely on this.
79156
// This is a high level of coupling though.
80-
return updates
81-
.map((update) => productsMap.get(update.id))
82-
.filter((product) => product !== undefined)
83-
}
84-
85-
// We should probably fix the column types in the database to avoid this
86-
// It would also match the types in ProductVariant, which are already numbers
87-
protected correctUpdateDTOTypes(update: any) {
88-
update.weight = update.weight?.toString()
89-
update.length = update.length?.toString()
90-
update.height = update.height?.toString()
91-
update.width = update.width?.toString()
157+
return productsToUpdate.map(
158+
(productToUpdate) => productsMap.get(productToUpdate.id)!
159+
)
92160
}
93161

94162
/**

0 commit comments

Comments
 (0)