Skip to content

Commit fb4a586

Browse files
ymc9ymc9
committed
fix(policy): revers lookup condition to parent entity is not properly built with compound id fields
fixes #1964
1 parent e0676f2 commit fb4a586

File tree

4 files changed

+177
-18
lines changed

4 files changed

+177
-18
lines changed

‎packages/runtime/src/enhancements/node/delegate.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -959,7 +959,7 @@ export class DelegateProxyHandler extends DefaultPrismaProxyHandler {
959959
this.injectWhereHierarchy(model, (args as any)?.where);
960960
this.doProcessUpdatePayload(model, (args as any)?.data);
961961
} else {
962-
const where = this.queryUtils.buildReversedQuery(context, false, false);
962+
const where = await this.queryUtils.buildReversedQuery(db, context, false, false);
963963
await this.queryUtils.transaction(db, async (tx) => {
964964
await this.doUpdateMany(tx, model, { ...args, where }, simpleUpdateMany);
965965
});
@@ -1022,15 +1022,15 @@ export class DelegateProxyHandler extends DefaultPrismaProxyHandler {
10221022
},
10231023

10241024
delete: async (model, _args, context) => {
1025-
const where = this.queryUtils.buildReversedQuery(context, false, false);
1025+
const where = await this.queryUtils.buildReversedQuery(db, context, false, false);
10261026
await this.queryUtils.transaction(db, async (tx) => {
10271027
await this.doDelete(tx, model, { where });
10281028
});
10291029
delete context.parent['delete'];
10301030
},
10311031

10321032
deleteMany: async (model, _args, context) => {
1033-
const where = this.queryUtils.buildReversedQuery(context, false, false);
1033+
const where = await this.queryUtils.buildReversedQuery(db, context, false, false);
10341034
await this.queryUtils.transaction(db, async (tx) => {
10351035
await this.doDeleteMany(tx, model, where);
10361036
});

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

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -809,7 +809,7 @@ export class PolicyProxyHandler<DbClient extends DbClientContract> implements Pr
809809
const unsafe = isUnsafeMutate(model, args, this.modelMeta);
810810

811811
// handles the connection to upstream entity
812-
const reversedQuery = this.policyUtils.buildReversedQuery(context, true, unsafe);
812+
const reversedQuery = await this.policyUtils.buildReversedQuery(db, context, true, unsafe);
813813
if ((!unsafe || context.field.isRelationOwner) && reversedQuery[context.field.backLink]) {
814814
// if mutation is safe, or current field owns the relation (so the other side has no fk),
815815
// and the reverse query contains the back link, then we can build a "connect" with it
@@ -885,7 +885,7 @@ export class PolicyProxyHandler<DbClient extends DbClientContract> implements Pr
885885
if (args.skipDuplicates) {
886886
// get a reversed query to include fields inherited from upstream mutation,
887887
// it'll be merged with the create payload for unique constraint checking
888-
const upstreamQuery = this.policyUtils.buildReversedQuery(context);
888+
const upstreamQuery = await this.policyUtils.buildReversedQuery(db, context);
889889
if (await this.hasDuplicatedUniqueConstraint(model, item, upstreamQuery, db)) {
890890
if (this.shouldLogQuery) {
891891
this.logger.info(`[policy] \`createMany\` skipping duplicate ${formatObject(item)}`);
@@ -910,7 +910,7 @@ export class PolicyProxyHandler<DbClient extends DbClientContract> implements Pr
910910
if (operation === 'disconnect') {
911911
// disconnect filter is not unique, need to build a reversed query to
912912
// locate the entity and use its id fields as unique filter
913-
const reversedQuery = this.policyUtils.buildReversedQuery(context);
913+
const reversedQuery = await this.policyUtils.buildReversedQuery(db, context);
914914
const found = await db[model].findUnique({
915915
where: reversedQuery,
916916
select: this.policyUtils.makeIdSelection(model),
@@ -936,7 +936,7 @@ export class PolicyProxyHandler<DbClient extends DbClientContract> implements Pr
936936
const visitor = new NestedWriteVisitor(this.modelMeta, {
937937
update: async (model, args, context) => {
938938
// build a unique query including upstream conditions
939-
const uniqueFilter = this.policyUtils.buildReversedQuery(context);
939+
const uniqueFilter = await this.policyUtils.buildReversedQuery(db, context);
940940

941941
// handle not-found
942942
const existing = await this.policyUtils.checkExistence(db, model, uniqueFilter, true);
@@ -997,7 +997,7 @@ export class PolicyProxyHandler<DbClient extends DbClientContract> implements Pr
997997
if (preValueSelect) {
998998
select = { ...select, ...preValueSelect };
999999
}
1000-
const reversedQuery = this.policyUtils.buildReversedQuery(context);
1000+
const reversedQuery = await this.policyUtils.buildReversedQuery(db, context);
10011001
const currentSetQuery = { select, where: reversedQuery };
10021002
this.policyUtils.injectAuthGuardAsWhere(db, currentSetQuery, model, 'read');
10031003

@@ -1027,7 +1027,7 @@ export class PolicyProxyHandler<DbClient extends DbClientContract> implements Pr
10271027
} else {
10281028
// we have to process `updateMany` separately because the guard may contain
10291029
// filters using relation fields which are not allowed in nested `updateMany`
1030-
const reversedQuery = this.policyUtils.buildReversedQuery(context);
1030+
const reversedQuery = await this.policyUtils.buildReversedQuery(db, context);
10311031
const updateWhere = this.policyUtils.and(reversedQuery, updateGuard);
10321032
if (this.shouldLogQuery) {
10331033
this.logger.info(
@@ -1066,7 +1066,7 @@ export class PolicyProxyHandler<DbClient extends DbClientContract> implements Pr
10661066

10671067
upsert: async (model, args, context) => {
10681068
// build a unique query including upstream conditions
1069-
const uniqueFilter = this.policyUtils.buildReversedQuery(context);
1069+
const uniqueFilter = await this.policyUtils.buildReversedQuery(db, context);
10701070

10711071
// branch based on if the update target exists
10721072
const existing = await this.policyUtils.checkExistence(db, model, uniqueFilter);
@@ -1143,7 +1143,7 @@ export class PolicyProxyHandler<DbClient extends DbClientContract> implements Pr
11431143

11441144
set: async (model, args, context) => {
11451145
// find the set of items to be replaced
1146-
const reversedQuery = this.policyUtils.buildReversedQuery(context);
1146+
const reversedQuery = await this.policyUtils.buildReversedQuery(db, context);
11471147
const findCurrSetArgs = {
11481148
select: this.policyUtils.makeIdSelection(model),
11491149
where: reversedQuery,
@@ -1162,7 +1162,7 @@ export class PolicyProxyHandler<DbClient extends DbClientContract> implements Pr
11621162

11631163
delete: async (model, args, context) => {
11641164
// build a unique query including upstream conditions
1165-
const uniqueFilter = this.policyUtils.buildReversedQuery(context);
1165+
const uniqueFilter = await this.policyUtils.buildReversedQuery(db, context);
11661166

11671167
// handle not-found
11681168
await this.policyUtils.checkExistence(db, model, uniqueFilter, true);
@@ -1179,7 +1179,7 @@ export class PolicyProxyHandler<DbClient extends DbClientContract> implements Pr
11791179
} else {
11801180
// we have to process `deleteMany` separately because the guard may contain
11811181
// filters using relation fields which are not allowed in nested `deleteMany`
1182-
const reversedQuery = this.policyUtils.buildReversedQuery(context);
1182+
const reversedQuery = await this.policyUtils.buildReversedQuery(db, context);
11831183
const deleteWhere = this.policyUtils.and(reversedQuery, guard);
11841184
if (this.shouldLogQuery) {
11851185
this.logger.info(`[policy] \`deleteMany\` ${model}:\n${formatObject({ where: deleteWhere })}`);

‎packages/runtime/src/enhancements/node/query-utils.ts‎

Lines changed: 34 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,17 @@ import {
1010
} from '../../cross';
1111
import type { CrudContract, DbClientContract } from '../../types';
1212
import { getVersion } from '../../version';
13+
import { formatObject } from '../edge';
1314
import { InternalEnhancementOptions } from './create-enhancement';
15+
import { Logger } from './logger';
1416
import { prismaClientUnknownRequestError, prismaClientValidationError } from './utils';
1517

1618
export class QueryUtils {
17-
constructor(private readonly prisma: DbClientContract, protected readonly options: InternalEnhancementOptions) {}
19+
private readonly logger: Logger;
20+
21+
constructor(private readonly prisma: DbClientContract, protected readonly options: InternalEnhancementOptions) {
22+
this.logger = new Logger(prisma);
23+
}
1824

1925
getIdFields(model: string) {
2026
return getIdFields(this.options.modelMeta, model, true);
@@ -60,7 +66,12 @@ export class QueryUtils {
6066
/**
6167
* Builds a reversed query for the given nested path.
6268
*/
63-
buildReversedQuery(context: NestedWriteVisitorContext, forMutationPayload = false, unsafeOperation = false) {
69+
async buildReversedQuery(
70+
db: CrudContract,
71+
context: NestedWriteVisitorContext,
72+
forMutationPayload = false,
73+
uncheckedOperation = false
74+
) {
6475
let result, currQuery: any;
6576
let currField: FieldInfo | undefined;
6677

@@ -102,17 +113,35 @@ export class QueryUtils {
102113
const shouldPreserveRelationCondition =
103114
// doing a mutation
104115
forMutationPayload &&
105-
// and it's a safe mutate
106-
!unsafeOperation &&
116+
// and it's not an unchecked mutate
117+
!uncheckedOperation &&
107118
// and the current segment is the direct parent (the last one is the mutate itself),
108119
// the relation condition should be preserved and will be converted to a "connect" later
109120
i === context.nestingPath.length - 2;
110121

111122
if (fkMapping && !shouldPreserveRelationCondition) {
112123
// turn relation condition into foreign key condition, e.g.:
113124
// { user: { id: 1 } } => { userId: 1 }
125+
126+
let parentPk = visitWhere;
127+
if (Object.keys(fkMapping).some((k) => !(k in parentPk) || parentPk[k] === undefined)) {
128+
// it can happen that the parent condition actually doesn't contain all id fields
129+
// （when the parent condition is not a primary key but unique constraints)
130+
// and in such case we need to load it to get the pks
131+
132+
if (this.options.logPrismaQuery && this.logger.enabled('info')) {
133+
this.logger.info(
134+
`[reverseLookup] \`findUniqueOrThrow\` ${model}: ${formatObject(where)}`
135+
);
136+
}
137+
parentPk = await db[model].findUniqueOrThrow({
138+
where,
139+
select: this.makeIdSelection(model),
140+
});
141+
}
142+
114143
for (const [r, fk] of Object.entries<string>(fkMapping)) {
115-
currQuery[fk] = visitWhere[r];
144+
currQuery[fk] = parentPk[r];
116145
}
117146

118147
if (i > 0) {

‎tests/regression/tests/issue-1964.test.ts‎

Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,130 @@
1+
import { loadSchema } from '@zenstackhq/testtools';
2+
3+
describe('issue 1964', () => {
4+
it('regression1', async () => {
5+
const { enhance } = await loadSchema(
6+
`
7+
model User {
8+
id Int @id
9+
orgId String
10+
}
11+
12+
model Author {
13+
id Int @id @default(autoincrement())
14+
orgId String
15+
name String
16+
posts Post[]
17+
18+
@@unique([orgId, name])
19+
@@allow('all', auth().orgId == orgId)
20+
}
21+
22+
model Post {
23+
id Int @id @default(autoincrement())
24+
orgId String
25+
title String
26+
author Author @relation(fields: [authorId], references: [id])
27+
authorId Int
28+
29+
@@allow('all', auth().orgId == orgId)
30+
}
31+
`,
32+
{
33+
previewFeatures: ['strictUndefinedChecks'],
34+
logPrismaQuery: true,
35+
}
36+
);
37+
38+
const db = enhance({ id: 1, orgId: 'org' });
39+
40+
const newauthor = await db.author.create({
41+
data: {
42+
name: `Foo ${Date.now()}`,
43+
orgId: 'org',
44+
posts: {
45+
createMany: { data: [{ title: 'Hello', orgId: 'org' }] },
46+
},
47+
},
48+
include: { posts: true },
49+
});
50+
51+
await expect(
52+
db.author.update({
53+
where: { orgId_name: { orgId: 'org', name: newauthor.name } },
54+
data: {
55+
name: `Bar ${Date.now()}`,
56+
posts: { deleteMany: { id: { equals: newauthor.posts[0].id } } },
57+
},
58+
})
59+
).toResolveTruthy();
60+
});
61+
62+
it('regression2', async () => {
63+
const { enhance } = await loadSchema(
64+
`
65+
model User {
66+
id Int @id @default(autoincrement())
67+
slug String @unique
68+
profile Profile?
69+
@@allow('all', true)
70+
}
71+
72+
model Profile {
73+
id Int @id @default(autoincrement())
74+
slug String @unique
75+
name String
76+
addresses Address[]
77+
userId Int? @unique
78+
user User? @relation(fields: [userId], references: [id])
79+
@@allow('all', true)
80+
}
81+
82+
model Address {
83+
id Int @id @default(autoincrement())
84+
profileId Int @unique
85+
profile Profile @relation(fields: [profileId], references: [id])
86+
city String
87+
@@allow('all', true)
88+
}
89+
`,
90+
{
91+
previewFeatures: ['strictUndefinedChecks'],
92+
logPrismaQuery: true,
93+
}
94+
);
95+
96+
const db = enhance({ id: 1, orgId: 'org' });
97+
98+
await db.user.create({
99+
data: {
100+
slug: `user1`,
101+
profile: {
102+
create: {
103+
name: `My Profile`,
104+
slug: 'profile1',
105+
addresses: {
106+
create: { id: 1, city: 'City' },
107+
},
108+
},
109+
},
110+
},
111+
});
112+
113+
await expect(
114+
db.user.update({
115+
where: { slug: 'user1' },
116+
data: {
117+
profile: {
118+
update: {
119+
addresses: {
120+
deleteMany: { id: { equals: 1 } },
121+
},
122+
},
123+
},
124+
},
125+
})
126+
).toResolveTruthy();
127+
128+
await expect(db.address.count()).resolves.toEqual(0);
129+
});
130+
});

0 commit comments

Comments
 (0)