chore(orchestration): validate PK when throwIfKeyNotFound (#11190)

CLOSES: FRMW-2895

.changeset/cool-walls-smoke.md

@@ -0,0 +1,7 @@
+---
+"@medusajs/orchestration": patch
+"@medusajs/core-flows": patch
+"@medusajs/framework": patch
+---
+
+chore(orchestration): validate missing PK filters when throwIfKeyNotFound

integration-tests/modules/__tests__/modules/remote-query.spec.ts

@@ -1,6 +1,6 @@
+import { medusaIntegrationTestRunner } from "@medusajs/test-utils"
 import { IRegionModuleService, RemoteQueryFunction } from "@medusajs/types"
 import { ContainerRegistrationKeys, Modules } from "@medusajs/utils"
-import { medusaIntegrationTestRunner } from "@medusajs/test-utils"
 import { createAdminUser } from "../../..//helpers/create-admin-user"
 import { adminHeaders } from "../../../helpers/create-admin-user"
 
@@ -70,6 +70,77 @@ medusaIntegrationTestRunner({
         )
       })
 
+      it("should fail to retrieve not passing primary key in filters", async () => {
+        const noPk = remoteQuery(
+          {
+            region: {
+              fields: ["id", "currency_code"],
+            },
+          },
+          { throwIfKeyNotFound: true }
+        )
+        await expect(noPk).rejects.toThrow(
+          "Region: Primary key(s) [id, iso_2] not found in filters"
+        )
+
+        const noPk2 = remoteQuery(
+          {
+            country: {
+              fields: ["*"],
+            },
+          },
+          { throwIfKeyNotFound: true }
+        )
+        await expect(noPk2).rejects.toThrow(
+          "Country: Primary key(s) [id, iso_2] not found in filters"
+        )
+
+        const noPk3 = remoteQuery(
+          {
+            country: {
+              fields: ["*"],
+              __args: {
+                iso_2: undefined,
+              },
+            },
+          },
+          { throwIfKeyNotFound: true }
+        )
+        await expect(noPk3).rejects.toThrow(
+          "Country: Value for primary key iso_2 not found in filters"
+        )
+
+        const noPk4 = remoteQuery(
+          {
+            region: {
+              fields: ["id", "currency_code"],
+              __args: {
+                id: null,
+              },
+            },
+          },
+          { throwIfKeyNotFound: true }
+        )
+        await expect(noPk4).rejects.toThrow(
+          "Region: Value for primary key id not found in filters"
+        )
+
+        const noPk5 = remoteQuery(
+          {
+            region: {
+              fields: ["id", "currency_code"],
+              __args: {
+                currency_code: "EUR",
+              },
+            },
+          },
+          { throwIfKeyNotFound: true }
+        )
+        await expect(noPk5).rejects.toThrow(
+          "Region: Primary key(s) [id, iso_2] not found in filters"
+        )
+      })
+
       it("should fail if a expected relation is not found", async () => {
         const region = await regionModule.createRegions({
           name: "Test Region",

packages/core/core-flows/src/order/workflows/claim/confirm-claim-request.ts

@@ -387,13 +387,6 @@ export const confirmClaimRequestWorkflow = createWorkflow(
       updateReturnsStep(updateReturnDate)
     })
 
-    const claimId = transform(
-      { createdClaimItems },
-      ({ createdClaimItems }) => {
-        return createdClaimItems?.[0]?.claim_id
-      }
-    )
-
     const { returnShippingMethod, claimShippingMethod } = transform(
       { orderPreview, orderClaim, returnId },
       extractShippingOption
@@ -421,7 +414,7 @@ export const confirmClaimRequestWorkflow = createWorkflow(
           "additional_items.item.variant.inventory_items.inventory.location_levels.stock_locations.sales_channels.id",
           "additional_items.item.variant.inventory_items.inventory.location_levels.stock_locations.sales_channels.name",
         ],
-        variables: { id: claimId },
+        variables: { id: input.claim_id },
         list: false,
         throw_if_key_not_found: true,
       }).config({ name: "claim-query" })
@@ -477,7 +470,6 @@ export const confirmClaimRequestWorkflow = createWorkflow(
           id: returnShippingMethod.shipping_option_id,
         },
         list: false,
-        throw_if_key_not_found: true,
       }).config({ name: "claim-return-shipping-option" })
 
       const fulfillmentData = transform(

packages/core/core-flows/src/order/workflows/create-fulfillment.ts

@@ -384,7 +384,6 @@ export const createOrderFulfillmentWorkflow = createWorkflow(
         id: shippingOptionId,
       },
       list: false,
-      throw_if_key_not_found: true,
     }).config({ name: "get-shipping-option" })
 
     const lineItemIds = transform(

packages/core/core-flows/src/order/workflows/exchange/confirm-exchange-request.ts

@@ -373,13 +373,6 @@ export const confirmExchangeRequestWorkflow = createWorkflow(
       updateReturnsStep(updateReturnData)
     })
 
