From 6c7ba30e13776d28f12adc796f408d321cc07bdc Mon Sep 17 00:00:00 2001 From: Aaron Heckmann Date: Wed, 20 Feb 2013 14:11:56 -0800 Subject: [PATCH] added; value at time of validation error Invalid values are now included in the error message as well as included in the ValidatorError constructor. --- lib/document.js | 15 +++- lib/errors/validator.js | 10 ++- lib/schematype.js | 14 ++-- lib/types/embedded.js | 14 +++- test/model.test.js | 32 ++++---- test/schema.test.js | 3 + test/types.document.test.js | 2 +- test/types.documentarray.test.js | 126 +++++++++++++++++++------------ 8 files changed, 137 insertions(+), 79 deletions(-) diff --git a/lib/document.js b/lib/document.js index de2ecb25a65..3f6dd9148a1 100644 --- a/lib/document.js +++ b/lib/document.js @@ -956,8 +956,9 @@ Document.prototype.validate = function (cb) { var p = self.schema.path(path); if (!p) return --total || complete(); - p.doValidate(self.getValue(path), function (err) { - if (err) self.invalidate(path, err, true); + var val = self.getValue(path); + p.doValidate(val, function (err) { + if (err) self.invalidate(path, err, undefined, /* embedded docs */true); --total || complete(); }, self); }); @@ -976,16 +977,22 @@ Document.prototype.validate = function (cb) { * * @param {String} path the field to invalidate * @param {String|Error} err the error which states the reason `path` was invalid + * @param {Object|String|Number|any} value optional invalid value * @api public */ -Document.prototype.invalidate = function (path, err) { +Document.prototype.invalidate = function (path, err, val) { if (!this.$__.validationError) { this.$__.validationError = new ValidationError(this); } if (!err || 'string' === typeof err) { - err = new ValidatorError(path, err); + // sniffing arguments: + // need to handle case where user does not pass value + // so our error message is cleaner + err = 2 < arguments.length + ? new ValidatorError(path, err, val) + : new ValidatorError(path, err) } this.$__.validationError.errors[path] = err; diff --git a/lib/errors/validator.js b/lib/errors/validator.js index 5499bee977a..2498b05ed16 100644 --- a/lib/errors/validator.js +++ b/lib/errors/validator.js @@ -9,19 +9,25 @@ var MongooseError = require('../error'); * * @param {String} path * @param {String} msg + * @param {String|Number|any} val * @inherits MongooseError * @api private */ -function ValidatorError (path, type) { +function ValidatorError (path, type, val) { var msg = type ? '"' + type + '" ' : ''; - MongooseError.call(this, 'Validator ' + msg + 'failed for path ' + path); + + var message = 'Validator ' + msg + 'failed for path ' + path + if (2 < arguments.length) message += ' with value `' + String(val) + '`'; + + MongooseError.call(this, message); Error.captureStackTrace(this, arguments.callee); this.name = 'ValidatorError'; this.path = path; this.type = type; + this.value = val; }; /*! diff --git a/lib/schematype.js b/lib/schematype.js index c02f3121205..0758463c4d9 100644 --- a/lib/schematype.js +++ b/lib/schematype.js @@ -559,12 +559,12 @@ SchemaType.prototype.doValidate = function (value, fn, scope) { if (!count) return fn(null); - function validate (val, msg) { + function validate (ok, msg, val) { if (err) return; - if (val === undefined || val) { + if (ok === undefined || ok) { --count || fn(null); } else { - fn(err = new ValidatorError(path, msg)); + fn(err = new ValidatorError(path, msg, val)); } } @@ -573,14 +573,14 @@ SchemaType.prototype.doValidate = function (value, fn, scope) { , message = v[1]; if (validator instanceof RegExp) { - validate(validator.test(value), message); + validate(validator.test(value), message, value); } else if ('function' === typeof validator) { if (2 === validator.length) { - validator.call(scope, value, function (val) { - validate(val, message); + validator.call(scope, value, function (ok) { + validate(ok, message, value); }); } else { - validate(validator.call(scope, value), message); + validate(validator.call(scope, value), message, value); } } }); diff --git a/lib/types/embedded.js b/lib/types/embedded.js index fc292f93420..a1e4c8b095b 100644 --- a/lib/types/embedded.js +++ b/lib/types/embedded.js @@ -138,12 +138,22 @@ EmbeddedDocument.prototype.inspect = function () { * @api public */ -EmbeddedDocument.prototype.invalidate = function (path, err, first) { +EmbeddedDocument.prototype.invalidate = function (path, err, val, first) { if (!this.__parent) return false; + var index = this.__parentArray.indexOf(this); var parentPath = this.__parentArray._path; var fullPath = [parentPath, index, path].join('.'); - this.__parent.invalidate(fullPath, err); + + // sniffing arguments: + // need to check if user passed a value to keep + // our error message clean. + if (2 < arguments.length) { + this.__parent.invalidate(fullPath, err, val); + } else { + this.__parent.invalidate(fullPath, err); + } + if (first) this.$__.validationError = ownerDocument(this).$__.validationError; return true; diff --git a/test/model.test.js b/test/model.test.js index 64917a89689..7f4445dc0ef 100644 --- a/test/model.test.js +++ b/test/model.test.js @@ -924,8 +924,8 @@ describe('model', function(){ assert.ok(err instanceof MongooseError); assert.ok(err instanceof ValidationError); assert.ok(err.errors.simple instanceof ValidatorError); - assert.equal(err.errors.simple.message,'Validator "must be abc" failed for path simple'); - assert.equal(post.errors.simple.message,'Validator "must be abc" failed for path simple'); + assert.equal(err.errors.simple.message,'Validator "must be abc" failed for path simple with value ``'); + assert.equal(post.errors.simple.message,'Validator "must be abc" failed for path simple with value ``'); post.set('simple', 'abc'); post.save(function(err){ @@ -948,7 +948,7 @@ describe('model', function(){ var doc = new IntrospectionValidation({name: 'hi'}); doc.save( function (err) { db.close(); - assert.equal(err.errors.name.message,"Validator \"Name cannot be greater than 1 character\" failed for path name"); + assert.equal(err.errors.name.message, "Validator \"Name cannot be greater than 1 character\" failed for path name with value `hi`"); assert.equal(err.name,"ValidationError"); assert.equal(err.message,"Validation failed"); done(); @@ -1006,17 +1006,17 @@ describe('model', function(){ assert.ok(err.errors.password instanceof ValidatorError); assert.ok(err.errors.email instanceof ValidatorError); assert.ok(err.errors.username instanceof ValidatorError); - assert.equal(err.errors.password.message,'Validator failed for path password'); - assert.equal(err.errors.email.message,'Validator failed for path email'); - assert.equal(err.errors.username.message,'Validator failed for path username'); + assert.equal(err.errors.password.message,'Validator failed for path password with value `short`'); + assert.equal(err.errors.email.message,'Validator failed for path email with value `too`'); + assert.equal(err.errors.username.message,'Validator failed for path username with value `nope`'); assert.equal(Object.keys(post.errors).length, 3); assert.ok(post.errors.password instanceof ValidatorError); assert.ok(post.errors.email instanceof ValidatorError); assert.ok(post.errors.username instanceof ValidatorError); - assert.equal(post.errors.password.message,'Validator failed for path password'); - assert.equal(post.errors.email.message,'Validator failed for path email'); - assert.equal(post.errors.username.message,'Validator failed for path username'); + assert.equal(post.errors.password.message,'Validator failed for path password with value `short`'); + assert.equal(post.errors.email.message,'Validator failed for path email with value `too`'); + assert.equal(post.errors.username.message,'Validator failed for path username with value `nope`'); done(); }); }); @@ -1132,9 +1132,9 @@ describe('model', function(){ assert.ok(err instanceof MongooseError); assert.ok(err instanceof ValidationError); assert.ok(err.errors['items.0.subs.0.required'] instanceof ValidatorError); - assert.equal(err.errors['items.0.subs.0.required'].message,'Validator "required" failed for path required'); + assert.equal(err.errors['items.0.subs.0.required'].message,'Validator "required" failed for path required with value ``'); assert.ok(post.errors['items.0.subs.0.required'] instanceof ValidatorError); - assert.equal(post.errors['items.0.subs.0.required'].message,'Validator "required" failed for path required'); + assert.equal(post.errors['items.0.subs.0.required'].message,'Validator "required" failed for path required with value ``'); assert.ok(!err.errors['items.0.required']); assert.ok(!err.errors['items.0.required']); @@ -1148,7 +1148,7 @@ describe('model', function(){ assert.ok(err); assert.ok(err.errors); assert.ok(err.errors['items.0.required'] instanceof ValidatorError); - assert.equal(err.errors['items.0.required'].message,'Validator "required" failed for path required'); + assert.equal(err.errors['items.0.required'].message,'Validator "required" failed for path required with value ``'); assert.ok(!err.errors['items.0.subs.0.required']); assert.ok(!err.errors['items.0.subs.0.required']); @@ -1190,7 +1190,7 @@ describe('model', function(){ assert.ok(err instanceof MongooseError); assert.ok(err instanceof ValidationError); assert.ok(err.errors.async instanceof ValidatorError); - assert.equal(err.errors.async.message,'Validator "async validator" failed for path async'); + assert.equal(err.errors.async.message,'Validator "async validator" failed for path async with value `test`'); assert.equal(true, executed); executed = false; @@ -3498,9 +3498,9 @@ describe('model', function(){ query.exec(function (err, found) { db.close(); assert.ifError(err); - assert.equal(found.length,2); - assert.equal(found[0]._id.id,createdOne._id.id); - assert.equal(found[1]._id.id,createdTwo._id.id); + assert.equal(found.length, 2); + assert.equal(found[0]._id.id, createdOne._id.id); + assert.equal(found[1]._id.id, createdTwo._id.id); done(); }); }); diff --git a/test/schema.test.js b/test/schema.test.js index 0e497c2189a..ded421faa61 100644 --- a/test/schema.test.js +++ b/test/schema.test.js @@ -350,6 +350,9 @@ describe('schema', function(){ Tobi.path('friends').doValidate(100, function(err){ assert.ok(err instanceof ValidatorError); + assert.equal('friends', err.path); + assert.equal('max', err.type); + assert.equal(100, err.value); }); Tobi.path('friends').doValidate(1, function(err){ diff --git a/test/types.document.test.js b/test/types.document.test.js index d6d334f0f6b..f99cf30a0c0 100644 --- a/test/types.document.test.js +++ b/test/types.document.test.js @@ -75,7 +75,7 @@ describe('types.document', function(){ a.save(function(err){ assert.ok(a.__parent.$__.validationError instanceof ValidationError); assert.equal(a.__parent.errors['jsconf.ar.0.work'].name, 'ValidatorError'); - assert.equal(a.__parent.$__.validationError.toString(), 'ValidationError: Validator "required" failed for path test, Validator failed for path work'); + assert.equal(a.__parent.$__.validationError.toString(), 'ValidationError: Validator "required" failed for path test with value ``, Validator failed for path work with value `nope`'); done(); }); }); diff --git a/test/types.documentarray.test.js b/test/types.documentarray.test.js index 9ed1247f801..f1406efbbb1 100644 --- a/test/types.documentarray.test.js +++ b/test/types.documentarray.test.js @@ -179,63 +179,61 @@ describe('types.documentarray', function(){ }) }) - describe('EmbeddedDocumentArray', function(){ - describe('create()', function(){ - it('works', function(done){ - var a = new MongooseDocumentArray([]); - assert.equal('function', typeof a.create); - - var schema = new Schema({ docs: [new Schema({ name: 'string' })] }); - var T = mongoose.model('embeddedDocument#create_test', schema, 'asdfasdfa'+ random()); - var t = new T; - assert.equal('function', typeof t.docs.create); - var subdoc = t.docs.create({ name: 100 }); - assert.ok(subdoc._id); - assert.equal(subdoc.name, '100'); - assert.ok(subdoc instanceof EmbeddedDocument); - done(); - }) + describe('create()', function(){ + it('works', function(done){ + var a = new MongooseDocumentArray([]); + assert.equal('function', typeof a.create); + + var schema = new Schema({ docs: [new Schema({ name: 'string' })] }); + var T = mongoose.model('embeddedDocument#create_test', schema, 'asdfasdfa'+ random()); + var t = new T; + assert.equal('function', typeof t.docs.create); + var subdoc = t.docs.create({ name: 100 }); + assert.ok(subdoc._id); + assert.equal(subdoc.name, '100'); + assert.ok(subdoc instanceof EmbeddedDocument); + done(); }) + }) - describe('push()', function(){ - it('does not re-cast instances of its embedded doc xxxxxx', function(done){ - var db = start(); + describe('push()', function(){ + it('does not re-cast instances of its embedded doc', function(done){ + var db = start(); - var child = new Schema({ name: String, date: Date }); - child.pre('save', function (next) { - this.date = new Date; - next(); - }); - var schema = Schema({ children: [child] }); - var M = db.model('embeddedDocArray-push-re-cast', schema, 'edarecast-'+random()); - var m = new M; - m.save(function (err) { + var child = new Schema({ name: String, date: Date }); + child.pre('save', function (next) { + this.date = new Date; + next(); + }); + var schema = Schema({ children: [child] }); + var M = db.model('embeddedDocArray-push-re-cast', schema, 'edarecast-'+random()); + var m = new M; + m.save(function (err) { + assert.ifError(err); + M.findById(m._id, function (err, doc) { assert.ifError(err); - M.findById(m._id, function (err, doc) { + var c = doc.children.create({ name: 'first' }) + assert.equal(undefined, c.date); + doc.children.push(c); + assert.equal(undefined, c.date); + doc.save(function (err) { assert.ifError(err); - var c = doc.children.create({ name: 'first' }) - assert.equal(undefined, c.date); + assert.ok(doc.children[doc.children.length-1].date); + assert.equal(c.date, doc.children[doc.children.length-1].date); + doc.children.push(c); - assert.equal(undefined, c.date); + doc.children.push(c); + doc.save(function (err) { assert.ifError(err); - assert.ok(doc.children[doc.children.length-1].date); - assert.equal(c.date, doc.children[doc.children.length-1].date); - - doc.children.push(c); - doc.children.push(c); - - doc.save(function (err) { + M.findById(m._id, function (err, doc) { + db.close() assert.ifError(err); - M.findById(m._id, function (err, doc) { - db.close() - assert.ifError(err); - assert.equal(3, doc.children.length); - doc.children.forEach(function (child) { - assert.equal(doc.children[0].id, child.id); - }) - done(); + assert.equal(3, doc.children.length); + doc.children.forEach(function (child) { + assert.equal(doc.children[0].id, child.id); }) + done(); }) }) }) @@ -289,4 +287,38 @@ describe('types.documentarray', function(){ }) }); + describe('invalidate()', function(){ + it('works', function(done){ + var schema = Schema({ docs: [{ name: 'string' }] }); + var T = mongoose.model('embeddedDocument#invalidate_test', schema, 'asdfasdfa'+ random()); + var t = new T; + t.docs.push({ name: 100 }); + var subdoc = t.docs[t.docs.length-1]; + subdoc.invalidate('name', 'boo boo', '%'); + + t.validate(function (err) { + var e = t.errors['docs.0.name']; + assert.ok(e); + assert.equal(e.path, 'docs.0.name'); + assert.equal(e.type, 'boo boo'); + assert.equal(e.value, '%'); + done(); + }) + }) + + it('handles validation failures', function(done){ + var db = start(); + var nested = Schema({ v: { type: Number, max: 30 }}); + var schema = Schema({ + docs: [nested] + }, { collection: 'embedded-invalidate-'+random() }); + var M = db.model('embedded-invalidate', schema); + var m = new M({ docs: [{ v: 900 }] }); + m.save(function (err) { + db.close(); + assert.equal(900, err.errors['docs.0.v'].value); + done(); + }); + }) + }) })