From 59bbff62d81861282d55a32d6c0c45285203c4e0 Mon Sep 17 00:00:00 2001 From: "Carlos R. L. Rodrigues" <37986729+carlos-r-l-rodrigues@users.noreply.github.com> Date: Mon, 19 May 2025 15:14:25 -0300 Subject: [PATCH] fix(index): Apply various fixes to the index engine (#12501) --- .changeset/plenty-shirts-smash.md | 9 ++ .../modules-sdk/src/remote-query/query.ts | 2 +- .../src/index-data/__tests__/index.spec.ts | 32 ++++++-- .../src/index-data/index-operator-map.ts | 20 ++--- .../query-input-config-filters.ts | 8 +- .../query-config/query-input-config.ts | 1 - .../common}/__tests__/compress-name.spec.ts | 0 .../utils/src/common}/compress-name.ts | 3 +- packages/core/utils/src/common/index.ts | 7 +- packages/core/utils/src/event-bus/utils.ts | 3 +- .../__tests__/query-builder.spec.ts | 82 +++++++------------ .../src/migrations/Migration20250515161913.ts | 59 +++++++++++++ .../index/src/services/postgres-provider.ts | 33 +------- packages/modules/index/src/types/index.ts | 1 - .../modules/index/src/utils/build-config.ts | 4 +- .../index/src/utils/create-partitions.ts | 11 ++- packages/modules/index/src/utils/index.ts | 1 + .../index/src/utils/normalze-table-name.ts | 10 +++ .../modules/index/src/utils/query-builder.ts | 39 +++++---- .../link-modules/src/utils/generate-entity.ts | 2 +- 20 files changed, 194 insertions(+), 133 deletions(-) create mode 100644 .changeset/plenty-shirts-smash.md rename packages/{modules/link-modules/src/utils => core/utils/src/common}/__tests__/compress-name.spec.ts (100%) rename packages/{modules/link-modules/src/utils => core/utils/src/common}/compress-name.ts (90%) create mode 100644 packages/modules/index/src/migrations/Migration20250515161913.ts create mode 100644 packages/modules/index/src/utils/normalze-table-name.ts diff --git a/.changeset/plenty-shirts-smash.md b/.changeset/plenty-shirts-smash.md new file mode 100644 index 0000000000..c2ebb99a31 --- /dev/null +++ b/.changeset/plenty-shirts-smash.md @@ -0,0 +1,9 @@ +--- +"@medusajs/link-modules": patch +"@medusajs/index": patch +"@medusajs/utils": patch +"@medusajs/modules-sdk": patch +"@medusajs/types": patch +--- + +fix(index): limit partition table name and index enum fields diff --git a/packages/core/modules-sdk/src/remote-query/query.ts b/packages/core/modules-sdk/src/remote-query/query.ts index 022a2bb050..f34e2e0dc4 100644 --- a/packages/core/modules-sdk/src/remote-query/query.ts +++ b/packages/core/modules-sdk/src/remote-query/query.ts @@ -232,7 +232,7 @@ export class Query { pagination: { // We pass through `take` to force the `select-in` query strategy // There might be a better way to do this, but for now this should do - take: queryOptions.pagination?.take, + take: queryOptions.pagination?.take ?? indexResponse.data.length, }, } diff --git a/packages/core/types/src/index-data/__tests__/index.spec.ts b/packages/core/types/src/index-data/__tests__/index.spec.ts index 1440280606..5384066ba8 100644 --- a/packages/core/types/src/index-data/__tests__/index.spec.ts +++ b/packages/core/types/src/index-data/__tests__/index.spec.ts @@ -23,14 +23,34 @@ describe("IndexQueryConfig", () => { expectTypeOf().toEqualTypeOf< | { - id?: string | string[] | OperatorMap - title?: string | string[] | OperatorMap + id?: string | string[] | OperatorMap | null + title?: + | string + | string[] + | OperatorMap + | null variants?: { - id?: string | string[] | OperatorMap - product_id?: string | string[] | OperatorMap - sku?: string | string[] | OperatorMap + id?: + | string + | string[] + | OperatorMap + | null + product_id?: + | string + | string[] + | OperatorMap + | null + sku?: + | string + | string[] + | OperatorMap + | null prices?: { - amount?: number | number[] | OperatorMap + amount?: + | number + | number[] + | OperatorMap + | null } } } diff --git a/packages/core/types/src/index-data/index-operator-map.ts b/packages/core/types/src/index-data/index-operator-map.ts index 5a679f4c36..01ed1f0651 100644 --- a/packages/core/types/src/index-data/index-operator-map.ts +++ b/packages/core/types/src/index-data/index-operator-map.ts @@ -1,12 +1,12 @@ export type IndexOperatorMap = { - $eq: T - $lt: T - $lte: T - $gt: T - $gte: T - $ne: T - $in: T - $is: T - $like: T - $ilike: T + $eq?: T + $lt?: T + $lte?: T + $gt?: T + $gte?: T + $ne?: T + $in?: T + $is?: T + $like?: T + $ilike?: T } diff --git a/packages/core/types/src/index-data/query-config/query-input-config-filters.ts b/packages/core/types/src/index-data/query-config/query-input-config-filters.ts index 07c844b1be..f2ecf70078 100644 --- a/packages/core/types/src/index-data/query-config/query-input-config-filters.ts +++ b/packages/core/types/src/index-data/query-config/query-input-config-filters.ts @@ -19,8 +19,12 @@ type ExtractFiltersOperators< ? never : Key extends ExcludedProps ? never - : TypeOnly extends string | number | boolean | Date - ? TypeOnly | TypeOnly[] | OperatorMap> + : TypeOnly extends string | number | boolean | Date | null + ? + | TypeOnly + | TypeOnly[] + | OperatorMap | TypeOnly[] | null> + | null : TypeOnly extends Array ? TypeOnly extends { __typename: any } ? IndexFilters diff --git a/packages/core/types/src/index-data/query-config/query-input-config.ts b/packages/core/types/src/index-data/query-config/query-input-config.ts index fe919bdf37..fb1dcd6a72 100644 --- a/packages/core/types/src/index-data/query-config/query-input-config.ts +++ b/packages/core/types/src/index-data/query-config/query-input-config.ts @@ -63,7 +63,6 @@ export type IndexQueryConfig = { filters?: IndexFilters joinFilters?: IndexFilters pagination?: Partial["pagination"]> - keepFilteredEntities?: boolean } export type QueryFunctionReturnPagination = { diff --git a/packages/modules/link-modules/src/utils/__tests__/compress-name.spec.ts b/packages/core/utils/src/common/__tests__/compress-name.spec.ts similarity index 100% rename from packages/modules/link-modules/src/utils/__tests__/compress-name.spec.ts rename to packages/core/utils/src/common/__tests__/compress-name.spec.ts diff --git a/packages/modules/link-modules/src/utils/compress-name.ts b/packages/core/utils/src/common/compress-name.ts similarity index 90% rename from packages/modules/link-modules/src/utils/compress-name.ts rename to packages/core/utils/src/common/compress-name.ts index b32809f7b8..ebd2a557c8 100644 --- a/packages/modules/link-modules/src/utils/compress-name.ts +++ b/packages/core/utils/src/common/compress-name.ts @@ -1,4 +1,5 @@ -import { camelToSnakeCase, simpleHash } from "@medusajs/framework/utils" +import { camelToSnakeCase } from "./camel-to-snake-case" +import { simpleHash } from "./simple-hash" export function compressName(name: string, limit = 58) { if (name.length <= limit) { diff --git a/packages/core/utils/src/common/index.ts b/packages/core/utils/src/common/index.ts index 7a8a75e518..b024d57fd8 100644 --- a/packages/core/utils/src/common/index.ts +++ b/packages/core/utils/src/common/index.ts @@ -4,6 +4,7 @@ export * from "./array-intersection" export * from "./build-query" export * from "./build-regexp-if-valid" export * from "./camel-to-snake-case" +export * from "./compress-name" export * from "./container" export * from "./convert-item-response-to-update-request" export * from "./create-container-like" @@ -80,11 +81,11 @@ export * from "./to-handle" export * from "./to-kebab-case" export * from "./to-pascal-case" export * from "./to-unix-slash" -export * from "./try-convert-to-number" export * from "./trim-zeros" +export * from "./try-convert-to-boolean" +export * from "./try-convert-to-number" export * from "./unflatten-object-keys" export * from "./upper-case-first" export * from "./validate-handle" -export * from "./wrap-handler" export * from "./validate-module-name" -export * from "./try-convert-to-boolean" +export * from "./wrap-handler" diff --git a/packages/core/utils/src/event-bus/utils.ts b/packages/core/utils/src/event-bus/utils.ts index 51617dcfb6..92b40046dd 100644 --- a/packages/core/utils/src/event-bus/utils.ts +++ b/packages/core/utils/src/event-bus/utils.ts @@ -44,8 +44,7 @@ export function buildModuleResourceEventName({ action: string }): string { const kebabCaseName = lowerCaseFirst(kebabCase(objectName)) - const conventionalePrefix = prefix && lowerCaseFirst(kebabCase(prefix)) - return `${prefix ? `${conventionalePrefix}.` : ""}${kebabCaseName}.${action}` + return `${prefix ? `${prefix}.` : ""}${kebabCaseName}.${action}` } /** diff --git a/packages/modules/index/integration-tests/__tests__/query-builder.spec.ts b/packages/modules/index/integration-tests/__tests__/query-builder.spec.ts index afe83e40c4..3ac70ff6bc 100644 --- a/packages/modules/index/integration-tests/__tests__/query-builder.spec.ts +++ b/packages/modules/index/integration-tests/__tests__/query-builder.spec.ts @@ -284,6 +284,34 @@ describe("IndexModuleService query", function () { afterEach(afterEach_) + it("should query all products where sku not null", async () => { + const { data } = await module.query({ + fields: ["product.*", "product.variants.*", "product.variants.prices.*"], + filters: { + product: { + variants: { + sku: { $ne: null }, + }, + }, + }, + }) + + expect(data.length).toEqual(1) + + const { data: data2 } = await module.query({ + fields: ["product.*", "product.variants.*", "product.variants.prices.*"], + filters: { + product: { + variants: { + sku: { $eq: null }, + }, + }, + }, + }) + + expect(data2.length).toEqual(0) + }) + it("should query all products ordered by sku DESC", async () => { const { data } = await module.query({ fields: ["product.*", "product.variants.*", "product.variants.prices.*"], @@ -710,60 +738,6 @@ describe("IndexModuleService query", function () { ]) }) - it("should query products filtering by price and returning the complete entity", async () => { - const { data, metadata } = await module.query({ - fields: ["product.*", "product.variants.*", "product.variants.prices.*"], - filters: { - product: { - variants: { - prices: { - amount: { $gt: 50 }, - }, - }, - }, - }, - keepFilteredEntities: true, - pagination: { - take: 100, - skip: 0, - }, - }) - - expect(metadata).toEqual({ - estimate_count: expect.any(Number), - skip: 0, - take: 100, - }) - - expect(data).toEqual([ - { - id: "prod_1", - variants: [ - { - id: "var_1", - sku: "aaa test aaa", - prices: [ - { - id: "money_amount_1", - amount: 100, - }, - ], - }, - { - id: "var_2", - sku: "sku 123", - prices: [ - { - id: "money_amount_2", - amount: 10, - }, - ], - }, - ], - }, - ]) - }) - it("should query all products", async () => { const { data } = await module.query({ fields: ["product.*", "product.variants.*", "product.variants.prices.*"], diff --git a/packages/modules/index/src/migrations/Migration20250515161913.ts b/packages/modules/index/src/migrations/Migration20250515161913.ts new file mode 100644 index 0000000000..ba301605ca --- /dev/null +++ b/packages/modules/index/src/migrations/Migration20250515161913.ts @@ -0,0 +1,59 @@ +import { Migration } from "@mikro-orm/migrations" + +export class Migration20250515161913 extends Migration { + override async up(): Promise { + this.addSql(` + DO $$ + DECLARE + r RECORD; + protected_tables TEXT[] := ARRAY[ + 'cat_linkproductsaleschannel', + 'cat_linkproductvariantpriceset', + 'cat_price', + 'cat_priceset', + 'cat_product', + 'cat_productvariant', + 'cat_saleschannel', + 'cat_pivot_linkproductsaleschannelsaleschannel', + 'cat_pivot_linkproductvariantpricesetpriceset', + 'cat_pivot_pricesetprice', + 'cat_pivot_productlinkproductsaleschannel', + 'cat_pivot_productproductvariant', + 'cat_pivot_productvariantlinkproductvariantpriceset' + ]; + BEGIN + FOR r IN + SELECT tablename + FROM pg_tables + WHERE schemaname = 'public' AND tablename LIKE 'cat\_%' + LOOP + IF r.tablename <> ALL (protected_tables) THEN + EXECUTE format('DROP TABLE IF EXISTS public.%I CASCADE;', r.tablename); + END IF; + END LOOP; + END $$; + + UPDATE index_sync SET last_key = NULL WHERE entity NOT IN ( + 'Product', + 'ProductVariant', + 'LinkProductVariantPriceSet', + 'LinkProductSalesChannel', + 'Price', + 'PriceSet', + 'SalesChannel' + ); + + UPDATE index_metadata SET status = 'pending' WHERE entity NOT IN ( + 'Product', + 'ProductVariant', + 'LinkProductVariantPriceSet', + 'LinkProductSalesChannel', + 'Price', + 'PriceSet', + 'SalesChannel' + ); + `) + } + + override async down(): Promise {} +} diff --git a/packages/modules/index/src/services/postgres-provider.ts b/packages/modules/index/src/services/postgres-provider.ts index 93f9b768cc..dc5d83beaf 100644 --- a/packages/modules/index/src/services/postgres-provider.ts +++ b/packages/modules/index/src/services/postgres-provider.ts @@ -237,12 +237,7 @@ export class PostgresProvider implements IndexTypes.StorageProvider { ): Promise> { await this.#isReady_ - const { - keepFilteredEntities, - fields = [], - filters = {}, - joinFilters = {}, - } = config + const { fields = [], filters = {}, joinFilters = {} } = config const { take, skip, order: inputOrderBy = {} } = config.pagination ?? {} const select = normalizeFieldsSelection(fields) @@ -281,7 +276,6 @@ export class PostgresProvider implements IndexTypes.StorageProvider { options: { skip, take, - keepFilteredEntities, orderBy, }, rawConfig: config, @@ -290,7 +284,6 @@ export class PostgresProvider implements IndexTypes.StorageProvider { const { sql, sqlCount } = qb.buildQuery({ hasPagination, - returnIdOnly: !!keepFilteredEntities, hasCount, }) @@ -310,30 +303,6 @@ export class PostgresProvider implements IndexTypes.StorageProvider { } as IndexTypes.QueryFunctionReturnPagination) : undefined - if (keepFilteredEntities) { - const mainEntity = Object.keys(select)[0] - - const ids = resultSet.map((r) => r[`${mainEntity}.id`]) - if (ids.length) { - const result = await this.query( - { - fields, - joinFilters, - filters: { - [mainEntity]: { - id: ids, - }, - }, - pagination: undefined, - keepFilteredEntities: false, - } as IndexTypes.IndexQueryConfig, - sharedContext - ) - result.metadata ??= resultMetadata - return result - } - } - return { data: qb.buildObjectFromResultset( resultSet diff --git a/packages/modules/index/src/types/index.ts b/packages/modules/index/src/types/index.ts index 8a4cde818c..b2da975d9a 100644 --- a/packages/modules/index/src/types/index.ts +++ b/packages/modules/index/src/types/index.ts @@ -29,5 +29,4 @@ export type QueryOptions = { skip?: number take?: number orderBy?: OrderBy | OrderBy[] - keepFilteredEntities?: boolean } diff --git a/packages/modules/index/src/utils/build-config.ts b/packages/modules/index/src/utils/build-config.ts index 94c11668d0..b8f99c9dde 100644 --- a/packages/modules/index/src/utils/build-config.ts +++ b/packages/modules/index/src/utils/build-config.ts @@ -1180,7 +1180,9 @@ function buildSchemaFromFilterableLinks( return } - const fieldType = fieldRef.type.toString() + const isEnum = + fieldRef.type?.astNode?.kind === GraphQLUtils.Kind.ENUM_TYPE_DEFINITION + const fieldType = isEnum ? "String" : fieldRef.type.toString() const isArray = fieldType.startsWith("[") const currentType = fieldType.replace(/\[|\]|\!/g, "") diff --git a/packages/modules/index/src/utils/create-partitions.ts b/packages/modules/index/src/utils/create-partitions.ts index 9bf083a80e..af9e0beab4 100644 --- a/packages/modules/index/src/utils/create-partitions.ts +++ b/packages/modules/index/src/utils/create-partitions.ts @@ -1,6 +1,7 @@ import { IndexTypes } from "@medusajs/framework/types" import { SqlEntityManager } from "@mikro-orm/postgresql" import { schemaObjectRepresentationPropertiesToOmit } from "@types" +import { getPivotTableName, normalizeTableName } from "./normalze-table-name" export async function createPartitions( schemaObjectRepresentation: IndexTypes.SchemaObjectRepresentation, @@ -18,7 +19,7 @@ export async function createPartitions( schemaObjectRepresentation[key].listeners.length > 0 ) .map((key) => { - const cName = key.toLowerCase() + const cName = normalizeTableName(key) if (createdPartitions.has(cName)) { return [] @@ -35,7 +36,8 @@ export async function createPartitions( continue } - const pName = `cat_pivot_${parent.ref.entity}${key}`.toLowerCase() + const pName = getPivotTableName(`${parent.ref.entity}${key}`) + if (createdPartitions.has(pName)) { continue } @@ -63,7 +65,7 @@ export async function createPartitions( schemaObjectRepresentation[key].listeners.length > 0 ) .map((key) => { - const cName = key.toLowerCase() + const cName = normalizeTableName(key) const part: string[] = [] part.push( @@ -79,7 +81,8 @@ export async function createPartitions( continue } - const pName = `cat_pivot_${parent.ref.entity}${key}`.toLowerCase() + const pName = getPivotTableName(`${parent.ref.entity}${key}`) + part.push( `CREATE INDEX CONCURRENTLY IF NOT EXISTS "IDX_${pName}_child_id" ON ${activeSchema}${pName} ("child_id")` ) diff --git a/packages/modules/index/src/utils/index.ts b/packages/modules/index/src/utils/index.ts index 5690bf3e90..491a294035 100644 --- a/packages/modules/index/src/utils/index.ts +++ b/packages/modules/index/src/utils/index.ts @@ -6,3 +6,4 @@ export * from "./sync/configuration" export * from "./index-metadata-status" export * from "./gql-to-types" export * from "./default-schema" +export * from "./normalze-table-name" diff --git a/packages/modules/index/src/utils/normalze-table-name.ts b/packages/modules/index/src/utils/normalze-table-name.ts new file mode 100644 index 0000000000..53a823b684 --- /dev/null +++ b/packages/modules/index/src/utils/normalze-table-name.ts @@ -0,0 +1,10 @@ +import { compressName } from "@medusajs/framework/utils" + +export function normalizeTableName(name: string): string { + return compressName(name.toLowerCase(), 58).replace(/[^a-z0-9_]/g, "_") +} + +export function getPivotTableName(tableName: string) { + const compressedName = normalizeTableName(tableName) + return `cat_pivot_${compressedName}` +} diff --git a/packages/modules/index/src/utils/query-builder.ts b/packages/modules/index/src/utils/query-builder.ts index 3ee0ec0f48..f802209fec 100644 --- a/packages/modules/index/src/utils/query-builder.ts +++ b/packages/modules/index/src/utils/query-builder.ts @@ -7,6 +7,7 @@ import { } from "@medusajs/framework/utils" import { Knex } from "@mikro-orm/knex" import { OrderBy, QueryFormat, QueryOptions, Select } from "@types" +import { getPivotTableName, normalizeTableName } from "./normalze-table-name" function escapeJsonPathString(val: string): string { // Escape for JSONPath string @@ -23,6 +24,10 @@ function buildSafeJsonPathQuery( jsonPathOperator = "==" } else if (operator.toUpperCase().includes("LIKE")) { jsonPathOperator = "like_regex" + } else if (operator === "IS") { + jsonPathOperator = "==" + } else if (operator === "IS NOT") { + jsonPathOperator = "!=" } if (typeof value === "string") { @@ -32,6 +37,10 @@ function buildSafeJsonPathQuery( val = val.replace(/%/g, ".*").replace(/_/g, ".") } value = `"${escapeJsonPathString(val)}"` + } else { + if ((operator === "IS" || operator === "IS NOT") && value === null) { + value = "null" + } } return `$.${field} ${jsonPathOperator} ${value}` @@ -530,14 +539,14 @@ export class QueryBuilder { aliasMapping[currentAliasPath] = alias if (level > 0) { - const cName = entity.ref.entity.toLowerCase() + const cName = normalizeTableName(entity.ref.entity) let joinTable = `cat_${cName} AS ${alias}` if (entity.isInverse || parEntity.isInverse) { const pName = `${entity.ref.entity}${parEntity.ref.entity}`.toLowerCase() - const pivotTable = `cat_pivot_${pName}` + const pivotTable = getPivotTableName(pName) joinBuilder.leftJoin( `${pivotTable} AS ${alias}_ref`, @@ -552,7 +561,7 @@ export class QueryBuilder { } else { const pName = `${parEntity.ref.entity}${entity.ref.entity}`.toLowerCase() - const pivotTable = `cat_pivot_${pName}` + const pivotTable = getPivotTableName(pName) joinBuilder.leftJoin( `${pivotTable} AS ${alias}_ref`, @@ -704,11 +713,9 @@ export class QueryBuilder { public buildQuery({ hasPagination = true, hasCount = false, - returnIdOnly = false, }: { hasPagination?: boolean hasCount?: boolean - returnIdOnly?: boolean }): { sql: string; sqlCount?: string } { const selectOnlyStructure = this.selector.select const structure = this.requestedFields @@ -816,7 +823,10 @@ export class QueryBuilder { } innerQueryBuilder.from( - `cat_${rootEntity} AS ${this.getShortAlias(aliasMapping, rootKey)}` + `cat_${normalizeTableName(rootEntity)} AS ${this.getShortAlias( + aliasMapping, + rootKey + )}` ) joinParts.forEach((joinPart) => { @@ -887,7 +897,10 @@ export class QueryBuilder { const innerQueryAlias = "paginated_ids" outerQueryBuilder.from( - `cat_${rootEntity} AS ${this.getShortAlias(aliasMapping, rootKey)}` + `cat_${normalizeTableName(rootEntity)} AS ${this.getShortAlias( + aliasMapping, + rootKey + )}` ) outerQueryBuilder.joinRaw( @@ -909,13 +922,11 @@ export class QueryBuilder { outerQueryBuilder.joinRaw(joinPart) }) - const finalSelectParts = !returnIdOnly - ? this.buildSelectParts( - selectOnlyStructure[rootKey] as Select, - rootKey, - aliasMapping - ) - : { [`${rootKey}.id`]: `${rootAlias}.id` } + const finalSelectParts = this.buildSelectParts( + selectOnlyStructure[rootKey] as Select, + rootKey, + aliasMapping + ) outerQueryBuilder.select(finalSelectParts) diff --git a/packages/modules/link-modules/src/utils/generate-entity.ts b/packages/modules/link-modules/src/utils/generate-entity.ts index 333f7ab75f..07ee2dbed7 100644 --- a/packages/modules/link-modules/src/utils/generate-entity.ts +++ b/packages/modules/link-modules/src/utils/generate-entity.ts @@ -4,13 +4,13 @@ import { } from "@medusajs/framework/types" import { composeTableName, + compressName, mikroOrmSoftDeletableFilterOptions, simpleHash, SoftDeletableFilterKey, } from "@medusajs/framework/utils" import { EntitySchema } from "@mikro-orm/core" -import { compressName } from "./compress-name" function getClass(...properties) { return class LinkModel {