Skip to content

Commit d8f0954

Browse files
ymc9ymc9
authored
fix: wrong payload injected for nested create in update (#715)
1 parent d061218 commit d8f0954

File tree

5 files changed

+225
-5
lines changed

5 files changed

+225
-5
lines changed

‎packages/runtime/src/enhancements/policy/handler.ts

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -622,8 +622,18 @@ export class PolicyProxyHandler<DbClient extends DbClientContract> implements Pr
622622
const _create = async (model: string, args: any, context: NestedWriteVisitorContext) => {
623623
let createData = args;
624624
if (context.field?.backLink) {
625+
// Check if the create payload contains any "unsafe" assignment:
626+
// assign id or foreign key fields.
627+
//
628+
// The reason why we need to do that is Prisma's mutations payload
629+
// structure has two mutually exclusive forms for safe and unsafe
630+
// operations. E.g.:
631+
// - safe: { data: { user: { connect: { id: 1 }} } }
632+
// - unsafe: { data: { userId: 1 } }
633+
const unsafe = this.isUnsafeMutate(model, args);
634+
625635
// handles the connection to upstream entity
626-
const reversedQuery = this.utils.buildReversedQuery(context);
636+
const reversedQuery = this.utils.buildReversedQuery(context, true, unsafe);
627637
if (reversedQuery[context.field.backLink]) {
628638
// the built reverse query contains a condition for the backlink field, build a "connect" with it
629639
createData = {
@@ -881,6 +891,19 @@ export class PolicyProxyHandler<DbClient extends DbClientContract> implements Pr
881891
return { result, postWriteChecks };
882892
}
883893

894+
private isUnsafeMutate(model: string, args: any) {
895+
if (!args) {
896+
return false;
897+
}
898+
for (const k of Object.keys(args)) {
899+
const field = resolveField(this.modelMeta, model, k);
900+
if (field?.isId || field?.isForeignKey) {
901+
return true;
902+
}
903+
}
904+
return false;
905+
}
906+
884907
async updateMany(args: any) {
885908
if (!args) {
886909
throw prismaClientValidationError(this.prisma, 'query argument is required');

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

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -484,7 +484,7 @@ export class PolicyUtil {
484484
/**
485485
* Builds a reversed query for the given nested path.
486486
*/
487-
buildReversedQuery(context: NestedWriteVisitorContext) {
487+
buildReversedQuery(context: NestedWriteVisitorContext, mutating = false, unsafeOperation = false) {
488488
let result, currQuery: any;
489489
let currField: FieldInfo | undefined;
490490

@@ -509,19 +509,41 @@ export class PolicyUtil {
509509
if (!currField.backLink) {
510510
throw this.unknownError(`field ${currField.type}.${currField.name} doesn't have a backLink`);
511511
}
512+
512513
const backLinkField = this.getModelField(currField.type, currField.backLink);
513-
if (backLinkField?.isArray) {
514+
if (!backLinkField) {
515+
throw this.unknownError(`missing backLink field ${currField.backLink} in ${currField.type}`);
516+
}
517+
518+
if (backLinkField.isArray) {
514519
// many-side of relationship, wrap with "some" query
515520
currQuery[currField.backLink] = { some: { ...visitWhere } };
516521
} else {
517-
if (where && backLinkField.isRelationOwner && backLinkField.foreignKeyMapping) {
518-
for (const [r, fk] of Object.entries<string>(backLinkField.foreignKeyMapping)) {
522+
const fkMapping = where && backLinkField.isRelationOwner && backLinkField.foreignKeyMapping;
523+
524+
// calculate if we should preserve the relation condition (e.g., { user: { id: 1 } })
525+
const shouldPreserveRelationCondition =
526+
// doing a mutation
527+
mutating &&
528+
// and it's a safe mutate
529+
!unsafeOperation &&
530+
// and the current segment is the direct parent (the last one is the mutate itself),
531+
// the relation condition should be preserved and will be converted to a "connect" later
532+
i === context.nestingPath.length - 2;
533+
534+
if (fkMapping && !shouldPreserveRelationCondition) {
535+
// turn relation condition into foreign key condition, e.g.:
536+
// { user: { id: 1 } } => { userId: 1 }
537+
for (const [r, fk] of Object.entries<string>(fkMapping)) {
519538
currQuery[fk] = visitWhere[r];
520539
}
540+
521541
if (i > 0) {
542+
// prepare for the next segment
522543
currQuery[currField.backLink] = {};
523544
}
524545
} else {
546+
// preserve the original structure
525547
currQuery[currField.backLink] = { ...visitWhere };
526548
}
527549
}

‎packages/runtime/src/types.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,11 @@ export type FieldInfo = {
110110
*/
111111
isRelationOwner: boolean;
112112

113+
/**
114+
* If the field is a foreign key field
115+
*/
116+
isForeignKey: boolean;
117+
113118
/**
114119
* Mapping from foreign key field names to relation field names
115120
*/

‎packages/schema/src/plugins/model-meta/index.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import {
1919
getDataModels,
2020
getLiteral,
2121
hasAttribute,
22+
isForeignKeyField,
2223
isIdField,
2324
PluginError,
2425
PluginFunction,
@@ -95,6 +96,7 @@ function generateModelMetadata(dataModels: DataModel[], writer: CodeBlockWriter)
9596
attributes: ${JSON.stringify(getFieldAttributes(f))},
9697
backLink: ${backlink ? "'" + backlink.name + "'" : 'undefined'},
9798
isRelationOwner: ${isRelationOwner(f, backlink)},
99+
isForeignKey: ${isForeignKeyField(f)},
98100
foreignKeyMapping: ${fkMapping ? JSON.stringify(fkMapping) : 'undefined'}
99101
},`);
100102
}

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

Lines changed: 168 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,168 @@
1+
import { createPostgresDb, dropPostgresDb, loadSchema } from '@zenstackhq/testtools';
2+
3+
const DB_NAME = 'issue-714';
4+
5+
describe('Regression: issue 714', () => {
6+
let dbUrl: string;
7+
let prisma: any;
8+
9+
beforeEach(async () => {
10+
dbUrl = await createPostgresDb(DB_NAME);
11+
});
12+
13+
afterEach(async () => {
14+
if (prisma) {
15+
await prisma.$disconnect();
16+
}
17+
await dropPostgresDb(DB_NAME);
18+
});
19+
20+
it('regression', async () => {
21+
const { prisma: _prisma, enhance } = await loadSchema(
22+
`
23+
model User {
24+
id Int @id @default(autoincrement())
25+
username String @unique
26+
27+
employedBy CompanyUser[]
28+
properties PropertyUser[]
29+
companies Company[]
30+
31+
@@allow('all', true)
32+
}
33+
34+
model Company {
35+
id Int @id @default(autoincrement())
36+
name String
37+
38+
companyUsers CompanyUser[]
39+
propertyUsers User[]
40+
properties Property[]
41+
42+
@@allow('all', true)
43+
}
44+
45+
model CompanyUser {
46+
company Company @relation(fields: [companyId], references: [id])
47+
companyId Int
48+
user User @relation(fields: [userId], references: [id])
49+
userId Int
50+
51+
dummyField String
52+
53+
@@id([companyId, userId])
54+
55+
@@allow('all', true)
56+
}
57+
58+
enum PropertyUserRoleType {
59+
Owner
60+
Administrator
61+
}
62+
63+
model PropertyUserRole {
64+
id Int @id @default(autoincrement())
65+
type PropertyUserRoleType
66+
67+
user PropertyUser @relation(fields: [userId], references: [id])
68+
userId Int
69+
70+
@@allow('all', true)
71+
}
72+
73+
model PropertyUser {
74+
id Int @id @default(autoincrement())
75+
dummyField String
76+
77+
property Property @relation(fields: [propertyId], references: [id])
78+
propertyId Int
79+
user User @relation(fields: [userId], references: [id])
80+
userId Int
81+
82+
roles PropertyUserRole[]
83+
84+
@@unique([propertyId, userId])
85+
86+
@@allow('all', true)
87+
}
88+
89+
model Property {
90+
id Int @id @default(autoincrement())
91+
name String
92+
93+
users PropertyUser[]
94+
company Company @relation(fields: [companyId], references: [id])
95+
companyId Int
96+
97+
@@allow('all', true)
98+
}
99+
`,
100+
{
101+
provider: 'postgresql',
102+
dbUrl,
103+
}
104+
);
105+
106+
prisma = _prisma;
107+
const db = enhance();
108+
109+
await db.user.create({
110+
data: {
111+
username: 'test@example.com',
112+
},
113+
});
114+
115+
await db.company.create({
116+
data: {
117+
name: 'My Company',
118+
companyUsers: {
119+
create: {
120+
dummyField: '',
121+
user: {
122+
connect: {
123+
id: 1,
124+
},
125+
},
126+
},
127+
},
128+
propertyUsers: {
129+
connect: {
130+
id: 1,
131+
},
132+
},
133+
properties: {
134+
create: [
135+
{
136+
name: 'Test',
137+
},
138+
],
139+
},
140+
},
141+
});
142+
143+
await db.property.update({
144+
data: {
145+
users: {
146+
create: {
147+
dummyField: '',
148+
roles: {
149+
createMany: {
150+
data: {
151+
type: 'Owner',
152+
},
153+
},
154+
},
155+
user: {
156+
connect: {
157+
id: 1,
158+
},
159+
},
160+
},
161+
},
162+
},
163+
where: {
164+
id: 1,
165+
},
166+
});
167+
});
168+
});

0 commit comments

Comments
 (0)