fix(DatabaseController): Do not match any entry when searching for null in relation field (#3924)

davimacedo · flovilmart · commit a0d1a3517ff1 · 2017-06-21T08:23:20.000-04:00
diff --git 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
@@ -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
@@ -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
@@ -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
         }
       });
+    } else {
+      queries = [{isNegation: false, 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();
+    // 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 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.resolve();
       });
+    });
 
-      return Promise.all(promises).then(() => {
-        return Promise.resolve();
-      })
+    return Promise.all(promises).then(() => {
+      return Promise.resolve();
+    })
 
-    }
-    return Promise.resolve();
   })
 
   return Promise.all(promises).then(() => {
