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

validateAllPaths option to validate() and save() that validates all paths in the schema, not just modified paths #14414

Closed
2 tasks done
Zig1375 opened this issue Mar 4, 2024 · 10 comments
Labels
new feature This change adds new functionality, like a new method or class
Milestone

Comments

@Zig1375
Copy link

Zig1375 commented Mar 4, 2024

Prerequisites

  • I have written a descriptive issue title
  • I have searched existing issues to ensure the bug has not already been reported

Mongoose version

8.2.0

Node.js version

18.x

MongoDB server version

7.0

Typescript version (if applicable)

No response

Description

I have a simple schema containing a subdocument.
The subdocument is validated upon creation, but not during updates.

Steps to Reproduce

const PhotoValidator = {
  validator: function(photo) {
    console.log('Validation');
    return photo.path.startsWith('photos/car/');
  },
  message: 'invalid'
};


const PhotoSchema = new mongoose.Schema({
  path: {
    type: String,
    required: true,
    trim: true,
  }
});

const CarSchema = new mongoose.Schema({
  photo: {
    type: PhotoSchema,
    required: true,
    validate: PhotoValidator
  }
});

const CarModel = mongoose.model('CarModel', CarSchema);


await CarModel.deleteMany({});
const car = new CarModel({ photo: { path: 'photos/car/bla.jpg' } });
await car.save();
console.log('Created');


car.set('photo.path', 'bla/bla/bla');
// car.photo.path = 'bla/bla/bla';
// const photo = car.get('photo'); photo.set('path', 'bla/bla/bla');

await car.save();
console.log('Saved');

As you can see, validation occurred only once, prior to the document being created.

Expected Behavior

Updating a record should trigger the same validation as creating a new record, and any validation errors should prevent the changes from being saved.

@Zig1375 Zig1375 changed the title Validation doesn't happen for subdocument Validation does not occur for subdocuments. Mar 4, 2024
@Zig1375
Copy link
Author

Zig1375 commented Mar 5, 2024

I provided it above. On my side that code does not work correctly

@Zig1375
Copy link
Author

Zig1375 commented Mar 5, 2024

repro.js.zip

import mongoose from 'mongoose';

await mongoose.connect('mongodb://localhost:27017/test?directConnection=true');

const PhotoValidator = {
  validator: function(photo) {
    console.log('Validation');
    return photo.path.startsWith('photos/car/');
  },
  message: 'invalid'
};


const PhotoSchema = new mongoose.Schema({
  path: {
    type: String,
    required: true,
    trim: true,
  }
});

const CarSchema = new mongoose.Schema({
  photo: {
    type: PhotoSchema,
    required: true,
    validate: PhotoValidator
  }
});

const CarModel = mongoose.model('CarModel', CarSchema);


await CarModel.deleteMany({});
const car = new CarModel({ photo: { path: 'photos/car/bla.jpg' } });
await car.save();
console.log('Created');


car.set('photo.path', 'bla/bla/bla');
// car.photo.path = 'bla/bla/bla';
// const photo = car.get('photo'); photo.set('path', 'bla/bla/bla');

await car.save();
console.log('Saved');


// One more
console.log('---------------------------------');

const car2 = new CarModel({ photo: { path: 'bla/bla' } });

try {
  await car.save();
  console.log('Created');
} catch (error) {
  console.log('Validation error occurred');
}

Result:

$ node repro.js
Validation
Created
Saved
---------------------------------
Validation
Validation error occurred

@Zig1375
Copy link
Author

Zig1375 commented Mar 5, 2024

So, as you can see, in the first case, the creation of a new 'car' occurs without any errors because the path is valid. However, when I attempt to change the path in an existing 'car' to an incorrect one and save it, it also gets saved without any errors...

In the second case, I create a 'car' with an incorrect path, and it doesn't save due to a validation error. The same error is expected to occur in the first case when updating.

@Zig1375
Copy link
Author

Zig1375 commented Mar 5, 2024

it happens not only with set. I commented out other cases it happens as well:

car.photo.path = 'bla/bla/bla';

also does not work.

Any of these ways cannot work

// car.set('photo.path', 'bla/bla/bla');
car.photo.path = 'bla/bla/bla';
// const photo = car.get('photo'); photo.set('path', 'bla/bla/bla');

car.save();

@Zig1375
Copy link
Author

Zig1375 commented Mar 5, 2024

but if I add the validate method inside the subdocument (directly to the path field) it works as expected.
The problem is only when validation is on an entire subdocument field...

