From 9b759c95ac9155e23f1b52efd6d8a46d417f0997 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miroslav=20Bajto=C5=A1?= Date: Mon, 12 Jan 2015 14:52:54 +0100 Subject: [PATCH] Include property value in the error message When building a list of errors for `ValidationError.message`, include the values of invalid properties too. In order to keep the message reasonably short, the values are truncated at approx 32 characters. --- lib/validations.js | 81 ++++++++++++++++++++++++++++++--------- test/manipulation.test.js | 4 +- test/relations.test.js | 20 ++++------ test/validations.test.js | 71 ++++++++++++++++++++++++++++++++++ 4 files changed, 143 insertions(+), 33 deletions(-) diff --git a/lib/validations.js b/lib/validations.js index ee371457..fafcf5c2 100644 --- a/lib/validations.js +++ b/lib/validations.js @@ -26,7 +26,7 @@ function Validatable() { } /** - * Validate presence of one or more specified properties. + * Validate presence of one or more specified properties. * Requires a model to include a property to be considered valid; fails when validated field is blank. * * For example, validate presence of title @@ -49,7 +49,7 @@ function Validatable() { Validatable.validatesPresenceOf = getConfigurator('presence'); /** - * Validate absence of one or more specified properties. + * Validate absence of one or more specified properties. * A model should not include a property to be considered valid; fails when validated field not blank. * * For example, validate absence of reserved @@ -85,7 +85,7 @@ Validatable.validatesAbsenceOf = getConfigurator('absence'); * User.validatesLengthOf('state', {is: 2, message: {is: 'is not valid state name'}}); * ``` * @param {String} propertyName Property name to validate. - * @options {Object} Options + * @options {Object} Options * @property {Number} is Value that property must equal to validate. * @property {Number} min Value that property must be less than to be valid. * @property {Number} max Value that property must be less than to be valid. @@ -103,7 +103,7 @@ Validatable.validatesLengthOf = getConfigurator('length'); * ``` * * @param {String} propertyName Property name to validate. - * @options {Object} Options + * @options {Object} Options * @property {Boolean} int If true, then property must be an integer to be valid. * @property {Object} message Optional object with string properties for 'int' for integer validation. Default error messages: * - number: is not a number @@ -123,7 +123,7 @@ Validatable.validatesNumericalityOf = getConfigurator('numericality'); * ``` * * @param {String} propertyName Property name to validate. - * @options {Object} Options + * @options {Object} Options * @property {Array} in Array Property must match one of the values in the array to be valid. * @property {String} message Optional error message if property is not valid. Default error message: "is not included in the list". */ @@ -135,7 +135,7 @@ Validatable.validatesInclusionOf = getConfigurator('inclusion'); * Example: `Company.validatesExclusionOf('domain', {in: ['www', 'admin']});` * * @param {String} propertyName Property name to validate. - * @options {Object} Options + * @options {Object} Options * @property {Array} in Array Property must match one of the values in the array to be valid. * @property {String} message Optional error message if property is not valid. Default error message: "is reserved". */ @@ -146,9 +146,9 @@ Validatable.validatesExclusionOf = getConfigurator('exclusion'); * * Require a model to include a property that matches the given format. Example: * `User.validatesFormat('name', {with: /\w+/});` - * + * * @param {String} propertyName Property name to validate. - * @options {Object} Options + * @options {Object} Options * @property {RegExp} with Regular expression to validate format. * @property {String} message Optional error message if property is not valid. Default error message: " is invalid". */ @@ -223,7 +223,7 @@ Validatable.validateAsync = getConfigurator('custom', {async: true}); * ``` * @param {String} propertyName Property name to validate. - * @options {Object} Options + * @options {Object} Options * @property {RegExp} with Regular expression to validate format. * @property {Array.} scopedTo List of properties defining the scope. * @property {String} message Optional error message if property is not valid. Default error message: "is not unique". @@ -437,7 +437,7 @@ Validatable.prototype.isValid = function (callback, data) { asyncFail = false; var attrs = Object.keys(validations || {}); - + attrs.forEach(function(attr) { var attrValidations = validations[attr] || []; attrValidations.forEach(function(v) { @@ -454,7 +454,7 @@ Validatable.prototype.isValid = function (callback, data) { } }); }); - + if (!async) { validationsDone.call(inst, function () { if (valid) cleanErrors(inst); @@ -498,12 +498,12 @@ function cleanErrors(inst) { 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) // that can be specified in conf - if (skipValidation(inst, conf, 'if') + if (skipValidation(inst, conf, 'if') || skipValidation(inst, conf, 'unless')) { if (cb) cb(false); return false; @@ -715,11 +715,11 @@ function ErrorCodes(messages) { * for (var key in changes) { * model[key] = changes[key]; * } - * + * * if (model.isValid()) { * return callback(null, { success: true }); * } - * + * * // This line shows how to create a ValidationError * err = new ValidationError(model); * callback(err); @@ -735,7 +735,7 @@ function ValidationError(obj) { this.message = util.format( 'The %s instance is not valid. Details: %s.', context ? '`' + context + '`' : 'model', - formatErrors(obj.errors) || '(unknown)' + formatErrors(obj.errors, obj.toJSON()) || '(unknown)' ); this.statusCode = 422; @@ -761,7 +761,9 @@ util.inherits(ValidationError, Error); var errorHasStackProperty = !!(new Error).stack; -function formatErrors(errors) { +ValidationError.maxPropertyStringLength = 32; + +function formatErrors(errors, propertyValues) { var DELIM = '; '; errors = errors || {}; return Object.getOwnPropertyNames(errors) @@ -770,9 +772,52 @@ function formatErrors(errors) { }) .map(function(propertyName) { var messages = errors[propertyName]; + var propertyValue = propertyValues[propertyName]; return messages.map(function(msg) { - return '`' + propertyName + '` ' + msg; + return formatPropertyError(propertyName, propertyValue, msg); }).join(DELIM); }) .join(DELIM); } + +function formatPropertyError(propertyName, propertyValue, errorMessage) { + var formattedValue; + var valueType = typeof propertyValue; + if (valueType === 'string') { + formattedValue = JSON.stringify(truncatePropertyString(propertyValue)); + } else if (propertyValue instanceof Date) { + formattedValue = propertyValue.toISOString(); + } else if (valueType === 'object') { + // objects and arrays + formattedValue = util.inspect(propertyValue, { + showHidden: false, + color: false, + // show top-level object properties only + depth: Array.isArray(propertyValue) ? 1 : 0 + }); + formattedValue = truncatePropertyString(formattedValue); + } else { + formattedValue = truncatePropertyString('' + propertyValue); + } + return '`' + propertyName + '` ' + errorMessage + + ' (value: ' + formattedValue + ')'; +} + +function truncatePropertyString(value) { + var len = ValidationError.maxPropertyStringLength; + if (value.length <= len) return value; + + // preserve few last characters like `}` or `]`, but no more than 3 + // this way the last `} ]` in the array of objects is included in the message + var tail; + var m = value.match(/([ \t})\]]+)$/); + if (m) { + tail = m[1].slice(-3); + len -= tail.length; + } else { + tail = value.slice(-3); + len -= 3; + } + + return value.slice(0, len-4) + '...' + tail; +} diff --git a/test/manipulation.test.js b/test/manipulation.test.js index 0f93ae6c..3f3dbed1 100644 --- a/test/manipulation.test.js +++ b/test/manipulation.test.js @@ -66,8 +66,8 @@ describe('manipulation', function () { it('should not allow user-defined value for the id of object - create', function (done) { Person.create({id: 123456}, function (err, p) { err.should.be.instanceof(ValidationError); - err.message.should.equal('The `Person` instance is not valid. Details: `id` can\'t be set.'); err.statusCode.should.equal(422); + err.details.messages.id.should.eql(['can\'t be set']); p.should.be.instanceof(Person); p.id.should.equal(123456); p.isNewRecord().should.be.true; @@ -80,8 +80,8 @@ describe('manipulation', function () { p.isNewRecord().should.be.true; p.save(function(err, inst) { err.should.be.instanceof(ValidationError); - err.message.should.equal('The `Person` instance is not valid. Details: `id` can\'t be set.'); err.statusCode.should.equal(422); + err.details.messages.id.should.eql(['can\'t be set']); inst.id.should.equal(123456); inst.isNewRecord().should.be.true; done(); diff --git a/test/relations.test.js b/test/relations.test.js index e9313acc..f6cd3680 100644 --- a/test/relations.test.js +++ b/test/relations.test.js @@ -2111,9 +2111,7 @@ describe('relations', function () { p.passportItem.create({}, function(err, passport) { should.exist(err); err.name.should.equal('ValidationError'); - var msg = 'The `Passport` instance is not valid.'; - msg += ' Details: `name` can\'t be blank.'; - err.message.should.equal(msg); + err.details.messages.name.should.eql(['can\'t be blank']); done(); }); }); @@ -2125,9 +2123,8 @@ describe('relations', function () { p.save(function(err) { should.exist(err); err.name.should.equal('ValidationError'); - var msg = 'The `Person` instance is not valid.'; - msg += ' Details: `passportItem` is invalid: `name` can\'t be blank.'; - err.message.should.equal(msg); + err.details.messages.passportItem + .should.eql(['is invalid: `name` can\'t be blank']); done(); }); }); @@ -2568,9 +2565,9 @@ describe('relations', function () { addresses.push({ id: 'work', street: '' }); Person.create({ name: 'Wilma', addresses: addresses }, function(err, p) { err.name.should.equal('ValidationError'); - var expected = 'The `Person` instance is not valid. '; - expected += 'Details: `addresses` contains invalid item: `work` (`street` can\'t be blank).'; - err.message.should.equal(expected); + err.details.messages.addresses.should.eql([ + 'contains invalid item: `work` (`street` can\'t be blank)' + ]); done(); }); }); @@ -2769,7 +2766,7 @@ describe('relations', function () { err.name.should.equal('ValidationError'); err.details.codes.street.should.eql(['presence']); var expected = 'The `Address` instance is not valid. '; - expected += 'Details: `street` can\'t be blank.'; + expected += 'Details: `street` can\'t be blank (value: undefined).'; err.message.should.equal(expected); done(); }); @@ -3225,9 +3222,6 @@ describe('relations', function () { should.exist(err); err.name.should.equal('ValidationError'); err.details.codes.jobs.should.eql(['uniqueness']); - var expected = 'The `Category` instance is not valid. '; - expected += 'Details: `jobs` contains duplicate `Job` instance.'; - err.message.should.equal(expected); done(); }); }); diff --git a/test/validations.test.js b/test/validations.test.js index 11833e10..3df0dd48 100644 --- a/test/validations.test.js +++ b/test/validations.test.js @@ -211,6 +211,16 @@ describe('validations', function () { }); }); + it('should include property value in err.message', function(done) { + delete User.validations; + User.validatesPresenceOf('name'); + User.create(function (e, u) { + should.exist(e); + e.message.should.match(/`name` can't be blank \(value: undefined\)/); + done(); + }); + }); + it('should include model name in err.message', function(done) { delete User.validations; User.validatesPresenceOf('name'); @@ -432,4 +442,65 @@ describe('validations', function () { })).should.be.false; }); }); + + describe('invalid value formatting', function() { + var origMaxLen; + beforeEach(function saveAndSetMaxLen() { + origMaxLen = ValidationError.maxPropertyStringLength; + }); + + afterEach(function restoreMaxLen() { + ValidationError.maxPropertyStringLength = origMaxLen; + }); + + it('should truncate long strings', function() { + ValidationError.maxPropertyStringLength = 9; + var err = givenValidationError('prop', '1234567890abc', 'is invalid'); + getErrorDetails(err) + .should.equal('`prop` is invalid (value: "12...abc").'); + }); + + it('should truncate long objects', function() { + ValidationError.maxPropertyStringLength = 12; + var err = givenValidationError('prop', { foo: 'bar' }, 'is invalid'); + getErrorDetails(err) + .should.equal('`prop` is invalid (value: { foo:... }).'); + }); + + it('should truncate long arrays', function() { + ValidationError.maxPropertyStringLength = 12; + var err = givenValidationError('prop', [{ a: 1, b: 2}], 'is invalid'); + getErrorDetails(err) + .should.equal('`prop` is invalid (value: [ { a...} ]).'); + }); + + it('should print only top-level object properties', function() { + var err = givenValidationError('prop', { a: { b: 'c' }}, 'is invalid'); + getErrorDetails(err) + .should.equal('`prop` is invalid (value: { a: [Object] }).'); + }); + + it('should print only top-level props of objects in array', function() { + var err = givenValidationError('prop', [{ a: { b: 'c' }}], 'is invalid'); + getErrorDetails(err) + .should.equal('`prop` is invalid (value: [ { a: [Object] } ]).'); + }); + + function givenValidationError(propertyName, propertyValue, errorMessage) { + var jsonVal = {}; + jsonVal[propertyName] = propertyValue; + var errorVal = {}; + errorVal[propertyName] = [errorMessage]; + + var obj = { + errors: errorVal, + toJSON: function() { return jsonVal; } + }; + return new ValidationError(obj); + } + + function getErrorDetails(err) { + return err.message.replace(/^.*Details: /, ''); + } + }); });