-    const exchangeId = transform(
-      { createdExchangeItems },
-      ({ createdExchangeItems }) => {
-        return createdExchangeItems?.[0]?.exchange_id
-      }
-    )
-
     const { returnShippingMethod, exchangeShippingMethod } = transform(
       { orderPreview, orderExchange, returnId },
       extractShippingOption
@@ -407,7 +400,7 @@ export const confirmExchangeRequestWorkflow = createWorkflow(
           "additional_items.item.variant.inventory_items.inventory.location_levels.stock_locations.sales_channels.id",
           "additional_items.item.variant.inventory_items.inventory.location_levels.stock_locations.sales_channels.name",
         ],
-        variables: { id: exchangeId },
+        variables: { id: input.exchange_id },
         list: false,
         throw_if_key_not_found: true,
       }).config({ name: "exchange-query" })
@@ -463,7 +456,6 @@ export const confirmExchangeRequestWorkflow = createWorkflow(
           id: returnShippingMethod.shipping_option_id,
         },
         list: false,
-        throw_if_key_not_found: true,
       }).config({ name: "exchange-return-shipping-option" })
 
       const fulfillmentData = transform(

packages/core/core-flows/src/order/workflows/payments/create-order-refund-credit-lines.ts

@@ -97,7 +97,6 @@ export const createOrderRefundCreditLinesWorkflow = createWorkflow(
         order_id: input.order_id,
         status: [OrderChangeStatus.PENDING],
       },
-      options: { throwIfKeyNotFound: true },
     }).config({ name: "order-change-query" })
 
     const orderChange = transform(

packages/core/core-flows/src/order/workflows/return/confirm-return-request.ts

@@ -326,7 +326,6 @@ export const confirmReturnRequestWorkflow = createWorkflow(
           id: returnShippingOptionId,
         },
         list: false,
-        throw_if_key_not_found: true,
       }).config({ name: "return-shipping-option" })
 
       const fulfillmentData = transform(

packages/core/core-flows/src/order/workflows/return/create-complete-return.ts

@@ -256,10 +256,7 @@ export type CreateCompleteReturnValidationStepInput = {
 export const createCompleteReturnValidationStep = createStep(
   "create-return-order-validation",
   async function (
-    {
+    { order, input }: CreateCompleteReturnValidationStepInput,
-      order,
-      input,
-    }: CreateCompleteReturnValidationStepInput,
     context
   ) {
     if (!input.items) {
@@ -348,7 +345,6 @@ export const createAndCompleteReturnOrderWorkflow = createWorkflow(
       ],
       variables: returnShippingOptionsVariables,
       list: false,
-      throw_if_key_not_found: true,
     }).config({ name: "return-shipping-option" })
 
     const shippingMethodData = transform(

packages/core/core-flows/src/order/workflows/transfer/accept-order-transfer.ts

@@ -18,9 +18,9 @@ import {
 } from "@medusajs/utils"
 
 import { useQueryGraphStep } from "../../../common"
-import { throwIfOrderIsCancelled } from "../../utils/order-validation"
 import { previewOrderChangeStep } from "../../steps"
 import { confirmOrderChanges } from "../../steps/confirm-order-changes"
+import { throwIfOrderIsCancelled } from "../../utils/order-validation"
 
 /**
  * The details of the order transfer acceptance to validate.
@@ -149,7 +149,6 @@ export const acceptOrderTransferWorkflow = createWorkflow(
         order_id: input.order_id,
         status: [OrderChangeStatus.REQUESTED],
       },
-      options: { throwIfKeyNotFound: true },
     }).config({ name: "order-change-query" })
 
     const orderChange = transform(

packages/core/core-flows/src/order/workflows/transfer/cancel-order-transfer.ts

@@ -143,7 +143,6 @@ export const cancelOrderTransferRequestWorkflow = createWorkflow(
         order_id: input.order_id,
         status: [OrderChangeStatus.PENDING, OrderChangeStatus.REQUESTED],
       },
-      options: { throwIfKeyNotFound: true },
     }).config({ name: "order-change-query" })
 
     const orderChange = transform(

packages/core/core-flows/src/order/workflows/transfer/decline-order-transfer.ts

@@ -133,7 +133,6 @@ export const declineOrderTransferRequestWorkflow = createWorkflow(
         order_id: input.order_id,
         status: [OrderChangeStatus.PENDING, OrderChangeStatus.REQUESTED],
       },
-      options: { throwIfKeyNotFound: true },
     }).config({ name: "order-change-query" })
 
     const orderChange = transform(

packages/core/framework/src/http/middlewares/ensure-publishable-api-key.ts

@@ -33,8 +33,7 @@ export async function ensurePublishableApiKeyMiddleware(
   const query = req.scope.resolve(ContainerRegistrationKeys.QUERY)
 
   try {
-    const { data } = await query.graph(
+    const { data } = await query.graph({
-      {
       entity: "api_key",
       fields: ["id", "token", "sales_channels_link.sales_channel_id"],
       filters: {
@@ -45,9 +44,7 @@ export async function ensurePublishableApiKeyMiddleware(
           { revoked_at: { $gt: new Date() } },
         ],
       },
-      },
+    })
-      { throwIfKeyNotFound: true }
-    )
 
     apiKey = data[0]
   } catch (e) {

packages/core/orchestration/src/__tests__/joiner/remote-joiner-data.ts

@@ -832,5 +832,21 @@ describe("RemoteJoiner", () => {
     await expect(dataNotFound).rejects.toThrowError(
       "order id not found: ord_1234556"
     )
+
+    const queryNotFoundNoParam = RemoteJoiner.parseQuery(`
+      query {
+        order {
+          id
+          number
+        }
+      }
+    `)
+    const dataNotFoundNoPK = joiner.query(queryNotFoundNoParam, {
+      throwIfKeyNotFound: true,
+    })
+
+    await expect(dataNotFoundNoPK).rejects.toThrowError(
+      "order: Primary key(s) [id] not found in filters"
+    )
   })
 })

packages/core/orchestration/src/joiner/remote-joiner.ts

@@ -37,6 +37,15 @@ type InternalImplodeMapping = {
   isList?: boolean
 }
 
+type InternalParseExpandsParams = {
+  initialService: RemoteExpandProperty
+  query: RemoteJoinerQuery
+  serviceConfig: InternalJoinerServiceConfig
+  expands: RemoteJoinerQuery["expands"]
+  implodeMapping: InternalImplodeMapping[]
+  options?: RemoteJoinerOptions
+}
+
 export class RemoteJoiner {
   private serviceConfigCache: Map<string, InternalJoinerServiceConfig> =
     new Map()
@@ -831,14 +840,9 @@ export class RemoteJoiner {
     })
   }
 
-  private parseExpands(params: {
+  private parseExpands(
-    initialService: RemoteExpandProperty
+    params: InternalParseExpandsParams
-    query: RemoteJoinerQuery
+  ): Map<string, RemoteExpandProperty> {
-    serviceConfig: InternalJoinerServiceConfig
-    expands: RemoteJoinerQuery["expands"]
-    implodeMapping: InternalImplodeMapping[]
-    options?: RemoteJoinerOptions
-  }): Map<string, RemoteExpandProperty> {
     const { initialService, query, serviceConfig, expands, implodeMapping } =
       params
 
@@ -1203,8 +1207,30 @@ export class RemoteJoiner {
       queryObj,
     })
 
+    if (options?.throwIfKeyNotFound) {
+      if (primaryKeyArg?.value == undefined) {
+        if (!primaryKeyArg) {
+          throw new MedusaError(
+            MedusaError.Types.NOT_FOUND,
+            `${
+              serviceConfig.entity ?? serviceConfig.serviceName
+            }: Primary key(s) [${serviceConfig.primaryKeys.join(
+              ", "
+            )}] not found in filters`
+          )
+        }
+
+        throw new MedusaError(
+          MedusaError.Types.NOT_FOUND,
+          `${
+            serviceConfig.entity ?? serviceConfig.serviceName
+          }: Value for primary key ${primaryKeyArg.name} not found in filters`
+        )
+      }
+    }
+
     const implodeMapping: InternalImplodeMapping[] = []
-    const parseExpandsConfig: Parameters<typeof this.parseExpands>[0] = {
+    const parseExpandsConfig: InternalParseExpandsParams = {
       initialService: {
         property: "",
         parent: "",