Merge pull request #245 from anatoliychakkaev/master

Hooks, validations, lifecycle

.travis.yml

@@ -2,4 +2,4 @@ language: node_js
 node_js:
   - 0.6
   - 0.8
-  - 0.9
+  - 0.10

lib/hooks.js

@@ -5,8 +5,8 @@ function Hookable() {
 };
 
 Hookable.afterInitialize = null;
-Hookable.beforeValidation = null;
+Hookable.beforeValidate = null;
-Hookable.afterValidation = null;
+Hookable.afterValidate = null;
 Hookable.beforeSave = null;
 Hookable.afterSave = null;
 Hookable.beforeCreate = null;
@@ -18,8 +18,12 @@ Hookable.afterDestroy = null;
 
 Hookable.prototype.trigger = function trigger(actionName, work, data) {
     var capitalizedName = capitalize(actionName);
-    var afterHook = this.constructor["after" + capitalizedName];
     var beforeHook = this.constructor["before" + capitalizedName];
+    var afterHook = this.constructor["after" + capitalizedName];
+    if (actionName === 'validate') {
+        beforeHook = beforeHook || this.constructor.beforeValidation;
+        afterHook = afterHook || this.constructor.afterValidation;
+    }
     var inst = this;
 
     // we only call "before" hook when we have actual action (work) to perform

lib/model.js

@@ -171,59 +171,49 @@ AbstractClass.create = function (data, callback) {
         callback = function () {};
     }
 
-    var obj = null;
+    var obj;
     // if we come from save
     if (data instanceof this && !data.id) {
         obj = data;
-        data = obj.toObject(true);
-        obj._initProperties(data, false);
-        create();
     } else {
         obj = new this(data);
-        data = obj.toObject(true);
-
-        obj.trigger('save', function(saveDone) {
-
-            // validation required
-            obj.isValid(function (valid) {
-                if (!valid) {
-                    callback(new Error('Validation error'), obj);
-                } else {
-                    create(saveDone);
-                }
-            });
-
-        }, obj);
     }
+    data = obj.toObject(true);
 
-    function create(saveDone) {
+    // validation required
-        obj.trigger('create', function (done) {
+    obj.isValid(function(valid) {
+        if (valid) {
+            create();
+        } else {
+            callback(new Error('Validation error'), obj);
+        }
+    }, data);
 
-            var data = this.toObject(true);  // Added this to fix the beforeCreate trigger not fire.
+    function create() {
-                                             // The fix is per issue #72 and the fix was found by by5739.
+        obj.trigger('create', function(createDone) {
+            obj.trigger('save', function(saveDone) {
 
-            this._adapter().create(modelName, this.constructor._forDB(data), function (err, id, rev) {
+                this._adapter().create(modelName, this.constructor._forDB(data), function (err, id, rev) {
-                if (id) {
+                    obj._initProperties(data, false);
-                    obj.__data.id = id;
+                    if (id) {
-                    obj.__dataWas.id = id;
+                        obj.__data.id = id;
-                    defineReadonlyProp(obj, 'id', id);
+                        obj.__dataWas.id = id;
-                }
+                        defineReadonlyProp(obj, 'id', id);
-                if (rev) {
-                    obj._rev = rev
-                }
-                done.call(this, function () {
-                    if (saveDone) {
-                        saveDone.call(obj, function () {
-                            if (callback) {
-                                callback(err, obj);
-                            }
-                        });
-                    } else if (callback) {
-                        callback(err, obj);
                     }
+                    if (rev) {
+                        obj._rev = rev
+                    }
+                    if (err) {
+                        return callback(err, obj);
+                    }
+                    saveDone.call(obj, function () {
+                        createDone.call(obj, function () {
+                            callback(err, obj);
+                        });
+                    });
                 });
-            }.bind(this));
+            }, data);
-        }, obj);
+        }, data);
     }
 };
 
@@ -618,52 +608,50 @@ AbstractClass.prototype.save = function (options, callback) {
         options.throws = false;
     }
 
-    if (options.validate) {
+    var inst = this;
-        this.isValid(function (valid) {
+    var data = inst.toObject(true);
-            if (valid) {
+    var Model = this.constructor;
-                save.call(this);
+    var modelName = Model.modelName;
-            } else {
+
-                var err = new Error('Validation error');
+    if (!this.id) {
-                // throws option is dangerous for async usage
+        return Model.create(this, callback);
-                if (options.throws) {
-                    throw err;
-                }
-                callback(err, this);
-            }
-        }.bind(this));
-    } else {
-        save.call(this);
     }
 
+    // validate first
+    if (!options.validate) {
+        return save();
+    }
+
+    inst.isValid(function (valid) {
+        if (valid) {
+            save();
+        } else {
+            var err = new Error('Validation error');
+            // throws option is dangerous for async usage
+            if (options.throws) {
+                throw err;
+            }
+            callback(err, inst);
+        }
+    });
+
+    // then save
     function save() {
-        this.trigger('save', function (saveDone) {
+        inst.trigger('save', function (saveDone) {
-            var modelName = this.constructor.modelName;
+            inst.trigger('update', function (updateDone) {
-            var data = this.toObject(true);
+                inst._adapter().save(modelName, inst.constructor._forDB(data), function (err) {
-            var inst = this;
+                    if (err) {
-            if (inst.id) {
+                        return callback(err, inst);
-                inst.trigger('update', function (updateDone) {
+                    }
-                    inst._adapter().save(modelName, inst.constructor._forDB(data), function (err) {
+                    inst._initProperties(data, false);
-                        if (err) {
+                    updateDone.call(inst, function () {
-                            console.log(err);
+                        saveDone.call(inst, function () {
-                        } else {
+                            callback(err, inst);
-                            inst._initProperties(data, false);
-                        }
-                        updateDone.call(inst, function () {
-                            saveDone.call(inst, function () {
-                                callback(err, inst);
-                            });
                         });
                     });
-                }, data);
-            } else {
-                inst.constructor.create(inst, function (err) {
-                    saveDone.call(inst, function () {
-                        callback(err, inst);
-                    });
                 });
-            }
+            }, data);
-
+        }, data);
-        }, this);
     }
 };
 
@@ -784,15 +772,15 @@ AbstractClass.prototype.updateAttributes = function updateAttributes(data, cb) {
         inst[key] = data[key];
     });
 
-    inst.trigger('save', function (saveDone) {
+    inst.isValid(function (valid) {
-        inst.trigger('update', function (done) {
+        if (!valid) {
+            if (cb) {
+                cb(new Error('Validation error'), inst);
+            }
+        } else {
+            inst.trigger('save', function (saveDone) {
+                inst.trigger('update', function (done) {
 
-            inst.isValid(function (valid) {
-                if (!valid) {
-                    if (cb) {
-                        cb(new Error('Validation error'), inst);
-                    }
-                } else {
                     Object.keys(data).forEach(function (key) {
                         inst[key] = data[key];
                     });
@@ -810,9 +798,9 @@ AbstractClass.prototype.updateAttributes = function updateAttributes(data, cb) {
                             });
                         });
                     });
-                }
+                }, data);
-            });
+            }, data);
-        }, data);
+        }
     }, data);
 };
 

lib/validations.js

@@ -332,14 +332,18 @@ function getConfigurator(name, opts) {
  * });
  * ```
  */
-Validatable.prototype.isValid = function (callback) {
+Validatable.prototype.isValid = function (callback, data) {
     var valid = true, inst = this, wait = 0, async = false;
 
     // exit with success when no errors
     if (!this.constructor._validations) {
         cleanErrors(this);
         if (callback) {
-            callback(valid);
+            this.trigger('validate', function (validationsDone) {
+                validationsDone.call(inst, function() {
+                    callback(valid);
+                });
+            });
         }
         return valid;
     }
@@ -350,7 +354,7 @@ Validatable.prototype.isValid = function (callback) {
         value: new Errors
     });
 
-    this.trigger('validation', function (validationsDone) {
+    this.trigger('validate', function (validationsDone) {
         var inst = this,
             asyncFail = false;
 
@@ -370,20 +374,20 @@ Validatable.prototype.isValid = function (callback) {
         });
 
         if (!async) {
-            validationsDone();
+            validationsDone.call(inst, callback);
         }
 
         function done(fail) {
             asyncFail = asyncFail || fail;
             if (--wait === 0 && callback) {
                 validationsDone.call(inst, function () {
-                    if( valid && !asyncFail ) cleanErrors(inst);
+                    if (valid && !asyncFail) cleanErrors(inst);
                     callback(valid && !asyncFail);
                 });
             }
         }
 
-    });
+    }, data);
 
     if (!async) {
         if (valid) cleanErrors(this);

test/basic-querying.test.js

@@ -146,7 +146,7 @@ describe('basic-querying', function() {
                 User.findOne(function(e, u) {
                     should.not.exist(e);
                     should.exist(u);
-                    u.id.should.equal(users[0].id);
+                    u.id.toString().should.equal(users[0].id.toString());
                     done();
                 });
             });
@@ -185,6 +185,16 @@ describe('basic-querying', function() {
             });
         });
 
+        it('should work even when find by id', function(done) {
+            User.findOne(function(e, u) {
+                User.findOne({where: {id: u.id}}, function(err, user) {
+                    should.not.exist(err);
+                    should.exist(user);
+                    done();
+                });
+            });
+        });
+
     });
 
     describe('exists', function() {

test/common.batch.js

@@ -1,2 +1,3 @@
 require('./basic-querying.test.js');
 require('./hooks.test.js');
+require('./relations.test.js');

test/hooks.test.js

@@ -42,7 +42,7 @@ describe('hooks', function() {
                 }
             };
             User.create({name: 'Nickolay'}, function(err, u) {
-                u.id.should.be.a('number');
+                u.id.should.be.ok;
                 u.name.should.equal('Nickolay Rozental');
                 done();
             });
@@ -105,7 +105,7 @@ describe('hooks', function() {
         it('should save full object', function(done) {
             User.create(function(err, user) {
                 User.beforeSave = function(next, data) {
-                    data.toObject().should.have.keys('id', 'name', 'email',
+                    data.should.have.keys('id', 'name', 'email',
                         'password', 'state')
                     done();
                 };
@@ -207,10 +207,11 @@ describe('hooks', function() {
     describe('destroy', function() {
         afterEach(removeHooks('Destroy'));
 
-        it('should be triggered on destroy', function() {
+        it('should be triggered on destroy', function(done) {
             var hook = 'not called';
-            User.beforeDestroy = function() {
+            User.beforeDestroy = function(next) {
                 hook = 'called';
+                next();
             };
             User.afterDestroy = function() {
                 hook.should.eql('called');
@@ -221,6 +222,96 @@ describe('hooks', function() {
             });
         });
     });
+
+    describe('lifecycle', function() {
+        var life = [], user;
+        before(function(done) {
+            User.beforeSave     = function(d){life.push('beforeSave');   d();};
+            User.beforeCreate   = function(d){life.push('beforeCreate'); d();};
+            User.beforeUpdate   = function(d){life.push('beforeUpdate'); d();};
+            User.beforeDestroy  = function(d){life.push('beforeDestroy');d();};
+            User.beforeValidate = function(d){life.push('beforeValidate');d();};
+            User.afterInitialize= function( ){life.push('afterInitialize');  };
+            User.afterSave      = function(d){life.push('afterSave');    d();};
+            User.afterCreate    = function(d){life.push('afterCreate');  d();};
+            User.afterUpdate    = function(d){life.push('afterUpdate');  d();};
+            User.afterDestroy   = function(d){life.push('afterDestroy'); d();};
+            User.afterValidate  = function(d){life.push('afterValidate');d();};
+            User.create(function(e, u) {
+                user = u;
+                life = [];
+                done();
+            });
+        });
+        beforeEach(function() {
+            life = [];
+        });
+
+        it('should describe create sequence', function(done) {
+            User.create(function() {
+                life.should.eql([
+                    'afterInitialize',
+                    'beforeValidate',
+                    'afterValidate',
+                    'beforeCreate',
+                    'beforeSave',
+                    'afterInitialize',
+                    'afterSave',
+                    'afterCreate'
+                ]);
+                done();
+            });
+        });
+
+        it('should describe new+save sequence', function(done) {
+            var u = new User;
+            u.save(function() {
+                life.should.eql([
+                    'afterInitialize',
+                    'beforeValidate',
+                    'afterValidate',
+                    'beforeCreate',
+                    'beforeSave',
+                    'afterInitialize',
+                    'afterSave',
+                    'afterCreate'
+                ]);
+                done();
+            });
+        });
+
+        it('should describe new+save sequence', function(done) {
+            var u = new User;
+            u.save(function() {
+                life.should.eql([
+                    'afterInitialize',
+                    'beforeValidate',
+                    'afterValidate',
+                    'beforeCreate',
+                    'beforeSave',
+                    'afterInitialize',
+                    'afterSave',
+                    'afterCreate'
+                ]);
+                done();
+            });
+        });
+
+        it('should describe updateAttributes sequence', function(done) {
+            user.updateAttributes({name: 'Antony'}, function() {
+                life.should.eql([
+                    'beforeValidate',
+                    'afterValidate',
+                    'beforeSave',
+                    'beforeUpdate',
+                    'afterUpdate',
+                    'afterSave',
+                ]);
+                done();
+            });
+        });
+
+    });
 });
 
 function addHooks(name, done) {

test/manipulation.test.js

@@ -76,10 +76,68 @@ describe('manipulation', function() {
     });
 
     describe('save', function() {
-        it('should save new object');
+
-        it('should save existing object');
+        it('should save new object', function(done) {
-        it('should save invalid object (skipping validation)');
+            var p = new Person;
-        it('should save throw error on validation');
+            p.save(function(err) {
+                should.not.exist(err);
+                should.exist(p.id);
+                done();
+            });
+        });
+
+        it('should save existing object', function(done) {
+            Person.findOne(function(err, p) {
+                should.not.exist(err);
+                p.name = 'Hans';
+                p.propertyChanged('name').should.be.true;
+                p.save(function(err) {
+                    should.not.exist(err);
+                    p.propertyChanged('name').should.be.false;
+                    Person.findOne(function(err, p) {
+                        should.not.exist(err);
+                        p.name.should.equal('Hans');
+                        p.propertyChanged('name').should.be.false;
+                        done();
+                    });
+                });
+            });
+        });
+
+        it('should save invalid object (skipping validation)', function(done) {
+            Person.findOne(function(err, p) {
+                should.not.exist(err);
+                p.isValid = function(done) {
+                    process.nextTick(done);
+                    return false;
+                };
+                p.name = 'Nana';
+                p.save(function(err) {
+                    should.exist(err);
+                    p.propertyChanged('name').should.be.true;
+                    p.save({validate: false}, function(err) {
+                        should.not.exist(err);
+                        p.propertyChanged('name').should.be.false;
+                        done();
+                    });
+                });
+            });
+        });
+
+        it('should save throw error on validation', function() {
+            Person.findOne(function(err, p) {
+                should.not.exist(err);
+                p.isValid = function(cb) {
+                    cb(false);
+                    return false;
+                };
+                (function() {
+                    p.save({
+                        'throws': true
+                    });
+                }).should.throw('Validation error');
+            });
+        });
     });
 
     describe('destroy', function() {

test/relations.test.js

@@ -43,7 +43,7 @@ describe('relations', function() {
             (new Author).readers.should.be.an.instanceOf(Function);
             Object.keys((new Reader).toObject()).should.include('authorId');
 
-            db.automigrate(done);
+            db.autoupdate(done);
         });
 
         it('should build record on scope', function(done) {
@@ -115,7 +115,20 @@ describe('relations', function() {
             // (new Fear).mind.build().should.be.an.instanceOf(Mind);
         });
 
-        it('can be declared in short form');
+        it('can be used to query data', function(done) {
+            List.hasMany('todos', {model: Item});
+            List.create(function(e, list) {
+                should.not.exist(e);
+                should.exist(list);
+                list.todos.create(function(err, todo) {
+                    todo.list(function(e, l) {
+                        should.not.exist(e);
+                        should.exist(l);
+                        done();
+                    });
+                });
+            });
+        });
     });
 
     describe('hasAndBelongsToMany', function() {