diff --git a/.travis.yml b/.travis.yml index 84fd7ca2..58f23716 100644 --- a/.travis.yml +++ b/.travis.yml @@ -2,4 +2,4 @@ language: node_js node_js: - 0.6 - 0.8 - - 0.9 + - 0.10 diff --git a/lib/hooks.js b/lib/hooks.js index bc5faa17..11ed5a4a 100644 --- a/lib/hooks.js +++ b/lib/hooks.js @@ -5,8 +5,8 @@ function Hookable() { }; Hookable.afterInitialize = null; -Hookable.beforeValidation = null; -Hookable.afterValidation = null; +Hookable.beforeValidate = 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 diff --git a/lib/model.js b/lib/model.js index bac516df..a868f309 100644 --- a/lib/model.js +++ b/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) { - obj.trigger('create', function (done) { + // validation required + 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. - // The fix is per issue #72 and the fix was found by by5739. + function create() { + obj.trigger('create', function(createDone) { + obj.trigger('save', function(saveDone) { - this._adapter().create(modelName, this.constructor._forDB(data), function (err, id, rev) { - if (id) { - obj.__data.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); + this._adapter().create(modelName, this.constructor._forDB(data), function (err, id, rev) { + obj._initProperties(data, false); + if (id) { + obj.__data.id = id; + obj.__dataWas.id = id; + defineReadonlyProp(obj, 'id', id); } + if (rev) { + obj._rev = rev + } + if (err) { + return callback(err, obj); + } + saveDone.call(obj, function () { + createDone.call(obj, function () { + callback(err, obj); + }); + }); }); - }.bind(this)); - }, obj); + }, data); + }, data); } }; @@ -618,52 +608,50 @@ AbstractClass.prototype.save = function (options, callback) { options.throws = false; } - if (options.validate) { - this.isValid(function (valid) { - if (valid) { - save.call(this); - } else { - var err = new Error('Validation error'); - // throws option is dangerous for async usage - if (options.throws) { - throw err; - } - callback(err, this); - } - }.bind(this)); - } else { - save.call(this); + var inst = this; + var data = inst.toObject(true); + var Model = this.constructor; + var modelName = Model.modelName; + + if (!this.id) { + return Model.create(this, callback); } + // 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) { - var modelName = this.constructor.modelName; - var data = this.toObject(true); - var inst = this; - if (inst.id) { - inst.trigger('update', function (updateDone) { - inst._adapter().save(modelName, inst.constructor._forDB(data), function (err) { - if (err) { - console.log(err); - } else { - inst._initProperties(data, false); - } - updateDone.call(inst, function () { - saveDone.call(inst, function () { - callback(err, inst); - }); + inst.trigger('save', function (saveDone) { + inst.trigger('update', function (updateDone) { + inst._adapter().save(modelName, inst.constructor._forDB(data), function (err) { + if (err) { + return 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); - }); }); - } - - }, this); + }, data); + }, data); } }; @@ -784,15 +772,15 @@ AbstractClass.prototype.updateAttributes = function updateAttributes(data, cb) { inst[key] = data[key]; }); - 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 { + 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); }; diff --git a/lib/validations.js b/lib/validations.js index e005ea7c..f4476550 100644 --- a/lib/validations.js +++ b/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); diff --git a/test/basic-querying.test.js b/test/basic-querying.test.js index 5129c5af..039f6321 100644 --- a/test/basic-querying.test.js +++ b/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() { diff --git a/test/common.batch.js b/test/common.batch.js index 9f0b4e40..f93f4535 100644 --- a/test/common.batch.js +++ b/test/common.batch.js @@ -1,2 +1,3 @@ require('./basic-querying.test.js'); require('./hooks.test.js'); +require('./relations.test.js'); diff --git a/test/hooks.test.js b/test/hooks.test.js index 47031856..30c97480 100644 --- a/test/hooks.test.js +++ b/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) { diff --git a/test/manipulation.test.js b/test/manipulation.test.js index 490ab3d9..723e2770 100644 --- a/test/manipulation.test.js +++ b/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 invalid object (skipping validation)'); - it('should save throw error on validation'); + + it('should save new object', function(done) { + var p = new Person; + 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() { diff --git a/test/relations.test.js b/test/relations.test.js index b41a606e..44eb8a52 100644 --- a/test/relations.test.js +++ b/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() {