Validator bug when call the save() function repeatedly

[lutherman]

Hi,

When I test my validator in mocha, I find that the validators will be wrong if call save() function repeatedly.

Here is my part of schema:

const ProjectSchema   = new Schema({
    name: String
    // ... 
    roles: {
        type: [{
            name: String, 
            folders: {
                type: [{
                    folderId: {
                        type: Schema.Types.ObjectId,
                        validate: {
                            validator: MongoValidator.rolesFoldersFolderId
                        },
                    },
                   // ...
                }],
                validate: {
                    validator: MongoValidator.rolesFolders
                }
            }
        }],
        validate: {
            validator: MongoValidator.roles
        }
    },
    // ...
    active: { type: Boolean, default: true, select: false},
});

And here is my part of mocha code:

// ...
it('Edit folder id in role to a duplicate id', () => {
       return expect(Project.findById('5b67ac70bdb7915e3041feab')
            .then(projectDoc => {
               // First save(): add a folder object
               projectDoc.roles.find(role => role.name === 'role1').folders.push({ folderId: '5b67aadf3405b05e09304b83' });
               return projectDoc.save();
            })
           .then(projectDoc => {
               // Second save(): edit the folder id
               projectDoc.roles.find(role => role.name === 'role1').folders.find(roleFolder => roleFolder.folderId.toString() === '5b67aadf3405b05e09304b83').folderId = '5b67aadf3405b05e09304b82';
               return projectDoc.save();
            }))
            .be.rejected.then(error => {
                  expect(error).to.have.property('name').equal('ValidationError');
 expect(error).to.have.property('errors').to.have.property('roles.1.folders').to.have.property('message').equal('Folder ID in role must be unique');
             });
});
// ...

After I run the code with debugger, I find two problems happened in second save().

---

Cannot Trigger the Validator

I find that the validator 'rolesFolders' will not be triggered to validate the 'folders' array of 'role1' object, but can be triggered by other role objects ('Administrator' and 'role2').

Here is the code of validator 'rolesFolders':

function rolesFolders(folders) {
  if ((this.isNew || this.isModified('folders')) && folders.find((folder, index) => folders.find((folderRef, indexRef) => folder.folderId.toString() === folderRef.folderId.toString() && index !== indexRef))) {
    throw new Error('Folder ID in role must be unique');
  } else {
    return true;
  }
}

---

Wrong 'this' refer

Also, the 'this' of validator 'rolesFoldersFolderId' will refer to path 'roles' when validates the 'folderId' in 'folders' array of 'role1' object. I think the correct path should be 'roles.folders', the current level of 'folderId'.

Here is the code of validator 'rolesFoldersFolderId':

function rolesFoldersFolderId(folderId) {
  if (this.isNew || this.isDirectModified('folderId')) {
    return Folder.findById(folderId)
      .then(folderDoc => {
        if (folderDoc) {
          return true;
        } else {
          throw new Error('Folder not exist');
        }
      });
  } else {
    return true;
  }
}

---

However, if I find the document again before second save() call, every thing will become correct.

Just like:

// ...
it('Edit folder id in role to a duplicate id', () => {
       return expect(Project.findById('5b67ac70bdb7915e3041feab')
            .then(projectDoc => {
               // First save(): add a folder
               projectDoc.roles.find(role => role.name === 'role1').folders.push({ folderId: '5b67aadf3405b05e09304b83' });
               return projectDoc.save();
            })
           .then(projectDoc => Project.findById('5b67ac70bdb7915e3041feab')) // !!! <-- HERE: find the document again !!!
           .then(projectDoc => {
               // Second save(): edit the folder id
               projectDoc.roles.find(role => role.name === 'role1').folders.find(roleFolder => roleFolder.folderId.toString() === '5b67aadf3405b05e09304b83').folderId = '5b67aadf3405b05e09304b82';
               return projectDoc.save();
            }))
            .be.rejected.then(error => {
                  expect(error).to.have.property('name').equal('ValidationError');
 expect(error).to.have.property('errors').to.have.property('roles.1.folders').to.have.property('message').equal('Folder ID in role must be unique');
             });
});
// ...

Since the validators give different results with same update process, I suppose it may be a bug.

mongoose version: 5.2.5
mongodb version 3.6.5

[Fonger]

I can't reproduce your bug. The following code works fine on me.

const mongoose = require('mongoose');
const Schema = mongoose.Schema;
mongoose.connect('mongodb://localhost:27017/test', { useNewUrlParser: true });

