Skip to content

Commit 24760be

Browse files
ymc9ymc9
authored
fix: invalid query sent to Prisma when doing nested update with multi-id (#553)
1 parent 011053f commit 24760be

File tree

4 files changed

+258
-14
lines changed

4 files changed

+258
-14
lines changed

‎packages/runtime/src/enhancements/nested-write-vistor.ts‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import { resolveField } from './model-meta';
66
import { ModelMeta } from './types';
77
import { enumerate, getModelFields } from './utils';
88

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

1111
/**
1212
* Context for visiting
@@ -113,7 +113,7 @@ export class NestedWriteVisitor {
113113
// visit payload
114114
switch (action) {
115115
case 'create':
116-
context.nestingPath.push({ field, where: {}, unique: false });
116+
context.nestingPath.push({ field, model, where: {}, unique: false });
117117
for (const item of enumerate(data)) {
118118
if (this.callback.create) {
119119
await this.callback.create(model, item, context);
@@ -125,7 +125,7 @@ export class NestedWriteVisitor {
125125
case 'createMany':
126126
// skip the 'data' layer so as to keep consistency with 'create'
127127
if (data.data) {
128-
context.nestingPath.push({ field, where: {}, unique: false });
128+
context.nestingPath.push({ field, model, where: {}, unique: false });
129129
for (const item of enumerate(data.data)) {
130130
if (this.callback.create) {
131131
await this.callback.create(model, item, context);
@@ -136,7 +136,7 @@ export class NestedWriteVisitor {
136136
break;
137137

138138
case 'connectOrCreate':
139-
context.nestingPath.push({ field, where: data.where, unique: true });
139+
context.nestingPath.push({ field, model, where: data.where, unique: true });
140140
for (const item of enumerate(data)) {
141141
if (this.callback.connectOrCreate) {
142142
await this.callback.connectOrCreate(model, item, context);
@@ -150,7 +150,7 @@ export class NestedWriteVisitor {
150150
for (const item of enumerate(data)) {
151151
const newContext = {
152152
...context,
153-
nestingPath: [...context.nestingPath, { field, where: item, unique: true }],
153+
nestingPath: [...context.nestingPath, { field, model, where: item, unique: true }],
154154
};
155155
await this.callback.connect(model, item, newContext);
156156
}
@@ -167,7 +167,7 @@ export class NestedWriteVisitor {
167167
...context,
168168
nestingPath: [
169169
...context.nestingPath,
170-
{ field, where: item, unique: typeof item === 'object' },
170+
{ field, model, where: item, unique: typeof item === 'object' },
171171
],
172172
};
173173
await this.callback.disconnect(model, item, newContext);
@@ -176,7 +176,7 @@ export class NestedWriteVisitor {
176176
break;
177177

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

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

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

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

221221
case 'deleteMany':
222222
if (this.callback.deleteMany) {
223-
context.nestingPath.push({ field, where: data.where, unique: false });
223+
context.nestingPath.push({ field, model, where: data.where, unique: false });
224224
for (const item of enumerate(data)) {
225225
await this.callback.deleteMany(model, item, context);
226226
}

‎packages/runtime/src/enhancements/policy/policy-utils.ts‎

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -443,11 +443,18 @@ export class PolicyUtil {
443443
let currField: FieldInfo | undefined;
444444

445445
for (let i = context.nestingPath.length - 1; i >= 0; i--) {
446-
const { field, where, unique } = context.nestingPath[i];
446+
const { field, model, where, unique } = context.nestingPath[i];
447+
448+
// never modify the original where because it's shared in the structure
449+
const visitWhere = { ...where };
450+
if (model && where) {
451+
// make sure composite unique condition is flattened
452+
await this.flattenGeneratedUniqueField(model, visitWhere);
453+
}
447454

448455
if (!result) {
449456
// first segment (bottom), just use its where clause
450-
result = currQuery = { ...where };
457+
result = currQuery = { ...visitWhere };
451458
currField = field;
452459
} else {
453460
if (!currField) {
@@ -456,7 +463,13 @@ export class PolicyUtil {
456463
if (!currField.backLink) {
457464
throw this.unknownError(`field ${currField.type}.${currField.name} doesn't have a backLink`);
458465
}
459-
currQuery[currField.backLink] = { ...where };
466+
const backLinkField = this.getModelField(currField.type, currField.backLink);
467+
if (backLinkField?.isArray) {
468+
// many-side of relationship, wrap with "some" query
469+
currQuery[currField.backLink] = { some: { ...visitWhere } };
470+
} else {
471+
currQuery[currField.backLink] = { ...visitWhere };
472+
}
460473
currQuery = currQuery[currField.backLink];
461474
currField = field;
462475
}
@@ -707,6 +720,11 @@ export class PolicyUtil {
707720
}
708721
}
709722

723+
private getModelField(model: string, backlinkField: string) {
724+
model = lowerCaseFirst(model);
725+
return this.modelMeta.fields[model]?.[backlinkField];
726+
}
727+
710728
private transaction(db: DbClientContract, action: (tx: Record<string, DbOperations>) => Promise<any>) {
711729
if (db.__zenstack_tx) {
712730
// already in transaction, don't nest

‎tests/integration/tests/regression/issues.test.ts‎

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -191,4 +191,113 @@ describe('GitHub issues regression', () => {
191191
`
192192
);
193193
});
194+
195+
it('issue 552', async () => {
196+
const { withPolicy, prisma } = await loadSchema(
197+
`
198+
model Tenant {
199+
id Int @id @default(autoincrement())
200+
name String
201+
202+
created_at DateTime @default(now())
203+
updated_at DateTime @updatedAt
204+
205+
users UserTenant[]
206+
207+
@@map("tenants")
208+
209+
210+
@@allow('all', auth().is_super_admin == true)
211+
@@allow('read', users?[user == auth() && status == 'ACTIVE' ])
212+
@@allow('all', users?[user == auth() && status == 'ACTIVE'])
213+
}
214+
215+
model User {
216+
id Int @id @default(autoincrement())
217+
name String
218+
is_super_admin Boolean @default(false) @omit
219+
220+
created_at DateTime @default(now())
221+
updated_at DateTime @updatedAt
222+
223+
associated_tenants UserTenant[]
224+
225+
@@map("users")
226+
227+
@@allow('read', auth().id == id)
228+
@@allow('all', auth().is_super_admin == true )
229+
@@allow('read', associated_tenants?[tenant.users?[user == auth() && status == 'ACTIVE']])
230+
@@allow('all', associated_tenants?[tenant.users?[user == auth() && status == 'ACTIVE']] )
231+
@@allow('create', associated_tenants?[tenant.users?[user == auth() && status == 'ACTIVE']] )
232+
@@allow('update', associated_tenants?[tenant.users?[user == auth() && status == 'ACTIVE']] )
233+
}
234+
235+
model UserTenant {
236+
user_id Int
237+
user User @relation(fields: [user_id], references: [id], onDelete: Cascade, onUpdate: Cascade)
238+
239+
tenant_id Int
240+
tenant Tenant @relation(fields: [tenant_id], references: [id], onDelete: Cascade, onUpdate: Cascade)
241+
242+
status String @default('INACTIVE')
243+
244+
@@map("user_tenants")
245+
246+
@@id([user_id, tenant_id])
247+
248+
@@index([user_id])
249+
@@index([tenant_id])
250+
@@index([user_id, tenant_id])
251+
252+
@@allow('all', auth().is_super_admin == true )
253+
@@allow('read', tenant.users?[user == auth() && status == 'ACTIVE' ])
254+
@@allow('all', tenant.users?[user == auth() && status == 'ACTIVE'])
255+
@@allow('update', tenant.users?[user == auth() && status == 'ACTIVE'])
256+
@@allow('delete', tenant.users?[user == auth() && status == 'ACTIVE'])
257+
@@allow('create', tenant.users?[user == auth() && status == 'ACTIVE'])
258+
}
259+
`
260+
);
261+
262+
await prisma.user.deleteMany();
263+
await prisma.tenant.deleteMany();
264+
265+
await prisma.tenant.create({
266+
data: {
267+
id: 1,
268+
name: 'tenant 1',
269+
},
270+
});
271+
272+
await prisma.user.create({
273+
data: {
274+
id: 1,
275+
name: 'user 1',
276+
},
277+
});
278+
279+
await prisma.userTenant.create({
280+
data: {
281+
user_id: 1,
282+
tenant_id: 1,
283+
},
284+
});
285+
286+
const db = withPolicy({ id: 1, is_super_admin: true });
287+
await db.userTenant.update({
288+
where: {
289+
user_id_tenant_id: {
290+
user_id: 1,
291+
tenant_id: 1,
292+
},
293+
},
294+
data: {
295+
user: {
296+
update: {
297+
name: 'user 1 updated',
298+
},
299+
},
300+
},
301+
});
302+
});
194303
});

‎tests/integration/tests/with-policy/multi-id-fields.test.ts‎

Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,4 +153,121 @@ describe('With Policy: multiple id fields', () => {
153153
).toBeRejectedByPolicy();
154154
await expect(db.q.create({ data: { owner: { connect: { x_y: { x: '1', y: '2' } } } } })).toResolveTruthy();
155155
});
156+
157+
it('multi-id to-one nested write', async () => {
158+
const { withPolicy } = await loadSchema(
159+
`
160+
model A {
161+
x Int
162+
y Int
163+
v Int
164+
b B @relation(fields: [bId], references: [id])
165+
bId Int @unique
166+
167+
@@id([x, y])
168+
@@allow('all', v > 0)
169+
}
170+
171+
model B {
172+
id Int @id
173+
v Int
174+
a A?
175+
176+
@@allow('all', v > 0)
177+
}
178+
`
179+
);
180+
const db = withPolicy();
181+
await expect(
182+
db.b.create({
183+
data: {
184+
id: 1,
185+
v: 1,
186+
a: {
187+
create: {
188+
x: 1,
189+
y: 2,
190+
v: 3,
191+
},
192+
},
193+
},
194+
})
195+
).toResolveTruthy();
196+
197+
await expect(
198+
db.a.update({
199+
where: { x_y: { x: 1, y: 2 } },
200+
data: { b: { update: { v: 5 } } },
201+
})
202+
).toResolveTruthy();
203+
204+
expect(await db.b.findUnique({ where: { id: 1 } })).toEqual(expect.objectContaining({ v: 5 }));
205+
});
206+
207+
it('multi-id to-many nested write', async () => {
208+
const { withPolicy } = await loadSchema(
209+
`
210+
model A {
211+
x Int
212+
y Int
213+
v Int
214+
b B @relation(fields: [bId], references: [id])
215+
bId Int @unique
216+
217+
@@id([x, y])
218+
@@allow('all', v > 0)
219+
}
220+
221+
model B {
222+
id Int @id
223+
v Int
224+
a A[]
225+
c C?
226+
227+
@@allow('all', v > 0)
228+
}
229+
230+
model C {
231+
id Int @id
232+
v Int
233+
b B @relation(fields: [bId], references: [id])
234+
bId Int @unique
235+
236+
@@allow('all', v > 0)
237+
}
238+
`
239+
);
240+
const db = withPolicy();
241+
await expect(
242+
db.b.create({
243+
data: {
244+
id: 1,
245+
v: 1,
246+
a: {
247+
create: {
248+
x: 1,
249+
y: 2,
250+
v: 2,
251+
},
252+
},
253+
c: {
254+
create: {
255+
id: 1,
256+
v: 3,
257+
},
258+
},
259+
},
260+
})
261+
).toResolveTruthy();
262+
263+
await expect(
264+
db.a.update({
265+
where: { x_y: { x: 1, y: 2 } },
266+
data: { b: { update: { v: 5, c: { update: { v: 6 } } } } },
267+
})
268+
).toResolveTruthy();
269+
270+
expect(await db.b.findUnique({ where: { id: 1 } })).toEqual(expect.objectContaining({ v: 5 }));
271+
expect(await db.c.findUnique({ where: { id: 1 } })).toEqual(expect.objectContaining({ v: 6 }));
272+
});
156273
});

0 commit comments

Comments
 (0)