From 8b50983231ed386f65852a1a87fcc21fa7ec2bba Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Sat, 18 Mar 2017 21:06:50 -0600 Subject: [PATCH 01/30] fix(update): more complete handling for overwrite option Re: #3556 --- lib/query.js | 10 +++++++++- lib/services/query/hasDollarKeys.js | 12 ++++++++++++ test/model.update.test.js | 21 +++++++++++++++++++++ 3 files changed, 42 insertions(+), 1 deletion(-) create mode 100644 lib/services/query/hasDollarKeys.js diff --git a/lib/query.js b/lib/query.js index 2578c42bb63..684e822eaa8 100644 --- a/lib/query.js +++ b/lib/query.js @@ -7,6 +7,7 @@ var QueryCursor = require('./querycursor'); var QueryStream = require('./querystream'); var cast = require('./cast'); var castUpdate = require('./services/query/castUpdate'); +var hasDollarKeys = require('./services/query/hasDollarKeys'); var helpers = require('./queryhelpers'); var mquery = require('mquery'); var readPref = require('./drivers').ReadPreference; @@ -2177,7 +2178,14 @@ Query.prototype._execUpdate = function(callback) { if (this.options.runValidators) { _this = this; - doValidate = updateValidators(this, schema, castedDoc, options); + if (this.options.overwrite && !hasDollarKeys(castedDoc)) { + castedDoc = new _this.model(castedDoc, null, true); + doValidate = function(callback) { + castedDoc.validate(callback); + }; + } else { + doValidate = updateValidators(this, schema, castedDoc, options); + } var _callback = function(err) { if (err) { return callback(err); diff --git a/lib/services/query/hasDollarKeys.js b/lib/services/query/hasDollarKeys.js new file mode 100644 index 00000000000..2917a847ba6 --- /dev/null +++ b/lib/services/query/hasDollarKeys.js @@ -0,0 +1,12 @@ +'use strict'; + +module.exports = function(obj) { + var keys = Object.keys(obj); + var len = keys.length; + for (var i = 0; i < len; ++i) { + if (keys[i].charAt(0) === '$') { + return true; + } + } + return false; +}; diff --git a/test/model.update.test.js b/test/model.update.test.js index d9297e269c8..55a054ac17b 100644 --- a/test/model.update.test.js +++ b/test/model.update.test.js @@ -2447,6 +2447,27 @@ describe('model: update:', function() { }); }); + it('overwrite doc with update validators (gh-3556)', function(done) { + var testSchema = new Schema({ + name: { + type: String, + required: true + }, + otherName: String + }); + var Test = db.model('gh3556', testSchema); + + var opts = { overwrite: true, runValidators: true }; + Test.update({}, { otherName: 'test' }, opts, function(error) { + assert.ok(error); + assert.ok(error.errors['name']); + Test.update({}, { $set: { otherName: 'test' } }, opts, function(error) { + assert.ifError(error); + done(); + }); + }); + }); + it('single embedded schema under document array (gh-4519)', function(done) { var PermissionSchema = new mongoose.Schema({ read: { type: Boolean, required: true }, From 8a7c67d06f52ef45bfc96ded77f9900209ba5f37 Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Sat, 18 Mar 2017 21:11:02 -0600 Subject: [PATCH 02/30] fix(query): handle overwrite findOneAndUpdate validation --- lib/query.js | 47 +++++++++++++++++------------ test/model.findOneAndUpdate.test.js | 21 +++++++++++++ 2 files changed, 48 insertions(+), 20 deletions(-) diff --git a/lib/query.js b/lib/query.js index 684e822eaa8..15c8540ab84 100644 --- a/lib/query.js +++ b/lib/query.js @@ -1998,29 +1998,36 @@ Query.prototype._findAndModify = function(type, callback) { opts.remove = false; } - castedDoc = castDoc(this, opts.overwrite); - castedDoc = setDefaultsOnInsert(this, schema, castedDoc, opts); - if (!castedDoc) { - if (opts.upsert) { - // still need to do the upsert to empty doc - var doc = utils.clone(castedQuery); - delete doc._id; - castedDoc = {$set: doc}; - } else { - return this.findOne(callback); - } - } else if (castedDoc instanceof Error) { - return callback(castedDoc); + if (opts.overwrite && !hasDollarKeys(this._update)) { + castedDoc = new model(this._update, null, true); + doValidate = function(callback) { + castedDoc.validate(callback); + }; } else { - // In order to make MongoDB 2.6 happy (see - // https://jira.mongodb.org/browse/SERVER-12266 and related issues) - // if we have an actual update document but $set is empty, junk the $set. - if (castedDoc.$set && Object.keys(castedDoc.$set).length === 0) { - delete castedDoc.$set; + castedDoc = castDoc(this, opts.overwrite); + castedDoc = setDefaultsOnInsert(this, schema, castedDoc, opts); + if (!castedDoc) { + if (opts.upsert) { + // still need to do the upsert to empty doc + var doc = utils.clone(castedQuery); + delete doc._id; + castedDoc = {$set: doc}; + } else { + return this.findOne(callback); + } + } else if (castedDoc instanceof Error) { + return callback(castedDoc); + } else { + // In order to make MongoDB 2.6 happy (see + // https://jira.mongodb.org/browse/SERVER-12266 and related issues) + // if we have an actual update document but $set is empty, junk the $set. + if (castedDoc.$set && Object.keys(castedDoc.$set).length === 0) { + delete castedDoc.$set; + } } - } - doValidate = updateValidators(this, schema, castedDoc, opts); + doValidate = updateValidators(this, schema, castedDoc, opts); + } } this._applyPaths(); diff --git a/test/model.findOneAndUpdate.test.js b/test/model.findOneAndUpdate.test.js index 294f9b3c6c6..96a0f5ffa11 100644 --- a/test/model.findOneAndUpdate.test.js +++ b/test/model.findOneAndUpdate.test.js @@ -1809,6 +1809,27 @@ describe('model: findByIdAndUpdate:', function() { }); }); + it('overwrite doc with update validators (gh-3556)', function(done) { + var testSchema = new Schema({ + name: { + type: String, + required: true + }, + otherName: String + }); + var Test = db.model('gh3556', testSchema); + + var opts = { overwrite: true, runValidators: true }; + Test.findOneAndUpdate({}, { otherName: 'test' }, opts, function(error) { + assert.ok(error); + assert.ok(error.errors['name']); + Test.findOneAndUpdate({}, { $set: { otherName: 'test' } }, opts, function(error) { + assert.ifError(error); + done(); + }); + }); + }); + it('properly handles casting nested objects in update (gh-4724)', function(done) { var locationSchema = new Schema({ _id: false, From 90d9da7344fcc3109a74177bbdf09831efc57fa9 Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Sun, 19 Mar 2017 11:13:50 -0600 Subject: [PATCH 03/30] chore: now working on 4.10.0 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index ade7f7f65fc..c9f426ef322 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "mongoose", "description": "Mongoose MongoDB ODM", - "version": "4.9.1-pre", + "version": "4.10.0-pre", "author": "Guillermo Rauch ", "keywords": [ "mongodb", From 608599ed25021b03ad0fc0cc327932d00d67b46e Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Thu, 23 Mar 2017 16:09:31 -0600 Subject: [PATCH 04/30] refactor(model): make sharding into a plugin instead of core Fix #5105 --- .eslintignore | 1 + lib/document.js | 39 ------------------ lib/index.js | 3 +- lib/model.js | 15 +------ lib/plugins/sharding.js | 71 ++++++++++++++++++++++++++++++++ lib/schema.js | 7 ++++ package.json | 1 + test/connection.test.js | 58 -------------------------- test/document.unit.test.js | 6 ++- test/model.discriminator.test.js | 2 +- test/shard.test.js | 31 ++++---------- tools/sharded.js | 42 +++++++++++++++++++ 12 files changed, 137 insertions(+), 139 deletions(-) create mode 100644 lib/plugins/sharding.js create mode 100644 tools/sharded.js diff --git a/.eslintignore b/.eslintignore index b1109fb5b0a..cda1980998b 100644 --- a/.eslintignore +++ b/.eslintignore @@ -2,3 +2,4 @@ docs/ bin/ test/triage/ test/model.discriminator.test.js +tools/ diff --git a/lib/document.js b/lib/document.js index 2118cca83d0..83d232d7c7e 100644 --- a/lib/document.js +++ b/lib/document.js @@ -328,7 +328,6 @@ Document.prototype.init = function(doc, opts, fn) { } init(this, doc, this._doc); - this.$__storeShard(); this.emit('init', this); this.constructor.emit('init', this); @@ -414,44 +413,6 @@ function init(self, obj, doc, prefix) { } } -/** - * Stores the current values of the shard keys. - * - * ####Note: - * - * _Shard key values do not / are not allowed to change._ - * - * @api private - * @method $__storeShard - * @memberOf Document - */ - -Document.prototype.$__storeShard = function() { - // backwards compat - var key = this.schema.options.shardKey || this.schema.options.shardkey; - if (!(key && utils.getFunctionName(key.constructor) === 'Object')) { - return; - } - - var orig = this.$__.shardval = {}, - paths = Object.keys(key), - len = paths.length, - val; - - for (var i = 0; i < len; ++i) { - val = this.getValue(paths[i]); - if (isMongooseObject(val)) { - orig[paths[i]] = val.toObject({depopulate: true, _isNested: true}); - } else if (val !== null && val !== undefined && val.valueOf && - // Explicitly don't take value of dates - (!val.constructor || utils.getFunctionName(val.constructor) !== 'Date')) { - orig[paths[i]] = val.valueOf(); - } else { - orig[paths[i]] = val; - } - } -}; - /*! * Set up middleware support */ diff --git a/lib/index.js b/lib/index.js index e0c644557e6..fa2316a40a4 100644 --- a/lib/index.js +++ b/lib/index.js @@ -20,6 +20,7 @@ var querystring = require('querystring'); var Aggregate = require('./aggregate'); var PromiseProvider = require('./promise_provider'); +var shardingPlugin = require('./plugins/sharding'); /** * Mongoose constructor. @@ -32,7 +33,7 @@ var PromiseProvider = require('./promise_provider'); function Mongoose() { this.connections = []; - this.plugins = []; + this.plugins = [[shardingPlugin, {}]]; this.models = {}; this.modelSchemas = {}; // default global options diff --git a/lib/model.js b/lib/model.js index 6ea5c2e75b0..7d931c45503 100644 --- a/lib/model.js +++ b/lib/model.js @@ -224,7 +224,6 @@ Model.prototype.$__save = function(options, callback) { } _this.$__reset(); - _this.$__storeShard(); var numAffected = 0; if (result) { @@ -674,7 +673,7 @@ Model.prototype.increment = function increment() { }; /** - * Returns a query object which applies shardkeys if they exist. + * Returns a query object * * @api private * @method $__where @@ -684,22 +683,10 @@ Model.prototype.increment = function increment() { Model.prototype.$__where = function _where(where) { where || (where = {}); - var paths, - len; - if (!where._id) { where._id = this._doc._id; } - if (this.$__.shardval) { - paths = Object.keys(this.$__.shardval); - len = paths.length; - - for (var i = 0; i < len; ++i) { - where[paths[i]] = this.$__.shardval[paths[i]]; - } - } - if (this._doc._id == null) { return new Error('No _id found on document!'); } diff --git a/lib/plugins/sharding.js b/lib/plugins/sharding.js new file mode 100644 index 00000000000..4a2f85ea1fe --- /dev/null +++ b/lib/plugins/sharding.js @@ -0,0 +1,71 @@ +'use strict'; + +var utils = require('../utils'); + +module.exports = function shardingPlugin(schema) { + schema.post('init', function() { + storeShard.call(this); + return this; + }); + schema.pre('save', function(next) { + applyWhere.call(this); + next(); + }); + schema.post('save', function() { + storeShard.call(this); + }); +}; + +function applyWhere() { + var paths; + var len; + + if (this.$__.shardval) { + paths = Object.keys(this.$__.shardval); + len = paths.length; + + this.$where = this.$where || {}; + for (var i = 0; i < len; ++i) { + this.$where[paths[i]] = this.$__.shardval[paths[i]]; + } + } +} + +/*! + * Stores the current values of the shard keys. + * + * ####Note: + * + * _Shard key values do not / are not allowed to change._ + * + * @api private + * @method $__storeShard + * @memberOf Document + */ +module.exports.storeShard = storeShard; + +function storeShard() { + // backwards compat + var key = this.schema.options.shardKey || this.schema.options.shardkey; + if (!(key && utils.getFunctionName(key.constructor) === 'Object')) { + return; + } + + var orig = this.$__.shardval = {}, + paths = Object.keys(key), + len = paths.length, + val; + + for (var i = 0; i < len; ++i) { + val = this.getValue(paths[i]); + if (utils.isMongooseObject(val)) { + orig[paths[i]] = val.toObject({depopulate: true, _isNested: true}); + } else if (val !== null && val !== undefined && val.valueOf && + // Explicitly don't take value of dates + (!val.constructor || utils.getFunctionName(val.constructor) !== 'Date')) { + orig[paths[i]] = val.valueOf(); + } else { + orig[paths[i]] = val; + } + } +} diff --git a/lib/schema.js b/lib/schema.js index 27b97a91bf7..179c05efb94 100644 --- a/lib/schema.js +++ b/lib/schema.js @@ -86,6 +86,7 @@ function Schema(obj, options) { this.tree = {}; this.query = {}; this.childSchemas = []; + this.plugins = []; this.s = { hooks: new Kareem(), @@ -1179,6 +1180,12 @@ Schema.prototype.post = function(method, fn) { */ Schema.prototype.plugin = function(fn, opts) { + if (opts && + opts.deduplicate && + this.plugins.find(function(p) { return p.fn === fn; })) { + return this; + } + this.plugins.push({ fn: fn, opts: opts }); fn(this, opts); return this; }; diff --git a/package.json b/package.json index 64480c7d5d2..53d743a3ad6 100644 --- a/package.json +++ b/package.json @@ -48,6 +48,7 @@ "marked": "0.3.6", "mocha": "3.2.0", "mongoose-long": "0.1.1", + "mongodb-topology-manager": "1.0.11", "node-static": "0.7.7", "power-assert": "1.4.1", "q": "1.4.1", diff --git a/test/connection.test.js b/test/connection.test.js index 2ddd6361da5..425b4fcb6e8 100644 --- a/test/connection.test.js +++ b/test/connection.test.js @@ -141,11 +141,6 @@ describe('connections:', function() { var mongod = 'mongodb://localhost:27017'; - var repl1 = process.env.MONGOOSE_SET_TEST_URI; - var repl2 = repl1.replace('mongodb://', '').split(','); - repl2.push(repl2.shift()); - repl2 = 'mongodb://' + repl2.join(','); - describe('with different host/port', function() { it('non-replica set', function(done) { var db = mongoose.createConnection(); @@ -195,59 +190,6 @@ describe('connections:', function() { }); }); }); - - it('replica set', function(done) { - var db = mongoose.createConnection(); - - db.openSet(repl1, function(err) { - if (err) { - return done(err); - } - - var hosts = db.hosts.slice(); - var db1 = db.db; - - db.close(function(err) { - if (err) { - return done(err); - } - - db.openSet(repl2, function(err) { - if (err) { - return done(err); - } - - db.hosts.forEach(function(host, i) { - assert.notEqual(host.port, hosts[i].port); - }); - assert.ok(db1 !== db.db); - - hosts = db.hosts.slice(); - var db2 = db.db; - - db.close(function(err) { - if (err) { - return done(err); - } - - db.openSet(repl1, function(err) { - if (err) { - return done(err); - } - - db.hosts.forEach(function(host, i) { - assert.notEqual(host.port, hosts[i].port); - }); - assert.ok(db2 !== db.db); - - db.close(); - done(); - }); - }); - }); - }); - }); - }); }); }); diff --git a/test/document.unit.test.js b/test/document.unit.test.js index 6f491d3a201..3f9748c0a6f 100644 --- a/test/document.unit.test.js +++ b/test/document.unit.test.js @@ -2,8 +2,10 @@ * Module dependencies. */ -var start = require('./common'); var assert = require('power-assert'); +var start = require('./common'); +var storeShard = require('../lib/plugins/sharding').storeShard; + var mongoose = start.mongoose; describe('sharding', function() { @@ -22,7 +24,7 @@ describe('sharding', function() { var currentTime = new Date(); d._doc = {date: currentTime}; - d.$__storeShard(); + storeShard.call(d); assert.equal(d.$__.shardval.date, currentTime); done(); }); diff --git a/test/model.discriminator.test.js b/test/model.discriminator.test.js index 26bae1ab58b..6c102041ad3 100644 --- a/test/model.discriminator.test.js +++ b/test/model.discriminator.test.js @@ -315,7 +315,7 @@ describe('model', function() { }); it('merges callQueue with base queue defined before discriminator types callQueue', function(done) { - assert.equal(Employee.schema.callQueue.length, 5); + assert.equal(Employee.schema.callQueue.length, 8); // PersonSchema.post('save') assert.strictEqual(Employee.schema.callQueue[0], Person.schema.callQueue[0]); diff --git a/test/shard.test.js b/test/shard.test.js index 2dee21ecd19..ee69b986b46 100644 --- a/test/shard.test.js +++ b/test/shard.test.js @@ -14,11 +14,6 @@ if (!uri) { 'Sharding must already be enabled on your database', '\033[39m' ); - - // let expresso shut down this test - exports.r = function expressoHack() { - }; - return; } var schema = new Schema({ @@ -62,29 +57,17 @@ describe('shard', function() { cmd.shardcollection = db.name + '.' + collection; cmd.key = P.schema.options.shardkey; - P.db.db.executeDbAdminCommand(cmd, function(err, res) { + P.db.db.executeDbAdminCommand(cmd, function(err) { assert.ifError(err); - if (!(res && res.documents && res.documents[0] && res.documents[0].ok)) { - err = new Error('could not shard test collection ' - + collection + '\n' - + res.documents[0].errmsg + '\n' - + 'Make sure to use a different database than what ' - + 'is used for the MULTI_MONGOS_TEST'); - return done(err); - } - - db.db.admin(function(err, admin) { + db.db.admin().serverStatus(function(err, info) { + db.close(); assert.ifError(err); - admin.serverStatus(function(err, info) { - db.close(); - assert.ifError(err); - version = info.version.split('.').map(function(n) { - return parseInt(n, 10); - }); - greaterThan20x = version[0] > 2 || version[0] === 2 && version[0] > 0; - done(); + version = info.version.split('.').map(function(n) { + return parseInt(n, 10); }); + greaterThan20x = version[0] > 2 || version[0] === 2 && version[0] > 0; + done(); }); }); }); diff --git a/tools/sharded.js b/tools/sharded.js new file mode 100644 index 00000000000..92ec2f1a899 --- /dev/null +++ b/tools/sharded.js @@ -0,0 +1,42 @@ +'use strict'; + +const co = require('co'); + +co(function*() { + var Sharded = require('mongodb-topology-manager').Sharded; + + // Create new instance + var topology = new Sharded({ + mongod: 'mongod', + mongos: 'mongos' + }); + + yield topology.addShard([{ + options: { + bind_ip: 'localhost', port: 31000, dbpath: `${__dirname}/31000` + } + }], { replSet: 'rs1' }); + + yield topology.addConfigurationServers([{ + options: { + bind_ip: 'localhost', port: 35000, dbpath: `${__dirname}/35000` + } + }], { replSet: 'rs0' }); + + yield topology.addProxies([{ + bind_ip: 'localhost', port: 51000, configdb: 'localhost:35000' + }], { + binary: 'mongos' + }); + + // Start up topology + yield topology.start(); + + // Shard db + yield topology.enableSharding('test'); + + console.log('done'); +}).catch(error => { + console.error(error); + process.exit(-1); +}); From 3d62d3558c15ec852bdeaab1a5138b1853b4f7cb Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Sun, 26 Mar 2017 17:30:03 -0600 Subject: [PATCH 05/30] feat(model): pass params to pre hooks Fix #5064 --- lib/model.js | 5 ++++- package.json | 2 +- test/model.test.js | 12 ++++++++++-- 3 files changed, 15 insertions(+), 4 deletions(-) diff --git a/lib/model.js b/lib/model.js index c5b071bb7b2..598717b835a 100644 --- a/lib/model.js +++ b/lib/model.js @@ -3609,7 +3609,10 @@ Model.compile = function compile(name, schema, collectionName, connection, base) model.Query.base = Query.base; applyQueryMethods(model, schema.query); - var kareemOptions = { useErrorHandlers: true }; + var kareemOptions = { + useErrorHandlers: true, + numCallbackParams: 1 + }; model.$__insertMany = model.hooks.createWrapper('insertMany', model.insertMany, model, kareemOptions); model.insertMany = function(arr, options, callback) { diff --git a/package.json b/package.json index 64480c7d5d2..04e5065af27 100644 --- a/package.json +++ b/package.json @@ -22,7 +22,7 @@ "async": "2.1.4", "bson": "~1.0.4", "hooks-fixed": "2.0.0", - "kareem": "1.2.1", + "kareem": "1.3.0", "mongodb": "2.2.25", "mpath": "0.2.1", "mpromise": "0.5.5", diff --git a/test/model.test.js b/test/model.test.js index 667eefd74b5..d9573c1802f 100644 --- a/test/model.test.js +++ b/test/model.test.js @@ -5345,7 +5345,15 @@ describe('Model', function() { }); var calledPre = 0; var calledPost = 0; - schema.pre('insertMany', function(next) { + schema.pre('insertMany', function(next, docs) { + assert.equal(docs.length, 2); + assert.equal(docs[0].name, 'Star Wars'); + ++calledPre; + next(); + }); + schema.pre('insertMany', function(next, docs) { + assert.equal(docs.length, 2); + assert.equal(docs[0].name, 'Star Wars'); ++calledPre; next(); }); @@ -5358,7 +5366,7 @@ describe('Model', function() { Movie.insertMany(arr, function(error, docs) { assert.ifError(error); assert.equal(docs.length, 2); - assert.equal(calledPre, 1); + assert.equal(calledPre, 2); assert.equal(calledPost, 1); done(); }); From 53a9d826e24961e8f4798fff430182bb4041a48a Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Sun, 26 Mar 2017 17:34:53 -0600 Subject: [PATCH 06/30] test: dont run shard tests if uri not set --- test/shard.test.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/shard.test.js b/test/shard.test.js index ee69b986b46..9d11710668c 100644 --- a/test/shard.test.js +++ b/test/shard.test.js @@ -14,6 +14,8 @@ if (!uri) { 'Sharding must already be enabled on your database', '\033[39m' ); + + return; } var schema = new Schema({ From 68a94d004f498c10af41ac7a64f6fc3f58110361 Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Mon, 3 Apr 2017 14:16:30 -0600 Subject: [PATCH 07/30] feat(validation): include failed paths in error message and inspect output Fix #3064 --- lib/error/validation.js | 48 +++++++++++++++++++++++++++------- test/document.test.js | 4 +-- test/model.populate.test.js | 3 ++- test/model.test.js | 2 +- test/schema.validation.test.js | 10 +++---- test/types.buffer.test.js | 6 ++--- 6 files changed, 51 insertions(+), 22 deletions(-) diff --git a/lib/error/validation.js b/lib/error/validation.js index e3322d4942d..c70a5f61901 100644 --- a/lib/error/validation.js +++ b/lib/error/validation.js @@ -14,10 +14,13 @@ var MongooseError = require('../error.js'); function ValidationError(instance) { this.errors = {}; + this._message = ''; if (instance && instance.constructor.name === 'model') { - MongooseError.call(this, instance.constructor.modelName + ' validation failed'); + this._message = instance.constructor.modelName + ' validation failed'; + MongooseError.call(this, this._message); } else { - MongooseError.call(this, 'Validation failed'); + this._message = 'Validation failed'; + MongooseError.call(this, this._message); } if (Error.captureStackTrace) { Error.captureStackTrace(this); @@ -37,24 +40,49 @@ function ValidationError(instance) { ValidationError.prototype = Object.create(MongooseError.prototype); ValidationError.prototype.constructor = MongooseError; +Object.defineProperty(ValidationError.prototype, 'message', { + get: function() { + return this._message + ': ' + _generateMessage(this); + }, + enumerable: true +}); /** * Console.log helper */ ValidationError.prototype.toString = function() { - var ret = this.name + ': '; + return this.name + ': ' + _generateMessage(this); +}; + +/*! + * inspect helper + */ + +ValidationError.prototype.inspect = function() { + return Object.assign(new Error(this.message), this); +}; + +/*! + * ignore + */ + +function _generateMessage(err) { + var keys = Object.keys(err.errors || {}); + var len = keys.length; var msgs = []; + var key; - Object.keys(this.errors || {}).forEach(function(key) { - if (this === this.errors[key]) { - return; + for (var i = 0; i < len; ++i) { + key = keys[i]; + if (err === err.errors[key]) { + continue; } - msgs.push(String(this.errors[key])); - }, this); + msgs.push(key + ': ' + err.errors[key].message); + } - return ret + msgs.join(', '); -}; + return msgs.join(', '); +} /*! * Module exports diff --git a/test/document.test.js b/test/document.test.js index 1793bd7ce0a..053bce18c89 100644 --- a/test/document.test.js +++ b/test/document.test.js @@ -1272,7 +1272,7 @@ describe('document', function() { var m = new M({name: 'gh1109-2', arr: [1]}); assert.equal(called, false); m.save(function(err) { - assert.equal(String(err), 'ValidationError: BAM'); + assert.equal(String(err), 'ValidationError: arr: BAM'); assert.equal(called, true); m.arr.push(2); called = false; @@ -1301,7 +1301,7 @@ describe('document', function() { assert.equal(err.errors.arr.message, 'Path `arr` is required.'); m.arr.push({nice: true}); m.save(function(err) { - assert.equal(String(err), 'ValidationError: BAM'); + assert.equal(String(err), 'ValidationError: arr: BAM'); m.arr.push(95); m.save(function(err) { assert.ifError(err); diff --git a/test/model.populate.test.js b/test/model.populate.test.js index cb052b6d6a5..429b80fb5bc 100644 --- a/test/model.populate.test.js +++ b/test/model.populate.test.js @@ -2023,7 +2023,8 @@ describe('model: populate:', function() { }); comment.save(function(err) { - assert.equal('CommentWithRequiredField validation failed', err && err.message); + assert.ok(err); + assert.ok(err.message.indexOf('CommentWithRequiredField validation failed') === 0, err.message); assert.ok('num' in err.errors); assert.ok('str' in err.errors); assert.ok('user' in err.errors); diff --git a/test/model.test.js b/test/model.test.js index 667eefd74b5..b6dd6dc7052 100644 --- a/test/model.test.js +++ b/test/model.test.js @@ -1139,7 +1139,7 @@ describe('Model', function() { db.close(); assert.equal(err.errors.name.message, 'Name cannot be greater than 1 character for path "name" with value `hi`'); assert.equal(err.name, 'ValidationError'); - assert.equal(err.message, 'IntrospectionValidation validation failed'); + assert.ok(err.message.indexOf('IntrospectionValidation validation failed') !== -1, err.message); done(); }); }); diff --git a/test/schema.validation.test.js b/test/schema.validation.test.js index b48cef24850..74dda6b3a5c 100644 --- a/test/schema.validation.test.js +++ b/test/schema.validation.test.js @@ -1031,7 +1031,7 @@ describe('schema', function() { bad.foods = 'waffles'; bad.validate(function(error) { assert.ok(error); - var errorMessage = 'CastError: Cast to Object failed for value ' + + var errorMessage = 'foods: Cast to Object failed for value ' + '"waffles" at path "foods"'; assert.ok(error.toString().indexOf(errorMessage) !== -1, error.toString()); done(); @@ -1045,7 +1045,7 @@ describe('schema', function() { var bad = new Breakfast({}); bad.validate(function(error) { assert.ok(error); - var errorMessage = 'ValidationError: Path `description` is required.'; + var errorMessage = 'ValidationError: description: Path `description` is required.'; assert.equal(errorMessage, error.toString()); done(); }); @@ -1059,7 +1059,7 @@ describe('schema', function() { var error = bad.validateSync(); assert.ok(error); - var errorMessage = 'ValidationError: Path `description` is required.'; + var errorMessage = 'ValidationError: description: Path `description` is required.'; assert.equal(errorMessage, error.toString()); done(); }); @@ -1088,7 +1088,7 @@ describe('schema', function() { var bad = new Breakfast({}); bad.validate(function(error) { assert.ok(error); - var errorMessage = 'ValidationError: Path `description` is required.'; + var errorMessage = 'ValidationError: description: Path `description` is required.'; assert.equal(errorMessage, error.toString()); done(); }); @@ -1106,7 +1106,7 @@ describe('schema', function() { var Breakfast = mongoose.model('gh2832', breakfast, 'gh2832'); Breakfast.create({description: undefined}, function(error) { assert.ok(error); - var errorMessage = 'ValidationError: CastError: Cast to String failed for value "undefined" at path "description"'; + var errorMessage = 'ValidationError: description: Cast to String failed for value "undefined" at path "description"'; assert.equal(errorMessage, error.toString()); assert.ok(error.errors.description); assert.equal(error.errors.description.reason.toString(), 'Error: oops'); diff --git a/test/types.buffer.test.js b/test/types.buffer.test.js index d633d78795f..20944461077 100644 --- a/test/types.buffer.test.js +++ b/test/types.buffer.test.js @@ -69,7 +69,7 @@ describe('types.buffer', function() { }); t.validate(function(err) { - assert.equal(err.message, 'UserBuffer validation failed'); + assert.ok(err.message.indexOf('UserBuffer validation failed') === 0, err.message); assert.equal(err.errors.required.kind, 'required'); t.required = {x: [20]}; t.save(function(err) { @@ -83,11 +83,11 @@ describe('types.buffer', function() { t.sub.push({name: 'Friday Friday'}); t.save(function(err) { - assert.equal(err.message, 'UserBuffer validation failed'); + assert.ok(err.message.indexOf('UserBuffer validation failed') === 0, err.message); assert.equal(err.errors['sub.0.buf'].kind, 'required'); t.sub[0].buf = new Buffer('well well'); t.save(function(err) { - assert.equal(err.message, 'UserBuffer validation failed'); + assert.ok(err.message.indexOf('UserBuffer validation failed') === 0, err.message); assert.equal(err.errors['sub.0.buf'].kind, 'user defined'); assert.equal(err.errors['sub.0.buf'].message, 'valid failed'); From 7d78a7576ad0ff438fdf8af82089748b5274c64a Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Sun, 9 Apr 2017 00:14:49 -0600 Subject: [PATCH 08/30] feat(query): add PoC for runSetters option Re: #4569 --- lib/schema.js | 4 ++++ lib/schema/string.js | 3 +++ test/model.query.casting.test.js | 20 ++++++++++++++++++++ 3 files changed, 27 insertions(+) diff --git a/lib/schema.js b/lib/schema.js index a5e72d3b18d..bd0feb6d4a9 100644 --- a/lib/schema.js +++ b/lib/schema.js @@ -689,6 +689,10 @@ Schema.interpretAsType = function(path, obj, options) { 'You can only nest using refs or arrays.'); } + obj = utils.clone(obj); + if (!('runSetters' in obj)) { + obj.runSetters = options.runSetters; + } return new MongooseTypes[name](path, obj); }; diff --git a/lib/schema/string.js b/lib/schema/string.js index d969280a9c6..8c5d95fd233 100644 --- a/lib/schema/string.js +++ b/lib/schema/string.js @@ -506,6 +506,9 @@ SchemaString.prototype.castForQuery = function($conditional, val) { if (Object.prototype.toString.call(val) === '[object RegExp]') { return val; } + if (this.options && this.options.runSetters) { + return this.applySetters(val, null); + } return this.cast(val); }; diff --git a/test/model.query.casting.test.js b/test/model.query.casting.test.js index 4a47bf88514..6f3a195ba07 100644 --- a/test/model.query.casting.test.js +++ b/test/model.query.casting.test.js @@ -1021,6 +1021,26 @@ describe('model query casting', function() { catch(done); }); + it('lowercase in query (gh-4569)', function(done) { + var db = start(); + + var testSchema = new Schema({ + name: { type: String, lowercase: true } + }, { runSetters: true }); + + var Test = db.model('gh-4569', testSchema); + Test.create({ name: 'val' }). + then(function() { + return Test.findOne({ name: 'VAL' }); + }). + then(function(doc) { + assert.ok(doc); + assert.equal(doc.name, 'val'); + done(); + }). + catch(done); + }); + it('_id = 0 (gh-4610)', function(done) { var db = start(); From 173d72efba76b5881eb604f0cbfc4ff3cf849231 Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Sun, 9 Apr 2017 19:30:13 -0600 Subject: [PATCH 09/30] feat(query): add more fleshed out runSettersOnQuery re: #4569 --- lib/schema.js | 6 ++-- lib/schema/boolean.js | 4 +-- lib/schema/buffer.js | 2 +- lib/schema/date.js | 2 +- lib/schema/decimal128.js | 20 -------------- lib/schema/embedded.js | 4 +++ lib/schema/number.js | 2 +- lib/schema/objectid.js | 2 +- lib/schema/string.js | 6 ++-- lib/schematype.js | 47 ++++++++++++++++++++++++-------- test/model.query.casting.test.js | 16 ++++++++--- 11 files changed, 63 insertions(+), 48 deletions(-) diff --git a/lib/schema.js b/lib/schema.js index bd0feb6d4a9..cd376b9a705 100644 --- a/lib/schema.js +++ b/lib/schema.js @@ -689,9 +689,9 @@ Schema.interpretAsType = function(path, obj, options) { 'You can only nest using refs or arrays.'); } - obj = utils.clone(obj); - if (!('runSetters' in obj)) { - obj.runSetters = options.runSetters; + obj = utils.clone(obj, { retainKeyOrder: true }); + if (!('runSettersOnQuery' in obj)) { + obj.runSettersOnQuery = options.runSettersOnQuery; } return new MongooseTypes[name](path, obj); }; diff --git a/lib/schema/boolean.js b/lib/schema/boolean.js index 206d9fd98f5..5951dd0513d 100644 --- a/lib/schema/boolean.js +++ b/lib/schema/boolean.js @@ -90,10 +90,10 @@ SchemaBoolean.prototype.castForQuery = function($conditional, val) { return handler.call(this, val); } - return this.cast(val); + return this._castForQuery(val); } - return this.cast($conditional); + return this._castForQuery($conditional); }; /*! diff --git a/lib/schema/buffer.js b/lib/schema/buffer.js index 56490dea878..f7ca7588a36 100644 --- a/lib/schema/buffer.js +++ b/lib/schema/buffer.js @@ -178,7 +178,7 @@ SchemaBuffer.prototype.castForQuery = function($conditional, val) { return handler.call(this, val); } val = $conditional; - var casted = this.cast(val); + var casted = this._castForQuery(val); return casted ? casted.toObject({ transform: false, virtuals: false }) : casted; }; diff --git a/lib/schema/date.js b/lib/schema/date.js index b7bcf227b33..9f97096b5ba 100644 --- a/lib/schema/date.js +++ b/lib/schema/date.js @@ -277,7 +277,7 @@ SchemaDate.prototype.castForQuery = function($conditional, val) { var handler; if (arguments.length !== 2) { - return this.cast($conditional); + return this._castForQuery($conditional); } handler = this.$conditionalHandlers[$conditional]; diff --git a/lib/schema/decimal128.js b/lib/schema/decimal128.js index 3400e36ef63..67e35df4830 100644 --- a/lib/schema/decimal128.js +++ b/lib/schema/decimal128.js @@ -139,26 +139,6 @@ Decimal128.prototype.$conditionalHandlers = $lte: handleSingle }); -/** - * Casts contents for queries. - * - * @param {String} $conditional - * @param {any} [val] - * @api private - */ - -Decimal128.prototype.castForQuery = function($conditional, val) { - var handler; - if (arguments.length === 2) { - handler = this.$conditionalHandlers[$conditional]; - if (!handler) { - throw new Error('Can\'t use ' + $conditional + ' with ObjectId.'); - } - return handler.call(this, val); - } - return this.cast($conditional); -}; - /*! * Module exports. */ diff --git a/lib/schema/embedded.js b/lib/schema/embedded.js index d1e293fdceb..9938563adc8 100644 --- a/lib/schema/embedded.js +++ b/lib/schema/embedded.js @@ -156,6 +156,10 @@ Embedded.prototype.castForQuery = function($conditional, val) { return val; } + if (this.options.runSetters) { + val = this._applySetters(val); + } + return new this.caster(val); }; diff --git a/lib/schema/number.js b/lib/schema/number.js index a91f6dda6df..e5b9bc7fa62 100644 --- a/lib/schema/number.js +++ b/lib/schema/number.js @@ -279,7 +279,7 @@ SchemaNumber.prototype.castForQuery = function($conditional, val) { } return handler.call(this, val); } - val = this.cast($conditional); + val = this._castForQuery($conditional); return val; }; diff --git a/lib/schema/objectid.js b/lib/schema/objectid.js index 28858236893..6f684191c90 100644 --- a/lib/schema/objectid.js +++ b/lib/schema/objectid.js @@ -184,7 +184,7 @@ ObjectId.prototype.castForQuery = function($conditional, val) { } return handler.call(this, val); } - return this.cast($conditional); + return this._castForQuery($conditional); }; /*! diff --git a/lib/schema/string.js b/lib/schema/string.js index 8c5d95fd233..dfd044ad2f8 100644 --- a/lib/schema/string.js +++ b/lib/schema/string.js @@ -506,10 +506,8 @@ SchemaString.prototype.castForQuery = function($conditional, val) { if (Object.prototype.toString.call(val) === '[object RegExp]') { return val; } - if (this.options && this.options.runSetters) { - return this.applySetters(val, null); - } - return this.cast(val); + + return this._castForQuery(val); }; /*! diff --git a/lib/schematype.js b/lib/schematype.js index 83495f6ef09..16078815796 100644 --- a/lib/schematype.js +++ b/lib/schematype.js @@ -622,20 +622,17 @@ SchemaType.prototype.getDefault = function(scope, init) { return ret; }; -/** - * Applies setters +/*! + * Applies setters without casting * - * @param {Object} value - * @param {Object} scope - * @param {Boolean} init * @api private */ -SchemaType.prototype.applySetters = function(value, scope, init, priorVal, options) { - var v = value, - setters = this.setters, - len = setters.length, - caster = this.caster; +SchemaType.prototype._applySetters = function(value, scope, init, priorVal) { + var v = value; + var setters = this.setters; + var len = setters.length; + var caster = this.caster; while (len--) { v = setters[len].call(scope, v, this); @@ -649,7 +646,22 @@ SchemaType.prototype.applySetters = function(value, scope, init, priorVal, optio v = newVal; } - if (v === null || v === undefined) { + return v; +}; + +/** + * Applies setters + * + * @param {Object} value + * @param {Object} scope + * @param {Boolean} init + * @api private + */ + +SchemaType.prototype.applySetters = function(value, scope, init, priorVal, options) { + var v = this._applySetters(value, scope, init, priorVal, options); + + if (v == null) { return v; } @@ -971,6 +983,19 @@ SchemaType.prototype.castForQuery = function($conditional, val) { return handler.call(this, val); } val = $conditional; + return this._castForQuery(val); +}; + +/*! + * Internal switch for runSetters + * + * @api private + */ + +SchemaType.prototype._castForQuery = function(val) { + if (this.options && this.options.runSettersOnQuery) { + return this.applySetters(val, null); + } return this.cast(val); }; diff --git a/test/model.query.casting.test.js b/test/model.query.casting.test.js index 6f3a195ba07..98e1d4fbdd1 100644 --- a/test/model.query.casting.test.js +++ b/test/model.query.casting.test.js @@ -1025,19 +1025,27 @@ describe('model query casting', function() { var db = start(); var testSchema = new Schema({ - name: { type: String, lowercase: true } - }, { runSetters: true }); + name: { type: String, lowercase: true }, + num: { type: Number, set: function(v) { return Math.floor(v); } } + }, { runSettersOnQuery: true }); var Test = db.model('gh-4569', testSchema); - Test.create({ name: 'val' }). + Test.create({ name: 'val', num: 3 }). then(function() { return Test.findOne({ name: 'VAL' }); }). then(function(doc) { assert.ok(doc); assert.equal(doc.name, 'val'); - done(); }). + then(function() { + return Test.findOne({ num: 3.14 }); + }). + then(function(doc) { + assert.ok(doc); + assert.equal(doc.name, 'val'); + }). + then(function() { done(); }). catch(done); }); From 0c0822279a4c2187e28bff14c271bb6237f3acc1 Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Mon, 10 Apr 2017 19:02:31 -0600 Subject: [PATCH 10/30] feat(aggregate): add mongoose-specific agg cursor Fix #5145 --- lib/aggregate.js | 8 ++++++-- test/aggregate.test.js | 13 +++++++++++++ 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/lib/aggregate.js b/lib/aggregate.js index 623fbaa8737..10c2210730e 100644 --- a/lib/aggregate.js +++ b/lib/aggregate.js @@ -2,10 +2,11 @@ * Module dependencies */ -var util = require('util'); -var utils = require('./utils'); +var AggregationCursor = require('./cursor/AggregationCursor'); var PromiseProvider = require('./promise_provider'); var Query = require('./query'); +var util = require('util'); +var utils = require('./utils'); var read = Query.prototype.read; /** @@ -631,6 +632,9 @@ Aggregate.prototype.exec = function(callback) { callback && callback(null, cursor); }); }); + } else if (options.cursor.useMongooseAggCursor) { + delete options.cursor.useMongooseAggCursor; + return new AggregationCursor(this); } var cursor = this._model.collection. aggregate(this._pipeline, this.options || {}); diff --git a/test/aggregate.test.js b/test/aggregate.test.js index 8bcadf83aa6..2350cadd017 100644 --- a/test/aggregate.test.js +++ b/test/aggregate.test.js @@ -898,6 +898,19 @@ describe('aggregate: ', function() { }); }); + it('cursor() with useMongooseAggCursor (gh-5145)', function(done) { + var db = start(); + + var MyModel = db.model('gh5145', { name: String }); + + var cursor = MyModel. + aggregate([{ $match: { name: 'test' } }]). + cursor({ useMongooseAggCursor: true }). + exec(); + assert.ok(cursor instanceof require('stream').Readable); + done(); + }); + it('cursor() eachAsync (gh-4300)', function(done) { var db = start(); From 894a5dd400643a4bf38042f5b1cf71c2b9156a6f Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Mon, 10 Apr 2017 19:05:20 -0600 Subject: [PATCH 11/30] chore: add missing files --- lib/cursor/AggregationCursor.js | 282 ++++++++++++++++++++++++++++++++ 1 file changed, 282 insertions(+) create mode 100644 lib/cursor/AggregationCursor.js diff --git a/lib/cursor/AggregationCursor.js b/lib/cursor/AggregationCursor.js new file mode 100644 index 00000000000..ce762b982b7 --- /dev/null +++ b/lib/cursor/AggregationCursor.js @@ -0,0 +1,282 @@ +/*! + * Module dependencies. + */ + +var PromiseProvider = require('../promise_provider'); +var Readable = require('stream').Readable; +var util = require('util'); + +/** + * An AggregationCursor is a concurrency primitive for processing aggregation + * results one document at a time. It is analogous to QueryCursor. + * + * An AggregationCursor fulfills the [Node.js streams3 API](https://strongloop.com/strongblog/whats-new-io-js-beta-streams3/), + * in addition to several other mechanisms for loading documents from MongoDB + * one at a time. + * + * Unless you're an advanced user, do **not** instantiate this class directly. + * Use [`Aggregate#cursor()`](/docs/api.html#aggregate_Aggregate-cursor) instead. + * + * @param {Aggregate} agg + * @param {Object} options + * @inherits Readable + * @event `cursor`: Emitted when the cursor is created + * @event `error`: Emitted when an error occurred + * @event `data`: Emitted when the stream is flowing and the next doc is ready + * @event `end`: Emitted when the stream is exhausted + * @api public + */ + +function AggregationCursor(agg) { + Readable.call(this, { objectMode: true }); + + this.cursor = null; + this.agg = agg; + this._transforms = []; + var _this = this; + var model = agg._model; + model.collection.aggregate(agg._pipeline, agg.options || {}, function(err, cursor) { + if (_this._error) { + cursor.close(function() {}); + _this.listeners('error').length > 0 && _this.emit('error', _this._error); + } + if (err) { + return _this.emit('error', err); + } + _this.cursor = cursor; + _this.emit('cursor', cursor); + }); +} + +util.inherits(AggregationCursor, Readable); + +/*! + * Necessary to satisfy the Readable API + */ + +AggregationCursor.prototype._read = function() { + var _this = this; + _next(this, function(error, doc) { + if (error) { + return _this.emit('error', error); + } + if (!doc) { + _this.push(null); + _this.cursor.close(function(error) { + if (error) { + return _this.emit('error', error); + } + setTimeout(function() { + _this.emit('close'); + }, 0); + }); + return; + } + _this.push(doc); + }); +}; + +/** + * Registers a transform function which subsequently maps documents retrieved + * via the streams interface or `.next()` + * + * ####Example + * + * // Map documents returned by `data` events + * Thing. + * find({ name: /^hello/ }). + * cursor(). + * map(function (doc) { + * doc.foo = "bar"; + * return doc; + * }) + * on('data', function(doc) { console.log(doc.foo); }); + * + * // Or map documents returned by `.next()` + * var cursor = Thing.find({ name: /^hello/ }). + * cursor(). + * map(function (doc) { + * doc.foo = "bar"; + * return doc; + * }); + * cursor.next(function(error, doc) { + * console.log(doc.foo); + * }); + * + * @param {Function} fn + * @return {QueryCursor} + * @api public + * @method map + */ + +AggregationCursor.prototype.map = function(fn) { + this._transforms.push(fn); + return this; +}; + +/*! + * Marks this cursor as errored + */ + +AggregationCursor.prototype._markError = function(error) { + this._error = error; + return this; +}; + +/** + * Marks this cursor as closed. Will stop streaming and subsequent calls to + * `next()` will error. + * + * @param {Function} callback + * @return {Promise} + * @api public + * @method close + * @emits close + * @see MongoDB driver cursor#close http://mongodb.github.io/node-mongodb-native/2.1/api/Cursor.html#close + */ + +AggregationCursor.prototype.close = function(callback) { + var Promise = PromiseProvider.get(); + var _this = this; + return new Promise.ES6(function(resolve, reject) { + _this.cursor.close(function(error) { + if (error) { + callback && callback(error); + reject(error); + return _this.listeners('error').length > 0 && + _this.emit('error', error); + } + _this.emit('close'); + resolve(); + callback && callback(); + }); + }); +}; + +/** + * Get the next document from this cursor. Will return `null` when there are + * no documents left. + * + * @param {Function} callback + * @return {Promise} + * @api public + * @method next + */ + +AggregationCursor.prototype.next = function(callback) { + var Promise = PromiseProvider.get(); + var _this = this; + return new Promise.ES6(function(resolve, reject) { + _next(_this, function(error, doc) { + if (error) { + callback && callback(error); + return reject(error); + } + callback && callback(null, doc); + resolve(doc); + }); + }); +}; + +/** + * Execute `fn` for every document in the cursor. If `fn` returns a promise, + * will wait for the promise to resolve before iterating on to the next one. + * Returns a promise that resolves when done. + * + * @param {Function} fn + * @param {Function} [callback] executed when all docs have been processed + * @return {Promise} + * @api public + * @method eachAsync + */ + +AggregationCursor.prototype.eachAsync = function(fn, callback) { + var Promise = PromiseProvider.get(); + var _this = this; + + var handleNextResult = function(doc, callback) { + var promise = fn(doc); + if (promise && typeof promise.then === 'function') { + promise.then( + function() { callback(null); }, + function(error) { callback(error); }); + } else { + callback(null); + } + }; + + var iterate = function(callback) { + return _next(_this, function(error, doc) { + if (error) { + return callback(error); + } + if (!doc) { + return callback(null); + } + handleNextResult(doc, function(error) { + if (error) { + return callback(error); + } + // Make sure to clear the stack re: gh-4697 + setTimeout(function() { + iterate(callback); + }, 0); + }); + }); + }; + + return new Promise.ES6(function(resolve, reject) { + iterate(function(error) { + if (error) { + callback && callback(error); + return reject(error); + } + callback && callback(null); + return resolve(); + }); + }); +}; + +/*! + * Get the next doc from the underlying cursor and mongooseify it + * (populate, etc.) + */ + +function _next(ctx, cb) { + var callback = cb; + if (ctx._transforms.length) { + callback = function(err, doc) { + if (err || doc === null) { + return cb(err, doc); + } + cb(err, ctx._transforms.reduce(function(doc, fn) { + return fn(doc); + }, doc)); + }; + } + + if (ctx._error) { + return process.nextTick(function() { + callback(ctx._error); + }); + } + + if (ctx.cursor) { + return ctx.cursor.next(function(error, doc) { + if (error) { + return callback(error); + } + if (!doc) { + return callback(null, null); + } + + callback(null, doc); + }); + } else { + ctx.once('cursor', function() { + _next(ctx, cb); + }); + } +} + +module.exports = AggregationCursor; From c25f258226cccb36031d50479d23fbc450501454 Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Thu, 13 Apr 2017 18:11:21 -0600 Subject: [PATCH 12/30] test(query): repro #4136 --- test/model.findOneAndUpdate.test.js | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/test/model.findOneAndUpdate.test.js b/test/model.findOneAndUpdate.test.js index 467c7262635..51aeb1bd861 100644 --- a/test/model.findOneAndUpdate.test.js +++ b/test/model.findOneAndUpdate.test.js @@ -1781,6 +1781,17 @@ describe('model: findOneAndUpdate:', function() { }); }); + it('strictQuery option (gh-4136)', function(done) { + var modelSchema = new Schema({ field: Number }, { strictQuery: 'throw' }); + + var Model = db.model('gh4136', modelSchema); + Model.find({ nonexistingField: 1 }).exec(function(error) { + assert.ok(error); + assert.ok(error.message.indexOf('strictQuery') !== -1, error.message); + done(); + }); + }); + it('strict option (gh-5108)', function(done) { var modelSchema = new Schema({ field: Number }, { strict: 'throw' }); From 0a1ea284d880acc478ea2ea23123f56edce7791d Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Thu, 13 Apr 2017 18:11:23 -0600 Subject: [PATCH 13/30] fix(query): add strictQuery option that throws when not querying on field not in schema Fix #4136 --- lib/cast.js | 3 +++ lib/query.js | 4 +++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/lib/cast.js b/lib/cast.js index 406c7d1c1d5..4ff8449dd54 100644 --- a/lib/cast.js +++ b/lib/cast.js @@ -179,6 +179,9 @@ module.exports = function cast(schema, obj, options) { } throw new StrictModeError(path, 'Path "' + path + '" is not in ' + 'schema, strict mode is `true`, and upsert is `true`.'); + } else if (options && options.strictQuery === 'throw') { + throw new StrictModeError(path, 'Path "' + path + '" is not in ' + + 'schema and strictQuery is true.'); } } else if (val === null || val === undefined) { obj[path] = null; diff --git a/lib/query.js b/lib/query.js index 58c45f51469..7da389ce858 100644 --- a/lib/query.js +++ b/lib/query.js @@ -2919,7 +2919,9 @@ Query.prototype.cast = function(model, obj) { return cast(model.schema, obj, { upsert: this.options && this.options.upsert, strict: (this.options && this.options.strict) || - (model.schema.options && model.schema.options.strict) + (model.schema.options && model.schema.options.strict), + strictQuery: (this.options && this.options.strictQuery) || + (model.schema.options && model.schema.options.strictQuery) }); } catch (err) { // CastError, assign model From b85897b3cb5c32de7c7cf9701de200f85670eee3 Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Thu, 13 Apr 2017 23:06:55 -0600 Subject: [PATCH 14/30] fix(model): return saved docs when create() fails Fix #2190 --- lib/model.js | 27 +++++++++++++++++++-------- 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/lib/model.js b/lib/model.js index 35bb745815c..ce8754317ff 100644 --- a/lib/model.js +++ b/lib/model.js @@ -1891,6 +1891,7 @@ Model.create = function create(doc, callback) { } var toExecute = []; + var firstError; args.forEach(function(doc) { toExecute.push(function(callback) { var Model = _this.discriminators && doc[discriminatorKey] ? @@ -1899,9 +1900,12 @@ Model.create = function create(doc, callback) { var toSave = doc instanceof Model ? doc : new Model(doc); var callbackWrapper = function(error, doc) { if (error) { - return callback(error); + if (!firstError) { + firstError = error; + } + return callback(null, { error: error }); } - callback(null, doc); + callback(null, { doc: doc }); }; // Hack to avoid getting a promise because of @@ -1914,12 +1918,20 @@ Model.create = function create(doc, callback) { }); }); - parallel(toExecute, function(error, savedDocs) { - if (error) { + parallel(toExecute, function(error, res) { + var savedDocs = []; + var len = res.length; + for (var i = 0; i < len; ++i) { + if (res[i].doc) { + savedDocs.push(res[i].doc); + } + } + + if (firstError) { if (cb) { - cb(error); + cb(firstError, savedDocs); } else { - reject(error); + reject(firstError); } return; } @@ -1930,8 +1942,7 @@ Model.create = function create(doc, callback) { } else { resolve.apply(promise, savedDocs); if (cb) { - savedDocs.unshift(null); - cb.apply(_this, savedDocs); + cb.apply(_this, [null].concat(savedDocs)); } } }); From 0d460a84fb65d5ecf3e37fb4f743472df9dfd4e6 Mon Sep 17 00:00:00 2001 From: Tuan Nguyen Date: Wed, 19 Apr 2017 11:27:03 +0700 Subject: [PATCH 15/30] Add alias schema option --- lib/schema.js | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/lib/schema.js b/lib/schema.js index 908761060d4..d0cf6141711 100644 --- a/lib/schema.js +++ b/lib/schema.js @@ -127,6 +127,35 @@ function Schema(obj, options) { if (this.options.timestamps) { this.setupTimestamp(this.options.timestamps); } + + // Assign virtual properties based on alias option + aliasFields(this); +} + +/*! + * Create virtual properties with alias field + */ +function aliasFields(schema) { + // console.log(schema.paths); + for (var path in schema.paths) { + var prop = schema.paths[path].path; + var alias = schema.paths[path].options.alias; + + if (alias && 'string' === typeof alias && alias.length > 0) { + schema + .virtual(alias) + .get((function(p) { + return function() { + return this.get(p); + }; + })(prop)) + .set((function(p) { + return function(v) { + return this.set(p, v); + }; + })(prop)); + } + } } /*! From 3b91fb5769a72b4b9c998702f136f86ab80e03df Mon Sep 17 00:00:00 2001 From: Tuan Nguyen Date: Thu, 20 Apr 2017 07:45:52 +0700 Subject: [PATCH 16/30] Safe check null options --- lib/schema.js | 2 ++ test.js | 20 ++++++++++++++++++++ 2 files changed, 22 insertions(+) create mode 100644 test.js diff --git a/lib/schema.js b/lib/schema.js index d0cf6141711..019ce018c2b 100644 --- a/lib/schema.js +++ b/lib/schema.js @@ -138,6 +138,8 @@ function Schema(obj, options) { function aliasFields(schema) { // console.log(schema.paths); for (var path in schema.paths) { + if (!schema.paths[path].options) continue; + var prop = schema.paths[path].path; var alias = schema.paths[path].options.alias; diff --git a/test.js b/test.js new file mode 100644 index 00000000000..dcd9671216c --- /dev/null +++ b/test.js @@ -0,0 +1,20 @@ +var mongoose = require('./index'); + +var Schema = mongoose.Schema; +var TestSchema = new Schema({ + foo: { type: String, alias: 'Fap' }, + bar: { + baz: { type: String, alias: 'Fap' }, + kar: { type: String } + } +}); + +var Test = mongoose.model('Test', TestSchema); +var t = new Test({ + foo: 'hey', + bar: { + baz: 'sup' + } +}); + +console.log(t.Fap); \ No newline at end of file From 5de4c7430871af74c1456ea4d3ae41cbfd83d5c2 Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Sat, 22 Apr 2017 21:10:16 -0600 Subject: [PATCH 17/30] feat(timestamps): support already defined timestamps Fix #4868 --- lib/schema.js | 20 ++++---- test/timestamps.test.js | 104 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 116 insertions(+), 8 deletions(-) create mode 100644 test/timestamps.test.js diff --git a/lib/schema.js b/lib/schema.js index 908761060d4..9c146444988 100644 --- a/lib/schema.js +++ b/lib/schema.js @@ -826,18 +826,22 @@ Schema.prototype.setupTimestamp = function(timestamps) { var parts = createdAt.split('.'); var i; var cur = schemaAdditions; - for (i = 0; i < parts.length; ++i) { - cur[parts[i]] = (i < parts.length - 1 ? - cur[parts[i]] || {} : - Date); + if (this.pathType(createdAt) === 'adhocOrUndefined') { + for (i = 0; i < parts.length; ++i) { + cur[parts[i]] = (i < parts.length - 1 ? + cur[parts[i]] || {} : + Date); + } } parts = updatedAt.split('.'); cur = schemaAdditions; - for (i = 0; i < parts.length; ++i) { - cur[parts[i]] = (i < parts.length - 1 ? - cur[parts[i]] || {} : - Date); + if (this.pathType(createdAt) === 'adhocOrUndefined') { + for (i = 0; i < parts.length; ++i) { + cur[parts[i]] = (i < parts.length - 1 ? + cur[parts[i]] || {} : + Date); + } } this.add(schemaAdditions); diff --git a/test/timestamps.test.js b/test/timestamps.test.js new file mode 100644 index 00000000000..f049ea95455 --- /dev/null +++ b/test/timestamps.test.js @@ -0,0 +1,104 @@ +'use strict'; + +var assert = require('assert'); +var start = require('./common'); + +var mongoose = start.mongoose; + +describe('timestamps', function() { + var db; + + before(function() { + db = start(); + }); + + after(function(done) { + db.close(done); + }); + + it('does not override timestamp params defined in schema (gh-4868)', function(done) { + var startTime = Date.now(); + var schema = new mongoose.Schema({ + createdAt: { + type: Date, + select: false + }, + updatedAt: { + type: Date, + select: true + }, + name: String + }, { timestamps: true }); + var M = db.model('gh4868', schema); + + M.create({ name: 'Test' }, function(error) { + assert.ifError(error); + M.findOne({}, function(error, doc) { + assert.ifError(error); + assert.ok(!doc.createdAt); + assert.ok(doc.updatedAt); + assert.ok(doc.updatedAt.valueOf() >= startTime); + done(); + }); + }); + }); + + it('does not override nested timestamp params defined in schema (gh-4868)', function(done) { + var startTime = Date.now(); + var schema = new mongoose.Schema({ + ts: { + createdAt: { + type: Date, + select: false + }, + updatedAt: { + type: Date, + select: true + } + }, + name: String + }, { timestamps: { createdAt: 'ts.createdAt', updatedAt: 'ts.updatedAt' } }); + var M = db.model('gh4868_0', schema); + + M.create({ name: 'Test' }, function(error) { + assert.ifError(error); + M.findOne({}, function(error, doc) { + assert.ifError(error); + assert.ok(!doc.ts.createdAt); + assert.ok(doc.ts.updatedAt); + assert.ok(doc.ts.updatedAt.valueOf() >= startTime); + done(); + }); + }); + }); + + it('does not override timestamps in nested schema (gh-4868)', function(done) { + var startTime = Date.now(); + var tsSchema = new mongoose.Schema({ + createdAt: { + type: Date, + select: false + }, + updatedAt: { + type: Date, + select: true + } + }); + var schema = new mongoose.Schema({ + ts: tsSchema, + name: String + }, { timestamps: { createdAt: 'ts.createdAt', updatedAt: 'ts.updatedAt' } }); + var M = db.model('gh4868_1', schema); + + M.create({ name: 'Test' }, function(error) { + assert.ifError(error); + M.findOne({}, function(error, doc) { + assert.ifError(error); + assert.ok(!doc.ts.createdAt); + assert.ok(doc.ts.updatedAt); + assert.ok(doc.ts.updatedAt.valueOf() >= startTime); + done(); + }); + }); + }); +}); From cd6bd5d9b472c285fc0c3b328464da41b1004239 Mon Sep 17 00:00:00 2001 From: Tuan Nguyen Date: Sun, 23 Apr 2017 16:40:30 +0700 Subject: [PATCH 18/30] Removed wrongly pushed file --- test.js | 20 -------------------- 1 file changed, 20 deletions(-) delete mode 100644 test.js diff --git a/test.js b/test.js deleted file mode 100644 index dcd9671216c..00000000000 --- a/test.js +++ /dev/null @@ -1,20 +0,0 @@ -var mongoose = require('./index'); - -var Schema = mongoose.Schema; -var TestSchema = new Schema({ - foo: { type: String, alias: 'Fap' }, - bar: { - baz: { type: String, alias: 'Fap' }, - kar: { type: String } - } -}); - -var Test = mongoose.model('Test', TestSchema); -var t = new Test({ - foo: 'hey', - bar: { - baz: 'sup' - } -}); - -console.log(t.Fap); \ No newline at end of file From 56b28a6fa774e83f5e509c974288b2cda4691072 Mon Sep 17 00:00:00 2001 From: Tuan Nguyen Date: Sun, 23 Apr 2017 16:41:03 +0700 Subject: [PATCH 19/30] Throws on invalid schema alias option --- lib/schema.js | 30 +++++++++++++++++------------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/lib/schema.js b/lib/schema.js index 019ce018c2b..89448cbd01f 100644 --- a/lib/schema.js +++ b/lib/schema.js @@ -143,19 +143,23 @@ function aliasFields(schema) { var prop = schema.paths[path].path; var alias = schema.paths[path].options.alias; - if (alias && 'string' === typeof alias && alias.length > 0) { - schema - .virtual(alias) - .get((function(p) { - return function() { - return this.get(p); - }; - })(prop)) - .set((function(p) { - return function(v) { - return this.set(p, v); - }; - })(prop)); + if (alias) { + if ('string' === typeof alias && alias.length > 0) { + schema + .virtual(alias) + .get((function(p) { + return function() { + return this.get(p); + }; + })(prop)) + .set((function(p) { + return function(v) { + return this.set(p, v); + }; + })(prop)); + } else { + throw new Error('Invalid value for alias option on ' + prop + ', got ' + alias); + } } } } From 8ded28529a26d4c2df88bf78bd7d93e1a0d3421a Mon Sep 17 00:00:00 2001 From: Tuan Nguyen Date: Sun, 23 Apr 2017 16:41:37 +0700 Subject: [PATCH 20/30] Add schema alias option tests --- test/schema.alias.test.js | 105 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 105 insertions(+) create mode 100644 test/schema.alias.test.js diff --git a/test/schema.alias.test.js b/test/schema.alias.test.js new file mode 100644 index 00000000000..2b81046e6d8 --- /dev/null +++ b/test/schema.alias.test.js @@ -0,0 +1,105 @@ + +/** + * Module dependencies. + */ + +var start = require('./common'), + mongoose = start.mongoose, + assert = require('power-assert'), + Schema = mongoose.Schema; + +describe('schema alias option', function() { + it('works with all basic schema types', function() { + var db = start(); + + var schema = new Schema({ + string: { type: String, alias: 'StringAlias' }, + number: { type: Number, alias: 'NumberAlias' }, + date: { type: Date, alias: 'DateAlias' }, + buffer: { type: Buffer, alias: 'BufferAlias' }, + boolean: { type: Boolean, alias: 'BooleanAlias' }, + mixed: { type: Schema.Types.Mixed, alias: 'MixedAlias' }, + objectId: { type: Schema.Types.ObjectId, alias: 'ObjectIdAlias'}, + array: { type: [], alias: 'ArrayAlias' } + }); + + var S = db.model('AliasSchemaType', schema); + S.create({ + string: 'hello', + number: 1, + date: new Date(), + buffer: Buffer.from('World'), + boolean: false, + mixed: [1, [], 'three', { four: 5 }], + objectId: new Schema.Types.ObjectId(), + array: ['a', 'b', 'c', 'd'] + }, function(err, s) { + assert.ifError(err); + + // Comparing with aliases + assert.equal(s.string, s.StringAlias); + assert.equal(s.number, s.NumberAlias); + assert.equal(s.date, s.DateAlias); + assert.equal(s.buffer, s.BufferAlias); + assert.equal(s.boolean, s.BooleanAlias); + assert.equal(s.mixed, s.MixedAlias); + assert.equal(s.objectId, s.ObjectIdAlias); + assert.equal(s.array, s.ArrayAlias); + }); + }); + + it('works with nested schema types', function() { + var db = start(); + + var schema = new Schema({ + nested: { + type: { + string: { type: String, alias: 'StringAlias' }, + number: { type: Number, alias: 'NumberAlias' }, + date: { type: Date, alias: 'DateAlias' }, + buffer: { type: Buffer, alias: 'BufferAlias' }, + boolean: { type: Boolean, alias: 'BooleanAlias' }, + mixed: { type: Schema.Types.Mixed, alias: 'MixedAlias' }, + objectId: { type: Schema.Types.ObjectId, alias: 'ObjectIdAlias'}, + array: { type: [], alias: 'ArrayAlias' } + }, + alias: 'NestedAlias' + } + }); + + var S = db.model('AliasNestedSchemaType', schema); + S.create({ + nested: { + string: 'hello', + number: 1, + date: new Date(), + buffer: Buffer.from('World'), + boolean: false, + mixed: [1, [], 'three', { four: 5 }], + objectId: new Schema.Types.ObjectId(), + array: ['a', 'b', 'c', 'd'] + } + }, function(err, s) { + assert.ifError(err); + + // Comparing with aliases + assert.equal(s.nested, s.NestedAlias); + assert.equal(s.nested.string, s.StringAlias); + assert.equal(s.nested.number, s.NumberAlias); + assert.equal(s.nested.date, s.DateAlias); + assert.equal(s.nested.buffer, s.BufferAlias); + assert.equal(s.nested.boolean, s.BooleanAlias); + assert.equal(s.nested.mixed, s.MixedAlias); + assert.equal(s.nested.objectId, s.ObjectIdAlias); + assert.equal(s.nested.array, s.ArrayAlias); + }); + }); + + it('throws when alias option is invalid', function() { + assert.throws(function() { + new Schema({ + foo: { type: String, alias: 456 } + }); + }); + }); +}); From 7976f4e2f8f3c282979de635fe75f9c6604dc6e6 Mon Sep 17 00:00:00 2001 From: Tuan Nguyen Date: Sun, 23 Apr 2017 16:50:39 +0700 Subject: [PATCH 21/30] Replace Buffer.from with new Buffer in tests for backward compatability with older versions of node --- test/schema.alias.test.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/schema.alias.test.js b/test/schema.alias.test.js index 2b81046e6d8..f696e6f992d 100644 --- a/test/schema.alias.test.js +++ b/test/schema.alias.test.js @@ -28,7 +28,7 @@ describe('schema alias option', function() { string: 'hello', number: 1, date: new Date(), - buffer: Buffer.from('World'), + buffer: new Buffer('World'), boolean: false, mixed: [1, [], 'three', { four: 5 }], objectId: new Schema.Types.ObjectId(), @@ -73,7 +73,7 @@ describe('schema alias option', function() { string: 'hello', number: 1, date: new Date(), - buffer: Buffer.from('World'), + buffer: new Buffer('World'), boolean: false, mixed: [1, [], 'three', { four: 5 }], objectId: new Schema.Types.ObjectId(), From 014cbbd095393e7b871f276822e3e6912930469f Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Sat, 13 May 2017 18:54:06 -0600 Subject: [PATCH 22/30] refactor: consolidate _plugins and plugins --- lib/index.js | 23 +++++++++++++++-------- lib/plugins/saveSubdocs.js | 2 -- lib/plugins/validateBeforeSave.js | 2 -- lib/schema.js | 2 +- lib/services/model/discriminator.js | 1 + test/model.discriminator.test.js | 2 -- 6 files changed, 17 insertions(+), 15 deletions(-) diff --git a/lib/index.js b/lib/index.js index 861fe8b1dac..e5720cc5c87 100644 --- a/lib/index.js +++ b/lib/index.js @@ -35,7 +35,6 @@ var shardingPlugin = require('./plugins/sharding'); function Mongoose() { this.connections = []; - this.plugins = [[shardingPlugin, {}]]; this.models = {}; this.modelSchemas = {}; // default global options @@ -46,6 +45,21 @@ function Mongoose() { conn.models = this.models; } +/*! + * ignore + */ + +Object.defineProperty(Mongoose.prototype, 'plugins', { + configurable: false, + enumerable: true, + writable: false, + value: [ + [saveSubdocs, { deduplicate: true }], + [validateBeforeSave, { deduplicate: true }], + [shardingPlugin, { deduplicate: true }] + ] +}); + /** * Expose connection states for user-land * @@ -367,13 +381,6 @@ Mongoose.prototype.model = function(name, schema, collection, skipInit) { } if (schema) { - // Reverse order because these both unshift() - if (!schema._plugins.saveSubdocs) { - schema.plugin(saveSubdocs); - } - if (!schema._plugins.validateBeforeSave) { - schema.plugin(validateBeforeSave); - } this._applyPlugins(schema); } diff --git a/lib/plugins/saveSubdocs.js b/lib/plugins/saveSubdocs.js index c51a8e117d1..ffb90b4d4a1 100644 --- a/lib/plugins/saveSubdocs.js +++ b/lib/plugins/saveSubdocs.js @@ -7,8 +7,6 @@ var each = require('async/each'); */ module.exports = function(schema) { - schema._plugins.saveSubdocs = true; - schema.callQueue.unshift(['pre', ['save', function(next) { if (this.ownerDocument) { next(); diff --git a/lib/plugins/validateBeforeSave.js b/lib/plugins/validateBeforeSave.js index 62734a56b1c..706cc0692b3 100644 --- a/lib/plugins/validateBeforeSave.js +++ b/lib/plugins/validateBeforeSave.js @@ -5,8 +5,6 @@ */ module.exports = function(schema) { - schema._plugins.validateBeforeSave = true; - schema.callQueue.unshift(['pre', ['save', function(next, options) { var _this = this; // Nested docs have their own presave diff --git a/lib/schema.js b/lib/schema.js index 439c678545a..c6207036230 100644 --- a/lib/schema.js +++ b/lib/schema.js @@ -273,7 +273,7 @@ Schema.prototype.clone = function() { s.callQueue = this.callQueue.map(function(f) { return f; }); s.methods = utils.clone(this.methods); s.statics = utils.clone(this.statics); - s._plugins = utils.clone(this._plugins); + s.plugins = Array.prototype.slice.call(this.plugins); s.s.hooks = this.s.hooks.clone(); return s; }; diff --git a/lib/services/model/discriminator.js b/lib/services/model/discriminator.js index 95a7d01e39b..68f0ba8b7dc 100644 --- a/lib/services/model/discriminator.js +++ b/lib/services/model/discriminator.js @@ -84,6 +84,7 @@ module.exports = function discriminator(model, name, schema) { schema.options.id = id; schema.s.hooks = model.schema.s.hooks.merge(schema.s.hooks); + schema.plugins = Array.prototype.slice(baseSchema.plugins); schema.callQueue = baseSchema.callQueue. concat(schema.callQueue.slice(schema._defaultMiddleware.length)); schema._requiredpaths = undefined; // reset just in case Schema#requiredPaths() was called on either schema diff --git a/test/model.discriminator.test.js b/test/model.discriminator.test.js index 61bd3df36da..147fd049571 100644 --- a/test/model.discriminator.test.js +++ b/test/model.discriminator.test.js @@ -316,8 +316,6 @@ describe('model', function() { it('merges callQueue with base queue defined before discriminator types callQueue', function(done) { assert.equal(Employee.schema.callQueue.length, 8); - // PersonSchema.post('save') - assert.strictEqual(Employee.schema.callQueue[0], Person.schema.callQueue[0]); // EmployeeSchema.pre('save') var queueIndex = Employee.schema.callQueue.length - 1; From 48cfb1c51138a328bc7180e7000f8309824dd705 Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Sat, 13 May 2017 18:55:48 -0600 Subject: [PATCH 23/30] test(discriminator): add coverage for child schema pre validate --- test/model.discriminator.test.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/test/model.discriminator.test.js b/test/model.discriminator.test.js index 147fd049571..427a19a4b83 100644 --- a/test/model.discriminator.test.js +++ b/test/model.discriminator.test.js @@ -451,6 +451,11 @@ describe('model', function() { name: String }); var childCalls = 0; + var childValidateCalls = 0; + childSchema.pre('validate', function(next) { + ++childValidateCalls; + next(); + }); childSchema.pre('save', function(next) { ++childCalls; next(); @@ -484,6 +489,7 @@ describe('model', function() { assert.equal(doc.heir.name, 'Robb Stark'); assert.equal(doc.children.length, 1); assert.equal(doc.children[0].name, 'Jon Snow'); + assert.equal(childValidateCalls, 2); assert.equal(childCalls, 2); assert.equal(parentCalls, 1); done(); From 9a19a82130d72ed723a147f2b1c0b4c7b02de2cd Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Sat, 13 May 2017 18:59:43 -0600 Subject: [PATCH 24/30] chore: dont use .find() for ES5 compat --- lib/schema.js | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/lib/schema.js b/lib/schema.js index c6207036230..c843c4eff02 100644 --- a/lib/schema.js +++ b/lib/schema.js @@ -1115,9 +1115,12 @@ Schema.prototype.plugin = function(fn, opts) { } if (opts && - opts.deduplicate && - this.plugins.find(function(p) { return p.fn === fn; })) { - return this; + opts.deduplicate) { + for (var i = 0; i < this.plugins.length; ++i) { + if (this.plugins[i].fn === fn) { + return this; + } + } } this.plugins.push({ fn: fn, opts: opts }); From 52c6b35186b1e17d117676af76e10b7beffb55d8 Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Sun, 14 May 2017 22:36:59 -0600 Subject: [PATCH 25/30] test(schema): repro unique index issue with primitive arrays re: #3347 --- test/model.indexes.test.js | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/test/model.indexes.test.js b/test/model.indexes.test.js index 34b57ba7a4a..c130b125c82 100644 --- a/test/model.indexes.test.js +++ b/test/model.indexes.test.js @@ -208,6 +208,19 @@ describe('model', function() { done(); }); + it('primitive arrays (gh-3347)', function(done) { + var schema = new Schema({ + arr: [{ type: String, unique: true }] + }); + + var indexes = schema.indexes(); + assert.equal(indexes.length, 1); + assert.deepEqual(indexes[0][0], { arr: 1 }); + assert.ok(indexes[0][1].unique); + + done(); + }); + it('error should emit on the model', function(done) { var db = start(), schema = new Schema({name: {type: String}}), From 5908ef029214146aa2f93d5cd81b5bffadef8b3f Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Sun, 14 May 2017 22:40:19 -0600 Subject: [PATCH 26/30] fix(schema): set unique indexes on primitive arrays Re: #3347 --- lib/schema.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/schema.js b/lib/schema.js index 671889dd491..0d039fff0bb 100644 --- a/lib/schema.js +++ b/lib/schema.js @@ -1368,7 +1368,7 @@ Schema.prototype.indexes = function() { if ((path instanceof MongooseTypes.DocumentArray) || path.$isSingleNested) { collectIndexes(path.schema, prefix + key + '.'); } else { - index = path._index; + index = path._index || (path.caster && path.caster._index); if (index !== false && index !== null && index !== undefined) { field = {}; From 25c350f690bd6043b6f05537ce5a479bbb986d49 Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Sun, 14 May 2017 22:40:44 -0600 Subject: [PATCH 27/30] fix(model): always emit 'index', even if no indexes Re: #3347 --- lib/model.js | 17 +++++++++-------- test/model.indexes.test.js | 13 +++++++------ 2 files changed, 16 insertions(+), 14 deletions(-) diff --git a/lib/model.js b/lib/model.js index d65ea2df9e6..7e9f2666ed8 100644 --- a/lib/model.js +++ b/lib/model.js @@ -929,14 +929,6 @@ Model.ensureIndexes = function ensureIndexes(options, callback) { function _ensureIndexes(model, options, callback) { var indexes = model.schema.indexes(); - if (!indexes.length) { - setImmediate(function() { - callback && callback(); - }); - return; - } - // Indexes are created one-by-one to support how MongoDB < 2.4 deals - // with background indexes. var done = function(err) { if (err && model.schema.options.emitIndexErrors) { @@ -946,6 +938,15 @@ function _ensureIndexes(model, options, callback) { callback && callback(err); }; + if (!indexes.length) { + setImmediate(function() { + done(); + }); + return; + } + // Indexes are created one-by-one to support how MongoDB < 2.4 deals + // with background indexes. + var indexSingleDone = function(err, fields, options, name) { model.emit('index-single-done', err, fields, options, name); }; diff --git a/test/model.indexes.test.js b/test/model.indexes.test.js index c130b125c82..7c0e7a89467 100644 --- a/test/model.indexes.test.js +++ b/test/model.indexes.test.js @@ -226,16 +226,17 @@ describe('model', function() { schema = new Schema({name: {type: String}}), Test = db.model('IndexError', schema, 'x' + random()); - Test.on('index', function(err) { - db.close(); - assert.ok(/E11000 duplicate key error/.test(err.message), err); - done(); - }); - Test.create({name: 'hi'}, {name: 'hi'}, function(err) { assert.strictEqual(err, null); Test.schema.index({name: 1}, {unique: true}); Test.schema.index({other: 1}); + + Test.on('index', function(err) { + db.close(); + assert.ok(/E11000 duplicate key error/.test(err.message), err); + done(); + }); + Test.init(); }); }); From 6987ba8c3ea721a59fc9b012615cbf344be78695 Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Mon, 15 May 2017 17:38:26 -0600 Subject: [PATCH 28/30] fix(document): make nested doc keys not enumerable again Fix #5078 --- lib/document.js | 63 ++++++++++++++++++++++++++++--------------- test/document.test.js | 27 +++++++++++++++++++ 2 files changed, 68 insertions(+), 22 deletions(-) diff --git a/lib/document.js b/lib/document.js index 5369cfcf3a4..be05e95a3ff 100644 --- a/lib/document.js +++ b/lib/document.js @@ -1767,25 +1767,44 @@ Document.prototype.$__dirty = function() { */ function compile(tree, proto, prefix, options) { - var keys = Object.keys(tree), - i = keys.length, - limb, - key; + var keys = Object.keys(tree); + var i = keys.length; + var len = keys.length; + var limb; + var key; - while (i--) { - key = keys[i]; - limb = tree[key]; - - defineKey(key, - ((utils.getFunctionName(limb.constructor) === 'Object' - && Object.keys(limb).length) - && (!limb[options.typeKey] || (options.typeKey === 'type' && limb.type.type)) - ? limb - : null) - , proto - , prefix - , keys - , options); + if (options.retainKeyOrder) { + for (i = 0; i < len; ++i) { + key = keys[i]; + limb = tree[key]; + + defineKey(key, + ((utils.getFunctionName(limb.constructor) === 'Object' + && Object.keys(limb).length) + && (!limb[options.typeKey] || (options.typeKey === 'type' && limb.type.type)) + ? limb + : null) + , proto + , prefix + , keys + , options); + } + } else { + while (i--) { + key = keys[i]; + limb = tree[key]; + + defineKey(key, + ((utils.getFunctionName(limb.constructor) === 'Object' + && Object.keys(limb).length) + && (!limb[options.typeKey] || (options.typeKey === 'type' && limb.type.type)) + ? limb + : null) + , proto + , prefix + , keys + , options); + } } } @@ -1796,7 +1815,7 @@ function getOwnPropertyDescriptors(object) { Object.getOwnPropertyNames(object).forEach(function(key) { result[key] = Object.getOwnPropertyDescriptor(object, key); - result[key].enumerable = true; + result[key].enumerable = ['isNew', '$__', 'errors', '_doc'].indexOf(key) === -1; }); return result; @@ -1844,7 +1863,7 @@ function defineKey(prop, subprops, prototype, prefix, keys, options) { } Object.defineProperty(nested, 'toObject', { - enumerable: true, + enumerable: false, configurable: true, writable: false, value: function() { @@ -1853,7 +1872,7 @@ function defineKey(prop, subprops, prototype, prefix, keys, options) { }); Object.defineProperty(nested, 'toJSON', { - enumerable: true, + enumerable: false, configurable: true, writable: false, value: function() { @@ -1862,7 +1881,7 @@ function defineKey(prop, subprops, prototype, prefix, keys, options) { }); Object.defineProperty(nested, '$__isNested', { - enumerable: true, + enumerable: false, configurable: true, writable: false, value: true diff --git a/test/document.test.js b/test/document.test.js index 455d17352a9..3f46f040792 100644 --- a/test/document.test.js +++ b/test/document.test.js @@ -4037,6 +4037,33 @@ describe('document', function() { done(); }); + it('iterating through nested doc keys (gh-5078)', function(done) { + var schema = new Schema({ + nested: { + test1: String, + test2: String + } + }, { retainKeyOrder: true }); + + schema.virtual('tests').get(function() { + return _.map(this.nested, function(v, key) { + return v; + }) + }); + + var M = db.model('gh5078', schema); + + var doc = new M({ nested: { test1: 'a', test2: 'b' } }); + + assert.deepEqual(doc.toObject({ virtuals: true }).tests, ['a', 'b']); + + // Should not throw + require('util').inspect(doc); + JSON.stringify(doc); + + done(); + }); + it('JSON.stringify nested errors (gh-5208)', function(done) { var AdditionalContactSchema = new Schema({ contactName: { From 636e922fd04981b94a131aa47b4fb925fa417308 Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Mon, 15 May 2017 17:42:14 -0600 Subject: [PATCH 29/30] style: fix lint --- test/document.test.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/document.test.js b/test/document.test.js index 3f46f040792..0419ee3d5b6 100644 --- a/test/document.test.js +++ b/test/document.test.js @@ -4046,9 +4046,9 @@ describe('document', function() { }, { retainKeyOrder: true }); schema.virtual('tests').get(function() { - return _.map(this.nested, function(v, key) { + return _.map(this.nested, function(v) { return v; - }) + }); }); var M = db.model('gh5078', schema); From 4bc7b3ee9bc81b4a818e1e94985217f0c32c0ffc Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Thu, 18 May 2017 14:05:55 -0600 Subject: [PATCH 30/30] docs: add missing ignores to sharding plugin --- lib/plugins/sharding.js | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/lib/plugins/sharding.js b/lib/plugins/sharding.js index 4a2f85ea1fe..1d5cf5944ec 100644 --- a/lib/plugins/sharding.js +++ b/lib/plugins/sharding.js @@ -2,6 +2,10 @@ var utils = require('../utils'); +/*! + * ignore + */ + module.exports = function shardingPlugin(schema) { schema.post('init', function() { storeShard.call(this); @@ -16,6 +20,10 @@ module.exports = function shardingPlugin(schema) { }); }; +/*! + * ignore + */ + function applyWhere() { var paths; var len; @@ -32,18 +40,15 @@ function applyWhere() { } /*! - * Stores the current values of the shard keys. - * - * ####Note: - * - * _Shard key values do not / are not allowed to change._ - * - * @api private - * @method $__storeShard - * @memberOf Document + * ignore */ + module.exports.storeShard = storeShard; +/*! + * ignore + */ + function storeShard() { // backwards compat var key = this.schema.options.shardKey || this.schema.options.shardkey;