fix(product): Deep update data retrieval bottleneck (#12538)

* fix(product): Deep update data retrieval bottleneck

* Create shiny-spiders-matter.md

* fix(product): Deep update data retrieval bottleneck

.changeset/shiny-spiders-matter.md

@@ -0,0 +1,5 @@
+---
+"@medusajs/product": patch
+---
+
+fix(product): Deep update data retrieval bottleneck

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

@@ -33,7 +33,6 @@ import {
 jest.setTimeout(300000)
 
 moduleIntegrationTestRunner<IProductModuleService>({
-  debug: true,
   moduleName: Modules.PRODUCT,
   injectedDependencies: {
     [Modules.EVENT_BUS]: new MockEventBusService(),

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

@@ -1,10 +1,14 @@
 import { Product, ProductOption } from "@models"
 
 import { Context, DAL, InferEntityType } from "@medusajs/framework/types"
-import { buildQuery, DALUtils } from "@medusajs/framework/utils"
+import {
+  arrayDifference,
+  buildQuery,
+  DALUtils,
+  MedusaError,
+} from "@medusajs/framework/utils"
 import { SqlEntityManager, wrap } from "@mikro-orm/postgresql"
 
-// eslint-disable-next-line max-len
 export class ProductRepository extends DALUtils.mikroOrmBaseRepositoryFactory(
   Product
 ) {
@@ -13,35 +17,102 @@ export class ProductRepository extends DALUtils.mikroOrmBaseRepositoryFactory(
     super(...arguments)
   }
 
+  /**
+   * Identify the relations to load for the given update.
+   * @param update
+   * @returns
+   */
+  static #getProductDeepUpdateRelationsToLoad(
+    productsToUpdate: any[]
+  ): string[] {
+    const relationsToLoad = new Set<string>()
+    productsToUpdate.forEach((productToUpdate) => {
+      if (productToUpdate.options) {
+        relationsToLoad.add("options")
+        relationsToLoad.add("options.values")
+      }
+      if (productToUpdate.variants) {
+        relationsToLoad.add("options")
+        relationsToLoad.add("options.values")
+        relationsToLoad.add("variants")
+        relationsToLoad.add("variants.options")
+        relationsToLoad.add("variants.options.option")
+      }
+      if (productToUpdate.tags) relationsToLoad.add("tags")
+      if (productToUpdate.categories) relationsToLoad.add("categories")
+      if (productToUpdate.images) relationsToLoad.add("images")
+      if (productToUpdate.collection) relationsToLoad.add("collection")
+      if (productToUpdate.type) relationsToLoad.add("type")
+    })
+    return Array.from(relationsToLoad)
+  }
+
+  // We should probably fix the column types in the database to avoid this
+  // It would also match the types in ProductVariant, which are already numbers
+  static #correctUpdateDTOTypes(productToUpdate: {
+    weight?: string | number
+    length?: string | number
+    height?: string | number
+    width?: string | number
+  }) {
+    productToUpdate.weight = productToUpdate.weight?.toString()
+    productToUpdate.length = productToUpdate.length?.toString()
+    productToUpdate.height = productToUpdate.height?.toString()
+    productToUpdate.width = productToUpdate.width?.toString()
+  }
+
   async deepUpdate(
-    updates: any[],
+    productsToUpdate: ({ id: string } & any)[],
     validateVariantOptions: (
       variants: any[],
       options: InferEntityType<typeof ProductOption>[]
     ) => void,
     context: Context = {}
   ): Promise<InferEntityType<typeof Product>[]> {
-    updates.forEach((update) => this.correctUpdateDTOTypes(update))
+    const productIdsToUpdate: string[] = []
+    productsToUpdate.forEach((productToUpdate) => {
+      ProductRepository.#correctUpdateDTOTypes(productToUpdate)
+      productIdsToUpdate.push(productToUpdate.id)
+    })
 
-    const products = await this.find(
+    const relationsToLoad =
-      buildQuery({ id: updates.map((p) => p.id) }, { relations: ["*"] }),
+      ProductRepository.#getProductDeepUpdateRelationsToLoad(productsToUpdate)
-      context
+
+    const findOptions = buildQuery(
+      { id: productIdsToUpdate },
+      {
+        relations: relationsToLoad,
+        take: productsToUpdate.length,
+      }
     )
+
+    const products = await this.find(findOptions, context)
     const productsMap = new Map(products.map((p) => [p.id, p]))
 
-    for (const update of updates) {
+    const productIds = Array.from(productsMap.keys())
-      const product = productsMap.get(update.id)!
+    const productsNotFound = arrayDifference(productIdsToUpdate, productIds)
+
+    if (productsNotFound.length > 0) {
+      throw new MedusaError(
+        MedusaError.Types.NOT_FOUND,
+        `Unable to update the products with ids: ${productsNotFound.join(", ")}`
+      )
+    }
+
+    for (const productToUpdate of productsToUpdate) {
+      const product = productsMap.get(productToUpdate.id)!
+      const wrappedProduct = wrap(product)
 
       // Assign the options first, so they'll be available for the variants loop below
-      if (update.options) {
+      if (productToUpdate.options) {
-        wrap(product).assign({ options: update.options })
+        wrappedProduct.assign({ options: productToUpdate.options })
-        delete update.options // already assigned above, so no longer necessary
+        delete productToUpdate.options // already assigned above, so no longer necessary
       }
 
-      if (update.variants) {
+      if (productToUpdate.variants) {
-        validateVariantOptions(update.variants, product.options)
+        validateVariantOptions(productToUpdate.variants, product.options)
 
-        update.variants.forEach((variant: any) => {
+        productToUpdate.variants.forEach((variant: any) => {
           if (variant.options) {
             variant.options = Object.entries(variant.options).map(
               ([key, value]) => {
@@ -58,37 +129,34 @@ export class ProductRepository extends DALUtils.mikroOrmBaseRepositoryFactory(
         })
       }
 
-      if (update.tags) {
+      if (productToUpdate.tags) {
-        update.tags = update.tags.map((t: { id: string }) => t.id)
+        productToUpdate.tags = productToUpdate.tags.map(
+          (t: { id: string }) => t.id
+        )
       }
-      if (update.categories) {
+      if (productToUpdate.categories) {
-        update.categories = update.categories.map((c: { id: string }) => c.id)
+        productToUpdate.categories = productToUpdate.categories.map(
+          (c: { id: string }) => c.id
+        )
       }
-      if (update.images) {
+      if (productToUpdate.images) {
-        update.images = update.images.map((image: any, index: number) => ({
+        productToUpdate.images = productToUpdate.images.map(
-          ...image,
+          (image: any, index: number) => ({
-          rank: index,
+            ...image,
-        }))
+            rank: index,
+          })
+        )
       }
 
-      wrap(product!).assign(update)
+      wrappedProduct.assign(productToUpdate)
     }
 
     // Doing this to ensure updates are returned in the same order they were provided,
     // since some core flows rely on this.
     // This is a high level of coupling though.
-    return updates
+    return productsToUpdate.map(
-      .map((update) => productsMap.get(update.id))
+      (productToUpdate) => productsMap.get(productToUpdate.id)!
-      .filter((product) => product !== undefined)
+    )
-  }
-
-  // We should probably fix the column types in the database to avoid this
-  // It would also match the types in ProductVariant, which are already numbers
-  protected correctUpdateDTOTypes(update: any) {
-    update.weight = update.weight?.toString()
-    update.length = update.length?.toString()
-    update.height = update.height?.toString()
-    update.width = update.width?.toString()
   }
 
   /**