Model Update method does not respect validators #860

Closed
ruimgoncalves opened this Issue Apr 24, 2012 · 57 comments
@ruimgoncalves

I can use the model.Update method to update to any value regardless of schema constraints

For example if I have a Schema with a enum property, using update I can change to any value outside of the enum constraint, thus invalidating the set.

The expected behavior was to throw an error if the updated value did not validate.

I suggest an extra option to the update method to exit on the first error or not.

@aheckmann

you may wish to enable the strict schema setting which will ignore any invalid schema paths and update the rest. no error is thrown.

new Schema({..}, { strict: true })

there is a similar pull request waiting to be merged that throws when setting paths that do not exist in the schema but it does not throw during model.update. maybe we should get that added to the PR above and have the err passed to the callback.

@aheckmann aheckmann closed this Apr 24, 2012
@ruimgoncalves

Hi again, thanks for your quick reply.

I was not referring to invalid paths but to constraint within a specific path.

Take this example

var User = new Schema({
username: {
type: String,
lowercase: true,
trim: true,
unique: true
},
name: {
type: String,
trim: true,
required: true,
unique: false,
index: true
},
password: {
type: String,
required: true,
select : false,
set: encrypt
},
role: {
type: String,
"default" : 'user',
"enum": ['user', 'admin', 'root']
},
created: {
type: Date,
"default": Date.now
}
}, { strict: true });

users.update({"username" : "test"}, {"role" : "thisShowldFail"}, function (err, val){
...
});

Notice that now "test" user has role "thisShowldFail" even with strict schema option enabled.
Shouldn't Update respect enums, and other similar options?

@aheckmann

You are correct, it should validate enums. I thought there was a ticket for this somewhere.

@aheckmann aheckmann reopened this Apr 26, 2012
@kof
kof commented May 4, 2012

+1, can't use #update because of this issue.

@kof

Is it possible to let .update make all things which a new document + .save would do?

  • defaults
  • setter
  • custom validations
  • enum

if upsert option is used, also

  • required
@aheckmann

not currently

@kof

I mean is it possible/planned to implement ... ? :)

@aheckmann

it might be nice. related #472

@skaapgif

+1

It's completely unintuitive that update wouldn't use: defaults, setters, validation and enums. What use is a schema if core operations don't adhere to it.

@yaronn

+1

I love mongoose but if it does not validate updates it is much less helpful to me. I have big documents and I don't want to download them just to do validation.

@mercmobily

This is me jumping in. I am taking a bit of a dive, trying to get this implemented.
The goal is to get:

  • defaults
  • setter
  • validations
  • enum
  • required

Not even sure I will be successful, but I will give it a go. I will start with validators.

Wish me good luck!

Merc.

@neetiraj

+1

@EFF
EFF commented Mar 2, 2013

+1

@rishabhp

Would had been great if .update supported Validations.

@eordano

Bump +1

@wanderer

Bumpy bump +1

@chyld

Please add validations on update!

@yvele

+1

Edit: -2 because I don't think mongoose should take validation in charge.. If you want validation, you should consider using an other specialized library like one using allowing JSON Schema validation

@aheckmann

@thalesfsp Yes. It's been attempted in the past but the rules get very wonky because no document exists in memory and break down in several circumstances leaving behavior inconsistent and confusing.

the beauty of open source: if you want a feature, you can write it and submit a pull request with passing tests and documentation that proves it works properly.

@aheckmann aheckmann closed this Jan 10, 2014
@BrianHoldsworth

A pretty serious limitation. In pursuing a patch, would it be practical to just crawl the Schema object directly to determine whether a validator should be run? I mainly care about custom validators and enum rules being applied to the update. The other schema constraints should have already been applied when the document was saved, I think. Does such a simplification of the problem down to just enums and validators make sense and remove the need to have any Document around during the update?

@dwmkerr

