From 5da9f44837124534903a0b68e53c4a497936219a Mon Sep 17 00:00:00 2001 From: Hafez Date: Sat, 16 May 2020 14:18:11 +0200 Subject: [PATCH 01/48] test: repro #6658 strictQuery --- test/schema.test.js | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/test/schema.test.js b/test/schema.test.js index 0a1e00f6b80..fa45ee07f90 100644 --- a/test/schema.test.js +++ b/test/schema.test.js @@ -2430,4 +2430,26 @@ describe('schema', function() { assert.throws(buildInvalidSchema, /`db` may not be used as a schema pathname/); }); }); + + it('setting `strictQuery` on base sets strictQuery to schema', function() { + // Arrange + mongoose.set('strictQuery', 'some value'); + + // Act + const schema = new Schema(); + + // Assert + assert.equal(schema.get('strictQuery'), 'some value'); + }); + + it('`strictQuery` set on base gets overwritten by option set on schema', function() { + // Arrange + mongoose.set('strictQuery', 'base option'); + + // Act + const schema = new Schema({}, { strictQuery: 'schema option' }); + + // Assert + assert.equal(schema.get('strictQuery'), 'schema option'); + }); }); From f7f9b4fbe2980f444a5d7c3a5e03c27924f9afb8 Mon Sep 17 00:00:00 2001 From: Hafez Date: Sat, 16 May 2020 14:21:42 +0200 Subject: [PATCH 02/48] fix(base): allow setting strictQuery globally --- lib/index.js | 1 + lib/schema.js | 1 + lib/validoptions.js | 1 + 3 files changed, 3 insertions(+) diff --git a/lib/index.js b/lib/index.js index 8c7c9f0aa90..30e51fd26f1 100644 --- a/lib/index.js +++ b/lib/index.js @@ -158,6 +158,7 @@ Mongoose.prototype.driver = require('./driver'); * - 'toObject': `{ transform: true, flattenDecimals: true }` by default. Overwrites default objects to [`toObject()`](/docs/api.html#document_Document-toObject) * - 'toJSON': `{ transform: true, flattenDecimals: true }` by default. Overwrites default objects to [`toJSON()`](/docs/api.html#document_Document-toJSON), for determining how Mongoose documents get serialized by `JSON.stringify()` * - 'strict': true by default, may be `false`, `true`, or `'throw'`. Sets the default strict mode for schemas. + * - 'strictQuery': false by default, may be `false`, `true`, or `'throw'`. Sets the default [strictQuery](/docs/guide.html#strictQuery) mode for schemas. * - 'selectPopulatedPaths': true by default. Set to false to opt out of Mongoose adding all fields that you `populate()` to your `select()`. The schema-level option `selectPopulatedPaths` overwrites this one. * - 'typePojoToMixed': true by default, may be `false` or `true`. Sets the default typePojoToMixed for schemas. * - 'maxTimeMS': If set, attaches [maxTimeMS](https://docs.mongodb.com/manual/reference/operator/meta/maxTimeMS/) to every query diff --git a/lib/schema.js b/lib/schema.js index 777cfce4434..996fd04f023 100644 --- a/lib/schema.js +++ b/lib/schema.js @@ -400,6 +400,7 @@ Schema.prototype.defaultOptions = function(options) { const baseOptions = get(this, 'base.options', {}); options = utils.options({ strict: 'strict' in baseOptions ? baseOptions.strict : true, + strictQuery: 'strictQuery' in baseOptions ? baseOptions.strictQuery : false, bufferCommands: true, capped: false, // { size, max, autoIndexId } versionKey: '__v', diff --git a/lib/validoptions.js b/lib/validoptions.js index 9464e4d8611..68a7b424dfd 100644 --- a/lib/validoptions.js +++ b/lib/validoptions.js @@ -17,6 +17,7 @@ const VALID_OPTIONS = Object.freeze([ 'runValidators', 'selectPopulatedPaths', 'strict', + 'strictQuery', 'toJSON', 'toObject', 'useCreateIndex', From 80d40e557645f9cca1ec54fd09283efde52acb6d Mon Sep 17 00:00:00 2001 From: Hafez Date: Sat, 16 May 2020 14:42:16 +0200 Subject: [PATCH 03/48] fix test causing other tests to fail --- test/schema.test.js | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/test/schema.test.js b/test/schema.test.js index fa45ee07f90..43b3e14d431 100644 --- a/test/schema.test.js +++ b/test/schema.test.js @@ -2431,8 +2431,9 @@ describe('schema', function() { }); }); - it('setting `strictQuery` on base sets strictQuery to schema', function() { + it('setting `strictQuery` on base sets strictQuery to schema (gh-6658)', function() { // Arrange + const originalValue = mongoose.get('strictQuery'); mongoose.set('strictQuery', 'some value'); // Act @@ -2440,10 +2441,14 @@ describe('schema', function() { // Assert assert.equal(schema.get('strictQuery'), 'some value'); + + // Cleanup + mongoose.set('strictQuery', originalValue); }); - it('`strictQuery` set on base gets overwritten by option set on schema', function() { + it('`strictQuery` set on base gets overwritten by option set on schema (gh-6658)', function() { // Arrange + const originalValue = mongoose.get('strictQuery'); mongoose.set('strictQuery', 'base option'); // Act @@ -2451,5 +2456,8 @@ describe('schema', function() { // Assert assert.equal(schema.get('strictQuery'), 'schema option'); + + // Cleanup + mongoose.set('strictQuery', originalValue); }); }); From 93b3ed6dbc2c8ec2a113b9db9bc19dd9d105422b Mon Sep 17 00:00:00 2001 From: Hafez Date: Sat, 16 May 2020 14:50:16 +0200 Subject: [PATCH 04/48] test: refactor tests re #6658, use beforeEach to guard against failed tests not cleaning up after failing which will in turn cause more tests to fail, making it difficult to know the root cause of all the failing tests --- test/schema.test.js | 41 ++++++++++++++++++++--------------------- 1 file changed, 20 insertions(+), 21 deletions(-) diff --git a/test/schema.test.js b/test/schema.test.js index 43b3e14d431..88dca91175d 100644 --- a/test/schema.test.js +++ b/test/schema.test.js @@ -2431,33 +2431,32 @@ describe('schema', function() { }); }); - it('setting `strictQuery` on base sets strictQuery to schema (gh-6658)', function() { - // Arrange - const originalValue = mongoose.get('strictQuery'); - mongoose.set('strictQuery', 'some value'); + describe('mongoose.set(`strictQuery`, value); (gh-6658)', function() { + let strictQueryOriginalValue; - // Act - const schema = new Schema(); + this.beforeEach(() => strictQueryOriginalValue = mongoose.get('strictQuery')); + this.afterEach(() => mongoose.set('strictQuery', strictQueryOriginalValue)); - // Assert - assert.equal(schema.get('strictQuery'), 'some value'); + it('setting `strictQuery` on base sets strictQuery to schema (gh-6658)', function() { + // Arrange + mongoose.set('strictQuery', 'some value'); - // Cleanup - mongoose.set('strictQuery', originalValue); - }); + // Act + const schema = new Schema(); - it('`strictQuery` set on base gets overwritten by option set on schema (gh-6658)', function() { - // Arrange - const originalValue = mongoose.get('strictQuery'); - mongoose.set('strictQuery', 'base option'); + // Assert + assert.equal(schema.get('strictQuery'), 'some value'); + }); - // Act - const schema = new Schema({}, { strictQuery: 'schema option' }); + it('`strictQuery` set on base gets overwritten by option set on schema (gh-6658)', function() { + // Arrange + mongoose.set('strictQuery', 'base option'); - // Assert - assert.equal(schema.get('strictQuery'), 'schema option'); + // Act + const schema = new Schema({}, { strictQuery: 'schema option' }); - // Cleanup - mongoose.set('strictQuery', originalValue); + // Assert + assert.equal(schema.get('strictQuery'), 'schema option'); + }); }); }); From a262140b37a91df1089ef007d2ff3dbaf0c757c5 Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Sat, 16 May 2020 18:13:14 -0400 Subject: [PATCH 05/48] feat(connection): add `transaction()` function to handle resetting `Document#isNew` when a transaction is aborted Fix #8852 --- lib/connection.js | 53 +++++++++++++++++++++++++++++++++ lib/helpers/symbols.js | 1 + lib/index.js | 4 ++- lib/plugins/trackTransaction.js | 20 +++++++++++++ test/docs/transactions.test.js | 21 ++++++++++++- 5 files changed, 97 insertions(+), 2 deletions(-) create mode 100644 lib/plugins/trackTransaction.js diff --git a/lib/connection.js b/lib/connection.js index 12f0517aa06..85a00c27bcb 100644 --- a/lib/connection.js +++ b/lib/connection.js @@ -22,6 +22,8 @@ const utils = require('./utils'); const parseConnectionString = require('mongodb/lib/core').parseConnectionString; +const sessionNewDocuments = require('./helpers/symbols').sessionNewDocuments; + let id = 0; /*! @@ -417,6 +419,57 @@ Connection.prototype.startSession = _wrapConnHelper(function startSession(option cb(null, session); }); +/** + * _Requires MongoDB >= 3.6.0._ Executes the wrapped async function + * in a transaction. Mongoose will commit the transaction if the + * async function executes successfully and attempt to retry if + * there was a retriable error. + * + * Calls the MongoDB driver's [`session.withTransaction()`](http://mongodb.github.io/node-mongodb-native/3.5/api/ClientSession.html#withTransaction), + * but also handles resetting Mongoose document state as shown below. + * + * ####Example: + * + * const doc = new Person({ name: 'Will Riker' }); + * await db.transaction(async function setRank(session) { + * doc.rank = 'Captain'; + * await doc.save({ session }); + * doc.isNew; // false + * + * // Throw an error to abort the transaction + * throw new Error('Oops!'); + * }).catch(() => {}); + * + * // true, `transaction()` reset the document's state because the + * // transaction was aborted. + * doc.isNew; + * + * @method transaction + * @param {Function} fn Function to execute in a transaction + * @return {Promise} promise that resolves to the returned value of `fn` + * @api public + */ + +Connection.prototype.transaction = function transaction(fn) { + return this.startSession().then(session => { + session[sessionNewDocuments] = []; + return session.withTransaction(() => fn(session)). + then(res => { + delete session[sessionNewDocuments]; + return res; + }). + catch(err => { + // If transaction was aborted, we need to reset newly + // inserted documents' `isNew`. + for (const doc of session[sessionNewDocuments]) { + doc.isNew = true; + } + delete session[sessionNewDocuments]; + throw err; + }); + }); +}; + /** * Helper for `dropCollection()`. Will delete the given collection, including * all documents and indexes. diff --git a/lib/helpers/symbols.js b/lib/helpers/symbols.js index 3860f237743..4db388edfe7 100644 --- a/lib/helpers/symbols.js +++ b/lib/helpers/symbols.js @@ -11,4 +11,5 @@ exports.modelSymbol = Symbol('mongoose#Model'); exports.objectIdSymbol = Symbol('mongoose#ObjectId'); exports.populateModelSymbol = Symbol('mongoose.PopulateOptions#Model'); exports.schemaTypeSymbol = Symbol('mongoose#schemaType'); +exports.sessionNewDocuments = Symbol('mongoose:ClientSession#newDocuments'); exports.validatorErrorSymbol = Symbol('mongoose:validatorError'); \ No newline at end of file diff --git a/lib/index.js b/lib/index.js index 8c7c9f0aa90..b0d3e086151 100644 --- a/lib/index.js +++ b/lib/index.js @@ -34,6 +34,7 @@ const pkg = require('../package.json'); const cast = require('./cast'); const removeSubdocs = require('./plugins/removeSubdocs'); const saveSubdocs = require('./plugins/saveSubdocs'); +const trackTransaction = require('./plugins/trackTransaction'); const validateBeforeSave = require('./plugins/validateBeforeSave'); const Aggregate = require('./aggregate'); @@ -106,7 +107,8 @@ function Mongoose(options) { [saveSubdocs, { deduplicate: true }], [validateBeforeSave, { deduplicate: true }], [shardingPlugin, { deduplicate: true }], - [removeSubdocs, { deduplicate: true }] + [removeSubdocs, { deduplicate: true }], + [trackTransaction, { deduplicate: true }] ] }); } diff --git a/lib/plugins/trackTransaction.js b/lib/plugins/trackTransaction.js new file mode 100644 index 00000000000..c307f9b759a --- /dev/null +++ b/lib/plugins/trackTransaction.js @@ -0,0 +1,20 @@ +'use strict'; + +const sessionNewDocuments = require('../helpers/symbols').sessionNewDocuments; + +module.exports = function trackTransaction(schema) { + schema.pre('save', function() { + if (!this.isNew) { + return; + } + + const session = this.$session(); + if (session == null) { + return; + } + if (session.transaction == null || session[sessionNewDocuments] == null) { + return; + } + session[sessionNewDocuments].push(this); + }); +}; \ No newline at end of file diff --git a/test/docs/transactions.test.js b/test/docs/transactions.test.js index e27662861d8..73daab51e83 100644 --- a/test/docs/transactions.test.js +++ b/test/docs/transactions.test.js @@ -360,7 +360,26 @@ describe('transactions', function() { const test = yield Test.create([{}], { session }).then(res => res[0]); yield test.save(); // throws DocumentNotFoundError })); - yield session.endSession(); + session.endSession(); + }); + }); + + it('correct `isNew` after abort (gh-8852)', function() { + return co(function*() { + const schema = Schema({ name: String }); + + const Test = db.model('gh8852', schema); + + yield Test.createCollection(); + const doc = new Test({ name: 'foo' }); + yield db. + transaction(session => co(function*() { + yield doc.save({ session }); + assert.ok(!doc.isNew); + throw new Error('Oops'); + })). + catch(err => assert.equal(err.message, 'Oops')); + assert.ok(doc.isNew); }); }); }); From eab6cc07858762cade92fa64e64e635557d69f6a Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Sat, 16 May 2020 18:17:57 -0400 Subject: [PATCH 06/48] style: fix lint --- lib/connection.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/connection.js b/lib/connection.js index 85a00c27bcb..d9dabf5b04e 100644 --- a/lib/connection.js +++ b/lib/connection.js @@ -424,7 +424,7 @@ Connection.prototype.startSession = _wrapConnHelper(function startSession(option * in a transaction. Mongoose will commit the transaction if the * async function executes successfully and attempt to retry if * there was a retriable error. - * + * * Calls the MongoDB driver's [`session.withTransaction()`](http://mongodb.github.io/node-mongodb-native/3.5/api/ClientSession.html#withTransaction), * but also handles resetting Mongoose document state as shown below. * @@ -435,7 +435,7 @@ Connection.prototype.startSession = _wrapConnHelper(function startSession(option * doc.rank = 'Captain'; * await doc.save({ session }); * doc.isNew; // false - * + * * // Throw an error to abort the transaction * throw new Error('Oops!'); * }).catch(() => {}); From 89451927edbdd91c15dc325b7b1979d484b576c7 Mon Sep 17 00:00:00 2001 From: Hafez Date: Tue, 19 May 2020 19:52:37 +0200 Subject: [PATCH 07/48] test: repro #9032 --- test/index.test.js | 52 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/test/index.test.js b/test/index.test.js index a8025689c86..bb9e070c74a 100644 --- a/test/index.test.js +++ b/test/index.test.js @@ -778,5 +778,57 @@ describe('mongoose module:', function() { done(); }); }); + + it('can set `setDefaultsOnInsert` as a global option (gh-9032)', function() { + return co(function* () { + const m = new mongoose.Mongoose(); + m.set('setDefaultsOnInsert', true); + const db = yield m.connect('mongodb://localhost:27017/mongoose_test_9032'); + + const schema = new m.Schema({ + title: String, + genre: { type: String, default: 'Action' } + }, { collection: 'movies_1' }); + + const Movie = db.model('Movie', schema); + + + yield Movie.updateOne( + {}, + { title: 'Cloud Atlas' }, + { upsert: true } + ); + + // lean is necessary to avoid defaults by casting + const movie = yield Movie.findOne({ title: 'Cloud Atlas' }).lean(); + assert.equal(movie.genre, 'Action'); + }); + }); + + it('setting `setDefaultOnInsert` on operation has priority over base option (gh-9032)', function() { + return co(function* () { + const m = new mongoose.Mongoose(); + m.set('setDefaultsOnInsert', true); + const db = yield m.connect('mongodb://localhost:27017/mongoose_test_9032'); + + const schema = new m.Schema({ + title: String, + genre: { type: String, default: 'Action' } + }, { collection: 'movies_2' }); + + const Movie = db.model('Movie', schema); + + + yield Movie.updateOne( + {}, + { title: 'The Man From Earth' }, + { upsert: true, setDefaultsOnInsert: false } + ); + + // lean is necessary to avoid defaults by casting + const movie = yield Movie.findOne({ title: 'The Man From Earth' }).lean(); + assert.ok(!movie.genre); + }); + }); }); }); From 4833de1b3649d89cd38adcbb39143dd5102c9ecc Mon Sep 17 00:00:00 2001 From: Hafez Date: Tue, 19 May 2020 19:54:07 +0200 Subject: [PATCH 08/48] feat(mongoose): add support for setting `setDefaultsOnInsert` as a global option --- lib/helpers/setDefaultsOnInsert.js | 17 +++++++++++------ lib/validoptions.js | 1 + 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/lib/helpers/setDefaultsOnInsert.js b/lib/helpers/setDefaultsOnInsert.js index 0545e80d3d4..747cb61ab8d 100644 --- a/lib/helpers/setDefaultsOnInsert.js +++ b/lib/helpers/setDefaultsOnInsert.js @@ -14,6 +14,17 @@ const modifiedPaths = require('./common').modifiedPaths; */ module.exports = function(filter, schema, castedDoc, options) { + options = options || {}; + + const shouldSetDefaultsOnInsert = + options.hasOwnProperty('setDefaultsOnInsert') ? + options.setDefaultsOnInsert : + schema.base.options.setDefaultsOnInsert; + + if (!options.upsert || !shouldSetDefaultsOnInsert) { + return castedDoc; + } + const keys = Object.keys(castedDoc || {}); const updatedKeys = {}; const updatedValues = {}; @@ -22,12 +33,6 @@ module.exports = function(filter, schema, castedDoc, options) { let hasDollarUpdate = false; - options = options || {}; - - if (!options.upsert || !options.setDefaultsOnInsert) { - return castedDoc; - } - for (let i = 0; i < numKeys; ++i) { if (keys[i].startsWith('$')) { modifiedPaths(castedDoc[keys[i]], '', modified); diff --git a/lib/validoptions.js b/lib/validoptions.js index 9464e4d8611..3ff4ada636c 100644 --- a/lib/validoptions.js +++ b/lib/validoptions.js @@ -16,6 +16,7 @@ const VALID_OPTIONS = Object.freeze([ 'objectIdGetter', 'runValidators', 'selectPopulatedPaths', + 'setDefaultsOnInsert', 'strict', 'toJSON', 'toObject', From e83a7a86152dcc7b9110b47197ac669abce5d00a Mon Sep 17 00:00:00 2001 From: Hafez Date: Fri, 5 Jun 2020 17:43:52 +0200 Subject: [PATCH 09/48] test: repro #9096 --- test/document.test.js | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/test/document.test.js b/test/document.test.js index 1d9dac4005e..8a10958f8c5 100644 --- a/test/document.test.js +++ b/test/document.test.js @@ -9051,4 +9051,29 @@ describe('document', function() { assert.equal(doc.nested.prop, 'some default value'); }); }); + + describe('Document#getChanges(...) (gh-9096)', function() { + it('returns an empty object when there are no changes', function() { + return co(function*() { + const User = db.model('User', { name: String, age: Number, country: String }); + const user = yield User.create({ name: 'Hafez', age: 25, country: 'Egypt' }); + + const changes = user.getChanges(); + assert.deepEqual(changes, {}); + }); + }); + + it('returns only the changed paths', function() { + return co(function*() { + const User = db.model('User', { name: String, age: Number, country: String }); + const user = yield User.create({ name: 'Hafez', age: 25, country: 'Egypt' }); + + user.country = undefined; + user.age = 26; + + const changes = user.getChanges(); + assert.deepEqual(changes, { $set: { age: 26 }, $unset: { country: 1 } }); + }); + }); + }); }); From 8be78a4eae051668cd6e5c3acca45c6dd1725ba4 Mon Sep 17 00:00:00 2001 From: Hafez Date: Fri, 5 Jun 2020 17:49:25 +0200 Subject: [PATCH 10/48] feat(document): add Document#getChanges --- lib/document.js | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/lib/document.js b/lib/document.js index 72449083b3d..bc948ebea01 100644 --- a/lib/document.js +++ b/lib/document.js @@ -3825,6 +3825,45 @@ Document.prototype.$__fullPath = function(path) { return path || ''; }; +/** + * Returns the changes that happened to the document + * in the format that will be sent to MongoDB. + * + * ###Example: + * const userSchema = new Schema({ + * name: String, + * age: Number, + * country: String + * }); + * const User = mongoose.model('User', userSchema); + * const user = await User.create({ + * name: 'Hafez', + * age: 25, + * country: 'Egypt' + * }); + * + * // returns an empty object, no changes happened yet + * user.getChanges(); // { } + * + * user.country = undefined; + * user.age = 26; + * + * user.getChanges(); // { $set: { age: 26 }, { $unset: { country: 1 } } } + * + * await user.save(); + * + * user.getChanges(); // { } + * + * @return {Object} changes + */ + +Document.prototype.getChanges = function() { + const delta = this.$__delta(); + + const changes = delta ? delta[1] : {}; + return changes; +}; + /*! * Module exports. */ From 2f8152172761dfa0464bfda82c6774148763f1f5 Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Sat, 13 Jun 2020 17:09:16 -0400 Subject: [PATCH 11/48] feat(SingleNestedPath+DocumentArray): add static `set()` function for global options, support setting `_id` globally Fix #8883 --- lib/schema.js | 6 ++++++ lib/schema/SingleNestedPath.js | 18 ++++++++++++++++++ lib/schema/documentarray.js | 18 ++++++++++++++++++ test/schema.documentarray.test.js | 16 ++++++++++++++++ test/schema.singlenestedpath.test.js | 19 +++++++++++++++++++ 5 files changed, 77 insertions(+) diff --git a/lib/schema.js b/lib/schema.js index 1724dd08be9..cc721b074ee 100644 --- a/lib/schema.js +++ b/lib/schema.js @@ -936,6 +936,12 @@ Schema.prototype.interpretAsType = function(path, obj, options) { if (options.hasOwnProperty('typePojoToMixed')) { childSchemaOptions.typePojoToMixed = options.typePojoToMixed; } + if (this._userProvidedOptions.hasOwnProperty('_id')) { + childSchemaOptions._id = this._userProvidedOptions._id; + } else if (Schema.Types.DocumentArray.defaultOptions._id != null) { + childSchemaOptions._id = Schema.Types.DocumentArray.defaultOptions._id; + } + const childSchema = new Schema(cast, childSchemaOptions); childSchema.$implicitlyCreated = true; return new MongooseTypes.DocumentArray(path, childSchema, obj); diff --git a/lib/schema/SingleNestedPath.js b/lib/schema/SingleNestedPath.js index d7abd6cea77..dc7f11ec5f8 100644 --- a/lib/schema/SingleNestedPath.js +++ b/lib/schema/SingleNestedPath.js @@ -300,6 +300,24 @@ SingleNestedPath.prototype.discriminator = function(name, schema, value) { return this.caster.discriminators[name]; }; +/** + * Sets a default option for all SingleNestedPath instances. + * + * ####Example: + * + * // Make all numbers have option `min` equal to 0. + * mongoose.Schema.Embedded.set('required', true); + * + * @param {String} option - The option you'd like to set the value for + * @param {*} value - value for option + * @return {undefined} + * @function set + * @static + * @api public + */ + +SingleNestedPath.set = SchemaType.set; + /*! * ignore */ diff --git a/lib/schema/documentarray.js b/lib/schema/documentarray.js index 57e2fa441b0..c263fd66255 100644 --- a/lib/schema/documentarray.js +++ b/lib/schema/documentarray.js @@ -510,6 +510,24 @@ function scopePaths(array, fields, init) { return hasKeys && selected || undefined; } +/** + * Sets a default option for all DocumentArray instances. + * + * ####Example: + * + * // Make all numbers have option `min` equal to 0. + * mongoose.Schema.DocumentArray.set('_id', false); + * + * @param {String} option - The option you'd like to set the value for + * @param {*} value - value for option + * @return {undefined} + * @function set + * @static + * @api public + */ + +DocumentArrayPath.set = SchemaType.set; + /*! * Module exports. */ diff --git a/test/schema.documentarray.test.js b/test/schema.documentarray.test.js index 983bf9634de..70d35378510 100644 --- a/test/schema.documentarray.test.js +++ b/test/schema.documentarray.test.js @@ -98,4 +98,20 @@ describe('schema.documentarray', function() { assert.equal(doc.nested[0].length, 3); assert.equal(doc.nested[0][1].title, 'second'); }); + + it('supports `set()` (gh-8883)', function() { + mongoose.deleteModel(/Test/); + mongoose.Schema.Types.DocumentArray.set('_id', false); + + const Model = mongoose.model('Test', mongoose.Schema({ + arr: { type: [{ name: String }] } + })); + + const doc = new Model({ arr: [{ name: 'test' }] }); + + assert.equal(doc.arr.length, 1); + assert.ok(!doc.arr[0]._id); + + mongoose.Schema.Types.DocumentArray.set('_id', true); + }); }); diff --git a/test/schema.singlenestedpath.test.js b/test/schema.singlenestedpath.test.js index 5365776f587..80f55965ed7 100644 --- a/test/schema.singlenestedpath.test.js +++ b/test/schema.singlenestedpath.test.js @@ -143,4 +143,23 @@ describe('SingleNestedPath', function() { }); }); }); + + it('supports `set()` (gh-8883)', function() { + mongoose.deleteModel(/Test/); + mongoose.Schema.Types.Embedded.set('required', true); + + const Model = mongoose.model('Test', mongoose.Schema({ + nested: mongoose.Schema({ + test: String + }) + })); + + const doc = new Model({}); + + const err = doc.validateSync(); + assert.ok(err); + assert.ok(err.errors['nested']); + + mongoose.Schema.Types.Embedded.set('required', false); + }); }); \ No newline at end of file From 93f34245839bbbbc88720dd05bc78560cdc73f2b Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Sat, 13 Jun 2020 17:15:31 -0400 Subject: [PATCH 12/48] fix: fix tests --- lib/schema.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/schema.js b/lib/schema.js index cc721b074ee..b02ec83109f 100644 --- a/lib/schema.js +++ b/lib/schema.js @@ -938,7 +938,8 @@ Schema.prototype.interpretAsType = function(path, obj, options) { } if (this._userProvidedOptions.hasOwnProperty('_id')) { childSchemaOptions._id = this._userProvidedOptions._id; - } else if (Schema.Types.DocumentArray.defaultOptions._id != null) { + } else if (Schema.Types.DocumentArray.defaultOptions && + Schema.Types.DocumentArray.defaultOptions._id != null) { childSchemaOptions._id = Schema.Types.DocumentArray.defaultOptions._id; } From d3858d610648137137fc6340ae88a9694f16a7d5 Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Sat, 13 Jun 2020 18:09:24 -0400 Subject: [PATCH 13/48] fix: clean up test failures re: #8883 --- lib/schema.js | 1 + lib/schema/SingleNestedPath.js | 2 ++ lib/schema/documentarray.js | 2 ++ test/schema.documentarray.test.js | 2 +- test/schema.test.js | 8 ++++---- 5 files changed, 10 insertions(+), 5 deletions(-) diff --git a/lib/schema.js b/lib/schema.js index b02ec83109f..1461a95ac3a 100644 --- a/lib/schema.js +++ b/lib/schema.js @@ -936,6 +936,7 @@ Schema.prototype.interpretAsType = function(path, obj, options) { if (options.hasOwnProperty('typePojoToMixed')) { childSchemaOptions.typePojoToMixed = options.typePojoToMixed; } + if (this._userProvidedOptions.hasOwnProperty('_id')) { childSchemaOptions._id = this._userProvidedOptions._id; } else if (Schema.Types.DocumentArray.defaultOptions && diff --git a/lib/schema/SingleNestedPath.js b/lib/schema/SingleNestedPath.js index dc7f11ec5f8..5b54e9a86ea 100644 --- a/lib/schema/SingleNestedPath.js +++ b/lib/schema/SingleNestedPath.js @@ -316,6 +316,8 @@ SingleNestedPath.prototype.discriminator = function(name, schema, value) { * @api public */ +SingleNestedPath.defaultOptions = {}; + SingleNestedPath.set = SchemaType.set; /*! diff --git a/lib/schema/documentarray.js b/lib/schema/documentarray.js index c263fd66255..81ee5f79405 100644 --- a/lib/schema/documentarray.js +++ b/lib/schema/documentarray.js @@ -526,6 +526,8 @@ function scopePaths(array, fields, init) { * @api public */ +DocumentArrayPath.defaultOptions = {}; + DocumentArrayPath.set = SchemaType.set; /*! diff --git a/test/schema.documentarray.test.js b/test/schema.documentarray.test.js index 70d35378510..89937a538d1 100644 --- a/test/schema.documentarray.test.js +++ b/test/schema.documentarray.test.js @@ -112,6 +112,6 @@ describe('schema.documentarray', function() { assert.equal(doc.arr.length, 1); assert.ok(!doc.arr[0]._id); - mongoose.Schema.Types.DocumentArray.set('_id', true); + mongoose.Schema.Types.DocumentArray.defaultOptions = {}; }); }); diff --git a/test/schema.test.js b/test/schema.test.js index bbb82886605..462ce6c7bee 100644 --- a/test/schema.test.js +++ b/test/schema.test.js @@ -2001,13 +2001,13 @@ describe('schema', function() { }); assert.equal(schema.childSchemas.length, 2); - assert.equal(schema.childSchemas[0].schema, schema1); - assert.equal(schema.childSchemas[1].schema, schema2); + assert.strictEqual(schema.childSchemas[0].schema, schema1); + assert.strictEqual(schema.childSchemas[1].schema, schema2); schema = schema.clone(); assert.equal(schema.childSchemas.length, 2); - assert.equal(schema.childSchemas[0].schema, schema1); - assert.equal(schema.childSchemas[1].schema, schema2); + assert.strictEqual(schema.childSchemas[0].schema, schema1); + assert.strictEqual(schema.childSchemas[1].schema, schema2); done(); }); From 9609e87418b97064adc98bcaa47f85a09d899c06 Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Sun, 14 Jun 2020 14:07:20 -0400 Subject: [PATCH 14/48] feat(document): support `defaults` option to disable adding defaults to a single document Fix #8271 --- lib/document.js | 19 ++++++++++++------- lib/types/subdocument.js | 5 ++++- test/document.test.js | 21 +++++++++++++++++++++ 3 files changed, 37 insertions(+), 8 deletions(-) diff --git a/lib/document.js b/lib/document.js index e48c5a2b8b2..9b3c2fe81ac 100644 --- a/lib/document.js +++ b/lib/document.js @@ -55,7 +55,8 @@ const specialProperties = utils.specialProperties; * * @param {Object} obj the values to set * @param {Object} [fields] optional object containing the fields which were selected in the query returning this document and any populated paths data - * @param {Boolean} [skipId] bool, should we auto create an ObjectId _id + * @param {Object} [options] various configuration options for the document + * @param {Boolean} [options.defaults=true] if `false`, skip applying default values to this document. * @inherits NodeJS EventEmitter http://nodejs.org/api/events.html#events_class_events_eventemitter * @event `init`: Emitted on a document after it has been retrieved from the db and fully hydrated by Mongoose. * @event `save`: Emitted when the document is successfully saved @@ -67,7 +68,9 @@ function Document(obj, fields, skipId, options) { options = skipId; skipId = options.skipId; } - options = options || {}; + options = Object.assign({}, options); + const defaults = get(options, 'defaults', true); + options.defaults = defaults; // Support `browserDocument.js` syntax if (this.schema == null) { @@ -126,9 +129,11 @@ function Document(obj, fields, skipId, options) { // By default, defaults get applied **before** setting initial values // Re: gh-6155 - $__applyDefaults(this, fields, skipId, exclude, hasIncludedChildren, true, { - isNew: this.isNew - }); + if (defaults) { + $__applyDefaults(this, fields, skipId, exclude, hasIncludedChildren, true, { + isNew: this.isNew + }); + } } if (obj) { @@ -147,13 +152,13 @@ function Document(obj, fields, skipId, options) { // Function defaults get applied **after** setting initial values so they // see the full doc rather than an empty one, unless they opt out. // Re: gh-3781, gh-6155 - if (options.willInit) { + if (options.willInit && defaults) { EventEmitter.prototype.once.call(this, 'init', () => { $__applyDefaults(this, fields, skipId, exclude, hasIncludedChildren, false, options.skipDefaults, { isNew: this.isNew }); }); - } else { + } else if (defaults) { $__applyDefaults(this, fields, skipId, exclude, hasIncludedChildren, false, options.skipDefaults, { isNew: this.isNew }); diff --git a/lib/types/subdocument.js b/lib/types/subdocument.js index 521237aee72..63cd42c0c11 100644 --- a/lib/types/subdocument.js +++ b/lib/types/subdocument.js @@ -29,7 +29,10 @@ function Subdocument(value, fields, parent, skipId, options) { } if (parent != null) { // If setting a nested path, should copy isNew from parent re: gh-7048 - options = Object.assign({}, options, { isNew: parent.isNew }); + options = Object.assign({}, options, { + isNew: parent.isNew, + defaults: parent.$__.$options.defaults + }); } Document.call(this, value, fields, skipId, options); diff --git a/test/document.test.js b/test/document.test.js index 9983736d38a..8d3f6bcae64 100644 --- a/test/document.test.js +++ b/test/document.test.js @@ -8975,4 +8975,25 @@ describe('document', function() { const axl = new Person({ fullName: 'Axl Rose' }); assert.equal(axl.fullName, 'Axl Rose'); }); + + it('supports skipping defaults on a document (gh-8271)', function() { + const testSchema = new mongoose.Schema({ + testTopLevel: { type: String, default: 'foo' }, + testNested: { + prop: { type: String, default: 'bar' } + }, + testArray: [{ prop: { type: String, defualt: 'baz' } }], + testSingleNested: new Schema({ + prop: { type: String, default: 'qux' } + }) + }); + const Test = db.model('Test', testSchema); + + const doc = new Test({ testArray: [{}], testSingleNested: {} }, null, + { defaults: false }); + assert.ok(!doc.testTopLevel); + assert.ok(!doc.testNested.prop); + assert.ok(!doc.testArray[0].prop); + assert.ok(!doc.testSingleNested.prop); + }); }); From 24f6a29281498d9e7b4d7e61c8a167f5f20ee66e Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Sun, 14 Jun 2020 14:40:45 -0400 Subject: [PATCH 15/48] feat(aggregate): add `Aggregate#search()` for Atlas Text Search Fix #9115 --- lib/aggregate.js | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/lib/aggregate.js b/lib/aggregate.js index cc31f09b98d..d275c2ed4da 100644 --- a/lib/aggregate.js +++ b/lib/aggregate.js @@ -909,6 +909,32 @@ Aggregate.prototype.facet = function(options) { return this.append({ $facet: options }); }; +/** + * Helper for [Atlas Text Search](https://docs.atlas.mongodb.com/reference/atlas-search/tutorial/)'s + * `$search` stage. + * + * ####Example: + * + * Model.aggregate(). + * search({ + * text: { + * query: 'baseball', + * path: 'plot' + * } + * }); + * + * // Output: [{ plot: '...', title: '...' }] + * + * @param {Object} $search options + * @return {Aggregate} this + * @see $search https://docs.atlas.mongodb.com/reference/atlas-search/tutorial/ + * @api public + */ + +Aggregate.prototype.search = function(options) { + return this.append({ $search: options }); +}; + /** * Returns the current pipeline * From 78f5dbb18ab84b42df6e409a078c8d75f4c99d31 Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Mon, 15 Jun 2020 14:43:26 -0400 Subject: [PATCH 16/48] test: fix tests re: #9097 --- lib/helpers/setDefaultsOnInsert.js | 2 +- test/index.test.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/helpers/setDefaultsOnInsert.js b/lib/helpers/setDefaultsOnInsert.js index 747cb61ab8d..b061a8d1759 100644 --- a/lib/helpers/setDefaultsOnInsert.js +++ b/lib/helpers/setDefaultsOnInsert.js @@ -17,7 +17,7 @@ module.exports = function(filter, schema, castedDoc, options) { options = options || {}; const shouldSetDefaultsOnInsert = - options.hasOwnProperty('setDefaultsOnInsert') ? + options.setDefaultsOnInsert != null ? options.setDefaultsOnInsert : schema.base.options.setDefaultsOnInsert; diff --git a/test/index.test.js b/test/index.test.js index 993d69f78b5..b50577bc512 100644 --- a/test/index.test.js +++ b/test/index.test.js @@ -766,7 +766,7 @@ describe('mongoose module:', function() { }, { collection: 'movies_1' }); const Movie = db.model('Movie', schema); - + yield Movie.deleteMany({}); yield Movie.updateOne( {}, From 688d19d4c923c1ca2f83ec4b72795798fd7fc84d Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Mon, 15 Jun 2020 15:48:34 -0400 Subject: [PATCH 17/48] feat(connection): make `transaction()` reset versionKey if transaction fails Re: #8380 --- lib/connection.js | 11 ++++++++--- lib/plugins/trackTransaction.js | 17 ++++++++++++----- test/docs/transactions.test.js | 22 ++++++++++++++++++++++ 3 files changed, 42 insertions(+), 8 deletions(-) diff --git a/lib/connection.js b/lib/connection.js index e5b3187154b..73d71780fed 100644 --- a/lib/connection.js +++ b/lib/connection.js @@ -452,7 +452,7 @@ Connection.prototype.startSession = _wrapConnHelper(function startSession(option Connection.prototype.transaction = function transaction(fn) { return this.startSession().then(session => { - session[sessionNewDocuments] = []; + session[sessionNewDocuments] = new Map(); return session.withTransaction(() => fn(session)). then(res => { delete session[sessionNewDocuments]; @@ -461,8 +461,13 @@ Connection.prototype.transaction = function transaction(fn) { catch(err => { // If transaction was aborted, we need to reset newly // inserted documents' `isNew`. - for (const doc of session[sessionNewDocuments]) { - doc.isNew = true; + for (const [doc, state] of session[sessionNewDocuments].entries()) { + if (state.hasOwnProperty('isNew')) { + doc.isNew = state.isNew; + } + if (state.hasOwnProperty('versionKey')) { + doc.set(doc.schema.options.versionKey, state.versionKey); + } } delete session[sessionNewDocuments]; throw err; diff --git a/lib/plugins/trackTransaction.js b/lib/plugins/trackTransaction.js index c307f9b759a..52d9d231f67 100644 --- a/lib/plugins/trackTransaction.js +++ b/lib/plugins/trackTransaction.js @@ -4,10 +4,6 @@ const sessionNewDocuments = require('../helpers/symbols').sessionNewDocuments; module.exports = function trackTransaction(schema) { schema.pre('save', function() { - if (!this.isNew) { - return; - } - const session = this.$session(); if (session == null) { return; @@ -15,6 +11,17 @@ module.exports = function trackTransaction(schema) { if (session.transaction == null || session[sessionNewDocuments] == null) { return; } - session[sessionNewDocuments].push(this); + + if (!session[sessionNewDocuments].has(this)) { + const initialState = {}; + if (this.isNew) { + initialState.isNew = true; + } + if (this.schema.options.versionKey) { + initialState.versionKey = this.get(this.schema.options.versionKey); + } + + session[sessionNewDocuments].set(this, initialState); + } }); }; \ No newline at end of file diff --git a/test/docs/transactions.test.js b/test/docs/transactions.test.js index 73daab51e83..8d66c775fc3 100644 --- a/test/docs/transactions.test.js +++ b/test/docs/transactions.test.js @@ -10,6 +10,7 @@ const Schema = mongoose.Schema; describe('transactions', function() { let db; let _skipped = false; + this.timeout(10000); before(function() { if (!process.env.REPLICA_SET) { @@ -382,4 +383,25 @@ describe('transactions', function() { assert.ok(doc.isNew); }); }); + + it('can save document after aborted transaction (gh-8380)', function() { + return co(function*() { + const schema = Schema({ name: String, arr: [String] }); + + const Test = db.model('gh8380', schema); + + yield Test.createCollection(); + yield Test.create({ name: 'foo', arr: ['bar'] }); + const doc = yield Test.findOne(); + yield db. + transaction(session => co(function*() { + doc.arr.pop(); + yield doc.save({ session }); + throw new Error('Oops'); + })). + catch(err => assert.equal(err.message, 'Oops')); + doc.set('arr.0', 'qux'); + yield doc.save(); + }); + }); }); From e98b2f7a23bfaddeda31b50203e0d6d6b3abd243 Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Mon, 15 Jun 2020 15:51:33 -0400 Subject: [PATCH 18/48] test: fix tests for node v4 and v6 --- lib/connection.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/connection.js b/lib/connection.js index 73d71780fed..a06e44dfd1f 100644 --- a/lib/connection.js +++ b/lib/connection.js @@ -461,7 +461,8 @@ Connection.prototype.transaction = function transaction(fn) { catch(err => { // If transaction was aborted, we need to reset newly // inserted documents' `isNew`. - for (const [doc, state] of session[sessionNewDocuments].entries()) { + for (const doc of session[sessionNewDocuments].keys()) { + const state = session[sessionNewDocuments].get(doc); if (state.hasOwnProperty('isNew')) { doc.isNew = state.isNew; } From eeb9c9e320116851c7e176efbb009e3caad80e1d Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Tue, 30 Jun 2020 17:28:24 -0400 Subject: [PATCH 19/48] feat(query): add overwriteDiscriminatorKey option that allows changing the discriminator key in `findOneAndUpdate()`, `updateOne()`, etc. Fix #6087 --- lib/query.js | 21 +++++++++++++++++++-- test/model.update.test.js | 29 +++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 2 deletions(-) diff --git a/lib/query.js b/lib/query.js index dbdd0f2ce8a..fec883f175c 100644 --- a/lib/query.js +++ b/lib/query.js @@ -1293,7 +1293,8 @@ Query.prototype.getOptions = function() { * - [writeConcern](https://docs.mongodb.com/manual/reference/method/db.collection.update/) * - [timestamps](https://mongoosejs.com/docs/guide.html#timestamps): If `timestamps` is set in the schema, set this option to `false` to skip timestamps for that particular update. Has no effect if `timestamps` is not enabled in the schema options. * - omitUndefined: delete any properties whose value is `undefined` when casting an update. In other words, if this is set, Mongoose will delete `baz` from the update in `Model.updateOne({}, { foo: 'bar', baz: undefined })` before sending the update to the server. - * + * - overwriteDiscriminatorKey: allow setting the discriminator key in the update. Will use the correct discriminator schema if the update changes the discriminator key. + * * The following options are only for `find()`, `findOne()`, `findById()`, `findOneAndUpdate()`, and `findByIdAndUpdate()`: * * - [lean](./api.html#query_Query-lean) @@ -1361,6 +1362,10 @@ Query.prototype.setOptions = function(options, overwrite) { this._mongooseOptions.setDefaultsOnInsert = options.setDefaultsOnInsert; delete options.setDefaultsOnInsert; } + if ('overwriteDiscriminatorKey' in options) { + this._mongooseOptions.overwriteDiscriminatorKey = options.overwriteDiscriminatorKey; + delete options.overwriteDiscriminatorKey; + } return Query.base.setOptions.call(this, options); }; @@ -4485,6 +4490,19 @@ Query.prototype._post = function(fn) { Query.prototype._castUpdate = function _castUpdate(obj, overwrite) { let strict; + let schema = this.schema; + + const discriminatorKey = schema.options.discriminatorKey; + const baseSchema = schema._baseSchema ? schema._baseSchema : schema; + if (this._mongooseOptions.overwriteDiscriminatorKey && + obj[discriminatorKey] != null && + baseSchema.discriminators) { + const _schema = baseSchema.discriminators[obj[discriminatorKey]]; + if (_schema != null) { + schema = _schema; + } + } + if ('strict' in this._mongooseOptions) { strict = this._mongooseOptions.strict; } else if (this.schema && this.schema.options) { @@ -4508,7 +4526,6 @@ Query.prototype._castUpdate = function _castUpdate(obj, overwrite) { upsert = this.options.upsert; } - let schema = this.schema; const filter = this._conditions; if (schema != null && utils.hasUserDefinedProperty(filter, schema.options.discriminatorKey) && diff --git a/test/model.update.test.js b/test/model.update.test.js index 8e8c517d865..a465a379287 100644 --- a/test/model.update.test.js +++ b/test/model.update.test.js @@ -3478,4 +3478,33 @@ describe('model: updateOne: ', function() { }); }); }); + + describe('overwriteDiscriminatorKey', function() { + it('allows changing discriminator key in update (gh-6087)', function() { + const baseSchema = new Schema({}, { discriminatorKey: 'type' }); + const baseModel = db.model('Test', baseSchema); + + const aSchema = Schema({ aThing: Number }, { _id: false, id: false }); + const aModel = baseModel.discriminator('A', aSchema); + + const bSchema = new Schema({ bThing: String }, { _id: false, id: false }); + const bModel = baseModel.discriminator('B', bSchema); + + return co(function*() { + // Model is created as a type A + let doc = yield baseModel.create({ type: 'A', aThing: 1 }); + + yield aModel.updateOne( + { _id: doc._id }, + { type: 'B', bThing: 'two' }, + { runValidators: true, overwriteDiscriminatorKey: true } + ); + + doc = yield baseModel.findById(doc); + assert.equal(doc.type, 'B'); + assert.ok(doc instanceof bModel); + assert.equal(doc.bThing, 'two'); + }); + }); + }); }); \ No newline at end of file From 655114b9a80923eb4f7941f426a9b408567684d6 Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Tue, 30 Jun 2020 17:50:01 -0400 Subject: [PATCH 20/48] style: fix lint --- lib/query.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/query.js b/lib/query.js index fec883f175c..3cc41c64562 100644 --- a/lib/query.js +++ b/lib/query.js @@ -1294,7 +1294,7 @@ Query.prototype.getOptions = function() { * - [timestamps](https://mongoosejs.com/docs/guide.html#timestamps): If `timestamps` is set in the schema, set this option to `false` to skip timestamps for that particular update. Has no effect if `timestamps` is not enabled in the schema options. * - omitUndefined: delete any properties whose value is `undefined` when casting an update. In other words, if this is set, Mongoose will delete `baz` from the update in `Model.updateOne({}, { foo: 'bar', baz: undefined })` before sending the update to the server. * - overwriteDiscriminatorKey: allow setting the discriminator key in the update. Will use the correct discriminator schema if the update changes the discriminator key. - * + * * The following options are only for `find()`, `findOne()`, `findById()`, `findOneAndUpdate()`, and `findByIdAndUpdate()`: * * - [lean](./api.html#query_Query-lean) From 156de0c850ecc916b7d4e3ef3dc1844ee566cabf Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Wed, 1 Jul 2020 19:07:51 -0400 Subject: [PATCH 21/48] feat(connection): add `getClient()` and `setClient()` function for interacting with a connection's underlying MongoClient instance Fix #9164 --- lib/connection.js | 285 ++++++++++++++++++++++++---------------- test/connection.test.js | 17 +++ 2 files changed, 187 insertions(+), 115 deletions(-) diff --git a/lib/connection.js b/lib/connection.js index d8e60bd6e00..31c35af3ee4 100644 --- a/lib/connection.js +++ b/lib/connection.js @@ -753,18 +753,6 @@ Connection.prototype.openUri = function(uri, options, callback) { }); }); - const _handleReconnect = () => { - // If we aren't disconnected, we assume this reconnect is due to a - // socket timeout. If there's no activity on a socket for - // `socketTimeoutMS`, the driver will attempt to reconnect and emit - // this event. - if (_this.readyState !== STATES.connected) { - _this.readyState = STATES.connected; - _this.emit('reconnect'); - _this.emit('reconnected'); - } - }; - const promise = new Promise((resolve, reject) => { if (_this.client != null) { _this.client.close(); @@ -778,111 +766,9 @@ Connection.prototype.openUri = function(uri, options, callback) { return reject(error); } - const db = dbName != null ? client.db(dbName) : client.db(); - _this.db = db; - - // `useUnifiedTopology` events - const type = get(db, 's.topology.s.description.type', ''); - if (options.useUnifiedTopology) { - if (type === 'Single') { - const server = Array.from(db.s.topology.s.servers.values())[0]; - - server.s.topology.on('serverHeartbeatSucceeded', () => { - _handleReconnect(); - }); - server.s.pool.on('reconnect', () => { - _handleReconnect(); - }); - client.on('serverDescriptionChanged', ev => { - const newDescription = ev.newDescription; - if (newDescription.type === 'Standalone') { - _handleReconnect(); - } else { - _this.readyState = STATES.disconnected; - } - }); - } else if (type.startsWith('ReplicaSet')) { - client.on('topologyDescriptionChanged', ev => { - // Emit disconnected if we've lost connectivity to _all_ servers - // in the replica set. - const description = ev.newDescription; - const servers = Array.from(ev.newDescription.servers.values()); - const allServersDisconnected = description.type === 'ReplicaSetNoPrimary' && - servers.reduce((cur, d) => cur || d.type === 'Unknown', false); - if (_this.readyState === STATES.connected && allServersDisconnected) { - // Implicitly emits 'disconnected' - _this.readyState = STATES.disconnected; - } else if (_this.readyState === STATES.disconnected && !allServersDisconnected) { - _handleReconnect(); - } - }); - - db.on('close', function() { - const type = get(db, 's.topology.s.description.type', ''); - if (type !== 'ReplicaSetWithPrimary') { - // Implicitly emits 'disconnected' - _this.readyState = STATES.disconnected; - } - }); - } - } - - // Backwards compat for mongoose 4.x - db.on('reconnect', function() { - _handleReconnect(); - }); - db.s.topology.on('reconnectFailed', function() { - _this.emit('reconnectFailed'); - }); - - if (!options.useUnifiedTopology) { - db.s.topology.on('left', function(data) { - _this.emit('left', data); - }); - } - db.s.topology.on('joined', function(data) { - _this.emit('joined', data); - }); - db.s.topology.on('fullsetup', function(data) { - _this.emit('fullsetup', data); - }); - if (get(db, 's.topology.s.coreTopology.s.pool') != null) { - db.s.topology.s.coreTopology.s.pool.on('attemptReconnect', function() { - _this.emit('attemptReconnect'); - }); - } - if (!options.useUnifiedTopology || !type.startsWith('ReplicaSet')) { - db.on('close', function() { - // Implicitly emits 'disconnected' - _this.readyState = STATES.disconnected; - }); - } - - if (!options.useUnifiedTopology) { - client.on('left', function() { - if (_this.readyState === STATES.connected && - get(db, 's.topology.s.coreTopology.s.replicaSetState.topologyType') === 'ReplicaSetNoPrimary') { - _this.readyState = STATES.disconnected; - } - }); - } - - db.on('timeout', function() { - _this.emit('timeout'); - }); - - delete _this.then; - delete _this.catch; - _this.readyState = STATES.connected; - - for (const i in _this.collections) { - if (utils.object.hasOwnProperty(_this.collections, i)) { - _this.collections[i].onOpen(); - } - } + _setClient(_this, client, options, dbName); resolve(_this); - _this.emit('open'); }); }); @@ -916,6 +802,125 @@ Connection.prototype.openUri = function(uri, options, callback) { return this; }; +function _setClient(conn, client, options, dbName) { + const db = dbName != null ? client.db(dbName) : client.db(); + conn.db = db; + conn.client = client; + + const _handleReconnect = () => { + // If we aren't disconnected, we assume this reconnect is due to a + // socket timeout. If there's no activity on a socket for + // `socketTimeoutMS`, the driver will attempt to reconnect and emit + // this event. + if (conn.readyState !== STATES.connected) { + conn.readyState = STATES.connected; + conn.emit('reconnect'); + conn.emit('reconnected'); + } + }; + + // `useUnifiedTopology` events + const type = get(db, 's.topology.s.description.type', ''); + if (options.useUnifiedTopology) { + if (type === 'Single') { + const server = Array.from(db.s.topology.s.servers.values())[0]; + server.s.topology.on('serverHeartbeatSucceeded', () => { + _handleReconnect(); + }); + server.s.pool.on('reconnect', () => { + _handleReconnect(); + }); + client.on('serverDescriptionChanged', ev => { + const newDescription = ev.newDescription; + if (newDescription.type === 'Standalone') { + _handleReconnect(); + } else { + conn.readyState = STATES.disconnected; + } + }); + } else if (type.startsWith('ReplicaSet')) { + client.on('topologyDescriptionChanged', ev => { + // Emit disconnected if we've lost connectivity to _all_ servers + // in the replica set. + const description = ev.newDescription; + const servers = Array.from(ev.newDescription.servers.values()); + const allServersDisconnected = description.type === 'ReplicaSetNoPrimary' && + servers.reduce((cur, d) => cur || d.type === 'Unknown', false); + if (conn.readyState === STATES.connected && allServersDisconnected) { + // Implicitly emits 'disconnected' + conn.readyState = STATES.disconnected; + } else if (conn.readyState === STATES.disconnected && !allServersDisconnected) { + _handleReconnect(); + } + }); + + db.on('close', function() { + const type = get(db, 's.topology.s.description.type', ''); + if (type !== 'ReplicaSetWithPrimary') { + // Implicitly emits 'disconnected' + conn.readyState = STATES.disconnected; + } + }); + } + } + + // Backwards compat for mongoose 4.x + db.on('reconnect', function() { + _handleReconnect(); + }); + db.s.topology.on('reconnectFailed', function() { + conn.emit('reconnectFailed'); + }); + + if (!options.useUnifiedTopology) { + db.s.topology.on('left', function(data) { + conn.emit('left', data); + }); + } + db.s.topology.on('joined', function(data) { + conn.emit('joined', data); + }); + db.s.topology.on('fullsetup', function(data) { + conn.emit('fullsetup', data); + }); + if (get(db, 's.topology.s.coreTopology.s.pool') != null) { + db.s.topology.s.coreTopology.s.pool.on('attemptReconnect', function() { + conn.emit('attemptReconnect'); + }); + } + if (!options.useUnifiedTopology || !type.startsWith('ReplicaSet')) { + db.on('close', function() { + // Implicitly emits 'disconnected' + conn.readyState = STATES.disconnected; + }); + } + + if (!options.useUnifiedTopology) { + client.on('left', function() { + if (conn.readyState === STATES.connected && + get(db, 's.topology.s.coreTopology.s.replicaSetState.topologyType') === 'ReplicaSetNoPrimary') { + conn.readyState = STATES.disconnected; + } + }); + } + + db.on('timeout', function() { + conn.emit('timeout'); + }); + + delete conn.then; + delete conn.catch; + conn.readyState = STATES.connected; + + for (const i in conn.collections) { + if (utils.object.hasOwnProperty(conn.collections, i)) { + conn.collections[i].onOpen(); + } + } + + conn.emit('open'); +} + /*! * ignore */ @@ -1330,6 +1335,56 @@ Connection.prototype.optionsProvideAuthenticationData = function(options) { ((options.pass) || this.authMechanismDoesNotRequirePassword()); }; +/** + * Returns the [MongoDB driver `MongoClient`](http://mongodb.github.io/node-mongodb-native/3.5/api/MongoClient.html) instance + * that this connection uses to talk to MongoDB. + * + * ####Example: + * const conn = await mongoose.createConnection('mongodb://localhost:27017/test'); + * + * conn.getClient(); // MongoClient { ... } + * + * @api public + * @return {MongoClient} + */ + +Connection.prototype.getClient = function getClient() { + return this.client; +}; + +/** + * Set the [MongoDB driver `MongoClient`](http://mongodb.github.io/node-mongodb-native/3.5/api/MongoClient.html) instance + * that this connection uses to talk to MongoDB. This is useful if you already have a MongoClient instance, and want to + * reuse it. + * + * ####Example: + * const client = await mongodb.MongoClient.connect('mongodb://localhost:27017/test'); + * + * const conn = mongoose.createConnection().setClient(client); + * + * conn.getClient(); // MongoClient { ... } + * conn.readyState; // 1, means 'CONNECTED' + * + * @api public + * @return {Connection} this + */ + +Connection.prototype.setClient = function setClient(client) { + if (!(client instanceof mongodb.MongoClient)) { + throw new MongooseError('Must call `setClient()` with an instance of MongoClient'); + } + if (this.client != null || this.readyState !== STATES.disconnected) { + throw new MongooseError('Cannot call `setClient()` on a connection that is already connected.'); + } + if (!client.isConnected()) { + throw new MongooseError('Cannot call `setClient()` with a MongoClient that is not connected.'); + } + + _setClient(this, client, { useUnifiedTopology: client.s.options.useUnifiedTopology }, client.s.options.dbName); + + return this; +}; + /** * Switches to a different database using the same connection pool. * diff --git a/test/connection.test.js b/test/connection.test.js index f259e6d590f..e0a81a31ee1 100644 --- a/test/connection.test.js +++ b/test/connection.test.js @@ -10,6 +10,7 @@ const Promise = require('bluebird'); const Q = require('q'); const assert = require('assert'); const co = require('co'); +const mongodb = require('mongodb'); const server = require('./common').server; const mongoose = start.mongoose; @@ -1185,4 +1186,20 @@ describe('connections:', function() { assert.equal(db2.config.useCreateIndex, true); }); }); + + it('allows setting client on a disconnected connection (gh-9164)', function() { + return co(function*() { + const client = yield mongodb.MongoClient.connect('mongodb://localhost:27017/mongoose_test', { + useNewUrlParser: true, + useUnifiedTopology: true + }); + const conn = mongoose.createConnection().setClient(client); + + assert.equal(conn.readyState, 1); + + yield conn.createCollection('test'); + const res = yield conn.dropCollection('test'); + assert.ok(res); + }); + }); }); From 1bca5d10931c4508555120ccbf6ee2f6adfc39f8 Mon Sep 17 00:00:00 2001 From: Hafez Date: Thu, 2 Jul 2020 04:37:51 +0200 Subject: [PATCH 22/48] docs(model): fix typo --- lib/model.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/model.js b/lib/model.js index 63eb74c3a15..8e46b28ca25 100644 --- a/lib/model.js +++ b/lib/model.js @@ -2784,7 +2784,7 @@ Model.findByIdAndDelete = function(id, options, callback) { * @param {Object} filter Replace the first document that matches this filter * @param {Object} [replacement] Replace with this document * @param {Object} [options] optional see [`Query.prototype.setOptions()`](http://mongoosejs.com/docs/api.html#query_Query-setOptions) - * @param {Boolean} [options.new=false] By default, `findOneAndUpdate()` returns the document as it was **before** `update` was applied. If you set `new: true`, `findOneAndUpdate()` will instead give you the object after `update` was applied. + * @param {Boolean} [options.new=false] By default, `findOneAndReplace()` returns the document as it was **before** `update` was applied. If you set `new: true`, `findOneAndReplace()` will instead give you the object after `update` was applied. * @param {Object} [options.lean] if truthy, mongoose will return the document as a plain JavaScript object rather than a mongoose document. See [`Query.lean()`](/docs/api.html#query_Query-lean) and [the Mongoose lean tutorial](/docs/tutorials/lean.html). * @param {ClientSession} [options.session=null] The session associated with this query. See [transactions docs](/docs/transactions.html). * @param {Boolean|String} [options.strict] overwrites the schema's [strict mode option](http://mongoosejs.com/docs/guide.html#strict) From 2398073c2ae3d130ea19f0d8539b759ecc60fd8d Mon Sep 17 00:00:00 2001 From: Hafez Date: Thu, 2 Jul 2020 05:01:21 +0200 Subject: [PATCH 23/48] test: repro #9183 --- test/model.test.js | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/test/model.test.js b/test/model.test.js index 9d674f158c5..55504f3ad3a 100644 --- a/test/model.test.js +++ b/test/model.test.js @@ -6839,4 +6839,36 @@ describe('Model', function() { ); }); }); + + describe('defaultNewOnFindAndUpdate', function() { + const originalValue = mongoose.get('defaultNewOnFindAndUpdate'); + beforeEach(() => { + mongoose.set('defaultNewOnFindAndUpdate', true); + }); + + afterEach(() => { + mongoose.set('defaultNewOnFindAndUpdate', originalValue); + }); + + it('Setting `defaultNewOnFindAndUpdate` works (gh-9183)', function() { + return co(function*() { + const userSchema = new Schema({ + name: { type: String } + }); + + const User = db.model('User', userSchema); + + const createdUser = yield User.create({ name: 'Hafez' }); + + const user1 = yield User.findOneAndUpdate({ _id: createdUser._id }, { name: 'Hafez1' }); + assert.equal(user1.name, 'Hafez1'); + + const user2 = yield User.findByIdAndUpdate(createdUser._id, { name: 'Hafez2' }); + assert.equal(user2.name, 'Hafez2'); + + const user3 = yield User.findOneAndReplace({ _id: createdUser._id }, { name: 'Hafez3' }); + assert.equal(user3.name, 'Hafez3'); + }); + }); + }); }); From d3bfee429d84107bcfcd83c24677f6b267b8f517 Mon Sep 17 00:00:00 2001 From: Hafez Date: Thu, 2 Jul 2020 05:07:13 +0200 Subject: [PATCH 24/48] feat(base): add option to set defaultNewOnFindAndUpdate --- lib/query.js | 33 +++++++++++++++++++++------------ lib/validoptions.js | 5 +++-- 2 files changed, 24 insertions(+), 14 deletions(-) diff --git a/lib/query.js b/lib/query.js index 3cc41c64562..4c36c2d0c2b 100644 --- a/lib/query.js +++ b/lib/query.js @@ -3004,20 +3004,23 @@ Query.prototype.findOneAndUpdate = function(criteria, doc, options, callback) { this._mergeUpdate(doc); } - if (options) { - options = utils.clone(options); - if (options.projection) { - this.select(options.projection); - delete options.projection; - } - if (options.fields) { - this.select(options.fields); - delete options.fields; - } + options = options ? utils.clone(options) : {}; - this.setOptions(options); + if (options.projection) { + this.select(options.projection); + delete options.projection; + } + if (options.fields) { + this.select(options.fields); + delete options.fields; + } + + if (Object.prototype.hasOwnProperty.call(options, 'new') === false) { + options.new = get(this, 'model.base.options.defaultNewOnFindAndUpdate'); } + this.setOptions(options); + if (!callback) { return this; } @@ -3335,7 +3338,13 @@ Query.prototype.findOneAndReplace = function(filter, replacement, options, callb this._mergeUpdate(replacement); } - options && this.setOptions(options); + options = options || {}; + + if (Object.prototype.hasOwnProperty.call(options, 'new') === false) { + options.new = get(this, 'model.base.options.defaultNewOnFindAndUpdate'); + } + + this.setOptions(options); if (!callback) { return this; diff --git a/lib/validoptions.js b/lib/validoptions.js index a9c8a352d23..39c037f2e79 100644 --- a/lib/validoptions.js +++ b/lib/validoptions.js @@ -12,6 +12,7 @@ const VALID_OPTIONS = Object.freeze([ 'bufferCommands', 'cloneSchemas', 'debug', + 'defaultNewOnFindAndUpdate', 'maxTimeMS', 'objectIdGetter', 'runValidators', @@ -21,12 +22,12 @@ const VALID_OPTIONS = Object.freeze([ 'strictQuery', 'toJSON', 'toObject', + 'typePojoToMixed', 'useCreateIndex', 'useFindAndModify', 'useNewUrlParser', 'usePushEach', - 'useUnifiedTopology', - 'typePojoToMixed' + 'useUnifiedTopology' ]); module.exports = VALID_OPTIONS; From 92610fc55c6e894dc2d44d3d05184933c29e06db Mon Sep 17 00:00:00 2001 From: Hafez Date: Thu, 2 Jul 2020 05:14:05 +0200 Subject: [PATCH 25/48] docs(base): add defaultNewOnFindAndUpdateOrReplace as a valid option this also changes option name defaultNewOnFindAndUpdate -> defaultNewOnFindAndUpdateOrReplace --- lib/index.js | 1 + lib/query.js | 4 ++-- lib/validoptions.js | 2 +- test/model.test.js | 10 +++++----- 4 files changed, 9 insertions(+), 8 deletions(-) diff --git a/lib/index.js b/lib/index.js index a72fdddffc6..02cab261e52 100644 --- a/lib/index.js +++ b/lib/index.js @@ -147,6 +147,7 @@ Mongoose.prototype.driver = require('./driver'); * * Currently supported options are: * - 'debug': If `true`, prints the operations mongoose sends to MongoDB to the console. If a writable stream is passed, it will log to that stream, without colorization. If a callback function is passed, it will receive the collection name, the method name, then all arugments passed to the method. For example, if you wanted to replicate the default logging, you could output from the callback `Mongoose: ${collectionName}.${methodName}(${methodArgs.join(', ')})`. + * - 'defaultNewOnFindAndUpdateOrReplace': If `true`, changes the default `new` option to `findOneAndUpdate()`, `findByIdAndUpdate` and `findOneAndReplace()` to true. * - 'bufferCommands': enable/disable mongoose's buffering mechanism for all connections and models * - 'useCreateIndex': false by default. Set to `true` to make Mongoose's default index build use `createIndex()` instead of `ensureIndex()` to avoid deprecation warnings from the MongoDB driver. * - 'useFindAndModify': true by default. Set to `false` to make `findOneAndUpdate()` and `findOneAndRemove()` use native `findOneAndUpdate()` rather than `findAndModify()`. diff --git a/lib/query.js b/lib/query.js index 4c36c2d0c2b..1bf11fa6265 100644 --- a/lib/query.js +++ b/lib/query.js @@ -3016,7 +3016,7 @@ Query.prototype.findOneAndUpdate = function(criteria, doc, options, callback) { } if (Object.prototype.hasOwnProperty.call(options, 'new') === false) { - options.new = get(this, 'model.base.options.defaultNewOnFindAndUpdate'); + options.new = get(this, 'model.base.options.defaultNewOnFindAndUpdateOrReplace'); } this.setOptions(options); @@ -3341,7 +3341,7 @@ Query.prototype.findOneAndReplace = function(filter, replacement, options, callb options = options || {}; if (Object.prototype.hasOwnProperty.call(options, 'new') === false) { - options.new = get(this, 'model.base.options.defaultNewOnFindAndUpdate'); + options.new = get(this, 'model.base.options.defaultNewOnFindAndUpdateOrReplace'); } this.setOptions(options); diff --git a/lib/validoptions.js b/lib/validoptions.js index 39c037f2e79..7b092b2fefb 100644 --- a/lib/validoptions.js +++ b/lib/validoptions.js @@ -12,7 +12,7 @@ const VALID_OPTIONS = Object.freeze([ 'bufferCommands', 'cloneSchemas', 'debug', - 'defaultNewOnFindAndUpdate', + 'defaultNewOnFindAndUpdateOrReplace', 'maxTimeMS', 'objectIdGetter', 'runValidators', diff --git a/test/model.test.js b/test/model.test.js index 96263769807..1b0fb28e503 100644 --- a/test/model.test.js +++ b/test/model.test.js @@ -6869,17 +6869,17 @@ describe('Model', function() { }); }); - describe('defaultNewOnFindAndUpdate', function() { - const originalValue = mongoose.get('defaultNewOnFindAndUpdate'); + describe('defaultNewOnFindAndUpdateOrReplace', function() { + const originalValue = mongoose.get('defaultNewOnFindAndUpdateOrReplace'); beforeEach(() => { - mongoose.set('defaultNewOnFindAndUpdate', true); + mongoose.set('defaultNewOnFindAndUpdateOrReplace', true); }); afterEach(() => { - mongoose.set('defaultNewOnFindAndUpdate', originalValue); + mongoose.set('defaultNewOnFindAndUpdateOrReplace', originalValue); }); - it('Setting `defaultNewOnFindAndUpdate` works (gh-9183)', function() { + it('Setting `defaultNewOnFindAndUpdateOrReplace` works (gh-9183)', function() { return co(function*() { const userSchema = new Schema({ name: { type: String } From 263f1c47e67a6154f64afe3bd52e6c13fb47d621 Mon Sep 17 00:00:00 2001 From: Hafez Date: Thu, 2 Jul 2020 05:26:19 +0200 Subject: [PATCH 26/48] fix(query): set default option only if it's not null --- lib/query.js | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/lib/query.js b/lib/query.js index 1bf11fa6265..09bc76a549f 100644 --- a/lib/query.js +++ b/lib/query.js @@ -3015,8 +3015,10 @@ Query.prototype.findOneAndUpdate = function(criteria, doc, options, callback) { delete options.fields; } - if (Object.prototype.hasOwnProperty.call(options, 'new') === false) { - options.new = get(this, 'model.base.options.defaultNewOnFindAndUpdateOrReplace'); + + const defaultNew = get(this, 'model.base.options.defaultNewOnFindAndUpdateOrReplace'); + if (defaultNew != null && Object.prototype.hasOwnProperty.call(options, 'new') === false) { + options.new = defaultNew; } this.setOptions(options); @@ -3340,8 +3342,9 @@ Query.prototype.findOneAndReplace = function(filter, replacement, options, callb options = options || {}; - if (Object.prototype.hasOwnProperty.call(options, 'new') === false) { - options.new = get(this, 'model.base.options.defaultNewOnFindAndUpdateOrReplace'); + const defaultNew = get(this, 'model.base.options.defaultNewOnFindAndUpdateOrReplace'); + if (defaultNew != null && Object.prototype.hasOwnProperty.call(options, 'new') === false) { + options.new = defaultNew; } this.setOptions(options); From 7613aed85dc40a1abb0708e84d7afd8101e01dda Mon Sep 17 00:00:00 2001 From: Hafez Date: Thu, 2 Jul 2020 05:31:11 +0200 Subject: [PATCH 27/48] treat `new` undefined as not mentioned on findOneAndUpdate --- lib/query.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/query.js b/lib/query.js index 09bc76a549f..447b9aab9d2 100644 --- a/lib/query.js +++ b/lib/query.js @@ -3017,7 +3017,7 @@ Query.prototype.findOneAndUpdate = function(criteria, doc, options, callback) { const defaultNew = get(this, 'model.base.options.defaultNewOnFindAndUpdateOrReplace'); - if (defaultNew != null && Object.prototype.hasOwnProperty.call(options, 'new') === false) { + if (defaultNew != null && options.new == null) { options.new = defaultNew; } @@ -3343,7 +3343,7 @@ Query.prototype.findOneAndReplace = function(filter, replacement, options, callb options = options || {}; const defaultNew = get(this, 'model.base.options.defaultNewOnFindAndUpdateOrReplace'); - if (defaultNew != null && Object.prototype.hasOwnProperty.call(options, 'new') === false) { + if (defaultNew != null && options.new == null) { options.new = defaultNew; } From 86f0c8413807f7484a8347de51eadc6a30e3fb98 Mon Sep 17 00:00:00 2001 From: Hafez Date: Thu, 2 Jul 2020 06:07:09 +0200 Subject: [PATCH 28/48] test(model): add test case verifying defaultNewOnFindAndUpdateOrReplace can be overwritten --- test/model.test.js | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/test/model.test.js b/test/model.test.js index 1b0fb28e503..a10d69346c4 100644 --- a/test/model.test.js +++ b/test/model.test.js @@ -6869,7 +6869,7 @@ describe('Model', function() { }); }); - describe('defaultNewOnFindAndUpdateOrReplace', function() { + describe('defaultNewOnFindAndUpdateOrReplace (gh-9183)', function() { const originalValue = mongoose.get('defaultNewOnFindAndUpdateOrReplace'); beforeEach(() => { mongoose.set('defaultNewOnFindAndUpdateOrReplace', true); @@ -6879,7 +6879,7 @@ describe('Model', function() { mongoose.set('defaultNewOnFindAndUpdateOrReplace', originalValue); }); - it('Setting `defaultNewOnFindAndUpdateOrReplace` works (gh-9183)', function() { + it('Setting `defaultNewOnFindAndUpdateOrReplace` works', function() { return co(function*() { const userSchema = new Schema({ name: { type: String } @@ -6899,5 +6899,26 @@ describe('Model', function() { assert.equal(user3.name, 'Hafez3'); }); }); + + it('`defaultNewOnFindAndUpdateOrReplace` can be overwritten', function() { + return co(function*() { + const userSchema = new Schema({ + name: { type: String } + }); + + const User = db.model('User', userSchema); + + const createdUser = yield User.create({ name: 'Hafez' }); + + const user1 = yield User.findOneAndUpdate({ _id: createdUser._id }, { name: 'Hafez1' }, { new: false }); + assert.equal(user1.name, 'Hafez'); + + const user2 = yield User.findByIdAndUpdate(createdUser._id, { name: 'Hafez2' }, { new: false }); + assert.equal(user2.name, 'Hafez1'); + + const user3 = yield User.findOneAndReplace({ _id: createdUser._id }, { name: 'Hafez3' }, { new: false }); + assert.equal(user3.name, 'Hafez2'); + }); + }); }); }); From 0edb94dfdbb484b8557e3651305867e38bfd8c4c Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Thu, 2 Jul 2020 16:54:31 -0400 Subject: [PATCH 29/48] feat(connection): make transaction() helper reset array atomics after failed transaction Fix #8380 --- lib/connection.js | 14 +++++++ lib/plugins/trackTransaction.js | 66 ++++++++++++++++++++++++++++++++- test/docs/transactions.test.js | 29 ++++++++++++--- 3 files changed, 102 insertions(+), 7 deletions(-) diff --git a/lib/connection.js b/lib/connection.js index 31c35af3ee4..6e34ff12bbf 100644 --- a/lib/connection.js +++ b/lib/connection.js @@ -22,6 +22,7 @@ const utils = require('./utils'); const parseConnectionString = require('mongodb/lib/core').parseConnectionString; +const arrayAtomicsSymbol = require('./helpers/symbols').arrayAtomicsSymbol; const sessionNewDocuments = require('./helpers/symbols').sessionNewDocuments; let id = 0; @@ -469,6 +470,19 @@ Connection.prototype.transaction = function transaction(fn) { if (state.hasOwnProperty('versionKey')) { doc.set(doc.schema.options.versionKey, state.versionKey); } + + for (const path of state.modifiedPaths) { + doc.$__.activePaths.paths[path] = 'modify'; + doc.$__.activePaths.states.modify[path] = true; + } + + for (const path of state.atomics.keys()) { + const val = doc.$__getValue(path); + if (val == null) { + continue; + } + val[arrayAtomicsSymbol] = state.atomics.get(path); + } } delete session[sessionNewDocuments]; throw err; diff --git a/lib/plugins/trackTransaction.js b/lib/plugins/trackTransaction.js index 52d9d231f67..4a7ddc45c5d 100644 --- a/lib/plugins/trackTransaction.js +++ b/lib/plugins/trackTransaction.js @@ -1,5 +1,6 @@ 'use strict'; +const arrayAtomicsSymbol = require('../helpers/symbols').arrayAtomicsSymbol; const sessionNewDocuments = require('../helpers/symbols').sessionNewDocuments; module.exports = function trackTransaction(schema) { @@ -21,7 +22,70 @@ module.exports = function trackTransaction(schema) { initialState.versionKey = this.get(this.schema.options.versionKey); } + initialState.modifiedPaths = new Set(Object.keys(this.$__.activePaths.states.modify)); + initialState.atomics = _getAtomics(this); + session[sessionNewDocuments].set(this, initialState); + } else { + const state = session[sessionNewDocuments].get(this); + + for (const path of Object.keys(this.$__.activePaths.states.modify)) { + state.modifiedPaths.add(path); + } + state.atomics = _getAtomics(this, state.atomics); } }); -}; \ No newline at end of file +}; + +function _getAtomics(doc, previous) { + const pathToAtomics = new Map(); + previous = previous || new Map(); + + const pathsToCheck = Object.keys(doc.$__.activePaths.init).concat(Object.keys(doc.$__.activePaths.modify)); + + for (const path of pathsToCheck) { + const val = doc.$__getValue(path); + if (val != null && + val instanceof Array && + val.isMongooseDocumentArray && + val.length && + val[arrayAtomicsSymbol] != null && + Object.keys(val[arrayAtomicsSymbol]).length > 0) { + const existing = previous.get(path) || {}; + pathToAtomics.set(path, mergeAtomics(existing, val[arrayAtomicsSymbol])); + } + } + + const dirty = doc.$__dirty(); + for (const dirt of dirty) { + const path = dirt.path; + + const val = dirt.value; + if (val != null && val[arrayAtomicsSymbol] != null && Object.keys(val[arrayAtomicsSymbol]).length > 0) { + const existing = previous.get(path) || {}; + pathToAtomics.set(path, mergeAtomics(existing, val[arrayAtomicsSymbol])); + } + } + + return pathToAtomics; +} + +function mergeAtomics(destination, source) { + destination = destination || {}; + + if (source.$pullAll != null) { + destination.$pullAll = (destination.$pullAll || []).concat(source.$pullAll); + } + if (source.$push != null) { + destination.$push = destination.$push || {}; + destination.$push.$each = (destination.$push.$each || []).concat(source.$push.$each); + } + if (source.$addToSet != null) { + destination.$addToSet = (destination.$addToSet || []).concat(source.$addToSet); + } + if (source.$set != null) { + destination.$set = Object.assign(destination.$set, source.$set); + } + + return destination; +} \ No newline at end of file diff --git a/test/docs/transactions.test.js b/test/docs/transactions.test.js index 8d66c775fc3..429f4fb5e5f 100644 --- a/test/docs/transactions.test.js +++ b/test/docs/transactions.test.js @@ -386,22 +386,39 @@ describe('transactions', function() { it('can save document after aborted transaction (gh-8380)', function() { return co(function*() { - const schema = Schema({ name: String, arr: [String] }); + const schema = Schema({ name: String, arr: [String], arr2: [String] }); const Test = db.model('gh8380', schema); yield Test.createCollection(); - yield Test.create({ name: 'foo', arr: ['bar'] }); + yield Test.create({ name: 'foo', arr: ['bar'], arr2: ['foo'] }); const doc = yield Test.findOne(); yield db. transaction(session => co(function*() { - doc.arr.pop(); + doc.arr.pull('bar'); + doc.arr2.push('bar'); + yield doc.save({ session }); + + doc.name = 'baz'; throw new Error('Oops'); })). - catch(err => assert.equal(err.message, 'Oops')); - doc.set('arr.0', 'qux'); - yield doc.save(); + catch(err => { + assert.equal(err.message, 'Oops'); + }); + + const changes = doc.$__delta()[1]; + assert.equal(changes.$set.name, 'baz'); + assert.deepEqual(changes.$pullAll.arr, ['bar']); + assert.deepEqual(changes.$push.arr2, { $each: ['bar'] }); + assert.ok(!changes.$set.arr2); + + yield doc.save({ session: null }); + + const newDoc = yield Test.collection.findOne(); + assert.equal(newDoc.name, 'baz'); + assert.deepEqual(newDoc.arr, []); + assert.deepEqual(newDoc.arr2, ['foo', 'bar']); }); }); }); From 78e2854bf1624d925df2b824c6a0a80ebecde1f7 Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Thu, 2 Jul 2020 17:59:57 -0400 Subject: [PATCH 30/48] feat: start work on upgrading to mongodb driver 3.6 --- lib/model.js | 4 ++-- package.json | 34 +++++++++++++++++++++++----------- test/model.indexes.test.js | 22 ++++++++++++---------- 3 files changed, 37 insertions(+), 23 deletions(-) diff --git a/lib/model.js b/lib/model.js index 34af9f5a98b..741c645bd88 100644 --- a/lib/model.js +++ b/lib/model.js @@ -1357,7 +1357,7 @@ Model.syncIndexes = function syncIndexes(options, callback) { cb = this.$wrapCallback(cb); this.createCollection(err => { - if (err) { + if (err != null && err.codeName !== 'NamespaceExists') { return cb(err); } this.cleanIndexes((err, dropped) => { @@ -4898,7 +4898,7 @@ Model.$wrapCallback = function(callback) { if (err != null && err.name === 'MongoServerSelectionError') { arguments[0] = serverSelectionError.assimilateError(err); } - if (err != null && err.name === 'MongoNetworkError' && err.message.endsWith('timed out')) { + if (err != null && err.name === 'MongoNetworkTimeoutError' && err.message.endsWith('timed out')) { _this.db.emit('timeout'); } diff --git a/package.json b/package.json index ac0becb777e..1f4bebeb3b5 100644 --- a/package.json +++ b/package.json @@ -21,7 +21,7 @@ "dependencies": { "bson": "^1.1.4", "kareem": "2.3.1", - "mongodb": "3.5.9", + "mongodb": "git@github.com:mongodb/node-mongodb-native.git#3.6", "mongoose-legacy-pluralize": "1.0.2", "mpath": "0.7.0", "mquery": "3.2.2", @@ -134,13 +134,22 @@ "no-const-assign": "error", "no-useless-rename": "error", "no-dupe-keys": "error", - "space-in-parens": ["error", "never"], - "spaced-comment": ["error", "always", { - "block": { - "markers": ["!"], - "balanced": true + "space-in-parens": [ + "error", + "never" + ], + "spaced-comment": [ + "error", + "always", + { + "block": { + "markers": [ + "!" + ], + "balanced": true + } } - }], + ], "key-spacing": [ "error", { @@ -156,10 +165,13 @@ } ], "array-bracket-spacing": 1, - "arrow-spacing": ["error", { - "before": true, - "after": true - }], + "arrow-spacing": [ + "error", + { + "before": true, + "after": true + } + ], "object-curly-spacing": [ "error", "always" diff --git a/test/model.indexes.test.js b/test/model.indexes.test.js index 43547aff57e..2c7a60059a4 100644 --- a/test/model.indexes.test.js +++ b/test/model.indexes.test.js @@ -20,7 +20,7 @@ describe('model', function() { before(function() { db = start(); - return db.createCollection('Test'); + return db.createCollection('Test').catch(() => {}); }); after(function(done) { @@ -458,16 +458,18 @@ describe('model', function() { const schema = new Schema({ arr: [childSchema] }); const Model = db.model('Test', schema); - return Model.init(). - then(() => Model.syncIndexes()). - then(() => Model.listIndexes()). - then(indexes => { - assert.equal(indexes.length, 2); - assert.ok(indexes[1].partialFilterExpression); - assert.deepEqual(indexes[1].partialFilterExpression, { - 'arr.name': { $exists: true } - }); + return co(function*() { + yield Model.init(); + + yield Model.syncIndexes(); + const indexes = yield Model.listIndexes(); + + assert.equal(indexes.length, 2); + assert.ok(indexes[1].partialFilterExpression); + assert.deepEqual(indexes[1].partialFilterExpression, { + 'arr.name': { $exists: true } }); + }); }); it('skips automatic indexing on childSchema if autoIndex: false (gh-9150)', function() { From ab3b38c15edf92563ced9cb64160402a5d6d4389 Mon Sep 17 00:00:00 2001 From: Hafez Date: Fri, 3 Jul 2020 19:10:37 +0200 Subject: [PATCH 31/48] rename `defaultNewOnFindAndUpdateOrReplace` to `returnOriginal` --- lib/index.js | 2 +- lib/query.js | 12 ++++++------ lib/validoptions.js | 2 +- test/model.test.js | 12 ++++++------ 4 files changed, 14 insertions(+), 14 deletions(-) diff --git a/lib/index.js b/lib/index.js index 02cab261e52..324f5af9d63 100644 --- a/lib/index.js +++ b/lib/index.js @@ -147,7 +147,7 @@ Mongoose.prototype.driver = require('./driver'); * * Currently supported options are: * - 'debug': If `true`, prints the operations mongoose sends to MongoDB to the console. If a writable stream is passed, it will log to that stream, without colorization. If a callback function is passed, it will receive the collection name, the method name, then all arugments passed to the method. For example, if you wanted to replicate the default logging, you could output from the callback `Mongoose: ${collectionName}.${methodName}(${methodArgs.join(', ')})`. - * - 'defaultNewOnFindAndUpdateOrReplace': If `true`, changes the default `new` option to `findOneAndUpdate()`, `findByIdAndUpdate` and `findOneAndReplace()` to true. + * - 'returnOriginal': If `false`, changes the default `new` option to `findOneAndUpdate()`, `findByIdAndUpdate` and `findOneAndReplace()` to true. * - 'bufferCommands': enable/disable mongoose's buffering mechanism for all connections and models * - 'useCreateIndex': false by default. Set to `true` to make Mongoose's default index build use `createIndex()` instead of `ensureIndex()` to avoid deprecation warnings from the MongoDB driver. * - 'useFindAndModify': true by default. Set to `false` to make `findOneAndUpdate()` and `findOneAndRemove()` use native `findOneAndUpdate()` rather than `findAndModify()`. diff --git a/lib/query.js b/lib/query.js index 447b9aab9d2..eb81328612e 100644 --- a/lib/query.js +++ b/lib/query.js @@ -3016,9 +3016,9 @@ Query.prototype.findOneAndUpdate = function(criteria, doc, options, callback) { } - const defaultNew = get(this, 'model.base.options.defaultNewOnFindAndUpdateOrReplace'); - if (defaultNew != null && options.new == null) { - options.new = defaultNew; + const returnOriginal = get(this, 'model.base.options.returnOriginal'); + if (options.new == null && returnOriginal != null) { + options.new = !returnOriginal; } this.setOptions(options); @@ -3342,9 +3342,9 @@ Query.prototype.findOneAndReplace = function(filter, replacement, options, callb options = options || {}; - const defaultNew = get(this, 'model.base.options.defaultNewOnFindAndUpdateOrReplace'); - if (defaultNew != null && options.new == null) { - options.new = defaultNew; + const returnOriginal = get(this, 'model.base.options.returnOriginal'); + if (options.new == null && returnOriginal != null) { + options.new = !returnOriginal; } this.setOptions(options); diff --git a/lib/validoptions.js b/lib/validoptions.js index 7b092b2fefb..6e50ec6b69d 100644 --- a/lib/validoptions.js +++ b/lib/validoptions.js @@ -12,9 +12,9 @@ const VALID_OPTIONS = Object.freeze([ 'bufferCommands', 'cloneSchemas', 'debug', - 'defaultNewOnFindAndUpdateOrReplace', 'maxTimeMS', 'objectIdGetter', + 'returnOriginal', 'runValidators', 'selectPopulatedPaths', 'setDefaultsOnInsert', diff --git a/test/model.test.js b/test/model.test.js index a10d69346c4..6fcfb2ca6f2 100644 --- a/test/model.test.js +++ b/test/model.test.js @@ -6869,17 +6869,17 @@ describe('Model', function() { }); }); - describe('defaultNewOnFindAndUpdateOrReplace (gh-9183)', function() { - const originalValue = mongoose.get('defaultNewOnFindAndUpdateOrReplace'); + describe('returnOriginal (gh-9183)', function() { + const originalValue = mongoose.get('returnOriginal'); beforeEach(() => { - mongoose.set('defaultNewOnFindAndUpdateOrReplace', true); + mongoose.set('returnOriginal', false); }); afterEach(() => { - mongoose.set('defaultNewOnFindAndUpdateOrReplace', originalValue); + mongoose.set('returnOriginal', originalValue); }); - it('Setting `defaultNewOnFindAndUpdateOrReplace` works', function() { + it('Setting `returnOriginal` works', function() { return co(function*() { const userSchema = new Schema({ name: { type: String } @@ -6900,7 +6900,7 @@ describe('Model', function() { }); }); - it('`defaultNewOnFindAndUpdateOrReplace` can be overwritten', function() { + it('`returnOriginal` can be overwritten', function() { return co(function*() { const userSchema = new Schema({ name: { type: String } From 90b3217f8d2dea718f50e55b53c6c556692b0f11 Mon Sep 17 00:00:00 2001 From: Hafez Date: Fri, 3 Jul 2020 19:18:11 +0200 Subject: [PATCH 32/48] docs(model): add note to use global option `returnOriginal` --- lib/model.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/model.js b/lib/model.js index cb7c472468f..d5845136d7f 100644 --- a/lib/model.js +++ b/lib/model.js @@ -2445,7 +2445,7 @@ Model.$where = function $where() { * @param {Object} [conditions] * @param {Object} [update] * @param {Object} [options] optional see [`Query.prototype.setOptions()`](http://mongoosejs.com/docs/api.html#query_Query-setOptions) - * @param {Boolean} [options.new=false] By default, `findOneAndUpdate()` returns the document as it was **before** `update` was applied. If you set `new: true`, `findOneAndUpdate()` will instead give you the object after `update` was applied. + * @param {Boolean} [options.new=false] By default, `findOneAndUpdate()` returns the document as it was **before** `update` was applied. If you set `new: true`, `findOneAndUpdate()` will instead give you the object after `update` was applied. To change the default to `true`, use `mongoose.set('returnOriginal', false);`. * @param {Object} [options.lean] if truthy, mongoose will return the document as a plain JavaScript object rather than a mongoose document. See [`Query.lean()`](/docs/api.html#query_Query-lean) and [the Mongoose lean tutorial](/docs/tutorials/lean.html). * @param {ClientSession} [options.session=null] The session associated with this query. See [transactions docs](/docs/transactions.html). * @param {Boolean|String} [options.strict] overwrites the schema's [strict mode option](http://mongoosejs.com/docs/guide.html#strict) @@ -2588,7 +2588,7 @@ function _decorateUpdateWithVersionKey(update, options, versionKey) { * @param {Object|Number|String} id value of `_id` to query by * @param {Object} [update] * @param {Object} [options] optional see [`Query.prototype.setOptions()`](http://mongoosejs.com/docs/api.html#query_Query-setOptions) - * @param {Boolean} [options.new=false] By default, `findByIdAndUpdate()` returns the document as it was **before** `update` was applied. If you set `new: true`, `findOneAndUpdate()` will instead give you the object after `update` was applied. + * @param {Boolean} [options.new=false] By default, `findByIdAndUpdate()` returns the document as it was **before** `update` was applied. If you set `new: true`, `findOneAndUpdate()` will instead give you the object after `update` was applied. To change the default to `true`, use `mongoose.set('returnOriginal', false);`. * @param {Object} [options.lean] if truthy, mongoose will return the document as a plain JavaScript object rather than a mongoose document. See [`Query.lean()`](/docs/api.html#query_Query-lean) and [the Mongoose lean tutorial](/docs/tutorials/lean.html). * @param {ClientSession} [options.session=null] The session associated with this query. See [transactions docs](/docs/transactions.html). * @param {Boolean|String} [options.strict] overwrites the schema's [strict mode option](http://mongoosejs.com/docs/guide.html#strict) @@ -2787,7 +2787,7 @@ Model.findByIdAndDelete = function(id, options, callback) { * @param {Object} filter Replace the first document that matches this filter * @param {Object} [replacement] Replace with this document * @param {Object} [options] optional see [`Query.prototype.setOptions()`](http://mongoosejs.com/docs/api.html#query_Query-setOptions) - * @param {Boolean} [options.new=false] By default, `findOneAndReplace()` returns the document as it was **before** `update` was applied. If you set `new: true`, `findOneAndReplace()` will instead give you the object after `update` was applied. + * @param {Boolean} [options.new=false] By default, `findOneAndReplace()` returns the document as it was **before** `update` was applied. If you set `new: true`, `findOneAndReplace()` will instead give you the object after `update` was applied. To change the default to `true`, use `mongoose.set('returnOriginal', false);`. * @param {Object} [options.lean] if truthy, mongoose will return the document as a plain JavaScript object rather than a mongoose document. See [`Query.lean()`](/docs/api.html#query_Query-lean) and [the Mongoose lean tutorial](/docs/tutorials/lean.html). * @param {ClientSession} [options.session=null] The session associated with this query. See [transactions docs](/docs/transactions.html). * @param {Boolean|String} [options.strict] overwrites the schema's [strict mode option](http://mongoosejs.com/docs/guide.html#strict) From 53fa7880fb5f773edfa344a348f63ece84f043a6 Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Fri, 3 Jul 2020 13:25:57 -0400 Subject: [PATCH 33/48] feat(document): add `useProjection` option to `toObject()` and `toJSON()` for hiding deselected fields on newly created documents Fix #9118 --- lib/document.js | 38 ++++++++++++++++++++++++++++++++++++++ test/document.test.js | 12 ++++++++++++ 2 files changed, 50 insertions(+) diff --git a/lib/document.js b/lib/document.js index da884c9e509..d52dddc5409 100644 --- a/lib/document.js +++ b/lib/document.js @@ -30,6 +30,7 @@ const isExclusive = require('./helpers/projection/isExclusive'); const inspect = require('util').inspect; const internalToObjectOptions = require('./options').internalToObjectOptions; const mpath = require('mpath'); +const queryhelpers = require('./queryhelpers'); const utils = require('./utils'); const isPromise = require('./helpers/isPromise'); @@ -3084,6 +3085,10 @@ Document.prototype.$toObject = function(options, json) { applySchemaTypeTransforms(this, ret); } + if (options.useProjection) { + omitDeselectedFields(this, ret); + } + if (transform === true || (schemaOptions.toObject && transform)) { const opts = options.json ? schemaOptions.toJSON : schemaOptions.toObject; @@ -3119,6 +3124,7 @@ Document.prototype.$toObject = function(options, json) { * - `depopulate` depopulate any populated paths, replacing them with their original refs, defaults to false * - `versionKey` whether to include the version key, defaults to true * - `flattenMaps` convert Maps to POJOs. Useful if you want to JSON.stringify() the result of toObject(), defaults to false + * - `useProjection` set to `true` to omit fields that are excluded in this document's projection. Unless you specified a projection, this will omit any field that has `select: false` in the schema. * * ####Getters/Virtuals * @@ -3243,6 +3249,7 @@ Document.prototype.$toObject = function(options, json) { * @param {Boolean} [options.depopulate=false] if true, replace any conventionally populated paths with the original id in the output. Has no affect on virtual populated paths. * @param {Boolean} [options.versionKey=true] if false, exclude the version key (`__v` by default) from the output * @param {Boolean} [options.flattenMaps=false] if true, convert Maps to POJOs. Useful if you want to `JSON.stringify()` the result of `toObject()`. + * @param {Boolean} [options.useProjection=false] - If true, omits fields that are excluded in this document's projection. Unless you specified a projection, this will omit any field that has `select: false` in the schema. * @return {Object} js object * @see mongodb.Binary http://mongodb.github.com/node-mongodb-native/api-bson-generated/binary.html * @api public @@ -3446,6 +3453,37 @@ function throwErrorIfPromise(path, transformedValue) { } } +/*! + * ignore + */ + +function omitDeselectedFields(self, json) { + const schema = self.schema; + const paths = Object.keys(schema.paths || {}); + const cur = self._doc; + + if (!cur) { + return json; + } + + let selected = self.$__.selected; + if (selected === void 0) { + selected = {}; + queryhelpers.applyPaths(selected, schema); + } + if (selected == null || Object.keys(selected).length === 0) { + return json; + } + + for (const path of paths) { + if (selected[path] != null && !selected[path]) { + delete json[path]; + } + } + + return json; +} + /** * The return value of this method is used in calls to JSON.stringify(doc). * diff --git a/test/document.test.js b/test/document.test.js index 7c714f9cf9a..7d9b19b0c3c 100644 --- a/test/document.test.js +++ b/test/document.test.js @@ -9057,4 +9057,16 @@ describe('document', function() { then(doc => Model.findById(doc)). then(doc => assert.strictEqual(doc.obj.key, 2)); }); + + it('supports `useProjection` option for `toObject()` (gh-9118)', function() { + const authorSchema = new mongoose.Schema({ + name: String, + hiddenField: { type: String, select: false } + }); + + const Author = db.model('Author', authorSchema); + + const example = new Author({ name: 'John', hiddenField: 'A secret' }); + assert.strictEqual(example.toJSON({ useProjection: true }).hiddenField, void 0); + }); }); From 751680e3f1d9751da7a90c72929aeeab30e383b0 Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Sat, 4 Jul 2020 12:52:19 -0400 Subject: [PATCH 34/48] fix(connection): make calling `mongoose.connect()` while already connected a no-op Fix #9203 --- lib/connection.js | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/lib/connection.js b/lib/connection.js index 6e34ff12bbf..a9af7c7d12b 100644 --- a/lib/connection.js +++ b/lib/connection.js @@ -636,9 +636,6 @@ Connection.prototype.onOpen = function() { */ Connection.prototype.openUri = function(uri, options, callback) { - this.readyState = STATES.connecting; - this._closeCalled = false; - if (typeof options === 'function') { callback = options; options = null; @@ -663,6 +660,16 @@ Connection.prototype.openUri = function(uri, options, callback) { typeof callback + '"'); } + if (this.readyState === STATES.connecting || this.readyState === STATES.connected) { + if (typeof callback === 'function') { + callback(null, this); + } + return this; + } + + this.readyState = STATES.connecting; + this._closeCalled = false; + const Promise = PromiseProvider.get(); const _this = this; @@ -768,10 +775,6 @@ Connection.prototype.openUri = function(uri, options, callback) { }); const promise = new Promise((resolve, reject) => { - if (_this.client != null) { - _this.client.close(); - } - const client = new mongodb.MongoClient(uri, options); _this.client = client; client.connect(function(error) { From ae9c686c5635f6669298664e2478c83a62dea127 Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Sun, 5 Jul 2020 15:10:52 -0400 Subject: [PATCH 35/48] fix(connection): throw error when calling `openUri()` on a connected or connecting connection with a different URI Fix #9203 --- lib/connection.js | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/lib/connection.js b/lib/connection.js index a9af7c7d12b..93c86d90501 100644 --- a/lib/connection.js +++ b/lib/connection.js @@ -661,12 +661,19 @@ Connection.prototype.openUri = function(uri, options, callback) { } if (this.readyState === STATES.connecting || this.readyState === STATES.connected) { + if (this._connectionString !== uri) { + throw new MongooseError('Can\'t call `openUri()` on an active connection with ' + + 'different connection strings. Make sure you aren\'t calling `mongoose.connect()` ' + + 'multiple times. See: https://mongoosejs.com/docs/connections.html#multiple_connections'); + } + if (typeof callback === 'function') { callback(null, this); } return this; } + this._connectionString = uri; this.readyState = STATES.connecting; this._closeCalled = false; @@ -1397,6 +1404,7 @@ Connection.prototype.setClient = function setClient(client) { throw new MongooseError('Cannot call `setClient()` with a MongoClient that is not connected.'); } + this._connectionString = client.s.url; _setClient(this, client, { useUnifiedTopology: client.s.options.useUnifiedTopology }, client.s.options.dbName); return this; From 337e3b94ecc56443f17c87b30595c1c22d6c8f6c Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Sun, 5 Jul 2020 17:11:32 -0400 Subject: [PATCH 36/48] feat(document+populate): add `parent()` function that allows you to get the parent document for populated docs Fix #8092 --- lib/document.js | 24 ++++++++++++++++++ lib/helpers/populate/assignVals.js | 10 ++++++++ lib/model.js | 3 +++ lib/options/PopulateOptions.js | 1 + test/model.populate.test.js | 40 ++++++++++++++++++++++++++++++ test/query.test.js | 15 +++++++---- 6 files changed, 88 insertions(+), 5 deletions(-) diff --git a/lib/document.js b/lib/document.js index d52dddc5409..b34dfa1f01d 100644 --- a/lib/document.js +++ b/lib/document.js @@ -548,6 +548,16 @@ Document.prototype.$__init = function(doc, opts) { } else { this.populated(item.path, item._docs[id], item); } + + if (item._childDocs == null) { + continue; + } + for (const child of item._childDocs) { + if (child == null || child.$__ == null) { + continue; + } + child.$__.parent = this; + } } } @@ -3506,6 +3516,20 @@ Document.prototype.toJSON = function(options) { return this.$toObject(options, true); }; +/** + * If this document is a subdocument or populated document, returns the document's + * parent. Returns `undefined` otherwise. + * + * @api public + * @method parent + * @memberOf Document + * @instance + */ + +Document.prototype.parent = function() { + return this.$__.parent; +}; + /** * Helper for console.log * diff --git a/lib/helpers/populate/assignVals.js b/lib/helpers/populate/assignVals.js index bd93e81afdc..a9e42192ae0 100644 --- a/lib/helpers/populate/assignVals.js +++ b/lib/helpers/populate/assignVals.js @@ -84,6 +84,16 @@ module.exports = function assignVals(o) { }, new Map()); } + if (isDoc && Array.isArray(valueToSet)) { + for (const val of valueToSet) { + if (val != null && val.$__ != null) { + val.$__.parent = docs[i]; + } + } + } else if (isDoc && valueToSet != null && valueToSet.$__ != null) { + valueToSet.$__.parent = docs[i]; + } + if (o.isVirtual && isDoc) { docs[i].populated(o.path, o.justOne ? originalIds[0] : originalIds, o.allOptions); // If virtual populate and doc is already init-ed, need to walk through diff --git a/lib/model.js b/lib/model.js index 34af9f5a98b..a11e7021a5e 100644 --- a/lib/model.js +++ b/lib/model.js @@ -4462,6 +4462,9 @@ function populate(model, docs, options, callback) { for (const arr of params) { const mod = arr[0]; const assignmentOpts = arr[3]; + for (const val of vals) { + mod.options._childDocs.push(val); + } _assign(model, vals, mod, assignmentOpts); } diff --git a/lib/options/PopulateOptions.js b/lib/options/PopulateOptions.js index b60d45abda6..5b9819460dc 100644 --- a/lib/options/PopulateOptions.js +++ b/lib/options/PopulateOptions.js @@ -5,6 +5,7 @@ const clone = require('../helpers/clone'); class PopulateOptions { constructor(obj) { this._docs = {}; + this._childDocs = []; if (obj == null) { return; diff --git a/test/model.populate.test.js b/test/model.populate.test.js index eaf15e3df71..3761e6d9d6d 100644 --- a/test/model.populate.test.js +++ b/test/model.populate.test.js @@ -9546,4 +9546,44 @@ describe('model: populate:', function() { assert.equal(docs[1].items[0].foo.title, 'doc1'); }); }); + + it('Sets the populated document\'s parent() (gh-8092)', function() { + const schema = new Schema({ + single: { type: Number, ref: 'Child' }, + arr: [{ type: Number, ref: 'Child' }], + docArr: [{ ref: { type: Number, ref: 'Child' } }] + }); + + schema.virtual('myVirtual', { + ref: 'Child', + localField: 'single', + foreignField: '_id', + justOne: true + }); + + const Parent = db.model('Parent', schema); + const Child = db.model('Child', Schema({ _id: Number, name: String })); + + return co(function*() { + yield Child.create({ _id: 1, name: 'test' }); + + yield Parent.create({ single: 1, arr: [1], docArr: [{ ref: 1 }] }); + + let doc = yield Parent.findOne().populate('single'); + assert.ok(doc.single.parent() === doc); + + doc = yield Parent.findOne().populate('arr'); + assert.ok(doc.arr[0].parent() === doc); + + doc = yield Parent.findOne().populate('docArr.ref'); + assert.ok(doc.docArr[0].ref.parent() === doc); + + doc = yield Parent.findOne().populate('myVirtual'); + assert.ok(doc.myVirtual.parent() === doc); + + doc = yield Parent.findOne(); + yield doc.populate('single').execPopulate(); + assert.ok(doc.single.parent() === doc); + }); + }); }); diff --git a/test/query.test.js b/test/query.test.js index 97a3defe70c..77892657370 100644 --- a/test/query.test.js +++ b/test/query.test.js @@ -832,7 +832,8 @@ describe('Query', function() { select: undefined, model: undefined, options: undefined, - _docs: {} + _docs: {}, + _childDocs: [] }; q.populate(o); assert.deepEqual(o, q._mongooseOptions.populate['yellow.brick']); @@ -844,7 +845,8 @@ describe('Query', function() { let o = { path: 'yellow.brick', match: { bricks: { $lt: 1000 } }, - _docs: {} + _docs: {}, + _childDocs: [] }; q.populate(Object.assign({}, o)); assert.equal(Object.keys(q._mongooseOptions.populate).length, 1); @@ -853,7 +855,8 @@ describe('Query', function() { q.populate('yellow.brick'); o = { path: 'yellow.brick', - _docs: {} + _docs: {}, + _childDocs: [] }; assert.equal(Object.keys(q._mongooseOptions.populate).length, 1); assert.deepEqual(q._mongooseOptions.populate['yellow.brick'], o); @@ -866,11 +869,13 @@ describe('Query', function() { assert.equal(Object.keys(q._mongooseOptions.populate).length, 2); assert.deepEqual(q._mongooseOptions.populate['yellow.brick'], { path: 'yellow.brick', - _docs: {} + _docs: {}, + _childDocs: [] }); assert.deepEqual(q._mongooseOptions.populate['dirt'], { path: 'dirt', - _docs: {} + _docs: {}, + _childDocs: [] }); done(); }); From e4090b18807542b92d2354b08a94199d03b2c9ed Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Sun, 5 Jul 2020 17:15:48 -0400 Subject: [PATCH 37/48] test: fix tests --- test/query.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/query.test.js b/test/query.test.js index 77892657370..e7be2f856de 100644 --- a/test/query.test.js +++ b/test/query.test.js @@ -1604,7 +1604,7 @@ describe('Query', function() { q.setOptions({ read: ['s', [{ dc: 'eu' }]] }); assert.equal(q.options.thing, 'cat'); - assert.deepEqual(q._mongooseOptions.populate.fans, { path: 'fans', _docs: {} }); + assert.deepEqual(q._mongooseOptions.populate.fans, { path: 'fans', _docs: {}, _childDocs: [] }); assert.equal(q.options.batchSize, 10); assert.equal(q.options.limit, 4); assert.equal(q.options.skip, 3); From 402de9cad8e0bc3dae846146e22bc61d9f9bd8bc Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Sun, 19 Jul 2020 14:18:32 -0400 Subject: [PATCH 38/48] refactor: set `returnOriginal` instead of `new` with global `returnOriginal` option --- lib/query.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/query.js b/lib/query.js index eb81328612e..4bca9b90623 100644 --- a/lib/query.js +++ b/lib/query.js @@ -3017,8 +3017,8 @@ Query.prototype.findOneAndUpdate = function(criteria, doc, options, callback) { const returnOriginal = get(this, 'model.base.options.returnOriginal'); - if (options.new == null && returnOriginal != null) { - options.new = !returnOriginal; + if (options.returnOriginal == null && returnOriginal != null) { + options.returnOriginal = returnOriginal; } this.setOptions(options); @@ -3343,8 +3343,8 @@ Query.prototype.findOneAndReplace = function(filter, replacement, options, callb options = options || {}; const returnOriginal = get(this, 'model.base.options.returnOriginal'); - if (options.new == null && returnOriginal != null) { - options.new = !returnOriginal; + if (options.returnOriginal == null && returnOriginal != null) { + options.returnOriginal = returnOriginal; } this.setOptions(options); From 04f9a7a5394dfa61b0f3d416fd44df4fc332f322 Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Sun, 19 Jul 2020 14:23:38 -0400 Subject: [PATCH 39/48] docs(returnOriginal): clarify that `returnOriginal` doesn't set `new`, and link to `findOneAndUpdate()` tutorial --- lib/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/index.js b/lib/index.js index 324f5af9d63..6a3e5ea2674 100644 --- a/lib/index.js +++ b/lib/index.js @@ -147,7 +147,7 @@ Mongoose.prototype.driver = require('./driver'); * * Currently supported options are: * - 'debug': If `true`, prints the operations mongoose sends to MongoDB to the console. If a writable stream is passed, it will log to that stream, without colorization. If a callback function is passed, it will receive the collection name, the method name, then all arugments passed to the method. For example, if you wanted to replicate the default logging, you could output from the callback `Mongoose: ${collectionName}.${methodName}(${methodArgs.join(', ')})`. - * - 'returnOriginal': If `false`, changes the default `new` option to `findOneAndUpdate()`, `findByIdAndUpdate` and `findOneAndReplace()` to true. + * - 'returnOriginal': If `false`, changes the default `returnOriginal` option to `findOneAndUpdate()`, `findByIdAndUpdate`, and `findOneAndReplace()` to false. This is equivalent to setting the `new` option to `true` for `findOneAndX()` calls by default. Read our [`findOneAndUpdate()` tutorial](/docs/tutorials/findoneandupdate.html) for more information. * - 'bufferCommands': enable/disable mongoose's buffering mechanism for all connections and models * - 'useCreateIndex': false by default. Set to `true` to make Mongoose's default index build use `createIndex()` instead of `ensureIndex()` to avoid deprecation warnings from the MongoDB driver. * - 'useFindAndModify': true by default. Set to `false` to make `findOneAndUpdate()` and `findOneAndRemove()` use native `findOneAndUpdate()` rather than `findAndModify()`. From 1a4146c463a9fd58198976f4807d77e2f511d167 Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Sat, 25 Jul 2020 14:57:05 -0400 Subject: [PATCH 40/48] test: fix tests on 5.10 for #9208 --- lib/types/subdocument.js | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/types/subdocument.js b/lib/types/subdocument.js index 1925e6c3528..61b53f0e45e 100644 --- a/lib/types/subdocument.js +++ b/lib/types/subdocument.js @@ -53,6 +53,7 @@ function Subdocument(value, fields, parent, skipId, options) { } delete options.priorDoc; + delete this.$__.$options.priorDoc; } } From 6a7b2755e253afc7e7f2d1c540287be5e3ec3a28 Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Mon, 27 Jul 2020 13:41:08 -0400 Subject: [PATCH 41/48] feat(schema+model): add `optimisticConcurrency` option to use OCC for `save()` Fix #9001 #5424 --- docs/guide.pug | 72 +++++++++++++++++++++++++++++++++++++++-- lib/model.js | 9 ++++-- lib/schema.js | 7 +++- test/versioning.test.js | 22 +++++++++++++ 4 files changed, 104 insertions(+), 6 deletions(-) diff --git a/docs/guide.pug b/docs/guide.pug index c4f501f4cdd..39c021d6682 100644 --- a/docs/guide.pug +++ b/docs/guide.pug @@ -434,6 +434,7 @@ block content - [useNestedStrict](#useNestedStrict) - [validateBeforeSave](#validateBeforeSave) - [versionKey](#versionKey) + - [optimisticConcurrency](#optimisticConcurrency) - [collation](#collation) - [selectPopulatedPaths](#selectPopulatedPaths) - [skipVersioning](#skipVersioning) @@ -891,9 +892,8 @@ block content thing.save(); // { _somethingElse: 0, name: 'mongoose v3' } ``` - Note that Mongoose versioning is **not** a full [optimistic concurrency](https://en.wikipedia.org/wiki/Optimistic_concurrency_control) - solution. Use [mongoose-update-if-current](https://github.com/eoin-obrien/mongoose-update-if-current) - for OCC support. Mongoose versioning only operates on arrays: + Note that Mongoose's default versioning is **not** a full [optimistic concurrency](https://en.wikipedia.org/wiki/Optimistic_concurrency_control) + solution. Mongoose's default versioning only operates on arrays as shown below. ```javascript // 2 copies of the same document @@ -911,6 +911,8 @@ block content await doc2.save(); ``` + If you need optimistic concurrency support for `save()`, you can set the [`optimisticConcurrency` option](#optimisticConcurrency) + Document versioning can also be disabled by setting the `versionKey` to `false`. _DO NOT disable versioning unless you [know what you are doing](http://aaronheckmann.blogspot.com/2012/06/mongoose-v3-part-1-versioning.html)._ @@ -946,6 +948,70 @@ block content }); ``` +

