From 2a55ba1b26ba55d55b775c2f4a0421bb3271596c Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Thu, 23 Jan 2025 13:52:47 -0500 Subject: [PATCH] fix(model): disallow `updateMany(update)` and fix TypeScript types re `updateMany()` Fix #15190 --- lib/model.js | 16 ++++++++++++++-- lib/query.js | 8 ++++++++ test/model.middleware.preposttypes.test.js | 4 ++-- test/model.test.js | 9 +++++++++ test/query.test.js | 2 +- types/models.d.ts | 11 +++++++---- types/query.d.ts | 14 ++++++++++---- 7 files changed, 51 insertions(+), 13 deletions(-) diff --git a/lib/model.js b/lib/model.js index 8d38d9ee083..45b577777e2 100644 --- a/lib/model.js +++ b/lib/model.js @@ -3878,6 +3878,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()` @@ -3898,10 +3902,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); }; /** @@ -3918,6 +3926,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 index 067a6e020ed..7ea8403b40b 100644 --- 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 index 952bc901001..93a42f8dc1f 100644 --- 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 index af518fe7340..4437641d30a 100644 --- a/test/model.test.js +++ b/test/model.test.js @@ -8419,6 +8419,15 @@ describe('Model', function() { assert.deepStrictEqual(toDrop, []); }); }); + + 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' } + ); + }); }); diff --git a/test/query.test.js b/test/query.test.js index bca5f706cfd..13b5b677b78 100644 --- 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 index fde1ad26124..1331ffc2e78 100644 --- a/types/models.d.ts +++ b/types/models.d.ts @@ -869,17 +869,20 @@ declare module 'mongoose' { /** Creates a `updateMany` query: updates all documents that match `filter` with `update`. */ updateMany( - filter?: RootFilterQuery, - update?: UpdateQuery | UpdateWithAggregationPipeline, + filter: RootFilterQuery, + update: UpdateQuery | UpdateWithAggregationPipeline, options?: (mongodb.UpdateOptions & MongooseUpdateQueryOptions) | null ): QueryWithHelpers; /** Creates a `updateOne` query: updates the first document that matches `filter` with `update`. */ updateOne( - filter?: RootFilterQuery, - update?: UpdateQuery | UpdateWithAggregationPipeline, + filter: RootFilterQuery, + update: UpdateQuery | UpdateWithAggregationPipeline, options?: (mongodb.UpdateOptions & MongooseUpdateQueryOptions) | null ): QueryWithHelpers; + updateOne( + update: UpdateQuery | UpdateWithAggregationPipeline + ): QueryWithHelpers; /** Creates a Query, applies the passed conditions, and returns the Query. */ where( diff --git a/types/query.d.ts b/types/query.d.ts index fbebf1b6467..78df823f5c0 100644 --- a/types/query.d.ts +++ b/types/query.d.ts @@ -850,20 +850,26 @@ declare module 'mongoose' { * the `multi` option. */ updateMany( - filter?: RootFilterQuery, - update?: UpdateQuery | UpdateWithAggregationPipeline, + filter: RootFilterQuery, + update: UpdateQuery | UpdateWithAggregationPipeline, options?: QueryOptions | null ): QueryWithHelpers; + updateMany( + update: UpdateQuery | UpdateWithAggregationPipeline + ): QueryWithHelpers; /** * 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, - update?: UpdateQuery | UpdateWithAggregationPipeline, + filter: RootFilterQuery, + update: UpdateQuery | UpdateWithAggregationPipeline, options?: QueryOptions | null ): QueryWithHelpers; + updateOne( + update: UpdateQuery | UpdateWithAggregationPipeline + ): QueryWithHelpers; /** * Sets the specified number of `mongod` servers, or tag set of `mongod` servers,