diff --git a/BREAKINGCHANGES.md b/BREAKINGCHANGES.md index 8612ff374..fe0a67e86 100644 --- a/BREAKINGCHANGES.md +++ b/BREAKINGCHANGES.md @@ -4,4 +4,4 @@ 1. `@omit` and `@password` attributes have been removed 1. SWR plugin is removed 1. `makeModelSchema()` no longer includes relation fields by default — use `include` or `select` options to opt in, mirroring ORM behaviour -1. ORM entity mutation catch-all hooks now need to be registered under `onEntityMutation.$all` +1. ORM entity mutation catch-all hooks should now be registered under `onEntityMutation.$all` (the previous shape still works but emits a deprecation warning); entities passed to per-model hooks are typed as raw database rows (same as `$qb` results) diff --git a/packages/orm/src/client/client-impl.ts b/packages/orm/src/client/client-impl.ts index 47dc82c2f..f6d9f9d76 100644 --- a/packages/orm/src/client/client-impl.ts +++ b/packages/orm/src/client/client-impl.ts @@ -45,6 +45,10 @@ import { createZenStackPromise, type ZenStackPromise } from './promise'; import { fieldHasDefaultValue, getField, isUnsupportedField, requireModel } from './query-utils'; import { ResultProcessor } from './result-processor'; +// tracks `onEntityMutation` objects that have already been validated/warned about, +// so that derived clients (e.g., from `$use` or transactions) don't warn repeatedly +const warnedPlugins = new WeakSet(); + type ExtResultFieldDef = { needs: Record; compute: (data: Record) => unknown; @@ -92,6 +96,10 @@ export class ClientImpl { ...this.$options.functions, }; + if (this.$options.plugins) { + this.$options.plugins = this.normalizePlugins(this.$options.plugins); + } + this.validateOptions(baseClient, options); // here we use kysely's props constructor so we can pass a custom query executor @@ -163,6 +171,104 @@ export class ClientImpl { return createClientProxy(this); } + /** + * Normalizes plugin definitions for backward compatibility and validates + * `onEntityMutation` keys. Legacy plugins that define hooks directly under + * `onEntityMutation` (instead of under `$all`) are converted to the new shape + * with a one-time warning. + */ + private normalizePlugins(plugins: AnyPlugin[]): AnyPlugin[] { + let changed = false; + const result = plugins.map((plugin) => { + const normalized = this.normalizeEntityMutationHooks(plugin); + if (normalized !== plugin) { + changed = true; + } + return normalized; + }); + return changed ? result : plugins; + } + + private normalizeEntityMutationHooks(plugin: AnyPlugin): AnyPlugin { + const hooks = plugin.onEntityMutation as Record | undefined; + if (!hooks || typeof hooks !== 'object') { + return plugin; + } + + const legacyKeys = ['beforeEntityMutation', 'afterEntityMutation', 'runAfterMutationWithinTransaction']; + const presentLegacyKeys = legacyKeys.filter((key) => key in hooks); + + const unknownKeys = Object.keys(hooks).filter( + (key) => key !== '$all' && !legacyKeys.includes(key) && !(key in this.$schema.models), + ); + + const warnOnce = (message: string) => { + if (!warnedPlugins.has(hooks)) { + console.warn(message); + } + }; + + if (unknownKeys.length > 0) { + warnOnce( + `Plugin "${plugin.id}" defines "onEntityMutation" hooks for unknown keys: ${unknownKeys + .map((k) => `"${k}"`) + .join(', ')}. Valid keys are "$all" and model names. These hooks will never be invoked.`, + ); + } + + if (presentLegacyKeys.length === 0) { + warnedPlugins.add(hooks); + return plugin; + } + + warnOnce( + `Plugin "${plugin.id}" defines "onEntityMutation" hooks in a deprecated shape. ` + + `Hooks that apply to all models should be nested under "$all": ` + + `{ onEntityMutation: { $all: { beforeEntityMutation, afterEntityMutation, runAfterMutationWithinTransaction } } }. ` + + `The legacy shape is still supported but will be removed in a future release.`, + ); + warnedPlugins.add(hooks); + + const legacy: Record = {}; + const rest: Record = {}; + for (const [key, value] of Object.entries(hooks)) { + if (legacyKeys.includes(key)) { + legacy[key] = value; + } else { + rest[key] = value; + } + } + + // bind legacy callbacks to the original plugin object to preserve `this` + for (const key of ['beforeEntityMutation', 'afterEntityMutation']) { + if (typeof legacy[key] === 'function') { + legacy[key] = (legacy[key] as Function).bind(plugin); + } + } + + const normalizedHooks = { + ...rest, + $all: { + // explicitly nested `$all` (if any) takes precedence over legacy keys + ...legacy, + ...(rest['$all'] as object | undefined), + }, + }; + // mark the normalized hooks as validated so derived clients don't warn again + warnedPlugins.add(normalizedHooks); + + // create a copy that preserves the plugin's prototype chain (plugins may be class + // instances with hooks defined on the prototype), overriding only `onEntityMutation` + const copy = Object.create(Object.getPrototypeOf(plugin), Object.getOwnPropertyDescriptors(plugin)); + Object.defineProperty(copy, 'onEntityMutation', { + value: normalizedHooks, + enumerable: true, + configurable: true, + writable: true, + }); + return copy as AnyPlugin; + } + private validateOptions(baseClient: ClientImpl | undefined, options: ClientOptions) { if (!baseClient && !options.skipValidationForComputedFields) { // validate computed fields configuration once for the root client diff --git a/packages/orm/src/client/executor/zenstack-query-executor.ts b/packages/orm/src/client/executor/zenstack-query-executor.ts index ebd0842bf..c7d9cbadc 100644 --- a/packages/orm/src/client/executor/zenstack-query-executor.ts +++ b/packages/orm/src/client/executor/zenstack-query-executor.ts @@ -450,8 +450,11 @@ export class ZenStackQueryExecutor extends DefaultQueryExecutor { } if (this.dialect.supportsReturning) { - // entities are returned in the query result - return queryResult.rows as Record[]; + // entities are returned in the query result; shallow-copy the rows so that + // hooks (which may run after the ORM has post-processed the query result + // in place, e.g., when deferred to after transaction commit) always see + // raw database rows + return (queryResult.rows as Record[]).map((row) => ({ ...row })); } else { const mutatedIds = InsertQueryNode.is(queryNode) ? this.getInsertIds(mutationInfo.model, queryNode, queryResult) diff --git a/packages/orm/src/client/plugin.ts b/packages/orm/src/client/plugin.ts index c9f0d791a..a706399a1 100644 --- a/packages/orm/src/client/plugin.ts +++ b/packages/orm/src/client/plugin.ts @@ -1,11 +1,11 @@ -import type { OperationNode, QueryId, QueryResult, RootOperationNode, UnknownRow } from 'kysely'; +import type { OperationNode, QueryId, QueryResult, RootOperationNode, Selectable, UnknownRow } from 'kysely'; import type { ZodType } from 'zod'; import type { ClientContract, ZModelFunction } from '.'; import type { GetModelFields, GetModels, NonRelationFields, SchemaDef } from '@zenstackhq/schema'; import type { MaybePromise } from '../utils/type-utils'; import type { MapModelFieldType } from './crud-types'; import type { AllCrudOperations, CoreCrudOperations } from './crud/operations/base'; -import type { DefaultModelResult } from './crud-types'; +import type { ToKyselySchema } from './query-builder'; type AllowedExtQueryArgKeys = CoreCrudOperations | '$create' | '$read' | '$update' | '$delete' | '$all'; @@ -388,12 +388,32 @@ export type PluginAfterEntityMutationArgs = MutationHo client: ClientContract; }; +/** + * The shape of an entity passed to per-model entity mutation hooks. Entities are + * loaded directly from the database table (the same way as `$qb` query results), + * so they contain only physical columns (no relations, no computed fields, and no + * fields inherited from a delegate base model), and values are in the form returned + * by the database driver, not post-processed by the ORM. + * + * For models using delegate inheritance (`@@delegate`), a mutation fires one event + * per physical table written, keyed by that table's model. E.g., creating a sub-model + * entity fires a base-model event followed by a sub-model event; updating only + * inherited fields fires only a base-model event; deleting a sub-model entity fires + * only a base-model event (the sub-model row is removed by cascade). Base-model + * entities carry the discriminator field, which can be used to identify the concrete + * sub-model. To observe all mutations of a sub-model, register hooks on its base model + * (or use `$all`). + */ +export type MutationHookEntity> = Selectable< + ToKyselySchema[Model] +>; + export type PluginModelBeforeEntityMutationArgs> = Omit< MutationHooksArgs, 'model' > & { model: Model; - loadBeforeMutationEntities(): Promise[] | undefined>; + loadBeforeMutationEntities(): Promise[] | undefined>; client: ClientContract; }; @@ -402,8 +422,8 @@ export type PluginModelAfterEntityMutationArgs & { model: Model; - loadAfterMutationEntities(): Promise[] | undefined>; - beforeMutationEntities?: DefaultModelResult[]; + loadAfterMutationEntities(): Promise[] | undefined>; + beforeMutationEntities?: MutationHookEntity[]; client: ClientContract; }; diff --git a/tests/e2e/orm/plugin-infra/entity-mutation-hooks.test.ts b/tests/e2e/orm/plugin-infra/entity-mutation-hooks.test.ts index 0bc5514e3..a6bc7f297 100644 --- a/tests/e2e/orm/plugin-infra/entity-mutation-hooks.test.ts +++ b/tests/e2e/orm/plugin-infra/entity-mutation-hooks.test.ts @@ -1,8 +1,11 @@ -import { type ClientContract } from '@zenstackhq/orm'; +import { type ClientContract, type MutationHookEntity } from '@zenstackhq/orm'; import { createTestClient } from '@zenstackhq/testtools'; import { DeleteQueryNode, InsertQueryNode, UpdateQueryNode } from 'kysely'; -import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import { sql } from 'kysely'; +import { afterEach, beforeEach, describe, expect, expectTypeOf, it, vi } from 'vitest'; import { schema } from '../schemas/basic'; +import { schema as delegateSchemaTyped } from '../schemas/delegate/schema'; +import { schema as typingSchema } from '../schemas/typing/schema'; describe('Entity mutation hooks tests', () => { let _client: ClientContract; @@ -869,12 +872,266 @@ describe('Entity mutation hooks tests', () => { data: { email: 'u1@test.com' }, }); }); + + it('after-mutation entities match $qb rows', async () => { + let postEntity: any; + let userEntity: any; + + const client = _client.$use({ + id: 'test', + onEntityMutation: { + Post: { + async afterEntityMutation(args) { + const entities = await args.loadAfterMutationEntities(); + postEntity = entities?.[0]; + // entity type must be identical to a `$qb` selectAll row + expectTypeOf(entities!).items.toEqualTypeOf>>(); + }, + }, + User: { + async afterEntityMutation(args) { + const entities = await args.loadAfterMutationEntities(); + userEntity = entities?.[0]; + }, + }, + }, + }); + + const qbPostQuery = () => client.$qb.selectFrom('Post').selectAll().executeTakeFirstOrThrow(); + + const user = await client.user.create({ + data: { + email: 'u1@test.com', + meta: { foo: 'bar' }, + posts: { create: { title: 'P1', published: true } }, + }, + }); + + const qbPost = await client.$qb + .selectFrom('Post') + .selectAll() + .where('authorId', '=', user.id) + .executeTakeFirstOrThrow(); + const qbUser = await client.$qb + .selectFrom('User') + .selectAll() + .where('id', '=', user.id) + .executeTakeFirstOrThrow(); + + expect(postEntity).toEqual(qbPost); + expect(userEntity).toEqual(qbUser); + }); + + it('before-mutation entities match $qb rows', async () => { + let beforeEntity: any; + let beforeEntityInAfterHook: any; + + const client = _client.$use({ + id: 'test', + onEntityMutation: { + Post: { + async beforeEntityMutation(args) { + beforeEntity = (await args.loadBeforeMutationEntities())?.[0]; + }, + afterEntityMutation(args) { + beforeEntityInAfterHook = args.beforeMutationEntities?.[0]; + }, + }, + }, + }); + + const user = await client.user.create({ data: { email: 'u1@test.com' } }); + const post = await client.post.create({ data: { title: 'P1', published: true, authorId: user.id } }); + const qbPostBefore = await client.$qb + .selectFrom('Post') + .selectAll() + .where('id', '=', post.id) + .executeTakeFirstOrThrow(); + + await client.post.update({ where: { id: post.id }, data: { title: 'P2' } }); + + expect(beforeEntity).toEqual(qbPostBefore); + expect(beforeEntityInAfterHook).toEqual(qbPostBefore); + }); + + it('supports legacy-shaped onEntityMutation (hooks directly under onEntityMutation) with a warning', async () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + try { + const calls: string[] = []; + let thisArg: unknown; + const legacy = { + id: 'legacy', + beforeEntityMutation(args: any) { + calls.push(`before:${args.model}:${args.action}`); + }, + onEntityMutation: { + beforeEntityMutation(this: any, args: any) { + thisArg = this; + calls.push(`before:${args.model}:${args.action}`); + }, + afterEntityMutation(args: any) { + calls.push(`after:${args.model}:${args.action}`); + }, + runAfterMutationWithinTransaction: true, + }, + }; + + const client = _client.$use(legacy as any); + expect(warn).toHaveBeenCalledTimes(1); + expect(warn.mock.calls[0]![0]).toContain('deprecated'); + + await client.user.create({ data: { email: 'u1@test.com' } }); + expect(calls).toEqual(['before:User:create', 'after:User:create']); + // `this` is the plugin object + expect(thisArg).toBe(legacy); + + // derived clients (e.g., transactions) should not warn again + await client.$transaction(async (tx) => { + await tx.user.create({ data: { email: 'u2@test.com' } }); + }); + expect(warn).toHaveBeenCalledTimes(1); + expect(calls).toContain('after:User:create'); + } finally { + warn.mockRestore(); + } + }); + + it('preserves prototype-defined hooks of class-based plugins using the legacy shape', async () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + try { + const calls: string[] = []; + + class LegacyPlugin { + id = 'class-legacy'; + onEntityMutation = { + afterEntityMutation: (args: any) => { + calls.push(`mutation:${args.action}`); + }, + }; + // the following hooks live on the prototype, not as own properties + onQuery(ctx: any) { + calls.push(`query:${ctx.operation}`); + return ctx.proceed(ctx.args); + } + onKyselyQuery(ctx: any) { + calls.push('kysely'); + return ctx.proceed(ctx.query); + } + } + + const client = _client.$use(new LegacyPlugin() as any); + expect(warn).toHaveBeenCalledTimes(1); + + await client.user.create({ data: { email: 'u1@test.com' } }); + expect(calls).toContain('mutation:create'); + expect(calls).toContain('query:create'); + expect(calls).toContain('kysely'); + } finally { + warn.mockRestore(); + } + }); + + it('explicit $all takes precedence over legacy keys when both are present', async () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + try { + const calls: string[] = []; + const client = _client.$use({ + id: 'mixed', + onEntityMutation: { + afterEntityMutation() { + calls.push('legacy'); + }, + $all: { + afterEntityMutation() { + calls.push('$all'); + }, + }, + }, + } as any); + + await client.user.create({ data: { email: 'u1@test.com' } }); + expect(calls).toEqual(['$all']); + } finally { + warn.mockRestore(); + } + }); + + it('warns on onEntityMutation keys that are not $all or a known model name', async () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + try { + const client = _client.$use({ + id: 'typo', + onEntityMutation: { + user: { + afterEntityMutation() {}, + }, + }, + } as any); + expect(warn).toHaveBeenCalledTimes(1); + expect(warn.mock.calls[0]![0]).toContain('"user"'); + + // no repeated warnings for derived clients + await client.$transaction(async (tx) => { + await tx.user.create({ data: { email: 'u1@test.com' } }); + }); + expect(warn).toHaveBeenCalledTimes(1); + } finally { + warn.mockRestore(); + } + }); }); -describe('Entity mutation hooks - delegate model interaction', () => { - it('update on child model succeeds with afterEntityMutation plugin', async () => { - const client = await createTestClient( +describe('Entity mutation hooks - computed fields', () => { + it('hook entities exclude computed fields in both type and value', async () => { + const db = await createTestClient( ` +model User { + id Int @id @default(autoincrement()) + firstName String + lastName String + fullName String @computed +} +`, + { + computedFields: { + User: { + fullName: (eb: any) => eb.fn('concat', [eb.ref('firstName'), sql.lit(' '), eb.ref('lastName')]), + }, + }, + } as any, + ); + + let entity: any; + const client = db.$use({ + id: 'test', + onEntityMutation: { + User: { + async afterEntityMutation(args: any) { + entity = (await args.loadAfterMutationEntities())?.[0]; + }, + }, + }, + }); + + await client.user.create({ data: { firstName: 'Alex', lastName: 'Smith' } }); + + // type-level: computed field `postCount` is not part of the hook entity type + type UserEntity = MutationHookEntity; + expectTypeOf().toHaveProperty('email'); + expectTypeOf().not.toHaveProperty('postCount'); + // relations are not part of the entity either + expectTypeOf().not.toHaveProperty('posts'); + + expect(entity).toBeDefined(); + expect(entity).toMatchObject({ firstName: 'Alex', lastName: 'Smith' }); + expect(entity).not.toHaveProperty('fullName'); + + await db.$disconnect(); + }); +}); + +describe('Entity mutation hooks - delegate model interaction', () => { + const delegateSchema = ` model Base { id Int @id @default(autoincrement()) orgId Int @@ -885,8 +1142,162 @@ model Base { model Child extends Base { childField Int } -`, - ); +`; + + it('hook entity types match per-table rows (no inherited or descendant fields)', () => { + // Asset (root base): only its own columns + type AssetEntity = MutationHookEntity; + expectTypeOf().toHaveProperty('assetType'); + expectTypeOf().toHaveProperty('ownerId'); + expectTypeOf().not.toHaveProperty('duration'); + expectTypeOf().not.toHaveProperty('rating'); + expectTypeOf().not.toHaveProperty('owner'); + + // Video (sub-model and delegate base): own columns, no inherited ones + type VideoEntity = MutationHookEntity; + expectTypeOf().toHaveProperty('id'); + expectTypeOf().toHaveProperty('duration'); + expectTypeOf().toHaveProperty('videoType'); + expectTypeOf().not.toHaveProperty('assetType'); + expectTypeOf().not.toHaveProperty('viewCount'); + expectTypeOf().not.toHaveProperty('rating'); + + // RatedVideo (chained sub-model): own columns only + type RatedVideoEntity = MutationHookEntity; + expectTypeOf().toHaveProperty('id'); + expectTypeOf().toHaveProperty('rating'); + expectTypeOf().toHaveProperty('userId'); + expectTypeOf().not.toHaveProperty('duration'); + expectTypeOf().not.toHaveProperty('assetType'); + }); + + it('Child hook entities contain only child-table columns; Base hook entities only base-table columns', async () => { + const db = await createTestClient(delegateSchema); + let childEntity: any; + let baseEntity: any; + + const client = db.$use({ + id: 'test', + onEntityMutation: { + Child: { + async afterEntityMutation(args: any) { + childEntity = (await args.loadAfterMutationEntities())?.[0]; + }, + }, + Base: { + async afterEntityMutation(args: any) { + baseEntity = (await args.loadAfterMutationEntities())?.[0]; + }, + }, + }, + }); + + const created = await client.child.create({ data: { orgId: 1, childField: 10 } }); + + const qbChild = await db.$qb + .selectFrom('Child') + .selectAll() + .where('id', '=', created.id) + .executeTakeFirstOrThrow(); + const qbBase = await db.$qb + .selectFrom('Base') + .selectAll() + .where('id', '=', created.id) + .executeTakeFirstOrThrow(); + + expect(childEntity).toEqual(qbChild); + expect(childEntity).toEqual({ id: created.id, childField: 10 }); + + expect(baseEntity).toEqual(qbBase); + expect(baseEntity).toEqual({ id: created.id, orgId: 1, type: 'Child' }); + + await db.$disconnect(); + }); + + it("fires per-model hooks once per table written, keyed by that table's model", async () => { + const db = await createTestClient(delegateSchema); + const events: string[] = []; + + const client = db.$use({ + id: 'test', + onEntityMutation: { + Base: { + afterEntityMutation(args: any) { + events.push(`Base:${args.action}`); + }, + }, + Child: { + afterEntityMutation(args: any) { + events.push(`Child:${args.action}`); + }, + }, + }, + }); + + // create writes both tables + const created = await client.child.create({ data: { orgId: 1, childField: 10 } }); + expect(events).toEqual(['Base:create', 'Child:create']); + + // updating only a sub-model field writes only the sub-model table + events.length = 0; + await client.child.update({ where: { id: created.id }, data: { childField: 11 } }); + expect(events).toEqual(['Child:update']); + + // updating only an inherited field writes only the base table, so only + // the base model's hook fires + events.length = 0; + await client.child.update({ where: { id: created.id }, data: { orgId: 2 } }); + expect(events).toEqual(['Base:update']); + + // updating both writes both tables + events.length = 0; + await client.child.update({ where: { id: created.id }, data: { orgId: 3, childField: 12 } }); + expect(events).toEqual(['Base:update', 'Child:update']); + + // deleting a sub-model entity deletes the base row (sub-model row is cascaded), + // so only the base model's hook fires + events.length = 0; + await client.child.delete({ where: { id: created.id } }); + expect(events).toEqual(['Base:delete']); + + await db.$disconnect(); + }); + + it('base model hook entities carry the discriminator so the concrete sub-model can be identified', async () => { + const db = await createTestClient(delegateSchema); + const types: string[] = []; + + const client = db.$use({ + id: 'test', + onEntityMutation: { + Base: { + async beforeEntityMutation(args: any) { + const entities = await args.loadBeforeMutationEntities(); + if (entities) { + types.push(...entities.map((e: any) => `${args.action}:${e.type}`)); + } + }, + async afterEntityMutation(args: any) { + const entities = await args.loadAfterMutationEntities(); + if (entities) { + types.push(...entities.map((e: any) => `${args.action}:${e.type}`)); + } + }, + }, + }, + }); + + const created = await client.child.create({ data: { orgId: 1, childField: 10 } }); + await client.child.update({ where: { id: created.id }, data: { orgId: 2 } }); + await client.child.delete({ where: { id: created.id } }); + + expect(types).toEqual(['create:Child', 'update:Child', 'update:Child', 'delete:Child']); + + await db.$disconnect(); + }); + + it('update on child model succeeds with afterEntityMutation plugin', async () => { + const client = await createTestClient(delegateSchema); const withPlugin = client.$use({ id: 'test',