Optimize redundant logic used in queries (#7061)
* Optimize redundant logic used in queries * Added CHANGELOG * Fixed comments and code style after recommendations. * Fixed code style after recommendation. * Improved explanation in comments * Added tests to for logic optimizations * Added two test cases more and some comments * Added extra test cases and fixed issue found with them. * Removed empty lines as requested. Co-authored-by: Pedro Diaz <p.diaz@wemersive.com>
This commit is contained in:
@@ -2,6 +2,7 @@
|
||||
|
||||
### master
|
||||
[Full Changelog](https://github.com/parse-community/parse-server/compare/4.5.0...master)
|
||||
- IMPROVE: Optimize queries on classes with pointer permissions. [#7061](https://github.com/parse-community/parse-server/pull/7061). Thanks to [Pedro Diaz](https://github.com/pdiaz)
|
||||
|
||||
### 4.5.0
|
||||
[Full Changelog](https://github.com/parse-community/parse-server/compare/4.4.0...4.5.0)
|
||||
|
||||
@@ -236,6 +236,57 @@ describe('DatabaseController', function () {
|
||||
done();
|
||||
});
|
||||
|
||||
it('should not return a $or operation if the query involves one of the two fields also used as array/pointer permissions', done => {
|
||||
const clp = buildCLP(['users', 'user']);
|
||||
const query = { a: 'b', user: createUserPointer(USER_ID) };
|
||||
schemaController.testPermissionsForClassName
|
||||
.withArgs(CLASS_NAME, ACL_GROUP, OPERATION)
|
||||
.and.returnValue(false);
|
||||
schemaController.getClassLevelPermissions.withArgs(CLASS_NAME).and.returnValue(clp);
|
||||
schemaController.getExpectedType
|
||||
.withArgs(CLASS_NAME, 'user')
|
||||
.and.returnValue({ type: 'Pointer' });
|
||||
schemaController.getExpectedType
|
||||
.withArgs(CLASS_NAME, 'users')
|
||||
.and.returnValue({ type: 'Array' });
|
||||
const output = databaseController.addPointerPermissions(
|
||||
schemaController,
|
||||
CLASS_NAME,
|
||||
OPERATION,
|
||||
query,
|
||||
ACL_GROUP
|
||||
);
|
||||
expect(output).toEqual({ ...query, user: createUserPointer(USER_ID) });
|
||||
done();
|
||||
});
|
||||
|
||||
it('should not return a $or operation if the query involves one of the fields also used as array/pointer permissions', done => {
|
||||
const clp = buildCLP(['user', 'users', 'userObject']);
|
||||
const query = { a: 'b', user: createUserPointer(USER_ID) };
|
||||
schemaController.testPermissionsForClassName
|
||||
.withArgs(CLASS_NAME, ACL_GROUP, OPERATION)
|
||||
.and.returnValue(false);
|
||||
schemaController.getClassLevelPermissions.withArgs(CLASS_NAME).and.returnValue(clp);
|
||||
schemaController.getExpectedType
|
||||
.withArgs(CLASS_NAME, 'user')
|
||||
.and.returnValue({ type: 'Pointer' });
|
||||
schemaController.getExpectedType
|
||||
.withArgs(CLASS_NAME, 'users')
|
||||
.and.returnValue({ type: 'Array' });
|
||||
schemaController.getExpectedType
|
||||
.withArgs(CLASS_NAME, 'userObject')
|
||||
.and.returnValue({ type: 'Object' });
|
||||
const output = databaseController.addPointerPermissions(
|
||||
schemaController,
|
||||
CLASS_NAME,
|
||||
OPERATION,
|
||||
query,
|
||||
ACL_GROUP
|
||||
);
|
||||
expect(output).toEqual({ ...query, user: createUserPointer(USER_ID) });
|
||||
done();
|
||||
});
|
||||
|
||||
it('should throw an error if for some unexpected reason the property specified in the CLP is neither a pointer nor an array', done => {
|
||||
const clp = buildCLP(['user']);
|
||||
const query = { a: 'b' };
|
||||
@@ -265,6 +316,51 @@ describe('DatabaseController', function () {
|
||||
done();
|
||||
});
|
||||
});
|
||||
|
||||
describe('reduceOperations', function () {
|
||||
const databaseController = new DatabaseController();
|
||||
|
||||
it('objectToEntriesStrings', done => {
|
||||
const output = databaseController.objectToEntriesStrings({ a: 1, b: 2, c: 3 });
|
||||
expect(output).toEqual(['"a":1', '"b":2', '"c":3']);
|
||||
done();
|
||||
});
|
||||
|
||||
it('reduceOrOperation', done => {
|
||||
expect(databaseController.reduceOrOperation({ a: 1 })).toEqual({ a: 1 });
|
||||
expect(databaseController.reduceOrOperation({ $or: [{ a: 1 }, { b: 2 }] })).toEqual({
|
||||
$or: [{ a: 1 }, { b: 2 }],
|
||||
});
|
||||
expect(databaseController.reduceOrOperation({ $or: [{ a: 1 }, { a: 2 }] })).toEqual({
|
||||
$or: [{ a: 1 }, { a: 2 }],
|
||||
});
|
||||
expect(databaseController.reduceOrOperation({ $or: [{ a: 1 }, { a: 1 }] })).toEqual({ a: 1 });
|
||||
expect(
|
||||
databaseController.reduceOrOperation({ $or: [{ a: 1, b: 2, c: 3 }, { a: 1 }] })
|
||||
).toEqual({ a: 1 });
|
||||
expect(
|
||||
databaseController.reduceOrOperation({ $or: [{ b: 2 }, { a: 1, b: 2, c: 3 }] })
|
||||
).toEqual({ b: 2 });
|
||||
done();
|
||||
});
|
||||
|
||||
it('reduceAndOperation', done => {
|
||||
expect(databaseController.reduceAndOperation({ a: 1 })).toEqual({ a: 1 });
|
||||
expect(databaseController.reduceAndOperation({ $and: [{ a: 1 }, { b: 2 }] })).toEqual({
|
||||
$and: [{ a: 1 }, { b: 2 }],
|
||||
});
|
||||
expect(databaseController.reduceAndOperation({ $and: [{ a: 1 }, { a: 2 }] })).toEqual({
|
||||
$and: [{ a: 1 }, { a: 2 }],
|
||||
});
|
||||
expect(databaseController.reduceAndOperation({ $and: [{ a: 1 }, { a: 1 }] })).toEqual({
|
||||
a: 1,
|
||||
});
|
||||
expect(
|
||||
databaseController.reduceAndOperation({ $and: [{ a: 1, b: 2, c: 3 }, { b: 2 }] })
|
||||
).toEqual({ a: 1, b: 2, c: 3 });
|
||||
done();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
function buildCLP(pointerNames) {
|
||||
|
||||
@@ -1365,6 +1365,83 @@ class DatabaseController {
|
||||
});
|
||||
}
|
||||
|
||||
// This helps to create intermediate objects for simpler comparison of
|
||||
// key value pairs used in query objects. Each key value pair will represented
|
||||
// in a similar way to json
|
||||
objectToEntriesStrings(query: any): Array<string> {
|
||||
return Object.entries(query).map(a => a.map(s => JSON.stringify(s)).join(':'));
|
||||
}
|
||||
|
||||
// Naive logic reducer for OR operations meant to be used only for pointer permissions.
|
||||
reduceOrOperation(query: { $or: Array<any> }): any {
|
||||
if (!query.$or) {
|
||||
return query;
|
||||
}
|
||||
const queries = query.$or.map(q => this.objectToEntriesStrings(q));
|
||||
let repeat = false;
|
||||
do {
|
||||
repeat = false;
|
||||
for (let i = 0; i < queries.length - 1; i++) {
|
||||
for (let j = i + 1; j < queries.length; j++) {
|
||||
const [shorter, longer] = queries[i].length > queries[j].length ? [j, i] : [i, j];
|
||||
const foundEntries = queries[shorter].reduce(
|
||||
(acc, entry) => acc + (queries[longer].includes(entry) ? 1 : 0),
|
||||
0
|
||||
);
|
||||
const shorterEntries = queries[shorter].length;
|
||||
if (foundEntries === shorterEntries) {
|
||||
// If the shorter query is completely contained in the longer one, we can strike
|
||||
// out the longer query.
|
||||
query.$or.splice(longer, 1);
|
||||
queries.splice(longer, 1);
|
||||
repeat = true;
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
} while (repeat);
|
||||
if (query.$or.length === 1) {
|
||||
query = { ...query, ...query.$or[0] };
|
||||
delete query.$or;
|
||||
}
|
||||
return query;
|
||||
}
|
||||
|
||||
// Naive logic reducer for AND operations meant to be used only for pointer permissions.
|
||||
reduceAndOperation(query: { $and: Array<any> }): any {
|
||||
if (!query.$and) {
|
||||
return query;
|
||||
}
|
||||
const queries = query.$and.map(q => this.objectToEntriesStrings(q));
|
||||
let repeat = false;
|
||||
do {
|
||||
repeat = false;
|
||||
for (let i = 0; i < queries.length - 1; i++) {
|
||||
for (let j = i + 1; j < queries.length; j++) {
|
||||
const [shorter, longer] = queries[i].length > queries[j].length ? [j, i] : [i, j];
|
||||
const foundEntries = queries[shorter].reduce(
|
||||
(acc, entry) => acc + (queries[longer].includes(entry) ? 1 : 0),
|
||||
0
|
||||
);
|
||||
const shorterEntries = queries[shorter].length;
|
||||
if (foundEntries === shorterEntries) {
|
||||
// If the shorter query is completely contained in the longer one, we can strike
|
||||
// out the shorter query.
|
||||
query.$and.splice(shorter, 1);
|
||||
queries.splice(shorter, 1);
|
||||
repeat = true;
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
} while (repeat);
|
||||
if (query.$and.length === 1) {
|
||||
query = { ...query, ...query.$and[0] };
|
||||
delete query.$and;
|
||||
}
|
||||
return query;
|
||||
}
|
||||
|
||||
// Constraints query using CLP's pointer permissions (PP) if any.
|
||||
// 1. Etract the user id from caller's ACLgroup;
|
||||
// 2. Exctract a list of field names that are PP for target collection and operation;
|
||||
@@ -1448,13 +1525,13 @@ class DatabaseController {
|
||||
}
|
||||
// if we already have a constraint on the key, use the $and
|
||||
if (Object.prototype.hasOwnProperty.call(query, key)) {
|
||||
return { $and: [queryClause, query] };
|
||||
return this.reduceAndOperation({ $and: [queryClause, query] });
|
||||
}
|
||||
// otherwise just add the constaint
|
||||
return Object.assign({}, query, queryClause);
|
||||
});
|
||||
|
||||
return queries.length === 1 ? queries[0] : { $or: queries };
|
||||
return queries.length === 1 ? queries[0] : this.reduceOrOperation({ $or: queries });
|
||||
} else {
|
||||
return query;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user