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, })