Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes #1271 #1295

Merged
merged 2 commits into from
Mar 31, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
18 changes: 18 additions & 0 deletions spec/ParseQuery.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
});
});
});
70 changes: 37 additions & 33 deletions spec/ParseRelation.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
});
});
Expand Down
78 changes: 42 additions & 36 deletions src/Controllers/DatabaseController.js
Original file line number Diff line number Diff line change
@@ -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;

Expand Down Expand Up @@ -487,18 +489,28 @@ 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.
// Clobbering $eq, $in and shorthand $eq (query.objectId === 'string') constraints
// is expected though.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@drew-gross @flovilmart
I ran into an issue migrating an app to Parse Server and I'm hoping you can shed some light on this. You can see the failing test I wrote here. On api.parse.com and Parse Server <= 2.2.4 this test passes.

I tracked the problem down to this comment. Should we really be clobbering $eq constraints here or should the test linked above pass? The hacky feeling workaround would be to replace query.equalTo("objectId", cake1.id) with query.containedIn("objectId", [cake1.id]) in my test.

Thanks!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JeremyPlease I believe the test should pass as the query construction seem to be valid. Do you have an idea for the fix?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@flovilmart Check out #2472 and let me know what you think.

if (!('objectId' in query) || typeof query.objectId === 'string') {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we move that up? be tough to follow why it's correct to override when typeof query.objectId === 'string' as it's a case managed on L493.

query.objectId = {};
}
query.objectId['$in'] = idsIntersection;

return query;
}
Expand All @@ -518,53 +530,47 @@ 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;
}
if (options.limit) {
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}]};
Expand Down
9 changes: 4 additions & 5 deletions src/transform.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down