Bug in required field validation in findOneAndReplace

[TarSzator]

Prerequisites

• I have written a descriptive issue title
• I have searched existing issues to ensure the bug has not already been reported

Mongoose version

7.4.2

Node.js version

18.17.0

MongoDB server version

5.0

Typescript version (if applicable)

5.1.6

Description

We use findOneAndUpdate with the overwrite option set to true (so equal to findOneAndReplace) to update.
We also set the new and runValidators option to true.

Then in our update block we use $set and $push in some cases also $setOnInsert to modify the entry.

Sadly since the update to mongoose v.7.4.1 and also after the update to v7.4.2 we get errors that required fields are not set.
In all cases this is not true. Either because the fields are in the the entry already and are not touched by the update at all, nor in the $setOnInsert cases because the fields are in the filter object. In these last cases also additionally adding them to the $set portion of the update object did not help.

As far as I understand the documentation the required validators should not be executed in these cases at all.
I like the idea that they are but I understand that this is not easy to achieve and the bug shows that the current way does not work and was probably not intended.

Steps to Reproduce

Sadly I can not share the company code but it is easy to reproduce by creating a model with some required fields.
Then add an entry to the collection.
Then try to update it with the options and update objects described in the Description of this issue.

Expected Behavior

No required fields errors in the named cases mentioned in the Description

[TarSzator]

I found the the error started to happen in v7.3.2
v7.3.1 is fine.

7.3.1...7.3.2

[TarSzator]

It seems to be only affected by the overwrite flag combined with the useValidators flag.

When you have the properties directly in the update object without $set etc. then it works fine

[TarSzator]

Will try to find the time to create a script for reproduction

[IslandRhythms]

const mongoose = require('mongoose');

const testSchema = new mongoose.Schema({
  name: { 
    type: String,
    required: true
  },
  age: {
    type: String,
    required: true
  }
});

const Test = mongoose.model('Test', testSchema);

async function run() {
  await mongoose.connect('mongodb://localhost:27017');
  await mongoose.connection.dropDatabase();

  await Test.create({
    name: 'Test Testerson',
    age: 100
  });

  await Test.findOneAndUpdate({}, { $set: { name: 'Test' } }, { new: true, runValidators: true, overwrite: true })
}

run();

[TarSzator]

Thanks @IslandRhythms Did not came around to it.

[vkarpov15]

@TarSzator this is expected behavior. Without runValidators, await Test.findOneAndUpdate({}, { $set: { name: 'Test' } }, { new: true, overwrite: true }) is equivalent to await Test.findOneAndReplace({}, {}, { new : true }). So replace with empty doc.

That's because Mongoose strict mode filters out unknown properties in the replacement doc, and $set is an unknown property. If Mongoose didn't filter out $set, then the MongoDB Node driver would throw MongoInvalidArgumentError: Replacement document must not contain atomic operators because you're not allowed to use atomic operators in findOneAndReplace().

So long story short, if you're using findOneAndUpdate() with overwrite (equivalent to findOneAndReplace()), you can't use $set, $push, or $setOnInsert. Which is not surprising, because $push and $setOnInsert don't make sense when you're replacing the whole document anyway. If you want to use update operators, don't use overwrite.

[TarSzator]

Thats only half the issue!

1. Mongoose documentation states here By default, if you don't include any [update operators](https://www.mongodb.com/docs/manual/reference/operator/update/) in update, Mongoose will wrap update in $set for you. This prevents you from accidentally overwriting the document. This option tells Mongoose to skip adding $set. An alternative to this would be using Model.findOneAndReplace. This is not true then right? It is not just not doing the $set injection, it will completely not use findOneAndUpdate but use MongoDBs findOneAndReplace. So a better text would be something like. This option is deprecated! If you do not want $set to be injected then use findOneAndReplace
2. Then it states that for runValidators, update validation is run. Here the documentation states that When using update validators, required validators only fail when you try to explicitly $unset the key.

So either the documentation needs updates or it is a bug

@vkarpov15 Why are you so fast in closing a ticket instead of asking the author for if this solves the issue and if he does not response for 2 days you close the ticket?

[vkarpov15]

@TarSzator I'm sorry, you make a fair point that our documentation is out of date.

In older versions of Mongoose and MongoDB (Mongoose 5 and older), findOneAndUpdate() was a wrapper for MongoDB's findAndModify() command, which could behave either as findOneAndUpdate() or findOneAndReplace() depending on the presence of update operators like $set. However, given that caused a fair number of unintentional overwriting of documents, MongoDB added separate findOneAndUpdate() and findOneAndReplace() commands; they removed findAndModify. So now, the MongoDB server prevents you from calling findOneAndUpdate() without atomic operators, nothing Mongoose can do about that. So all we can do is update our docs.

[TarSzator]

Since the ticket is now marked a "doc" (Thanks for that 😃 ) I would like to add another thing. Maybe I overlooked it, but when findOneAndUpdate is used with upsert and update operators like $set and $setOnInsert is the query merged into $setOnInsert. Did not remember seeing an explicit statement to that in the doc.

[vkarpov15]

What do you mean by "is the query merged into $setOnInsert"? I don't quite understand.

[TarSzator]

When you check the documentation here, it is stated that MongoDB will insert one by combining filter and update. This is definitly true for the case that update does not include update operations but what if it does? Would be nice to have a sentence in the documentation for this case.

[vkarpov15]

That merging happens on the MongoDB server. await Character.findOneAndUpdate({ name: 'Will Riker' }, { age: 29 }, { upsert: true }) is equivalent to await Character.findOneAndUpdate({ name: 'Will Riker' }, { $set: { age: 29 } }, { upsert: true })

[TarSzator]

Ohhh sorry. Did not know that this is controlled via MongoDB and not Mongoose.

[TarSzator]

Thanks @vkarpov15 for the work you put into this