const ProjectSchema = new Schema({
  name: String,
  roles: {
    type: [{
      name: String,
      folders: {
        type: [{
          folderId: {
            type: Schema.Types.ObjectId,
            validate: {
              validator: rolesFoldersFolderId
            },
          },
        }],
        validate: {
          validator: rolesFolders
        }
      }
    }],
    validate: {
      validator: roles
    }
  },
  active: { type: Boolean, default: true, select: false },
});

const Project = mongoose.model('Project', ProjectSchema);

//test
it('Edit folder id in role to a duplicate id', () => {
  return Project.findById('5b67ac70bdb7915e3041feab')
    .then(projectDoc => {
      // First save(): add a folder object
      projectDoc.roles
        .find(role => role.name === 'role1')
        .folders
        .push({ folderId: '5b67aadf3405b05e09304b83' });
      return projectDoc.save();
    })
    .then(projectDoc => {
      // Second save(): edit the folder id
      projectDoc.roles
        .find(role => role.name === 'role1')
        .folders
        .find(roleFolder => roleFolder.folderId.toString() === '5b67aadf3405b05e09304b83')
        .folderId = '5b67aadf3405b05e09304b82';
      return projectDoc.save();
    });
});

function rolesFolders(folders) {
  let folderIds = folders.map(f => f.folderId.toString());

  if ((this.isNew || this.isModified('folders')) && (new Set(folderIds)).size !== folderIds.length) {
    throw new Error('Folder ID in role must be unique');
  } else {
    return true;
  }
}

function rolesFoldersFolderId(folderId) {
  if (this.isNew || this.isDirectModified('folderId')) {
    return Promise.resolve(true);
  } else {
    return true;
  }
}

function roles() {
  return true;
}

[lutherman]

@Fonger

Please try the code below.

var chai = require("chai");
var chaiAsPromised = require("chai-as-promised");

chai.use(chaiAsPromised);
var expect = chai.expect;

const mongoose = require('mongoose');
const Schema = mongoose.Schema;
mongoose.connect('mongodb://localhost:27017/mongoose-repeated-save-issue', { useNewUrlParser: true });

const ProjectSchema = new Schema({
  name: String,
  roles: {
    type: [{
      name: String,
      folders: {
        type: [{
          folderId: {
            type: Schema.Types.ObjectId,
            validate: {
              validator: rolesFoldersFolderId
            },
          },
        }],
        validate: {
          validator: rolesFolders
        }
      }
    }],
    validate: {
      validator: roles
    }
  },
  active: { type: Boolean, default: true, select: false },
});

const Project = mongoose.model('Project', ProjectSchema);

//test
describe('Test', () => {
  let project1Id;
  const folderId1 = '5b67aadf3405b05e09304b82';
  const folderId2 = '5b67aadf3405b05e09304b83';
  before('Add project document sample', () => {
    return Project.remove({})
      .then(() => Project.create({
        name: 'Project1',
        roles: [
          { name: 'Administrator' },
          { name: 'role1', folders: [{ folderId: folderId1 }] },
          { name: 'role2' }
        ]
      }))
      .then(projectDoc => {
        project1Id = projectDoc._id;
      });
  });
  after('DB disconnect', () => {
    mongoose.disconnect();
  });
  it('Edit folder id in role to a duplicate id', () => {
    return expect(Project.findById(project1Id)
      .then(projectDoc => {
        // First save(): add a folder object
        projectDoc.roles
          .find(role => role.name === 'role1')
          .folders
          .push({ folderId: folderId2 });
        return projectDoc.save();
      })
      //.then(projectDoc => Project.findById(project1Id)) // !!! <-- HERE: find the document again. Then, can pass the mocha test. !!!
      .then(projectDoc => {
        // Second save(): edit the folder id
        projectDoc.roles
          .find(role => role.name === 'role1')
          .folders
          .find(roleFolder => roleFolder.folderId.toString() === folderId2)
          .folderId = folderId1;
        return projectDoc.save();
      }))
      .be.rejected.then(error => {
        expect(error).to.have.property('name').equal('ValidationError');
        expect(error).to.have.property('errors').to.have.property('roles.1.folders').to.have.property('reason').to.have.property('message').equal('Folder ID in role must be unique');
      });
  });
})

function rolesFolders(folders) {
  let folderIds = folders.map(f => f.folderId.toString());

  if ((this.isNew || this.isModified('folders')) && (new Set(folderIds)).size !== folderIds.length) {
    throw new Error('Folder ID in role must be unique');
  } else {
    return true;
  }
}

function rolesFoldersFolderId(folderId) {
  if (this.isNew || this.isDirectModified('folderId')) {
    return Promise.resolve(true);
  } else {
    return true;
  }
}

function roles() {
  return true;
}

[Fonger]

@lutherman bug confirmed and investigating

[lutherman]

Thanks for fixing the issue.