@vkarpov15 vkarpov15 added this to the 8.2.2 milestone Mar 5, 2024
@vkarpov15 vkarpov15 added the has repro script There is a repro script, the Mongoose devs need to confirm that it reproduces the issue label Mar 5, 2024
@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 7, 2024
@IslandRhythms
Copy link
Collaborator

const mongoose = require('mongoose');

const PhotoValidator = {
  validator: function(photo) {
    console.log('Validation');
    return photo.path.startsWith('photos/car/');
  },
  message: 'invalid'
};


const PhotoSchema = new mongoose.Schema({
  path: {
    type: String,
    required: true,
    trim: true,
  }
});

const CarSchema = new mongoose.Schema({
  photo: {
    type: PhotoSchema,
    required: true,
    validate: PhotoValidator
  }
});

const CarModel = mongoose.model('CarModel', CarSchema);

run().then(() => process.exit(0)).catch(e => {
  console.log(e);
  process.exit(-1);
})

async function run() {
  await mongoose.connect('mongodb://localhost:27017');
  await mongoose.connection.dropDatabase();
  
  
  
  await CarModel.deleteMany({});
  const car = new CarModel({ photo: { path: 'photos/car/bla.jpg' } });
  await car.save();
  console.log('Created');
  
  
  car.set('photo.path', 'bla/bla/bla');
  // car.photo.path = 'bla/bla/bla';
  // const photo = car.get('photo'); photo.set('path', 'bla/bla/bla');
  
  await car.save();
  console.log('Saved, should not have saved');
  
  
  // One more
  console.log('---------------------------------');
  
  const car2 = new CarModel({ photo: { path: 'bla/bla' } });
  
  try {
    await car.save();
    console.log('Created');
  } catch (error) {
    console.log('Validation error occurred');
  }
}

@Automattic Automattic deleted a comment from donoftime2018 Mar 7, 2024
@Automattic Automattic deleted a comment from donoftime2018 Mar 7, 2024
@Automattic Automattic deleted a comment from donoftime2018 Mar 7, 2024
@Automattic Automattic deleted a comment from donoftime2018 Mar 7, 2024
@Automattic Automattic deleted a comment from donoftime2018 Mar 7, 2024
@vkarpov15
Copy link
Collaborator

This is expected behavior. When saving an existing document, Mongoose only runs validation on paths that have been changed. That means we don't run validation on subdocument paths if the subdocument wasn't directly modified. So if you were to do car.photo = { path: '/bla/bla' } or car.set('photo.path', '/bla/bla'); car.markModified('photo'); the validator would run, but if you modify photo.path directly then Mongoose won't re-run photo validation.

In general, we recommend putting validators on the paths they're validating. So PhotoValidator belongs on the path property, not photo. Does that make sense?

@vkarpov15 vkarpov15 added help This issue can likely be resolved in GitHub issues. No bug fixes, features, or docs necessary and removed confirmed-bug We've confirmed this is a bug in Mongoose and will fix it. labels Mar 7, 2024
@vkarpov15 vkarpov15 removed this from the 8.2.2 milestone Mar 7, 2024
@Zig1375
Copy link
Author

Zig1375 commented Mar 7, 2024

is there a way to run "force" validation? to validate all fields?

@vkarpov15
Copy link
Collaborator

Yes, but not as elegant as we would like. You need to markModified() every path you want to validate, like car.markModified('photo'). So to validate all paths, you can just markModified() every path in the schema as follows:

car.schema.eachPath(path => car.markModified(path));

We will add a validateAllPaths option in our next minor release that will run validation on every path, not just modified paths.

@vkarpov15 vkarpov15 changed the title Validation does not occur for subdocuments. validateAllPaths option to validate() and save() that validates all paths in the schema, not just modified paths Mar 11, 2024
@vkarpov15 vkarpov15 modified the milestones: 8.2.2, 8.3 Mar 11, 2024
@vkarpov15 vkarpov15 added new feature This change adds new functionality, like a new method or class and removed help This issue can likely be resolved in GitHub issues. No bug fixes, features, or docs necessary labels Mar 11, 2024
@Zig1375
Copy link
Author

Zig1375 commented Mar 11, 2024

thank you!

vkarpov15 added a commit that referenced this issue Mar 27, 2024
feat(document): add `validateAllPaths` option to `validate()` and `validateSync()`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new feature This change adds new functionality, like a new method or class
Projects
None yet
Development

No branches or pull requests

3 participants