From a0d1a3517ff158c6c889641f103bca4ac88787b3 Mon Sep 17 00:00:00 2001 From: Antonio Davi Macedo Coelho de Castro Date: Wed, 21 Jun 2017 09:23:20 -0300 Subject: [PATCH] fix(DatabaseController): Do not match any entry when searching for null in relation field (#3924) --- spec/ParseQuery.spec.js | 91 +++++++++++++++++++ spec/ParseRole.spec.js | 79 ++++++++++++++++ .../Postgres/PostgresStorageAdapter.js | 23 ++++- src/Controllers/DatabaseController.js | 60 ++++++------ 4 files changed, 220 insertions(+), 33 deletions(-) diff --git a/spec/ParseQuery.spec.js b/spec/ParseQuery.spec.js index 62ae35ac..8eee6098 100644 --- a/spec/ParseQuery.spec.js +++ b/spec/ParseQuery.spec.js @@ -23,6 +23,60 @@ describe('Parse.Query testing', () => { }); }); + it("searching for null", function(done) { + var baz = new TestObject({ foo: null }); + var qux = new TestObject({ foo: 'qux' }); + var qux2 = new TestObject({ }); + Parse.Object.saveAll([baz, qux, qux2], function() { + var query = new Parse.Query(TestObject); + query.equalTo('foo', null); + query.find({ + success: function(results) { + equal(results.length, 2); + qux.set('foo', null); + qux.save({ + success: function () { + query.find({ + success: function (results) { + equal(results.length, 3); + done(); + } + }); + } + }); + } + }); + }); + }); + + it("searching for not null", function(done) { + var baz = new TestObject({ foo: null }); + var qux = new TestObject({ foo: 'qux' }); + var qux2 = new TestObject({ }); + Parse.Object.saveAll([baz, qux, qux2], function() { + var query = new Parse.Query(TestObject); + query.notEqualTo('foo', null); + query.find({ + success: function(results) { + equal(results.length, 1); + qux.set('foo', null); + qux.save({ + success: function () { + query.find({ + success: function (results) { + equal(results.length, 0); + done(); + } + }); + }, + error: function (error) { console.log(error); } + }); + }, + error: function (error) { console.log(error); } + }); + }); + }); + it("notEqualTo with Relation is working", function(done) { var user = new Parse.User(); user.setPassword("asdf"); @@ -52,6 +106,7 @@ describe('Parse.Query testing', () => { var relDislike1 = cake1.relation("hater"); relDislike1.add(user2); + return cake1.save(); }).then(function(){ var rellike2 = cake2.relation("liker"); @@ -60,6 +115,9 @@ describe('Parse.Query testing', () => { var relDislike2 = cake2.relation("hater"); relDislike2.add(user2); + var relSomething = cake2.relation("something"); + relSomething.add(user); + return cake2.save(); }).then(function(){ var rellike3 = cake3.relation("liker"); @@ -143,6 +201,21 @@ describe('Parse.Query testing', () => { return query.find().then(function(results){ equal(results.length, 0); }); + }).then(function(){ + var query = new Parse.Query(Cake); + query.equalTo("hater", null); + query.equalTo("liker", null); + // user doesn't hate any cake so this should be 0 + return query.find().then(function(results){ + equal(results.length, 0); + }); + }).then(function(){ + var query = new Parse.Query(Cake); + query.equalTo("something", null); + // user doesn't hate any cake so this should be 0 + return query.find().then(function(results){ + equal(results.length, 0); + }); }).then(function(){ done(); }).catch((err) => { @@ -2485,6 +2558,24 @@ describe('Parse.Query testing', () => { }); }); + it('query should not match on array when searching for null', (done) => { + var target = {__type: 'Pointer', className: 'TestObject', objectId: '123'}; + var obj = new Parse.Object('TestObject'); + obj.set('someKey', 'someValue'); + obj.set('someObjs', [target]); + obj.save().then(() => { + var query = new Parse.Query('TestObject'); + query.equalTo('someKey', 'someValue'); + query.equalTo('someObjs', null); + return query.find(); + }).then((results) => { + expect(results.length).toEqual(0); + done(); + }, (error) => { + console.log(error); + }); + }); + // #371 it('should properly interpret a query v1', (done) => { var query = new Parse.Query("C1"); diff --git a/spec/ParseRole.spec.js b/spec/ParseRole.spec.js index f9a2c870..cf8dff82 100644 --- a/spec/ParseRole.spec.js +++ b/spec/ParseRole.spec.js @@ -448,4 +448,83 @@ describe('Parse Role testing', () => { .catch(done.fail); }); + it('should match when matching in users relation', (done) => { + var user = new Parse.User(); + user + .save({ username: 'admin', password: 'admin' }) + .then((user) => { + var aCL = new Parse.ACL(); + aCL.setPublicReadAccess(true); + aCL.setPublicWriteAccess(true); + var role = new Parse.Role('admin', aCL); + var users = role.relation('users'); + users.add(user); + role + .save({}, { useMasterKey: true }) + .then(() => { + var query = new Parse.Query(Parse.Role); + query.equalTo('name', 'admin'); + query.equalTo('users', user); + query.find().then(function (roles) { + expect(roles.length).toEqual(1); + done(); + }); + }); + }); + }); + + it('should not match any entry when not matching in users relation', (done) => { + var user = new Parse.User(); + user + .save({ username: 'admin', password: 'admin' }) + .then((user) => { + var aCL = new Parse.ACL(); + aCL.setPublicReadAccess(true); + aCL.setPublicWriteAccess(true); + var role = new Parse.Role('admin', aCL); + var users = role.relation('users'); + users.add(user); + role + .save({}, { useMasterKey: true }) + .then(() => { + var otherUser = new Parse.User(); + otherUser + .save({ username: 'otherUser', password: 'otherUser' }) + .then((otherUser) => { + var query = new Parse.Query(Parse.Role); + query.equalTo('name', 'admin'); + query.equalTo('users', otherUser); + query.find().then(function(roles) { + expect(roles.length).toEqual(0); + done(); + }); + }); + }); + }); + }); + + it('should not match any entry when searching for null in users relation', (done) => { + var user = new Parse.User(); + user + .save({ username: 'admin', password: 'admin' }) + .then((user) => { + var aCL = new Parse.ACL(); + aCL.setPublicReadAccess(true); + aCL.setPublicWriteAccess(true); + var role = new Parse.Role('admin', aCL); + var users = role.relation('users'); + users.add(user); + role + .save({}, { useMasterKey: true }) + .then(() => { + var query = new Parse.Query(Parse.Role); + query.equalTo('name', 'admin'); + query.equalTo('users', null); + query.find().then(function (roles) { + expect(roles.length).toEqual(0); + done(); + }); + }); + }); + }); }); diff --git a/src/Adapters/Storage/Postgres/PostgresStorageAdapter.js b/src/Adapters/Storage/Postgres/PostgresStorageAdapter.js index 2489b3ce..c6af74d8 100644 --- a/src/Adapters/Storage/Postgres/PostgresStorageAdapter.js +++ b/src/Adapters/Storage/Postgres/PostgresStorageAdapter.js @@ -188,7 +188,7 @@ const buildWhereClause = ({ schema, query, index }) => { // nothingin the schema, it's gonna blow up if (!schema.fields[fieldName]) { // as it won't exist - if (fieldValue.$exists === false) { + if (fieldValue && fieldValue.$exists === false) { continue; } } @@ -202,7 +202,16 @@ const buildWhereClause = ({ schema, query, index }) => { }); let name = components.slice(0, components.length - 1).join('->'); name += '->>' + components[components.length - 1]; - patterns.push(`${name} = '${fieldValue}'`); + if (fieldValue === null) { + patterns.push(`${name} IS NULL`); + } else { + patterns.push(`${name} = '${fieldValue}'`); + } + } else if (fieldValue === null) { + patterns.push(`$${index}:name IS NULL`); + values.push(fieldName); + index += 1; + continue; } else if (typeof fieldValue === 'string') { patterns.push(`$${index}:name = $${index + 1}`); values.push(fieldName, fieldValue); @@ -231,13 +240,16 @@ const buildWhereClause = ({ schema, query, index }) => { values.push(...clauseValues); } - if (fieldValue.$ne) { + if (fieldValue.$ne !== undefined) { if (isArrayField) { fieldValue.$ne = JSON.stringify([fieldValue.$ne]); patterns.push(`NOT array_contains($${index}:name, $${index + 1})`); } else { if (fieldValue.$ne === null) { - patterns.push(`$${index}:name <> $${index + 1}`); + patterns.push(`$${index}:name IS NOT NULL`); + values.push(fieldName); + index += 1; + continue; } else { // if not null, we need to manually exclude null patterns.push(`($${index}:name <> $${index + 1} OR $${index}:name IS NULL)`); @@ -764,6 +776,9 @@ export class PostgresStorageAdapter { validateKeys(object); Object.keys(object).forEach(fieldName => { + if (object[fieldName] === null) { + return; + } var authDataMatch = fieldName.match(/^_auth_data_([a-zA-Z0-9_]+)$/); if (authDataMatch) { var provider = authDataMatch[1]; diff --git a/src/Controllers/DatabaseController.js b/src/Controllers/DatabaseController.js index 81b37162..1f445467 100644 --- a/src/Controllers/DatabaseController.js +++ b/src/Controllers/DatabaseController.js @@ -616,13 +616,14 @@ DatabaseController.prototype.reduceInRelation = function(className, query, schem } const promises = Object.keys(query).map((key) => { + const t = schema.getExpectedType(className, key); + if (!t || t.type !== 'Relation') { + return Promise.resolve(query); + } + let queries = null; if (query[key] && (query[key]['$in'] || query[key]['$ne'] || query[key]['$nin'] || query[key].__type == 'Pointer')) { - const t = schema.getExpectedType(className, key); - if (!t || t.type !== 'Relation') { - return Promise.resolve(query); - } // Build the list of queries - const queries = Object.keys(query[key]).map((constraintKey) => { + queries = Object.keys(query[key]).map((constraintKey) => { let relatedIds; let isNegation = false; if (constraintKey === 'objectId') { @@ -643,31 +644,32 @@ DatabaseController.prototype.reduceInRelation = function(className, query, schem relatedIds } }); - - // remove the current queryKey as we don,t need it anymore - delete query[key]; - // execute each query independnently to build the list of - // $in / $nin - const promises = queries.map((q) => { - if (!q) { - return Promise.resolve(); - } - return this.owningIds(className, key, q.relatedIds).then((ids) => { - if (q.isNegation) { - this.addNotInObjectIdsIds(ids, query); - } else { - this.addInObjectIdsIds(ids, query); - } - return Promise.resolve(); - }); - }); - - return Promise.all(promises).then(() => { - return Promise.resolve(); - }) - + } else { + queries = [{isNegation: false, relatedIds: []}]; } - return Promise.resolve(); + + // remove the current queryKey as we don,t need it anymore + delete query[key]; + // execute each query independnently to build the list of + // $in / $nin + const promises = queries.map((q) => { + if (!q) { + return Promise.resolve(); + } + return this.owningIds(className, key, q.relatedIds).then((ids) => { + if (q.isNegation) { + this.addNotInObjectIdsIds(ids, query); + } else { + this.addInObjectIdsIds(ids, query); + } + return Promise.resolve(); + }); + }); + + return Promise.all(promises).then(() => { + return Promise.resolve(); + }) + }) return Promise.all(promises).then(() => {