Skip to content

fix: invalid query sent to Prisma when doing nested update with multi-id #553

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 5, 2023
Merged

fix: invalid query sent to Prisma when doing nested update with multi-id #553

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 11 additions & 11 deletions packages/runtime/src/enhancements/nested-write-vistor.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { resolveField } from './model-meta';
import { ModelMeta } from './types';
import { enumerate, getModelFields } from './utils';

type NestingPathItem = { field?: FieldInfo; where: any; unique: boolean };
type NestingPathItem = { field?: FieldInfo; model: string; where: any; unique: boolean };

/**
* Context for visiting
Expand Down Expand Up @@ -113,7 +113,7 @@ export class NestedWriteVisitor {
// visit payload
switch (action) {
case 'create':
context.nestingPath.push({ field, where: {}, unique: false });
context.nestingPath.push({ field, model, where: {}, unique: false });
for (const item of enumerate(data)) {
if (this.callback.create) {
await this.callback.create(model, item, context);
Expand All @@ -125,7 +125,7 @@ export class NestedWriteVisitor {
case 'createMany':
// skip the 'data' layer so as to keep consistency with 'create'
if (data.data) {
context.nestingPath.push({ field, where: {}, unique: false });
context.nestingPath.push({ field, model, where: {}, unique: false });
for (const item of enumerate(data.data)) {
if (this.callback.create) {
await this.callback.create(model, item, context);
Expand All @@ -136,7 +136,7 @@ export class NestedWriteVisitor {
break;

case 'connectOrCreate':
context.nestingPath.push({ field, where: data.where, unique: true });
context.nestingPath.push({ field, model, where: data.where, unique: true });
for (const item of enumerate(data)) {
if (this.callback.connectOrCreate) {
await this.callback.connectOrCreate(model, item, context);
Expand All @@ -150,7 +150,7 @@ export class NestedWriteVisitor {
for (const item of enumerate(data)) {
const newContext = {
...context,
nestingPath: [...context.nestingPath, { field, where: item, unique: true }],
nestingPath: [...context.nestingPath, { field, model, where: item, unique: true }],
};
await this.callback.connect(model, item, newContext);
}
Expand All @@ -167,7 +167,7 @@ export class NestedWriteVisitor {
...context,
nestingPath: [
...context.nestingPath,
{ field, where: item, unique: typeof item === 'object' },
{ field, model, where: item, unique: typeof item === 'object' },
],
};
await this.callback.disconnect(model, item, newContext);
Expand All @@ -176,7 +176,7 @@ export class NestedWriteVisitor {
break;

case 'update':
context.nestingPath.push({ field, where: data.where, unique: false });
context.nestingPath.push({ field, model, where: data.where, unique: false });
for (const item of enumerate(data)) {
if (this.callback.update) {
await this.callback.update(model, item, context);
Expand All @@ -187,7 +187,7 @@ export class NestedWriteVisitor {
break;

case 'updateMany':
context.nestingPath.push({ field, where: data.where, unique: false });
context.nestingPath.push({ field, model, where: data.where, unique: false });
for (const item of enumerate(data)) {
if (this.callback.updateMany) {
await this.callback.updateMany(model, item, context);
Expand All @@ -197,7 +197,7 @@ export class NestedWriteVisitor {
break;

case 'upsert': {
context.nestingPath.push({ field, where: data.where, unique: true });
context.nestingPath.push({ field, model, where: data.where, unique: true });
for (const item of enumerate(data)) {
if (this.callback.upsert) {
await this.callback.upsert(model, item, context);
Expand All @@ -210,7 +210,7 @@ export class NestedWriteVisitor {

case 'delete': {
if (this.callback.delete) {
context.nestingPath.push({ field, where: data.where, unique: false });
context.nestingPath.push({ field, model, where: data.where, unique: false });
for (const item of enumerate(data)) {
await this.callback.delete(model, item, context);
}
Expand All @@ -220,7 +220,7 @@ export class NestedWriteVisitor {

case 'deleteMany':
if (this.callback.deleteMany) {
context.nestingPath.push({ field, where: data.where, unique: false });
context.nestingPath.push({ field, model, where: data.where, unique: false });
for (const item of enumerate(data)) {
await this.callback.deleteMany(model, item, context);
}
Expand Down
24 changes: 21 additions & 3 deletions packages/runtime/src/enhancements/policy/policy-utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -443,11 +443,18 @@ export class PolicyUtil {
let currField: FieldInfo | undefined;

for (let i = context.nestingPath.length - 1; i >= 0; i--) {
const { field, where, unique } = context.nestingPath[i];
const { field, model, where, unique } = context.nestingPath[i];

// never modify the original where because it's shared in the structure
const visitWhere = { ...where };
if (model && where) {
// make sure composite unique condition is flattened
await this.flattenGeneratedUniqueField(model, visitWhere);
}

if (!result) {
// first segment (bottom), just use its where clause
result = currQuery = { ...where };
result = currQuery = { ...visitWhere };
currField = field;
} else {
if (!currField) {
Expand All @@ -456,7 +463,13 @@ export class PolicyUtil {
if (!currField.backLink) {
throw this.unknownError(`field ${currField.type}.${currField.name} doesn't have a backLink`);
}
currQuery[currField.backLink] = { ...where };
const backLinkField = this.getModelField(currField.type, currField.backLink);
if (backLinkField?.isArray) {
// many-side of relationship, wrap with "some" query
currQuery[currField.backLink] = { some: { ...visitWhere } };
} else {
currQuery[currField.backLink] = { ...visitWhere };
}
currQuery = currQuery[currField.backLink];
currField = field;
}
Expand Down Expand Up @@ -707,6 +720,11 @@ export class PolicyUtil {
}
}

private getModelField(model: string, backlinkField: string) {
model = lowerCaseFirst(model);
return this.modelMeta.fields[model]?.[backlinkField];
}

private transaction(db: DbClientContract, action: (tx: Record<string, DbOperations>) => Promise<any>) {
if (db.__zenstack_tx) {
// already in transaction, don't nest
Expand Down
109 changes: 109 additions & 0 deletions tests/integration/tests/regression/issues.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,4 +191,113 @@ describe('GitHub issues regression', () => {
`
);
});

it('issue 552', async () => {
const { withPolicy, prisma } = await loadSchema(
`
model Tenant {
id Int @id @default(autoincrement())
name String

created_at DateTime @default(now())
updated_at DateTime @updatedAt

users UserTenant[]

@@map("tenants")


@@allow('all', auth().is_super_admin == true)
@@allow('read', users?[user == auth() && status == 'ACTIVE' ])
@@allow('all', users?[user == auth() && status == 'ACTIVE'])
}

model User {
id Int @id @default(autoincrement())
name String
is_super_admin Boolean @default(false) @omit

created_at DateTime @default(now())
updated_at DateTime @updatedAt

associated_tenants UserTenant[]

@@map("users")

@@allow('read', auth().id == id)
@@allow('all', auth().is_super_admin == true )
@@allow('read', associated_tenants?[tenant.users?[user == auth() && status == 'ACTIVE']])
@@allow('all', associated_tenants?[tenant.users?[user == auth() && status == 'ACTIVE']] )
@@allow('create', associated_tenants?[tenant.users?[user == auth() && status == 'ACTIVE']] )
@@allow('update', associated_tenants?[tenant.users?[user == auth() && status == 'ACTIVE']] )
}

model UserTenant {
user_id Int
user User @relation(fields: [user_id], references: [id], onDelete: Cascade, onUpdate: Cascade)

tenant_id Int
tenant Tenant @relation(fields: [tenant_id], references: [id], onDelete: Cascade, onUpdate: Cascade)

status String @default('INACTIVE')

@@map("user_tenants")

@@id([user_id, tenant_id])

@@index([user_id])
@@index([tenant_id])
@@index([user_id, tenant_id])

@@allow('all', auth().is_super_admin == true )
@@allow('read', tenant.users?[user == auth() && status == 'ACTIVE' ])
@@allow('all', tenant.users?[user == auth() && status == 'ACTIVE'])
@@allow('update', tenant.users?[user == auth() && status == 'ACTIVE'])
@@allow('delete', tenant.users?[user == auth() && status == 'ACTIVE'])
@@allow('create', tenant.users?[user == auth() && status == 'ACTIVE'])
}
`
);

await prisma.user.deleteMany();
await prisma.tenant.deleteMany();

await prisma.tenant.create({
data: {
id: 1,
name: 'tenant 1',
},
});

await prisma.user.create({
data: {
id: 1,
name: 'user 1',
},
});

await prisma.userTenant.create({
data: {
user_id: 1,
tenant_id: 1,
},
});

const db = withPolicy({ id: 1, is_super_admin: true });
await db.userTenant.update({
where: {
user_id_tenant_id: {
user_id: 1,
tenant_id: 1,
},
},
data: {
user: {
update: {
name: 'user 1 updated',
},
},
},
});
});
});
117 changes: 117 additions & 0 deletions tests/integration/tests/with-policy/multi-id-fields.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,4 +153,121 @@ describe('With Policy: multiple id fields', () => {
).toBeRejectedByPolicy();
await expect(db.q.create({ data: { owner: { connect: { x_y: { x: '1', y: '2' } } } } })).toResolveTruthy();
});

it('multi-id to-one nested write', async () => {
const { withPolicy } = await loadSchema(
`
model A {
x Int
y Int
v Int
b B @relation(fields: [bId], references: [id])
bId Int @unique

@@id([x, y])
@@allow('all', v > 0)
}

model B {
id Int @id
v Int
a A?

@@allow('all', v > 0)
}
`
);
const db = withPolicy();
await expect(
db.b.create({
data: {
id: 1,
v: 1,
a: {
create: {
x: 1,
y: 2,
v: 3,
},
},
},
})
).toResolveTruthy();

await expect(
db.a.update({
where: { x_y: { x: 1, y: 2 } },
data: { b: { update: { v: 5 } } },
})
).toResolveTruthy();

expect(await db.b.findUnique({ where: { id: 1 } })).toEqual(expect.objectContaining({ v: 5 }));
});

it('multi-id to-many nested write', async () => {
const { withPolicy } = await loadSchema(
`
model A {
x Int
y Int
v Int
b B @relation(fields: [bId], references: [id])
bId Int @unique

@@id([x, y])
@@allow('all', v > 0)
}

model B {
id Int @id
v Int
a A[]
c C?

@@allow('all', v > 0)
}

model C {
id Int @id
v Int
b B @relation(fields: [bId], references: [id])
bId Int @unique

@@allow('all', v > 0)
}
`
);
const db = withPolicy();
await expect(
db.b.create({
data: {
id: 1,
v: 1,
a: {
create: {
x: 1,
y: 2,
v: 2,
},
},
c: {
create: {
id: 1,
v: 3,
},
},
},
})
).toResolveTruthy();

await expect(
db.a.update({
where: { x_y: { x: 1, y: 2 } },
data: { b: { update: { v: 5, c: { update: { v: 6 } } } } },
})
).toResolveTruthy();

expect(await db.b.findUnique({ where: { id: 1 } })).toEqual(expect.objectContaining({ v: 5 }));
expect(await db.c.findUnique({ where: { id: 1 } })).toEqual(expect.objectContaining({ v: 6 }));
});
});