Skip to content

Include property value in the error message #389

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
raymondfeng merged 2 commits into from
Jan 14, 2015
+197 −59
Merged

Include property value in the error message #389

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9b759c9
Include property value in the error message
Jan 12, 2015
e4fc387
Fix Model.prototype.inspect
Jan 12, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 36 additions & 16 deletions lib/model.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand All @@ -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
Expand Down Expand Up @@ -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;
}
Expand All @@ -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++) {
Expand All @@ -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;
Expand All @@ -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
Expand All @@ -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++) {
Expand All @@ -216,19 +216,19 @@ 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] + '');
} catch (e) {
self.__data[p] = String(self.__data[p]);
}
}

if (type.prototype instanceof ModelBaseClass) {
if (!(self.__data[p] instanceof type)
&& typeof self.__data[p] === 'object'
Expand Down Expand Up @@ -438,8 +438,28 @@ ModelBaseClass.prototype.reset = function () {
}
};

ModelBaseClass.prototype.inspect = function () {
return util.inspect(this.__data, false, 4, true);
// Node v0.11+ allows custom inspect functions to return an object
// 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) {
Expand Down
81 changes: 63 additions & 18 deletions lib/validations.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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.
Expand All @@ -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
Expand All @@ -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".
*/
Expand All @@ -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".
*/
Expand All @@ -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".
*/
Expand Down Expand Up @@ -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".
Expand Down Expand Up @@ -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) {
Expand All @@ -454,7 +454,7 @@ Validatable.prototype.isValid = function (callback, data) {
}
});
});

if (!async) {
validationsDone.call(inst, function () {
if (valid) cleanErrors(inst);
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand All @@ -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;
Expand All @@ -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)
Expand All @@ -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;
}
4 changes: 2 additions & 2 deletions test/manipulation.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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();
Expand Down
Loading