From 58a6ddd3099b1ae0ed20ecbc117adb6c94cd5754 Mon Sep 17 00:00:00 2001 From: ymc9 <104139426+ymc9@users.noreply.github.com> Date: Tue, 12 Mar 2024 20:18:22 -0700 Subject: [PATCH 1/2] fix: make sure fields inherited from abstract base models are properly recognized as id --- packages/sdk/src/utils.ts | 9 +- .../tests/regression/issue-1129.test.ts | 86 +++++++++++++++++++ 2 files changed, 92 insertions(+), 3 deletions(-) create mode 100644 tests/integration/tests/regression/issue-1129.test.ts diff --git a/packages/sdk/src/utils.ts b/packages/sdk/src/utils.ts index d32962f11..a32abc068 100644 --- a/packages/sdk/src/utils.ts +++ b/packages/sdk/src/utils.ts @@ -218,11 +218,14 @@ export function isIdField(field: DataModelField) { return true; } + // NOTE: we have to use name to match fields because the fields + // may be inherited from an abstract base and have cloned identities + const model = field.$container as DataModel; // model-level @@id attribute with a list of fields const modelLevelIds = getModelIdFields(model); - if (modelLevelIds.includes(field)) { + if (modelLevelIds.map((f) => f.name).includes(field.name)) { return true; } @@ -234,12 +237,12 @@ export function isIdField(field: DataModelField) { // then, the first field with @unique can be used as id const firstUniqueField = model.fields.find((f) => hasAttribute(f, '@unique')); if (firstUniqueField) { - return firstUniqueField === field; + return firstUniqueField.name === field.name; } // last, the first model level @@unique can be used as id const modelLevelUnique = getModelUniqueFields(model); - if (modelLevelUnique.includes(field)) { + if (modelLevelUnique.map((f) => f.name).includes(field.name)) { return true; } diff --git a/tests/integration/tests/regression/issue-1129.test.ts b/tests/integration/tests/regression/issue-1129.test.ts new file mode 100644 index 000000000..28fc2ffd6 --- /dev/null +++ b/tests/integration/tests/regression/issue-1129.test.ts @@ -0,0 +1,86 @@ +import { createPostgresDb, dropPostgresDb, loadSchema } from '@zenstackhq/testtools'; + +describe('Regression for issue 1129', () => { + let dbUrl: string; + + beforeAll(async () => { + dbUrl = await createPostgresDb('regression-issue-1229'); + }); + + afterAll(async () => { + await dropPostgresDb('regression-issue-1229'); + }); + + it('regression', async () => { + const { enhance } = await loadSchema( + ` + model Relation1 { + id String @id @default(cuid()) + field1 String + concrete Concrete[] + @@allow('all', true) + } + + model Relation2 { + id String @id @default(cuid()) + field2 String + concrete Concrete[] + @@allow('all', true) + } + + abstract model WithRelation1 { + relation1Id String + relation1 Relation1 @relation(fields: [relation1Id], references: [id]) + } + abstract model WithRelation2 { + relation2Id String + relation2 Relation2 @relation(fields: [relation2Id], references: [id]) + } + + model Concrete extends WithRelation1, WithRelation2 { + concreteField String + @@id([relation1Id, relation2Id]) + @@allow('all', true) + } + `, + { provider: 'postgresql', dbUrl } + ); + + const db = enhance(); + + await db.$transaction(async (tx: any) => { + await tx.relation2.createMany({ + data: [ + { + id: 'relation2Id1', + field2: 'field2Value1', + }, + { + id: 'relation2Id2', + field2: 'field2Value2', + }, + ], + }); + + await tx.relation1.create({ + data: { + field1: 'field1Value', + concrete: { + createMany: { + data: [ + { + concreteField: 'concreteFieldValue1', + relation2Id: 'relation2Id1', + }, + { + concreteField: 'concreteFieldValue2', + relation2Id: 'relation2Id2', + }, + ], + }, + }, + }, + }); + }); + }); +}); From f9bc3c831f0d5d6983966baf406a4040d1837563 Mon Sep 17 00:00:00 2001 From: ymc9 <104139426+ymc9@users.noreply.github.com> Date: Tue, 12 Mar 2024 20:36:17 -0700 Subject: [PATCH 2/2] fix tests --- .../tests/regression/issue-1129.test.ts | 91 ++++++++++--------- 1 file changed, 46 insertions(+), 45 deletions(-) diff --git a/tests/integration/tests/regression/issue-1129.test.ts b/tests/integration/tests/regression/issue-1129.test.ts index 28fc2ffd6..49198a5cb 100644 --- a/tests/integration/tests/regression/issue-1129.test.ts +++ b/tests/integration/tests/regression/issue-1129.test.ts @@ -1,19 +1,13 @@ import { createPostgresDb, dropPostgresDb, loadSchema } from '@zenstackhq/testtools'; describe('Regression for issue 1129', () => { - let dbUrl: string; - - beforeAll(async () => { - dbUrl = await createPostgresDb('regression-issue-1229'); - }); - - afterAll(async () => { - await dropPostgresDb('regression-issue-1229'); - }); - it('regression', async () => { - const { enhance } = await loadSchema( - ` + let prisma; + const dbUrl = await createPostgresDb('regression-issue-1129'); + + try { + const r = await loadSchema( + ` model Relation1 { id String @id @default(cuid()) field1 String @@ -43,44 +37,51 @@ describe('Regression for issue 1129', () => { @@allow('all', true) } `, - { provider: 'postgresql', dbUrl } - ); + { provider: 'postgresql', dbUrl } + ); - const db = enhance(); + prisma = r.prisma; + const db = r.enhance(); - await db.$transaction(async (tx: any) => { - await tx.relation2.createMany({ - data: [ - { - id: 'relation2Id1', - field2: 'field2Value1', - }, - { - id: 'relation2Id2', - field2: 'field2Value2', - }, - ], - }); + await db.$transaction(async (tx: any) => { + await tx.relation2.createMany({ + data: [ + { + id: 'relation2Id1', + field2: 'field2Value1', + }, + { + id: 'relation2Id2', + field2: 'field2Value2', + }, + ], + }); - await tx.relation1.create({ - data: { - field1: 'field1Value', - concrete: { - createMany: { - data: [ - { - concreteField: 'concreteFieldValue1', - relation2Id: 'relation2Id1', - }, - { - concreteField: 'concreteFieldValue2', - relation2Id: 'relation2Id2', - }, - ], + await tx.relation1.create({ + data: { + field1: 'field1Value', + concrete: { + createMany: { + data: [ + { + concreteField: 'concreteFieldValue1', + relation2Id: 'relation2Id1', + }, + { + concreteField: 'concreteFieldValue2', + relation2Id: 'relation2Id2', + }, + ], + }, }, }, - }, + }); }); - }); + } finally { + if (prisma) { + await prisma.$disconnect(); + } + await dropPostgresDb('regression-issue-1129'); + } }); });