Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Merge pull request #398 from ktmud/validate

also pass data to `beforeValidate` hook, even when validation is not set
  • Loading branch information...
commit 7370ced70c501f47aae0c6b2f40177260dcf059d 2 parents 5fba357 + 7088e7a
@1602 authored
View
2  lib/hooks.js
@@ -40,7 +40,7 @@ Hookable.prototype.trigger = function trigger(actionName, work, data, quit) {
beforeHook.call(inst, function (err) {
if (err) {
if (quit) {
- quit(err);
+ quit.call(inst, err);
}
return;
}
View
56 lib/model.js
@@ -251,32 +251,32 @@ AbstractClass.create = function (data, callback) {
}
if (data instanceof Array) {
- var instances = [];
+ var instances = Array(data.length);
var errors = Array(data.length);
var gotError = false;
var wait = data.length;
if (wait === 0) callback(null, []);
- var instances = [];
for (var i = 0; i < data.length; i += 1) {
(function(d, i) {
- instances.push(Model.create(d, function(err, inst) {
+ Model.create(d, function(err, inst) {
if (err) {
errors[i] = err;
gotError = true;
}
+ instances[i] = inst;
modelCreated();
- }));
+ });
})(data[i], i);
}
- return instances;
-
function modelCreated() {
if (--wait === 0) {
callback(gotError ? errors : null, instances);
}
}
+
+ return instances;
}
@@ -294,11 +294,12 @@ AbstractClass.create = function (data, callback) {
}
else {
// validation required
- obj.isValid(function(valid) {
+ obj.isValid(function(err, valid) {
if (valid) {
create();
} else {
- callback(new ValidationError(obj), obj);
+ err = err || new ValidationError(obj);
+ callback(err, obj);
}
}, data);
}
@@ -317,6 +318,7 @@ AbstractClass.create = function (data, callback) {
rev = Model._fromDB(rev);
obj._rev = rev
}
+ // if error occurs, we should not return a valid obj
if (err) {
return callback(err, obj);
}
@@ -685,17 +687,16 @@ AbstractClass.prototype.save = function (options, callback) {
return save();
}
- inst.isValid(function (valid) {
+ inst.isValid(function (err, valid) {
if (valid) {
- save();
- } else {
- var err = new ValidationError(inst);
- // throws option is dangerous for async usage
- if (options.throws) {
- throw err;
- }
- callback(err, inst);
+ return save();
+ }
+ err = err || new ValidationError(inst);
+ // throws option is dangerous for async usage
+ if (options.throws) {
+ throw err;
}
+ callback.call(inst, err);
}, data);
// then save
@@ -709,7 +710,7 @@ AbstractClass.prototype.save = function (options, callback) {
inst._initProperties(data, false);
updateDone.call(inst, function () {
saveDone.call(inst, function () {
- callback(err, inst);
+ callback.call(inst, err, inst);
});
});
});
@@ -831,14 +832,14 @@ AbstractClass.prototype.updateAttribute = function updateAttribute(name, value,
* @param {Object} data - data to update
* @param {Function} callback - callback called with (err, instance)
*/
-AbstractClass.prototype.updateAttributes = function updateAttributes(data, cb) {
+AbstractClass.prototype.updateAttributes = function updateAttributes(data, callback) {
if (stillConnecting(this.constructor.schema, this, arguments)) return;
var inst = this;
var modelName = this.constructor.modelName;
if (typeof data === 'function') {
- cb = data;
+ callback = data;
data = null;
}
@@ -851,10 +852,11 @@ AbstractClass.prototype.updateAttributes = function updateAttributes(data, cb) {
inst[key] = data[key];
});
- inst.isValid(function (valid) {
+ inst.isValid(function (err, valid) {
if (!valid) {
- if (cb) {
- cb(new ValidationError(inst), inst);
+ err = err || new ValidationError(inst);
+ if (callback) {
+ callback.call(inst, err);
}
} else {
inst.trigger('save', function (saveDone) {
@@ -873,14 +875,14 @@ AbstractClass.prototype.updateAttributes = function updateAttributes(data, cb) {
}
done.call(inst, function () {
saveDone.call(inst, function () {
- if (cb) {
- cb(err, inst);
+ if (callback) {
+ callback.call(inst, err, inst);
}
});
});
});
- }, data, cb);
- }, data, cb);
+ }, data, callback);
+ }, data, callback);
}
}, data);
};
View
37 lib/validations.js
@@ -22,7 +22,7 @@ var Validatable = require('./model.js');
/**
* Validate presence. This validation fails when validated field is blank.
- *
+ *
* Default error message "can't be blank"
*
* @example presence of title
@@ -92,7 +92,7 @@ Validatable.validatesNumericalityOf = getConfigurator('numericality');
/**
* Validate inclusion in set
*
- * @example
+ * @example
* ```
* User.validatesInclusionOf('gender', {in: ['male', 'female']});
* User.validatesInclusionOf('role', {
@@ -338,20 +338,38 @@ function getConfigurator(name, opts) {
* if (valid) res.render({user: user});
* else res.flash('error', 'User is not valid'), console.log(user.errors), res.redirect('/users');
* });
+ *
+ * // when the callback needs two arguments,
+ * // the first argument is the validation errors,
+ * // or error from `beforeValidate/afterValidate` hook
+ * user.isValid(function (error, isValid) {
+ * error; // error from `beforeValidate`, mostly `null`
+ * })
* ```
*/
Validatable.prototype.isValid = function (callback, data) {
var valid = true, inst = this, wait = 0, async = false;
+ callback = callback || NOOP;
+
+ // callback only needs one arguments, then the signature is `callback(valid)`
+ // convert it to standard node.js callback style
+ if (callback.length < 2) {
+ var _callback = callback;
+ callback = function (err, valid) {
+ _callback(valid);
+ };
+ }
+
// exit with success when no errors
if (!this.constructor._validations) {
cleanErrors(this);
if (callback) {
this.trigger('validate', function (validationsDone) {
validationsDone.call(inst, function() {
- callback(valid);
+ callback(null, valid);
});
- });
+ }, data, function (err) { callback(err, false) });
}
return valid;
}
@@ -385,7 +403,7 @@ Validatable.prototype.isValid = function (callback, data) {
validationsDone.call(inst, function() {
if (valid) cleanErrors(inst);
if (callback) {
- callback(valid);
+ callback(valid ? null : new ValidationError(this), valid);
}
});
}
@@ -396,13 +414,13 @@ Validatable.prototype.isValid = function (callback, data) {
validationsDone.call(inst, function () {
if (valid && !asyncFail) cleanErrors(inst);
if (callback) {
- callback(valid && !asyncFail);
+ callback(valid ? null : new ValidationError(this), valid && !asyncFail);
}
});
}
}
- }, data);
+ }, data, function (err) { callback(err, false) });
if (async) {
// in case of async validation we should return undefined here,
@@ -418,7 +436,7 @@ function cleanErrors(inst) {
Object.defineProperty(inst, 'errors', {
enumerable: false,
configurable: true,
- value: false
+ value: null
});
}
@@ -625,4 +643,7 @@ function ValidationError(obj) {
Error.call(this);
};
+
+function NOOP(error, result) {}
+
ValidationError.prototype.__proto__ = Error.prototype;
View
7 test/manipulation.test.js
@@ -106,7 +106,8 @@ describe('manipulation', function() {
should.exist(persons);
persons.should.have.lengthOf(batch.length);
- persons[0].errors.should.be.false;
+ should.not.exist(persons[0].errors);
+ should.exist(persons[2].errors);
done();
}).should.be.instanceOf(Array);
}).should.have.lengthOf(3);
@@ -173,7 +174,7 @@ describe('manipulation', function() {
return false;
};
p.isValid().should.be.false;
-
+
p.save({ validate: false }, function (err) {
should.not.exist(err);
p.isNewRecord().should.be.false;
@@ -234,7 +235,7 @@ describe('manipulation', function() {
describe('destroy', function() {
it('should destroy record', function(done) {
- Person.create(function(err, p){
+ Person.create(function(err, p){
p.destroy(function(err) {
should.not.exist(err);
Person.exists(p.id, function(err, ex) {
View
54 test/validations.test.js
@@ -41,6 +41,7 @@ describe('validations', function() {
});
beforeEach(function(done) {
+ User.beforeValidate = null
User.destroyAll(function() {
delete User._validations;
done();
@@ -51,6 +52,51 @@ describe('validations', function() {
db.disconnect();
});
+ describe('hooks', function() {
+
+ it('should trigger beforeValidate with data (has validations)', function(done) {
+
+ User.validatesPresenceOf('name');
+ User.beforeValidate = function(next, data) {
+ should.exist(data)
+ next(new Error('Fail'));
+ };
+
+ var user = new User;
+ user.isValid(function(valid) {
+ // when validate hook fails, valid should be false
+ valid.should.equal(false)
+ done()
+ }, { name: 'test' })
+ });
+
+ it('should trigger beforeValidate with data (no validations set)', function(done) {
+ User.beforeValidate = function(next, data) {
+ should.exist(data)
+ data.name.should.equal('test')
+ next();
+ };
+ var user = new User;
+ user.isValid(function(valid) {
+ valid.should.equal(true)
+ done()
+ }, { name: 'test' })
+ });
+
+ it('should allow flow break by pass error to callback', function(done) {
+
+ User.beforeValidate = function(next) {
+ next(new Error('failed'));
+ };
+ User.create(function(err, model) {
+ should.exist(err);
+ should.exist(model);
+ done()
+ })
+ })
+
+ })
+
describe('commons', function() {
describe('skipping', function() {
@@ -215,7 +261,7 @@ describe('validations', function() {
User.validatesLengthOf('gender', {max: 6});
var u = new User(getValidAttributes());
u.isValid(function(valid) {
- u.errors.should.be.not.ok;
+ should.not.exist(u.errors);
valid.should.be.true;
u.gender = 'undefined';
u.isValid(function(valid) {
@@ -234,7 +280,7 @@ describe('validations', function() {
valid.should.be.false;
u.bio = 'undefined';
u.isValid(function(valid) {
- u.errors.should.be.not.ok;
+ should.not.exist(u.errors);
valid.should.be.true;
done();
});
@@ -245,11 +291,11 @@ describe('validations', function() {
User.validatesLengthOf('countryCode', {is: 2});
var u = new User(getValidAttributes());
u.isValid(function(valid) {
- u.errors.should.be.not.ok;
+ should.not.exist(u.errors);
valid.should.be.true;
u.countryCode = 'RUS';
u.isValid(function(valid) {
- u.errors.should.be.ok;
+ should.exist(u.errors);
valid.should.be.false;
done();
});
Please sign in to comment.
Something went wrong with that request. Please try again.