Skip to content
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.

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

"update" and "findByIdAndUpdate" don't increment "__v" #6994

Closed
FelixFurtmayr opened this issue Sep 8, 2018 · 6 comments
Closed

"update" and "findByIdAndUpdate" don't increment "__v" #6994

FelixFurtmayr opened this issue Sep 8, 2018 · 6 comments
Labels

Comments

@FelixFurtmayr
Copy link

I guess by design, someone thought the update functions do not need to increment __v.
For me it seems like a bug, as I expected it always to be updated.

There has already been an issue about it, but was not resolved: #1265
The Issue is in all mongoose Versions (tested with mongoose 5, 4, 3).

Problem

Someone with an older version of the doc could overwrite a newer version.
Example: Two people editing a document in the frontend - and they both want to save it. We might get version conflicts, but as __v is not updated, we cannot check it.

Conclusion

  • It is good to always increment __v, also on the "update" functions.
  • Why
    • allow prevent conflicts using __v
    • it seems intuitive to me, as other functions increment __v.
    • the basic idea if __v is to be incremented

Code Snippet for Testing and Workaround

I made a script for testing - copy it in a file 'mongoose__v.js' and execute it with:

node mongoose__v.js

The script will create a document 'catty' in the collection 'kitten' on the first run. Execute it several times to try to update the version. The console.logs will show, that __v is not updated.

const mongoose = require('mongoose');

// prepare  database
const db = mongoose.connection;
const kittySchema = mongoose.Schema({
   name: String,
   dummy: Array
});
const kitten = mongoose.model('kitten', kittySchema);
mongoose.connect('mongodb://localhost/test', {useNewUrlParser: true});


db.once('open', function () {



   // imagine we just fetched a document from the database
   // and we want to write it back using 'findOneAndUpdate'.
   // our object might look like this
   let catty = {
      _id: '5b9397d6ef2995794af001a3',
      dummy: [ 1, 2, 3 ],
      name: 'catty',
      __v: 0 // as expected, there is a mongoose version here
   };


   // by default, the function "findOneAndUpdate" below will not update __v
   // we can force it manual with the "$inc" operator in the following two lines:
   // delete catty.__v; // ensure, there is no __v in the update object, otherwise it would be editied twice => error
  // catty.$inc = { __v: 1};


   kitten.findOneAndUpdate({
      'name': 'catty' // will not be found on first run, so it will be created
   }, catty, {
      upsert: true,
      new: true
   }, function (err, cattyFromDb) {
      if (err) return console.error(err);
      console.log('updated kitty "catty":', cattyFromDb.__v);
      process.exit(0);
   });
});

workaround

Uncomment those lines and execute the script again several times - now __v should be updated.

   delete catty.__v; // ensure, there is no __v in the update object, otherwise it would be editied twice => error
   catty.$inc = { __v: 1};

Also this works, it would be really great, if it would be the mongoose default.

@vkarpov15 vkarpov15 added this to the 5.2.17 milestone Sep 11, 2018
@vkarpov15
Copy link
Collaborator

Thanks for reporting, will investigate ASAP

@vkarpov15
Copy link
Collaborator

I took a closer look at this and this is going to have to wait for a minor release. Right now update operations never update the version key, we'll have to add a versionKey option to queries to opt into incrementing versions. Currently Mongoose only increments the version key through save(), I'll add a note to the docs for now.

@FelixFurtmayr
Copy link
Author

Hi vkarpov15, thank you for preparing this feature :-)
An option to enable it would be great.
I guess it should be enabled by default, but for legacy reasons, disabling it will be the best thing in the near future.

@vkarpov15
Copy link
Collaborator

I dug into this for 5.4 and I think its best to implement this as a plugin for now. Check out mongoose-update-versioning on npm and our docs

@vkarpov15 vkarpov15 removed this from the 5.4 milestone Dec 6, 2018
@TendieShop
Copy link

is this being looked at still?

@vkarpov15
Copy link
Collaborator

@DigivolveIo please check the mongoose-update-versioning plugin docs

@Automattic Automattic locked as resolved and limited conversation to collaborators May 12, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

3 participants