diff --git a/.evergreen/install-dependencies.sh b/.evergreen/install-dependencies.sh index acfd7f2fe11..e856ee76f48 100644 --- a/.evergreen/install-dependencies.sh +++ b/.evergreen/install-dependencies.sh @@ -9,6 +9,7 @@ NPM_TMP_DIR="${NODE_ARTIFACTS_PATH}/tmp" # this needs to be explicitly exported for the nvm install below export NVM_DIR="${NODE_ARTIFACTS_PATH}/nvm" +export XDG_CONFIG_HOME=${NODE_ARTIFACTS_PATH} # create node artifacts path if needed mkdir -p ${NODE_ARTIFACTS_PATH} @@ -16,9 +17,9 @@ mkdir -p ${NPM_CACHE_DIR} mkdir -p "${NPM_TMP_DIR}" # install Node.js -curl -o- https://raw.githubusercontent.com/creationix/nvm/v0.33.8/install.sh | bash +curl -o- https://raw.githubusercontent.com/creationix/nvm/v0.35.3/install.sh | bash [ -s "${NVM_DIR}/nvm.sh" ] && \. "${NVM_DIR}/nvm.sh" -nvm install --lts=${NODE_LTS_NAME} +nvm install --no-progress --lts=${NODE_LTS_NAME} # setup npm cache in a local directory cat < .npmrc diff --git a/lib/mongo_client.js b/lib/mongo_client.js index 2d58247788b..7b0c603a64f 100644 --- a/lib/mongo_client.js +++ b/lib/mongo_client.js @@ -241,28 +241,35 @@ MongoClient.prototype.close = function(force, callback) { force = false; } - const client = this; + const topology = this.topology; + return maybePromise(this, callback, cb => { const completeClose = err => { - client.emit('close', client); + this.emit('close', this); - if (!(client.topology instanceof NativeTopology)) { - for (const item of client.s.dbCache) { - item[1].emit('close', client); + if (!(topology instanceof NativeTopology)) { + for (const item of this.s.dbCache) { + item[1].emit('close', this); } } - client.removeAllListeners('close'); + this.removeAllListeners('close'); + + // clear out references to old topology + this.topology = undefined; + this.s.dbCache = new Map(); + this.s.sessions = new Set(); + cb(err); }; - if (client.topology == null) { + if (topology == null) { completeClose(); return; } - client.topology.close(force, err => { - const autoEncrypter = client.topology.s.options.autoEncrypter; + topology.close(force, err => { + const autoEncrypter = topology.s.options.autoEncrypter; if (!autoEncrypter) { completeClose(err); return; diff --git a/lib/operations/connect.js b/lib/operations/connect.js index fc415e475d2..d0157f676b1 100644 --- a/lib/operations/connect.js +++ b/lib/operations/connect.js @@ -260,6 +260,11 @@ function connect(mongoClient, url, options, callback) { throw new Error('no callback function provided'); } + // Has a connection already been established? + if (mongoClient.topology && mongoClient.topology.isConnected()) { + throw new Error(`'connect' cannot be called when already connected`); + } + let didRequestAuthentication = false; const logger = Logger('MongoClient', options); diff --git a/package.json b/package.json index 56e7576de74..1ef4959cb55 100644 --- a/package.json +++ b/package.json @@ -68,7 +68,7 @@ "test": "npm run lint && mocha --recursive test/functional test/unit test/core", "test-nolint": "mocha --recursive test/functional test/unit test/core", "coverage": "istanbul cover mongodb-test-runner -- -t 60000 test/core test/unit test/functional", - "lint": "eslint lib test", + "lint": "eslint -v && eslint lib test", "format": "prettier --print-width 100 --tab-width 2 --single-quote --write 'test/**/*.js' 'lib/**/*.js'", "bench": "node test/benchmarks/driverBench/", "generate-evergreen": "node .evergreen/generate_evergreen_tasks.js", diff --git a/test/functional/change_stream.test.js b/test/functional/change_stream.test.js index 020de3d6c44..5a834a92feb 100644 --- a/test/functional/change_stream.test.js +++ b/test/functional/change_stream.test.js @@ -4,7 +4,9 @@ var Transform = require('stream').Transform; const MongoError = require('../../lib/core').MongoError; var MongoNetworkError = require('../../lib/core').MongoNetworkError; var setupDatabase = require('./shared').setupDatabase; -var withTempDb = require('./shared').withTempDb; +var withClient = require('./shared').withClient; +var withDb = require('./shared').withDb; +var withCollection = require('./shared').withCollection; var delay = require('./shared').delay; var co = require('co'); var mock = require('mongodb-mock-server'); @@ -59,8 +61,12 @@ function tryNext(changeStream, callback) { let complete = false; function done(err, result) { if (complete) return; - // if the arity is 1 then this a callback for `more` + // if the arity is 1 then this is a callback for `more` if (arguments.length === 1) { + if (err instanceof Error) { + callback(err); + return; + } result = err; const batch = result.cursor.firstBatch || result.cursor.nextBatch; if (batch.length === 0) { @@ -2632,65 +2638,67 @@ describe('Change Streams', function() { }); describe('tryNext', function() { + function withTemporaryCollectionOnDb(database, testFn) { + return function() { + return withClient.bind(this)( + withDb( + database, + { helper: { drop: true } }, + withCollection( + { + collection: { w: 'majority' }, + helper: { create: true } + }, + testFn + ) + ) + ); + }; + } it('should return null on single iteration of empty cursor', { metadata: { requires: { topology: 'replicaset', mongodb: '>=3.6' } }, - test: function() { - return withTempDb( - 'testTryNext', - { w: 'majority' }, - this.configuration.newClient(), - db => done => { - const changeStream = db.collection('test').watch(); - tryNext(changeStream, (err, doc) => { - expect(err).to.not.exist; - expect(doc).to.not.exist; + test: withTemporaryCollectionOnDb('test_try_next_empty', (collection, done) => { + const changeStream = collection.watch(); + tryNext(changeStream, (err, doc) => { + expect(err).to.not.exist; + expect(doc).to.not.exist; - changeStream.close(done); - }); - } - ); - } + changeStream.close(done); + }); + }) }); it('should iterate a change stream until first empty batch', { metadata: { requires: { topology: 'replicaset', mongodb: '>=3.6' } }, - test: function() { - return withTempDb( - 'testTryNext', - { w: 'majority' }, - this.configuration.newClient(), - db => done => { - const collection = db.collection('test'); - const changeStream = collection.watch(); - waitForStarted(changeStream, () => { - collection.insertOne({ a: 42 }, err => { - expect(err).to.not.exist; + test: withTemporaryCollectionOnDb('test_try_next_results', (collection, done) => { + const changeStream = collection.watch(); + waitForStarted(changeStream, () => { + collection.insertOne({ a: 42 }, err => { + expect(err).to.not.exist; - collection.insertOne({ b: 24 }, err => { - expect(err).to.not.exist; - }); - }); + collection.insertOne({ b: 24 }, err => { + expect(err).to.not.exist; }); + }); + }); - tryNext(changeStream, (err, doc) => { - expect(err).to.not.exist; - expect(doc).to.exist; + tryNext(changeStream, (err, doc) => { + expect(err).to.not.exist; + expect(doc).to.exist; - tryNext(changeStream, (err, doc) => { - expect(err).to.not.exist; - expect(doc).to.exist; + tryNext(changeStream, (err, doc) => { + expect(err).to.not.exist; + expect(doc).to.exist; - tryNext(changeStream, (err, doc) => { - expect(err).to.not.exist; - expect(doc).to.not.exist; + tryNext(changeStream, (err, doc) => { + expect(err).to.not.exist; + expect(doc).to.not.exist; - changeStream.close(done); - }); - }); + changeStream.close(done); }); - } - ); - } + }); + }); + }) }); }); diff --git a/test/functional/connection.test.js b/test/functional/connection.test.js index a75916eeb6f..a4ce997d64f 100644 --- a/test/functional/connection.test.js +++ b/test/functional/connection.test.js @@ -1,7 +1,8 @@ 'use strict'; -const test = require('./shared').assert, - setupDatabase = require('./shared').setupDatabase, - expect = require('chai').expect; +const test = require('./shared').assert; +const setupDatabase = require('./shared').setupDatabase; +const withClient = require('./shared').withClient; +const expect = require('chai').expect; describe('Connection', function() { before(function() { @@ -514,4 +515,27 @@ describe('Connection', function() { }); } }); + + it('should be able to connect again after close', function() { + return withClient.call(this, (client, done) => { + expect(client.isConnected()).to.be.true; + const collection = () => client.db('testReconnect').collection('test'); + collection().insertOne({ a: 1 }, (err, result) => { + expect(err).to.not.exist; + expect(result).to.exist; + client.close(err => { + expect(err).to.not.exist; + client.connect(err => { + expect(err).to.not.exist; + collection().insertOne({ b: 2 }, (err, result) => { + expect(err).to.not.exist; + expect(result).to.exist; + expect(client.topology.isDestroyed()).to.be.false; + done(); + }); + }); + }); + }); + }); + }); }); diff --git a/test/functional/sessions.test.js b/test/functional/sessions.test.js index c79510001d2..b4f88b68e6f 100644 --- a/test/functional/sessions.test.js +++ b/test/functional/sessions.test.js @@ -125,7 +125,7 @@ describe('Sessions', function() { // verify that the `endSessions` command was sent const lastCommand = test.commands.started[test.commands.started.length - 1]; expect(lastCommand.commandName).to.equal('endSessions'); - expect(client.topology.s.sessionPool.sessions).to.have.length(0); + expect(client.topology).to.not.exist; }); }); }); @@ -149,7 +149,7 @@ describe('Sessions', function() { // verify that the `endSessions` command was sent const lastCommand = test.commands.started[test.commands.started.length - 1]; expect(lastCommand.commandName).to.equal('endSessions'); - expect(client.topology.s.sessionPool.sessions).to.have.length(0); + expect(client.topology).to.not.exist; }); }); } diff --git a/test/functional/shared.js b/test/functional/shared.js index 7b03073e43f..5ee460f526d 100644 --- a/test/functional/shared.js +++ b/test/functional/shared.js @@ -57,8 +57,100 @@ function setupDatabase(configuration, dbsToClean) { ); } -function makeCleanupFn(client) { - return function(err) { +/** + * use as the `testFn` of `withDb` + * + * @param {string} [name='test'] database name + * @param {object} [options] options + * @param {object} [options.collection={}] collection options + * @param {object} [options.helper={}] helper options + * @param {boolean} [options.helper.create] create collection before test + * @param {boolean} [options.helper.drop] drop collection after test + * @param {Function} testFn test function to execute + */ +function withCollection(name, options, testFn) { + if (arguments.length === 1) { + testFn = name; + name = 'test'; + options = { collection: {}, helper: {} }; + } else if (arguments.length === 2) { + testFn = options; + if (typeof name === 'string') { + options = { collection: {}, helper: {} }; + } else { + options = name; + name = 'test'; + } + } + function runTest(collection, done) { + testFn(collection, options.helper.drop ? () => collection.drop(done) : done); + } + if (options.helper.create) { + return (db, done) => + db.createCollection(name, options, (err, collection) => { + if (err) return done(err); + runTest(collection, done); + }); + } + return (db, done) => { + const collection = db.collection(name, options.collection); + runTest(collection, done); + }; +} + + +/** + * use as the `operation` of `withClient` + * + * @param {string} [name='test'] database name + * @param {object} [options] options + * @param {object} [options.db={}] database options + * @param {object} [options.helper={}] helper options + * @param {boolean} [options.helper.drop] drop database after test + * @param {Function} testFn test function to execute + + */ +function withDb(name, options, testFn) { + if (arguments.length === 1) { + testFn = name; + name = 'test'; + options = { db: {}, helper: {} }; + } else if (arguments.length === 2) { + testFn = options; + if (typeof name === 'string') { + options = { db: {}, helper: {} }; + } else { + options = name; + name = 'test'; + } + } + return client => + new Promise(resolve => { + const db = client.db(name, options.db); + testFn(db, options.helper.drop ? () => db.dropDatabase(resolve) : resolve); + }); +} + +/** + * Safely perform a test with provided MongoClient, ensuring client won't leak. + * + * @param {MongoClient} [client] if not provided, withClient must be bound to test function `this` + * @param {Function} operation (client):Promise or (client, done):void + * @param {Function} [errorHandler] + */ +function withClient(client, operation, errorHandler) { + if (!(client instanceof MongoClient)) { + errorHandler = operation; + operation = client; + client = this.configuration.newClient(); + } + + if (operation.length === 2) { + const callback = operation; + operation = client => new Promise(resolve => callback(client, resolve)); + } + + function cleanup(err) { return new Promise((resolve, reject) => { try { client.close(closeErr => { @@ -72,29 +164,7 @@ function makeCleanupFn(client) { return reject(err || e); } }); - }; -} - -function withTempDb(name, options, client, operation, errorHandler) { - return withClient( - client, - client => done => { - const db = client.db(name, options); - operation.call(this, db)(() => db.dropDatabase(done)); - }, - errorHandler - ); -} - -/** - * Safely perform a test with provided MongoClient, ensuring client won't leak. - * - * @param {MongoClient} client - * @param {Function|Promise} operation - * @param {Function|Promise} [errorHandler] - */ -function withClient(client, operation, errorHandler) { - const cleanup = makeCleanupFn(client); + } return client .connect() @@ -259,7 +329,8 @@ module.exports = { delay, withClient, withMonitoredClient, - withTempDb, + withDb, + withCollection, filterForCommands, filterOutCommands, ignoreNsNotFound,