fix: depopulate if push() or addToSet() with an ObjectId on a pop…

…ulated array

Fix #1635

lib/types/array/methods/index.js

@@ -410,6 +410,7 @@ const methods = {
 
   addToSet() {
     _checkManualPopulation(this, arguments);
+    _depopulateIfNecessary(this, arguments);
 
     const values = [].map.call(arguments, this._mapCast, this);
     const added = [];
@@ -691,6 +692,7 @@ const methods = {
     }
 
     _checkManualPopulation(this, values);
+    _depopulateIfNecessary(this, values);
 
     values = [].map.call(values, this._mapCast, this);
     let ret;
@@ -1009,6 +1011,30 @@ function _checkManualPopulation(arr, docs) {
   }
 }
 
+/*!
+ * If `docs` isn't all instances of the right model, depopulate `arr`
+ */
+
+function _depopulateIfNecessary(arr, docs) {
+  const ref = arr == null ?
+    null :
+    arr[arraySchemaSymbol] && arr[arraySchemaSymbol].caster && arr[arraySchemaSymbol].caster.options && arr[arraySchemaSymbol].caster.options.ref || null;
+  const parentDoc = arr[arrayParentSymbol];
+  const path = arr[arrayPathSymbol];
+  if (!ref || !parentDoc.populated(path)) {
+    return;
+  }
+  for (const doc of docs) {
+    if (doc == null) {
+      continue;
+    }
+    if (typeof doc !== 'object' || doc instanceof String || doc instanceof Number || doc instanceof Buffer || utils.isMongooseType(doc)) {
+      parentDoc.depopulate(path);
+      break;
+    }
+  }
+}
+
 const returnVanillaArrayMethods = [
   'filter',
   'flat',

test/model.populate.setting.test.js

@@ -152,7 +152,7 @@ describe('model: populate:', function() {
           assert.equal(doc.fans[6], null);
 
           const _id = construct[id]();
-          doc.fans.addToSet(_id);
+          doc.fans.addToSet({ _id });
           if (Buffer.isBuffer(_id)) {
             assert.equal(doc.fans[7]._id.toString('utf8'), _id.toString('utf8'));
           } else {

test/model.populate.test.js

@@ -11131,4 +11131,30 @@ describe('model: populate:', function() {
     }
     assert.equal(posts.length, 2);
   });
+
+  it('depopulates if pushing ObjectId to a populated array (gh-1635)', async function() {
+    const ParentModel = db.model('Test', mongoose.Schema({
+      name: String,
+      children: [{ type: 'ObjectId', ref: 'Child' }]
+    }));
+    const ChildModel = db.model('Child', mongoose.Schema({ name: String }));
+
+    const children = await ChildModel.create([{ name: 'Luke' }, { name: 'Leia' }]);
+    const newChild = await ChildModel.create({ name: 'Taco' });
+    const { _id } = await ParentModel.create({ name: 'Anakin', children });
+
+    const doc = await ParentModel.findById(_id).populate('children');
+    doc.children.push(newChild._id);
+
+    assert.ok(doc.children[0] instanceof mongoose.Types.ObjectId);
+    assert.ok(doc.children[1] instanceof mongoose.Types.ObjectId);
+    assert.ok(doc.children[2] instanceof mongoose.Types.ObjectId);
+
+    await doc.save();
+
+    const fromDb = await ParentModel.findById(_id);
+    assert.equal(fromDb.children[0].toHexString(), children[0]._id.toHexString());
+    assert.equal(fromDb.children[1].toHexString(), children[1]._id.toHexString());
+    assert.equal(fromDb.children[2].toHexString(), newChild._id.toHexString());
+  });
 });