chore(product): revamp upsertWithReplace and Remove its usage from product creation (#11585)

adrien2p · carlos-r-l-rodrigues · web-flow · commit eeebb35758ea · 2025-02-26T09:53:13.000Z
**What**
- Move create product to use native create by structuring the data appropriately, it means no more `upsertWithReplace` being very poorly performant and got 20x better performances on staging
- Improvements in `upsertWithReplace` to still get performance boost for places that still relies on it. Mostly bulking the operations when possible

Co-authored-by: Carlos R. L. Rodrigues &lt;37986729+carlos-r-l-rodrigues@users.noreply.github.com&gt;
diff --git a/.changeset/clean-cars-build.md b/.changeset/clean-cars-build.md
@@ -0,0 +1,6 @@
+---
+"@medusajs/product": patch
+"@medusajs/utils": patch
+---
+
+chore(product): Remove upsertWithReplace where needed
diff --git a/integration-tests/http/__tests__/product/admin/product-import.spec.ts b/integration-tests/http/__tests__/product/admin/product-import.spec.ts
@@ -533,7 +533,7 @@ medusaIntegrationTestRunner({
         )
       })
 
-      it("should fail on non-existent product fields being present in the CSV", async () => {
+      it("should successfully skip non-existent product fields being present in the CSV", async () => {
         const subscriberExecution = TestEventUtils.waitSubscribersExecution(
           `${Modules.NOTIFICATION}.notification.${CommonEvents.CREATED}`,
           eventBus
@@ -582,10 +582,21 @@ medusaIntegrationTestRunner({
           expect.objectContaining({
             data: expect.objectContaining({
               title: "Product import",
-              description: `Failed to import products from file test.csv`,
+              description:
+                "Product import of file test.csv completed successfully!",
             }),
           })
         )
+
+        const [importedProduct] = (
+          await api.get("/admin/products?limit=1&order=-id", adminHeaders)
+        ).data.products
+
+        expect(importedProduct).not.toEqual(
+          expect.objectContaining({
+            field: "Test product",
+          })
+        )
       })
 
       it("supports importing the v1 template", async () => {
diff --git a/packages/core/utils/src/dal/mikro-orm/integration-tests/__tests__/mikro-orm-repository.spec.ts b/packages/core/utils/src/dal/mikro-orm/integration-tests/__tests__/mikro-orm-repository.spec.ts
@@ -693,6 +693,96 @@ describe("mikroOrmRepository", () => {
       )
     })
 
+    it("should successfully update, create, and delete subentities an entity with a one-to-many relation within a transaction", async () => {
+      const entity1 = {
+        id: "1",
+        title: "en1",
+        entity2: [
+          { id: "2", title: "en2-1", handle: "some-handle" },
+          { id: "3", title: "en2-2", handle: "some-other-handle" },
+        ] as any[],
+      }
+
+      const { entities: entities1, performedActions: performedActions1 } =
+        await manager1().transaction(async (txManager) => {
+          return await manager1().upsertWithReplace(
+            [entity1],
+            {
+              relations: ["entity2"],
+            },
+            {
+              transactionManager: txManager,
+            }
+          )
+        })
+
+      expect(performedActions1).toEqual({
+        created: {
+          [Entity1.name]: [expect.objectContaining({ id: entity1.id })],
+          [Entity2.name]: entities1[0].entity2.map((entity2) =>
+            expect.objectContaining({ id: entity2.id })
+          ),
+        },
+        updated: {},
+        deleted: {},
+      })
+
+      entity1.entity2 = [
+        { id: "2", title: "newen2-1" },
+        { title: "en2-3", handle: "some-new-handle" },
+      ]
+
+      const { entities: entities2, performedActions: performedActions2 } =
+        await manager1().transaction(async (txManager) => {
+          return await manager1().upsertWithReplace(
+            [entity1],
+            {
+              relations: ["entity2"],
+            },
+            { transactionManager: txManager }
+          )
+        })
+
+      const entity2En23 = entities2[0].entity2.find((e) => e.title === "en2-3")!
+
+      expect(performedActions2).toEqual({
+        created: {
+          [Entity2.name]: [expect.objectContaining({ id: entity2En23.id })],
+        },
+        updated: {
+          [Entity1.name]: [expect.objectContaining({ id: entity1.id })],
+          [Entity2.name]: [expect.objectContaining({ id: "2" })],
+        },
+        deleted: {
+          [Entity2.name]: [expect.objectContaining({ id: "3" })],
+        },
+      })
+
+      const listedEntities = await manager1().find({
+        where: { id: "1" },
+        options: { populate: ["entity2"] },
+      })
+
+      expect(listedEntities).toHaveLength(1)
+      expect(listedEntities[0]).toEqual(
+        expect.objectContaining({
+          id: "1",
+          title: "en1",
+        })
+      )
+      expect(listedEntities[0].entity2.getItems()).toHaveLength(2)
+      expect(listedEntities[0].entity2.getItems()).toEqual(
+        expect.arrayContaining([
+          expect.objectContaining({
+            title: "newen2-1",
+          }),
+          expect.objectContaining({
+            title: "en2-3",
+          }),
+        ])
+      )
+    })
+
     it("should update an entity with a one-to-many relation that has the same unique constraint key", async () => {
       const entity1 = {
         id: "1",
@@ -1106,7 +1196,9 @@ describe("mikroOrmRepository", () => {
   describe("error mapping", () => {
     it("should map UniqueConstraintViolationException to MedusaError on upsertWithReplace", async () => {
       const entity3 = { title: "en3" }
+
       await manager3().upsertWithReplace([entity3])
+
       const err = await manager3()
         .upsertWithReplace([entity3])
         .catch((e) => e.message)
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
@@ -803,15 +803,20 @@ export function mikroOrmBaseRepositoryFactory<const T extends object>(
           }
         })
 
-        const qb = manager.qb(relation.pivotEntity)
-        await qb.insert(pivotData).onConflict().ignore().execute()
-
-        await manager.nativeDelete(relation.pivotEntity, {
-          [parentPivotColumn]: (data as any).id,
-          [currentPivotColumn]: {
-            $nin: pivotData.map((d) => d[currentPivotColumn]),
-          },
-        })
+        await promiseAll([
+          manager
+            .qb(relation.pivotEntity)
+            .insert(pivotData)
+            .onConflict()
+            .ignore()
+            .execute(),
+          manager.nativeDelete(relation.pivotEntity, {
+            [parentPivotColumn]: (data as any).id,
+            [currentPivotColumn]: {
+              $nin: pivotData.map((d) => d[currentPivotColumn]),
+            },
+          }),
+        ])
 
         return { entities: normalizedData, performedActions }
       }
@@ -826,27 +831,23 @@ export function mikroOrmBaseRepositoryFactory<const T extends object>(
           joinColumnsConstraints[joinColumn] = data[referencedColumnName]
         })
 
-        const toDeleteEntities = await manager.find<any, any, "id">(
-          relation.type,
-          {
-            ...joinColumnsConstraints,
-            id: { $nin: normalizedData.map((d: any) => d.id) },
-          },
-          {
-            fields: ["id"],
-          }
+        const deletedRelations = await (
+          manager.getTransactionContext() ?? manager.getKnex()
         )
-        const toDeleteIds = toDeleteEntities.map((d: any) => d.id)
-
-        await manager.nativeDelete(relation.type, {
-          ...joinColumnsConstraints,
-          id: { $in: toDeleteIds },
-        })
+          .queryBuilder()
+          .from(relation.targetMeta!.collection)
+          .delete()
+          .where(joinColumnsConstraints)
+          .whereNotIn(
+            "id",
+            normalizedData.map((d: any) => d.id)
+          )
+          .returning("id")
 
-        if (toDeleteEntities.length) {
+        if (deletedRelations.length) {
           performedActions.deleted[relation.type] ??= []
           performedActions.deleted[relation.type].push(
-            ...toDeleteEntities.map((d) => ({ id: d.id }))
+            ...deletedRelations.map((row) => ({ id: row.id }))
           )
         }
 
@@ -970,38 +971,46 @@ export function mikroOrmBaseRepositoryFactory<const T extends object>(
         deleted: {},
       }
 
-      await promiseAll(
-        entries.map(async (data) => {
-          const existingEntity = existingEntitiesMap.get(data.id)
-          orderedEntities.push(data)
-          if (existingEntity) {
-            if (skipUpdate) {
-              return
-            }
-            await manager.nativeUpdate(entityName, { id: data.id }, data)
-            performedActions.updated[entityName] ??= []
-            performedActions.updated[entityName].push({ id: data.id })
-          } else {
-            const qb = manager.qb(entityName)
-            if (skipUpdate) {
-              const res = await qb
-                .insert(data)
-                .onConflict()
-                .ignore()
-                .execute("all", true)
-              if (res) {
-                performedActions.created[entityName] ??= []
-                performedActions.created[entityName].push({ id: data.id })
-              }
-            } else {
-              await manager.insert(entityName, data)
-              performedActions.created[entityName] ??= []
-              performedActions.created[entityName].push({ id: data.id })
-              // await manager.insert(entityName, data)
-            }
+      const promises: Promise<any>[] = []
+      const toInsert: unknown[] = []
+      let shouldInsert = false
+
+      entries.map(async (data) => {
+        const existingEntity = existingEntitiesMap.get(data.id)
+        orderedEntities.push(data)
+        if (existingEntity) {
+          if (skipUpdate) {
+            return
           }
-        })
-      )
+          const update = manager.nativeUpdate(entityName, { id: data.id }, data)
+          promises.push(update)
+
+          performedActions.updated[entityName] ??= []
+          performedActions.updated[entityName].push({ id: data.id })
+        } else {
+          shouldInsert = true
+          toInsert.push(data)
+        }
+      })
+
+      if (shouldInsert) {
+        let insertQb = manager.qb(entityName).insert(toInsert).returning("id")
+
+        if (skipUpdate) {
+          insertQb = insertQb.onConflict().ignore()
+        }
+
+        promises.push(
+          insertQb.execute("all", true).then((res: { id: string }[]) => {
+            performedActions.created[entityName] ??= []
+            performedActions.created[entityName].push(
+              ...res.map((data) => ({ id: data.id }))
+            )
+          })
+        )
+      }
+
+      await promiseAll(promises)
 
       return { orderedEntities, performedActions }
     }
diff --git a/packages/core/utils/src/dml/helpers/entity-builder/define-relationship.ts b/packages/core/utils/src/dml/helpers/entity-builder/define-relationship.ts
@@ -283,14 +283,17 @@ export function defineHasManyRelationship(
 ) {
   const shouldRemoveRelated = !!cascades.delete?.includes(relationship.name)
 
-  OneToMany({
+  const options: Parameters<typeof OneToMany>[0] = {
     entity: relatedModelName,
     orphanRemoval: true,
     mappedBy: relationship.mappedBy || camelToSnakeCase(MikroORMEntity.name),
-    cascade: shouldRemoveRelated
-      ? (["persist", "soft-remove"] as any)
-      : undefined,
-  })(MikroORMEntity.prototype, relationship.name)
+  }
+
+  if (shouldRemoveRelated) {
+    options.cascade = ["persist", "soft-remove"] as any
+  }
+
+  OneToMany(options)(MikroORMEntity.prototype, relationship.name)
 }
 
 /**
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
@@ -48,11 +48,11 @@ export const buildProductAndRelationsData = ({
   images,
   status,
   type_id,
-  tags,
+  tag_ids,
   options,
   variants,
   collection_id,
-}: Partial<ProductTypes.CreateProductDTO> & { tags: { value: string }[] }) => {
+}: Partial<ProductTypes.CreateProductDTO>) => {
   const defaultOptionTitle = "test-option"
   const defaultOptionValue = "test-value"
 
@@ -66,7 +66,7 @@ export const buildProductAndRelationsData = ({
     status: status ?? ProductStatus.PUBLISHED,
     images: (images ?? []) as ProductImage[],
     type_id,
-    tags: tags ?? [{ value: "tag-1" }],
+    tag_ids,
     collection_id,
     options: options ?? [
       {
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
diff --git a/packages/modules/product/package.json b/packages/modules/product/package.json
diff --git a/packages/modules/product/src/services/product-module-service.ts b/packages/modules/product/src/services/product-module-service.ts
