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

Validator called more than once when using document.set #10141

Closed
qqilihq opened this issue Apr 15, 2021 · 2 comments
Closed

Validator called more than once when using document.set #10141

qqilihq opened this issue Apr 15, 2021 · 2 comments
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

@qqilihq
Copy link

qqilihq commented Apr 15, 2021

Do you want to request a feature or report a bug?
Bug

What is the current behavior?
For Object properties, the validate function gets called several times, when using a setter doc.set.

If the current behavior is a bug, please provide the steps to reproduce.

import mongoose from 'mongoose';

beforeAll(async () => mongoose.connect(process.env.MONGO_URL as string));

afterAll(async () => mongoose.disconnect());

describe('Mongoose object validation', () => {
  let validatorCallCount = 0;

  // eslint-disable-next-line @typescript-eslint/naming-convention
  const Model = mongoose.model(
    'model',
    new mongoose.Schema({
      name: { type: String },
      object: {
        type: Object,
        validate: (v: any) => {
          // console.log('called validator with', v);
          validatorCallCount++;
          return true;
        }
      }
    })
  );

  beforeEach(() => {
    validatorCallCount = 0;
  });

  it('when using constructor', async () => {
    const doc = new Model({
      name: 'bob',
      object: {
        foo: 1,
        bar: 'one',
        baz: { key: 'value' }
      }
    });
    await doc.validate();
    expect(validatorCallCount).toEqual(1);
  });

  it('when using setter for parent property', async () => {
    const doc = new Model({ name: 'bob' });
    doc.set('object', {
      foo: 1,
      bar: 'one',
      baz: { key: 'value' }
    });
    await doc.validate();
    expect(validatorCallCount).toEqual(1);
  });

  // THIS CURRENTLY FAILS
  it('when using setter for nested properties', async () => {
    const doc = new Model({ name: 'bob' });
    doc.set('object.foo', 1);
    doc.set('object.bar', 'one');
    doc.set('object.baz', { key: 'value' });
    await doc.validate();
    expect(validatorCallCount).toEqual(1);
    // will call the validator 4 times with:
    // { foo: 1, bar: 'one', baz: { key: 'value' } }
    // 1
    // 'one'
    // { key: 'value' }
  });
});

What is the expected behavior?
For the test “when using setter for single properties”, the validator should be called once (as in the other two cases)

What are the versions of Node.js, Mongoose and MongoDB you are using? Note that "latest" is not a version.

  • Mongoose 5.12.4
  • Node.js v12.20.0

Maybe related to:

@IslandRhythms
Copy link
Collaborator

const mongoose = require('mongoose');

(async () => {
    await mongoose.connect('mongodb://localhost:27017/test', {
        useNewUrlParser: true,
        useUnifiedTopology: true
      });
    
    await mongoose.connection.dropDatabase();

    let validatorCallCount = 0;
    const Model = mongoose.model(
        'model',
        new mongoose.Schema({
          name: { type: String },
          object: {
            type: Object,
            validate: (v) => {
              console.log('called validator with', v);
              validatorCallCount++;
              return true;
            }
          }
        })
      );

    const doc = new Model({
        name: 'bob',
        object: {
          foo: 1,
          bar: 'one',
          baz: { key: 'value' }
        }
      });
      await doc.validate();

    const doc1 = new Model({ name: 'bob' });
    doc1.set('object', {
      foo: 1,
      bar: 'one',
      baz: { key: 'value' }
    });
    await doc1.validate();

    const doc2 = new Model({ name: 'bob' });
    doc2.set('object.foo', 1);
    doc2.set('object.bar', 'one');
    doc2.set('object.baz', { key: 'value' });
    await doc2.validate();
})();

@IslandRhythms IslandRhythms added 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 labels Apr 16, 2021
@vkarpov15 vkarpov15 added this to the 5.12.7 milestone Apr 23, 2021
vkarpov15 added a commit that referenced this issue Apr 28, 2021
@vkarpov15
Copy link
Collaborator

So it is a bug that we call the validator multiple times, but I think the correct behavior is that Mongoose shouldn't call the validator. That is, expect(validatorCallCount).toEqual(0); Because, in other cases, like subdocuments or nested paths, Mongoose doesn't call parent validators if a subpath is modified.

Fix will be in Mongoose v5.12.7.

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

No branches or pull requests

3 participants