From 0a7b2d48cba4f6a3135dc7a1b479c2afab62a972 Mon Sep 17 00:00:00 2001 From: Jason Quense Date: Mon, 1 Feb 2016 14:08:13 -0500 Subject: [PATCH] [fixed] type and whitelist/blacklist checks threw inconsistent errors closes #19 --- .eslintrc | 2 +- src/mixed.js | 127 ++++++++++++++++++++--------------- src/util/createValidation.js | 17 ++--- src/util/validation-error.js | 1 + test/mixed.js | 34 +++++++++- 5 files changed, 113 insertions(+), 68 deletions(-) diff --git a/.eslintrc b/.eslintrc index d04ebeec3..2be25f81d 100644 --- a/.eslintrc +++ b/.eslintrc @@ -23,7 +23,7 @@ "no-unused-vars": [2, {"vars": "all", "args": "after-used"}], "no-use-before-define": 0, "key-spacing": 0, - "consitent-return": 0, + "consistent-return": 0, "no-shadow": 0, "no-sequences": 0 } diff --git a/src/mixed.js b/src/mixed.js index f079ef03c..71f76f007 100644 --- a/src/mixed.js +++ b/src/mixed.js @@ -10,10 +10,14 @@ var Promise = require('promise/lib/es6-extensions') , createValidation = require('./util/createValidation') , BadSet = require('./util/set'); -let formatError = ValidationError.formatError - let notEmpty = value => !isAbsent(value); +function runValidations(validations, endEarly, value, path) { + return endEarly + ? Promise.all(validations) + : _.collectErrors(validations, value, path) +} + module.exports = SchemaType function SchemaType(options = {}){ @@ -27,7 +31,10 @@ function SchemaType(options = {}){ this._blacklist = new BadSet() this.tests = [] this.transforms = [] - this._typeError = formatError(locale.notType) + + this.withMutation(() => { + this.typeError(locale.notType) + }) if (_.has(options, 'default')) this._defaultDefault = options.default @@ -114,9 +121,7 @@ SchemaType.prototype = { //-- tests _validate(_value, options = {}, state = {}) { - let valids = this._whitelist - , invalids = this._blacklist - , context = options.context + let context = options.context , parent = state.parent , value = _value , schema, endEarly, isStrict; @@ -127,41 +132,28 @@ SchemaType.prototype = { let path = state.path - let errors = []; - let reject = () => Promise.reject(new ValidationError(errors, value)); - if (!state.isCast && !isStrict) value = schema._cast(value, options) // value is cast, we can check if it meets type requirements - if (value !== undefined && !schema.isType(value)) { - errors.push(schema._typeError({ value, path, type: schema._type })) - if ( endEarly ) return reject() - } - - // next check Whitelist for matching values - if (valids.length && !valids.has(value)) { - errors.push(schema._whitelistError(valids.values(), path)) - if (endEarly) return reject() - } - - // next check Blacklist for matching values - if (invalids.has(value)) { - errors.push(schema._blacklistError(invalids.values(), path)) - if (endEarly) return reject() - } - - // It makes no sense to validate further at this point if their are errors - if (errors.length) - return reject() - - let result = schema.tests.map(fn => fn({ value, path, state, schema, options })) - - result = endEarly - ? Promise.all(result) - : _.collectErrors(result, value, path) - - return result.then(() => value); + let validationParams = { value, path, state, schema, options } + let initialTests = [] + + if (schema._typeError) + initialTests.push(schema._typeError(validationParams)); + if (schema._whitelistError) + initialTests.push(schema._whitelistError(validationParams)); + if (schema._blacklistError) + initialTests.push(schema._blacklistError(validationParams)); + + return runValidations(initialTests, endEarly, value, path) + .then(() => runValidations( + schema.tests.map(fn => fn(validationParams)) + , endEarly + , value + , path + )) + .then(() => value) }, validate(value, options, cb) { @@ -212,12 +204,6 @@ SchemaType.prototype = { ) }, - typeError(msg){ - var next = this.clone() - next._typeError = formatError(msg) - return next - }, - nullable(value) { var next = this.clone() next._nullable = value === false ? false : true @@ -296,34 +282,67 @@ SchemaType.prototype = { return next }, - oneOf(enums, msg) { + typeError(message) { var next = this.clone() - if( next.tests.length ) - throw new TypeError('Cannot specify values when there are validation rules specified') + next._typeError = createValidation({ + message, + name: 'typeError', + test(value) { + if (value !== undefined && !this.schema.isType(value)) + return this.createError({ + params: { type: this.schema._type } + }) + return true + } + }) + return next + }, - next._whitelistError = (valids, path) => - formatError(msg || locale.oneOf, { values: valids.join(', '), path }) + oneOf(enums, message = locale.oneOf) { + var next = this.clone(); - enums.forEach( val => { + if (next.tests.length) + throw new TypeError('Cannot specify values when there are validation rules specified') + + enums.forEach(val => { next._blacklist.delete(val) next._whitelist.add(val) }) + next._whitelistError = createValidation({ + message, + name: 'oneOf', + test(value) { + let valids = this.schema._whitelist + if (valids.length && !valids.has(value)) + return this.createError({ params: { values: valids.values().join(', ') }}) + return true + } + }) + return next }, - notOneOf(enums, msg) { - var next = this.clone() - - next._blacklistError = (invalids, path) => - formatError(msg || locale.notOneOf, { values: invalids.join(', '), path }) + notOneOf(enums, message = locale.notOneOf) { + var next = this.clone(); enums.forEach( val => { next._whitelist.delete(val) next._blacklist.add(val) }) + next._blacklistError = createValidation({ + message, + name: 'notOneOf', + test(value) { + let invalids = this.schema._blacklist + if (invalids.length && invalids.has(value)) + return this.createError({ params: { values: invalids.values().join(', ') }}) + return true + } + }) + return next }, diff --git a/src/util/createValidation.js b/src/util/createValidation.js index b9aa1d057..09aee019a 100644 --- a/src/util/createValidation.js +++ b/src/util/createValidation.js @@ -1,24 +1,19 @@ 'use strict'; var Promise = require('promise/lib/es6-extensions') - , Condition = require('./condition') - , ValidationError = require('./validation-error') - , getter = require('property-expr').getter - , locale = require('../locale.js').mixed - , _ = require('./_'); + , ValidationError = require('./validation-error'); let formatError = ValidationError.formatError - -function createErrorFactory(orginalMessage, orginalPath, value, params, originalType) { - return function createError({ path = orginalPath, message = orginalMessage, type = originalType } = {}) { +function createErrorFactory(orginalMessage, orginalPath, value, orginalParams, originalType) { + return function createError({ path = orginalPath, message = orginalMessage, type = originalType, params } = {}) { return new ValidationError( - formatError(message, { path, value, ...params}), value, path, type) + formatError(message, { path, value, ...orginalParams, ...params }), value, path, type) } } module.exports = function createValidation(options) { let { name, message, test, params, useCallback } = options - + function validate({ value, path, state: { parent }, ...rest }) { var createError = createErrorFactory(message, path, value, params, name) var ctx = { path, parent, createError, type: name, ...rest } @@ -43,3 +38,5 @@ module.exports = function createValidation(options) { return validate } + +module.exports.createErrorFactory = createErrorFactory diff --git a/src/util/validation-error.js b/src/util/validation-error.js index e0900267c..b1cce69fb 100644 --- a/src/util/validation-error.js +++ b/src/util/validation-error.js @@ -38,6 +38,7 @@ ValidationError.isError = function(err){ } ValidationError.formatError = function(message, params) { + if ( typeof message === 'string') message = replace(message) diff --git a/test/mixed.js b/test/mixed.js index b766d2634..7f30668aa 100644 --- a/test/mixed.js +++ b/test/mixed.js @@ -40,6 +40,23 @@ describe( 'Mixed Types ', function(){ return inst.cast().should.equal('hello') }) + it('should check types', function(){ + var inst = string().strict().typeError('must be a ${type}!') + + return Promise.all([ + inst.validate(5).should.be.rejected.then(function(err) { + err.type.should.equal('typeError') + err.message.should.equal('must be a string!') + err.inner.length.should.equal(0) + }), + inst.validate(5, { abortEarly: false }).should.be.rejected.then(function(err) { + chai.expect(err.type).to.not.exist + err.message.should.equal('must be a string!') + err.inner.length.should.equal(1) + }) + ]) + }) + it('should limit values', function(){ var inst = mixed().oneOf(['hello', 5]) @@ -65,11 +82,22 @@ describe( 'Mixed Types ', function(){ inst.validate(5).should.be.rejected.then(function(err){ err.errors[0].should.equal('this must not be one the following values: hello, 5') }), - inst.oneOf([5]).isValid(5).should.eventually.equal(true) ]) }) + it('should run subset of validations first', function(){ + var called = false; + var inst = string() + .strict() + .test('test', 'boom', () => called = true) + + return inst.validate(25).should.be.rejected + .then(() => { + called.should.equal(false) + }) + }) + it('should respect strict', function(){ var inst = string().equals(['hello', '5']) @@ -181,7 +209,7 @@ describe( 'Mixed Types ', function(){ message: 'invalid', exclusive: true, name: 'max', - test: function(){ + test() { this.path.should.equal('test') this.parent.should.eql({ other: 5, test : 'hi' }) this.options.context.should.eql({ user: 'jason' }) @@ -197,7 +225,7 @@ describe( 'Mixed Types ', function(){ var inst = mixed().test({ message: 'invalid ${path}', name: 'max', - test: function(){ + test() { return this.createError({ path: 'my.path' }) } })