Skip to content

Commit 179634e

Browse files
ymc9ymc9
authored
fix(runtime): improved query reduction to workaround Prisma issue prisma/prisma#21856 (#1634)
1 parent ba8a888 commit 179634e

File tree

4 files changed

+193
-51
lines changed

4 files changed

+193
-51
lines changed

‎packages/runtime/jest.config.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
../../jest.config.ts

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

Lines changed: 75 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import { Logger } from '../logger';
3030
import { QueryUtils } from '../query-utils';
3131
import type { EntityChecker, ModelPolicyDef, PermissionCheckerFunc, PolicyDef, PolicyFunc, ZodSchemas } from '../types';
3232
import { formatObject, prismaClientKnownRequestError } from '../utils';
33+
import { isPlainObject } from 'is-plain-object';
3334

3435
/**
3536
* Access policy enforcement utilities
@@ -107,23 +108,63 @@ export class PolicyUtil extends QueryUtils {
107108
// Static True/False conditions
108109
// https://www.prisma.io/docs/concepts/components/prisma-client/null-and-undefined#the-effect-of-null-and-undefined-on-conditionals
109110

110-
public isTrue(condition: object) {
111-
if (condition === null || condition === undefined) {
111+
private singleKey(obj: object | null | undefined, key: string): obj is { [key: string]: unknown } {
112+
if (!obj) {
112113
return false;
113114
} else {
114-
return (
115-
(typeof condition === 'object' && Object.keys(condition).length === 0) ||
116-
('AND' in condition && Array.isArray(condition.AND) && condition.AND.length === 0)
117-
);
115+
return Object.keys(obj).length === 1 && Object.keys(obj)[0] === key;
118116
}
119117
}
120118

121-
public isFalse(condition: object) {
122-
if (condition === null || condition === undefined) {
119+
public isTrue(condition: object | null | undefined) {
120+
if (condition === null || condition === undefined || !isPlainObject(condition)) {
123121
return false;
124-
} else {
125-
return 'OR' in condition && Array.isArray(condition.OR) && condition.OR.length === 0;
126122
}
123+
124+
// {} is true
125+
if (Object.keys(condition).length === 0) {
126+
return true;
127+
}
128+
129+
// { OR: TRUE } is true
130+
if (this.singleKey(condition, 'OR') && typeof condition.OR === 'object' && this.isTrue(condition.OR)) {
131+
return true;
132+
}
133+
134+
// { NOT: FALSE } is true
135+
if (this.singleKey(condition, 'NOT') && typeof condition.NOT === 'object' && this.isFalse(condition.NOT)) {
136+
return true;
137+
}
138+
139+
// { AND: [] } is true
140+
if (this.singleKey(condition, 'AND') && Array.isArray(condition.AND) && condition.AND.length === 0) {
141+
return true;
142+
}
143+
144+
return false;
145+
}
146+
147+
public isFalse(condition: object | null | undefined) {
148+
if (condition === null || condition === undefined || !isPlainObject(condition)) {
149+
return false;
150+
}
151+
152+
// { AND: FALSE } is false
153+
if (this.singleKey(condition, 'AND') && typeof condition.AND === 'object' && this.isFalse(condition.AND)) {
154+
return true;
155+
}
156+
157+
// { NOT: TRUE } is false
158+
if (this.singleKey(condition, 'NOT') && typeof condition.NOT === 'object' && this.isTrue(condition.NOT)) {
159+
return true;
160+
}
161+
162+
// { OR: [] } is false
163+
if (this.singleKey(condition, 'OR') && Array.isArray(condition.OR) && condition.OR.length === 0) {
164+
return true;
165+
}
166+
167+
return false;
127168
}
128169

129170
private makeTrue() {
@@ -149,11 +190,6 @@ export class PolicyUtil extends QueryUtils {
149190

150191
const result: any = {};
151192
for (const [key, value] of Object.entries<any>(condition)) {
152-
if (this.isFalse(result)) {
153-
// already false, no need to continue
154-
break;
155-
}
156-
157193
if (value === null || value === undefined) {
158194
result[key] = value;
159195
continue;
@@ -165,14 +201,13 @@ export class PolicyUtil extends QueryUtils {
165201
.map((c: any) => this.reduce(c))
166202
.filter((c) => c !== undefined && !this.isTrue(c));
167203
if (children.length === 0) {
168-
result[key] = []; // true
204+
// { ..., AND: [] }
205+
result[key] = [];
169206
} else if (children.some((c) => this.isFalse(c))) {
170-
result['OR'] = []; // false
207+
// { ..., AND: { OR: [] } }
208+
result[key] = this.makeFalse();
171209
} else {
172-
if (!this.isTrue({ AND: result[key] })) {
173-
// use AND only if it's not already true
174-
result[key] = !Array.isArray(value) && children.length === 1 ? children[0] : children;
175-
}
210+
result[key] = !Array.isArray(value) && children.length === 1 ? children[0] : children;
176211
}
177212
break;
178213
}
@@ -182,54 +217,43 @@ export class PolicyUtil extends QueryUtils {
182217
.map((c: any) => this.reduce(c))
183218
.filter((c) => c !== undefined && !this.isFalse(c));
184219
if (children.length === 0) {
185-
result[key] = []; // false
220+
// { ..., OR: [] }
221+
result[key] = [];
186222
} else if (children.some((c) => this.isTrue(c))) {
187-
result['AND'] = []; // true
223+
// { ..., OR: { AND: [] } }
224+
result[key] = this.makeTrue();
188225
} else {
189-
if (!this.isFalse({ OR: result[key] })) {
190-
// use OR only if it's not already false
191-
result[key] = !Array.isArray(value) && children.length === 1 ? children[0] : children;
192-
}
226+
result[key] = !Array.isArray(value) && children.length === 1 ? children[0] : children;
193227
}
194228
break;
195229
}
196230

197231
case 'NOT': {
198-
const children = enumerate(value)
199-
.map((c: any) => this.reduce(c))
200-
.filter((c) => c !== undefined && !this.isFalse(c));
201-
if (children.length === 0) {
202-
// all clauses are false, result is a constant true,
203-
// thus eliminated (not adding into result)
204-
} else if (children.some((c) => this.isTrue(c))) {
205-
// some clauses are true, result is a constant false,
206-
// eliminate all other keys and set entire condition to false
207-
Object.keys(result).forEach((k) => delete result[k]);
208-
result['OR'] = []; // this will cause the outer loop to exit too
209-
} else {
210-
result[key] = !Array.isArray(value) && children.length === 1 ? children[0] : children;
211-
}
232+
const children = enumerate(value).map((c: any) => this.reduce(c));
233+
result[key] = !Array.isArray(value) && children.length === 1 ? children[0] : children;
212234
break;
213235
}
214236

215237
default: {
216-
const booleanKeys = ['AND', 'OR', 'NOT', 'is', 'isNot', 'none', 'every', 'some'];
217-
if (
218-
typeof value === 'object' &&
219-
value &&
220-
// recurse only if the value has at least one boolean key
221-
Object.keys(value).some((k) => booleanKeys.includes(k))
222-
) {
223-
result[key] = this.reduce(value);
224-
} else {
238+
if (!isPlainObject(value)) {
239+
// don't visit into non-plain object values - could be Date, array, etc.
225240
result[key] = value;
241+
} else {
242+
result[key] = this.reduce(value);
226243
}
227244
break;
228245
}
229246
}
230247
}
231248

232-
return result;
249+
// finally normalize constant true/false conditions
250+
if (this.isTrue(result)) {
251+
return this.makeTrue();
252+
} else if (this.isFalse(result)) {
253+
return this.makeFalse();
254+
} else {
255+
return result;
256+
}
233257
}
234258

235259
//#endregion

‎packages/runtime/tests/policy/reduction.test.ts

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
/* eslint-disable @typescript-eslint/no-explicit-any */
2+
import { PolicyUtil } from '../../src/enhancements/policy/policy-utils';
3+
4+
// eslint-disable-next-line jest/no-disabled-tests
5+
describe.skip('Prisma query reduction tests', () => {
6+
function reduce(query: any) {
7+
const util = new PolicyUtil({} as any, {} as any);
8+
return util['reduce'](query);
9+
}
10+
11+
const TRUE = { AND: [] };
12+
const FALSE = { OR: [] };
13+
14+
it('should keep regular queries unchanged', () => {
15+
expect(reduce(null)).toEqual(null);
16+
expect(reduce({ x: 1, y: 'hello' })).toEqual({ x: 1, y: 'hello' });
17+
const d = new Date();
18+
expect(reduce({ x: d })).toEqual({ x: d });
19+
});
20+
21+
it('should keep regular logical queries unchanged', () => {
22+
expect(reduce({ AND: [{ x: 1 }, { y: 'hello' }] })).toEqual({ AND: [{ x: 1 }, { y: 'hello' }] });
23+
expect(reduce({ OR: [{ x: 1 }, { y: 'hello' }] })).toEqual({ OR: [{ x: 1 }, { y: 'hello' }] });
24+
expect(reduce({ NOT: [{ x: 1 }, { y: 'hello' }] })).toEqual({ NOT: [{ x: 1 }, { y: 'hello' }] });
25+
expect(reduce({ AND: { x: 1 }, OR: { y: 'hello' }, NOT: { z: 2 } })).toEqual({
26+
AND: { x: 1 },
27+
OR: { y: 'hello' },
28+
NOT: { z: 2 },
29+
});
30+
expect(reduce({ AND: { x: 1, OR: { y: 'hello', NOT: [{ z: 2 }] } } })).toEqual({
31+
AND: { x: 1, OR: { y: 'hello', NOT: [{ z: 2 }] } },
32+
});
33+
});
34+
35+
it('should handle constant true false', () => {
36+
expect(reduce(undefined)).toEqual(TRUE);
37+
expect(reduce({})).toEqual(TRUE);
38+
expect(reduce(TRUE)).toEqual(TRUE);
39+
expect(reduce(FALSE)).toEqual(FALSE);
40+
});
41+
42+
it('should reduce simple true false', () => {
43+
expect(reduce({ AND: TRUE })).toEqual(TRUE);
44+
expect(reduce({ AND: FALSE })).toEqual(FALSE);
45+
expect(reduce({ OR: TRUE })).toEqual(TRUE);
46+
expect(reduce({ OR: FALSE })).toEqual(FALSE);
47+
expect(reduce({ NOT: TRUE })).toEqual(FALSE);
48+
expect(reduce({ NOT: FALSE })).toEqual(TRUE);
49+
});
50+
51+
it('should reduce AND queries', () => {
52+
expect(reduce({ AND: [{ x: 1 }, TRUE, { y: 2 }] })).toEqual({ AND: [{ x: 1 }, { y: 2 }] });
53+
expect(reduce({ AND: [{ x: 1 }, FALSE, { y: 2 }] })).toEqual(FALSE);
54+
expect(reduce({ AND: [{ x: 1 }, TRUE, FALSE, { y: 2 }] })).toEqual(FALSE);
55+
});
56+
57+
it('should reduce OR queries', () => {
58+
expect(reduce({ OR: [{ x: 1 }, TRUE, { y: 2 }] })).toEqual(TRUE);
59+
expect(reduce({ OR: [{ x: 1 }, FALSE, { y: 2 }] })).toEqual({ OR: [{ x: 1 }, { y: 2 }] });
60+
expect(reduce({ OR: [{ x: 1 }, TRUE, FALSE, { y: 2 }] })).toEqual(TRUE);
61+
});
62+
63+
it('should reduce NOT queries', () => {
64+
expect(reduce({ NOT: { AND: [FALSE, { x: 1 }] } })).toEqual(TRUE);
65+
expect(reduce({ NOT: { OR: [TRUE, { x: 1 }] } })).toEqual(FALSE);
66+
});
67+
});

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

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
import { loadSchema } from '@zenstackhq/testtools';
2+
describe('issue 1627', () => {
3+
it('regression', async () => {
4+
const { prisma, enhance } = await loadSchema(
5+
`
6+
model User {
7+
id String @id
8+
memberships GymUser[]
9+
}
10+
11+
model Gym {
12+
id String @id
13+
members GymUser[]
14+
15+
@@allow('all', true)
16+
}
17+
18+
model GymUser {
19+
id String @id
20+
userID String
21+
user User @relation(fields: [userID], references: [id])
22+
gymID String?
23+
gym Gym? @relation(fields: [gymID], references: [id])
24+
role String
25+
26+
@@allow('read',gym.members?[user == auth() && (role == "ADMIN" || role == "TRAINER")])
27+
@@unique([userID, gymID])
28+
}
29+
`
30+
);
31+
32+
await prisma.user.create({ data: { id: '1' } });
33+
34+
await prisma.gym.create({
35+
data: {
36+
id: '1',
37+
members: {
38+
create: {
39+
id: '1',
40+
user: { connect: { id: '1' } },
41+
role: 'ADMIN',
42+
},
43+
},
44+
},
45+
});
46+
47+
const db = enhance();
48+
await expect(db.gymUser.findMany()).resolves.toHaveLength(0);
49+
});
50+
});

0 commit comments

Comments
 (0)