+1 Bump, this would be a very useful feature - when doing updates I'd like to see mins/maxes etc respected from the schema - otherwise I'm doing a lot of boilerplate logic for something mongoose can do

@fiznool

It would be great to see this happen. At the moment my workaround is finding the object, altering the fields and then saving it, which fires the validation middleware. As per the docs:

Tank.findById(id, function (err, tank) {
  tank.size = 'large';
  tank.save(function (err) {
   // Document updated, do something with it
  });
});

I understand that this is made tricky, since the update command delegates directly to Mongo, and the full document doesn't sit in memory to be validated. So the approach suggested by @BrianHoldsworth seems like a good place to start, parsing the Schema running validations only against the fields that are to be updated.

@aheckmann could you provide us with some more details on the previous (failed) implementation efforts so whoever attempts this patch doesn't make the same mistakes again?

@fiznool

I mainly care about custom validators and enum rules being applied to the update. The other schema constraints should have already been applied when the document was saved, I think. Does such a simplification of the problem down to just enums and validators make sense and remove the need to have any Document around during the update?

@BrianHoldsworth I think this might be an oversimplification. What happens if a field with a required: true validation constraint is updated to be an empty string? We would need this to fire a validation error.

@mattcasey

I'm also interested in this. Perhaps a plug-in could be written to override the .update() method and run validation? That way, even a partial solution can be implemented. If it was in core, on the other hand, it would be expected to handle every kind of validation and be 100% robust.

@davepowell

I have run into this as well, with both custom validators and enums. While doing a find and then update is possible, it does make it very difficult to write general case code when documents have differing sub-document structures.

+1 bump this.

Brian's solution seem pretty elegant. Would love to be notified when patches hit beta.

@m-unterberger

need this +1

@vkarpov15 vkarpov15 added this to the 4.0 milestone Jun 27, 2014
@noducks

+1

@mekanics

+1

@vkarpov15 vkarpov15 modified the milestone: 3.9.3, 4.0 Sep 8, 2014
@vkarpov15 vkarpov15 reopened this Sep 21, 2014
@cuva

+1

@vkarpov15 vkarpov15 added a commit that referenced this issue Sep 26, 2014
@vkarpov15 vkarpov15 Half-functioning POC for #860 7ccc571
@vkarpov15 vkarpov15 added a commit that referenced this issue Sep 26, 2014
@vkarpov15 vkarpov15 More complete tests for #860 8375feb
@vkarpov15

