Skip to content

Commit d698333

Browse files
mess-lelouchMusa Yassin-Fortdplewis
authored
Optimizing pointer CLP query decoration done by DatabaseController#addPointerPermissions (#6747)
* Optimize CLP pointer query * remove console log * Update changelog * Fix flow type checker issues * Remove unused properties * Fix typo, add one more test case for coverage * Add support for CLP entry of type Object Co-authored-by: Musa Yassin-Fort <[email protected]> Co-authored-by: Diamond Lewis <[email protected]>
1 parent 78239ac commit d698333

File tree

3 files changed

+299
-19
lines changed

3 files changed

+299
-19
lines changed

‎CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
### master
44
[Full Changelog](https://github.com/parse-community/parse-server/compare/4.2.0...master)
5+
- FIX: Optimize query decoration based on pointer CLPs by looking at the class schema to determine the field's type.
56
- NEW (EXPERIMENTAL): Idempotency enforcement for client requests. This deduplicates requests where the client intends to send one request to Parse Server but due to network issues the server receives the request multiple times. **Caution, this is an experimental feature that may not be appropriate for production.** [#6744](https://github.com/parse-community/parse-server/issues/6744). Thanks to [Manuel Trezza](https://github.com/mtrezza).
67

78
### 4.2.0

‎spec/DatabaseController.spec.js

+267-7
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
const DatabaseController = require('../lib/Controllers/DatabaseController.js');
22
const validateQuery = DatabaseController._validateQuery;
33

4-
describe('DatabaseController', function() {
5-
describe('validateQuery', function() {
6-
it('should not restructure simple cases of SERVER-13732', done => {
4+
describe('DatabaseController', function () {
5+
describe('validateQuery', function () {
6+
it('should not restructure simple cases of SERVER-13732', (done) => {
77
const query = {
88
$or: [{ a: 1 }, { a: 2 }],
99
_rperm: { $in: ['a', 'b'] },
@@ -18,7 +18,7 @@ describe('DatabaseController', function() {
1818
done();
1919
});
2020

21-
it('should not restructure SERVER-13732 queries with $nears', done => {
21+
it('should not restructure SERVER-13732 queries with $nears', (done) => {
2222
let query = { $or: [{ a: 1 }, { b: 1 }], c: { $nearSphere: {} } };
2323
validateQuery(query);
2424
expect(query).toEqual({
@@ -31,7 +31,7 @@ describe('DatabaseController', function() {
3131
done();
3232
});
3333

34-
it('should not push refactored keys down a tree for SERVER-13732', done => {
34+
it('should not push refactored keys down a tree for SERVER-13732', (done) => {
3535
const query = {
3636
a: 1,
3737
$or: [{ $or: [{ b: 1 }, { b: 2 }] }, { $or: [{ c: 1 }, { c: 2 }] }],
@@ -45,14 +45,274 @@ describe('DatabaseController', function() {
4545
done();
4646
});
4747

48-
it('should reject invalid queries', done => {
48+
it('should reject invalid queries', (done) => {
4949
expect(() => validateQuery({ $or: { a: 1 } })).toThrow();
5050
done();
5151
});
5252

53-
it('should accept valid queries', done => {
53+
it('should accept valid queries', (done) => {
5454
expect(() => validateQuery({ $or: [{ a: 1 }, { b: 2 }] })).not.toThrow();
5555
done();
5656
});
5757
});
58+
59+
describe('addPointerPermissions', function () {
60+
const CLASS_NAME = 'Foo';
61+
const USER_ID = 'userId';
62+
const ACL_GROUP = [USER_ID];
63+
const OPERATION = 'find';
64+
65+
const databaseController = new DatabaseController();
66+
const schemaController = jasmine.createSpyObj('SchemaController', [
67+
'testPermissionsForClassName',
68+
'getClassLevelPermissions',
69+
'getExpectedType',
70+
]);
71+
72+
it('should not decorate query if no pointer CLPs are present', (done) => {
73+
const clp = buildCLP();
74+
const query = { a: 'b' };
75+
76+
schemaController.testPermissionsForClassName
77+
.withArgs(CLASS_NAME, ACL_GROUP, OPERATION)
78+
.and.returnValue(true);
79+
schemaController.getClassLevelPermissions
80+
.withArgs(CLASS_NAME)
81+
.and.returnValue(clp);
82+
83+
const output = databaseController.addPointerPermissions(
84+
schemaController,
85+
CLASS_NAME,
86+
OPERATION,
87+
query,
88+
ACL_GROUP
89+
);
90+
91+
expect(output).toEqual({ ...query });
92+
93+
done();
94+
});
95+
96+
it('should decorate query if a pointer CLP entry is present', (done) => {
97+
const clp = buildCLP(['user']);
98+
const query = { a: 'b' };
99+
100+
schemaController.testPermissionsForClassName
101+
.withArgs(CLASS_NAME, ACL_GROUP, OPERATION)
102+
.and.returnValue(false);
103+
schemaController.getClassLevelPermissions
104+
.withArgs(CLASS_NAME)
105+
.and.returnValue(clp);
106+
schemaController.getExpectedType
107+
.withArgs(CLASS_NAME, 'user')
108+
.and.returnValue({ type: 'Pointer' });
109+
110+
const output = databaseController.addPointerPermissions(
111+
schemaController,
112+
CLASS_NAME,
113+
OPERATION,
114+
query,
115+
ACL_GROUP
116+
);
117+
118+
expect(output).toEqual({ ...query, user: createUserPointer(USER_ID) });
119+
120+
done();
121+
});
122+
123+
it('should decorate query if an array CLP entry is present', (done) => {
124+
const clp = buildCLP(['users']);
125+
const query = { a: 'b' };
126+
127+
schemaController.testPermissionsForClassName
128+
.withArgs(CLASS_NAME, ACL_GROUP, OPERATION)
129+
.and.returnValue(false);
130+
schemaController.getClassLevelPermissions
131+
.withArgs(CLASS_NAME)
132+
.and.returnValue(clp);
133+
schemaController.getExpectedType
134+
.withArgs(CLASS_NAME, 'users')
135+
.and.returnValue({ type: 'Array' });
136+
137+
const output = databaseController.addPointerPermissions(
138+
schemaController,
139+
CLASS_NAME,
140+
OPERATION,
141+
query,
142+
ACL_GROUP
143+
);
144+
145+
expect(output).toEqual({
146+
...query,
147+
users: { $all: [createUserPointer(USER_ID)] },
148+
});
149+
150+
done();
151+
});
152+
153+
it('should decorate query if an object CLP entry is present', (done) => {
154+
const clp = buildCLP(['user']);
155+
const query = { a: 'b' };
156+
157+
schemaController.testPermissionsForClassName
158+
.withArgs(CLASS_NAME, ACL_GROUP, OPERATION)
159+
.and.returnValue(false);
160+
schemaController.getClassLevelPermissions
161+
.withArgs(CLASS_NAME)
162+
.and.returnValue(clp);
163+
schemaController.getExpectedType
164+
.withArgs(CLASS_NAME, 'user')
165+
.and.returnValue({ type: 'Object' });
166+
167+
const output = databaseController.addPointerPermissions(
168+
schemaController,
169+
CLASS_NAME,
170+
OPERATION,
171+
query,
172+
ACL_GROUP
173+
);
174+
175+
expect(output).toEqual({
176+
...query,
177+
user: createUserPointer(USER_ID),
178+
});
179+
180+
done();
181+
});
182+
183+
it('should decorate query if a pointer CLP is present and the same field is part of the query', (done) => {
184+
const clp = buildCLP(['user']);
185+
const query = { a: 'b', user: 'a' };
186+
187+
schemaController.testPermissionsForClassName
188+
.withArgs(CLASS_NAME, ACL_GROUP, OPERATION)
189+
.and.returnValue(false);
190+
schemaController.getClassLevelPermissions
191+
.withArgs(CLASS_NAME)
192+
.and.returnValue(clp);
193+
schemaController.getExpectedType
194+
.withArgs(CLASS_NAME, 'user')
195+
.and.returnValue({ type: 'Pointer' });
196+
197+
const output = databaseController.addPointerPermissions(
198+
schemaController,
199+
CLASS_NAME,
200+
OPERATION,
201+
query,
202+
ACL_GROUP
203+
);
204+
205+
expect(output).toEqual({
206+
$and: [{ user: createUserPointer(USER_ID) }, { ...query }],
207+
});
208+
209+
done();
210+
});
211+
212+
it('should transform the query to an $or query if multiple array/pointer CLPs are present', (done) => {
213+
const clp = buildCLP(['user', 'users', 'userObject']);
214+
const query = { a: 'b' };
215+
216+
schemaController.testPermissionsForClassName
217+
.withArgs(CLASS_NAME, ACL_GROUP, OPERATION)
218+
.and.returnValue(false);
219+
schemaController.getClassLevelPermissions
220+
.withArgs(CLASS_NAME)
221+
.and.returnValue(clp);
222+
schemaController.getExpectedType
223+
.withArgs(CLASS_NAME, 'user')
224+
.and.returnValue({ type: 'Pointer' });
225+
schemaController.getExpectedType
226+
.withArgs(CLASS_NAME, 'users')
227+
.and.returnValue({ type: 'Array' });
228+
schemaController.getExpectedType
229+
.withArgs(CLASS_NAME, 'userObject')
230+
.and.returnValue({ type: 'Object' });
231+
232+
const output = databaseController.addPointerPermissions(
233+
schemaController,
234+
CLASS_NAME,
235+
OPERATION,
236+
query,
237+
ACL_GROUP
238+
);
239+
240+
expect(output).toEqual({
241+
$or: [
242+
{ ...query, user: createUserPointer(USER_ID) },
243+
{ ...query, users: { $all: [createUserPointer(USER_ID)] } },
244+
{ ...query, userObject: createUserPointer(USER_ID) },
245+
],
246+
});
247+
248+
done();
249+
});
250+
251+
it('should throw an error if for some unexpected reason the property specified in the CLP is neither a pointer nor an array', (done) => {
252+
const clp = buildCLP(['user']);
253+
const query = { a: 'b' };
254+
255+
schemaController.testPermissionsForClassName
256+
.withArgs(CLASS_NAME, ACL_GROUP, OPERATION)
257+
.and.returnValue(false);
258+
schemaController.getClassLevelPermissions
259+
.withArgs(CLASS_NAME)
260+
.and.returnValue(clp);
261+
schemaController.getExpectedType
262+
.withArgs(CLASS_NAME, 'user')
263+
.and.returnValue({ type: 'Number' });
264+
265+
expect(() => {
266+
databaseController.addPointerPermissions(
267+
schemaController,
268+
CLASS_NAME,
269+
OPERATION,
270+
query,
271+
ACL_GROUP
272+
);
273+
}).toThrow(
274+
Error(
275+
`An unexpected condition occurred when resolving pointer permissions: ${CLASS_NAME} user`
276+
)
277+
);
278+
279+
done();
280+
});
281+
});
58282
});
283+
284+
function buildCLP(pointerNames) {
285+
const OPERATIONS = [
286+
'count',
287+
'find',
288+
'get',
289+
'create',
290+
'update',
291+
'delete',
292+
'addField',
293+
];
294+
295+
const clp = OPERATIONS.reduce((acc, op) => {
296+
acc[op] = {};
297+
298+
if (pointerNames && pointerNames.length) {
299+
acc[op].pointerFields = pointerNames;
300+
}
301+
302+
return acc;
303+
}, {});
304+
305+
clp.protectedFields = {};
306+
clp.writeUserFields = [];
307+
clp.readUserFields = [];
308+
309+
return clp;
310+
}
311+
312+
function createUserPointer(userId) {
313+
return {
314+
__type: 'Pointer',
315+
className: '_User',
316+
objectId: userId,
317+
};
318+
}

‎src/Controllers/DatabaseController.js

+31-12
Original file line numberDiff line numberDiff line change
@@ -1567,23 +1567,42 @@ class DatabaseController {
15671567
objectId: userId,
15681568
};
15691569

1570-
const ors = permFields.flatMap(key => {
1571-
// constraint for single pointer setup
1572-
const q = {
1573-
[key]: userPointer,
1574-
};
1575-
// constraint for users-array setup
1576-
const qa = {
1577-
[key]: { $all: [userPointer] },
1578-
};
1570+
const queries = permFields.map((key) => {
1571+
const fieldDescriptor = schema.getExpectedType(className, key);
1572+
const fieldType =
1573+
fieldDescriptor &&
1574+
typeof fieldDescriptor === 'object' &&
1575+
Object.prototype.hasOwnProperty.call(fieldDescriptor, 'type')
1576+
? fieldDescriptor.type
1577+
: null;
1578+
1579+
let queryClause;
1580+
1581+
if (fieldType === 'Pointer') {
1582+
// constraint for single pointer setup
1583+
queryClause = { [key]: userPointer };
1584+
} else if (fieldType === 'Array') {
1585+
// constraint for users-array setup
1586+
queryClause = { [key]: { $all: [userPointer] } };
1587+
} else if (fieldType === 'Object') {
1588+
// constraint for object setup
1589+
queryClause = { [key]: userPointer };
1590+
} else {
1591+
// This means that there is a CLP field of an unexpected type. This condition should not happen, which is
1592+
// why is being treated as an error.
1593+
throw Error(
1594+
`An unexpected condition occurred when resolving pointer permissions: ${className} ${key}`
1595+
);
1596+
}
15791597
// if we already have a constraint on the key, use the $and
15801598
if (Object.prototype.hasOwnProperty.call(query, key)) {
1581-
return [{ $and: [q, query] }, { $and: [qa, query] }];
1599+
return { $and: [queryClause, query] };
15821600
}
15831601
// otherwise just add the constaint
1584-
return [Object.assign({}, query, q), Object.assign({}, query, qa)];
1602+
return Object.assign({}, query, queryClause);
15851603
});
1586-
return { $or: ors };
1604+
1605+
return queries.length === 1 ? queries[0] : { $or: queries };
15871606
} else {
15881607
return query;
15891608
}

0 commit comments

Comments
 (0)