LearnBoost/mongoose — pull request #1256 by mercmobily (2012-12-19 06:27:03+00:00)
Title: Up1: initial work on validation on update

Hi,

A few points:

* My goal is to have something that works first, and will polish later

* It's very initial code. Sorry if it doesn't follow your guidelines, at this very early stage I will not be as anal about things

* Validation does work. The validation part is possibly OK (although it will no doubt be improved)

* a new function is added in model: _applyValidation(). Name is lame, and should say that it's for updates

* I think we need to modify query.js to get things to work. Explanation follows

OK, SO WHERE DOES THIS CODE GO?
------------------------------------------------------

I spent quite a while this morning trying to figure it out. A better question is: where should validation happen?
My first instinct was to place it into model.js, and check for data _before_ even getting to the query. It turns out I can't, because I could well run something like:

Workspace.findOneAndUpdate( { _id: userId, 'access._id': req.application.userId }, updateObject).exec(function( err, doc)

Basically, findOneAndUpdate will return a "promise" (in case a callback was passed) or a query object (which can then be executed etc.).
It shouldn't go in exec(), for the same reason (it would be missed if we run findOneAndUpdate with a callback).

My final idea is that it should go in query.js, right in _findAndModify(). It already works, by simply running:

this.model._applyValidation(this._updateArg, function(err) {

Note that _updateArg is pretty much guaranteed to work even if you decide to allow chaining of update fields for example. It's the final deal.

NOW.

This function is _really_ confusing me. It returns again either a query, or a callback. The only case we really care about is if there is a callback.

Partially because I haven't managed to figure out promises (and why we are returning them), partially because of my lack of understanding of the whole "let's return a promise" idea in this context, I haven't been able to slot this checking code anywhere in _findAndModify(). I have spent a lot of hours on this now, and I am gracefully asking for help.

Once you give me the OK that this is indeed the right direction, and -- please -- tell me how to make it actually check for the results, I will go ahead with:

* Getting the code so that it follows mongoose's standards
* General clean up
* Some real tests

Note that the scope for the validators are an empty object {}. We don't really have a choice here. A possible solution is to create a "recognisable" object, so that developers can check for that in their code.

Bye.

Merc.