From 1b99c5dc22833b8a5e840598ab8fe0cda309d4ef Mon Sep 17 00:00:00 2001 From: Sebastian Rindom Date: Fri, 19 Jan 2024 12:03:17 +0100 Subject: [PATCH] fix(medusa): ensure transaction is reused when service calls itself (#6089) **What** - ProductService.create calls ProductService.retrieve before returning. This fix ensures that the manager created in the atomicPhase is used when ProductService.retrieve is called. **Why** - Without the explicit use of the same manager in the retrieve call, race conditions easily occur if you have concurrent ProductService.create calls. The code below can reproduce the scenario: ```javascript const productData = [ { barcode: `1234233423-${Math.random() * 100}`, external_id: `1234233423-${Math.random() * 100}`, description: "super cool product", discountable: true, hs_code: "1234213", origin_country: "DK", title: `Super cool product ${Math.random() * 100}`, type: { value: "Eyewear" }, sales_channels: [{ id: sc.id }], }, { barcode: `1234233423-${Math.random() * 100}`, external_id: `random-external-id-${Math.random() * 100}`, description: "super cool product", discountable: true, hs_code: "1234213", origin_country: "DK", title: `Super cool product ${Math.random() * 100}`, type: { value: "Eyewear" }, sales_channels: [{ id: sc.id }], }, ]; const result = await Promise.all( productData.map(async (product) => productService.create(product)) ); ``` **Explaination** What happens is the following: - Request 1 calls `ProductService.create`. This in turn calls `atomicPhase` which sets the `ProductService.transactionManager_ = txForReqOne` - Request 1 creates the product in the DB with `txForReqOne`. - Request 2 calls `ProductService.create`. This in turn calls `atomicPhase` which sets the `ProductService.transactionManager_ = txForReqTwo` - Request 1 reaches the end of `ProductService.create` where `this.retrieve` is called. Because the ProductService is a singleton the retrieve call will attempt to use `txForReqTwo` to fetch the product. - **Error**: since `txForReqTwo` can't read the data of `txForReqOne`, the product is not found. Co-authored-by: Oli Juhl <59018053+olivermrbl@users.noreply.github.com> --- packages/medusa/src/services/product.ts | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/packages/medusa/src/services/product.ts b/packages/medusa/src/services/product.ts index 349f50eb97..dfd67e145c 100644 --- a/packages/medusa/src/services/product.ts +++ b/packages/medusa/src/services/product.ts @@ -1,12 +1,13 @@ +import { RemoteQueryFunction } from "@medusajs/types" import { buildRelations, buildSelects, FlagRouter, MedusaV2Flag, objectToStringPath, - promiseAll, selectorConstraintsToString, + promiseAll, + selectorConstraintsToString, } from "@medusajs/utils" -import { RemoteQueryFunction } from "@medusajs/types" import { isDefined, MedusaError } from "medusa-core-utils" import { EntityManager, In } from "typeorm" @@ -42,9 +43,9 @@ import { ProductSelector, UpdateProductInput, } from "../types/product" +import { CreateProductVariantInput } from "../types/product-variant" import { buildQuery, isString, setMetadata } from "../utils" import EventBusService from "./event-bus" -import { CreateProductVariantInput } from "../types/product-variant" import SalesChannelService from "./sales-channel" type InjectedDependencies = { @@ -594,7 +595,7 @@ class ProductService extends TransactionBaseService { ) } - const result = await this.retrieve(product.id, { + const result = await this.withTransaction(manager).retrieve(product.id, { relations: ["options"], }) @@ -645,7 +646,7 @@ class ProductService extends TransactionBaseService { } } - const product = await this.retrieve(productId, { + const product = await this.withTransaction(manager).retrieve(productId, { relations, }) @@ -805,7 +806,7 @@ class ProductService extends TransactionBaseService { this.productOptionRepository_ ) - const product = await this.retrieve(productId, { + const product = await this.withTransaction(manager).retrieve(productId, { relations: ["options", "variants"], }) @@ -833,7 +834,7 @@ class ProductService extends TransactionBaseService { ) } - const result = await this.retrieve(productId) + const result = await this.withTransaction(manager).retrieve(productId) await this.eventBus_ .withTransaction(manager) @@ -849,7 +850,7 @@ class ProductService extends TransactionBaseService { return await this.atomicPhase_(async (manager) => { const productRepo = manager.withRepository(this.productRepository_) - const product = await this.retrieve(productId, { + const product = await this.withTransaction(manager).retrieve(productId, { relations: ["variants"], }) @@ -898,7 +899,9 @@ class ProductService extends TransactionBaseService { this.productOptionRepository_ ) - const product = await this.retrieve(productId, { relations: ["options"] }) + const product = await this.withTransaction(manager).retrieve(productId, { + relations: ["options"], + }) const { title, values } = data @@ -973,7 +976,7 @@ class ProductService extends TransactionBaseService { this.productOptionRepository_ ) - const product = await this.retrieve(productId, { + const product = await this.withTransaction(manager).retrieve(productId, { relations: ["variants", "variants.options"], })