From e77a02aca5d301e5cfe964b42adcaa9a500f1a7e Mon Sep 17 00:00:00 2001 From: Adrien de Peretti Date: Mon, 18 Mar 2024 09:37:59 +0100 Subject: [PATCH] feat(): Update transformer middleware and API (#6647) **What** Update all transform middleware to support the new API - deprecate `defaultRelations` - deprecate `allowedRelations` - Add `defaults` and `allowed` in replacement for `defaultFields` and `allowedFields` respectively - in the `defaults` it is possible to specify a field such as `*variants` in order to be recognized as a relation only without specifying any property - add support for `remoteQueryConfig` assigned to req like we have for `listConfig` and `retrieveConfig` - add support to override `allowed|allowedFields` if a previous middleware have set it up on the req.allowed - The api now accepts `fields` as the only accepted fields to manage the requested props and relations, the `expand` property have been deprecated. New supported symbols have been added in complement of the fields - `+` (e.g `/store/products?fields=+description`) to specify that description should be added as part of the returned data among the other defined fields - `-` (e.g `/store/products?fields=-description`) to specify that description should be removed as part of the returned data - `*` (e.g `/store/products?fields=*variants`) to specify that the variants relations should be added as part of the returned data among the other defined fields without having to specify which property of the variants should be returned. In the `defaults` config of the transform middleware it is also possible to use this symbol - In the case no symbol is provided, it will replace the default fields and mean that only the specified fields must be returned About the allowed validation, all fields in the `defaults` configuration must be present in the `allowed` configuration. In case the `defaults` contains full relation selection (e.g `*product.variants`) it should be present in the `allowed` as `product.variants`. In case in the `defaults` you add `product.variants.id`, it will be allowed if the `allowed` configuration includes either `product.variants.id` as full match or `product.variants` as it means that we allow all properties from `product.variants` Also, support for `*` selection on the remote query/joiner have been added **Note** All v2 end points refactoring can be done separately --- .../api/__tests__/admin/product-category.ts | 4 +- .../api/__tests__/admin/product.js | 2 +- .../api/__tests__/store/order-edit.js | 2 +- .../api/__tests__/store/orders.js | 10 +- .../api/__tests__/store/product-category.ts | 13 +- .../api/__tests__/store/products.js | 10 +- .../src/medusa-test-runner.ts | 13 - .../src/api-v2/admin/products/query-config.ts | 22 +- .../medusa/src/api-v2/admin/products/route.ts | 6 +- .../src/api-v2/store/carts/[id]/route.ts | 2 +- .../src/api-v2/store/carts/query-config.ts | 34 +- .../__tests__/transform-query.spec.ts | 622 ++++++++++++++++++ .../src/api/middlewares/transform-query.ts | 153 ++--- .../admin/orders/__tests__/get-order.js | 11 +- .../admin/products/__tests__/get-product.js | 7 +- .../admin/regions/__tests__/get-region.js | 5 +- .../admin/regions/__tests__/list-regions.js | 10 +- .../src/api/routes/store/carts/get-cart.ts | 2 +- .../store/regions/__tests__/get-region.js | 4 +- .../store/regions/__tests__/list-regions.js | 7 +- .../src/strategies/batch-jobs/order/export.ts | 10 +- .../strategies/batch-jobs/product/export.ts | 4 +- packages/medusa/src/types/common.ts | 24 + packages/medusa/src/types/global.ts | 41 +- packages/medusa/src/types/routing.ts | 35 +- packages/medusa/src/utils/get-query-config.ts | 308 +++++---- packages/modules-sdk/src/remote-query.ts | 4 + .../src/__tests__/joiner/remote-joiner.ts | 145 ++++ .../orchestration/src/joiner/remote-joiner.ts | 22 +- .../mikro-orm/mikro-orm-create-connection.ts | 5 + 30 files changed, 1186 insertions(+), 351 deletions(-) create mode 100644 packages/medusa/src/api/middlewares/__tests__/transform-query.spec.ts diff --git a/integration-tests/api/__tests__/admin/product-category.ts b/integration-tests/api/__tests__/admin/product-category.ts index 0ab98ceac4..51d527eea9 100644 --- a/integration-tests/api/__tests__/admin/product-category.ts +++ b/integration-tests/api/__tests__/admin/product-category.ts @@ -1157,7 +1157,7 @@ describe("/admin/product-categories", () => { expect(error.response.status).toEqual(400) expect(error.response.data).toEqual({ - message: "Relations [products] are not valid", + message: "Requested fields [products] are not valid", type: "invalid_data", }) }) @@ -1291,7 +1291,7 @@ describe("/admin/product-categories", () => { expect(error.response.status).toEqual(400) expect(error.response.data).toEqual({ - message: "Relations [products] are not valid", + message: "Requested fields [products] are not valid", type: "invalid_data", }) }) diff --git a/integration-tests/api/__tests__/admin/product.js b/integration-tests/api/__tests__/admin/product.js index 4cd74724e5..3037f63294 100644 --- a/integration-tests/api/__tests__/admin/product.js +++ b/integration-tests/api/__tests__/admin/product.js @@ -1071,9 +1071,9 @@ medusaIntegrationTestRunner({ expect(res.data.product.id).toEqual(productId) expect(keysInResponse).toEqual( expect.arrayContaining([ - // fields "id", "created_at", + // fields "updated_at", "deleted_at", "title", diff --git a/integration-tests/api/__tests__/store/order-edit.js b/integration-tests/api/__tests__/store/order-edit.js index 1a1172d269..419f86eb44 100644 --- a/integration-tests/api/__tests__/store/order-edit.js +++ b/integration-tests/api/__tests__/store/order-edit.js @@ -240,7 +240,7 @@ describe("/store/order-edits", () => { .catch((e) => e) expect(err.response.data.message).toBe( - "Fields [internal_note] are not valid" + "Requested fields [internal_note] are not valid" ) }) }) diff --git a/integration-tests/api/__tests__/store/orders.js b/integration-tests/api/__tests__/store/orders.js index 01a9b6d758..5f387f59fd 100644 --- a/integration-tests/api/__tests__/store/orders.js +++ b/integration-tests/api/__tests__/store/orders.js @@ -215,9 +215,12 @@ describe("/store/carts", () => { "/store/orders?display_id=111&email=test@email.com&fields=status,email" ) - expect(Object.keys(response.data.order)).toHaveLength(22) + expect(Object.keys(response.data.order)).toHaveLength(24) expect(Object.keys(response.data.order)).toEqual( expect.arrayContaining([ + "id", + "created_at", + // fields "status", "email", @@ -252,9 +255,11 @@ describe("/store/carts", () => { const response = await api.get("/store/orders/order_test?fields=status") - expect(Object.keys(response.data.order)).toHaveLength(21) + expect(Object.keys(response.data.order)).toHaveLength(22) expect(Object.keys(response.data.order)).toEqual( expect.arrayContaining([ + "id", + // fields "status", @@ -292,6 +297,7 @@ describe("/store/carts", () => { expect(Object.keys(response.data.order).sort()).toEqual( [ + "id", // fields "status", diff --git a/integration-tests/api/__tests__/store/product-category.ts b/integration-tests/api/__tests__/store/product-category.ts index 1a2a8200b2..40f6b21afe 100644 --- a/integration-tests/api/__tests__/store/product-category.ts +++ b/integration-tests/api/__tests__/store/product-category.ts @@ -1,10 +1,11 @@ -import { ProductCategory } from "@medusajs/medusa" +import {ProductCategory} from "@medusajs/medusa" import path from "path" -import startServerWithEnvironment from "../../../environment-helpers/start-server-with-environment" -import { useApi } from "../../../environment-helpers/use-api" -import { useDb } from "../../../environment-helpers/use-db" -import { simpleProductCategoryFactory } from "../../../factories" +import startServerWithEnvironment + from "../../../environment-helpers/start-server-with-environment" +import {useApi} from "../../../environment-helpers/use-api" +import {useDb} from "../../../environment-helpers/use-db" +import {simpleProductCategoryFactory} from "../../../factories" jest.setTimeout(30000) @@ -138,7 +139,7 @@ describe("/store/product-categories", () => { expect(error.response.status).toEqual(400) expect(error.response.data.type).toEqual("invalid_data") expect(error.response.data.message).toEqual( - "Fields [mpath] are not valid" + "Requested fields [mpath] are not valid" ) }) diff --git a/integration-tests/api/__tests__/store/products.js b/integration-tests/api/__tests__/store/products.js index 045cb3fcc3..4bfbf6c3f0 100644 --- a/integration-tests/api/__tests__/store/products.js +++ b/integration-tests/api/__tests__/store/products.js @@ -112,8 +112,9 @@ describe("/store/products", () => { response.data.products.find((p) => p.id === testProductId1) ) - expect(testProductIndex).toBe(3) - expect(testProduct1Index).toBe(4) + // Since they have the same variant titles for rank 2, the order is not guaranteed + expect([3, 4]).toContain(testProductIndex) + expect([3, 4]).toContain(testProduct1Index) }) it("returns a list of ordered products by variants title ASC", async () => { @@ -262,9 +263,12 @@ describe("/store/products", () => { expect(response.status).toEqual(200) - expect(Object.keys(response.data.products[0])).toHaveLength(8) + expect(Object.keys(response.data.products[0])).toHaveLength(10) expect(Object.keys(response.data.products[0])).toEqual( expect.arrayContaining([ + "id", + "created_at", + // fields "handle", // relations diff --git a/packages/medusa-test-utils/src/medusa-test-runner.ts b/packages/medusa-test-utils/src/medusa-test-runner.ts index f33df322fe..e038e7affc 100644 --- a/packages/medusa-test-utils/src/medusa-test-runner.ts +++ b/packages/medusa-test-utils/src/medusa-test-runner.ts @@ -7,19 +7,6 @@ import { createMedusaContainer } from "@medusajs/utils" const axios = require("axios").default -const keepTables = [ - /*"store",*/ - /* "staged_job", - "shipping_profile", - "fulfillment_provider", - "payment_provider", - "country", - "region_country", - "currency", - "migrations", - "mikro_orm_migrations",*/ -] - const DB_HOST = process.env.DB_HOST const DB_USERNAME = process.env.DB_USERNAME const DB_PASSWORD = process.env.DB_PASSWORD diff --git a/packages/medusa/src/api-v2/admin/products/query-config.ts b/packages/medusa/src/api-v2/admin/products/query-config.ts index 3ccd209f2e..1c0703948f 100644 --- a/packages/medusa/src/api-v2/admin/products/query-config.ts +++ b/packages/medusa/src/api-v2/admin/products/query-config.ts @@ -44,8 +44,6 @@ export const defaultAdminProductsOptionFields = ["id", "title"] export const retrieveOptionConfig = { defaultFields: defaultAdminProductsOptionFields, - defaultRelations: [], - allowedRelations: [], isList: false, } @@ -55,7 +53,7 @@ export const listOptionConfig = { isList: true, } -export const allowedAdminProductRelations = [ +/* export const allowedAdminProductRelations = [ "variants", // TODO: Add in next iteration // "variants.prices", @@ -68,21 +66,21 @@ export const allowedAdminProductRelations = [ "tags", "type", "collection", -] +]*/ // TODO: This is what we had in the v1 list. Do we still want to expand that much by default? Also this doesn't work in v2 it seems. -export const defaultAdminProductRelations = [ +/* export const defaultAdminProductRelations = [ "variants", - "variants.prices", - "variants.options", - "profiles", + // "variants.prices", + // "variants.options", + // "profiles", "images", "options", - "options.values", + // "options.values", "tags", "type", "collection", -] +]*/ export const defaultAdminProductFields = [ "id", @@ -143,9 +141,7 @@ export const defaultAdminProductFields = [ ] export const retrieveTransformQueryConfig = { - defaultFields: defaultAdminProductFields, - defaultRelations: defaultAdminProductRelations, - allowedRelations: allowedAdminProductRelations, + defaults: defaultAdminProductFields, isList: false, } diff --git a/packages/medusa/src/api-v2/admin/products/route.ts b/packages/medusa/src/api-v2/admin/products/route.ts index d042dd1c87..c25a76cbed 100644 --- a/packages/medusa/src/api-v2/admin/products/route.ts +++ b/packages/medusa/src/api-v2/admin/products/route.ts @@ -54,11 +54,9 @@ export const GET = async ( entryPoint: "product", variables: { filters: filterableFields, - order: req.listConfig.order, - skip: req.listConfig.skip, - take: req.listConfig.take, + ...req.remoteQueryConfig.pagination, }, - fields: req.listConfig.select as string[], + fields: req.remoteQueryConfig.fields, }) const { rows: products, metadata } = await remoteQuery(queryObject) diff --git a/packages/medusa/src/api-v2/store/carts/[id]/route.ts b/packages/medusa/src/api-v2/store/carts/[id]/route.ts index 1db6e194b4..853ff965d9 100644 --- a/packages/medusa/src/api-v2/store/carts/[id]/route.ts +++ b/packages/medusa/src/api-v2/store/carts/[id]/route.ts @@ -12,7 +12,7 @@ export const GET = async (req: MedusaRequest, res: MedusaResponse) => { const query = remoteQueryObjectFromString({ entryPoint: Modules.CART, - fields: defaultStoreCartFields, + fields: req.remoteQueryConfig.fields, }) const [cart] = await remoteQuery(query, { cart: variables }) diff --git a/packages/medusa/src/api-v2/store/carts/query-config.ts b/packages/medusa/src/api-v2/store/carts/query-config.ts index 295a28556a..f7aced7472 100644 --- a/packages/medusa/src/api-v2/store/carts/query-config.ts +++ b/packages/medusa/src/api-v2/store/carts/query-config.ts @@ -75,38 +75,10 @@ export const defaultStoreCartFields = [ "payment_collection.payment_sessions", ] -export const defaultStoreCartRelations = [ - "items", - "items.tax_lines", - "items.adjustments", - "region", - "customer", - "customer.groups", - "shipping_address", - "billing_address", - "shipping_methods", - "shipping_methods.tax_lines", - "shipping_methods.adjustments", -] - -export const allowedRelations = [ - "items", - "items.tax_lines", - "items.adjustments", - "region", - "customer", - "customer.groups", - "shipping_address", - "billing_address", - "shipping_methods", - "shipping_methods.tax_lines", - "shipping_methods.adjustments", - "sales_channel", -] +const allowedFields = [...defaultStoreCartFields] export const retrieveTransformQueryConfig = { - defaultFields: defaultStoreCartFields, - defaultRelations: defaultStoreCartRelations, - allowedRelations: defaultStoreCartRelations, + defaults: defaultStoreCartFields, + allowed: allowedFields, isList: false, } diff --git a/packages/medusa/src/api/middlewares/__tests__/transform-query.spec.ts b/packages/medusa/src/api/middlewares/__tests__/transform-query.spec.ts new file mode 100644 index 0000000000..6031076427 --- /dev/null +++ b/packages/medusa/src/api/middlewares/__tests__/transform-query.spec.ts @@ -0,0 +1,622 @@ +import { NextFunction, Request, Response } from "express" +import { transformQuery } from "../transform-query" +import { extendedFindParamsMixin } from "../../../types/common" +import { MedusaError } from "medusa-core-utils" + +describe("transformQuery", () => { + afterEach(() => { + jest.clearAllMocks() + }) + + it("should transform the input query", async () => { + let mockRequest = { + query: {}, + } as Request + const mockResponse = {} as Response + const nextFunction: NextFunction = jest.fn() + + const expectations = ({ + offset, + limit, + inputOrder, + transformedOrder, + }: { + offset: number + limit: number + inputOrder: string | undefined + transformedOrder: Record + relations?: string[] + }) => { + expect(mockRequest.validatedQuery).toEqual({ + offset, + limit, + order: inputOrder, + }) + expect(mockRequest.filterableFields).toEqual({}) + expect(mockRequest.allowedProperties).toEqual([ + "id", + "created_at", + "updated_at", + "deleted_at", + "metadata.id", + "metadata.parent.id", + "metadata.children.id", + "metadata.product.id", + "metadata", + "metadata.parent", + "metadata.children", + "metadata.product", + ]) + expect(mockRequest.listConfig).toEqual({ + take: limit, + skip: offset, + select: [ + "id", + "created_at", + "updated_at", + "deleted_at", + "metadata.id", + "metadata.parent.id", + "metadata.children.id", + "metadata.product.id", + ], + relations: [ + "metadata", + "metadata.parent", + "metadata.children", + "metadata.product", + ], + order: transformedOrder, + }) + expect(mockRequest.remoteQueryConfig).toEqual({ + fields: [ + "id", + "created_at", + "updated_at", + "deleted_at", + "metadata.id", + "metadata.parent.id", + "metadata.children.id", + "metadata.product.id", + ], + pagination: { + order: transformedOrder, + skip: offset, + take: limit, + }, + }) + } + + let queryConfig: any = { + defaultFields: [ + "id", + "created_at", + "updated_at", + "deleted_at", + "metadata.id", + "metadata.parent.id", + "metadata.children.id", + "metadata.product.id", + ], + defaultRelations: [ + "metadata", + "metadata.parent", + "metadata.children", + "metadata.product", + ], + isList: true, + } + + let middleware = transformQuery(extendedFindParamsMixin(), queryConfig) + + await middleware(mockRequest, mockResponse, nextFunction) + + expectations({ + limit: 20, + offset: 0, + inputOrder: undefined, + transformedOrder: { + created_at: "DESC", + }, + }) + + ////////////////////////////// + + mockRequest = { + query: { + limit: "10", + offset: "5", + order: "created_at", + }, + } as unknown as Request + + middleware = transformQuery(extendedFindParamsMixin(), queryConfig) + + await middleware(mockRequest, mockResponse, nextFunction) + + expectations({ + limit: 10, + offset: 5, + inputOrder: "created_at", + transformedOrder: { created_at: "ASC" }, + }) + + ////////////////////////////// + + mockRequest = { + query: { + limit: "10", + offset: "5", + order: "created_at", + }, + } as unknown as Request + + queryConfig = { + defaults: [ + "id", + "created_at", + "updated_at", + "deleted_at", + "metadata.id", + "metadata.parent.id", + "metadata.children.id", + "metadata.product.id", + ], + isList: true, + } + + middleware = transformQuery(extendedFindParamsMixin(), queryConfig) + + await middleware(mockRequest, mockResponse, nextFunction) + + expectations({ + limit: 10, + offset: 5, + inputOrder: "created_at", + transformedOrder: { created_at: "ASC" }, + }) + }) + + it("should transform the input query taking into account the fields symbols (+,- or no symbol)", async () => { + let mockRequest = { + query: { + fields: "id", + }, + } as unknown as Request + const mockResponse = {} as Response + const nextFunction: NextFunction = jest.fn() + + let queryConfig: any = { + defaultFields: [ + "id", + "created_at", + "updated_at", + "deleted_at", + "metadata.id", + "metadata.parent.id", + "metadata.children.id", + "metadata.product.id", + ], + defaultRelations: [ + "metadata", + "metadata.parent", + "metadata.children", + "metadata.product", + ], + isList: true, + } + + let middleware = transformQuery(extendedFindParamsMixin(), queryConfig) + + await middleware(mockRequest, mockResponse, nextFunction) + + expect(mockRequest.listConfig).toEqual( + expect.objectContaining({ + select: ["id", "created_at"], + }) + ) + + ////////////////////////////// + + mockRequest = { + query: { + fields: "+test_prop,-prop-test-something", + }, + } as unknown as Request + + queryConfig = { + defaultFields: [ + "id", + "prop-test-something", + "created_at", + "updated_at", + "deleted_at", + "metadata.id", + "metadata.parent.id", + "metadata.children.id", + "metadata.product.id", + ], + defaultRelations: [ + "metadata", + "metadata.parent", + "metadata.children", + "metadata.product", + ], + isList: true, + } + + middleware = transformQuery(extendedFindParamsMixin(), queryConfig) + + await middleware(mockRequest, mockResponse, nextFunction) + + expect(mockRequest.listConfig).toEqual( + expect.objectContaining({ + select: [ + "id", + "created_at", + "updated_at", + "deleted_at", + "metadata.id", + "metadata.parent.id", + "metadata.children.id", + "metadata.product.id", + "test_prop", + ], + }) + ) + + ////////////////////////////// + + mockRequest = { + query: { + fields: "+test_prop,-updated_at", + }, + } as unknown as Request + + queryConfig = { + defaults: [ + "id", + "created_at", + "updated_at", + "deleted_at", + "metadata.id", + "metadata.parent.id", + "metadata.children.id", + "metadata.product.id", + ], + isList: true, + } + + middleware = transformQuery(extendedFindParamsMixin(), queryConfig) + + await middleware(mockRequest, mockResponse, nextFunction) + + expect(mockRequest.listConfig).toEqual( + expect.objectContaining({ + select: [ + "id", + "created_at", + "deleted_at", + "metadata.id", + "metadata.parent.id", + "metadata.children.id", + "metadata.product.id", + "test_prop", + ], + }) + ) + }) + + it(`should transform the input and manage the allowed fields and relations properly without error`, async () => { + let mockRequest = { + query: { + fields: "*product.variants,+product.id", + }, + } as unknown as Request + const mockResponse = {} as Response + const nextFunction: NextFunction = jest.fn() + + let queryConfig: any = { + defaults: [ + "id", + "created_at", + "deleted_at", + "metadata.id", + "metadata.parent.id", + "metadata.children.id", + "metadata.product.id", + ], + allowed: [ + "id", + "created_at", + "updated_at", + "deleted_at", + "metadata.id", + "metadata.parent.id", + "metadata.children.id", + "metadata.product.id", + "product", + "product.variants", + ], + isList: true, + } + + let middleware = transformQuery(extendedFindParamsMixin(), queryConfig) + + await middleware(mockRequest, mockResponse, nextFunction) + + expect(mockRequest.listConfig).toEqual( + expect.objectContaining({ + select: [ + "id", + "created_at", + "deleted_at", + "metadata.id", + "metadata.parent.id", + "metadata.children.id", + "metadata.product.id", + "product.id", + ], + relations: [ + "metadata", + "metadata.parent", + "metadata.children", + "metadata.product", + "product", + "product.variants", + ], + }) + ) + expect(mockRequest.remoteQueryConfig).toEqual( + expect.objectContaining({ + fields: [ + "id", + "created_at", + "deleted_at", + "metadata.id", + "metadata.parent.id", + "metadata.children.id", + "metadata.product.id", + "product.id", + "product.variants.*", + ], + }) + ) + }) + + it("should throw when attempting to transform the input if disallowed fields are requested", async () => { + let mockRequest = { + query: { + fields: "+test_prop", + }, + } as unknown as Request + const mockResponse = {} as Response + const nextFunction: NextFunction = jest.fn() + + let queryConfig: any = { + defaultFields: [ + "id", + "created_at", + "updated_at", + "deleted_at", + "metadata.id", + "metadata.parent.id", + "metadata.children.id", + "metadata.product.id", + ], + defaultRelations: [ + "metadata", + "metadata.parent", + "metadata.children", + "metadata.product", + ], + allowedFields: [ + "id", + "created_at", + "updated_at", + "deleted_at", + "metadata.id", + "metadata.parent.id", + "metadata.children.id", + "metadata.product.id", + ], + isList: true, + } + + let middleware = transformQuery(extendedFindParamsMixin(), queryConfig) + + await middleware(mockRequest, mockResponse, nextFunction) + + expect(nextFunction).toHaveBeenCalledWith( + new MedusaError( + MedusaError.Types.INVALID_DATA, + `Requested fields [test_prop] are not valid` + ) + ) + + ////////////////////////////// + + mockRequest = { + query: { + expand: "product", + }, + } as unknown as Request + + queryConfig = { + defaultFields: [ + "id", + "created_at", + "updated_at", + "deleted_at", + "metadata.id", + "metadata.parent.id", + "metadata.children.id", + "metadata.product.id", + ], + defaultRelations: [ + "metadata", + "metadata.parent", + "metadata.children", + "metadata.product", + ], + allowedFields: [ + "id", + "created_at", + "updated_at", + "deleted_at", + "metadata.id", + "metadata.parent.id", + "metadata.children.id", + "metadata.product.id", + ], + allowedRelations: [ + "metadata", + "metadata.parent", + "metadata.children", + "metadata.product", + ], + isList: true, + } + + middleware = transformQuery(extendedFindParamsMixin(), queryConfig) + + await middleware(mockRequest, mockResponse, nextFunction) + + expect(nextFunction).toHaveBeenCalledWith( + new MedusaError( + MedusaError.Types.INVALID_DATA, + `Requested fields [product] are not valid` + ) + ) + + ////////////////////////////// + + mockRequest = { + query: { + fields: "*product", + }, + } as unknown as Request + + queryConfig = { + defaults: [ + "id", + "created_at", + "updated_at", + "deleted_at", + "metadata.id", + "metadata.parent.id", + "metadata.children.id", + "metadata.product.id", + ], + allowed: [ + "id", + "created_at", + "updated_at", + "deleted_at", + "metadata.id", + "metadata.parent.id", + "metadata.children.id", + "metadata.product.id", + ], + isList: true, + } + + middleware = transformQuery(extendedFindParamsMixin(), queryConfig) + + await middleware(mockRequest, mockResponse, nextFunction) + + expect(nextFunction).toHaveBeenCalledWith( + new MedusaError( + MedusaError.Types.INVALID_DATA, + `Requested fields [product] are not valid` + ) + ) + + ////////////////////////////// + + mockRequest = { + query: { + fields: "*product.variants", + }, + } as unknown as Request + + queryConfig = { + defaults: [ + "id", + "created_at", + "updated_at", + "deleted_at", + "metadata.id", + "metadata.parent.id", + "metadata.children.id", + "metadata.product.id", + ], + allowed: [ + "id", + "created_at", + "updated_at", + "deleted_at", + "metadata.id", + "metadata.parent.id", + "metadata.children.id", + "metadata.product.id", + "product", + ], + isList: true, + } + + middleware = transformQuery(extendedFindParamsMixin(), queryConfig) + + await middleware(mockRequest, mockResponse, nextFunction) + + expect(nextFunction).toHaveBeenCalledWith( + new MedusaError( + MedusaError.Types.INVALID_DATA, + `Requested fields [product.variants] are not valid` + ) + ) + + ////////////////////////////// + + mockRequest = { + query: { + fields: "product", + }, + } as unknown as Request + + queryConfig = { + defaults: [ + "id", + "created_at", + "updated_at", + "deleted_at", + "metadata.id", + "metadata.parent.id", + "metadata.children.id", + "metadata.product.id", + ], + allowed: [ + "id", + "created_at", + "updated_at", + "deleted_at", + "metadata.id", + "metadata.parent.id", + "metadata.children.id", + "metadata.product.id", + ], + isList: true, + } + + middleware = transformQuery(extendedFindParamsMixin(), queryConfig) + + await middleware(mockRequest, mockResponse, nextFunction) + + expect(nextFunction).toHaveBeenCalledWith( + new MedusaError( + MedusaError.Types.INVALID_DATA, + `Requested fields [product] are not valid` + ) + ) + }) +}) diff --git a/packages/medusa/src/api/middlewares/transform-query.ts b/packages/medusa/src/api/middlewares/transform-query.ts index cb867cd6c4..b9e5f2df44 100644 --- a/packages/medusa/src/api/middlewares/transform-query.ts +++ b/packages/medusa/src/api/middlewares/transform-query.ts @@ -1,4 +1,3 @@ -import { buildSelects, objectToStringPath } from "@medusajs/utils" import { ValidatorOptions } from "class-validator" import { NextFunction, Request, Response } from "express" import { omit } from "lodash" @@ -24,10 +23,7 @@ export function transformQuery< TEntity extends BaseEntity >( plainToClass: ClassConstructor, - queryConfig?: Omit< - QueryConfig, - "allowedRelations" | "allowedFields" - >, + queryConfig: QueryConfig = {}, config: ValidatorOptions = {} ): (req: Request, res: Response, next: NextFunction) => Promise { return async (req: Request, res: Response, next: NextFunction) => { @@ -40,12 +36,41 @@ export function transformQuery< ) req.validatedQuery = validated req.filterableFields = getFilterableFields(validated) - req.allowedProperties = getAllowedProperties( - validated, - req.includes ?? {}, - queryConfig + + attachListOrRetrieveConfig(req, { + ...queryConfig, + allowed: + req.allowed ?? queryConfig.allowed ?? queryConfig.allowedFields ?? [], + }) + + /** + * TODO: the bellow allowedProperties should probably need to be reworked which would create breaking changes everywhere + * cleanResponseData is used. It is in fact, what is expected to be returned which IMO + * should correspond to the select/relations + * + * Kept it as it is to maintain backward compatibility + */ + const queryConfigRes = !queryConfig.isList + ? req.retrieveConfig + : req.listConfig + const includesRelations = Object.keys(req.includes ?? {}) + req.allowedProperties = Array.from( + new Set( + [ + ...(req.validatedQuery.fields + ? queryConfigRes.select ?? [] + : req.allowed ?? + queryConfig.allowed ?? + queryConfig.allowedFields ?? + (queryConfig.defaults as string[]) ?? + queryConfig.defaultFields ?? + []), + ...(req.validatedQuery.expand || includesRelations.length + ? [...(validated.expand?.split(",") || []), ...includesRelations] // For backward compatibility, the includes takes precedence over the relations for the returnable fields + : queryConfig.allowedRelations ?? queryConfigRes.relations ?? []), // For backward compatibility, the allowedRelations takes precedence over the relations for the returnable fields + ].filter(Boolean) + ) ) - attachListOrRetrieveConfig(req, queryConfig) next() } catch (e) { @@ -59,6 +84,8 @@ export function transformQuery< * @param plainToClass * @param queryConfig * @param config + * + * @deprecated use `transformQuery` instead */ export function transformStoreQuery< T extends RequestQueryFields, @@ -68,28 +95,7 @@ export function transformStoreQuery< queryConfig?: QueryConfig, config: ValidatorOptions = {} ): (req: Request, res: Response, next: NextFunction) => Promise { - return async (req: Request, res: Response, next: NextFunction) => { - try { - normalizeQuery()(req, res, () => void 0) - const validated: T = await validator>( - plainToClass, - req.query, - config - ) - req.validatedQuery = validated - req.filterableFields = getFilterableFields(validated) - req.allowedProperties = getStoreAllowedProperties( - validated, - req.includes ?? {}, - queryConfig - ) - attachListOrRetrieveConfig(req, queryConfig) - - next() - } catch (e) { - next(e) - } - } + return transformQuery(plainToClass, queryConfig, config) } /** @@ -100,6 +106,9 @@ function getFilterableFields(obj: T): T { const result = omit(obj, [ "limit", "offset", + /** + * @deprecated + */ "expand", "fields", "order", @@ -108,80 +117,22 @@ function getFilterableFields(obj: T): T { } /** - * build and attach the `retrieveConfig` or `listConfig` + * build and attach the `retrieveConfig` or `listConfig` and remoteQueryConfig to the request object * @param req * @param queryConfig */ function attachListOrRetrieveConfig( req: Request, - queryConfig?: QueryConfig + queryConfig: QueryConfig = {} ) { const validated = req.validatedQuery - if (queryConfig?.isList) { - req.listConfig = prepareListQuery( - validated, - queryConfig - ) as FindConfig - } else { - req.retrieveConfig = prepareRetrieveQuery( - validated, - queryConfig - ) as FindConfig - } -} -/** - * Build the store allowed props based on the custom fields query params, the allowed props config and the includes options. - * This can be used later with `cleanResponseData` in order to clean up the returned objects to the client. - * @param queryConfig - * @param validated - * @param includesOptions - */ -function getStoreAllowedProperties( - validated: RequestQueryFields, - includesOptions: Record, - queryConfig?: QueryConfig -): string[] { - const allowed: string[] = [] - - const includeKeys = Object.keys(includesOptions) - const fields = validated.fields - ? validated.fields?.split(",") - : queryConfig?.allowedFields || [] - const expand = - validated.expand || includeKeys.length - ? [...(validated.expand?.split(",") || []), ...includeKeys] - : queryConfig?.allowedRelations || [] - - allowed.push(...fields, ...objectToStringPath(buildSelects(expand))) - - return allowed -} - -/** - * Build the admin allowed props based on the custom fields query params, the defaults and the includes options. - * Since admin can access everything, it is only in order to return what the user asked for through fields and expand query params. - * This can be used later with `cleanResponseData` in order to clean up the returned objects to the client. - * @param queryConfig - * @param validated - * @param includesOptions - */ -function getAllowedProperties( - validated: RequestQueryFields, - includesOptions: Record, - queryConfig?: QueryConfig -): string[] { - const allowed: (string | keyof TEntity)[] = [] - - const includeKeys = Object.keys(includesOptions) - const fields = validated.fields - ? validated.fields?.split(",") - : queryConfig?.defaultFields || [] - const expand = - validated.expand || includeKeys.length - ? [...(validated.expand?.split(",") || []), ...includeKeys] - : queryConfig?.defaultRelations || [] - - allowed.push(...fields, ...objectToStringPath(buildSelects(expand))) - - return allowed as string[] + const config = queryConfig.isList + ? prepareListQuery(validated, queryConfig) + : prepareRetrieveQuery(validated, queryConfig) + + req.listConfig = ("listConfig" in config && + config.listConfig) as FindConfig + req.retrieveConfig = ("retrieveConfig" in config && + config.retrieveConfig) as FindConfig + req.remoteQueryConfig = config.remoteQueryConfig } diff --git a/packages/medusa/src/api/routes/admin/orders/__tests__/get-order.js b/packages/medusa/src/api/routes/admin/orders/__tests__/get-order.js index e200f3ef6c..86ca3f7422 100644 --- a/packages/medusa/src/api/routes/admin/orders/__tests__/get-order.js +++ b/packages/medusa/src/api/routes/admin/orders/__tests__/get-order.js @@ -30,6 +30,15 @@ describe("GET /admin/orders", () => { it("calls orderService retrieve", () => { expect(OrderServiceMock.retrieveWithTotals).toHaveBeenCalledTimes(1) + + const expectedRelations = [ + ...defaultAdminOrdersRelations, + "sales_channel", + ] + + expect( + OrderServiceMock.retrieveWithTotals.mock.calls[0][1].relations + ).toHaveLength(expectedRelations.length) expect(OrderServiceMock.retrieveWithTotals).toHaveBeenCalledWith( IdMap.getId("test-order"), { @@ -50,7 +59,7 @@ describe("GET /admin/orders", () => { } ), // TODO [MEDUSA_FF_SALES_CHANNELS]: Remove when sales channel flag is removed entirely - relations: [...defaultAdminOrdersRelations, "sales_channel"].sort(), + relations: expect.arrayContaining(expectedRelations), }, { includes: undefined, diff --git a/packages/medusa/src/api/routes/admin/products/__tests__/get-product.js b/packages/medusa/src/api/routes/admin/products/__tests__/get-product.js index 4a58d2f808..834ffda4d2 100644 --- a/packages/medusa/src/api/routes/admin/products/__tests__/get-product.js +++ b/packages/medusa/src/api/routes/admin/products/__tests__/get-product.js @@ -26,6 +26,9 @@ describe("GET /admin/products/:id", () => { it("calls get product from productService", () => { expect(ProductServiceMock.retrieve).toHaveBeenCalledTimes(1) + expect( + ProductServiceMock.retrieve.mock.calls[0][1].relations + ).toHaveLength(11) expect(ProductServiceMock.retrieve).toHaveBeenCalledWith( IdMap.getId("product1"), { @@ -55,7 +58,7 @@ describe("GET /admin/products/:id", () => { "deleted_at", "metadata", ], - relations: [ + relations: expect.arrayContaining([ "collection", "images", "options", @@ -67,7 +70,7 @@ describe("GET /admin/products/:id", () => { "variants", "variants.options", "variants.prices", - ], + ]), } ) }) diff --git a/packages/medusa/src/api/routes/admin/regions/__tests__/get-region.js b/packages/medusa/src/api/routes/admin/regions/__tests__/get-region.js index a5e14fceb4..072434e933 100644 --- a/packages/medusa/src/api/routes/admin/regions/__tests__/get-region.js +++ b/packages/medusa/src/api/routes/admin/regions/__tests__/get-region.js @@ -45,11 +45,14 @@ describe("GET /admin/regions/:region_id", () => { it("calls service addCountry", () => { expect(RegionServiceMock.retrieve).toHaveBeenCalledTimes(1) + expect( + RegionServiceMock.retrieve.mock.calls[0][1].relations + ).toHaveLength(defaultRelations.length) expect(RegionServiceMock.retrieve).toHaveBeenCalledWith( IdMap.getId("testRegion"), { select: defaultFields, - relations: defaultRelations, + relations: expect.arrayContaining(defaultRelations), } ) }) diff --git a/packages/medusa/src/api/routes/admin/regions/__tests__/list-regions.js b/packages/medusa/src/api/routes/admin/regions/__tests__/list-regions.js index 1d4e7d7aa7..d3591fe21b 100644 --- a/packages/medusa/src/api/routes/admin/regions/__tests__/list-regions.js +++ b/packages/medusa/src/api/routes/admin/regions/__tests__/list-regions.js @@ -48,11 +48,14 @@ describe("GET /admin/regions", () => { it("calls service list", () => { expect(RegionServiceMock.listAndCount).toHaveBeenCalledTimes(1) + expect( + RegionServiceMock.listAndCount.mock.calls[0][1].relations + ).toHaveLength(defaultRelations.length) expect(RegionServiceMock.listAndCount).toHaveBeenCalledWith( {}, { select: defaultFields, - relations: defaultRelations, + relations: expect.arrayContaining(defaultRelations), take: 50, skip: 0, order: { created_at: "DESC" }, @@ -84,11 +87,14 @@ describe("GET /admin/regions", () => { it("calls service list", () => { expect(RegionServiceMock.listAndCount).toHaveBeenCalledTimes(1) + expect( + RegionServiceMock.listAndCount.mock.calls[0][1].relations + ).toHaveLength(defaultRelations.length) expect(RegionServiceMock.listAndCount).toHaveBeenCalledWith( {}, { select: defaultFields, - relations: defaultRelations, + relations: expect.arrayContaining(defaultRelations), take: 20, skip: 10, order: { created_at: "DESC" }, diff --git a/packages/medusa/src/api/routes/store/carts/get-cart.ts b/packages/medusa/src/api/routes/store/carts/get-cart.ts index 885612b4c9..193c3c2a59 100644 --- a/packages/medusa/src/api/routes/store/carts/get-cart.ts +++ b/packages/medusa/src/api/routes/store/carts/get-cart.ts @@ -112,7 +112,7 @@ export default async (req, res) => { rel.includes("variant") ) - const select = [...req.retrieveConfig.select] + const select = [...(req.retrieveConfig.select ?? [])] const salesChannelsEnabled = featureFlagRouter.isFeatureEnabled( SalesChannelFeatureFlag.key ) diff --git a/packages/medusa/src/api/routes/store/regions/__tests__/get-region.js b/packages/medusa/src/api/routes/store/regions/__tests__/get-region.js index cc13e0948e..f314d0a10c 100644 --- a/packages/medusa/src/api/routes/store/regions/__tests__/get-region.js +++ b/packages/medusa/src/api/routes/store/regions/__tests__/get-region.js @@ -23,9 +23,9 @@ describe("Get region by id", () => { { relations: [ "countries", - "currency", - "fulfillment_providers", "payment_providers", + "fulfillment_providers", + "currency", ], select: [ "id", diff --git a/packages/medusa/src/api/routes/store/regions/__tests__/list-regions.js b/packages/medusa/src/api/routes/store/regions/__tests__/list-regions.js index 5fba8918ab..e0217eabbc 100644 --- a/packages/medusa/src/api/routes/store/regions/__tests__/list-regions.js +++ b/packages/medusa/src/api/routes/store/regions/__tests__/list-regions.js @@ -15,15 +15,18 @@ describe("List regions", () => { it("calls list from region service", () => { expect(RegionServiceMock.listAndCount).toHaveBeenCalledTimes(1) + expect( + RegionServiceMock.listAndCount.mock.calls[0][1].relations + ).toHaveLength(4) expect(RegionServiceMock.listAndCount).toHaveBeenCalledWith( {}, { - relations: [ + relations: expect.arrayContaining([ "countries", "currency", "fulfillment_providers", "payment_providers", - ], + ]), select: [ "id", "name", diff --git a/packages/medusa/src/strategies/batch-jobs/order/export.ts b/packages/medusa/src/strategies/batch-jobs/order/export.ts index cd94244811..809958f163 100644 --- a/packages/medusa/src/strategies/batch-jobs/order/export.ts +++ b/packages/medusa/src/strategies/batch-jobs/order/export.ts @@ -1,10 +1,10 @@ import { FlagRouter } from "@medusajs/utils" import { EntityManager } from "typeorm" import { - OrderDescriptor, - OrderExportBatchJob, - OrderExportBatchJobContext, - orderExportPropertiesDescriptors, + OrderDescriptor, + OrderExportBatchJob, + OrderExportBatchJobContext, + orderExportPropertiesDescriptors, } from "." import { AdminPostBatchesReq } from "../../../api" import { AbstractBatchJobStrategy, IFileService } from "../../../interfaces" @@ -101,7 +101,7 @@ class OrderExportStrategy extends AbstractBatchJobStrategy { ...context } = batchJob.context as OrderExportBatchJobContext - const listConfig = prepareListQuery( + const { listConfig } = prepareListQuery( { limit, offset, diff --git a/packages/medusa/src/strategies/batch-jobs/product/export.ts b/packages/medusa/src/strategies/batch-jobs/product/export.ts index dae28f8746..a0357e2a10 100644 --- a/packages/medusa/src/strategies/batch-jobs/product/export.ts +++ b/packages/medusa/src/strategies/batch-jobs/product/export.ts @@ -1,5 +1,5 @@ import { MedusaContainer } from "@medusajs/types" -import { FlagRouter, MedusaV2Flag, createContainerLike } from "@medusajs/utils" +import { createContainerLike, FlagRouter, MedusaV2Flag } from "@medusajs/utils" import { humanizeAmount } from "medusa-core-utils" import { EntityManager } from "typeorm" import { defaultAdminProductRelations } from "../../../api" @@ -114,7 +114,7 @@ export default class ProductExportStrategy extends AbstractBatchJobStrategy { ...context } = (batchJob?.context ?? {}) as ProductExportBatchJobContext - const listConfig = prepareListQuery( + const { listConfig } = prepareListQuery( { limit, offset, diff --git a/packages/medusa/src/types/common.ts b/packages/medusa/src/types/common.ts index 77e25bbb80..a6f859311c 100644 --- a/packages/medusa/src/types/common.ts +++ b/packages/medusa/src/types/common.ts @@ -104,9 +104,29 @@ export interface CustomFindOptions { } export type QueryConfig = { + /** + * Default fields and relations to return + */ + defaults?: (keyof TEntity | string)[] + /** + * @deprecated Use `defaults` instead + */ defaultFields?: (keyof TEntity | string)[] + /** + * @deprecated Use `defaultFields` instead and the relations will be inferred + */ defaultRelations?: string[] + /** + * Fields and relations that are allowed to be requested + */ + allowed?: string[] + /** + * @deprecated Use `allowed` instead + */ allowedFields?: string[] + /** + * @deprecated Use `allowedFields` instead and the relations will be inferred + */ allowedRelations?: string[] defaultLimit?: number isList?: boolean @@ -120,10 +140,13 @@ export type QueryConfig = { export type RequestQueryFields = { /** * Comma-separated relations that should be expanded in the returned data. + * @deprecated Use `fields` instead and the relations will be inferred */ expand?: string /** * Comma-separated fields that should be included in the returned data. + * if a field is prefixed with `+` it will be added to the default fields, using `-` will remove it from the default fields. + * without prefix it will replace the entire default fields. */ fields?: string /** @@ -511,6 +534,7 @@ export class AddressCreatePayload { export class FindParams { /** * {@inheritDoc RequestQueryFields.expand} + * @deprecated */ @IsString() @IsOptional() diff --git a/packages/medusa/src/types/global.ts b/packages/medusa/src/types/global.ts index dad58aa74a..26f7f98b85 100644 --- a/packages/medusa/src/types/global.ts +++ b/packages/medusa/src/types/global.ts @@ -12,11 +12,46 @@ declare global { scope: MedusaContainer validatedQuery: RequestQueryFields & Record validatedBody: unknown - listConfig: FindConfig - retrieveConfig: FindConfig - filterableFields: Record + /** + * TODO: shouldn't this correspond to returnable fields instead of allowed fields? also it is used by the cleanResponseData util + */ allowedProperties: string[] + /** + * An object containing the select, relation, skip, take and order to be used with medusa internal services + */ + listConfig: FindConfig + /** + * An object containing the select, relation to be used with medusa internal services + */ + retrieveConfig: FindConfig + /** + * An object containing fields and variables to be used with the remoteQuery + */ + remoteQueryConfig: { + fields: string[] + pagination: { + order?: Record + skip?: number + take?: number + } + } + /** + * An object containing the fields that are filterable e.g `{ id: Any }` + */ + filterableFields: Record includes?: Record + /** + * An array of fields and relations that are allowed to be queried, this can be set by the + * consumer as part of a middleware and it will take precedence over the defaultAllowedFields + * @deprecated use `allowed` instead + */ + allowedFields?: string[] + /** + * An array of fields and relations that are allowed to be queried, this can be set by the + * consumer as part of a middleware and it will take precedence over the defaultAllowedFields set + * by the api + */ + allowed?: string[] errors: string[] requestId?: string } diff --git a/packages/medusa/src/types/routing.ts b/packages/medusa/src/types/routing.ts index c58290918f..2a130056db 100644 --- a/packages/medusa/src/types/routing.ts +++ b/packages/medusa/src/types/routing.ts @@ -1,14 +1,45 @@ import type { Customer, User } from "../models" import type { NextFunction, Request, Response } from "express" -import { MedusaContainer } from "@medusajs/types" -import { RequestQueryFields } from "@medusajs/types" +import { MedusaContainer, RequestQueryFields } from "@medusajs/types" +import { FindConfig } from "./common" export interface MedusaRequest extends Request { validatedBody: Body validatedQuery: RequestQueryFields & Record + /** + * TODO: shouldn't this correspond to returnable fields instead of allowed fields? also it is used by the cleanResponseData util + */ allowedProperties: string[] + /** + * An object containing the select, relation, skip, take and order to be used with medusa internal services + */ + listConfig: FindConfig + /** + * An object containing the select, relation to be used with medusa internal services + */ + retrieveConfig: FindConfig + /** + * An object containing fields and variables to be used with the remoteQuery + */ + remoteQueryConfig: { fields: string[]; pagination: { order?: Record, skip?: number, take?: number } } + /** + * An object containing the fields that are filterable e.g `{ id: Any }` + */ + filterableFields: Record includes?: Record + /** + * An array of fields and relations that are allowed to be queried, this can be set by the + * consumer as part of a middleware and it will take precedence over the defaultAllowedFields + * @deprecated use `allowed` instead + */ + allowedFields?: string[] + /** + * An array of fields and relations that are allowed to be queried, this can be set by the + * consumer as part of a middleware and it will take precedence over the defaultAllowedFields set + * by the api + */ + allowed?: string[] errors: string[] scope: MedusaContainer session?: any diff --git a/packages/medusa/src/utils/get-query-config.ts b/packages/medusa/src/utils/get-query-config.ts index 7abb232777..4f1e298f37 100644 --- a/packages/medusa/src/utils/get-query-config.ts +++ b/packages/medusa/src/utils/get-query-config.ts @@ -2,6 +2,7 @@ import { pick } from "lodash" import { FindConfig, QueryConfig, RequestQueryFields } from "../types/common" import { isDefined, MedusaError } from "medusa-core-utils" import { BaseEntity } from "../interfaces" +import { getSetDifference, stringToSelectRelationObject } from "@medusajs/utils" export function pickByConfig( obj: TModel | TModel[], @@ -19,93 +20,146 @@ export function pickByConfig( return obj } -export function getRetrieveConfig( - defaultFields: (keyof TModel)[], - defaultRelations: string[], - fields?: (keyof TModel)[], - expand?: string[] -): FindConfig { - let includeFields: (keyof TModel)[] = [] - if (isDefined(fields)) { - includeFields = Array.from(new Set([...fields, "id"])).map((field) => { - return typeof field === "string" ? field.trim() : field - }) as (keyof TModel)[] - } - - let expandFields: string[] = [] - if (isDefined(expand)) { - expandFields = expand.map((expandRelation) => expandRelation.trim()) - } - - return { - select: includeFields.length ? includeFields : defaultFields, - relations: isDefined(expand) ? expandFields : defaultRelations, - } -} - -export function getListConfig( - defaultFields: (keyof TModel)[], - defaultRelations: string[], - fields?: (keyof TModel)[], - expand?: string[], - limit = 50, - offset = 0, - order: { [k: string | symbol]: "DESC" | "ASC" } = {} -): FindConfig { - let includeFields: (keyof TModel)[] = [] - if (isDefined(fields)) { - const fieldSet = new Set(fields) - // Ensure created_at is included, since we are sorting on this - fieldSet.add("created_at") - fieldSet.add("id") - includeFields = Array.from(fieldSet) as (keyof TModel)[] - } - - let expandFields: string[] = [] - if (isDefined(expand)) { - expandFields = expand - } - - const orderBy = order - - if (!Object.keys(order).length) { - orderBy["created_at"] = "DESC" - } - - return { - select: includeFields.length ? includeFields : defaultFields, - relations: isDefined(expand) ? expandFields : defaultRelations, - skip: offset, - take: limit, - order: orderBy, - } -} - export function prepareListQuery< T extends RequestQueryFields, TEntity extends BaseEntity ->(validated: T, queryConfig?: QueryConfig) { - const { order, fields, expand, limit, offset } = validated +>(validated: T, queryConfig: QueryConfig = {}) { + const { order, fields, limit = 50, expand, offset = 0 } = validated + let { + allowed = [], + defaults = [], + defaultFields = [], + defaultLimit, + allowedFields = [], + allowedRelations = [], + defaultRelations = [], + isList, + } = queryConfig - let expandRelations: string[] | undefined = undefined - if (isDefined(expand)) { - expandRelations = expand.split(",").filter((v) => v) - } + allowedFields = allowed.length ? allowed : allowedFields + defaultFields = defaults.length ? defaults : defaultFields + + // e.g *product.variants meaning that we want all fields from the product.variants + // in that case it wont be part of the select but it will be part of the relations. + // For the remote query we will have to add the fields to the fields array as product.variants.* + const starFields: Set = new Set() + + let allFields = new Set(defaultFields) as Set - let expandFields: (keyof TEntity)[] | undefined = undefined if (isDefined(fields)) { - expandFields = (fields.split(",") as (keyof TEntity)[]).filter((v) => v) + const customFields = fields.split(",").filter(Boolean) + const shouldReplaceDefaultFields = + !customFields.length || + customFields.some((field) => { + return !( + field.startsWith("-") || + field.startsWith("+") || + field.startsWith("*") + ) + }) + if (shouldReplaceDefaultFields) { + allFields = new Set(customFields.map((f) => f.replace(/^[+-]/, ""))) + } else { + customFields.forEach((field) => { + if (field.startsWith("+")) { + allFields.add(field.replace(/^\+/, "")) + } else if (field.startsWith("-")) { + allFields.delete(field.replace(/^-/, "")) + } else { + allFields.add(field) + } + }) + } + + // TODO: Maintain backward compatibility, remove in future. the created at was only added in the list query for default order + if (queryConfig.isList) { + allFields.add("created_at") + } + allFields.add("id") } - if (expandFields?.length && queryConfig?.allowedFields?.length) { - validateFields(expandFields as string[], queryConfig.allowedFields) + allFields.forEach((field) => { + if (field.startsWith("*")) { + starFields.add(field.replace(/^\*/, "")) + allFields.delete(field) + } + }) + + const allAllowedFields = new Set(allowedFields) // In case there is no allowedFields, allow all fields + const notAllowedFields: string[] = [] + + if (allowedFields.length) { + ;[...allFields, ...Array.from(starFields)].forEach((field) => { + const hasAllowedField = allowedFields.includes(field) + + if (hasAllowedField) { + return + } + + // Select full relation in that case it must match an allowed field fully + // e.g product.variants in that case we must have a product.variants in the allowedFields + if (starFields.has(field)) { + if (hasAllowedField) { + return + } + notAllowedFields.push(field) + return + } + + const fieldStartsWithAllowedField = allowedFields.some((allowedField) => + field.startsWith(allowedField) + ) + + if (!fieldStartsWithAllowedField) { + notAllowedFields.push(field) + return + } + }) } - if (expandRelations?.length && queryConfig?.allowedRelations?.length) { - validateRelations(expandRelations, queryConfig.allowedRelations) + if (allFields.size && notAllowedFields.length) { + throw new MedusaError( + MedusaError.Types.INVALID_DATA, + `Requested fields [${Array.from(notAllowedFields).join( + ", " + )}] are not valid` + ) } - let orderBy: { [k: symbol]: "DESC" | "ASC" } | undefined + const { select, relations } = stringToSelectRelationObject( + Array.from(allFields) + ) + + // TODO: maintain backward compatibility, remove in the future + let allRelations = new Set([ + ...relations, + ...defaultRelations, + ...Array.from(starFields), + ]) + + if (isDefined(expand)) { + allRelations = new Set(expand.split(",").filter(Boolean)) + } + + const allAllowedRelations = new Set([ + ...Array.from(allAllowedFields), + ...allowedRelations, + ]) + const notAllowedRelations = !allowedRelations.length + ? new Set() + : getSetDifference(allRelations, allAllowedRelations) + + if (allRelations.size && notAllowedRelations.size) { + throw new MedusaError( + MedusaError.Types.INVALID_DATA, + `Requested fields [${Array.from(notAllowedRelations).join( + ", " + )}] are not valid` + ) + } + // End of expand compatibility + + let orderBy: { [k: symbol]: "DESC" | "ASC" } | undefined = {} if (isDefined(order)) { let orderField = order if (order.startsWith("-")) { @@ -125,82 +179,52 @@ export function prepareListQuery< `Order field ${orderField} is not valid` ) } + } else { + orderBy["created_at"] = "DESC" } - return getListConfig( - queryConfig?.defaultFields as (keyof TEntity)[], - (queryConfig?.defaultRelations ?? []) as string[], - expandFields, - expandRelations, - limit ?? queryConfig?.defaultLimit, - offset ?? 0, - orderBy - ) + return { + listConfig: { + select: select.length ? select : undefined, + relations: Array.from(allRelations), + skip: offset, + take: limit ?? defaultLimit, + order: orderBy, + }, + remoteQueryConfig: { + // Add starFields that are relations only on which we want all properties with a dedicated format to the remote query + fields: [ + ...Array.from(allFields), + ...Array.from(starFields).map((f) => `${f}.*`), + ], + pagination: isList + ? { + skip: offset, + take: limit ?? defaultLimit, + order: orderBy, + } + : {}, + }, + } } export function prepareRetrieveQuery< T extends RequestQueryFields, TEntity extends BaseEntity >(validated: T, queryConfig?: QueryConfig) { - const { fields, expand } = validated - - let expandRelations: string[] | undefined = undefined - if (isDefined(expand)) { - expandRelations = expand.split(",").filter((v) => v) - } - - let expandFields: (keyof TEntity)[] | undefined = undefined - if (isDefined(fields)) { - expandFields = (fields.split(",") as (keyof TEntity)[]).filter((v) => v) - } - - if (expandFields?.length && queryConfig?.allowedFields?.length) { - validateFields(expandFields as string[], queryConfig.allowedFields) - } - - if (expandRelations?.length && queryConfig?.allowedRelations?.length) { - validateRelations(expandRelations, queryConfig.allowedRelations) - } - - return getRetrieveConfig( - queryConfig?.defaultFields as (keyof TEntity)[], - (queryConfig?.defaultRelations ?? []) as string[], - expandFields, - expandRelations + const { listConfig, remoteQueryConfig } = prepareListQuery( + validated, + queryConfig ) -} -function validateRelations( - relations: string[], - allowed: string[] -): void | never { - const disallowedRelationsFound: string[] = [] - relations?.forEach((field) => { - if (!allowed.includes(field as string)) { - disallowedRelationsFound.push(field) - } - }) - - if (disallowedRelationsFound.length) { - throw new MedusaError( - MedusaError.Types.INVALID_DATA, - `Relations [${disallowedRelationsFound.join(", ")}] are not valid` - ) - } -} - -function validateFields(fields: string[], allowed: string[]): void | never { - const disallowedFieldsFound: string[] = [] - fields?.forEach((field) => { - if (!allowed.includes(field as string)) { - disallowedFieldsFound.push(field) - } - }) - - if (disallowedFieldsFound.length) { - throw new MedusaError( - MedusaError.Types.INVALID_DATA, - `Fields [${disallowedFieldsFound.join(", ")}] are not valid` - ) + return { + retrieveConfig: { + select: listConfig.select, + relations: listConfig.relations, + }, + remoteQueryConfig: { + fields: remoteQueryConfig.fields, + pagination: {}, + }, } } diff --git a/packages/modules-sdk/src/remote-query.ts b/packages/modules-sdk/src/remote-query.ts index 123149f7ec..e5b6258886 100644 --- a/packages/modules-sdk/src/remote-query.ts +++ b/packages/modules-sdk/src/remote-query.ts @@ -90,6 +90,10 @@ export class RemoteQuery { let relations: string[] = [] data.fields?.forEach((field: string) => { + if (field === "*") { + // Select all, so we don't specify any field and rely on relation only + return + } fields.add(prefix ? `${prefix}.${field}` : field) }) args[prefix] = data.args diff --git a/packages/orchestration/src/__tests__/joiner/remote-joiner.ts b/packages/orchestration/src/__tests__/joiner/remote-joiner.ts index 93acc27667..4e78bb927d 100644 --- a/packages/orchestration/src/__tests__/joiner/remote-joiner.ts +++ b/packages/orchestration/src/__tests__/joiner/remote-joiner.ts @@ -192,6 +192,151 @@ describe("RemoteJoiner", () => { ) }) + it("should filter the fields and attach the values correctly taking into account the * fields selection", () => { + const data = { + id: "prod_01H1PN579TJ707BRK938E2ME2N", + title: "7468915", + handle: "7468915", + subtitle: null, + description: null, + collection_id: null, + collection: null, + type_id: "ptyp_01GX66TMARS55DBNYE31DDT8ZV", + type: { + id: "ptyp_01GX66TMARS55DBNYE31DDT8ZV", + value: "test-type-1", + }, + options: [ + { + id: "opt_01H1PN57AQE8G3FK365EYNH917", + title: "4108194", + product_id: "prod_01H1PN579TJ707BRK938E2ME2N", + product: "prod_01H1PN579TJ707BRK938E2ME2N", + values: [ + { + id: "optval_01H1PN57EAMXYFRGSJJJE9P0TJ", + value: "4108194", + option_id: "opt_01H1PN57AQE8G3FK365EYNH917", + option: "opt_01H1PN57AQE8G3FK365EYNH917", + variant_id: "variant_01H1PN57E99TMZAGNEZBSS3FM3", + variant: "variant_01H1PN57E99TMZAGNEZBSS3FM3", + }, + ], + }, + ], + variants: [ + { + id: "variant_01H1PN57E99TMZAGNEZBSS3FM3", + product_id: "prod_01H1PN579TJ707BRK938E2ME2N", + product: "prod_01H1PN579TJ707BRK938E2ME2N", + options: [ + { + id: "optval_01H1PN57EAMXYFRGSJJJE9P0TJ", + value: "4108194", + option_id: "opt_01H1PN57AQE8G3FK365EYNH917", + option: "opt_01H1PN57AQE8G3FK365EYNH917", + variant_id: "variant_01H1PN57E99TMZAGNEZBSS3FM3", + variant: "variant_01H1PN57E99TMZAGNEZBSS3FM3", + }, + ], + }, + ], + tags: [], + images: [], + } + + const fields = [ + "id", + "title", + "subtitle", + "description", + "handle", + "images", + "tags", + "type", + "collection", + "options", + "variants_id", + ] + + const expands = { + collection: { + fields: ["id", "title", "handle"], + }, + images: { + fields: ["url"], + }, + options: { + fields: ["title", "values"], + expands: { + values: { + fields: ["id", "value"], + }, + }, + }, + tags: { + fields: ["value"], + }, + type: { + fields: ["value"], + }, + variants: { + fields: ["*"], + expands: { + options: { + fields: ["id", "value"], + }, + }, + }, + } + + const filteredFields = (RemoteJoiner as any).filterFields( + data, + fields, + expands + ) + + expect(filteredFields).toEqual( + expect.objectContaining({ + id: "prod_01H1PN579TJ707BRK938E2ME2N", + title: "7468915", + subtitle: null, + description: null, + handle: "7468915", + images: [], + tags: [], + type: { + value: "test-type-1", + }, + collection: null, + options: [ + { + title: "4108194", + values: [ + { + id: "optval_01H1PN57EAMXYFRGSJJJE9P0TJ", + value: "4108194", + }, + ], + }, + ], + variants: [ + { + id: "variant_01H1PN57E99TMZAGNEZBSS3FM3", + product_id: "prod_01H1PN579TJ707BRK938E2ME2N", + product: "prod_01H1PN579TJ707BRK938E2ME2N", + options: [ + { + id: "optval_01H1PN57EAMXYFRGSJJJE9P0TJ", + value: "4108194", + }, + ], + }, + ], + }) + ) + }) + it("Simple query of a service, its id and no fields specified", async () => { const query = { service: "user", diff --git a/packages/orchestration/src/joiner/remote-joiner.ts b/packages/orchestration/src/joiner/remote-joiner.ts index 7dd107db12..8eb77a12b0 100644 --- a/packages/orchestration/src/joiner/remote-joiner.ts +++ b/packages/orchestration/src/joiner/remote-joiner.ts @@ -37,20 +37,26 @@ export class RemoteJoiner { data: any, fields: string[], expands?: RemoteNestedExpands - ): Record { + ): Record | undefined { if (!fields || !data) { return data } - const filteredData = fields.reduce((acc: any, field: string) => { - const fieldValue = data?.[field] + let filteredData: Record = {} - if (isDefined(fieldValue)) { - acc[field] = data?.[field] - } + if (fields.includes("*")) { + filteredData = data + } else { + filteredData = fields.reduce((acc: any, field: string) => { + const fieldValue = data?.[field] - return acc - }, {}) + if (isDefined(fieldValue)) { + acc[field] = data?.[field] + } + + return acc + }, {}) + } if (expands) { for (const key of Object.keys(expands ?? {})) { diff --git a/packages/utils/src/dal/mikro-orm/mikro-orm-create-connection.ts b/packages/utils/src/dal/mikro-orm/mikro-orm-create-connection.ts index e8db1bed15..a580bbf301 100644 --- a/packages/utils/src/dal/mikro-orm/mikro-orm-create-connection.ts +++ b/packages/utils/src/dal/mikro-orm/mikro-orm-create-connection.ts @@ -103,6 +103,11 @@ export async function mikroOrmCreateConnection( migrations: { path: pathToMigrations, generator: TSMigrationGenerator, + silent: !( + database.debug ?? + process.env.NODE_ENV?.startsWith("dev") ?? + false + ), }, pool: database.pool as any, })