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

{ optimisticConcurrency: true }, unwanted rollback of __v #10128

Closed
sebamisc opened this issue Apr 13, 2021 · 1 comment · Fixed by #10163
Closed

{ optimisticConcurrency: true }, unwanted rollback of __v #10128

sebamisc opened this issue Apr 13, 2021 · 1 comment · Fixed by #10163
Assignees
Labels
confirmed-bug We've confirmed this is a bug in Mongoose and will fix it. has repro script There is a repro script, the Mongoose devs need to confirm that it reproduces the issue
Milestone

Comments

@sebamisc
Copy link

sebamisc commented Apr 13, 2021

Environment
"mongoose": "^5.11.17"

Working with the option { optimisticConcurrency: true } (overwriting versionKey does not affect behavior)

High-Level Description
When relying on optimisticConcurrency to manage the versionKey, I noticed a bug, causing the versionKey to roll back.

Schema Description
The following schema was used for reproduction:

const thingSchema = new mongoose.Schema(
  {
    price: {
      type: Number
    },
  }, 
  {
    optimisticConcurrency: true,
  }
);

const Thing = mongoose.model('Thing', thingSchema);

Testing Steps
The following script was used to cause the rollback:

const thing = new Thing.create({ price: 1 });
await thing.save();
console.log(thing);
// prints out __v: 0

// get thing 1st time
const thing_1 = await Thing.findById(thing.id);

// get thing 2nd time
const thing_2 = await Thing.findById(thing.id);

// manipulate the things
thing_1.set({price: 2});
await thing_1.save();  // persist correctly __v: 1

thing_2.set({price: 1});
await thing_2.save();  // persists rollback to  __v: 0

Expected
An error on thing_2.save();, however operation went through sucessfully and reset version to __v:0.

Note
When setting thing_2 dirty 'forceably' , error is thrown, as such:

thing_2.set({price: 2});
thing_2.set({price: 1});
await thing_2.save();  // throws!

Manual workaround
The only manual workaround for now seems to be to set

thingSchema.pre('save', async function (done) {
 this.increment();
 done()
});
@IslandRhythms IslandRhythms added the confirmed-bug We've confirmed this is a bug in Mongoose and will fix it. label Apr 13, 2021
@IslandRhythms
Copy link
Collaborator

const mongoose = require('mongoose');

const run = async () => {
    await mongoose.connect('mongodb://localhost:27017/test', {
      useNewUrlParser: true,
      useUnifiedTopology: true,
    });
    await mongoose.connection.dropDatabase();
    const thingSchema = new mongoose.Schema({price: Number},{optimisticConcurrency: true});
    const Thing = mongoose.model('Thing', thingSchema);
    const thing = await Thing.create({price: 1});
    await thing.save();
    console.log(thing);
    const thing_1 = await Thing.findById(thing.id);
    const thing_2 = await Thing.findById(thing.id);
    thing_1.set({price: 2});
    await thing_1.save();
    console.log(thing_1);
    thing_2.set({price:1});
    await thing_2.save();
    console.log(thing_2);

}
run().catch(err => console.log(err));

@IslandRhythms IslandRhythms added the has repro script There is a repro script, the Mongoose devs need to confirm that it reproduces the issue label Apr 15, 2021
@vkarpov15 vkarpov15 modified the milestones: 5.12.5, 5.12.6 Apr 17, 2021
@IslandRhythms IslandRhythms linked a pull request Apr 21, 2021 that will close this issue
vkarpov15 added a commit that referenced this issue Apr 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed-bug We've confirmed this is a bug in Mongoose and will fix it. has repro script There is a repro script, the Mongoose devs need to confirm that it reproduces the issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants