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

Mongoose schema setter not being called when using $inc #13158

Closed
1 task done
znadir opened this issue Mar 10, 2023 · 6 comments
Closed
1 task done

Mongoose schema setter not being called when using $inc #13158

znadir opened this issue Mar 10, 2023 · 6 comments
Labels
confirmed-bug We've confirmed this is a bug in Mongoose and will fix it.
Milestone

Comments

@znadir
Copy link

znadir commented Mar 10, 2023

Prerequisites

  • I have written a descriptive issue title

Mongoose version

^6.9.0

Node.js version

18.12.1

MongoDB version

5.0.15

Operating system

Windows

Operating system version (i.e. 20.04, 11.3, 10)

11

Issue

I run in a problem. I have defined a setter in my model:

const userSchema = new mongoose.Schema({
    money: {
        balance: {
            type: Number,
            set: (v) => Number(v)?.toFixed(4),
            default: 0,
        },
    },
});

When I try increasing balance of a user, it does format the number when the value is positive but sometimes not when its negative, which is weird.

await User.findByIdAndUpdate(userId, {
        $inc: {
            'money.balance': earnings,
        },
    },
    { runValidators: true }
);

Thanks for helping me

@znadir znadir added help This issue can likely be resolved in GitHub issues. No bug fixes, features, or docs necessary help wanted labels Mar 10, 2023
@vkarpov15 vkarpov15 added has repro script There is a repro script, the Mongoose devs need to confirm that it reproduces the issue and removed help This issue can likely be resolved in GitHub issues. No bug fixes, features, or docs necessary help wanted labels Mar 13, 2023
@vkarpov15 vkarpov15 added this to the 6.10.4 milestone Mar 13, 2023
@IslandRhythms IslandRhythms added confirmed-bug We've confirmed this is a bug in Mongoose and will fix it. and removed has repro script There is a repro script, the Mongoose devs need to confirm that it reproduces the issue labels Mar 13, 2023
@IslandRhythms
Copy link
Collaborator

const mongoose = require('mongoose');

const testSchema = new mongoose.Schema({
  money: {
      balance: {
          type: Number,
          set: (v) => Number(v)?.toFixed(4),
          default: 0,
      },
  },
});

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

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

  await Test.create({
    money: { balance: -4}
  });
  const doc = await Test.findOne();
  console.log(doc.money.balance);
  const update = await Test.findOneAndUpdate({}, {$inc: { 'money.balance': 1.5432 }}, { runValidators: true, returnDocument: 'after' });
  console.log('what is update', update);
}

run();

output:

-4
what is update {
  money: { balance: -2.4568000000000003 },
  _id: new ObjectId("640f9a776f1f8ebed5209f59"),
  __v: 0
}

@vkarpov15
Copy link
Collaborator

@IslandRhythms the issue with your script is that you're rounding to 4 decimal places, and 1.5432 is already at 4 decimal places. And remember that adding/subtracting floating points can have some precision issues in MongoDB and in Node.

Round to 2 and your script works fine:

const mongoose = require('mongoose');

const testSchema = new mongoose.Schema({
  money: {
      balance: {
          type: Number,
          set: (v) => Number(v)?.toFixed(2),
          default: 0,
      },
  },
});

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

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

  await Test.create({
    money: { balance: -4}
  });
  const doc = await Test.findOne();
  console.log(doc.money.balance);
  const update = await Test.findOneAndUpdate({}, {$inc: { 'money.balance': 1.5432 }}, { runValidators: true, returnDocument: 'after' });
  console.log('what is update', update);
}

run();

Output:

-4
what is update {
  money: { balance: -2.46 },
  _id: new ObjectId("6411e8439161dff75582c9d4"),
  __v: 0
}

Also works fine with negative numbers:

  const update = await Test.findOneAndUpdate({}, {$inc: { 'money.balance': -1.5432 }}, { runValidators: true, returnDocument: 'after' });

Output:

-4
what is update {
  money: { balance: -5.54 },
  _id: new ObjectId("6411e8fbadd0e470b3c77254"),
  __v: 0
}

However, I think the issue is that using the document $inc() method does not currently call setters, for example:

const mongoose = require('mongoose');

const testSchema = new mongoose.Schema({
  money: {
      balance: {
          type: Number,
          set: (v) => Number(v)?.toFixed(2),
          default: 0,
      },
  },
});

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

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

  await Test.create({
    money: { balance: -4}
  });
  const doc = await Test.findOne();
  console.log(doc.money.balance);
  doc.$inc('money.balance', -1.5432);
  await doc.save();
  const update = await Test.findOne({ _id: doc._id });
  console.log('what is update', update);
}

run();

Output:

what is update {
  money: { balance: -5.5432 },
  _id: new ObjectId("6411e9691dd9bd127222770b"),
  __v: 0
}

@znadir
Copy link
Author

znadir commented Mar 15, 2023

@vkarpov15, if the document $inc() method doesn't call setters, why does it call it in your first example?

Here's what the $inc returns in different cases:

-4 + 1.5432 = (1 decimal) -2.5 ✔️
-4 + 1.5432 = (2 decimals) -2.46 ✔️
-4 + 1.5432 = (3 decimals) -2.457 ✔️
-4 + 1.5432 = (4 decimals) -2.4568000000000003 ❌
-4 + 1.5432 = (5 decimals) -2.4568000000000003 ❌
-4 + 1.5432 = (6 decimals) -2.4568000000000003 ❌
-4 +- 1.5432 = (1 decimals) -5.5 ✔️
-4 +- 1.5432 = (2 decimals) -5.54 ✔️
-4 +- 1.5432 = (4 decimals) -5.5432 ✔️
-4 +-1.54322533 = (5 decimals) = -5.54323 ✔️

@vkarpov15
Copy link
Collaborator

@nadirzebiri findOneAndUpdate() with $inc is a different code path than doc.$inc().

The reason why you're seeing 2.4568000000000003 is binary floating point precision issues

@znadir
Copy link
Author

znadir commented Mar 15, 2023

@vkarpov15 ok so how not to have this floating error? $inc looks like calling setters but not in some cases. Does the error come from mongoose or MangoDB?

vkarpov15 added a commit that referenced this issue Mar 15, 2023
vkarpov15 added a commit that referenced this issue Mar 17, 2023
fix(document): apply setters on resulting value when calling Document.prototype.$inc()
@vkarpov15
Copy link
Collaborator

@nadirzebiri we have a fix in #13178, will ship the fix later today.

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.
Projects
None yet
Development

No branches or pull requests

3 participants