Skip to content

Commit

Permalink
fix(medusa): ensure transaction is reused when service calls itself (#…
Browse files Browse the repository at this point in the history
…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>
  • Loading branch information
srindom and olivermrbl authored Jan 19, 2024
1 parent 12aa173 commit 1b99c5d
Showing 1 changed file with 13 additions and 10 deletions.
23 changes: 13 additions & 10 deletions packages/medusa/src/services/product.ts
Original file line number Diff line number Diff line change
@@ -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"

Expand Down Expand Up @@ -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 = {
Expand Down Expand Up @@ -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"],
})

Expand Down Expand Up @@ -645,7 +646,7 @@ class ProductService extends TransactionBaseService {
}
}

const product = await this.retrieve(productId, {
const product = await this.withTransaction(manager).retrieve(productId, {
relations,
})

Expand Down Expand Up @@ -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"],
})

Expand Down Expand Up @@ -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)
Expand All @@ -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"],
})

Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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"],
})

Expand Down

0 comments on commit 1b99c5d

Please sign in to comment.