Skip to content

Fix: Add strictFilter option to findOneAndUpdate (#14913) #15402

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
May 12, 2025
Merged

Fix: Add strictFilter option to findOneAndUpdate (#14913) #15402

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
ce4c3d5
Fix: Add strictFilter option to findOneAndUpdate with tests (#14913)
maazahmed-mangotech May 5, 2025
19fa17b
Add requireFilter for all methods with comprehensive tests, fix befor…
muazahmed-dev May 8, 2025
b826ca2
Merge branch 'Automattic:master' into fix-findOneAndUpdate-empty-filter
muazahmed-dev May 8, 2025
aa64400
Merge branch 'Automattic:master' into fix-findOneAndUpdate-empty-filter
muazahmed-dev May 9, 2025
35018b6
Remove Array.isArray check, add // empty object validation in require…
muazahmed-dev May 9, 2025
458ea69
Merge branch 'fix-findOneAndUpdate-empty-filter' of https://github.co…
muazahmed-dev May 9, 2025
4ab2e4b
Update query.js
vkarpov15 May 12, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 39 additions & 0 deletions lib/query.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,15 @@
}
}

// Helper function to check empty/invalid filter
function checkRequireFilter(filter, options) {
if (options && options.requireFilter &&
(filter == null ||
(typeof filter === 'object' && filter !== null && !Array.isArray(filter) && Object.keys(filter).length === 0))) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think filter can be an array, so I would get rid of this Array.isArray(filter) check.

Also, it would likely be worthwhile to check for empty objects in $and, $or, and $nor as well. For example, deleteOne({ $and: [{}] }) and deleteOne({ $or: [{}] }) is equivalent to deleteOne({}). That would make requireFilter more valuable.

throw new Error('Empty or invalid filter not allowed with requireFilter enabled');
}
}

