Mongoose: Saving document with projected sub document modifies the wrong sub document

[mseegers]

Hi,

when retrieving a document using findOne while limiting the result of an array of sub documents via an $elemMatch projection, the wrong array element will get modified in the database when saving the document back.

Please see the following test case:

https://gist.github.com/4711139

Which produced the following debug output:

Mongoose: docs.findOne({ someId: 1 }) { fields: { subDocs: { '$elemMatch': { someValue: 3 } } }, safe: undefined }
Mongoose: docs.update({ _id: ObjectId("511054f985a8270000000002") }) { '$set': { 'subDocs.0.someValue': 4 } } {}

Notice up the update command sets 'subDocs.0.someValue' instead of 'subDocs.2.someValue'. I assume this is because an array with only a single element was returned due to the projection.

We have been relying on mongoose's ability to selectively update parts of large documents including many subdocuments depending on what data had been modified. Without the ability to use projection, the document retrieval kills our performance as node will spend a bulk of its time parsing the bson data. See also the following issue for a similar issue that we've encountered:

https://github.com/LearnBoost/mongoose/issues/1303

Thanks!

This is using mongoose 3.5.4

[aheckmann]

Mongoose is not a good fit for this use case because it was designed such that document.save() result in a single atomic write to the database, either all changes are commited or nothing is when an error occurs. To facilitate this, mongoose uses positional notation (subDocs.0.someValue) to handle multiple subdocument updates. MongoDB currently does not provide any other syntax to update more than one array element at a time. This has been a solid solution until recently when MongoDB added $elemMatch in a projection which ruins knowledge of element positions. More generally, nobodys code can rely on positional notation in updates based on results returned from projections which include $elemMatch.

Work-around:

• When using $elemMatch in a projection, do not use document.save(). Instead, manually update your document using Model.update().

You may also consider using lean() in combination with $elemMatch projections since lean returns plain objects (not Mongoose documents) which do not have a save() method at all, further forcing you to use Model.update manually.

[aheckmann]

I'm thinking returning an error to the save callback when this is attempted is the "right thing" in this circumstance.

[whitecolor]

Are you going to redesign something in mongoose in future to fit more such cases? Mongoose is great but sometimes it forces to query exessive data from db. (

[aheckmann]

Not in the foreseeable future. If mongodb adds some new feature we'll revisit.

[mephju]

It should also be noted that this doesn't work for queries where only the matching subdocument is returned.

    User.findOne(
        {'orders._id': orderId},
        {'orders.$': 1},
        done
    );

Changes made to user.orders[0] will be saved to the very first subdocument within user.
No error is returned here, unfortunately.

[vkarpov15]

Related to #2031

[geloescht]

This might actually be doable using $(update) since version 2.6 of MongoDB: https://docs.mongodb.org/v2.6/reference/operator/update/positional/
We'd have to pull out the projection clause (for $elemMatch) or query criterion (for $) from the original query and copy it to our update filter. Then, instead of using "array.0.element" to update the array we need to use "array.$.element". This works only with projection operators that select a single array element (thus, not with $slice) and if no new elements were added to the array.
I admit, it sounds quite complicated and fragile, but do you see any chance that this is ever going to be implemented @vkarpov15 ? I had a look at the code generating queries for Document.save() myself, but do not feel confident enough to touch it.

[vkarpov15]

@geloescht can you provide a code sample that demonstrates what you're looking to do?

[geloescht]

I am developing a solution for content localization. Basically, instead of modifying a translatable document property directly, I use a virtual that modifies an entry inside an array with localized data. To save memory and computing power for documents with a lot of translations I use projection to only retrieve localized data for a single language. This is all wired up using mongoose middleware.
Basically it boils down to:

apps.findOne(
{
  _id: ObjectId("567452bae5b25d6e6c1a0f7e")
},
{
  localization: { '$elemMatch': { language: 'de' } }, dummy_: 0
}).exec(function(err, app)
{
  //a real gentleman woud handle this error
  app.localization[0].name =  "Neuer Name";
  app.save(); //throws DivergentArrayError
  //this is the query that saves the way I want
  apps.findOneAndUpdate(
  {
    _id: ObjectId("567452bae5b25d6e6c1a0f7e"),
    localization: { '$elemMatch': { language: 'de' } }
  },
  {
    $set: { 'localization.$.name' : "Neuer Name" }
  }).exec(//...
});

[vkarpov15]

Yeah supporting that behavior in app.save() would be tricky. Will investigate what it would take to support this behavior for a future release. In the meantime, your findOneAndUdate() solution works well.

[geloescht]

Thanks for looking into this!

[filipetedim]

@geloescht a thousand thanks to you. I've spent the last 2 hours trying to code a workaround, then 1 more trying to find a solution until I stumbled upon yours, great work! I guess besides the '.$' there is still no other support for elemMatch and saving?

[geloescht]

@filipetedim My PR adds support for saving fields selected via $elemMatch but it is not merged into Mongoose yet. In the meantime you need to use update and '$.' or maintain your own fork with my patch.

[filipetedim]

@geloescht Yeah I'm using the '$.' for the moment - tbh I thought that was your code. So how will I call your function to use it? Or will I just hit "save()" and it will work? Again, nice work finding the $. solution and implementing a new feature on the PR.

[vkarpov15]

Yeah upon closer inspection OP's issue was fixed in 4.5.0 with #4053