In 3.9.3 update() will have 2 special options, setDefaultsOnInsert and runValidators, which will set defaults and run validators on your query. See tests for example, no real docs yet :(

@vkarpov15 vkarpov15 closed this Sep 29, 2014
@davepowell

Thank you so much - this is a great fix!

@yads

@vkarpov15 I used your code to also apply validation to findOneAndUpdate. See PR #2393.

@AlexandreAWE

Hi, do you know when the stable version 3.9 will be released in order to use validation on update ?

Thanks

@vkarpov15

@AlexandreAWE good question. I'm currently wrapping things up on 4.0 and beta testing the 3.9.x branch, but right now its a "it'll be done when its done" thing. I hope to have an RC out before Christmas.

@andrewholsted

@vkarpov15 How's it coming on the 3.9.x stable? It'd be great to have validations on update in lieu of finding and saving every time for my current project.

@vkarpov15

@andrewholsted waiting on mongodb driver 2.0 and mongodb server 2.8 to stabilize - that's hopefully happening this month, but can't really ship 4.0 without supporting latest version of mongodb and latest driver. See my blog for a little more detail.

@zacharynevin

Potentially dumb question by me, but why not take the data being updated and validate that against the schema before passing to mongodb?

Also, +1

@zhiqingchen

mark & wait

@iamceege

Can't wait until 3.9 is stable :)

@Jeff-Lewis Jeff-Lewis referenced this issue in strongloop/loopback-datasource-juggler Mar 5, 2015
Open

Pain-points of the new Operation hooks #482

6 of 8 tasks complete
@bunbutter

Is this feature in yet? Eagerly waiting for 3.9.. Wen will it be available?

@vkarpov15

You can npm install mongoose@unstable to get the unstable version. Current ETA for 4.0 is march 25 - a good place to check this is the milestones page

@andrewdelprete

Just ran into this issue last night. Glad I'm not the only one! I'm excited for this change to be released, thanks for all you do!

+1

@Weblors

Don't know why this issue is marked closed. I'm still facing it.

@m1cah

I'm actually not sure that this works on enum validation for findOneAndUpdate(), even with runValidators set to true.

@vkarpov15

@m1cah please provide example that demonstrates what you're trying to do. We do have tests for this and they pass...

@vkarpov15

Well one issue is that the example above is using a prehistoric version of mongoose:

root@runnable:~# head node_modules/mongoose/package.json                                                                                                                 
{                                                                                                                                                                         
  "name": "mongoose",                                                                                                                                                     
  "description": "Elegant MongoDB object modeling for Node.js",                                                                                                           
  "version": "3.6.14",                                                                                                                                                    
  "author": {                                                                                                                                                             
    "name": "Guillermo Rauch",                                                                                                                                            
    "email": "guillermo@learnboost.com"                                                                                                                                   
  },                                                                                                                                                                      
  "keywords": [                                                                                                                                                           
    "mongodb",  

Try upgrading to 4.x and it should work.

@nirio69510

Are you sure it works with an enum and findOneAndUpdate method ?
On mongoose 4.2.6 it seems to fail, I can set a bad value.

Schema :

var UserSchema = new Schema({
    first_name: {
        type: String,
        required: true,
    },
    last_name: {
        type: String,
        required: true,
    },
    email: {
        type: String,
        unique: true,
        required: true,
    },
    embededData: [{
        type: {
            type: String,
            enum: ['value1', 'value2', 'value3']
            required: true
        }
    }]
}, { strict: true });

FindOneAndUpdate method :

UserModel.findOneAndUpdate(
    {_id: uid}, 
    {$push: {embededData: data}}, 
    { runValidators: true }, function(err) {
});

Then I can push embededData.type = 'Panda';

@vkarpov15

See update validator docs and #2933 - update validators don't run on $push, only on $set and $unset

@ajbraus

+1

@rmacqueen

As far as I can see, this still doesn't work for the case where you are using a custom validator on a field that is an array.

For example, this code snippet:

var mongoose = require('mongoose');
mongoose.connect('mongodb://localhost/myTestDB');

var db = mongoose.connection;

db.on('error', function (err) {
console.log('connection error', err);
});
db.once('open', function () {
console.log('connected.');
});

var Schema = mongoose.Schema;
var userSchema = new Schema({
  _id : String,
  name : {
    type: [String],
    validate: {
        validator: function (str) {
            return str.length > 1
        }
    },
  }
});


var User = mongoose.model('User', userSchema);

User.findOneAndUpdate({"name": ["John", "Doe"]}, { 
  $setOnInsert: {
    name: ["John"],
  },
}, { runValidators: true, upsert: true, new: true }, function (err, data) {
  if (err) {
    return console.log(err);
  } else {
    // console.log(data.validateSync())
    return console.log('Updated', data);
  }
});

will allow you to update the user to have a name field of ["John"] without throwing any errors, even though the custom validator I included explicitly prohibits any name array of length less than or equal to 1. The validator itself works just fine, as can be seen by the fact that if you uncomment the line console.log(data.validateSync()), thereby forcing the validation to take place, it will in fact return the appropriate error message. The problem is that this validation is not taking place within the findOneAndUpdate() call, despite the fact that I include the runValidators=true option.

@vkarpov15

Looks like a bug, can you open up a separate issue for that?

@rmacqueen

Yep, opened #4039

@vkarpov15

Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment