From 3f16b011fa8c21d273a8252181877d5dae7724b7 Mon Sep 17 00:00:00 2001 From: Riqwan Thamir Date: Thu, 27 Jun 2024 12:24:34 +0200 Subject: [PATCH] feat(utils,types): DML Index can generate where SQL from a query builder (#7849) what: - introduces a simple query builder - uses the query builder to tranform an object in where to SQL when applying indexes ``` Examples: { where: { column: null } } -> column IS NULL { where: { column: { $ne: null } } } -> column IS NOT NULL { where: { boolean_column: true } } -> boolean_column IS TRUE { where: { column: "value", another_column: { $ne: 30 } } } -> column = "value" AND another_column != 30 ``` ``` const user = model .define("user", { email: model.text(), account: model.text(), organization: model.text(), }) .indexes([ { on: ["organization", "account"], where: { email: { $ne: null } }, }, { name: "IDX-email-account-special", on: ["organization", "account"], where: { email: { $ne: null }, account: null, }, }, ``` RESOLVES CORE-2392 --- packages/core/types/src/dml/index.ts | 15 ++- packages/core/utils/src/common/index.ts | 19 +-- packages/core/utils/src/common/is-boolean.ts | 3 + .../src/dml/__tests__/entity-builder.spec.ts | 122 ++++++++++++++---- packages/core/utils/src/dml/entity.ts | 7 +- .../helpers/entity-builder/build-indexes.ts | 26 +++- .../helpers/entity-builder/query-builder.ts | 62 +++++++++ .../dml/helpers/mikro-orm/build-indexes.ts | 6 +- 8 files changed, 215 insertions(+), 45 deletions(-) create mode 100644 packages/core/utils/src/common/is-boolean.ts create mode 100644 packages/core/utils/src/dml/helpers/entity-builder/query-builder.ts diff --git a/packages/core/types/src/dml/index.ts b/packages/core/types/src/dml/index.ts index 299c099995..4fd0a4e016 100644 --- a/packages/core/types/src/dml/index.ts +++ b/packages/core/types/src/dml/index.ts @@ -180,9 +180,20 @@ export type InferIndexableProperties = keyof (T extends IDmlEntity< } & InferForeignKeys : never) -export type EntityIndex = { +export type EntityIndex< + TSchema extends DMLSchema = DMLSchema, + TWhere = string +> = { name?: string unique?: boolean on: InferIndexableProperties>[] - where?: string + where?: TWhere +} + +export type SimpleQueryValue = string | number | boolean | null +export type NeQueryValue = { $ne: SimpleQueryValue } +export type QueryValue = SimpleQueryValue | NeQueryValue + +export interface QueryCondition { + [key: string]: QueryValue | QueryCondition | QueryCondition[] } diff --git a/packages/core/utils/src/common/index.ts b/packages/core/utils/src/common/index.ts index abac60fb68..eee51800e5 100644 --- a/packages/core/utils/src/common/index.ts +++ b/packages/core/utils/src/common/index.ts @@ -2,6 +2,7 @@ export * from "./alter-columns-helper" export * from "./array-difference" export * from "./array-intersection" export * from "./build-query" +export * from "./build-regexp-if-valid" export * from "./camel-to-snake-case" export * from "./container" export * from "./convert-item-response-to-update-request" @@ -11,29 +12,36 @@ export * from "./deduplicate" export * from "./deep-copy" export * from "./deep-equal-obj" export * from "./deep-flat-map" +export * from "./define-config" export * from "./errors" +export * from "./file-system" export * from "./generate-entity-id" export * from "./generate-linkable-keys-map" +export * from "./get-caller-file-path" export * from "./get-config-file" export * from "./get-duplicates" export * from "./get-iso-string-from-date" export * from "./get-selects-and-relations-from-object-array" export * from "./get-set-difference" +export * from "./graceful-shutdown-server" export * from "./group-by" export * from "./handle-postgres-database-error" export * from "./is-big-number" +export * from "./is-boolean" export * from "./is-date" export * from "./is-defined" export * from "./is-email" export * from "./is-object" export * from "./is-present" export * from "./is-string" +export * from "./load-env" export * from "./lower-case-first" export * from "./map-object-to" export * from "./medusa-container" export * from "./object-from-string-path" export * from "./object-to-string-path" export * from "./optional-numeric-serializer" +export * from "./parse-cors-origins" export * from "./partition-array" export * from "./pick-deep" export * from "./pick-value-from-object" @@ -51,18 +59,11 @@ export * from "./simple-hash" export * from "./string-to-select-relation-object" export * from "./stringify-circular" export * from "./to-camel-case" +export * from "./to-handle" export * from "./to-kebab-case" export * from "./to-pascal-case" export * from "./transaction" export * from "./trim-zeros" export * from "./upper-case-first" -export * from "./wrap-handler" -export * from "./to-handle" export * from "./validate-handle" -export * from "./parse-cors-origins" -export * from "./build-regexp-if-valid" -export * from "./load-env" -export * from "./define-config" -export * from "./file-system" -export * from "./graceful-shutdown-server" -export * from "./get-caller-file-path" +export * from "./wrap-handler" diff --git a/packages/core/utils/src/common/is-boolean.ts b/packages/core/utils/src/common/is-boolean.ts new file mode 100644 index 0000000000..ea858a60d8 --- /dev/null +++ b/packages/core/utils/src/common/is-boolean.ts @@ -0,0 +1,3 @@ +export function isBoolean(val: any): val is boolean { + return val != null && typeof val === "boolean" +} diff --git a/packages/core/utils/src/dml/__tests__/entity-builder.spec.ts b/packages/core/utils/src/dml/__tests__/entity-builder.spec.ts index 2ce81e850b..2a09226a27 100644 --- a/packages/core/utils/src/dml/__tests__/entity-builder.spec.ts +++ b/packages/core/utils/src/dml/__tests__/entity-builder.spec.ts @@ -2626,35 +2626,113 @@ describe("Entity builder", () => { ]) }) - test("should throw an error if field is unknown for an index", () => { - try { - const group = model.define("group", { - id: model.number(), - name: model.text(), - users: model.hasMany(() => user), + test("should define indexes with a query builder", () => { + const group = model.define("group", { + id: model.number(), + name: model.text(), + users: model.hasMany(() => user), + }) + + const user = model + .define("user", { + email: model.text(), + account: model.text(), + organization: model.text(), + is_owner: model.boolean(), + group: model.belongsTo(() => group, { mappedBy: "users" }), }) - - const user = model - .define("user", { - email: model.text(), - account: model.text(), - organization: model.text(), - group: model.belongsTo(() => group, { mappedBy: "users" }), - }) - .indexes([ - { - on: ["email", "account", "doesnotexist", "anotherdoesnotexist"], + .indexes([ + { + on: ["organization", "account"], + where: { email: { $ne: null } }, + }, + { + name: "IDX-email-account-special", + on: ["organization", "account"], + where: { + email: { $ne: null }, + account: null, }, - ]) + }, + { + name: "IDX_unique-name", + unique: true, + on: ["organization", "account", "group_id"], + }, + { + on: ["organization", "group_id"], + where: { is_owner: false }, + }, + { + on: ["account", "group_id"], + where: { is_owner: true }, + }, + ]) + const metaData = MetadataStorage.getMetadataFromDecorator( toMikroORMEntity(user) + ) - throw "should not reach" + expect(metaData.indexes).toEqual([ + { + expression: + 'CREATE INDEX IF NOT EXISTS "IDX_user_organization_account" ON "user" (organization, account) WHERE email IS NOT NULL AND deleted_at IS NULL', + name: "IDX_user_organization_account", + }, + { + expression: + 'CREATE INDEX IF NOT EXISTS "IDX-email-account-special" ON "user" (organization, account) WHERE email IS NOT NULL AND account IS NULL AND deleted_at IS NULL', + name: "IDX-email-account-special", + }, + { + expression: + 'CREATE UNIQUE INDEX IF NOT EXISTS "IDX_unique-name" ON "user" (organization, account, group_id) WHERE deleted_at IS NULL', + name: "IDX_unique-name", + }, + { + expression: + 'CREATE INDEX IF NOT EXISTS "IDX_user_organization_group_id" ON "user" (organization, group_id) WHERE is_owner IS FALSE AND deleted_at IS NULL', + name: "IDX_user_organization_group_id", + }, + { + expression: + 'CREATE INDEX IF NOT EXISTS "IDX_user_account_group_id" ON "user" (account, group_id) WHERE is_owner IS TRUE AND deleted_at IS NULL', + name: "IDX_user_account_group_id", + }, + ]) + }) + + test("should throw an error if field is unknown for an index", () => { + const group = model.define("group", { + id: model.number(), + name: model.text(), + users: model.hasMany(() => user), + }) + + const user = model + .define("user", { + email: model.text(), + account: model.text(), + organization: model.text(), + group: model.belongsTo(() => group, { mappedBy: "users" }), + }) + .indexes([ + { + on: ["email", "account", "doesnotexist", "anotherdoesnotexist"], + }, + ]) + + let err: any + + try { + toMikroORMEntity(user) } catch (e) { - expect(e.message).toEqual( - "Fields (doesnotexist, anotherdoesnotexist) are not found when applying indexes from DML entity" - ) + err = e } + + expect(err.message).toEqual( + `Cannot apply indexes on fields (doesnotexist, anotherdoesnotexist) for model User` + ) }) }) diff --git a/packages/core/utils/src/dml/entity.ts b/packages/core/utils/src/dml/entity.ts index 2c536e84bb..30ebba5441 100644 --- a/packages/core/utils/src/dml/entity.ts +++ b/packages/core/utils/src/dml/entity.ts @@ -5,6 +5,7 @@ import { ExtractEntityRelations, IDmlEntity, IsDmlEntity, + QueryCondition, } from "@medusajs/types" import { isObject, isString, toCamelCase } from "../common" import { transformIndexWhere } from "./helpers/entity-builder/build-indexes" @@ -121,13 +122,13 @@ export class DmlEntity implements IDmlEntity { return this } - indexes(indexes: EntityIndex[]) { + indexes(indexes: EntityIndex[]) { for (const index of indexes) { index.where = transformIndexWhere(index) - index.unique = index.unique ?? false + index.unique ??= false } - this.#indexes = indexes + this.#indexes = indexes as EntityIndex[] return this } } diff --git a/packages/core/utils/src/dml/helpers/entity-builder/build-indexes.ts b/packages/core/utils/src/dml/helpers/entity-builder/build-indexes.ts index c091813e31..aecac73774 100644 --- a/packages/core/utils/src/dml/helpers/entity-builder/build-indexes.ts +++ b/packages/core/utils/src/dml/helpers/entity-builder/build-indexes.ts @@ -1,5 +1,6 @@ -import { DMLSchema, EntityIndex } from "@medusajs/types" -import { isPresent } from "../../../common" +import { DMLSchema, EntityIndex, QueryCondition } from "@medusajs/types" +import { isObject, isPresent } from "../../../common" +import { buildWhereQuery } from "./query-builder" /* The DML provides an opinionated soft deletable entity as a part of every model @@ -8,9 +9,22 @@ import { isPresent } from "../../../common" this will need to be updated to include that case. */ export function transformIndexWhere( - index: EntityIndex -) { - let where = index.where + index: EntityIndex +): string { + return isObject(index.where) + ? transformWhereQb(index.where) + : transformWhere(index.where) +} + +function transformWhereQb(where: QueryCondition): string { + if (!isPresent(where.deleted_at)) { + where.deleted_at = null + } + + return buildWhereQuery(where) +} + +function transformWhere(where?: string): string { const lowerCaseWhere = where?.toLowerCase() const whereIncludesDeleteable = lowerCaseWhere?.includes("deleted_at is null") || @@ -22,7 +36,7 @@ export function transformIndexWhere( } // If where scope isn't present, we will set an opinionated where scope to the index - if (!isPresent(where)) { + if (!where?.length) { where = "deleted_at IS NULL" } diff --git a/packages/core/utils/src/dml/helpers/entity-builder/query-builder.ts b/packages/core/utils/src/dml/helpers/entity-builder/query-builder.ts new file mode 100644 index 0000000000..5cfb874862 --- /dev/null +++ b/packages/core/utils/src/dml/helpers/entity-builder/query-builder.ts @@ -0,0 +1,62 @@ +import { QueryCondition, QueryValue, SimpleQueryValue } from "@medusajs/types" +import { isBoolean, isDefined, isObject, isString } from "../../../common" + +/* + When creating indexes on the entity, we provide a basic query builder to generate + the SQL for where query upon the index. Since this is not a full query builder, + the onus is upon the user to ensure that the SQL is accurately generated. + + Examples: + + { where: { column: null } } + { where: { column: { $ne: null } } } + { where: { boolean_column: true } } + { where: { column: "value", another_column: { $ne: 30 }, boolean_column: true } } +*/ +export function buildWhereQuery(query: QueryCondition): string { + const conditions: string[] = [] + + for (const [key, value] of Object.entries(query)) { + if (isQueryCondition(value)) { + conditions.push(buildWhereQuery(value)) + } else { + conditions.push(buildCondition(key, value as QueryValue)) + } + } + + return conditions.filter((condition) => condition?.length).join(" AND ") +} + +function isQueryCondition(value: any): value is QueryCondition { + return isObject(value) && value !== null && !("$ne" in value) +} + +function buildCondition(key: string, value: QueryValue): string { + if (value === null) { + return `${key} IS NULL` + } else if (isObject(value) && "$ne" in value) { + if (value.$ne === null) { + return `${key} IS NOT NULL` + } else { + return `${key} != ${formatValue(value.$ne)}` + } + } else if (isBoolean(value)) { + return `${key} IS ${formatValue(value)}` + } else if (!isDefined(value)) { + return "" + } else { + return `${key} = ${formatValue(value)}` + } +} + +function formatValue(value: SimpleQueryValue): string { + if (isString(value)) { + return `'${value.replace(/'/g, "''")}'` // Escape single quotes + } + + if (isBoolean(value)) { + return value ? "TRUE" : "FALSE" + } + + return String(value) +} diff --git a/packages/core/utils/src/dml/helpers/mikro-orm/build-indexes.ts b/packages/core/utils/src/dml/helpers/mikro-orm/build-indexes.ts index 20668bab45..af725ce7a3 100644 --- a/packages/core/utils/src/dml/helpers/mikro-orm/build-indexes.ts +++ b/packages/core/utils/src/dml/helpers/mikro-orm/build-indexes.ts @@ -24,9 +24,9 @@ export function validateIndexFields( if (invalidFields.length) { throw new Error( - `Fields (${invalidFields.join( - ", " - )}) are not found when applying indexes from DML entity` + `Cannot apply indexes on fields (${invalidFields.join(", ")}) for model ${ + MikroORMEntity.name + }` ) } }