Merge pull request #15586 from Automattic/vkarpov15/gh-14424

BREAKING CHANGE: disallow update pipelines by default, require updatePipeline option

Author: vkarpov15

docs/migrating_to_9.md

@@ -68,6 +68,23 @@ schema.pre('save', function(next, arg) {
 
 In Mongoose 9, `next(null, 'new arg')` doesn't overwrite the args to the next middleware.
 
+## Update pipelines disallowed by default
+
+As of MongoDB 4.2, you can pass an array of pipeline stages to `updateOne()`, `updateMany()`, and `findOneAndUpdate()` to modify the document in multiple stages.
+Mongoose does not cast update pipelines at all, so for Mongoose 9 we've made using update pipelines throw an error by default.
+
+```javascript
+// Throws in Mongoose 9. Works in Mongoose 8
+await Model.updateOne({}, [{ $set: { newProp: 'test2' } }]);
+```
+
+Set `updatePipeline: true` to enable update pipelines.
+
+```javascript
+// Works in Mongoose 9
+await Model.updateOne({}, [{ $set: { newProp: 'test2' } }], { updatePipeline: true });
+```
+
 ## Removed background option for indexes
 
 [MongoDB no longer supports the `background` option for indexes as of MongoDB 4.2](https://www.mongodb.com/docs/manual/core/index-creation/#index-operations). Mongoose 9 will no longer set the background option by default and Mongoose 9 no longer supports setting the `background` option on `Schema.prototype.index()`.

lib/query.js

@@ -1748,6 +1748,10 @@ Query.prototype.setOptions = function(options, overwrite) {
     this._mongooseOptions.overwriteImmutable = options.overwriteImmutable;
     delete options.overwriteImmutable;
   }
+  if ('updatePipeline' in options) {
+    this._mongooseOptions.updatePipeline = options.updatePipeline;
+    delete options.updatePipeline;
+  }
   if ('sanitizeProjection' in options) {
     if (options.sanitizeProjection && !this._mongooseOptions.sanitizeProjection) {
       sanitizeProjection(this._fields);
@@ -3385,7 +3389,7 @@ function prepareDiscriminatorCriteria(query) {
  * @memberOf Query
  * @instance
  * @param {Object|Query} [filter]
- * @param {Object} [doc]
+ * @param {Object} [update]
  * @param {Object} [options]
  * @param {Boolean} [options.includeResultMetadata] if true, returns the full [ModifyResult from the MongoDB driver](https://mongodb.github.io/node-mongodb-native/4.9/interfaces/ModifyResult.html) rather than just the document
  * @param {Boolean|String} [options.strict] overwrites the schema's [strict mode option](https://mongoosejs.com/docs/guide.html#strict)
@@ -3407,9 +3411,9 @@ function prepareDiscriminatorCriteria(query) {
  * @api public
  */
 
-Query.prototype.findOneAndUpdate = function(filter, doc, options) {
+Query.prototype.findOneAndUpdate = function(filter, update, options) {
   if (typeof filter === 'function' ||
-      typeof doc === 'function' ||
+      typeof update === 'function' ||
       typeof options === 'function' ||
       typeof arguments[3] === 'function') {
     throw new MongooseError('Query.prototype.findOneAndUpdate() no longer accepts a callback');
@@ -3423,7 +3427,7 @@ Query.prototype.findOneAndUpdate = function(filter, doc, options) {
       options = undefined;
       break;
     case 1:
-      doc = filter;
+      update = filter;
       filter = options = undefined;
       break;
   }
@@ -3436,11 +3440,6 @@ Query.prototype.findOneAndUpdate = function(filter, doc, options) {
     );
   }
 
-  // apply doc
-  if (doc) {
-    this._mergeUpdate(doc);
-  }
-
   options = options ? clone(options) : {};
 
   if (options.projection) {
@@ -3463,6 +3462,11 @@ Query.prototype.findOneAndUpdate = function(filter, doc, options) {
 
   this.setOptions(options);
 
+  // apply doc
+  if (update) {
+    this._mergeUpdate(update);
+  }
+
   return this;
 };
 
@@ -3997,39 +4001,43 @@ function _completeManyLean(schema, docs, path, opts) {
  * Override mquery.prototype._mergeUpdate to handle mongoose objects in
  * updates.
  *
- * @param {Object} doc
+ * @param {Object} update
  * @method _mergeUpdate
  * @memberOf Query
  * @instance
  * @api private
  */
 
-Query.prototype._mergeUpdate = function(doc) {
+Query.prototype._mergeUpdate = function(update) {
+  const updatePipeline = this._mongooseOptions.updatePipeline;
+  if (!updatePipeline && Array.isArray(update)) {
+    throw new MongooseError('Cannot pass an array to query updates unless the `updatePipeline` option is set.');
+  }
   if (!this._update) {
-    this._update = Array.isArray(doc) ? [] : {};
+    this._update = Array.isArray(update) ? [] : {};
   }
 
-  if (doc == null || (typeof doc === 'object' && Object.keys(doc).length === 0)) {
+  if (update == null || (typeof update === 'object' && Object.keys(update).length === 0)) {
     return;
   }
 
-  if (doc instanceof Query) {
+  if (update instanceof Query) {
     if (Array.isArray(this._update)) {
-      throw new Error('Cannot mix array and object updates');
+      throw new MongooseError('Cannot mix array and object updates');
     }
-    if (doc._update) {
-      utils.mergeClone(this._update, doc._update);
+    if (update._update) {
+      utils.mergeClone(this._update, update._update);
     }
-  } else if (Array.isArray(doc)) {
+  } else if (Array.isArray(update)) {
     if (!Array.isArray(this._update)) {
-      throw new Error('Cannot mix array and object updates');
+      throw new MongooseError('Cannot mix array and object updates');
     }
-    this._update = this._update.concat(doc);
+    this._update = this._update.concat(update);
   } else {
     if (Array.isArray(this._update)) {
-      throw new Error('Cannot mix array and object updates');
+      throw new MongooseError('Cannot mix array and object updates');
     }
-    utils.mergeClone(this._update, doc);
+    utils.mergeClone(this._update, update);
   }
 };
 

test/model.updateOne.test.js

@@ -2766,10 +2766,16 @@ describe('model: updateOne: ', function() {
       const Model = db.model('Test', schema);
 
       await Model.create({ oldProp: 'test' });
+
+      assert.throws(
+        () => Model.updateOne({}, [{ $set: { newProp: 'test2' } }]),
+        /Cannot pass an array to query updates unless the `updatePipeline` option is set/
+      );
+
       await Model.updateOne({}, [
         { $set: { newProp: 'test2' } },
         { $unset: ['oldProp'] }
-      ]);
+      ], { updatePipeline: true });
       let doc = await Model.findOne();
       assert.equal(doc.newProp, 'test2');
       assert.strictEqual(doc.oldProp, void 0);
@@ -2778,7 +2784,7 @@ describe('model: updateOne: ', function() {
       await Model.updateOne({}, [
         { $addFields: { oldProp: 'test3' } },
         { $project: { newProp: 0 } }
-      ]);
+      ], { updatePipeline: true });
       doc = await Model.findOne();
       assert.equal(doc.oldProp, 'test3');
       assert.strictEqual(doc.newProp, void 0);
@@ -2792,7 +2798,7 @@ describe('model: updateOne: ', function() {
       await Model.updateOne({}, [
         { $set: { newProp: 'test2' } },
         { $unset: 'oldProp' }
-      ]);
+      ], { updatePipeline: true });
       const doc = await Model.findOne();
       assert.equal(doc.newProp, 'test2');
       assert.strictEqual(doc.oldProp, void 0);
@@ -2805,8 +2811,11 @@ describe('model: updateOne: ', function() {
       const updatedAt = cat.updatedAt;
 
       await new Promise(resolve => setTimeout(resolve), 50);
-      const updated = await Cat.findOneAndUpdate({ _id: cat._id },
-        [{ $set: { name: 'Raikou' } }], { new: true });
+      const updated = await Cat.findOneAndUpdate(
+        { _id: cat._id },
+        [{ $set: { name: 'Raikou' } }],
+        { new: true, updatePipeline: true }
+      );
       assert.ok(updated.updatedAt.getTime() > updatedAt.getTime());
     });
   });

test/timestamps.test.js

@@ -1034,9 +1034,7 @@ describe('timestamps', function() {
       sub: { subName: 'John' }
     });
     await doc.save();
-    await Test.updateMany({}, [{ $set: { updateCounter: 1 } }]);
-    // oddly enough, the null property is not accessible. Doing check.null doesn't return anything even though
-    // if you were to console.log() the output of a findOne you would be able to see it. This is the workaround.
+    await Test.updateMany({}, [{ $set: { updateCounter: 1 } }], { updatePipeline: true });
     const test = await Test.countDocuments({ null: { $exists: true } });
     assert.equal(test, 0);
     // now we need to make sure that the solution didn't prevent the updateCounter addition

types/query.d.ts

@@ -43,7 +43,8 @@ declare module 'mongoose' {
     | 'setDefaultsOnInsert'
     | 'strict'
     | 'strictQuery'
-    | 'translateAliases';
+    | 'translateAliases'
+    | 'updatePipeline';
 
   type MongooseBaseQueryOptions<DocType = unknown> = Pick<QueryOptions<DocType>, MongooseBaseQueryOptionKeys>;
 
@@ -219,6 +220,11 @@ declare module 'mongoose' {
     translateAliases?: boolean;
     upsert?: boolean;
     useBigInt64?: boolean;
+    /**
+     * Set to true to allow passing in an update pipeline instead of an update document.
+     * Mongoose disallows update pipelines by default because Mongoose does not cast update pipelines.
+     */
+    updatePipeline?: boolean;
     writeConcern?: mongodb.WriteConcern;
 
     [other: string]: any;