/*!
* inherit mquery
*/
Expand Down Expand Up @@ -3111,6 +3120,7 @@
*
* @param {Object|Query} [filter] mongodb selector
* @param {Object} [options] optional see [`Query.prototype.setOptions()`](https://mongoosejs.com/docs/api/query.html#Query.prototype.setOptions())
* @param {Boolean} [options.requireFilter=false] If true, throws an error if the filter is empty (`{}`)
* @return {Query} this
* @see DeleteResult https://mongodb.github.io/node-mongodb-native/6.15/interfaces/DeleteResult.html
* @see deleteOne https://mongodb.github.io/node-mongodb-native/6.15/classes/Collection.html#deleteOne
Expand Down Expand Up @@ -3148,6 +3158,9 @@
this._applyTranslateAliases();
this._castConditions();

// Check for empty/invalid filter with requireFilter option
checkRequireFilter(this._conditions, this.options);

if (this.error() != null) {
throw this.error();
}
Expand Down Expand Up @@ -3183,6 +3196,7 @@
*
* @param {Object|Query} [filter] mongodb selector
* @param {Object} [options] optional see [`Query.prototype.setOptions()`](https://mongoosejs.com/docs/api/query.html#Query.prototype.setOptions())
* @param {Boolean} [options.requireFilter=false] If true, throws an error if the filter is empty (`{}`)
* @return {Query} this
* @see DeleteResult https://mongodb.github.io/node-mongodb-native/6.15/interfaces/DeleteResult.html
* @see deleteMany https://mongodb.github.io/node-mongodb-native/6.15/classes/Collection.html#deleteMany
Expand Down Expand Up @@ -3220,6 +3234,9 @@
this._applyTranslateAliases();
this._castConditions();

// Check for empty/invalid filter with requireFilter option
checkRequireFilter(this._conditions, this.options);

Check failure on line 3239 in lib/query.js

View workflow job for this annotation

GitHub Actions / Lint JS-Files 

Trailing spaces not allowed
if (this.error() != null) {
throw this.error();
}
Expand Down Expand Up @@ -3311,6 +3328,7 @@
* - `maxTimeMS`: puts a time limit on the query - requires mongodb >= 2.6.0
* - `runValidators`: if true, runs [update validators](https://mongoosejs.com/docs/validation.html#update-validators) on this command. Update validators validate the update operation against the model's schema.
* - `setDefaultsOnInsert`: `true` by default. If `setDefaultsOnInsert` and `upsert` are true, mongoose will apply the [defaults](https://mongoosejs.com/docs/defaults.html) specified in the model's schema if a new document is created.
* - `requireFilter`: bool - if true, throws an error if the filter is empty (`{}`). Defaults to false.
*
* #### Example:
*
Expand All @@ -3337,6 +3355,7 @@
* @param {Boolean} [options.translateAliases=null] If set to `true`, translates any schema-defined aliases in `filter`, `projection`, `update`, and `distinct`. Throws an error if there are any conflicts where both alias and raw property are defined on the same object.
* @param {Boolean} [options.overwriteDiscriminatorKey=false] Mongoose removes discriminator key updates from `update` by default, set `overwriteDiscriminatorKey` to `true` to allow updating the discriminator key
* @param {Boolean} [options.overwriteImmutable=false] Mongoose removes updated immutable properties from `update` by default (excluding $setOnInsert). Set `overwriteImmutable` to `true` to allow updating immutable properties using other update operators.
* @param {Boolean} [options.requireFilter=false] If true, throws an error if the filter is empty (`{}`)
* @see Tutorial https://mongoosejs.com/docs/tutorials/findoneandupdate.html
* @see findAndModify command https://www.mongodb.com/docs/manual/reference/command/findAndModify/
* @see ModifyResult https://mongodb.github.io/node-mongodb-native/4.9/interfaces/ModifyResult.html
Expand Down Expand Up @@ -3417,6 +3436,9 @@
this._applyTranslateAliases();
this._castConditions();

// Check for empty/invalid filter with requireFilter option
checkRequireFilter(this._conditions, this.options);

_castArrayFilters(this);

if (this.error()) {
Expand Down Expand Up @@ -3500,6 +3522,7 @@
*
* - `sort`: if multiple docs are found by the conditions, sets the sort order to choose which doc to update
* - `maxTimeMS`: puts a time limit on the query - requires mongodb >= 2.6.0
* - `requireFilter`: bool - if true, throws an error if the filter is empty (`{}`). Defaults to false.
*
* #### Example:
*
Expand All @@ -3512,6 +3535,7 @@
* @param {Object} [filter]
* @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} [options.requireFilter=false] If true, throws an error if the filter is empty (`{}`)
* @param {ClientSession} [options.session=null] The session associated with this query. See [transactions docs](https://mongoosejs.com/docs/transactions.html).
* @param {Boolean|String} [options.strict] overwrites the schema's [strict mode option](https://mongoosejs.com/docs/guide.html#strict)
* @return {Query} this
Expand Down Expand Up @@ -3551,6 +3575,9 @@
this._applyTranslateAliases();
this._castConditions();

// Check for empty/invalid filter with requireFilter option
checkRequireFilter(this._conditions, this.options);

if (this.error() != null) {
throw this.error();
}
Expand Down Expand Up @@ -3590,6 +3617,7 @@
* - `sort`: if multiple docs are found by the conditions, sets the sort order to choose which doc to update
* - `maxTimeMS`: puts a time limit on the query - requires mongodb >= 2.6.0
* - `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
* - `requireFilter`: bool - if true, throws an error if the filter is empty (`{}`). Defaults to false.
*
* #### Example:
*
Expand All @@ -3612,6 +3640,7 @@
* @param {Boolean} [options.timestamps=null] If set to `false` and [schema-level timestamps](https://mongoosejs.com/docs/guide.html#timestamps) are enabled, skip timestamps for this update. Note that this allows you to overwrite timestamps. Does nothing if schema-level timestamps are not set.
* @param {Boolean} [options.returnOriginal=null] An alias for the `new` option. `returnOriginal: false` is equivalent to `new: true`.
* @param {Boolean} [options.translateAliases=null] If set to `true`, translates any schema-defined aliases in `filter`, `projection`, `update`, and `distinct`. Throws an error if there are any conflicts where both alias and raw property are defined on the same object.
* @param {Boolean} [options.requireFilter=false] If true, throws an error if the filter is empty (`{}`)
* @return {Query} this
* @api public
*/
Expand Down Expand Up @@ -3667,6 +3696,10 @@
Query.prototype._findOneAndReplace = async function _findOneAndReplace() {
this._applyTranslateAliases();
this._castConditions();

// Check for empty/invalid filter with requireFilter option
checkRequireFilter(this._conditions, this.options);

if (this.error() != null) {
throw this.error();
}
Expand Down Expand Up @@ -3969,6 +4002,9 @@

this._castConditions();

// Check for empty/invalid filter with requireFilter option
checkRequireFilter(this._conditions, this.options);

_castArrayFilters(this);

if (this.error() != null) {
Expand Down Expand Up @@ -4119,6 +4155,7 @@
* @param {Boolean} [options.translateAliases=null] If set to `true`, translates any schema-defined aliases in `filter`, `projection`, `update`, and `distinct`. Throws an error if there are any conflicts where both alias and raw property are defined on the same object.
* @param {Boolean} [options.overwriteDiscriminatorKey=false] Mongoose removes discriminator key updates from `update` by default, set `overwriteDiscriminatorKey` to `true` to allow updating the discriminator key
* @param {Boolean} [options.overwriteImmutable=false] Mongoose removes updated immutable properties from `update` by default (excluding $setOnInsert). Set `overwriteImmutable` to `true` to allow updating immutable properties using other update operators.
* @param {Boolean} [options.requireFilter=false] If true, throws an error if the filter is empty (`{}`)
* @return {Query} this
* @see Model.update https://mongoosejs.com/docs/api/model.html#Model.update()
* @see Query docs https://mongoosejs.com/docs/queries.html
Expand Down Expand Up @@ -4193,6 +4230,7 @@
* @param {Boolean} [options.translateAliases=null] If set to `true`, translates any schema-defined aliases in `filter`, `projection`, `update`, and `distinct`. Throws an error if there are any conflicts where both alias and raw property are defined on the same object.
* @param {Boolean} [options.overwriteDiscriminatorKey=false] Mongoose removes discriminator key updates from `update` by default, set `overwriteDiscriminatorKey` to `true` to allow updating the discriminator key
* @param {Boolean} [options.overwriteImmutable=false] Mongoose removes updated immutable properties from `update` by default (excluding $setOnInsert). Set `overwriteImmutable` to `true` to allow updating immutable properties using other update operators.
* @param {Boolean} [options.requireFilter=false] If true, throws an error if the filter is empty (`{}`)
* @return {Query} this
* @see Model.update https://mongoosejs.com/docs/api/model.html#Model.update()
* @see Query docs https://mongoosejs.com/docs/queries.html
Expand Down Expand Up @@ -4259,6 +4297,7 @@
* @param {Object} [options.writeConcern=null] sets the [write concern](https://www.mongodb.com/docs/manual/reference/write-concern/) for replica sets. Overrides the [schema-level write concern](https://mongoosejs.com/docs/guide.html#writeConcern)
* @param {Boolean} [options.timestamps=null] If set to `false` and [schema-level timestamps](https://mongoosejs.com/docs/guide.html#timestamps) are enabled, skip timestamps for this update. Does nothing if schema-level timestamps are not set.
* @param {Boolean} [options.translateAliases=null] If set to `true`, translates any schema-defined aliases in `filter`, `projection`, `update`, and `distinct`. Throws an error if there are any conflicts where both alias and raw property are defined on the same object.
* @param {Boolean} [options.requireFilter=false] If true, throws an error if the filter is empty (`{}`)
* @return {Query} this
* @see Model.update https://mongoosejs.com/docs/api/model.html#Model.update()
* @see Query docs https://mongoosejs.com/docs/queries.html
Expand Down
Loading
Loading