From 82368eef0ad9faa0ea308d4e125d7b1a5d0abbdb Mon Sep 17 00:00:00 2001 From: Benjamin Pannell Date: Wed, 11 Dec 2013 12:26:43 +0200 Subject: [PATCH] Fixed validation issues, updated tests --- lib/Instance.js | 4 +--- lib/Model.js | 8 +++++--- lib/utils/validation.js | 41 ++++++++++++++++++++++------------------- test/instance.js | 18 +++++++++++------- test/validation.js | 19 +++++++++++++++++-- 5 files changed, 56 insertions(+), 34 deletions(-) diff --git a/lib/Instance.js b/lib/Instance.js index 95bd90b..ea9def2 100644 --- a/lib/Instance.js +++ b/lib/Instance.js @@ -24,8 +24,6 @@ function Instance(model, doc, isNew) { "use strict"; var $ = this; - - validate.extra = model.extraValidators; if(!isNew) model.fromSource(doc); @@ -49,7 +47,7 @@ function Instance(model, doc, isNew) { }, set: function(value) { /// Set the value of this field. Changes may be committed by calling save() on this instance. - var validation = validate(schema[targetProperty], value, targetProperty); + var validation = validate(schema[targetProperty], value, targetProperty, model.extraValidators); if (!validation.passed) throw validation.toError(); newDoc[targetProperty] = value; diff --git a/lib/Model.js b/lib/Model.js index 84db740..804bcdb 100644 --- a/lib/Model.js +++ b/lib/Model.js @@ -68,13 +68,15 @@ function Model(database, collection, schema, options) { enumerable: false }); + + var extraValidators = []; for(var i = 0; i < database.plugins.length; i++) { if(database.plugins[i].validate) - validation.extra.push(database.plugins[i].validate); + extraValidators.push(database.plugins[i].validate); } Object.defineProperty(this, 'extraValidators', { - get: function() { return validation.extra; } + get: function() { return extraValidators; } }); for(var i = 0; i < database.plugins.length; i++) { @@ -263,7 +265,7 @@ Model.prototype.insert = Model.prototype.create = function (object, callback) { if(err) return prepNext(err); // Validate the object - var validation = validate($.options.schema, obj); + var validation = validate($.options.schema, obj, $.extraValidators); if(!validation.passed) return prepNext(validation.toError()); // Handle any renames diff --git a/lib/utils/validation.js b/lib/utils/validation.js index 29f25d7..0f16878 100644 --- a/lib/utils/validation.js +++ b/lib/utils/validation.js @@ -4,11 +4,14 @@ var _ = require('lodash'); require('./String'); -var validateType = (require.modules || {}).validation = module.exports = function (schemaType, value, property) { +(require.modules || {}).validation = module.exports = validateType; + +function validateType(schemaType, value, property, extra) { /// /// Validates the given value against a specified schema type field /// The type of data specified to be contained within the Schema /// The data to validate + /// Additional validation engines /// "use strict"; @@ -34,8 +37,8 @@ var validateType = (require.modules || {}).validation = module.exports = functio got: got }, toError: function () { - if (this.reason.property) return new Error(String.format('Failed to validate, expected value of {0} to be a {1} but got {2}', this.reason.property, this.reason.expected, this.reason.got)); - return new Error(String.format('Failed to validate, expected {0} but got {1}', this.reason.expected, this.reason.got)); + if (this.reason.property) return new Error(String.format('Failed to validate, expected value of {0} to be a {1} but got {2}', this.reason.property, this.reason.expected, this.reason.got || 'nothing')); + return new Error(String.format('Failed to validate, expected {0} but got {1}', this.reason.expected, this.reason.got || 'nothing')); } }; }; @@ -72,14 +75,14 @@ var validateType = (require.modules || {}).validation = module.exports = functio if (_.isPlainObject(schemaType)) { if (schemaType.$type) { if (schemaType.$required === false && !value) return pass; - return validateType(schemaType.$type, value, property); + return validateType(schemaType.$type, value, property, extra); } else if (schemaType.$propertyType) { if (schemaType.$required === false && !value) return pass; // Check sub properties for (var k in value) { - var result = validateType(schemaType.$propertyType, value[k], propertyPrefix + k); + var result = validateType(schemaType.$propertyType, value[k], propertyPrefix + k, extra); if (!result.passed) return result; } return pass; @@ -89,7 +92,7 @@ var validateType = (require.modules || {}).validation = module.exports = functio // Check sub properties for (var k in schemaType) { - var result = validateType(schemaType[k], value[k], propertyPrefix + k); + var result = validateType(schemaType[k], value[k], propertyPrefix + k, extra); if (!result.passed) return result; } return pass; @@ -100,7 +103,7 @@ var validateType = (require.modules || {}).validation = module.exports = functio // Validate array values for (var i = 0; i < value.length; i++) { - var result = validateType(schemaType[0], value[i], property + '[' + i + ']'); + var result = validateType(schemaType[0], value[i], property + '[' + i + ']', extra); if (!result.passed) return result; } @@ -116,18 +119,18 @@ var validateType = (require.modules || {}).validation = module.exports = functio if (schemaType === Date) return assert(_.isDate(value), 'date'); if (schemaType instanceof RegExp) return assert(schemaType.test(value || ''), 'regex match on ' + schemaType.toString(), (value || 'nothing').toString()); - var virtualThis = { - fail: fail, - assert: assert, - pass: pass - }; + if(extra) { + var virtualThis = { + fail: fail, + assert: assert, + pass: pass + }; - for(var i = 0; i < validateType.extra.length; i++) { - var result = validateType.extra[i].call(virtualThis, schemaType, value, property); - if(result) return result; + for(var i = 0; i < extra.length; i++) { + var result = extra[i].call(virtualThis, schemaType, value, property); + if(result) return result; + } } - return assert(value instanceof schemaType, 'instanceof ' + schemaType.toString()); -}; - -validateType.extra = []; \ No newline at end of file + return pass; +}; \ No newline at end of file diff --git a/test/instance.js b/test/instance.js index 875644a..236858b 100644 --- a/test/instance.js +++ b/test/instance.js @@ -8,6 +8,10 @@ describe('orm', function () { "use strict"; describe('Instance', function () { + var db = { + plugins: [] + }; + describe('diff', function () { it('should generate $set for basic changes', function () { Instance.diff({ x: 1 }, { x: 2 }).should.eql({ $set: { x: 2 } }); @@ -33,7 +37,7 @@ describe('orm', function () { describe('constructor', function () { it('should present all properties of the document', function () { - var model = new Model(null, 'model', { + var model = new Model(db, 'model', { name: String }, { preprocessors: [] @@ -49,7 +53,7 @@ describe('orm', function () { }); it('should allow renaming of properties', function () { - var model = new Model(null, 'model', { + var model = new Model(db, 'model', { pretty: String },{ preprocessors: [ @@ -68,7 +72,7 @@ describe('orm', function () { }); it('should allow the creation of methods', function () { - var model = new Model(null, 'model', {}, { + var model = new Model(db, 'model', {}, { methods: { test: function() { return true; } } @@ -82,7 +86,7 @@ describe('orm', function () { }); it('should correctly pass all arguments to a method', function () { - var model = new Model(null, 'model', {}, { + var model = new Model(db, 'model', {}, { methods: { test: function (a, b, c) { should.equal(a, 'a'); @@ -100,7 +104,7 @@ describe('orm', function () { }); it('should allow the creation of virtual properties', function () { - var model = new Model(null, 'model', {}, { + var model = new Model(db, 'model', {}, { virtuals: { fullname: function () { return this.firstname + ' ' + this.lastname; } } @@ -116,7 +120,7 @@ describe('orm', function () { }); it('should allow the creation of virtual getter/setters', function() { - var model = new Model(null, 'model', {}, { + var model = new Model(db, 'model', {}, { virtuals: { fullname: { get: function () { return this.firstname + ' ' + this.lastname; }, @@ -142,7 +146,7 @@ describe('orm', function () { }); it('should allow a custom schema', function () { - var model = new Model(null, 'model', { + var model = new Model(db, 'model', { name: String, age: { type: Number, required: false } }, { diff --git a/test/validation.js b/test/validation.js index 37dd2f3..0f7c9ed 100644 --- a/test/validation.js +++ b/test/validation.js @@ -12,8 +12,8 @@ describe('utils', function () { describe('validation', function () { var validation = require('../lib/utils/validation'); - function validate(schema, value, pass, message) { - var result = validation(schema, value); + function validate(schema, value, pass, message, extra) { + var result = validation(schema, value, '', extra); result.should.have.ownProperty('passed', pass, message); return result; } @@ -89,5 +89,20 @@ describe('utils', function () { result.should.have.ownProperty('toError'); should(result.toError() instanceof Error); }); + + it('should allow the use of extra validators', function() { + var extra = function(schema, value, propertyName) { + if(schema == 'Positive') + return this.assert(value >= 0, 'positive real number', value); + }; + + var result = validation({ positive: 'Positive' }, { positive: 1 }, undefined, [extra]); + result.should.have.ownProperty('passed', true); + + validate({ positive: 'Positive', normal: Number }, { positive: 1, normal: -1 }, true, + 'Expected a positive integer to pass validation', [extra]); + validate({ positive: 'Positive', normal: Number }, { positive: -1, normal: -1 }, false, + 'Expected a negative integer to fail validation', [extra]); + }); }); }); \ No newline at end of file