From 6b989353acbbb51712e75d819210389aeb0dcf11 Mon Sep 17 00:00:00 2001 From: Adrien de Peretti Date: Tue, 22 Oct 2024 17:16:36 +0200 Subject: [PATCH] fix: API validation management issues (#9693) **What** Currently, the API validation layer is broken in both responsibilities and validation itself. This pr introduce the following fixes and patterns: - Always create a `*Fields` schema that only takes care of defining the schema validation without `effect` - Use the previous point into the API schema validator including `$and` and `$or` capabilities plus the recursive effects - remove `normalizeArray` which does not have to exists since array are already treated as they should - Add recursive transformation to take into account `$and` and `$or` as well or any other similar operators - New util `applyAndAndOrOperators` to wrap the management of those operators and to be merged to an existing schema Tasks - [x] store domain - [ ] admin domain --- .../__tests__/product/store/product.spec.ts | 30 +++++++++ .../src/api/store/collections/validators.ts | 23 ++++--- .../src/api/store/currencies/validators.ts | 17 ++--- .../medusa/src/api/store/orders/validators.ts | 18 ++--- .../store/product-categories/validators.ts | 39 ++++++----- .../src/api/store/products/middlewares.ts | 8 +-- .../src/api/store/products/validators.ts | 67 ++++++++++--------- .../src/api/store/regions/validators.ts | 21 +++--- .../medusa/src/api/store/return/validators.ts | 17 ++--- .../api/store/shipping-options/validators.ts | 17 ++--- .../src/api/utils/common-validators/common.ts | 38 +++++++++++ .../utils/common-validators/products/index.ts | 29 +++----- 12 files changed, 198 insertions(+), 126 deletions(-) diff --git a/integration-tests/http/__tests__/product/store/product.spec.ts b/integration-tests/http/__tests__/product/store/product.spec.ts index f7e50cb3dd..e845aa938e 100644 --- a/integration-tests/http/__tests__/product/store/product.spec.ts +++ b/integration-tests/http/__tests__/product/store/product.spec.ts @@ -8,6 +8,7 @@ import { generateStoreHeaders, } from "../../../../helpers/create-admin-user" import { getProductFixture } from "../../../../helpers/fixtures" +import qs from "qs" jest.setTimeout(30000) @@ -639,6 +640,35 @@ medusaIntegrationTestRunner({ ]) }) + it("should list all products for a category using $and filters", async () => { + const category = await createCategory( + { name: "test", is_internal: false, is_active: true }, + [product.id] + ) + + const category2 = await createCategory( + { name: "test2", is_internal: true, is_active: true }, + [product4.id] + ) + + const searchParam = qs.stringify({ + $and: [{ category_id: [category.id, category2.id] }], + }) + + const response = await api.get( + `/store/products?${searchParam}`, + storeHeaders + ) + + expect(response.status).toEqual(200) + expect(response.data.count).toEqual(1) + expect(response.data.products).toEqual([ + expect.objectContaining({ + id: product.id, + }), + ]) + }) + it("returns a list of ordered products by id ASC", async () => { const response = await api.get("/store/products?order=id", storeHeaders) expect(response.status).toEqual(200) diff --git a/packages/medusa/src/api/store/collections/validators.ts b/packages/medusa/src/api/store/collections/validators.ts index c8a1ae9e9b..625f43d6ef 100644 --- a/packages/medusa/src/api/store/collections/validators.ts +++ b/packages/medusa/src/api/store/collections/validators.ts @@ -4,9 +4,18 @@ import { createOperatorMap, createSelectParams, } from "../../utils/validators" +import { applyAndAndOrOperators } from "../../utils/common-validators" export const StoreGetCollectionParams = createSelectParams() +export const StoreGetCollectionsParamsFields = z.object({ + q: z.string().optional(), + title: z.union([z.string(), z.array(z.string())]).optional(), + handle: z.union([z.string(), z.array(z.string())]).optional(), + created_at: createOperatorMap().optional(), + updated_at: createOperatorMap().optional(), +}) + export type StoreGetCollectionsParamsType = z.infer< typeof StoreGetCollectionsParams > @@ -14,14 +23,6 @@ export const StoreGetCollectionsParams = createFindParams({ offset: 0, limit: 10, order: "-created_at", -}).merge( - z.object({ - q: z.string().optional(), - title: z.union([z.string(), z.array(z.string())]).optional(), - handle: z.union([z.string(), z.array(z.string())]).optional(), - created_at: createOperatorMap().optional(), - updated_at: createOperatorMap().optional(), - $and: z.lazy(() => StoreGetCollectionsParams.array()).optional(), - $or: z.lazy(() => StoreGetCollectionsParams.array()).optional(), - }) -) +}) + .merge(StoreGetCollectionsParamsFields) + .merge(applyAndAndOrOperators(StoreGetCollectionsParamsFields)) diff --git a/packages/medusa/src/api/store/currencies/validators.ts b/packages/medusa/src/api/store/currencies/validators.ts index 2eba5d2af8..242e5a6689 100644 --- a/packages/medusa/src/api/store/currencies/validators.ts +++ b/packages/medusa/src/api/store/currencies/validators.ts @@ -1,19 +1,20 @@ import { z } from "zod" import { createFindParams, createSelectParams } from "../../utils/validators" +import { applyAndAndOrOperators } from "../../utils/common-validators" export const StoreGetCurrencyParams = createSelectParams() +export const StoreGetCurrenciesParamsFields = z.object({ + q: z.string().optional(), + code: z.union([z.string(), z.array(z.string())]).optional(), +}) + export type StoreGetCurrenciesParamsType = z.infer< typeof StoreGetCurrenciesParams > export const StoreGetCurrenciesParams = createFindParams({ offset: 0, limit: 50, -}).merge( - z.object({ - q: z.string().optional(), - code: z.union([z.string(), z.array(z.string())]).optional(), - $and: z.lazy(() => StoreGetCurrenciesParams.array()).optional(), - $or: z.lazy(() => StoreGetCurrenciesParams.array()).optional(), - }) -) +}) + .merge(StoreGetCurrenciesParamsFields) + .merge(applyAndAndOrOperators(StoreGetCurrenciesParamsFields)) diff --git a/packages/medusa/src/api/store/orders/validators.ts b/packages/medusa/src/api/store/orders/validators.ts index 48d888aab2..a2736efc1b 100644 --- a/packages/medusa/src/api/store/orders/validators.ts +++ b/packages/medusa/src/api/store/orders/validators.ts @@ -1,18 +1,20 @@ import { z } from "zod" import { createFindParams, createSelectParams } from "../../utils/validators" +import { applyAndAndOrOperators } from "../../utils/common-validators" export const StoreGetOrderParams = createSelectParams() export type StoreGetOrderParamsType = z.infer +export const StoreGetOrdersParamsFields = z.object({ + id: z.union([z.string(), z.array(z.string())]).optional(), + status: z.union([z.string(), z.array(z.string())]).optional(), +}) + export const StoreGetOrdersParams = createFindParams({ offset: 0, limit: 50, -}).merge( - z.object({ - id: z.union([z.string(), z.array(z.string())]).optional(), - status: z.union([z.string(), z.array(z.string())]).optional(), - $and: z.lazy(() => StoreGetOrdersParams.array()).optional(), - $or: z.lazy(() => StoreGetOrdersParams.array()).optional(), - }) -) +}) + .merge(StoreGetOrdersParamsFields) + .merge(applyAndAndOrOperators(StoreGetOrdersParamsFields)) + export type StoreGetOrdersParamsType = z.infer diff --git a/packages/medusa/src/api/store/product-categories/validators.ts b/packages/medusa/src/api/store/product-categories/validators.ts index e1ca327989..9b850e8181 100644 --- a/packages/medusa/src/api/store/product-categories/validators.ts +++ b/packages/medusa/src/api/store/product-categories/validators.ts @@ -1,5 +1,8 @@ import { z } from "zod" -import { booleanString } from "../../utils/common-validators" +import { + applyAndAndOrOperators, + booleanString, +} from "../../utils/common-validators" import { createFindParams, createOperatorMap, @@ -16,26 +19,26 @@ export const StoreProductCategoryParams = createSelectParams().merge( }) ) +export const StoreProductCategoriesParamsFields = z.object({ + q: z.string().optional(), + id: z.union([z.string(), z.array(z.string())]).optional(), + name: z.union([z.string(), z.array(z.string())]).optional(), + description: z.union([z.string(), z.array(z.string())]).optional(), + handle: z.union([z.string(), z.array(z.string())]).optional(), + parent_category_id: z.union([z.string(), z.array(z.string())]).optional(), + include_ancestors_tree: booleanString().optional(), + include_descendants_tree: booleanString().optional(), + created_at: createOperatorMap().optional(), + updated_at: createOperatorMap().optional(), + deleted_at: createOperatorMap().optional(), +}) + export type StoreProductCategoriesParamsType = z.infer< typeof StoreProductCategoriesParams > export const StoreProductCategoriesParams = createFindParams({ offset: 0, limit: 50, -}).merge( - z.object({ - q: z.string().optional(), - id: z.union([z.string(), z.array(z.string())]).optional(), - name: z.union([z.string(), z.array(z.string())]).optional(), - description: z.union([z.string(), z.array(z.string())]).optional(), - handle: z.union([z.string(), z.array(z.string())]).optional(), - parent_category_id: z.union([z.string(), z.array(z.string())]).optional(), - include_ancestors_tree: booleanString().optional(), - include_descendants_tree: booleanString().optional(), - created_at: createOperatorMap().optional(), - updated_at: createOperatorMap().optional(), - deleted_at: createOperatorMap().optional(), - $and: z.lazy(() => StoreProductCategoriesParams.array()).optional(), - $or: z.lazy(() => StoreProductCategoriesParams.array()).optional(), - }) -) +}) + .merge(StoreProductCategoriesParamsFields) + .merge(applyAndAndOrOperators(StoreProductCategoriesParamsFields)) diff --git a/packages/medusa/src/api/store/products/middlewares.ts b/packages/medusa/src/api/store/products/middlewares.ts index 0244e636f7..344562fd94 100644 --- a/packages/medusa/src/api/store/products/middlewares.ts +++ b/packages/medusa/src/api/store/products/middlewares.ts @@ -16,10 +16,7 @@ import { import { validateAndTransformQuery } from "@medusajs/framework" import { maybeApplyStockLocationId } from "./helpers" import * as QueryConfig from "./query-config" -import { - StoreGetProductsParams, - StoreGetProductsParamsType, -} from "./validators" +import { StoreGetProductsParams } from "./validators" export const storeProductRoutesMiddlewares: MiddlewareRoute[] = [ { @@ -41,7 +38,8 @@ export const storeProductRoutesMiddlewares: MiddlewareRoute[] = [ }), applyDefaultFilters({ status: ProductStatus.PUBLISHED, - categories: (filters: StoreGetProductsParamsType, fields: string[]) => { + // TODO: the type here seems off and the implementation does not take into account $and and $or possible filters. Might be worth re working (original type used here was StoreGetProductsParamsType) + categories: (filters: any, fields: string[]) => { const categoryIds = filters.category_id delete filters.category_id diff --git a/packages/medusa/src/api/store/products/validators.ts b/packages/medusa/src/api/store/products/validators.ts index f5bb825730..3c188dc123 100644 --- a/packages/medusa/src/api/store/products/validators.ts +++ b/packages/medusa/src/api/store/products/validators.ts @@ -1,6 +1,9 @@ import { z } from "zod" import { + applyAndAndOrOperators, GetProductsParams, + recursivelyNormalizeSchema, + StoreGetProductParamsDirectFields, transformProductParams, } from "../../utils/common-validators" import { @@ -9,64 +12,68 @@ import { createSelectParams, } from "../../utils/validators" +export const StoreGetProductParamsFields = z.object({ + region_id: z.string().optional(), + country_code: z.string().optional(), + province: z.string().optional(), + cart_id: z.string().optional(), +}) + export type StoreGetProductParamsType = z.infer export const StoreGetProductParams = createSelectParams().merge( - // These are used to populate the tax and pricing context - z.object({ - region_id: z.string().optional(), - country_code: z.string().optional(), - province: z.string().optional(), - cart_id: z.string().optional(), - }) + StoreGetProductParamsFields ) +export const StoreGetProductVariantsParamsFields = z.object({ + q: z.string().optional(), + id: z.union([z.string(), z.array(z.string())]).optional(), + options: z.object({ value: z.string(), option_id: z.string() }).optional(), + created_at: createOperatorMap().optional(), + updated_at: createOperatorMap().optional(), + deleted_at: createOperatorMap().optional(), +}) + export type StoreGetProductVariantsParamsType = z.infer< typeof StoreGetProductVariantsParams > export const StoreGetProductVariantsParams = createFindParams({ offset: 0, limit: 50, -}).merge( - z.object({ - q: z.string().optional(), - id: z.union([z.string(), z.array(z.string())]).optional(), - options: z.object({ value: z.string(), option_id: z.string() }).optional(), - created_at: createOperatorMap().optional(), - updated_at: createOperatorMap().optional(), - deleted_at: createOperatorMap().optional(), - $and: z.lazy(() => StoreGetProductsParams.array()).optional(), - $or: z.lazy(() => StoreGetProductsParams.array()).optional(), +}) + .merge(StoreGetProductVariantsParamsFields) + .merge(applyAndAndOrOperators(StoreGetProductVariantsParamsFields)) + +export const StoreGetProductsParamsFields = z + .object({ + region_id: z.string().optional(), + country_code: z.string().optional(), + province: z.string().optional(), + cart_id: z.string().optional(), }) -) + .merge(GetProductsParams) + .strict() export type StoreGetProductsParamsType = z.infer export const StoreGetProductsParams = createFindParams({ offset: 0, limit: 50, }) + .merge(StoreGetProductsParamsFields) .merge( z .object({ - // These are used to populate the tax and pricing context - region_id: z.string().optional(), - country_code: z.string().optional(), - province: z.string().optional(), - cart_id: z.string().optional(), - variants: z + .object({ options: z .object({ value: z.string(), option_id: z.string() }) .optional(), - $and: z.lazy(() => StoreGetProductsParams.array()).optional(), - $or: z.lazy(() => StoreGetProductsParams.array()).optional(), }) + .merge(applyAndAndOrOperators(StoreGetProductVariantsParamsFields)) .optional(), - $and: z.lazy(() => StoreGetProductsParams.array()).optional(), - $or: z.lazy(() => StoreGetProductsParams.array()).optional(), }) - .merge(GetProductsParams) + .merge(applyAndAndOrOperators(StoreGetProductParamsDirectFields)) .strict() ) - .transform(transformProductParams) + .transform(recursivelyNormalizeSchema(transformProductParams)) diff --git a/packages/medusa/src/api/store/regions/validators.ts b/packages/medusa/src/api/store/regions/validators.ts index 438dca5dc1..e1f8228c16 100644 --- a/packages/medusa/src/api/store/regions/validators.ts +++ b/packages/medusa/src/api/store/regions/validators.ts @@ -1,20 +1,21 @@ import { z } from "zod" import { createFindParams, createSelectParams } from "../../utils/validators" +import { applyAndAndOrOperators } from "../../utils/common-validators" export type StoreGetRegionParamsType = z.infer export const StoreGetRegionParams = createSelectParams() +export const StoreGetRegionsParamsFields = z.object({ + q: z.string().optional(), + id: z.union([z.string(), z.array(z.string())]).optional(), + currency_code: z.union([z.string(), z.array(z.string())]).optional(), + name: z.union([z.string(), z.array(z.string())]).optional(), +}) + export type StoreGetRegionsParamsType = z.infer export const StoreGetRegionsParams = createFindParams({ limit: 50, offset: 0, -}).merge( - z.object({ - q: z.string().optional(), - id: z.union([z.string(), z.array(z.string())]).optional(), - currency_code: z.union([z.string(), z.array(z.string())]).optional(), - name: z.union([z.string(), z.array(z.string())]).optional(), - $and: z.lazy(() => StoreGetRegionsParams.array()).optional(), - $or: z.lazy(() => StoreGetRegionsParams.array()).optional(), - }) -) +}) + .merge(StoreGetRegionsParamsFields) + .merge(applyAndAndOrOperators(StoreGetRegionsParamsFields)) diff --git a/packages/medusa/src/api/store/return/validators.ts b/packages/medusa/src/api/store/return/validators.ts index e6222c1db2..08deed770b 100644 --- a/packages/medusa/src/api/store/return/validators.ts +++ b/packages/medusa/src/api/store/return/validators.ts @@ -1,18 +1,19 @@ import { z } from "zod" import { createFindParams, createSelectParams } from "../../utils/validators" +import { applyAndAndOrOperators } from "../../utils/common-validators" export type ReturnParamsType = z.infer export const ReturnParams = createSelectParams() +export const ReturnsParamsFields = z.object({ + id: z.union([z.string(), z.array(z.string())]).optional(), + order_id: z.union([z.string(), z.array(z.string())]).optional(), +}) + export type ReturnsParamsType = z.infer -export const ReturnsParams = createFindParams().merge( - z.object({ - id: z.union([z.string(), z.array(z.string())]).optional(), - order_id: z.union([z.string(), z.array(z.string())]).optional(), - $and: z.lazy(() => ReturnsParams.array()).optional(), - $or: z.lazy(() => ReturnsParams.array()).optional(), - }) -) +export const ReturnsParams = createFindParams() + .merge(ReturnsParamsFields) + .merge(applyAndAndOrOperators(ReturnsParamsFields)) const ReturnShippingSchema = z.object({ option_id: z.string(), diff --git a/packages/medusa/src/api/store/shipping-options/validators.ts b/packages/medusa/src/api/store/shipping-options/validators.ts index 8080aaa09f..b036471e2e 100644 --- a/packages/medusa/src/api/store/shipping-options/validators.ts +++ b/packages/medusa/src/api/store/shipping-options/validators.ts @@ -1,5 +1,11 @@ import { z } from "zod" import { createFindParams } from "../../utils/validators" +import { applyAndAndOrOperators } from "../../utils/common-validators" + +export const StoreGetShippingOptionsFields = z.object({ + cart_id: z.string(), + is_return: z.boolean().optional(), +}) export type StoreGetShippingOptionsType = z.infer< typeof StoreGetShippingOptions @@ -7,11 +13,6 @@ export type StoreGetShippingOptionsType = z.infer< export const StoreGetShippingOptions = createFindParams({ limit: 20, offset: 0, -}).merge( - z.object({ - cart_id: z.string(), - is_return: z.boolean().optional(), - $and: z.lazy(() => StoreGetShippingOptions.array()).optional(), - $or: z.lazy(() => StoreGetShippingOptions.array()).optional(), - }) -) +}) + .merge(StoreGetShippingOptionsFields) + .merge(applyAndAndOrOperators(StoreGetShippingOptionsFields)) diff --git a/packages/medusa/src/api/utils/common-validators/common.ts b/packages/medusa/src/api/utils/common-validators/common.ts index 69d189f0e1..cb316ea3ab 100644 --- a/packages/medusa/src/api/utils/common-validators/common.ts +++ b/packages/medusa/src/api/utils/common-validators/common.ts @@ -25,6 +25,21 @@ export const BigNumberInput = z.union([ }), ]) +/** + * Return a zod object to apply the $and and $or operators on a schema. + * + * @param {ZodObject} schema + * @return {ZodObject} + */ +export const applyAndAndOrOperators = (schema: z.ZodObject) => { + return schema.merge( + z.object({ + $and: z.lazy(() => schema.array()).optional(), + $or: z.lazy(() => schema.array()).optional(), + }) + ) +} + /** * Validates that a value is a boolean when it is passed as a string. */ @@ -37,3 +52,26 @@ export const booleanString = () => .transform((value) => { return value.toString().toLowerCase() === "true" }) + +/** + * Apply a transformer on a schema when the data are validated and recursively normalize the data $and and $or. + * + * @param {(data: Data) => NormalizedData} transform + * @return {(data: Data) => NormalizedData} + */ +export function recursivelyNormalizeSchema< + Data extends object, + NormalizedData extends object +>(transform: (data: Data) => NormalizedData): (data: Data) => NormalizedData { + return (data: any) => { + const normalizedData = transform(data) + + Object.keys(normalizedData) + .filter((key) => ["$and", "$or"].includes(key)) + .forEach((key) => { + normalizedData[key] = normalizedData[key].map(transform) + }) + + return normalizedData + } +} diff --git a/packages/medusa/src/api/utils/common-validators/products/index.ts b/packages/medusa/src/api/utils/common-validators/products/index.ts index 655923dab1..bcb910cdf4 100644 --- a/packages/medusa/src/api/utils/common-validators/products/index.ts +++ b/packages/medusa/src/api/utils/common-validators/products/index.ts @@ -6,14 +6,13 @@ import { booleanString } from "../common" export const ProductStatusEnum = z.nativeEnum(ProductStatus) -export const GetProductsParams = z.object({ +export const StoreGetProductParamsDirectFields = z.object({ q: z.string().optional(), id: z.union([z.string(), z.array(z.string())]).optional(), title: z.string().optional(), handle: z.string().optional(), is_giftcard: booleanString().optional(), category_id: z.union([z.string(), z.array(z.string())]).optional(), - sales_channel_id: z.union([z.string(), z.array(z.string())]).optional(), collection_id: z.union([z.string(), z.array(z.string())]).optional(), tag_id: z.union([z.string(), z.array(z.string())]).optional(), type_id: z.union([z.string(), z.array(z.string())]).optional(), @@ -22,6 +21,12 @@ export const GetProductsParams = z.object({ deleted_at: createOperatorMap().optional(), }) +export const GetProductsParams = z + .object({ + sales_channel_id: z.union([z.string(), z.array(z.string())]).optional(), + }) + .merge(StoreGetProductParamsDirectFields) + type HttpProductFilters = FilterableProductProps & { tag_id?: string | string[] category_id?: string | string[] @@ -32,8 +37,8 @@ export const transformProductParams = ( ): FilterableProductProps => { const res = { ...data, - tags: normalizeArray(data, "tag_id"), - categories: normalizeArray(data, "category_id"), + tags: { id: data.tag_id }, + categories: { id: data.category_id }, } delete res.tag_id @@ -41,19 +46,3 @@ export const transformProductParams = ( return res as FilterableProductProps } - -const normalizeArray = (filters: HttpProductFilters, key: string) => { - if (filters[key]) { - if (Array.isArray(filters[key])) { - return { - id: { $in: filters[key] }, - } - } else { - return { - id: filters[key] as string, - } - } - } - - return undefined -}