diff --git a/CHANGES.md b/CHANGES.md index 8ff138aa..ab5092e0 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,3 +1,23 @@ +2015-01-14, Version 2.14.0 +========================== + + * Remove console.log (Raymond Feng) + + * Fix for #369 (Dallon Feldner) + + * Fix virtual id get function. (Berkeley Martinez) + + * Fix Model.prototype.inspect (Miroslav Bajtoš) + + * Include property value in the error message (Miroslav Bajtoš) + + * Update datasource.js (Rand McKinney) + + * Change Model to BaseModel for clarity (Fabien Franzen) + + * Don't coerce nested objects into Model instances (Fabien Franzen) + + 2015-01-07, Version 2.13.0 ========================== @@ -368,13 +388,6 @@ * Properly handle LDL for polymorphic relations (Fabien Franzen) - * Check null (Raymond Feng) - - -2014-08-15, Version 2.4.0 -========================= - - 2014-08-15, Version 2.4.1 ========================= @@ -383,6 +396,12 @@ * Check null (Raymond Feng) + +2014-08-15, Version 2.4.0 +========================= + + * Bump version (Raymond Feng) + * Fix the test cases to avoid hard-coded ids (Raymond Feng) * Add strict flag to sortObjectsByIds (Fabien Franzen) @@ -429,16 +448,19 @@ * Cleanup mixin tests (Fabien Franzen) - -2014-08-08, Version 2.3.1 -========================= - * Fix a name conflict in scope metadata (Raymond Feng) 2014-08-08, Version 2.3.0 ========================= + + +2014-08-08, Version 2.3.1 +========================= + + * Fix a name conflict in scope metadata (Raymond Feng) + * Fix the test case so that it works with other DBs (Raymond Feng) * Bump version (Raymond Feng) diff --git a/lib/connectors/memory.js b/lib/connectors/memory.js index 4d98e05e..7c80930c 100644 --- a/lib/connectors/memory.js +++ b/lib/connectors/memory.js @@ -436,7 +436,7 @@ function applyFilter(filter) { return undefined; } - if (typeof example === 'object') { + if (typeof example === 'object' && example !== null) { // ignore geo near filter if (example.near) { return true; diff --git a/lib/dao.js b/lib/dao.js index f800d79c..a4d12867 100644 --- a/lib/dao.js +++ b/lib/dao.js @@ -21,6 +21,7 @@ var setScopeValuesFromWhere = utils.setScopeValuesFromWhere; var mergeQuery = utils.mergeQuery; var util = require('util'); var assert = require('assert'); +var BaseModel = require('./model'); /** * Base class for all persistent objects. @@ -593,6 +594,10 @@ DataAccessObject._coerce = function (where) { continue; } + if (DataType.prototype instanceof BaseModel) { + continue; + } + if (DataType === geo.GeoPoint) { // Skip the GeoPoint as the near operator breaks the assumption that // an operation has only one property diff --git a/lib/datasource.js b/lib/datasource.js index 0781c286..2d125015 100644 --- a/lib/datasource.js +++ b/lib/datasource.js @@ -65,7 +65,7 @@ var slice = Array.prototype.slice; * // work with database * }); * ``` - * @class Define new DataSource + * @class DataSource * @param {String} name Type of dataSource connector (mysql, mongoose, oracle, redis) * @param {Object} settings Database-specific settings to establish connection (settings depend on specific connector). See above. */ diff --git a/lib/model-builder.js b/lib/model-builder.js index 87eed664..8af0a02d 100644 --- a/lib/model-builder.js +++ b/lib/model-builder.js @@ -253,7 +253,7 @@ ModelBuilder.prototype.define = function defineClass(className, properties, sett if (idProp !== 'id') { Object.defineProperty(ModelClass.prototype, 'id', { get: function () { - var idProp = ModelClass.definition.idNames[0]; + var idProp = ModelClass.definition.idNames()[0]; return this.__data[idProp]; }, configurable: true, diff --git a/lib/model.js b/lib/model.js index eee95e16..916e09d5 100644 --- a/lib/model.js +++ b/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 () { - 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) { 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/package.json b/package.json index e15b9492..fecf1d69 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "loopback-datasource-juggler", - "version": "2.13.0", + "version": "2.14.0", "description": "LoopBack DataSoure Juggler", "keywords": [ "StrongLoop", diff --git a/test/datatype.test.js b/test/datatype.test.js index 13f83a20..b4761ffa 100644 --- a/test/datatype.test.js +++ b/test/datatype.test.js @@ -7,13 +7,16 @@ describe('datatypes', function () { before(function (done) { db = getSchema(); + Nested = db.define('Nested', {}); + Model = db.define('Model', { str: String, date: Date, num: Number, bool: Boolean, list: {type: [String]}, - arr: Array + arr: Array, + nested: Nested }); db.automigrate(function () { Model.destroyAll(done); @@ -114,4 +117,10 @@ describe('datatypes', function () { }); } }); + + it('should not coerce nested objects into ModelConstructor types', function() { + var coerced = Model._coerce({ nested: { foo: 'bar' } }); + coerced.nested.constructor.name.should.equal('Object'); + }); + }); 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/memory.test.js b/test/memory.test.js index f3f16fd0..6b1a608e 100644 --- a/test/memory.test.js +++ b/test/memory.test.js @@ -194,7 +194,6 @@ describe('Memory connector', function () { it('should sort undefined values to the end when ordered DESC', function (done) { User.find({order: 'vip ASC, order DESC'}, function (err, posts) { - console.log(posts); should.not.exist(err); posts[4].seq.should.be.eql(1); 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..85c469b0 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'); @@ -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: /, ''); + } + }); });