Merge pull request #15199 from Automattic/vkarpov15/gh-15190

fix(model): disallow `updateMany(update)` and fix TypeScript types re: `updateMany()`
Automattic · Jan 24, 2025 · fa05375 · fa05375
2 parents 3e4ba3f + acbb613
commit fa05375
Show file tree

Hide file tree

Showing 7 changed files with 51 additions and 13 deletions.
diff --git a/lib/model.js b/lib/model.js
@@ -3982,6 +3982,10 @@ Model.hydrate = function(obj, projection, options) {
  *     res.upsertedId; // null or an id containing a document that had to be upserted.
  *     res.upsertedCount; // Number indicating how many documents had to be upserted. Will either be 0 or 1.
  *
+ *     // Other supported syntaxes
+ *     await Person.find({ name: /Stark$/ }).updateMany({ isDeleted: true }); // Using chaining syntax
+ *     await Person.find().updateMany({ isDeleted: true }); // Set `isDeleted` on _all_ Person documents
+ *
  * This function triggers the following middleware.
  *
  * - `updateMany()`
@@ -4002,10 +4006,14 @@ Model.hydrate = function(obj, projection, options) {
  * @api public
  */
 
-Model.updateMany = function updateMany(conditions, doc, options) {
+Model.updateMany = function updateMany(conditions, update, options) {
   _checkContext(this, 'updateMany');
 
-  return _update(this, 'updateMany', conditions, doc, options);
+  if (update == null) {
+    throw new MongooseError('updateMany `update` parameter cannot be nullish');
+  }
+
+  return _update(this, 'updateMany', conditions, update, options);
 };
 
 /**
@@ -4022,6 +4030,10 @@ Model.updateMany = function updateMany(conditions, doc, options) {
  *     res.upsertedId; // null or an id containing a document that had to be upserted.
  *     res.upsertedCount; // Number indicating how many documents had to be upserted. Will either be 0 or 1.
  *
+ *     // Other supported syntaxes
+ *     await Person.findOne({ name: 'Jean-Luc Picard' }).updateOne({ ship: 'USS Enterprise' }); // Using chaining syntax
+ *     await Person.updateOne({ ship: 'USS Enterprise' }); // Updates first doc's `ship` property
+ *
  * This function triggers the following middleware.
  *
  * - `updateOne()`

diff --git a/lib/query.js b/lib/query.js
@@ -3992,6 +3992,10 @@ Query.prototype._replaceOne = async function _replaceOne() {
  *     res.n; // Number of documents matched
  *     res.nModified; // Number of documents modified
  *
+ *     // Other supported syntaxes
+ *     await Person.find({ name: /Stark$/ }).updateMany({ isDeleted: true }); // Using chaining syntax
+ *     await Person.find().updateMany({ isDeleted: true }); // Set `isDeleted` on _all_ Person documents
+ *
  * This function triggers the following middleware.
  *
  * - `updateMany()`
@@ -4062,6 +4066,10 @@ Query.prototype.updateMany = function(conditions, doc, options, callback) {
  *     res.upsertedCount; // Number of documents that were upserted
  *     res.upsertedId; // Identifier of the inserted document (if an upsert took place)
  *
+ *     // Other supported syntaxes
+ *     await Person.findOne({ name: 'Jean-Luc Picard' }).updateOne({ ship: 'USS Enterprise' }); // Using chaining syntax
+ *     await Person.updateOne({ ship: 'USS Enterprise' }); // Updates first doc's `ship` property
+ *
  * This function triggers the following middleware.
  *
  * - `updateOne()`

diff --git a/test/model.middleware.preposttypes.test.js b/test/model.middleware.preposttypes.test.js
@@ -288,8 +288,8 @@ describe('pre/post hooks, type of this', function() {
       await Doc.findOneAndReplace({}, { data: 'valueRep' }).exec();
       await Doc.findOneAndUpdate({}, { data: 'valueUpd' }).exec();
       await Doc.replaceOne({}, { data: 'value' }).exec();
-      await Doc.updateOne({ data: 'value' }).exec();
-      await Doc.updateMany({ data: 'value' }).exec();
+      await Doc.updateOne({}, { data: 'value' }).exec();
+      await Doc.updateMany({}, { data: 'value' }).exec();
 
       // MongooseQueryOrDocumentMiddleware, use Query
       await Doc.deleteOne({}).exec(); await Doc.create({ data: 'value' });

diff --git a/test/model.test.js b/test/model.test.js
@@ -8505,6 +8505,15 @@ describe('Model', function() {
     });
   });
 
+  it('throws error if calling `updateMany()` with no update param (gh-15190)', async function() {
+    const Test = db.model('Test', mongoose.Schema({ foo: String }));
+
+    assert.throws(
+      () => Test.updateMany({ foo: 'bar' }),
+      { message: 'updateMany `update` parameter cannot be nullish' }
+    );
+  });
+
   describe('insertOne() (gh-14843)', function() {
     it('should insert a new document', async function() {
       const userSchema = new Schema({

diff --git a/test/query.test.js b/test/query.test.js
@@ -1962,7 +1962,7 @@ describe('Query', function() {
       });
 
       schema.pre('deleteOne', { document: true, query: false }, async function() {
-        await this.constructor.updateOne({ isDeleted: true });
+        await this.updateOne({ isDeleted: true });
         this.$isDeleted(true);
       });
 

diff --git a/types/models.d.ts b/types/models.d.ts
@@ -883,17 +883,20 @@ declare module 'mongoose' {
 
     /** Creates a `updateMany` query: updates all documents that match `filter` with `update`. */
     updateMany<ResultDoc = THydratedDocumentType>(
-      filter?: RootFilterQuery<TRawDocType>,
-      update?: UpdateQuery<TRawDocType> | UpdateWithAggregationPipeline,
+      filter: RootFilterQuery<TRawDocType>,
+      update: UpdateQuery<TRawDocType> | UpdateWithAggregationPipeline,
       options?: (mongodb.UpdateOptions & MongooseUpdateQueryOptions<TRawDocType>) | null
     ): QueryWithHelpers<UpdateWriteOpResult, ResultDoc, TQueryHelpers, TRawDocType, 'updateMany', TInstanceMethods & TVirtuals>;
 
     /** Creates a `updateOne` query: updates the first document that matches `filter` with `update`. */
     updateOne<ResultDoc = THydratedDocumentType>(
-      filter?: RootFilterQuery<TRawDocType>,
-      update?: UpdateQuery<TRawDocType> | UpdateWithAggregationPipeline,
+      filter: RootFilterQuery<TRawDocType>,
+      update: UpdateQuery<TRawDocType> | UpdateWithAggregationPipeline,
       options?: (mongodb.UpdateOptions & MongooseUpdateQueryOptions<TRawDocType>) | null
     ): QueryWithHelpers<UpdateWriteOpResult, ResultDoc, TQueryHelpers, TRawDocType, 'updateOne', TInstanceMethods & TVirtuals>;
+    updateOne<ResultDoc = THydratedDocumentType>(
+      update: UpdateQuery<TRawDocType> | UpdateWithAggregationPipeline
+    ): QueryWithHelpers<UpdateWriteOpResult, ResultDoc, TQueryHelpers, TRawDocType, 'updateOne', TInstanceMethods & TVirtuals>;
 
     /** Creates a Query, applies the passed conditions, and returns the Query. */
     where<ResultDoc = THydratedDocumentType>(

diff --git a/types/query.d.ts b/types/query.d.ts
@@ -850,20 +850,26 @@ declare module 'mongoose' {
      * the `multi` option.
      */
     updateMany(
-      filter?: RootFilterQuery<RawDocType>,
-      update?: UpdateQuery<RawDocType> | UpdateWithAggregationPipeline,
+      filter: RootFilterQuery<RawDocType>,
+      update: UpdateQuery<RawDocType> | UpdateWithAggregationPipeline,
       options?: QueryOptions<DocType> | null
     ): QueryWithHelpers<UpdateWriteOpResult, DocType, THelpers, RawDocType, 'updateMany', TDocOverrides>;
+    updateMany(
+      update: UpdateQuery<RawDocType> | UpdateWithAggregationPipeline
+    ): QueryWithHelpers<UpdateWriteOpResult, DocType, THelpers, RawDocType, 'updateMany', TDocOverrides>;
 
     /**
      * Declare and/or execute this query as an updateOne() operation. Same as
      * `update()`, except it does not support the `multi` or `overwrite` options.
      */
     updateOne(
-      filter?: RootFilterQuery<RawDocType>,
-      update?: UpdateQuery<RawDocType> | UpdateWithAggregationPipeline,
+      filter: RootFilterQuery<RawDocType>,
+      update: UpdateQuery<RawDocType> | UpdateWithAggregationPipeline,
       options?: QueryOptions<DocType> | null
     ): QueryWithHelpers<UpdateWriteOpResult, DocType, THelpers, RawDocType, 'updateOne', TDocOverrides>;
+    updateOne(
+      update: UpdateQuery<RawDocType> | UpdateWithAggregationPipeline
+    ): QueryWithHelpers<UpdateWriteOpResult, DocType, THelpers, RawDocType, 'updateOne', TDocOverrides>;
 
     /**
      * Sets the specified number of `mongod` servers, or tag set of `mongod` servers,