Merge pull request #12419 from Automattic/vkarpov15/gh-12294

Improve handling of defaults when using `pull()`

lib/types/array/methods/index.js

@@ -5,6 +5,7 @@ const ArraySubdocument = require('../../ArraySubdocument');
 const MongooseError = require('../../../error/mongooseError');
 const cleanModifiedSubpaths = require('../../../helpers/document/cleanModifiedSubpaths');
 const internalToObjectOptions = require('../../../options').internalToObjectOptions;
+const mpath = require('mpath');
 const utils = require('../../../utils');
 const isBsonType = require('../../../helpers/isBsonType');
 
@@ -341,8 +342,23 @@ const methods = {
       const pullOp = atomics['$pull'] || (atomics['$pull'] = {});
       if (val[0] instanceof ArraySubdocument) {
         selector = pullOp['$or'] || (pullOp['$or'] = []);
-        Array.prototype.push.apply(selector, val.map(function(v) {
-          return v.toObject({ transform: false, virtuals: false });
+        Array.prototype.push.apply(selector, val.map(v => {
+          return v.toObject({
+            transform: (doc, ret) => {
+              if (v == null || v.$__ == null) {
+                return ret;
+              }
+
+              Object.keys(v.$__.activePaths.getStatePaths('default')).forEach(path => {
+                mpath.unset(path, ret);
+
+                _minimizePath(ret, path);
+              });
+
+              return ret;
+            },
+            virtuals: false
+          });
         }));
       } else {
         selector = pullOp['_id'] || (pullOp['_id'] = { $in: [] });
@@ -590,7 +606,11 @@ const methods = {
 
     if (values[0] instanceof ArraySubdocument) {
       this._registerAtomic('$pullDocs', values.map(function(v) {
-        return v.$__getValue('_id') || v;
+        const _id = v.$__getValue('_id');
+        if (_id === undefined || v.$isDefault('_id')) {
+          return v;
+        }
+        return _id;
       }));
     } else {
       this._registerAtomic('$pullAll', values);
@@ -920,6 +940,40 @@ function _isAllSubdocs(docs, ref) {
   return true;
 }
 
+/*!
+ * Minimize _just_ empty objects along the path chain specified
+ * by `parts`, ignoring all other paths. Useful in cases where
+ * you want to minimize after unsetting a path.
+ *
+ * #### Example:
+ *
+ *     const obj = { foo: { bar: { baz: {} } }, a: {} };
+ *     _minimizePath(obj, 'foo.bar.baz');
+ *     obj; // { a: {} }
+ */
+
+function _minimizePath(obj, parts, i) {
+  if (typeof parts === 'string') {
+    if (parts.indexOf('.') === -1) {
+      return;
+    }
+
+    parts = mpath.stringToParts(parts);
+  }
+  i = i || 0;
+  if (i >= parts.length) {
+    return;
+  }
+  if (obj == null || typeof obj !== 'object') {
+    return;
+  }
+
+  _minimizePath(obj[parts[0]], parts, i + 1);
+  if (obj[parts[0]] != null && typeof obj[parts[0]] === 'object' && Object.keys(obj[parts[0]]).length === 0) {
+    delete obj[parts[0]];
+  }
+}
+
 /*!
  * ignore
  */

test/types.array.test.js

@@ -822,6 +822,56 @@ describe('types array', function() {
         });
       });
     });
+
+    it('avoids adding default paths to query filter (gh-12294)', async function() {
+      const catschema = new Schema({
+        name: String,
+        colors: [{
+          _id: false,
+          hex: { type: String, default: '#ffffff' },
+          properties: {
+            hue: { type: Number, default: 0 }
+          },
+          name: String
+        }]
+      });
+      const Cat = db.model('Test', catschema);
+
+      const cat = new Cat({});
+      cat.init({
+        name: 'Garfield',
+        colors: [{ name: 'Orange' }]
+      });
+
+      cat.colors.pull({ name: 'Orange' });
+      assert.deepStrictEqual(cat.colors.$__getAtomics(), [[
+        '$pull',
+        { $or: [{ name: 'Orange' }] }
+      ]]);
+    });
+
+    it('avoids adding default paths to query filter with _id (gh-12294)', async function() {
+      const catschema = new Schema({
+        name: String,
+        colors: [{
+          hex: { type: String, default: '#ffffff' },
+          name: String
+        }]
+      });
+      const Cat = db.model('Test', catschema);
+
+      const cat = new Cat({});
+      cat.init({
+        name: 'Garfield',
+        colors: [{ name: 'Orange' }]
+      });
+
+      cat.colors.pull({ name: 'Orange' });
+      assert.deepStrictEqual(cat.colors.$__getAtomics(), [[
+        '$pull',
+        { $or: [{ name: 'Orange' }] }
+      ]]);
+    });
   });
 
   describe('$pop()', function() {