fix(medusa): Transaction lock issues on create/update cart items

.changeset/perfect-bears-invent.md

@@ -0,0 +1,5 @@
+---
+"@medusajs/medusa": patch
+---
+
+fix(medusa): Transaction lock issues on create/update cart items

packages/medusa/src/api/routes/store/carts/__tests__/create-line-item.js

@@ -25,7 +25,7 @@ describe("POST /store/carts/:id", () => {
     })
 
     it("calls CartService retrieve", () => {
-      expect(CartServiceMock.retrieve).toHaveBeenCalledTimes(2)
+      expect(CartServiceMock.retrieve).toHaveBeenCalledTimes(1)
       expect(CartServiceMock.retrieveWithTotals).toHaveBeenCalledTimes(1)
     })
 

packages/medusa/src/api/routes/store/carts/create-line-item.ts → packages/medusa/src/api/routes/store/carts/create-line-item/index.ts

@@ -1,9 +1,16 @@
 import { IsInt, IsOptional, IsString } from "class-validator"
 import { EntityManager } from "typeorm"
-import { defaultStoreCartFields, defaultStoreCartRelations } from "."
-import { CartService, LineItemService } from "../../../../services"
-import { validator } from "../../../../utils/validator"
-import { FlagRouter } from "../../../../utils/flag-router"
+import { validator } from "../../../../../utils/validator"
+import {
+  CreateLineItemSteps,
+  handleAddOrUpdateLineItem,
+} from "./utils/handler-steps"
+import { IdempotencyKey } from "../../../../../models"
+import {
+  initializeIdempotencyRequest,
+  runIdempotencyStep,
+  RunIdempotencyStepOptions,
+} from "../../../../../utils/idempotency"
 
 /**
  * @oas [post] /carts/{id}/line-items
@@ -63,46 +70,65 @@ import { FlagRouter } from "../../../../utils/flag-router"
 export default async (req, res) => {
   const { id } = req.params
 
-  const customerId = req.user?.customer_id
+  const customerId: string | undefined = req.user?.customer_id
   const validated = await validator(StorePostCartsCartLineItemsReq, req.body)
 
-  const lineItemService: LineItemService = req.scope.resolve("lineItemService")
-  const cartService: CartService = req.scope.resolve("cartService")
-
   const manager: EntityManager = req.scope.resolve("manager")
-  const featureFlagRouter: FlagRouter = req.scope.resolve("featureFlagRouter")
 
-  await manager.transaction(async (m) => {
-    const txCartService = cartService.withTransaction(m)
-    const cart = await txCartService.retrieve(id)
+  let idempotencyKey!: IdempotencyKey
+  try {
+    idempotencyKey = await initializeIdempotencyRequest(req, res)
+  } catch {
+    res.status(409).send("Failed to create idempotency key")
+    return
+  }
 
-    const line = await lineItemService
-      .withTransaction(m)
-      .generate(validated.variant_id, cart.region_id, validated.quantity, {
-        customer_id: customerId || cart.customer_id,
-        metadata: validated.metadata,
-      })
+  let inProgress = true
+  let err: unknown = false
 
-    await txCartService.addLineItem(id, line, {
-      validateSalesChannels:
-        featureFlagRouter.isFeatureEnabled("sales_channels"),
-    })
+  const stepOptions: RunIdempotencyStepOptions = {
+    manager,
+    idempotencyKey,
+    container: req.scope,
+    isolationLevel: "SERIALIZABLE",
+  }
 
-    const updated = await txCartService.retrieve(id, {
-      relations: ["payment_sessions"],
-    })
+  while (inProgress) {
+    switch (idempotencyKey.recovery_point) {
+      case CreateLineItemSteps.STARTED: {
+        await runIdempotencyStep(async ({ manager }) => {
+          return await handleAddOrUpdateLineItem(
+            id,
+            {
+              customer_id: customerId,
+              metadata: validated.metadata,
+              quantity: validated.quantity,
+              variant_id: validated.variant_id,
+            },
+            {
+              manager,
+              container: req.scope,
+            }
+          )
+        }, stepOptions).catch((e) => {
+          inProgress = false
+          err = e
+        })
+        break
+      }
 
-    if (updated.payment_sessions?.length) {
-      await txCartService.setPaymentSessions(id)
+      case CreateLineItemSteps.FINISHED: {
+        inProgress = false
+        break
+      }
     }
-  })
+  }
 
-  const data = await cartService.retrieveWithTotals(id, {
-    select: defaultStoreCartFields,
-    relations: defaultStoreCartRelations,
-  })
+  if (err) {
+    throw err
+  }
 
-  res.status(200).json({ cart: data })
+  res.status(idempotencyKey.response_code).json(idempotencyKey.response_body)
 }
 
 export class StorePostCartsCartLineItemsReq {

packages/medusa/src/api/routes/store/carts/create-line-item/utils/handler-steps.ts

@@ -0,0 +1,67 @@
+import { AwilixContainer } from "awilix"
+import { EntityManager } from "typeorm"
+import { CartService, LineItemService } from "../../../../../../services"
+import { FlagRouter } from "../../../../../../utils/flag-router"
+import { defaultStoreCartFields, defaultStoreCartRelations } from "../../index"
+import { IdempotencyCallbackResult } from "../../../../../../types/idempotency-key"
+import { WithRequiredProperty } from "../../../../../../types/common"
+import { Cart } from "../../../../../../models"
+
+export const CreateLineItemSteps = {
+  STARTED: "started",
+  FINISHED: "finished",
+}
+
+export async function handleAddOrUpdateLineItem(
+  cartId: string,
+  data: {
+    metadata?: Record<string, unknown>
+    customer_id?: string
+    variant_id: string
+    quantity: number
+  },
+  { container, manager }: { container: AwilixContainer; manager: EntityManager }
+): Promise<IdempotencyCallbackResult> {
+  const cartService: CartService = container.resolve("cartService")
+  const lineItemService: LineItemService = container.resolve("lineItemService")
+  const featureFlagRouter: FlagRouter = container.resolve("featureFlagRouter")
+
+  const txCartService = cartService.withTransaction(manager)
+
+  let cart = await txCartService.retrieve(cartId, {
+    select: ["id", "region_id", "customer_id"],
+  })
+
+  const line = await lineItemService
+    .withTransaction(manager)
+    .generate(data.variant_id, cart.region_id, data.quantity, {
+      customer_id: data.customer_id || cart.customer_id,
+      metadata: data.metadata,
+    })
+
+  await txCartService.addLineItem(cart.id, line, {
+    validateSalesChannels: featureFlagRouter.isFeatureEnabled("sales_channels"),
+  })
+
+  cart = await txCartService.retrieveWithTotals(cart.id, {
+    select: defaultStoreCartFields,
+    relations: [
+      ...defaultStoreCartRelations,
+      "billing_address",
+      "region.payment_providers",
+      "payment_sessions",
+      "customer",
+    ],
+  })
+
+  if (cart.payment_sessions?.length) {
+    await txCartService.setPaymentSessions(
+      cart as WithRequiredProperty<Cart, "total">
+    )
+  }
+
+  return {
+    response_code: 200,
+    response_body: { cart },
+  }
+}

packages/medusa/src/services/__mocks__/idempotency-key.js

@@ -11,24 +11,20 @@ export const IdempotencyKeyService = {
     }
   }),
   workStage: jest.fn().mockImplementation(async (key, fn) => {
-    try {
-      const { recovery_point, response_code, response_body } = await fn(
-        MockManager
-      )
+    const { recovery_point, response_code, response_body } = await fn(
+      MockManager
+    )
 
-      if (recovery_point) {
-        return {
-          recovery_point,
-        }
-      } else {
-        return {
-          recovery_point: "finished",
-          response_body,
-          response_code,
-        }
+    if (recovery_point) {
+      return {
+        recovery_point,
+      }
+    } else {
+      return {
+        recovery_point: "finished",
+        response_body,
+        response_code,
       }
-    } catch (err) {
-      return { error: err }
     }
   }),
 }

packages/medusa/src/services/__tests__/cart.js

@@ -284,7 +284,8 @@ describe("CartService", () => {
 
   describe("addLineItem", () => {
     const lineItemService = {
-      update: jest.fn(),
+      update: jest.fn().mockImplementation(() => Promise.resolve()),
+      list: jest.fn().mockImplementation(() => Promise.resolve([])),
       create: jest.fn(),
       withTransaction: function () {
         return this
@@ -353,7 +354,27 @@ describe("CartService", () => {
       manager: MockManager,
       totalsService,
       cartRepository,
-      lineItemService,
+      lineItemService: {
+        ...lineItemService,
+        list: jest.fn().mockImplementation((where) => {
+          if (
+            where.cart_id === IdMap.getId("cartWithLine") &&
+            where.variant_id === IdMap.getId("existing") &&
+            where.should_merge
+          ) {
+            return Promise.resolve([
+              {
+                ...where,
+                id: IdMap.getId("merger"),
+                quantity: 1,
+                metadata: {},
+              },
+            ])
+          }
+
+          return Promise.resolve([])
+        }),
+      },
       lineItemRepository: MockRepository(),
       newTotalsService: newTotalsServiceMock,
       eventBusService,
@@ -453,12 +474,14 @@ describe("CartService", () => {
         variant_id: IdMap.getId("existing"),
         should_merge: true,
         quantity: 1,
+        metadata: {},
       }
 
       await cartService.addLineItem(IdMap.getId("cartWithLine"), lineItem)
 
-      expect(lineItemService.update).toHaveBeenCalledTimes(1)
-      expect(lineItemService.update).toHaveBeenCalledWith(
+      expect(lineItemService.update).toHaveBeenCalledTimes(2)
+      expect(lineItemService.update).toHaveBeenNthCalledWith(
+        1,
         IdMap.getId("merger"),
         {
           quantity: 2,
@@ -519,8 +542,9 @@ describe("CartService", () => {
 
   describe("addLineItem w. SalesChannel", () => {
     const lineItemService = {
-      update: jest.fn(),
+      update: jest.fn().mockImplementation(() => Promise.resolve()),
       create: jest.fn(),
+      list: jest.fn().mockImplementation(() => Promise.resolve([])),
       withTransaction: function () {
         return this
       },