Merge pull request #259 from fabien/fix/refactor-validations

Validations configuration as object

lib/dao.js

@@ -8,8 +8,7 @@ module.exports = DataAccessObject;
  * Module dependencies
  */
 var jutil = require('./jutil');
-var validations = require('./validations.js');
-var ValidationError = validations.ValidationError;
+var ValidationError = require('./validations').ValidationError;
 var Relation = require('./relations.js');
 var Inclusion = require('./include.js');
 var List = require('./list.js');

lib/model.js

@@ -11,7 +11,7 @@ var util = require('util');
 var jutil = require('./jutil');
 var List = require('./list');
 var Hookable = require('./hooks');
-var validations = require('./validations.js');
+var validations = require('./validations');
 var _extend = util._extend;
 
 // Set up an object for quick lookup

lib/validations.js

@@ -1,4 +1,6 @@
 var util = require('util');
+var extend = util._extend;
+
 /*!
  * Module exports
  */
@@ -369,7 +371,9 @@ var validators = {
 
 function getConfigurator(name, opts) {
   return function () {
-    configure(this, name, arguments, opts);
+    var args = Array.prototype.slice.call(arguments);
+    args[1] = args[1] || {};
+    configure(this, name, args, opts);
   };
 }
 
@@ -407,9 +411,10 @@ function getConfigurator(name, opts) {
  */
 Validatable.prototype.isValid = function (callback, data) {
   var valid = true, inst = this, wait = 0, async = false;
+  var validations = this.constructor.validations;
 
   // exit with success when no errors
-  if (!this.constructor._validations) {
+  if (typeof validations !== 'object') {
     cleanErrors(this);
     if (callback) {
       this.trigger('validate', function (validationsDone) {
@@ -431,21 +436,25 @@ Validatable.prototype.isValid = function (callback, data) {
     var inst = this,
       asyncFail = false;
 
-    this.constructor._validations.forEach(function (v) {
-      if (v[2] && v[2].async) {
-        async = true;
-        wait += 1;
-        process.nextTick(function () {
-          validationFailed(inst, v, done);
-        });
-      } else {
-        if (validationFailed(inst, v)) {
-          valid = false;
+    var attrs = Object.keys(validations || {});
+    
+    attrs.forEach(function(attr) {
+      var attrValidations = validations[attr] || [];
+      attrValidations.forEach(function(v) {
+        if (v.options && v.options.async) {
+          async = true;
+          wait += 1;
+          process.nextTick(function () {
+            validationFailed(inst, attr, v, done);
+          });
+        } else {
+          if (validationFailed(inst, attr, v)) {
+            valid = false;
+          }
         }
-      }
-
+      });
     });
-
+    
     if (!async) {
       validationsDone.call(inst, function () {
         if (valid) cleanErrors(inst);
@@ -487,11 +496,9 @@ function cleanErrors(inst) {
   });
 }
 
-function validationFailed(inst, v, cb) {
-  var attr = v[0];
-  var conf = v[1];
-  var opts = v[2] || {};
-
+function validationFailed(inst, attr, conf, cb) {
+  var opts = conf.options || {};
+  
   if (typeof attr !== 'string') return false;
 
   // here we should check skip validation conditions (if, unless)
@@ -615,12 +622,12 @@ function blank(v) {
 }
 
 function configure(cls, validation, args, opts) {
-  if (!cls._validations) {
-    Object.defineProperty(cls, '_validations', {
+  if (!cls.validations) {
+    Object.defineProperty(cls, 'validations', {
       writable: true,
       configurable: true,
       enumerable: false,
-      value: []
+      value: {}
     });
   }
   args = [].slice.call(args);
@@ -634,9 +641,13 @@ function configure(cls, validation, args, opts) {
     conf.customValidator = args.pop();
   }
   conf.validation = validation;
-  args.forEach(function (attr) {
-    cls._validations.push([attr, conf, opts]);
-  });
+  var attr = args[0];
+  if (typeof attr === 'string') {
+    var validation = extend({}, conf);
+    validation.options = opts || {};
+    cls.validations[attr] = cls.validations[attr] || [];
+    cls.validations[attr].push(validation);
+  }
 }
 
 function Errors() {

test/manipulation.test.js

@@ -102,7 +102,7 @@ describe('manipulation', function () {
 
         Person.validatesPresenceOf('name');
         Person.create(batch,function (errors, persons) {
-          delete Person._validations;
+          delete Person.validations;
           should.exist(errors);
           errors.should.have.lengthOf(batch.length);
           should.not.exist(errors[0]);

test/validations.test.js

@@ -39,7 +39,7 @@ describe('validations', function () {
 
   beforeEach(function (done) {
     User.destroyAll(function () {
-      delete User._validations;
+      delete User.validations;
       done();
     });
   });
@@ -67,7 +67,7 @@ describe('validations', function () {
     describe('lifecycle', function () {
 
       it('should work on create', function (done) {
-        delete User._validations;
+        delete User.validations;
         User.validatesPresenceOf('name');
         User.create(function (e, u) {
           should.exist(e);
@@ -79,7 +79,7 @@ describe('validations', function () {
       });
 
       it('should work on update', function (done) {
-        delete User._validations;
+        delete User.validations;
         User.validatesPresenceOf('name');
         User.create({name: 'Valid'}, function (e, d) {
           d.updateAttribute('name', null, function (e) {
@@ -95,7 +95,7 @@ describe('validations', function () {
       });
 
       it('should return error code', function (done) {
-        delete User._validations;
+        delete User.validations;
         User.validatesPresenceOf('name');
         User.create(function (e, u) {
           should.exist(e);
@@ -112,7 +112,7 @@ describe('validations', function () {
       });
 
       it('should include validation messages in err.message', function(done) {
-        delete User._validations;
+        delete User.validations;
         User.validatesPresenceOf('name');
         User.create(function (e, u) {
           should.exist(e);
@@ -122,7 +122,7 @@ describe('validations', function () {
       });
 
       it('should include model name in err.message', function(done) {
-        delete User._validations;
+        delete User.validations;
         User.validatesPresenceOf('name');
         User.create(function (e, u) {
           should.exist(e);
@@ -130,6 +130,14 @@ describe('validations', function () {
           done();
         });
       });
+      
+      it('should return validation metadata', function() {
+        var expected = {name:[{validation: 'presence', options: {}}]};
+        delete User.validations;
+        User.validatesPresenceOf('name');
+        var validations = User.validations;
+        validations.should.eql(expected);
+      });
     });
   });