Fixed edge case behavior where falsy fields where followed by dot notation

9 years ago · 9e07cedd18
parent 4adf9c8491
commit 9e07cedd18
4 changed files with 114 additions and 37 deletions
--- a/lib/cursor.js
+++ b/lib/cursor.js
@ -11,7 +11,7 @@ var model = require('./model')
 * Create a new cursor for this collection
 * @param {Datastore} db - The datastore this cursor is bound to
 * @param {Query} query - The query this cursor will operate on
- * @param {Function} execDn - Handler to be executed after cursor has found the results and before the callback passed to find/findOne/update/remove
+ * @param {Function} execFn - Handler to be executed after cursor has found the results and before the callback passed to find/findOne/update/remove
 */
 function Cursor (db, query, execFn) {
  this.db = db;
@ -83,7 +83,21 @@ Cursor.prototype.project = function (candidates) {

  // Do the actual projection
  candidates.forEach(function (candidate) {
-    var toPush = action === 1 ? _.pick(candidate, keys) : _.omit(candidate, keys);
+    var toPush;
+    if (action === 1) {   // pick-type projection
+      toPush = _.pick(candidate, keys);
+    } else {   // omit-type projection
+      //console.log('--------------------------------------------');
+      //console.log(candidate);
+      toPush = { $unset: {} };
+      keys.forEach(function (k) { toPush.$unset[k] = true });
+
+      //console.log(toPush);
+
+      toPush = model.modify(candidate, toPush);
+
+      //console.log(toPush);
+    }
    if (keepId) {
      toPush._id = candidate._id;
    } else {
--- a/lib/model.js
+++ b/lib/model.js
@ -416,7 +416,7 @@ function createModifierFunction (modifier) {
    if (fieldParts.length === 1) {
      lastStepModifierFunctions[modifier](obj, field, value);
    } else {
-      obj[fieldParts[0]] = obj[fieldParts[0]] || {};
+      if (obj[fieldParts[0]] === undefined) { obj[fieldParts[0]] = {}; }
      modifierFunctions[modifier](obj[fieldParts[0]], fieldParts.slice(1), value);
    }
  };
@ -457,7 +457,7 @@ function modify (obj, updateQuery) {

      if (!modifierFunctions[m]) { throw new Error("Unknown modifier " + m); }

-      // Can't rely on Object.keys throwing on non objects since ES6{
+      // Can't rely on Object.keys throwing on non objects since ES6
      // Not 100% satisfying as non objects can be interpreted as objects but no false negatives so we can live with it
      if (typeof updateQuery[m] !== 'object') {
        throw new Error("Modifier " + m + "'s argument must be an object");
--- a/test/cursor.test.js
+++ b/test/cursor.test.js
@ -684,9 +684,9 @@ describe('Cursor', function () {

    beforeEach(function (done) {
      // We don't know the order in which docs wil be inserted but we ensure correctness by testing both sort orders
-      d.insert({ age: 5, name: 'Jo', planet: 'B' }, function (err, _doc0) {
+      d.insert({ age: 5, name: 'Jo', planet: 'B', toys: { bebe: true, ballon: 'much' } }, function (err, _doc0) {
        doc0 = _doc0;
-        d.insert({ age: 57, name: 'Louis', planet: 'R' }, function (err, _doc1) {
+        d.insert({ age: 57, name: 'Louis', planet: 'R', toys: { ballon: 'yeah', bebe: false } }, function (err, _doc1) {
          doc1 = _doc1;
          d.insert({ age: 52, name: 'Grafitti', planet: 'C' }, function (err, _doc2) {
            doc2 = _doc2;
@ -766,20 +766,20 @@ describe('Cursor', function () {
        assert.isNull(err);
        docs.length.should.equal(5);
        // Takes the _id by default
-        assert.deepEqual(docs[0], { planet: 'B', _id: doc0._id });
+        assert.deepEqual(docs[0], { planet: 'B', _id: doc0._id, toys: { bebe: true, ballon: 'much' } });
        assert.deepEqual(docs[1], { planet: 'S', _id: doc3._id });
        assert.deepEqual(docs[2], { planet: 'C', _id: doc2._id });
-        assert.deepEqual(docs[3], { planet: 'R', _id: doc1._id });
+        assert.deepEqual(docs[3], { planet: 'R', _id: doc1._id, toys: { bebe: false, ballon: 'yeah' } });
        assert.deepEqual(docs[4], { planet: 'Earth', _id: doc4._id });

        cursor.projection({ age: 0, name: 0, _id: 0 });
        cursor.exec(function (err, docs) {
          assert.isNull(err);
          docs.length.should.equal(5);
-          assert.deepEqual(docs[0], { planet: 'B' });
+          assert.deepEqual(docs[0], { planet: 'B', toys: { bebe: true, ballon: 'much' } });
          assert.deepEqual(docs[1], { planet: 'S' });
          assert.deepEqual(docs[2], { planet: 'C' });
-          assert.deepEqual(docs[3], { planet: 'R' });
+          assert.deepEqual(docs[3], { planet: 'R', toys: { bebe: false, ballon: 'yeah' } });
          assert.deepEqual(docs[4], { planet: 'Earth' });

          done();
@ -794,10 +794,59 @@ describe('Cursor', function () {
      cursor.exec(function (err, docs) {
        assert.isNotNull(err);
        assert.isUndefined(docs);
-        done();
+
+        cursor.projection({ age: 1, _id: 0 });
+        cursor.exec(function (err, docs) {
+          assert.isNull(err);
+          assert.deepEqual(docs[0], { age: 5 });
+          assert.deepEqual(docs[1], { age: 23 });
+          assert.deepEqual(docs[2], { age: 52 });
+          assert.deepEqual(docs[3], { age: 57 });
+          assert.deepEqual(docs[4], { age: 89 });
+
+          cursor.projection({ age: 0, toys: 0, planet: 0, _id: 1 });
+          cursor.exec(function (err, docs) {
+            assert.isNull(err);
+            assert.deepEqual(docs[0], { name: 'Jo', _id: doc0._id });
+            assert.deepEqual(docs[1], { name: 'LM', _id: doc3._id });
+            assert.deepEqual(docs[2], { name: 'Grafitti', _id: doc2._id });
+            assert.deepEqual(docs[3], { name: 'Louis', _id: doc1._id });
+            assert.deepEqual(docs[4], { _id: doc4._id });
+
+            done();
+          });
+        });
      });
    });

+    it("Projections on embedded documents - omit type", function (done) {
+      //var query = { $set: { 'a.b': 1, 'a.c': 'world', 'single': true } };
+      //var obj = model.modify({}, query);
+
+      //console.log("==================");
+      //console.log(obj);
+
+      //obj = model.modify(obj, { $unset: { 'a.b': true, 'a.c': true } });
+
+      //console.log("==================");
+      //console.log(obj);
+
+      var obj = model.modify({ argh: true }, { $unset: { 'nope.nono': 'but yes' } });
+
+      console.log(obj);
+
+      done();
+
+      //var cursor = new Cursor(d, { age: 23 });
+      //cursor.sort({ age: 1 });   // For easier finding
+      //cursor.projection({ name: 0, planet: 0, 'toys.bebe': 0 });
+      //cursor.exec(function (err, docs) {
+        //console.log(docs);
+
+        //done();
+      //});
+    });
+
  });   // ==== End of 'Projections' ====

 });
--- a/test/model.test.js
+++ b/test/model.test.js
@ -256,36 +256,36 @@ describe('Model', function () {
      res.subobj.a.should.equal('b');
      res.subobj.b.should.equal('c');
    });
-    
+
    it('Should deep copy the contents of an array', function () {
      var a = [{ hello: 'world' }]
        , b = model.deepCopy(a)
-        ;
-        
+      ;
+
      b[0].hello.should.equal('world');
      b[0].hello = 'another';
      b[0].hello.should.equal('another');
-      a[0].hello.should.equal('world');      
+      a[0].hello.should.equal('world');
    });
-    
+
    it('Without the strictKeys option, everything gets deep copied', function () {
      var a = { a: 4, $e: 'rrr', 'eee.rt': 42, nested: { yes: 1, 'tt.yy': 2, $nopenope: 3 }, array: [{ 'rr.hh': 1 }, { yes: true }, { $yes: false }] }
        , b = model.deepCopy(a)
-        ;
-        
+      ;
+
      assert.deepEqual(a, b);
    });
-    
+
    it('With the strictKeys option, only valid keys gets deep copied', function () {
      var a = { a: 4, $e: 'rrr', 'eee.rt': 42, nested: { yes: 1, 'tt.yy': 2, $nopenope: 3 }, array: [{ 'rr.hh': 1 }, { yes: true }, { $yes: false }] }
        , b = model.deepCopy(a, true)
-        ;
-        
+      ;
+
      assert.deepEqual(b, { a: 4, nested: { yes: 1 }, array: [{}, { yes: true }, {}] });
    });

  });   // ==== End of 'Deep copying' ==== //
-  
+

  describe('Modifying documents', function () {

@ -380,13 +380,21 @@ describe('Model', function () {

        _.isEqual(modified, { yup: { subfield: 'changed', yop: 'yes indeed' }, totally: { doesnt: { exist: 'now it does' } } }).should.equal(true);
      });
+
+      it("Doesn't replace a falsy field by an object when recursively following dot notation", function () {
+        var obj = { nested: false }
+          , updateQuery = { $set: { "nested.now": 'it is' } }
+          , modified = model.modify(obj, updateQuery);
+
+        assert.deepEqual(modified, { nested: false });   // Object not modified as the nested field doesn't exist
+      });
    });   // End of '$set modifier'
-    
+
    describe('$unset modifier', function () {
-    
+
      it('Can delete a field, not throwing an error if the field doesnt exist', function () {
        var obj, updateQuery, modified;
-      
+
        obj = { yup: 'yes', other: 'also' }
        updateQuery = { $unset: { yup: true } }
        modified = model.modify(obj, updateQuery);
@ -396,46 +404,52 @@ describe('Model', function () {
        updateQuery = { $unset: { nope: true } }
        modified = model.modify(obj, updateQuery);
        assert.deepEqual(modified, obj);
-        
+
        obj = { yup: 'yes', other: 'also' }
        updateQuery = { $unset: { nope: true, other: true } }
        modified = model.modify(obj, updateQuery);
        assert.deepEqual(modified, { yup: 'yes' });
      });
-      
+
      it('Can unset sub-fields and entire nested documents', function () {
        var obj, updateQuery, modified;
-      
+
        obj = { yup: 'yes', nested: { a: 'also', b: 'yeah' } }
        updateQuery = { $unset: { nested: true } }
        modified = model.modify(obj, updateQuery);
        assert.deepEqual(modified, { yup: 'yes' });
-      
+
        obj = { yup: 'yes', nested: { a: 'also', b: 'yeah' } }
        updateQuery = { $unset: { 'nested.a': true } }
        modified = model.modify(obj, updateQuery);
        assert.deepEqual(modified, { yup: 'yes', nested: { b: 'yeah' } });
-      
+
        obj = { yup: 'yes', nested: { a: 'also', b: 'yeah' } }
        updateQuery = { $unset: { 'nested.a': true, 'nested.b': true } }
        modified = model.modify(obj, updateQuery);
        assert.deepEqual(modified, { yup: 'yes', nested: {} });
      });
-      
+
+      it.only("When unsetting nested fields, should not create an empty parent to nested field", function () {
+        var obj = model.modify({ argh: true }, { $unset: { 'bad.worse': true } });
+
+        console.log(obj);
+      });
+
    });   // End of '$unset modifier'

    describe('$inc modifier', function () {
      it('Throw an error if you try to use it with a non-number or on a non number field', function () {
        (function () {
-        var obj = { some: 'thing', yup: 'yes', nay: 2 }
-          , updateQuery = { $inc: { nay: 'notanumber' } }
-          , modified = model.modify(obj, updateQuery);
+          var obj = { some: 'thing', yup: 'yes', nay: 2 }
+            , updateQuery = { $inc: { nay: 'notanumber' } }
+            , modified = model.modify(obj, updateQuery);
        }).should.throw();

        (function () {
-        var obj = { some: 'thing', yup: 'yes', nay: 'nope' }
-          , updateQuery = { $inc: { nay: 1 } }
-          , modified = model.modify(obj, updateQuery);
+          var obj = { some: 'thing', yup: 'yes', nay: 'nope' }
+            , updateQuery = { $inc: { nay: 1 } }
+            , modified = model.modify(obj, updateQuery);
        }).should.throw();
      });