Skip to content

Commit e8268d0

Browse files
ymc9ymc9
authored
fix: properly handle missing fields when evaluating @@validate model-level rules (#1097)
1 parent 0f9f025 commit e8268d0

File tree

6 files changed

+255
-16
lines changed

6 files changed

+255
-16
lines changed

‎packages/schema/src/plugins/zod/utils/schema-gen.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import {
44
getAttributeArg,
55
getAttributeArgLiteral,
66
getLiteral,
7+
isDataModelFieldReference,
78
isFromStdlib,
89
} from '@zenstackhq/sdk';
910
import {
@@ -205,10 +206,17 @@ export function makeValidationRefinements(model: DataModel) {
205206
const message = messageArg ? `, { message: ${JSON.stringify(messageArg)} }` : '';
206207

207208
try {
208-
const expr = new TypeScriptExpressionTransformer({
209+
let expr = new TypeScriptExpressionTransformer({
209210
context: ExpressionContext.ValidationRule,
210211
fieldReferenceContext: 'value',
211212
}).transform(valueArg);
213+
214+
if (isDataModelFieldReference(valueArg)) {
215+
// if the expression is a simple field reference, treat undefined
216+
// as true since the all fields are optional in validation context
217+
expr = `${expr} ?? true`;
218+
}
219+
212220
return `.refine((value: any) => ${expr}${message})`;
213221
} catch (err) {
214222
if (err instanceof TypeScriptExpressionTransformerError) {

‎packages/schema/src/utils/typescript-expression-transformer.ts

Lines changed: 38 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ import {
1717
ThisExpr,
1818
UnaryExpr,
1919
} from '@zenstackhq/language/ast';
20-
import { ExpressionContext, getLiteral, isFromStdlib, isFutureExpr } from '@zenstackhq/sdk';
20+
import { ExpressionContext, getLiteral, isDataModelFieldReference, isFromStdlib, isFutureExpr } from '@zenstackhq/sdk';
2121
import { match, P } from 'ts-pattern';
2222
import { getIdFields } from './ast-utils';
2323

@@ -258,7 +258,13 @@ export class TypeScriptExpressionTransformer {
258258
}
259259

260260
private ensureBoolean(expr: string) {
261-
return `(${expr} ?? false)`;
261+
if (this.options.context === ExpressionContext.ValidationRule) {
262+
// all fields are optional in a validation context, so we treat undefined
263+
// as boolean true
264+
return `(${expr} ?? true)`;
265+
} else {
266+
return `(${expr} ?? false)`;
267+
}
262268
}
263269

264270
// #endregion
@@ -300,8 +306,18 @@ export class TypeScriptExpressionTransformer {
300306
}
301307
}
302308

303-
private unary(expr: UnaryExpr, normalizeUndefined: boolean): string {
304-
return `(${expr.operator} ${this.transform(expr.operand, normalizeUndefined)})`;
309+
private unary(expr: UnaryExpr, normalizeUndefined: boolean) {
310+
const operand = this.transform(expr.operand, normalizeUndefined);
311+
let result = `(${expr.operator} ${operand})`;
312+
if (
313+
expr.operator === '!' &&
314+
this.options.context === ExpressionContext.ValidationRule &&
315+
isDataModelFieldReference(expr.operand)
316+
) {
317+
// in a validation context, we treat unary involving undefined as boolean true
318+
result = `(${operand} !== undefined ? (${result}): true)`;
319+
}
320+
return result;
305321
}
306322

307323
private isModelType(expr: Expression) {
@@ -316,16 +332,24 @@ export class TypeScriptExpressionTransformer {
316332
left = `(${left}?.id ?? null)`;
317333
right = `(${right}?.id ?? null)`;
318334
}
319-
const _default = `(${left} ${expr.operator} ${right})`;
335+
336+
let _default = `(${left} ${expr.operator} ${right})`;
337+
338+
if (this.options.context === ExpressionContext.ValidationRule) {
339+
// in a validation context, we treat binary involving undefined as boolean true
340+
if (isDataModelFieldReference(expr.left)) {
341+
_default = `(${left} !== undefined ? (${_default}): true)`;
342+
}
343+
if (isDataModelFieldReference(expr.right)) {
344+
_default = `(${right} !== undefined ? (${_default}): true)`;
345+
}
346+
}
320347

321348
return match(expr.operator)
322-
.with(
323-
'in',
324-
() =>
325-
`(${this.transform(expr.right, false)}?.includes(${this.transform(
326-
expr.left,
327-
normalizeUndefined
328-
)}) ?? false)`
349+
.with('in', () =>
350+
this.ensureBoolean(
351+
`${this.transform(expr.right, false)}?.includes(${this.transform(expr.left, normalizeUndefined)})`
352+
)
329353
)
330354
.with(P.union('==', '!='), () => {
331355
if (isThisExpr(expr.left) || isThisExpr(expr.right)) {
@@ -363,8 +387,8 @@ export class TypeScriptExpressionTransformer {
363387
const predicate = innerTransformer.transform(expr.right, normalizeUndefined);
364388

365389
return match(operator)
366-
.with('?', () => `!!((${operand})?.some((_item: any) => ${predicate}))`)
367-
.with('!', () => `!!((${operand})?.every((_item: any) => ${predicate}))`)
390+
.with('?', () => this.ensureBoolean(`(${operand})?.some((_item: any) => ${predicate})`))
391+
.with('!', () => this.ensureBoolean(`(${operand})?.every((_item: any) => ${predicate})`))
368392
.with('^', () => `!((${operand})?.some((_item: any) => ${predicate}))`)
369393
.exhaustive();
370394
}

‎packages/testtools/src/schema.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ plugin policy {
105105
106106
plugin zod {
107107
provider = '@core/zod'
108-
// preserveTsFiles = true
108+
preserveTsFiles = true
109109
modelOnly = ${!options.fullZod}
110110
}
111111
`;

‎tests/integration/tests/cli/plugins.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,7 @@ describe('CLI Plugins Tests', () => {
116116
strict: true,
117117
lib: ['esnext', 'dom'],
118118
esModuleInterop: true,
119+
skipLibCheck: true,
119120
},
120121
})
121122
);

‎tests/integration/tests/enhancements/with-policy/field-validation.test.ts

Lines changed: 154 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -609,3 +609,157 @@ describe('With Policy: field validation', () => {
609609
expect(u.tasks[0]).toMatchObject({ slug: 'slug2' });
610610
});
611611
});
612+
613+
describe('With Policy: model-level validation', () => {
614+
it('create', async () => {
615+
const { enhance } = await loadSchema(`
616+
model Model {
617+
id Int @id @default(autoincrement())
618+
x Int
619+
y Int
620+
621+
@@validate(x > 0)
622+
@@validate(x >= y)
623+
@@allow('all', true)
624+
}
625+
`);
626+
627+
const db = enhance();
628+
629+
await expect(db.model.create({ data: { x: 0, y: 0 } })).toBeRejectedByPolicy();
630+
await expect(db.model.create({ data: { x: 1, y: 2 } })).toBeRejectedByPolicy();
631+
await expect(db.model.create({ data: { x: 2, y: 1 } })).toResolveTruthy();
632+
});
633+
634+
it('update', async () => {
635+
const { enhance } = await loadSchema(`
636+
model Model {
637+
id Int @id @default(autoincrement())
638+
x Int
639+
y Int
640+
641+
@@validate(x >= y)
642+
@@allow('all', true)
643+
}
644+
`);
645+
646+
const db = enhance();
647+
648+
await expect(db.model.create({ data: { x: 2, y: 1 } })).toResolveTruthy();
649+
await expect(db.model.create({ data: { x: 1, y: 2 } })).toBeRejectedByPolicy();
650+
});
651+
652+
it('int optionality', async () => {
653+
const { enhance } = await loadSchema(`
654+
model Model {
655+
id Int @id @default(autoincrement())
656+
x Int?
657+
658+
@@validate(x > 0)
659+
@@allow('all', true)
660+
}
661+
`);
662+
663+
const db = enhance();
664+
665+
await expect(db.model.create({ data: { x: 0 } })).toBeRejectedByPolicy();
666+
await expect(db.model.create({ data: { x: 1 } })).toResolveTruthy();
667+
await expect(db.model.create({ data: {} })).toResolveTruthy();
668+
});
669+
670+
it('boolean optionality', async () => {
671+
const { enhance } = await loadSchema(`
672+
model Model {
673+
id Int @id @default(autoincrement())
674+
x Boolean?
675+
676+
@@validate(x)
677+
@@allow('all', true)
678+
}
679+
`);
680+
681+
const db = enhance();
682+
683+
await expect(db.model.create({ data: { x: false } })).toBeRejectedByPolicy();
684+
await expect(db.model.create({ data: { x: true } })).toResolveTruthy();
685+
await expect(db.model.create({ data: {} })).toResolveTruthy();
686+
});
687+
688+
it('optionality with comparison', async () => {
689+
const { enhance } = await loadSchema(`
690+
model Model {
691+
id Int @id @default(autoincrement())
692+
x Int?
693+
y Int?
694+
695+
@@validate(x > y)
696+
@@allow('all', true)
697+
}
698+
`);
699+
700+
const db = enhance();
701+
702+
await expect(db.model.create({ data: { x: 1, y: 2 } })).toBeRejectedByPolicy();
703+
await expect(db.model.create({ data: { x: 1 } })).toResolveTruthy();
704+
await expect(db.model.create({ data: { y: 1 } })).toResolveTruthy();
705+
await expect(db.model.create({ data: {} })).toResolveTruthy();
706+
});
707+
708+
it('optionality with complex expression', async () => {
709+
const { enhance } = await loadSchema(`
710+
model Model {
711+
id Int @id @default(autoincrement())
712+
x Int?
713+
y Int?
714+
715+
@@validate(y > 1 && x > y)
716+
@@allow('all', true)
717+
}
718+
`);
719+
720+
const db = enhance();
721+
722+
await expect(db.model.create({ data: { y: 1 } })).toBeRejectedByPolicy();
723+
await expect(db.model.create({ data: { y: 2 } })).toResolveTruthy();
724+
await expect(db.model.create({ data: {} })).toResolveTruthy();
725+
await expect(db.model.create({ data: { x: 1, y: 2 } })).toBeRejectedByPolicy();
726+
await expect(db.model.create({ data: { x: 3, y: 2 } })).toResolveTruthy();
727+
});
728+
729+
it('optionality with negation', async () => {
730+
const { enhance } = await loadSchema(`
731+
model Model {
732+
id Int @id @default(autoincrement())
733+
x Boolean?
734+
735+
@@validate(!x)
736+
@@allow('all', true)
737+
}
738+
`);
739+
740+
const db = enhance();
741+
742+
await expect(db.model.create({ data: { x: true } })).toBeRejectedByPolicy();
743+
await expect(db.model.create({ data: { x: false } })).toResolveTruthy();
744+
await expect(db.model.create({ data: {} })).toResolveTruthy();
745+
});
746+
747+
it('update implied optionality', async () => {
748+
const { enhance } = await loadSchema(`
749+
model Model {
750+
id Int @id @default(autoincrement())
751+
x Int
752+
y Int
753+
754+
@@validate(x > y)
755+
@@allow('all', true)
756+
}
757+
`);
758+
759+
const db = enhance();
760+
761+
await expect(db.model.create({ data: { id: 1, x: 2, y: 1 } })).toResolveTruthy();
762+
await expect(db.model.update({ where: { id: 1 }, data: { y: 1 } })).toResolveTruthy();
763+
await expect(db.model.update({ where: { id: 1 }, data: {} })).toResolveTruthy();
764+
});
765+
});

‎tests/integration/tests/regression/issue-1078.test.ts

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
import { loadSchema } from '@zenstackhq/testtools';
2+
3+
describe('issue 1078', () => {
4+
it('regression', async () => {
5+
const { prisma, enhance } = await loadSchema(
6+
`
7+
model Counter {
8+
id String @id
9+
10+
name String
11+
value Int
12+
13+
@@validate(value >= 0)
14+
@@allow('all', true)
15+
}
16+
`
17+
);
18+
19+
const db = enhance();
20+
21+
const counter = await db.counter.create({
22+
data: { id: '1', name: 'It should create', value: 1 },
23+
});
24+
25+
//! This query fails validation
26+
const updated = await db.counter.update({
27+
where: { id: '1' },
28+
data: { name: 'It should update' },
29+
});
30+
});
31+
32+
it('read', async () => {
33+
const { prisma, enhance } = await loadSchema(
34+
`
35+
model Post {
36+
id Int @id() @default(autoincrement())
37+
title String @allow('read', true, true)
38+
content String
39+
}
40+
`,
41+
{ logPrismaQuery: true }
42+
);
43+
44+
const db = enhance();
45+
46+
const post = await prisma.post.create({ data: { title: 'Post1', content: 'Content' } });
47+
await expect(db.post.findUnique({ where: { id: post.id } })).toResolveNull();
48+
await expect(db.post.findUnique({ where: { id: post.id }, select: { title: true } })).resolves.toEqual({
49+
title: 'Post1',
50+
});
51+
});
52+
});

0 commit comments

Comments
 (0)