diff --git a/packages/orm/src/client/zod/factory.ts b/packages/orm/src/client/zod/factory.ts index 53014d4b1..288403078 100644 --- a/packages/orm/src/client/zod/factory.ts +++ b/packages/orm/src/client/zod/factory.ts @@ -176,6 +176,19 @@ export class ZodSchemaFactory< return Object.entries(modelDef.fields).filter(([, def]) => def.type !== 'Unsupported'); } + /** + * Maps each relation field of a model to the foreign-key fields it sets. + */ + private getRelationForeignKeys(model: string): Record { + const result: Record = {}; + for (const [name, fieldDef] of this.getModelFields(model)) { + if (fieldDef.relation?.fields?.length) { + result[name] = fieldDef.relation.fields; + } + } + return result; + } + private shouldIncludeRelations(options?: CreateSchemaOptions): boolean { return options?.relationDepth === undefined || options.relationDepth > 0; } @@ -2117,11 +2130,23 @@ export class ZodSchemaFactory< } }); + // an update payload holds only the fields it changes + const partialOptions = { partial: true, relationForeignKeys: this.getRelationForeignKeys(model) }; const uncheckedUpdateSchema = this.extraValidationsEnabled - ? ZodUtils.addCustomValidation(z.strictObject(uncheckedVariantFields), modelDef.attributes) + ? ZodUtils.addCustomValidation( + z.strictObject(uncheckedVariantFields), + modelDef.attributes, + undefined, + partialOptions, + ) : z.strictObject(uncheckedVariantFields); const checkedUpdateSchema = this.extraValidationsEnabled - ? ZodUtils.addCustomValidation(z.strictObject(checkedVariantFields), modelDef.attributes) + ? ZodUtils.addCustomValidation( + z.strictObject(checkedVariantFields), + modelDef.attributes, + undefined, + partialOptions, + ) : z.strictObject(checkedVariantFields); const result = !hasRelation ? uncheckedUpdateSchema : z.union([uncheckedUpdateSchema, checkedUpdateSchema]); diff --git a/packages/zod/src/utils.ts b/packages/zod/src/utils.ts index db773096e..a8309d292 100644 --- a/packages/zod/src/utils.ts +++ b/packages/zod/src/utils.ts @@ -307,8 +307,8 @@ export function addListValidation( } /** - * Recursively collects all field names referenced by `kind: 'field'` nodes - * inside an expression tree. + * Recursively collects all field names referenced by `kind: 'field'` nodes and + * by `this.field` member nodes inside an expression tree. */ export function collectFieldRefs(expr: Expression): Set { const refs = new Set(); @@ -331,7 +331,12 @@ export function collectFieldRefs(expr: Expression): Set { e.items.forEach(walk); break; case 'member': - walk(e.receiver); + if (e.receiver.kind === 'this') { + // `this.field` references `field` + if (e.members[0]) refs.add(e.members[0]); + } else { + walk(e.receiver); + } break; // literal / null / this / binding — no field refs } @@ -351,6 +356,10 @@ export function collectFieldRefs(expr: Expression): Set { * Omit `presentFields` (or pass `undefined`) to apply all rules * unconditionally, which is the correct behaviour for full-model schemas. * + * Pass `options.partial` when the data is an update payload, which holds only + * the fields it changes. A rule is then skipped for data that sends none of + * the fields it references: the update cannot change the rule's result. + * * Note: `@@validate` conditions are restricted by the ZModel compiler to * scalar fields of the same model only — relation fields are a compile error. * A flat field-name set is therefore sufficient. @@ -359,6 +368,19 @@ export function addCustomValidation( schema: z.ZodSchema, attributes: readonly AttributeApplication[] | undefined, presentFields?: ReadonlySet, + options?: { + /** + * The data holds only some of the model's fields, as in an update. A rule is then + * skipped when the data sends none of the fields it references. + */ + partial?: boolean; + /** + * Maps each relation field to the foreign-key fields it sets. In partial mode, a + * relation field in the data counts as sending its foreign keys, unless it holds + * only a nested `update`, which cannot change them. + */ + relationForeignKeys?: Record; + }, ): z.ZodSchema { const attrs = attributes?.filter((a) => a.name === '@@validate'); if (!attrs || attrs.length === 0) { @@ -386,16 +408,71 @@ export function addCustomValidation( if (pathExpr && ExpressionUtils.isArray(pathExpr)) { path = pathExpr.items.map((e) => ExpressionUtils.getLiteralValue(e) as string); } - result = applyValidation(result, expr, message, path); + result = applyValidation( + result, + expr, + message, + path, + options?.partial ? partialTriggers(expr, options.relationForeignKeys) : undefined, + ); } return result; } +type PartialTriggers = { + /** The fields the rule references. */ + fields: string[]; + /** The relation fields that set a foreign key the rule references. */ + relations: string[]; +}; + +/** + * Returns the fields and relation fields that make a rule apply to partial data. + */ +function partialTriggers( + expr: Expression, + relationForeignKeys: Record = {}, +): PartialTriggers { + const refs = collectFieldRefs(expr); + return { + fields: [...refs], + relations: Object.entries(relationForeignKeys) + .filter(([, foreignKeys]) => foreignKeys.some((fk) => refs.has(fk))) + .map(([relation]) => relation), + }; +} + +/** + * Tells whether a relation payload can change the foreign keys of its relation. Every nested + * operation can, except `update`, which writes to the related row only. + */ +function relationChangesForeignKey(payload: unknown) { + return typeof payload === 'object' && payload !== null && Object.keys(payload).some((op) => op !== 'update'); +} + +/** + * Tells whether partial data sends a field or a foreign-key change that a rule depends on. + */ +function partialDataTriggersRule(data: unknown, triggers: PartialTriggers) { + if (typeof data !== 'object' || data === null) { + return false; + } + return ( + triggers.fields.some((field) => field in data) || + triggers.relations.some((relation) => relationChangesForeignKey((data as Record)[relation])) + ); +} + +/** + * Adds one `@@validate` rule to `schema` as a refinement. With `triggers`, the data is partial and + * the refinement passes for data that does not trigger the rule. + */ function applyValidation( schema: z.ZodSchema, expr: Expression, message: string | undefined, path: string[] | undefined, + triggers: PartialTriggers | undefined, ) { const options: Parameters[1] = {}; if (message) { @@ -404,7 +481,12 @@ function applyValidation( if (path) { options.path = path; } - return schema.refine((data) => Boolean(evalExpression(data, expr)), options); + return schema.refine((data) => { + if (triggers && !partialDataTriggersRule(data, triggers)) { + return true; + } + return Boolean(evalExpression(data, expr)); + }, options); } function evalExpression(data: any, expr: Expression): unknown { diff --git a/tests/regression/test/issue-2904.test.ts b/tests/regression/test/issue-2904.test.ts new file mode 100644 index 000000000..100fd73c0 --- /dev/null +++ b/tests/regression/test/issue-2904.test.ts @@ -0,0 +1,150 @@ +import { createTestClient } from '@zenstackhq/testtools'; +import { describe, expect, it } from 'vitest'; + +// https://github.com/zenstackhq/zenstack/issues/2904 +describe('Regression for issue 2904', () => { + const schema = ` +model User { + id Int @id @default(autoincrement()) + bots Bot[] +} + +model Bot { + id Int @id @default(autoincrement()) + name String + useCase String? + modelName String? + owner User? @relation(fields: [ownerId], references: [id]) + ownerId Int? + + @@validate(useCase != null || modelName != null, 'needs a use case or a model') +} +`; + + it('accepts an update that sends none of the fields a rule references', async () => { + const db = await createTestClient(schema); + const user = await db.user.create({ data: {} }); + const bot = await db.bot.create({ data: { name: 'a', useCase: 'chat', ownerId: user.id } }); + + await expect(db.bot.update({ where: { id: bot.id }, data: { name: 'b' } })).resolves.toMatchObject({ + name: 'b', + useCase: 'chat', + }); + await expect(db.bot.updateMany({ where: { id: bot.id }, data: { name: 'c' } })).resolves.toEqual({ + count: 1, + }); + await expect( + db.bot.upsert({ + where: { id: bot.id }, + create: { name: 'x', useCase: 'chat' }, + update: { name: 'd' }, + }), + ).resolves.toMatchObject({ name: 'd' }); + await expect( + db.user.update({ + where: { id: user.id }, + data: { bots: { update: { where: { id: bot.id }, data: { name: 'e' } } } }, + include: { bots: true }, + }), + ).resolves.toMatchObject({ bots: [{ name: 'e' }] }); + }); + + it('still checks a rule when the update sends a field it references', async () => { + const db = await createTestClient(schema); + const bot = await db.bot.create({ data: { name: 'a', useCase: 'chat' } }); + + await expect( + db.bot.update({ where: { id: bot.id }, data: { useCase: null, modelName: null } }), + ).toBeRejectedByValidation(['needs a use case or a model']); + await expect( + db.bot.update({ where: { id: bot.id }, data: { useCase: null, modelName: 'gpt' } }), + ).resolves.toMatchObject({ useCase: null, modelName: 'gpt' }); + }); + + it('still checks every rule on create', async () => { + const db = await createTestClient(schema); + await expect(db.bot.create({ data: { name: 'a' } })).toBeRejectedByValidation(['needs a use case or a model']); + }); + + it('checks a rule when a relation field sets a foreign key the rule references', async () => { + const db = await createTestClient(` +model User { + id Int @id @default(autoincrement()) + posts Post[] +} + +model Post { + id Int @id @default(autoincrement()) + title String + author User? @relation(fields: [authorId], references: [id]) + authorId Int? + + @@validate(authorId != null, 'needs an author') +} +`); + const user = await db.user.create({ data: {} }); + const post = await db.post.create({ data: { title: 'a', authorId: user.id } }); + + await expect(db.post.update({ where: { id: post.id }, data: { title: 'b' } })).resolves.toMatchObject({ + title: 'b', + }); + await expect( + db.post.update({ where: { id: post.id }, data: { author: { disconnect: true } } }), + ).toBeRejectedByValidation(['needs an author']); + }); + + it('accepts a nested update of a relation whose foreign key a rule references', async () => { + const db = await createTestClient(` +model User { + id Int @id @default(autoincrement()) + name String? + posts Post[] +} + +model Post { + id Int @id @default(autoincrement()) + title String + author User? @relation(fields: [authorId], references: [id]) + authorId Int? + + @@validate(authorId != null, 'needs an author') +} +`); + const user = await db.user.create({ data: {} }); + const post = await db.post.create({ data: { title: 'a', authorId: user.id } }); + + await expect( + db.post.update({ + where: { id: post.id }, + data: { author: { update: { name: 'x' } } }, + include: { author: true }, + }), + ).resolves.toMatchObject({ authorId: user.id, author: { name: 'x' } }); + await expect( + db.post.update({ + where: { id: post.id }, + data: { author: { update: { name: 'y' }, disconnect: true } }, + }), + ).toBeRejectedByValidation(['needs an author']); + }); + + it('checks a rule that references a field through this', async () => { + const db = await createTestClient(` +model Item { + id Int @id @default(autoincrement()) + name String + amount Int + + @@validate(this.amount > 0, 'amount must be positive') +} +`); + const item = await db.item.create({ data: { name: 'a', amount: 1 } }); + + await expect(db.item.update({ where: { id: item.id }, data: { name: 'b' } })).resolves.toMatchObject({ + name: 'b', + }); + await expect(db.item.update({ where: { id: item.id }, data: { amount: -1 } })).toBeRejectedByValidation([ + 'amount must be positive', + ]); + }); +});