Skip to content

Adding $nor operator support #4768

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

Merged
merged 14 commits into from
May 18, 2018
42 changes: 42 additions & 0 deletions spec/ParseQuery.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -2567,6 +2567,48 @@ describe('Parse.Query testing', () => {
});
});

it('$select inside $nor', (done) => {
Copy link
Member

Choose a reason for hiding this comment

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

Change the name as $select isn't being used here

const objects = Array.from(Array(10).keys()).map((rating) => {
return new TestObject({ 'rating': rating });
});

const highValue = 5;
const lowValue = 3;
const options = Object.assign({}, masterKeyOptions, {
body: {
where: {
$nor: [
{ rating : { $gt : highValue } },
{ rating : { $lte : lowValue } },
]
},
}
});

Parse.Object.saveAll(objects).then(() => {
return rp.get(Parse.serverURL + "/classes/TestObject", options);
}).then((results) => {
expect(results.results.length).toBe(highValue - lowValue);
expect(results.results.every(res => res.rating > lowValue && res.rating <= highValue)).toBe(true);
done();
});
});

it('$nor invalid query', (done) => {
Copy link
Member

Choose a reason for hiding this comment

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

Maybe another test when $nor: 1234 (non array)

const options = Object.assign({}, masterKeyOptions, {
body: {
where: { $nor: [] },
}
});
const obj = new TestObject();
obj.save().then(() => {
return rp.get(Parse.serverURL + "/classes/TestObject", options);
}).then(done.fail).catch((error) => {
equal(error.error.code, Parse.Error.INVALID_JSON);
Copy link
Member

Choose a reason for hiding this comment

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

Should be INVALID_QUERY.

done();
});
});

it("dontSelect query", function(done) {
const RestaurantObject = Parse.Object.extend("Restaurant");
const PersonObject = Parse.Object.extend("Person");
Expand Down
4 changes: 2 additions & 2 deletions src/Adapters/Storage/Mongo/MongoTransform.js
Original file line number Diff line number Diff line change
Expand Up @@ -247,9 +247,9 @@ function transformQueryKeyValue(className, key, value, schema) {
case '_perishable_token':
case '_email_verify_token': return {key, value}
case '$or':
return {key: '$or', value: value.map(subQuery => transformWhere(className, subQuery, schema))};
case '$and':
return {key: '$and', value: value.map(subQuery => transformWhere(className, subQuery, schema))};
case '$nor':
return {key: key, value: value.map(subQuery => transformWhere(className, subQuery, schema))};
case 'lastUsed':
if (valueAsDate(value)) {
return {key: '_last_used', value: valueAsDate(value)}
Expand Down
5 changes: 4 additions & 1 deletion src/Adapters/Storage/Postgres/PostgresStorageAdapter.js
Original file line number Diff line number Diff line change
Expand Up @@ -306,12 +306,15 @@ const buildWhereClause = ({ schema, query, index }): WhereClause => {
patterns.push(`$${index}:name = $${index + 1}`);
values.push(fieldName, fieldValue);
index += 2;
} else if (fieldName === '$or' || fieldName === '$and') {
} else if (['$or', '$nor', '$and'].includes(fieldName)) {
const clauses = [];
const clauseValues = [];
fieldValue.forEach((subQuery) => {
const clause = buildWhereClause({ schema, query: subQuery, index });
if (clause.pattern.length > 0) {
if (fieldName === '$nor') {
clause.pattern = `(NOT ${clause.pattern})`;
}
Copy link
Contributor

@flovilmart flovilmart May 18, 2018

Choose a reason for hiding this comment

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

you're doing (NOT query1) AND (NOT query2) instead of NOT (query1 OR query2) whhich tecnically work but can be a bit misleading for maintenance. Can« you document it or revert the logic with the help of line 323? Event if it's the same, rtaht will help for comprehension in the future ;)

Copy link
Member

Choose a reason for hiding this comment

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

True I was pretty lazy 😅

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah I figured it worked but letMs write it cleanly please

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll fix that

clauses.push(clause.pattern);
clauseValues.push(...clause.values);
index += clause.values.length;
Expand Down
18 changes: 13 additions & 5 deletions src/Controllers/DatabaseController.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ const transformObjectACL = ({ ACL, ...result }) => {
return result;
}

const specialQuerykeys = ['$and', '$or', '_rperm', '_wperm', '_perishable_token', '_email_verify_token', '_email_verify_token_expires_at', '_account_lockout_expires_at', '_failed_login_count'];
const specialQuerykeys = ['$and', '$or', '$nor', '_rperm', '_wperm', '_perishable_token', '_email_verify_token', '_email_verify_token_expires_at', '_account_lockout_expires_at', '_failed_login_count'];

const isSpecialQueryKey = key => {
return specialQuerykeys.indexOf(key) >= 0;
Expand All @@ -62,7 +62,7 @@ const validateQuery = (query: any): void => {
}

if (query.$or) {
if (query.$or instanceof Array) {
Copy link
Member

Choose a reason for hiding this comment

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

Revert these changes to $or and $and as are they are causing tests to fail.

They also have nothing to do with the PR.

if (query.$or instanceof Array && query.$or.length > 1) {
query.$or.forEach(validateQuery);

/* In MongoDB, $or queries which are not alone at the top level of the
Expand Down Expand Up @@ -99,15 +99,23 @@ const validateQuery = (query: any): void => {
});
query.$or.forEach(validateQuery);
} else {
throw new Parse.Error(Parse.Error.INVALID_QUERY, 'Bad $or format - use an array value.');
throw new Parse.Error(Parse.Error.INVALID_QUERY, 'Bad $or format - use an array of at least 2 values.');
}
}

if (query.$and) {
if (query.$and instanceof Array) {
if (query.$and instanceof Array && query.$and.length > 1) {
query.$and.forEach(validateQuery);
} else {
throw new Parse.Error(Parse.Error.INVALID_QUERY, 'Bad $and format - use an array value.');
throw new Parse.Error(Parse.Error.INVALID_QUERY, 'Bad $and format - use an array of at least 2 values.');
}
}

if (query.$nor) {
if (query.$nor instanceof Array && query.$nor.length > 0) {
query.$nor.forEach(validateQuery);
} else {
throw new Parse.Error(Parse.Error.INVALID_QUERY, 'Bad $nor format - use an array of at least 1 value.');
}
}

Expand Down