feat(medusa): Prevent cart completion conflict (#5814)

This commit is contained in:
Adrien de Peretti
2023-12-19 10:47:41 +01:00
committed by GitHub
parent 9cc787cac4
commit 496dcf10c4
10 changed files with 179 additions and 125 deletions
+5
View File
@@ -0,0 +1,5 @@
---
"@medusajs/medusa": patch
---
Feat/cart completion conflict fixes
@@ -26,7 +26,7 @@ describe("POST /store/carts/:id", () => {
it("calls CartService retrieve", () => { it("calls CartService retrieve", () => {
expect(CartServiceMock.retrieve).toHaveBeenCalledTimes(1) expect(CartServiceMock.retrieve).toHaveBeenCalledTimes(1)
expect(CartServiceMock.retrieveWithTotals).toHaveBeenCalledTimes(1) expect(CartServiceMock.retrieveWithTotals).toHaveBeenCalledTimes(2)
}) })
it("calls LineItemService generate", () => { it("calls LineItemService generate", () => {
@@ -22,7 +22,7 @@ describe("POST /store/carts/:id/payment-sessions", () => {
}) })
it("calls Cart service retrieve", () => { it("calls Cart service retrieve", () => {
expect(CartServiceMock.retrieveWithTotals).toHaveBeenCalledTimes(1) expect(CartServiceMock.retrieveWithTotals).toHaveBeenCalledTimes(2)
}) })
it("returns 200", () => { it("returns 200", () => {
@@ -4,7 +4,7 @@ import { validator } from "../../../../../utils/validator"
import { import {
addOrUpdateLineItem, addOrUpdateLineItem,
CreateLineItemSteps, CreateLineItemSteps,
setPaymentSession, setPaymentSessions,
setVariantAvailability, setVariantAvailability,
} from "./utils/handler-steps" } from "./utils/handler-steps"
import { IdempotencyKey } from "../../../../../models" import { IdempotencyKey } from "../../../../../models"
@@ -13,7 +13,6 @@ import { cleanResponseData } from "../../../../../utils/clean-response-data"
import IdempotencyKeyService from "../../../../../services/idempotency-key" import IdempotencyKeyService from "../../../../../services/idempotency-key"
import { defaultStoreCartFields, defaultStoreCartRelations } from "../index" import { defaultStoreCartFields, defaultStoreCartRelations } from "../index"
import { CartService } from "../../../../../services" import { CartService } from "../../../../../services"
import { promiseAll } from "@medusajs/utils"
/** /**
* @oas [post] /store/carts/{id}/line-items * @oas [post] /store/carts/{id}/line-items
@@ -130,37 +129,42 @@ export default async (req, res) => {
case CreateLineItemSteps.SET_PAYMENT_SESSIONS: { case CreateLineItemSteps.SET_PAYMENT_SESSIONS: {
try { try {
const cartService: CartService = req.scope.resolve("cartService") const cartService: CartService = req.scope.resolve("cartService")
const getCart = async () => {
const cart = await cartService return await cartService
.withTransaction(manager) .withTransaction(manager)
.retrieveWithTotals(id, { .retrieveWithTotals(id, {
select: defaultStoreCartFields, select: defaultStoreCartFields,
relations: [ relations: [
...defaultStoreCartRelations, ...defaultStoreCartRelations,
"billing_address", "region.tax_rates",
"region.payment_providers", "customer",
"payment_sessions", ],
"customer", })
],
})
const args = {
cart,
container: req.scope,
manager,
} }
await promiseAll([ const cart = await getCart()
setVariantAvailability(args),
setPaymentSession(args), await manager.transaction(async (transactionManager) => {
]) await setPaymentSessions({
cart,
container: req.scope,
manager: transactionManager,
})
})
const freshCart = await getCart()
await setVariantAvailability({
cart: freshCart,
container: req.scope,
manager,
})
idempotencyKey = await idempotencyKeyService idempotencyKey = await idempotencyKeyService
.withTransaction(manager) .withTransaction(manager)
.update(idempotencyKey.idempotency_key, { .update(idempotencyKey.idempotency_key, {
recovery_point: CreateLineItemSteps.FINISHED, recovery_point: CreateLineItemSteps.FINISHED,
response_code: 200, response_code: 200,
response_body: { cart }, response_body: { cart: freshCart },
}) })
} catch (e) { } catch (e) {
inProgress = false inProgress = false
@@ -44,7 +44,7 @@ export async function addOrUpdateLineItem({
}) })
} }
export async function setPaymentSession({ cart, container, manager }) { export async function setPaymentSessions({ cart, container, manager }) {
const cartService: CartService = container.resolve("cartService") const cartService: CartService = container.resolve("cartService")
const txCartService = cartService.withTransaction(manager) const txCartService = cartService.withTransaction(manager)
@@ -1,12 +1,12 @@
import { import { CartService } from "../../../../services"
CartService,
ProductVariantInventoryService,
} from "../../../../services"
import { defaultStoreCartFields, defaultStoreCartRelations } from "." import { defaultStoreCartFields, defaultStoreCartRelations } from "."
import { EntityManager } from "typeorm" import { EntityManager } from "typeorm"
import IdempotencyKeyService from "../../../../services/idempotency-key" import IdempotencyKeyService from "../../../../services/idempotency-key"
import { cleanResponseData } from "../../../../utils/clean-response-data" import { cleanResponseData } from "../../../../utils/clean-response-data"
import { setVariantAvailability } from "./create-line-item/utils/handler-steps"
import { WithRequiredProperty } from "../../../../types/common"
import { Cart } from "../../../../models"
/** /**
* @oas [post] /store/carts/{id}/payment-sessions * @oas [post] /store/carts/{id}/payment-sessions
@@ -55,14 +55,10 @@ import { cleanResponseData } from "../../../../utils/clean-response-data"
export default async (req, res) => { export default async (req, res) => {
const { id } = req.params const { id } = req.params
const cartService: CartService = req.scope.resolve("cartService")
const idempotencyKeyService: IdempotencyKeyService = req.scope.resolve( const idempotencyKeyService: IdempotencyKeyService = req.scope.resolve(
"idempotencyKeyService" "idempotencyKeyService"
) )
const productVariantInventoryService: ProductVariantInventoryService =
req.scope.resolve("productVariantInventoryService")
const manager: EntityManager = req.scope.resolve("manager") const manager: EntityManager = req.scope.resolve("manager")
const headerKey = req.get("Idempotency-Key") || "" const headerKey = req.get("Idempotency-Key") || ""
@@ -88,40 +84,53 @@ export default async (req, res) => {
while (inProgress) { while (inProgress) {
switch (idempotencyKey.recovery_point) { switch (idempotencyKey.recovery_point) {
case "started": { case "started": {
await manager try {
.transaction("SERIALIZABLE", async (transactionManager) => { const cartService: CartService = req.scope.resolve("cartService")
idempotencyKey = await idempotencyKeyService const getCart = async () => {
.withTransaction(transactionManager) return await cartService
.workStage( .withTransaction(manager)
idempotencyKey.idempotency_key, .retrieveWithTotals(
async (stageManager) => { id,
await cartService {
.withTransaction(stageManager) select: defaultStoreCartFields,
.setPaymentSessions(id) relations: [
...defaultStoreCartRelations,
const cart = await cartService "region.tax_rates",
.withTransaction(stageManager) "customer",
.retrieveWithTotals(id, { ],
select: defaultStoreCartFields, },
relations: defaultStoreCartRelations, { force_taxes: true }
})
await productVariantInventoryService.setVariantAvailability(
cart.items.map((i) => i.variant),
cart.sales_channel_id!
)
return {
response_code: 200,
response_body: { cart },
}
}
) )
}
const cart = await getCart()
await manager.transaction(async (transactionManager) => {
const txCartService =
cartService.withTransaction(transactionManager)
await txCartService.setPaymentSessions(
cart as WithRequiredProperty<Cart, "total">
)
}) })
.catch((e) => {
inProgress = false const freshCart = await getCart()
err = e await setVariantAvailability({
cart: freshCart,
container: req.scope,
manager,
}) })
idempotencyKey = await idempotencyKeyService
.withTransaction(manager)
.update(idempotencyKey.idempotency_key, {
recovery_point: "finished",
response_code: 200,
response_body: { cart: freshCart },
})
} catch (e) {
inProgress = false
err = e
}
break break
} }
+15 -6
View File
@@ -1641,23 +1641,32 @@ describe("CartService", () => {
const cartRepository = MockRepository({ const cartRepository = MockRepository({
findOneWithRelations: (rel, q) => { findOneWithRelations: (rel, q) => {
if (q.where.id === IdMap.getId("cart-to-filter")) { if (q.where.id === IdMap.getId("cart-to-filter")) {
return Promise.resolve(cart3) return Promise.resolve({
id: IdMap.getId("cart-to-filter"),
...cart3,
})
} }
if (q.where.id === IdMap.getId("cart-with-session")) { if (q.where.id === IdMap.getId("cart-with-session")) {
return Promise.resolve(cart2) return Promise.resolve({
id: IdMap.getId("cart-with-session"),
...cart2,
})
} }
if (q.where.id === IdMap.getId("cart-remove")) { if (q.where.id === IdMap.getId("cart-remove")) {
return Promise.resolve(cart4) return Promise.resolve({ id: IdMap.getId("cart-remove"), ...cart4 })
} }
if (q.where.id === IdMap.getId("cart-negative")) { if (q.where.id === IdMap.getId("cart-negative")) {
return Promise.resolve(cart4) return Promise.resolve({ id: IdMap.getId("cart-negative"), ...cart4 })
} }
if ( if (
q.where.id === IdMap.getId("cartWithMixedSelectedInitiatedSessions") q.where.id === IdMap.getId("cartWithMixedSelectedInitiatedSessions")
) { ) {
return Promise.resolve(cart5) return Promise.resolve({
id: IdMap.getId("cartWithMixedSelectedInitiatedSessions"),
...cart5,
})
} }
return Promise.resolve(cart1) return Promise.resolve({ id: q.where.id, ...cart1 })
}, },
}) })
+42 -37
View File
@@ -1693,27 +1693,32 @@ class CartService extends TransactionBaseService {
* a payment object, that we will use to update our cart payment with. * a payment object, that we will use to update our cart payment with.
* Additionally, if the payment does not require more or fails, we will * Additionally, if the payment does not require more or fails, we will
* set the payment on the cart. * set the payment on the cart.
* @param cartId - the id of the cart to authorize payment for * @param cartOrId - the id of the cart to authorize payment for
* @param context - object containing whatever is relevant for * @param context - object containing whatever is relevant for
* authorizing the payment with the payment provider. As an example, * authorizing the payment with the payment provider. As an example,
* this could be IP address or similar for fraud handling. * this could be IP address or similar for fraud handling.
* @return the resulting cart * @return the resulting cart
*/ */
async authorizePayment( async authorizePayment(
cartId: string, cartOrId: string | WithRequiredProperty<Cart, "total">,
context: Record<string, unknown> & { context: Record<string, unknown> = { cart_id: "" }
cart_id: string
} = { cart_id: "" }
): Promise<Cart> { ): Promise<Cart> {
context = {
...context,
cart_id: isString(cartOrId) ? cartOrId : cartOrId.id,
}
return await this.atomicPhase_( return await this.atomicPhase_(
async (transactionManager: EntityManager) => { async (transactionManager: EntityManager) => {
const cartRepository = transactionManager.withRepository( const cartRepository = transactionManager.withRepository(
this.cartRepository_ this.cartRepository_
) )
const cart = await this.retrieveWithTotals(cartId, { const cart = !isString(cartOrId)
relations: ["payment_sessions", "items.variant.product.profiles"], ? cartOrId
}) : await this.retrieveWithTotals(cartOrId, {
relations: ["payment_sessions", "items.variant.product.profiles"],
})
// If cart total is 0, we don't perform anything payment related // If cart total is 0, we don't perform anything payment related
if (cart.total! <= 0) { if (cart.total! <= 0) {
@@ -1875,8 +1880,7 @@ class CartService extends TransactionBaseService {
await this.eventBus_ await this.eventBus_
.withTransaction(transactionManager) .withTransaction(transactionManager)
.emit(CartService.Events.UPDATED, { id: cartId }) .emit(CartService.Events.UPDATED, { id: cartId })
}, }
"SERIALIZABLE"
) )
} }
@@ -1889,7 +1893,9 @@ class CartService extends TransactionBaseService {
* @param cartOrCartId - the id of the cart to set payment session for * @param cartOrCartId - the id of the cart to set payment session for
* @return the result of the update operation. * @return the result of the update operation.
*/ */
async setPaymentSessions(cartOrCartId: Cart | string): Promise<void> { async setPaymentSessions(
cartOrCartId: WithRequiredProperty<Cart, "total"> | string
): Promise<void> {
return await this.atomicPhase_( return await this.atomicPhase_(
async (transactionManager: EntityManager) => { async (transactionManager: EntityManager) => {
const psRepo = transactionManager.withRepository( const psRepo = transactionManager.withRepository(
@@ -1899,31 +1905,30 @@ class CartService extends TransactionBaseService {
const paymentProviderServiceTx = const paymentProviderServiceTx =
this.paymentProviderService_.withTransaction(transactionManager) this.paymentProviderService_.withTransaction(transactionManager)
const cartId = const cart = !isString(cartOrCartId)
typeof cartOrCartId === `string` ? cartOrCartId : cartOrCartId.id ? cartOrCartId
: await this.retrieveWithTotals(
const cart = await this.retrieveWithTotals( cartOrCartId,
cartId, {
{ relations: [
relations: [ "items.variant.product.profiles",
"items.variant.product.profiles", "items.adjustments",
"items.adjustments", "discounts",
"discounts", "discounts.rule",
"discounts.rule", "gift_cards",
"gift_cards", "shipping_methods",
"shipping_methods", "shipping_methods.shipping_option",
"shipping_methods.shipping_option", "billing_address",
"billing_address", "shipping_address",
"shipping_address", "region",
"region", "region.tax_rates",
"region.tax_rates", "region.payment_providers",
"region.payment_providers", "payment_sessions",
"payment_sessions", "customer",
"customer", ],
], },
}, { force_taxes: true }
{ force_taxes: true } )
)
const { total, region } = cart const { total, region } = cart
@@ -1957,7 +1962,7 @@ class CartService extends TransactionBaseService {
currency_code: cart.region.currency_code, currency_code: cart.region.currency_code,
} }
const partialPaymentSessionData = { const partialPaymentSessionData = {
cart_id: cartId, cart_id: cart.id,
data: {}, data: {},
status: PaymentSessionStatus.PENDING, status: PaymentSessionStatus.PENDING,
amount: total, amount: total,
@@ -62,9 +62,10 @@ const toTest = [
expect(cartServiceMock.authorizePayment).toHaveBeenCalledTimes(1) expect(cartServiceMock.authorizePayment).toHaveBeenCalledTimes(1)
expect(cartServiceMock.authorizePayment).toHaveBeenCalledWith( expect(cartServiceMock.authorizePayment).toHaveBeenCalledWith(
"test-cart", expect.objectContaining({
id: "test-cart",
}),
{ {
cart_id: "test-cart",
idempotency_key: { idempotency_key: {
idempotency_key: "ikey", idempotency_key: "ikey",
recovery_point: "tax_lines_created", recovery_point: "tax_lines_created",
@@ -2,12 +2,13 @@ import {
IEventBusService, IEventBusService,
IInventoryService, IInventoryService,
ReservationItemDTO, ReservationItemDTO,
WithRequiredProperty,
} from "@medusajs/types" } from "@medusajs/types"
import { import {
AbstractCartCompletionStrategy, AbstractCartCompletionStrategy,
CartCompletionResponse, CartCompletionResponse,
} from "../interfaces" } from "../interfaces"
import { IdempotencyKey, Order } from "../models" import { Cart, IdempotencyKey, Order } from "../models"
import { import {
PaymentProviderService, PaymentProviderService,
ProductVariantInventoryService, ProductVariantInventoryService,
@@ -84,7 +85,7 @@ class CartCompletionStrategy extends AbstractCartCompletionStrategy {
switch (idempotencyKey.recovery_point) { switch (idempotencyKey.recovery_point) {
case "started": { case "started": {
await this.activeManager_ await this.activeManager_
.transaction("SERIALIZABLE", async (transactionManager) => { .transaction(async (transactionManager) => {
idempotencyKey = await this.idempotencyKeyService_ idempotencyKey = await this.idempotencyKeyService_
.withTransaction(transactionManager) .withTransaction(transactionManager)
.workStage( .workStage(
@@ -101,7 +102,7 @@ class CartCompletionStrategy extends AbstractCartCompletionStrategy {
} }
case "tax_lines_created": { case "tax_lines_created": {
await this.activeManager_ await this.activeManager_
.transaction("SERIALIZABLE", async (transactionManager) => { .transaction(async (transactionManager) => {
idempotencyKey = await this.idempotencyKeyService_ idempotencyKey = await this.idempotencyKeyService_
.withTransaction(transactionManager) .withTransaction(transactionManager)
.workStage( .workStage(
@@ -122,7 +123,7 @@ class CartCompletionStrategy extends AbstractCartCompletionStrategy {
case "payment_authorized": { case "payment_authorized": {
await this.activeManager_ await this.activeManager_
.transaction("SERIALIZABLE", async (transactionManager) => { .transaction(async (transactionManager) => {
idempotencyKey = await this.idempotencyKeyService_ idempotencyKey = await this.idempotencyKeyService_
.withTransaction(transactionManager) .withTransaction(transactionManager)
.workStage( .workStage(
@@ -246,19 +247,39 @@ class CartCompletionStrategy extends AbstractCartCompletionStrategy {
const txCartService = this.cartService_.withTransaction(manager) const txCartService = this.cartService_.withTransaction(manager)
let cart = await txCartService.retrieve(id, { let cart: Cart | WithRequiredProperty<Cart, "total"> =
relations: ["payment_sessions"], await txCartService.retrieveWithTotals(id, {
}) relations: [
"items.variant.product.profiles",
"items.adjustments",
"discounts",
"discounts.rule",
"gift_cards",
"shipping_methods",
"shipping_methods.shipping_option",
"billing_address",
"shipping_address",
"region",
"region.tax_rates",
"region.payment_providers",
"payment_sessions",
"customer",
],
})
if (cart.payment_sessions?.length) { if (cart.payment_sessions?.length) {
await txCartService.setPaymentSessions(id) await txCartService.setPaymentSessions(
cart as WithRequiredProperty<Cart, "total">
)
} }
cart = await txCartService.authorizePayment(id, { cart = await txCartService.authorizePayment(
...context, cart as WithRequiredProperty<Cart, "total">,
cart_id: id, {
idempotency_key: idempotencyKey, ...context,
}) idempotency_key: idempotencyKey,
}
)
if (cart.payment_session) { if (cart.payment_session) {
if ( if (