From 0defd21ba3e8ffb86b00b594b9938e75935f1245 Mon Sep 17 00:00:00 2001 From: ymc9 <104139426+ymc9@users.noreply.github.com> Date: Sat, 24 Feb 2024 10:32:26 -0800 Subject: [PATCH 1/2] fix(zmodel): check optionality consistency between relation and fk fields --- .../validator/datamodel-validator.ts | 24 ++++++++++++++--- .../tests/regression/issue-1014.test.ts | 3 +-- .../tests/regression/issue-177.test.ts | 27 +++++++++++++++++++ 3 files changed, 48 insertions(+), 6 deletions(-) create mode 100644 tests/integration/tests/regression/issue-177.test.ts diff --git a/packages/schema/src/language-server/validator/datamodel-validator.ts b/packages/schema/src/language-server/validator/datamodel-validator.ts index 379d4e46a..fe268667b 100644 --- a/packages/schema/src/language-server/validator/datamodel-validator.ts +++ b/packages/schema/src/language-server/validator/datamodel-validator.ts @@ -64,8 +64,8 @@ export default class DataModelValidator implements AstValidator { } const isArray = idField.type.array; - const isScalar = SCALAR_TYPES.includes(idField.type.type as typeof SCALAR_TYPES[number]) - const isValidType = isScalar || isEnum(idField.type.reference?.ref) + const isScalar = SCALAR_TYPES.includes(idField.type.type as (typeof SCALAR_TYPES)[number]); + const isValidType = isScalar || isEnum(idField.type.reference?.ref); if (isArray || !isValidType) { accept('error', 'Field with @id attribute must be of scalar or enum type', { node: idField }); @@ -121,7 +121,7 @@ export default class DataModelValidator implements AstValidator { fields = (arg.value as ArrayExpr).items as ReferenceExpr[]; if (fields.length === 0) { if (accept) { - accept('error', `"fields" value cannot be emtpy`, { + accept('error', `"fields" value cannot be empty`, { node: arg, }); } @@ -131,7 +131,7 @@ export default class DataModelValidator implements AstValidator { references = (arg.value as ArrayExpr).items as ReferenceExpr[]; if (references.length === 0) { if (accept) { - accept('error', `"references" value cannot be emtpy`, { + accept('error', `"references" value cannot be empty`, { node: arg, }); } @@ -148,6 +148,7 @@ export default class DataModelValidator implements AstValidator { if (accept) { accept('error', `Both "fields" and "references" must be provided`, { node: relAttr }); } + valid = false; } } else { // validate "fields" and "references" typing consistency @@ -157,14 +158,28 @@ export default class DataModelValidator implements AstValidator { } } else { for (let i = 0; i < fields.length; i++) { + if (!field.type.optional && fields[i].$resolvedType?.nullable) { + // if relation is not optional, then fk field must not be nullable + if (accept) { + accept( + 'error', + `relation "${field.name}" is not optional, but field "${fields[i].target.$refText}" is optional`, + { node: fields[i].target.ref! } + ); + } + valid = false; + } + if (!fields[i].$resolvedType) { if (accept) { accept('error', `field reference is unresolved`, { node: fields[i] }); + valid = false; } } if (!references[i].$resolvedType) { if (accept) { accept('error', `field reference is unresolved`, { node: references[i] }); + valid = false; } } @@ -176,6 +191,7 @@ export default class DataModelValidator implements AstValidator { accept('error', `values of "references" and "fields" must have the same type`, { node: relAttr, }); + valid = false; } } } diff --git a/tests/integration/tests/regression/issue-1014.test.ts b/tests/integration/tests/regression/issue-1014.test.ts index 7f374d24d..ad862db42 100644 --- a/tests/integration/tests/regression/issue-1014.test.ts +++ b/tests/integration/tests/regression/issue-1014.test.ts @@ -19,8 +19,7 @@ describe('issue 1014', () => { @@allow('read', true) } - `, - { logPrismaQuery: true } + ` ); const db = enhance(); diff --git a/tests/integration/tests/regression/issue-177.test.ts b/tests/integration/tests/regression/issue-177.test.ts new file mode 100644 index 000000000..d270580c5 --- /dev/null +++ b/tests/integration/tests/regression/issue-177.test.ts @@ -0,0 +1,27 @@ +import { loadModelWithError } from '@zenstackhq/testtools'; + +describe('issue 177', () => { + it('regression', async () => { + await expect( + loadModelWithError( + ` + model Foo { + id String @id @default(cuid()) + + bar Bar @relation(fields: [barId1, barId2], references: [id1, id2]) + barId1 String? + barId2 String + } + + model Bar { + id1 String @default(cuid()) + id2 String @default(cuid()) + foos Foo[] + + @@id([id1, id2]) + } + ` + ) + ).resolves.toContain('relation "bar" is not optional, but field "barId1" is optional'); + }); +}); From 18eb4f2a36d749b4388f887aa35105d92035605b Mon Sep 17 00:00:00 2001 From: ymc9 <104139426+ymc9@users.noreply.github.com> Date: Sat, 24 Feb 2024 10:36:51 -0800 Subject: [PATCH 2/2] update --- .../src/language-server/validator/datamodel-validator.ts | 5 ----- 1 file changed, 5 deletions(-) diff --git a/packages/schema/src/language-server/validator/datamodel-validator.ts b/packages/schema/src/language-server/validator/datamodel-validator.ts index fe268667b..09af0971c 100644 --- a/packages/schema/src/language-server/validator/datamodel-validator.ts +++ b/packages/schema/src/language-server/validator/datamodel-validator.ts @@ -148,7 +148,6 @@ export default class DataModelValidator implements AstValidator { if (accept) { accept('error', `Both "fields" and "references" must be provided`, { node: relAttr }); } - valid = false; } } else { // validate "fields" and "references" typing consistency @@ -167,19 +166,16 @@ export default class DataModelValidator implements AstValidator { { node: fields[i].target.ref! } ); } - valid = false; } if (!fields[i].$resolvedType) { if (accept) { accept('error', `field reference is unresolved`, { node: fields[i] }); - valid = false; } } if (!references[i].$resolvedType) { if (accept) { accept('error', `field reference is unresolved`, { node: references[i] }); - valid = false; } } @@ -191,7 +187,6 @@ export default class DataModelValidator implements AstValidator { accept('error', `values of "references" and "fields" must have the same type`, { node: relAttr, }); - valid = false; } } }