Merge pull request #389 from strongloop/feature/include-value-in-validation-error-message

Include property value in the error message

lib/model.js

@@ -53,14 +53,14 @@ function ModelBaseClass(data, options) {
 ModelBaseClass.prototype._initProperties = function (data, options) {
   var self = this;
   var ctor = this.constructor;
-  
+
   if(data instanceof ctor) {
     // Convert the data to be plain object to avoid pollutions
     data = data.toObject(false);
   }
   var properties = _extend({}, ctor.definition.properties);
   data = data || {};
-  
+
   if (typeof ctor.applyProperties === 'function') {
     ctor.applyProperties(data);
   }
@@ -72,7 +72,7 @@ ModelBaseClass.prototype._initProperties = function (data, options) {
   if(strict === undefined) {
     strict = ctor.definition.settings.strict;
   }
-  
+
   if (ctor.hideInternalProperties) {
     // Object.defineProperty() is expensive. We only try to make the internal
     // properties hidden (non-enumerable) if the model class has the
@@ -122,7 +122,7 @@ ModelBaseClass.prototype._initProperties = function (data, options) {
     this.__strict = strict;
     this.__persisted = false;
   }
-  
+
   if (options.persisted !== undefined) {
     this.__persisted = options.persisted === true;
   }
@@ -132,13 +132,13 @@ ModelBaseClass.prototype._initProperties = function (data, options) {
   }
 
   var keys = Object.keys(data);
-  
+
   if (Array.isArray(options.fields)) {
     keys = keys.filter(function(k) {
       return (options.fields.indexOf(k) != -1);
     });
   }
-  
+
   var size = keys.length;
   var p, propVal;
   for (var k = 0; k < size; k++) {
@@ -156,7 +156,7 @@ ModelBaseClass.prototype._initProperties = function (data, options) {
       }
     } else if (ctor.relations[p]) {
       var relationType = ctor.relations[p].type;
-      
+
       if (!properties[p]) {
         var modelTo = ctor.relations[p].modelTo || ModelBaseClass;
         var multiple = ctor.relations[p].multiple;
@@ -165,7 +165,7 @@ ModelBaseClass.prototype._initProperties = function (data, options) {
         properties[p] = { name: typeName, type: propType };
         this.setStrict(false);
       }
-      
+
       // Relation
       if (relationType === 'belongsTo' && propVal != null) {
         // If the related model is populated
@@ -182,15 +182,15 @@ ModelBaseClass.prototype._initProperties = function (data, options) {
       }
     }
   }
-  
+
   keys = Object.keys(properties);
-  
+
   if (Array.isArray(options.fields)) {
     keys = keys.filter(function(k) {
       return (options.fields.indexOf(k) != -1);
     });
   }
-  
+
   size = keys.length;
 
   for (k = 0; k < size; k++) {
@@ -216,11 +216,11 @@ ModelBaseClass.prototype._initProperties = function (data, options) {
         self.__data[p] = def;
       }
     }
-    
+
     // Handle complex types (JSON/Object)
     var type = properties[p].type;
     if (!BASE_TYPES[type.name]) {
-      
+
       if (typeof self.__data[p] !== 'object' && self.__data[p]) {
         try {
           self.__data[p] = JSON.parse(self.__data[p] + '');
@@ -228,7 +228,7 @@ ModelBaseClass.prototype._initProperties = function (data, options) {
           self.__data[p] = String(self.__data[p]);
         }
       }
-      
+
       if (type.prototype instanceof ModelBaseClass) {
         if (!(self.__data[p] instanceof type)
           && typeof self.__data[p] === 'object'
@@ -438,8 +438,28 @@ ModelBaseClass.prototype.reset = function () {
   }
 };
 
-ModelBaseClass.prototype.inspect = function () {
+// Node v0.11+ allows custom inspect functions to return an object
-  return util.inspect(this.__data, false, 4, true);
+// instead of string. That way options like `showHidden` and `colors`
+// can be preserved.
+var versionParts = process.versions.node
+  .split(/\./g).map(function(v) { return +v; });
+
+var INSPECT_SUPPORTS_OBJECT_RETVAL =
+ versionParts[0] > 0 ||
+ versionParts[1] > 11 ||
+ (versionParts[0] === 11 && versionParts[1] >= 14);
+
+ModelBaseClass.prototype.inspect = function (depth) {
+  if (INSPECT_SUPPORTS_OBJECT_RETVAL)
+     return this.__data;
+
+  // Workaround for older versions
+  // See also https://github.com/joyent/node/commit/66280de133
+  return util.inspect(this.__data, {
+    showHidden: false,
+    depth: depth,
+    colors: false
+  });
 };
 
 ModelBaseClass.mixin = function (anotherClass, options) {

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.<String>} 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;
+}

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();

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.';
+        err.details.messages.name.should.eql(['can\'t be blank']);
-        msg += ' Details: `name` can\'t be blank.';
-        err.message.should.equal(msg);
         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.';
+          err.details.messages.passportItem
-          msg += ' Details: `passportItem` is invalid: `name` can\'t be blank.';
+            .should.eql(['is invalid: `name` can\'t be blank']);
-          err.message.should.equal(msg);
           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. ';
+        err.details.messages.addresses.should.eql([
-        expected += 'Details: `addresses` contains invalid item: `work` (`street` can\'t be blank).';
+          'contains invalid item: `work` (`street` can\'t be blank)'
-        err.message.should.equal(expected);
+        ]);
         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();
         });
       });

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');
@@ -220,7 +230,7 @@ describe('validations', function () {
           done();
         });
       });
-      
+
       it('should return validation metadata', function() {
         var expected = {name:[{validation: 'presence', options: {}}]};
         delete User.validations;
@@ -235,11 +245,11 @@ describe('validations', function () {
 
     it('should validate presence', function () {
       User.validatesPresenceOf('name', 'email');
-      
+
       var validations = User.validations;
       validations.name.should.eql([{validation: 'presence', options: {}}]);
       validations.email.should.eql([{validation: 'presence', options: {}}]);
-      
+
       var u = new User;
       u.isValid().should.not.be.true;
       u.name = 1;
@@ -263,7 +273,7 @@ describe('validations', function () {
     });
 
   });
-  
+
   describe('absence', function () {
 
     it('should validate absence', function () {
@@ -345,7 +355,7 @@ describe('validations', function () {
         done(err);
       });
     });
-    
+
     it('should skip blank values', function (done) {
       User.validatesUniquenessOf('email');
       var u = new User({email: '  '});
@@ -360,9 +370,9 @@ describe('validations', function () {
         });
       })).should.be.false;
     });
-    
+
     it('should work with if/unless', function (done) {
-      User.validatesUniquenessOf('email', { 
+      User.validatesUniquenessOf('email', {
         if: function() { return true; },
         unless: function() { return false; }
       });
@@ -404,7 +414,7 @@ describe('validations', function () {
       Boolean(u.isValid()).should.be.false;
       u.errors.codes.should.eql({ email: ['invalid-email'] });
     });
-    
+
     it('should validate and return detailed error messages', function() {
       User.validate('global', function (err) {
         if (this.email === 'hello' || this.email === 'hey') {
@@ -417,11 +427,11 @@ describe('validations', function () {
       u.errors.should.eql({ email: ['Cannot be `hello`'] });
       u.errors.codes.should.eql({ email: ['invalid-email'] });
     });
-    
+
     it('should validate using custom async validation', function(done) {
       User.validateAsync('email', function (err, next) {
         process.nextTick(next);
-      }, { 
+      }, {
         if: function() { return true; },
         unless: function() { return false; }
       });
@@ -432,4 +442,73 @@ 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] } ]).');
+    });
+
+    it('should exclude colors from Model values', function() {
+      var obj = new User();
+      obj.email = 'test@example.com';
+      var err = givenValidationError('user', obj, 'is invalid');
+      getErrorDetails(err).should.equal(
+        '`user` is invalid (value: { email: \'test@example.com\' }).');
+    });
+
+    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: /, '');
+    }
+  });
 });