option: optimisticConcurrency

+ + [Optimistic concurrency](https://en.wikipedia.org/wiki/Optimistic_concurrency_control) is a strategy to ensure + the document you're updating didn't change between when you loaded it using `find()` or `findOne()`, and when + you update it using `save()`. + + For example, suppose you have a `House` model that contains a list of `photos`, and a `status` that represents + whether this house shows up in searches. Suppose that a house that has status `'APPROVED'` must have at least + two `photos`. You might implement the logic of approving a house document as shown below: + + ```javascript + async function markApproved(id) { + const house = await House.findOne({ _id }); + if (house.photos.length < 2) { + throw new Error('House must have at least two photos!'); + } + + house.status = 'APPROVED'; + await house.save(); + } + ``` + + The `markApproved()` function looks right in isolation, but there might be a potential issue: what if another + function removes the house's photos between the `findOne()` call and the `save()` call? For example, the below + code will succeed: + + ```javascript + const house = await House.findOne({ _id }); + if (house.photos.length < 2) { + throw new Error('House must have at least two photos!'); + } + + const house2 = await House.findOne({ _id }); + house2.photos = []; + await house2.save(); + + // Marks the house as 'APPROVED' even though it has 0 photos! + house.status = 'APPROVED'; + await house.save(); + ``` + + If you set the `optimisticConcurrency` option on the `House` model's schema, the above script will throw an + error. + + ```javascript + const House = mongoose.model('House', Schema({ + status: String, + photos: [String] + }, { optimisticConcurrency: true })); + + const house = await House.findOne({ _id }); + if (house.photos.length < 2) { + throw new Error('House must have at least two photos!'); + } + + const house2 = await House.findOne({ _id }); + house2.photos = []; + await house2.save(); + + // Throws 'VersionError: No matching document found for id "..." version 0' + house.status = 'APPROVED'; + await house.save(); + ``` +

option: collation

Sets a default [collation](https://docs.mongodb.com/manual/reference/collation/) diff --git a/lib/model.js b/lib/model.js index b6633d7ad80..da175b20e38 100644 --- a/lib/model.js +++ b/lib/model.js @@ -542,6 +542,11 @@ function operand(self, where, delta, data, val, op) { // already marked for versioning? if (VERSION_ALL === (VERSION_ALL & self.$__.version)) return; + if (self.schema.options.optimisticConcurrency) { + self.$__.version = VERSION_ALL; + return; + } + switch (op) { case '$set': case '$unset': @@ -2950,10 +2955,10 @@ Model.findByIdAndRemove = function(id, options, callback) { * * // Insert one new `Character` document * await Character.create({ name: 'Jean-Luc Picard' }); - * + * * // Insert multiple new `Character` documents * await Character.create([{ name: 'Will Riker' }, { name: 'Geordi LaForge' }]); - * + * * // Create a new character within a transaction. Note that you **must** * // pass an array as the first parameter to `create()` if you want to * // specify options. diff --git a/lib/schema.js b/lib/schema.js index c64e829b848..b7099cb8bac 100644 --- a/lib/schema.js +++ b/lib/schema.js @@ -69,7 +69,7 @@ let id = 0; * - [typePojoToMixed](/docs/guide.html#typePojoToMixed) - boolean - defaults to true. Determines whether a type set to a POJO becomes a Mixed path or a Subdocument * - [useNestedStrict](/docs/guide.html#useNestedStrict) - boolean - defaults to false * - [validateBeforeSave](/docs/guide.html#validateBeforeSave) - bool - defaults to `true` - * - [versionKey](/docs/guide.html#versionKey): string - defaults to "__v" + * - [versionKey](/docs/guide.html#versionKey): string or object - defaults to "__v" * - [collation](/docs/guide.html#collation): object - defaults to null (which means use no collation) * - [selectPopulatedPaths](/docs/guide.html#selectPopulatedPaths): boolean - defaults to `true` * - [skipVersioning](/docs/guide.html#skipVersioning): object - paths to exclude from versioning @@ -404,6 +404,7 @@ Schema.prototype.defaultOptions = function(options) { bufferCommands: true, capped: false, // { size, max, autoIndexId } versionKey: '__v', + optimisticConcurrency: false, discriminatorKey: '__t', minimize: true, autoIndex: null, @@ -423,6 +424,10 @@ Schema.prototype.defaultOptions = function(options) { options.read = readPref(options.read); } + if (options.optimisticConcurrency && !options.versionKey) { + throw new MongooseError('Must set `versionKey` if using `optimisticConcurrency`'); + } + return options; }; diff --git a/test/versioning.test.js b/test/versioning.test.js index ec3cd6ea64d..538e49c177c 100644 --- a/test/versioning.test.js +++ b/test/versioning.test.js @@ -593,4 +593,26 @@ describe('versioning', function() { }). catch(done); }); + + it('optimistic concurrency (gh-9001) (gh-5424)', function() { + const schema = new Schema({ name: String }, { optimisticConcurrency: true }); + const M = db.model('Test', schema); + + const doc = new M({ name: 'foo' }); + + return co(function*() { + yield doc.save(); + + const d1 = yield M.findOne(); + const d2 = yield M.findOne(); + + d1.name = 'bar'; + yield d1.save(); + + d2.name = 'qux'; + const err = yield d2.save().then(() => null, err => err); + assert.ok(err); + assert.equal(err.name, 'VersionError'); + }); + }); }); From ec19595270916540b4feff46004435671cc7cb73 Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Tue, 28 Jul 2020 12:07:44 -0400 Subject: [PATCH 42/48] feat(query): handle casting `$or` when each clause contains a different discriminator key Fix #9018 --- lib/cast.js | 5 ++++ .../getSchemaDiscriminatorByValue.js | 24 +++++++++++++++++++ test/model.discriminator.querying.test.js | 24 ++++++++++++++++++- 3 files changed, 52 insertions(+), 1 deletion(-) create mode 100644 lib/helpers/discriminator/getSchemaDiscriminatorByValue.js diff --git a/lib/cast.js b/lib/cast.js index b047939948f..1c24bc7fe65 100644 --- a/lib/cast.js +++ b/lib/cast.js @@ -9,6 +9,7 @@ const StrictModeError = require('./error/strict'); const Types = require('./schema/index'); const castTextSearch = require('./schema/operators/text'); const get = require('./helpers/get'); +const getSchemaDiscriminatorByValue = require('./helpers/discriminator/getSchemaDiscriminatorByValue'); const isOperator = require('./helpers/query/isOperator'); const util = require('util'); const isObject = require('./helpers/isObject'); @@ -42,6 +43,10 @@ module.exports = function cast(schema, obj, options, context) { delete obj._bsontype; } + if (schema != null && schema.discriminators != null && obj[schema.options.discriminatorKey] != null) { + schema = getSchemaDiscriminatorByValue(schema, obj[schema.options.discriminatorKey]) || schema; + } + const paths = Object.keys(obj); let i = paths.length; let _keys; diff --git a/lib/helpers/discriminator/getSchemaDiscriminatorByValue.js b/lib/helpers/discriminator/getSchemaDiscriminatorByValue.js new file mode 100644 index 00000000000..f3e71a093a9 --- /dev/null +++ b/lib/helpers/discriminator/getSchemaDiscriminatorByValue.js @@ -0,0 +1,24 @@ +'use strict'; + +/*! +* returns discriminator by discriminatorMapping.value +* +* @param {Schema} schema +* @param {string} value +*/ + +module.exports = function getSchemaDiscriminatorByValue(schema, value) { + if (schema == null || schema.discriminators == null) { + return null; + } + for (const key of Object.keys(schema.discriminators)) { + const discriminatorSchema = schema.discriminators[key]; + if (discriminatorSchema.discriminatorMapping == null) { + continue; + } + if (discriminatorSchema.discriminatorMapping.value === value) { + return discriminatorSchema; + } + } + return null; +}; \ No newline at end of file diff --git a/test/model.discriminator.querying.test.js b/test/model.discriminator.querying.test.js index c7e981fe39c..4cd246f3b3e 100644 --- a/test/model.discriminator.querying.test.js +++ b/test/model.discriminator.querying.test.js @@ -27,7 +27,7 @@ function BaseSchema() { util.inherits(BaseSchema, Schema); const EventSchema = new BaseSchema(); -const ImpressionEventSchema = new BaseSchema(); +const ImpressionEventSchema = new BaseSchema({ element: String }); const ConversionEventSchema = new BaseSchema({ revenue: Number }); const SecretEventSchema = new BaseSchema({ secret: { type: String, select: false } }); @@ -178,6 +178,28 @@ describe('model', function() { checkHydratesCorrectModels({ name: 1 }, done); }); + it('casts underneath $or if discriminator key in filter (gh-9018)', function() { + return co(function*() { + yield ImpressionEvent.create({ name: 'Impression event', element: '42' }); + yield ConversionEvent.create({ name: 'Conversion event', revenue: 1.337 }); + + let docs = yield BaseEvent.find({ __t: 'Impression', element: 42 }); + assert.equal(docs.length, 1); + assert.equal(docs[0].name, 'Impression event'); + + docs = yield BaseEvent.find({ $or: [{ __t: 'Impression', element: 42 }] }); + assert.equal(docs.length, 1); + assert.equal(docs[0].name, 'Impression event'); + + docs = yield BaseEvent.find({ + $or: [{ __t: 'Impression', element: 42 }, { __t: 'Conversion', revenue: '1.337' }] + }).sort({ __t: 1 }); + assert.equal(docs.length, 2); + assert.equal(docs[0].name, 'Conversion event'); + assert.equal(docs[1].name, 'Impression event'); + }); + }); + describe('discriminator model only finds documents of its type', function() { describe('using "ModelDiscriminator#findById"', function() { From 21de3863bd92e2ee5830eebc6e69503f646ff9a5 Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Fri, 31 Jul 2020 10:32:05 -0400 Subject: [PATCH 43/48] test: move some more transactions tests to async/await --- test/es-next/transactions.test.es6.js | 89 +++++++++++++-------------- 1 file changed, 42 insertions(+), 47 deletions(-) diff --git a/test/es-next/transactions.test.es6.js b/test/es-next/transactions.test.es6.js index f8d32b4828c..560bcbfc33c 100644 --- a/test/es-next/transactions.test.es6.js +++ b/test/es-next/transactions.test.es6.js @@ -353,60 +353,55 @@ describe('transactions', function() { session.endSession(); }); - it('correct `isNew` after abort (gh-8852)', function() { - return co(function*() { - const schema = Schema({ name: String }); - - const Test = db.model('gh8852', schema); - - yield Test.createCollection(); - const doc = new Test({ name: 'foo' }); - yield db. - transaction(session => co(function*() { - yield doc.save({ session }); - assert.ok(!doc.isNew); - throw new Error('Oops'); - })). - catch(err => assert.equal(err.message, 'Oops')); - assert.ok(doc.isNew); - }); - }); + it('correct `isNew` after abort (gh-8852)', async function() { + const schema = Schema({ name: String }); - it('can save document after aborted transaction (gh-8380)', function() { - return co(function*() { - const schema = Schema({ name: String, arr: [String], arr2: [String] }); + const Test = db.model('gh8852', schema); - const Test = db.model('gh8380', schema); + await Test.createCollection(); + const doc = new Test({ name: 'foo' }); + await db. + transaction(async (session) => { + await doc.save({ session }); + assert.ok(!doc.isNew); + throw new Error('Oops'); + }). + catch(err => assert.equal(err.message, 'Oops')); + assert.ok(doc.isNew); + }); - yield Test.createCollection(); - yield Test.create({ name: 'foo', arr: ['bar'], arr2: ['foo'] }); - const doc = yield Test.findOne(); - yield db. - transaction(session => co(function*() { - doc.arr.pull('bar'); - doc.arr2.push('bar'); + it('can save document after aborted transaction (gh-8380)', async function() { + const schema = Schema({ name: String, arr: [String], arr2: [String] }); - yield doc.save({ session }); + const Test = db.model('gh8380', schema); - doc.name = 'baz'; - throw new Error('Oops'); - })). - catch(err => { - assert.equal(err.message, 'Oops'); - }); + await Test.createCollection(); + await Test.create({ name: 'foo', arr: ['bar'], arr2: ['foo'] }); + const doc = await Test.findOne(); + await db. + transaction(async (session) => { + doc.arr.pull('bar'); + doc.arr2.push('bar'); + + await doc.save({ session }); + doc.name = 'baz'; + throw new Error('Oops'); + }). + catch(err => { + assert.equal(err.message, 'Oops'); + }); - const changes = doc.$__delta()[1]; - assert.equal(changes.$set.name, 'baz'); - assert.deepEqual(changes.$pullAll.arr, ['bar']); - assert.deepEqual(changes.$push.arr2, { $each: ['bar'] }); - assert.ok(!changes.$set.arr2); + const changes = doc.$__delta()[1]; + assert.equal(changes.$set.name, 'baz'); + assert.deepEqual(changes.$pullAll.arr, ['bar']); + assert.deepEqual(changes.$push.arr2, { $each: ['bar'] }); + assert.ok(!changes.$set.arr2); - yield doc.save({ session: null }); + await doc.save({ session: null }); - const newDoc = yield Test.collection.findOne(); - assert.equal(newDoc.name, 'baz'); - assert.deepEqual(newDoc.arr, []); - assert.deepEqual(newDoc.arr2, ['foo', 'bar']); - }); + const newDoc = await Test.collection.findOne(); + assert.equal(newDoc.name, 'baz'); + assert.deepEqual(newDoc.arr, []); + assert.deepEqual(newDoc.arr2, ['foo', 'bar']); }); }); From 58cfc157d1183be524ad3c4fb28b760968bcf27f Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Fri, 31 Jul 2020 10:32:33 -0400 Subject: [PATCH 44/48] docs: correct link to transactions examples --- website.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/website.js b/website.js index 6408030ccf9..6687a41e802 100644 --- a/website.js +++ b/website.js @@ -23,7 +23,7 @@ markdown.setOptions({ const tests = [ ...acquit.parse(fs.readFileSync('./test/geojson.test.js').toString()), - ...acquit.parse(fs.readFileSync('./test/docs/transactions.test.js').toString()), + ...acquit.parse(fs.readFileSync('./test/es-next/transactions.test.es6.js').toString()), ...acquit.parse(fs.readFileSync('./test/schema.alias.test.js').toString()), ...acquit.parse(fs.readFileSync('./test/model.middleware.test.js').toString()), ...acquit.parse(fs.readFileSync('./test/docs/date.test.js').toString()), From 065d549caa08d6f0722ae21b33cacad58e6f786d Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Fri, 31 Jul 2020 10:39:20 -0400 Subject: [PATCH 45/48] feat: use mongodb driver 3.6.0 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 1f4bebeb3b5..0a05f3d089e 100644 --- a/package.json +++ b/package.json @@ -21,7 +21,7 @@ "dependencies": { "bson": "^1.1.4", "kareem": "2.3.1", - "mongodb": "git@github.com:mongodb/node-mongodb-native.git#3.6", + "mongodb": "3.6.0", "mongoose-legacy-pluralize": "1.0.2", "mpath": "0.7.0", "mquery": "3.2.2", From d374f141f395066b0f60c9a49ccdbdc408e78191 Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Fri, 14 Aug 2020 11:19:29 -0400 Subject: [PATCH 46/48] fix: work around https://jira.mongodb.org/projects/NODE/issues/NODE-2741 Re: #9188 --- lib/helpers/model/castBulkWrite.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/helpers/model/castBulkWrite.js b/lib/helpers/model/castBulkWrite.js index 0455562f461..8e47afce706 100644 --- a/lib/helpers/model/castBulkWrite.js +++ b/lib/helpers/model/castBulkWrite.js @@ -71,7 +71,6 @@ module.exports = function castBulkWrite(originalModel, op, options) { overwrite: false, upsert: op['updateOne'].upsert }); - } catch (error) { return callback(error, null); } @@ -152,6 +151,7 @@ module.exports = function castBulkWrite(originalModel, op, options) { if (error) { return callback(error, null); } + op['replaceOne']['replacement'] = op['replaceOne']['replacement'].toBSON(); callback(null); }); }; From bd455c440584c0a8e332b627cba812bcabdbfbc3 Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Fri, 14 Aug 2020 11:29:56 -0400 Subject: [PATCH 47/48] fix(update): allow upsert with empty updates Re: #9188 Re: mongodb/node-mongodb-native#2490 --- lib/helpers/model/castBulkWrite.js | 16 ++++++++++++---- lib/helpers/query/castUpdate.js | 6 ++++++ 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/lib/helpers/model/castBulkWrite.js b/lib/helpers/model/castBulkWrite.js index 8e47afce706..619281285e9 100644 --- a/lib/helpers/model/castBulkWrite.js +++ b/lib/helpers/model/castBulkWrite.js @@ -37,8 +37,12 @@ module.exports = function castBulkWrite(originalModel, op, options) { } else if (op['updateOne']) { return (callback) => { try { - if (!op['updateOne']['filter']) throw new Error('Must provide a filter object.'); - if (!op['updateOne']['update']) throw new Error('Must provide an update object.'); + if (!op['updateOne']['filter']) { + throw new Error('Must provide a filter object.'); + } + if (!op['updateOne']['update']) { + throw new Error('Must provide an update object.'); + } const model = decideModelByObject(originalModel, op['updateOne']['filter']); const schema = model.schema; @@ -80,8 +84,12 @@ module.exports = function castBulkWrite(originalModel, op, options) { } else if (op['updateMany']) { return (callback) => { try { - if (!op['updateMany']['filter']) throw new Error('Must provide a filter object.'); - if (!op['updateMany']['update']) throw new Error('Must provide an update object.'); + if (!op['updateMany']['filter']) { + throw new Error('Must provide a filter object.'); + } + if (!op['updateMany']['update']) { + throw new Error('Must provide an update object.'); + } const model = decideModelByObject(originalModel, op['updateMany']['filter']); const schema = model.schema; diff --git a/lib/helpers/query/castUpdate.js b/lib/helpers/query/castUpdate.js index 327d9d2f862..6009852fdd8 100644 --- a/lib/helpers/query/castUpdate.js +++ b/lib/helpers/query/castUpdate.js @@ -105,6 +105,12 @@ module.exports = function castUpdate(schema, obj, options, context, filter) { } } + if (Object.keys(ret).length === 0 && options.upsert) { + // Trick the driver into allowing empty upserts to work around + // https://github.com/mongodb/node-mongodb-native/pull/2490 + return { $fake: true, toBSON: () => ({}) }; + } + return ret; }; From 7646d9e3cb6e4359d30d3db65d99079c0cb63d83 Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Fri, 14 Aug 2020 11:54:29 -0400 Subject: [PATCH 48/48] fix: alternative fix for allowing empty update on upsert Fix #9188 --- lib/helpers/model/castBulkWrite.js | 4 ++-- lib/helpers/query/castUpdate.js | 6 ++++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/lib/helpers/model/castBulkWrite.js b/lib/helpers/model/castBulkWrite.js index 619281285e9..6e7a8300754 100644 --- a/lib/helpers/model/castBulkWrite.js +++ b/lib/helpers/model/castBulkWrite.js @@ -74,7 +74,7 @@ module.exports = function castBulkWrite(originalModel, op, options) { strict: strict, overwrite: false, upsert: op['updateOne'].upsert - }); + }, model, op['updateOne']['filter']); } catch (error) { return callback(error, null); } @@ -121,7 +121,7 @@ module.exports = function castBulkWrite(originalModel, op, options) { strict: strict, overwrite: false, upsert: op['updateMany'].upsert - }); + }, model, op['updateMany']['filter']); } catch (error) { return callback(error, null); diff --git a/lib/helpers/query/castUpdate.js b/lib/helpers/query/castUpdate.js index 6009852fdd8..8afd60af8f7 100644 --- a/lib/helpers/query/castUpdate.js +++ b/lib/helpers/query/castUpdate.js @@ -105,10 +105,12 @@ module.exports = function castUpdate(schema, obj, options, context, filter) { } } - if (Object.keys(ret).length === 0 && options.upsert) { + if (Object.keys(ret).length === 0 && + options.upsert && + Object.keys(filter).length > 0) { // Trick the driver into allowing empty upserts to work around // https://github.com/mongodb/node-mongodb-native/pull/2490 - return { $fake: true, toBSON: () => ({}) }; + return { $setOnInsert: filter }; } return ret;