From 6311c9578577a29e5bc163d399b44ae4fd3b1c70 Mon Sep 17 00:00:00 2001 From: Drew Gross Date: Wed, 30 Mar 2016 19:35:54 -0700 Subject: [PATCH 1/2] Fixes #1271 --- package.json | 1 + spec/ParseQuery.spec.js | 18 +++++++ spec/ParseRelation.spec.js | 70 ++++++++++++------------ src/Controllers/DatabaseController.js | 76 ++++++++++++++------------- src/transform.js | 9 ++-- 5 files changed, 100 insertions(+), 74 deletions(-) diff --git a/package.json b/package.json index 2096309700..302ce6e888 100644 --- a/package.json +++ b/package.json @@ -26,6 +26,7 @@ "commander": "^2.9.0", "deepcopy": "^0.6.1", "express": "^4.13.4", + "intersect": "^1.0.1", "lru-cache": "^4.0.0", "mailgun-js": "^0.7.7", "mime": "^1.3.4", diff --git a/spec/ParseQuery.spec.js b/spec/ParseQuery.spec.js index 08d5ed46c8..334ac1b62c 100644 --- a/spec/ParseQuery.spec.js +++ b/spec/ParseQuery.spec.js @@ -2228,4 +2228,22 @@ describe('Parse.Query testing', () => { }); }); }); + + it('objectId containedIn with multiple large array', done => { + let obj = new Parse.Object('MyClass'); + obj.save().then(obj => { + let longListOfStrings = []; + for (let i = 0; i < 130; i++) { + longListOfStrings.push(i.toString()); + } + longListOfStrings.push(obj.id); + let q = new Parse.Query('MyClass'); + q.containedIn('objectId', longListOfStrings); + q.containedIn('objectId', longListOfStrings); + return q.find(); + }).then(results => { + expect(results.length).toEqual(1); + done(); + }); + }); }); diff --git a/spec/ParseRelation.spec.js b/spec/ParseRelation.spec.js index 8b38a8e31c..fbb2b1d35c 100644 --- a/spec/ParseRelation.spec.js +++ b/spec/ParseRelation.spec.js @@ -248,46 +248,50 @@ describe('Parse.Relation testing', () => { }); }); - it("queries on relation fields with multiple ins", (done) => { - var ChildObject = Parse.Object.extend("ChildObject"); - var childObjects = []; - for (var i = 0; i < 10; i++) { + it("queries on relation fields with multiple containedIn (regression test for #1271)", (done) => { + let ChildObject = Parse.Object.extend("ChildObject"); + let childObjects = []; + for (let i = 0; i < 10; i++) { childObjects.push(new ChildObject({x: i})); } Parse.Object.saveAll(childObjects).then(() => { - var ParentObject = Parse.Object.extend("ParentObject"); - var parent = new ParentObject(); + let ParentObject = Parse.Object.extend("ParentObject"); + let parent = new ParentObject(); parent.set("x", 4); - var relation = parent.relation("child"); - relation.add(childObjects[0]); - relation.add(childObjects[1]); - relation.add(childObjects[2]); - var parent2 = new ParentObject(); + let parent1Children = parent.relation("child"); + parent1Children.add(childObjects[0]); + parent1Children.add(childObjects[1]); + parent1Children.add(childObjects[2]); + let parent2 = new ParentObject(); parent2.set("x", 3); - var relation2 = parent2.relation("child"); - relation2.add(childObjects[4]); - relation2.add(childObjects[5]); - relation2.add(childObjects[6]); - - var otherChild2 = parent2.relation("otherChild"); - otherChild2.add(childObjects[0]); - otherChild2.add(childObjects[1]); - otherChild2.add(childObjects[2]); - - var parents = []; - parents.push(parent); - parents.push(parent2); - return Parse.Object.saveAll(parents); + let parent2Children = parent2.relation("child"); + parent2Children.add(childObjects[4]); + parent2Children.add(childObjects[5]); + parent2Children.add(childObjects[6]); + + let parent2OtherChildren = parent2.relation("otherChild"); + parent2OtherChildren.add(childObjects[0]); + parent2OtherChildren.add(childObjects[1]); + parent2OtherChildren.add(childObjects[2]); + + return Parse.Object.saveAll([parent, parent2]); }).then(() => { - var query = new Parse.Query(ParentObject); - var objects = []; - objects.push(childObjects[0]); - query.containedIn("child", objects); - query.containedIn("otherChild", [childObjects[0]]); - return query.find(); - }).then((list) => { - equal(list.length, 2, "There should be 2 results"); + let objectsWithChild0InBothChildren = new Parse.Query(ParentObject); + objectsWithChild0InBothChildren.containedIn("child", [childObjects[0]]); + objectsWithChild0InBothChildren.containedIn("otherChild", [childObjects[0]]); + return objectsWithChild0InBothChildren.find(); + }).then(objectsWithChild0InBothChildren => { + //No parent has child 0 in both it's "child" and "otherChild" field; + expect(objectsWithChild0InBothChildren.length).toEqual(0); + }).then(() => { + let objectsWithChild4andOtherChild1 = new Parse.Query(ParentObject); + objectsWithChild4andOtherChild1.containedIn("child", [childObjects[4]]); + objectsWithChild4andOtherChild1.containedIn("otherChild", [childObjects[1]]); + return objectsWithChild4andOtherChild1.find(); + }).then(objects => { + // parent2 has child 4 and otherChild 1 + expect(objects.length).toEqual(1); done(); }); }); diff --git a/src/Controllers/DatabaseController.js b/src/Controllers/DatabaseController.js index 7494cc88ff..cf3e2bf6bc 100644 --- a/src/Controllers/DatabaseController.js +++ b/src/Controllers/DatabaseController.js @@ -1,6 +1,8 @@ // A database adapter that works with data exported from the hosted // Parse database. +import intersect from 'intersect'; + var mongodb = require('mongodb'); var Parse = require('parse/node').Parse; @@ -487,18 +489,26 @@ DatabaseController.prototype.reduceRelationKeys = function(className, query) { } }; -DatabaseController.prototype.addInObjectIdsIds = function(ids, query) { - if (typeof query.objectId == 'string') { - // Add equality op as we are sure - // we had a constraint on that one - query.objectId = {'$eq': query.objectId}; +DatabaseController.prototype.addInObjectIdsIds = function(ids = null, query) { + let idsFromString = typeof query.objectId === 'string' ? [query.objectId] : null; + let idsFromEq = query.objectId && query.objectId['$eq'] ? [query.objectId['$eq']] : null; + let idsFromIn = query.objectId && query.objectId['$in'] ? query.objectId['$in'] : null; + + let allIds = [idsFromString, idsFromEq, idsFromIn, ids].filter(list => list !== null); + let totalLength = allIds.reduce((memo, list) => memo + list.length, 0); + + let idsIntersection = []; + if (totalLength > 125) { + idsIntersection = intersect.big(allIds); + } else { + idsIntersection = intersect(allIds); } - query.objectId = query.objectId || {}; - let queryIn = [].concat(query.objectId['$in'] || [], ids || []); - // make a set and spread to remove duplicates - // replace the $in operator as other constraints - // may be set - query.objectId['$in'] = [...new Set(queryIn)]; + + // Need to make sure we don't clobber existing $lt or other constraints on objectId + if (!('objectId' in query) || typeof query.objectId === 'string') { + query.objectId = {}; + } + query.objectId['$in'] = idsIntersection; return query; } @@ -518,7 +528,7 @@ DatabaseController.prototype.addInObjectIdsIds = function(ids, query) { // anything about users, ideally. Then, improve the format of the ACL // arg to work like the others. DatabaseController.prototype.find = function(className, query, options = {}) { - var mongoOptions = {}; + let mongoOptions = {}; if (options.skip) { mongoOptions.skip = options.skip; } @@ -526,45 +536,39 @@ DatabaseController.prototype.find = function(className, query, options = {}) { mongoOptions.limit = options.limit; } - var isMaster = !('acl' in options); - var aclGroup = options.acl || []; - var acceptor = function(schema) { - return schema.hasKeys(className, keysForQuery(query)); - }; - var schema; - return this.loadSchema(acceptor).then((s) => { + let isMaster = !('acl' in options); + let aclGroup = options.acl || []; + let acceptor = schema => schema.hasKeys(className, keysForQuery(query)) + let schema = null; + return this.loadSchema(acceptor).then(s => { schema = s; if (options.sort) { mongoOptions.sort = {}; - for (var key in options.sort) { - var mongoKey = transform.transformKey(schema, className, key); + for (let key in options.sort) { + let mongoKey = transform.transformKey(schema, className, key); mongoOptions.sort[mongoKey] = options.sort[key]; } } if (!isMaster) { - var op = 'find'; - var k = Object.keys(query); - if (k.length == 1 && typeof query.objectId == 'string') { - op = 'get'; - } + let op = typeof query.objectId == 'string' && Object.keys(query).length === 1 ? + 'get' : + 'find'; return schema.validatePermission(className, aclGroup, op); } return Promise.resolve(); - }).then(() => { - return this.reduceRelationKeys(className, query); - }).then(() => { - return this.reduceInRelation(className, query, schema); - }).then(() => { - return this.adaptiveCollection(className); - }).then(collection => { - var mongoWhere = transform.transformWhere(schema, className, query); + }) + .then(() => this.reduceRelationKeys(className, query)) + .then(() => this.reduceInRelation(className, query, schema)) + .then(() => this.adaptiveCollection(className)) + .then(collection => { + let mongoWhere = transform.transformWhere(schema, className, query); if (!isMaster) { - var orParts = [ + let orParts = [ {"_rperm" : { "$exists": false }}, {"_rperm" : { "$in" : ["*"]}} ]; - for (var acl of aclGroup) { + for (let acl of aclGroup) { orParts.push({"_rperm" : { "$in" : [acl]}}); } mongoWhere = {'$and': [mongoWhere, {'$or': orParts}]}; diff --git a/src/transform.js b/src/transform.js index 3ca9739b11..6c1b85ec32 100644 --- a/src/transform.js +++ b/src/transform.js @@ -187,13 +187,12 @@ export function transformKeyValue(schema, className, restKey, restValue, options // Returns the mongo form of the query. // Throws a Parse.Error if the input query is invalid. function transformWhere(schema, className, restWhere) { - var mongoWhere = {}; + let mongoWhere = {}; if (restWhere['ACL']) { - throw new Parse.Error(Parse.Error.INVALID_QUERY, - 'Cannot query on ACL.'); + throw new Parse.Error(Parse.Error.INVALID_QUERY, 'Cannot query on ACL.'); } - for (var restKey in restWhere) { - var out = transformKeyValue(schema, className, restKey, restWhere[restKey], + for (let restKey in restWhere) { + let out = transformKeyValue(schema, className, restKey, restWhere[restKey], {query: true, validate: true}); mongoWhere[out.key] = out.value; } From 73bca3b64ca082cb970ff2dfc9d2e50a5e6c8810 Mon Sep 17 00:00:00 2001 From: Drew Gross Date: Wed, 30 Mar 2016 19:48:33 -0700 Subject: [PATCH 2/2] Improve comments --- src/Controllers/DatabaseController.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Controllers/DatabaseController.js b/src/Controllers/DatabaseController.js index cf3e2bf6bc..8faa05b735 100644 --- a/src/Controllers/DatabaseController.js +++ b/src/Controllers/DatabaseController.js @@ -504,7 +504,9 @@ DatabaseController.prototype.addInObjectIdsIds = function(ids = null, query) { idsIntersection = intersect(allIds); } - // Need to make sure we don't clobber existing $lt or other constraints on objectId + // Need to make sure we don't clobber existing $lt or other constraints on objectId. + // Clobbering $eq, $in and shorthand $eq (query.objectId === 'string') constraints + // is expected though. if (!('objectId' in query) || typeof query.objectId === 'string') { query.objectId = {}; }