From b5e66ef52230d047448f978554e35cb42545b801 Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Tue, 18 Jun 2024 15:56:01 -0400 Subject: [PATCH 1/5] feat: remove default connection if setting createInitialConnection to false after Mongoose instance created re: #8302 --- lib/mongoose.js | 12 ++++++++++++ lib/validOptions.js | 1 + test/index.test.js | 10 ++++++++++ 3 files changed, 23 insertions(+) diff --git a/lib/mongoose.js b/lib/mongoose.js index cfac2331ced..0676ceeb355 100644 --- a/lib/mongoose.js +++ b/lib/mongoose.js @@ -33,6 +33,7 @@ const SetOptionError = require('./error/setOptionError'); const applyEmbeddedDiscriminators = require('./helpers/discriminator/applyEmbeddedDiscriminators'); const defaultMongooseSymbol = Symbol.for('mongoose:default'); +const defaultConnectionSymbol = Symbol('mongoose:defaultConnection'); require('./helpers/printJestWarning'); @@ -73,6 +74,7 @@ function Mongoose(options) { const createInitialConnection = utils.getOption('createInitialConnection', this.options) ?? true; if (createInitialConnection && this.__driver != null) { const conn = this.createConnection(); // default connection + conn[defaultConnectionSymbol] = true; conn.models = this.models; } @@ -292,6 +294,16 @@ Mongoose.prototype.set = function(key, value) { } else if (!optionValue && _mongoose.transactionAsyncLocalStorage) { delete _mongoose.transactionAsyncLocalStorage; } + } else if (optionKey === 'createInitialConnection') { + if (optionValue && !_mongoose.connection) { + const conn = this.createConnection(); // default connection + conn[defaultConnectionSymbol] = true; + conn.models = this.models; + } else if (optionValue === false && _mongoose.connection && _mongoose.connection[defaultConnectionSymbol]) { + if (_mongoose.connection.readyState === STATES.disconnected && Object.keys(_mongoose.connection.models).length === 0) { + _mongoose.connections.shift(); + } + } } } diff --git a/lib/validOptions.js b/lib/validOptions.js index 2654a7521ed..15fd3e634e7 100644 --- a/lib/validOptions.js +++ b/lib/validOptions.js @@ -15,6 +15,7 @@ const VALID_OPTIONS = Object.freeze([ 'bufferCommands', 'bufferTimeoutMS', 'cloneSchemas', + 'createInitialConnection', 'debug', 'id', 'timestamps.createdAt.immutable', diff --git a/test/index.test.js b/test/index.test.js index 27f975a9d21..9087b67211d 100644 --- a/test/index.test.js +++ b/test/index.test.js @@ -1180,4 +1180,14 @@ describe('mongoose module:', function() { } }); }); + + it('setting createInitialConnection after creation deletes existing connection (gh-8302)', async function() { + const m = new mongoose.Mongoose(); + assert.ok(m.connection); + m.set('createInitialConnection', false); + assert.strictEqual(m.connection, undefined); + + const conn = m.createConnection(); + assert.equal(conn, m.connection); + }); }); From 51679926a6f6b6226a9e5b218234be8b8df978c1 Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Fri, 21 Jun 2024 14:01:09 -0400 Subject: [PATCH 2/5] fix: recreate initial connection if calling mongoose.connect() after deleting initial connection Re: #8302 --- lib/mongoose.js | 5 +++++ test/index.test.js | 9 +++++++++ 2 files changed, 14 insertions(+) diff --git a/lib/mongoose.js b/lib/mongoose.js index 0676ceeb355..2ec80b84bef 100644 --- a/lib/mongoose.js +++ b/lib/mongoose.js @@ -436,6 +436,11 @@ Mongoose.prototype.connect = async function connect(uri, options) { } const _mongoose = this instanceof Mongoose ? this : mongoose; + if (_mongoose.connection == null) { + const conn = this.createConnection(); // default connection + conn[defaultConnectionSymbol] = true; + conn.models = this.models; + } const conn = _mongoose.connection; return conn.openUri(uri, options).then(() => _mongoose); diff --git a/test/index.test.js b/test/index.test.js index 1a5969e2a80..d395e5c8ae3 100644 --- a/test/index.test.js +++ b/test/index.test.js @@ -1189,5 +1189,14 @@ describe('mongoose module:', function() { const conn = m.createConnection(); assert.equal(conn, m.connection); + + const m2 = new mongoose.Mongoose(); + assert.ok(m2.connection); + m2.set('createInitialConnection', false); + assert.strictEqual(m2.connection, undefined); + + await m2.connect(start.uri); + assert.ok(m2.connection); + await m2.disconnect(); }); }); From 8eafe6e22545d089290f2780517514289851b924 Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Mon, 24 Jun 2024 10:38:31 -0400 Subject: [PATCH 3/5] test: expand tests for createInitialConnection re: code review comments --- test/index.test.js | 63 +++++++++++++++++++++++++++++++++------------- 1 file changed, 46 insertions(+), 17 deletions(-) diff --git a/test/index.test.js b/test/index.test.js index d395e5c8ae3..04bd2184450 100644 --- a/test/index.test.js +++ b/test/index.test.js @@ -1181,22 +1181,51 @@ describe('mongoose module:', function() { }); }); - it('setting createInitialConnection after creation deletes existing connection (gh-8302)', async function() { - const m = new mongoose.Mongoose(); - assert.ok(m.connection); - m.set('createInitialConnection', false); - assert.strictEqual(m.connection, undefined); - - const conn = m.createConnection(); - assert.equal(conn, m.connection); - - const m2 = new mongoose.Mongoose(); - assert.ok(m2.connection); - m2.set('createInitialConnection', false); - assert.strictEqual(m2.connection, undefined); - - await m2.connect(start.uri); - assert.ok(m2.connection); - await m2.disconnect(); + describe('createInitialConnection (gh-8302)', function() { + let m; + + beforeEach(function() { + m = new mongoose.Mongoose(); + }); + + afterEach(async function() { + await m.disconnect(); + }); + + it('should delete existing connection when setting createInitialConnection to false (part 1)', function() { + assert.ok(m.connection); + m.set('createInitialConnection', false); + assert.strictEqual(m.connection, undefined); + }); + + it('should create connection when createConnection is called', function() { + m.set('createInitialConnection', false); + const conn = m.createConnection(); + assert.equal(conn, m.connection); + }); + + it('should create a new connection automatically when connect() is called if no existing default connection', async function() { + assert.ok(m.connection); + m.set('createInitialConnection', false); + assert.strictEqual(m.connection, undefined); + + await m.connect(start.uri); + assert.ok(m.connection); + }); + + it('should not delete default connection if it has models', async function() { + assert.ok(m.connection); + m.model('Test', new m.Schema({ name: String })); + m.set('createInitialConnection', false); + assert.ok(m.connection); + }); + + it('should not delete default connection if it is connected', async function() { + assert.ok(m.connection); + await m.connect(start.uri); + m.set('createInitialConnection', false); + assert.ok(m.connection); + assert.equal(m.connection.readyState, 1); + }); }); }); From c04a75d2238b609c4b735ad6c06e632bc2f4fe01 Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Mon, 24 Jun 2024 10:52:12 -0400 Subject: [PATCH 4/5] refactor: create new _createDefaultConnection() helper function re: code review comments Re: #8302 --- lib/mongoose.js | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/lib/mongoose.js b/lib/mongoose.js index 2ec80b84bef..9a9c26d2bb1 100644 --- a/lib/mongoose.js +++ b/lib/mongoose.js @@ -73,9 +73,7 @@ function Mongoose(options) { }, options); const createInitialConnection = utils.getOption('createInitialConnection', this.options) ?? true; if (createInitialConnection && this.__driver != null) { - const conn = this.createConnection(); // default connection - conn[defaultConnectionSymbol] = true; - conn.models = this.models; + _createDefaultConnection(this); } if (this.options.pluralization) { @@ -296,9 +294,7 @@ Mongoose.prototype.set = function(key, value) { } } else if (optionKey === 'createInitialConnection') { if (optionValue && !_mongoose.connection) { - const conn = this.createConnection(); // default connection - conn[defaultConnectionSymbol] = true; - conn.models = this.models; + _createDefaultConnection(_mongoose); } else if (optionValue === false && _mongoose.connection && _mongoose.connection[defaultConnectionSymbol]) { if (_mongoose.connection.readyState === STATES.disconnected && Object.keys(_mongoose.connection.models).length === 0) { _mongoose.connections.shift(); @@ -437,9 +433,7 @@ Mongoose.prototype.connect = async function connect(uri, options) { const _mongoose = this instanceof Mongoose ? this : mongoose; if (_mongoose.connection == null) { - const conn = this.createConnection(); // default connection - conn[defaultConnectionSymbol] = true; - conn.models = this.models; + _createDefaultConnection(_mongoose); } const conn = _mongoose.connection; @@ -1332,6 +1326,20 @@ Mongoose.prototype.overwriteMiddlewareResult = Kareem.overwriteResult; Mongoose.prototype.omitUndefined = require('./helpers/omitUndefined'); +/*! + * Create a new default connection (`mongoose.connection`) for a Mongoose instance. + * No-op if there is already a default connection. + */ + +function _createDefaultConnection(mongoose) { + if (mongoose.connection) { + return; + } + const conn = mongoose.createConnection(); // default connection + conn[defaultConnectionSymbol] = true; + conn.models = mongoose.models; +} + /** * The exports object is an instance of Mongoose. * From 38ab7e66068239a97cccfecfd00a394ed101ca30 Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Mon, 24 Jun 2024 12:19:33 -0400 Subject: [PATCH 5/5] Update test/index.test.js Co-authored-by: hasezoey --- test/index.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/index.test.js b/test/index.test.js index 04bd2184450..cfbd644f1f3 100644 --- a/test/index.test.js +++ b/test/index.test.js @@ -1192,7 +1192,7 @@ describe('mongoose module:', function() { await m.disconnect(); }); - it('should delete existing connection when setting createInitialConnection to false (part 1)', function() { + it('should delete existing connection when setting createInitialConnection to false', function() { assert.ok(m.connection); m.set('createInitialConnection', false); assert.strictEqual(m.connection, undefined);