fix(populate): update top-level populated() when updating document …

…array with populated subpaths

Fix #8265

lib/types/core_array.js

@@ -634,24 +634,6 @@ class CoreMongooseArray extends Array {
       undefined, { skipDocumentArrayCast: true });
     const ret = [].push.apply(this, values);
 
-    // If this is a document array, each element may contain single
-    // populated paths, so we need to modify the top-level document's
-    // populated cache. See gh-8247.
-    if (this.isMongooseDocumentArray && parent.$__.populated != null) {
-      const populatedPaths = Object.keys(parent.$__.populated).
-        filter(p => p.startsWith(this[arrayPathSymbol] + '.'));
-
-      for (const path of populatedPaths) {
-        const remnant = path.slice((this[arrayPathSymbol] + '.').length);
-        if (!Array.isArray(parent.$__.populated[path].value)) {
-          continue;
-        }
-        for (const val of values) {
-          parent.$__.populated[path].value.push(val.populated(remnant));
-        }
-      }
-    }
-
     this._registerAtomic('$push', values);
     this._markModified();
     return ret;

lib/types/documentarray.js

@@ -172,6 +172,64 @@ class CoreDocumentArray extends CoreMongooseArray {
     }));
   }
 
+  /**
+   * Wraps [`Array#push`](https://developer.mozilla.org/en/JavaScript/Reference/Global_Objects/Array/push) with proper change tracking.
+   *
+   * @param {Object} [args...]
+   * @api public
+   * @method push
+   * @memberOf MongooseDocumentArray
+   */
+
+  push() {
+    const ret = super.push.apply(this, arguments);
+
+    _updateParentPopulated(this);
+
+    return ret;
+  }
+
+  /**
+   * Pulls items from the array atomically.
+   *
+   * @param {Object} [args...]
+   * @api public
+   * @method pull
+   * @memberOf MongooseDocumentArray
+   */
+
+  pull() {
+    const ret = super.pull.apply(this, arguments);
+
+    _updateParentPopulated(this);
+
+    return ret;
+  }
+
+  /**
+   * Wraps [`Array#shift`](https://developer.mozilla.org/en/JavaScript/Reference/Global_Objects/Array/unshift) with proper change tracking.
+   */
+
+  shift() {
+    const ret = super.shift.apply(this, arguments);
+
+    _updateParentPopulated(this);
+
+    return ret;
+  }
+
+  /**
+   * Wraps [`Array#splice`](https://developer.mozilla.org/en/JavaScript/Reference/Global_Objects/Array/splice) with proper change tracking and casting.
+   */
+
+  splice() {
+    const ret = super.splice.apply(this, arguments);
+
+    _updateParentPopulated(this);
+
+    return ret;
+  }
+
   /**
    * Helper for console.log
    *
@@ -254,6 +312,29 @@ if (util.inspect.custom) {
     CoreDocumentArray.prototype.inspect;
 }
 
+/*!
+ * If this is a document array, each element may contain single
+ * populated paths, so we need to modify the top-level document's
+ * populated cache. See gh-8247, gh-8265.
+ */
+
+function _updateParentPopulated(arr) {
+  const parent = arr[arrayParentSymbol];
+  if (parent.$__.populated != null) {
+    const populatedPaths = Object.keys(parent.$__.populated).
+      filter(p => p.startsWith(arr[arrayPathSymbol] + '.'));
+
+    for (const path of populatedPaths) {
+      const remnant = path.slice((arr[arrayPathSymbol] + '.').length);
+      if (!Array.isArray(parent.$__.populated[path].value)) {
+        continue;
+      }
+
+      parent.$__.populated[path].value = arr.map(val => val.populated(remnant));
+    }
+  }
+}
+
 /**
  * DocumentArray constructor
  *

test/model.populate.test.js

@@ -8727,7 +8727,7 @@ describe('model: populate:', function() {
       });
     });
 
-    it('updates top-level populated() when pushing elements onto a document array with single populated path (gh-8247)', function() {
+    it('updates top-level populated() when pushing elements onto a document array with single populated path (gh-8247) (gh-8265)', function() {
       return co(function*() {
         const docs = yield Author.create([
           { name: 'test1' },
@@ -8747,6 +8747,14 @@ describe('model: populate:', function() {
         assert.equal(pop[0].toHexString(), docs[0]._id.toHexString());
         assert.equal(pop[1], null);
 
+        fromDb.comments.pull({ _id: fromDb.comments[1]._id });
+        pop = fromDb.populated('comments.author');
+        assert.equal(pop.length, 1);
+
+        fromDb.comments.shift();
+        pop = fromDb.populated('comments.author');
+        assert.equal(pop.length, 0);
+
         // And try setting to populated path
         fromDb = yield Page.findOne().populate('comments.author');
         assert.ok(Array.isArray(fromDb.populated('comments.author')));
@@ -8756,6 +8764,10 @@ describe('model: populate:', function() {
         fromDb.comments.push({ author: docs[1] });
         pop = fromDb.populated('comments.author');
         assert.equal(pop.length, 2);
+
+        fromDb.comments.splice(1, 1);
+        pop = fromDb.populated('comments.author');
+        assert.equal(pop.length, 1);
       });
     });
   });