From 35106a3b93dedf909a539d360d17e76926f8de11 Mon Sep 17 00:00:00 2001 From: Rhys Date: Wed, 16 Dec 2020 02:19:35 -0500 Subject: [PATCH] Revert "fix: Explicitly pass read preference with db.command commands COMPASS-4539 (#282)" This reverts commit f2bd36d67f169e2fbbed963a1bbec7583e0bd89e. --- lib/instance-detail-helper.js | 42 +++++++++------------- lib/native-client.js | 6 +--- package-lock.json | 7 ++-- package.json | 6 ++-- test/instance-detail-helper-mocked.test.js | 38 -------------------- 5 files changed, 25 insertions(+), 74 deletions(-) diff --git a/lib/instance-detail-helper.js b/lib/instance-detail-helper.js index 3d3f8d7d..dc8b229f 100644 --- a/lib/instance-detail-helper.js +++ b/lib/instance-detail-helper.js @@ -20,13 +20,6 @@ const { const debug = require('debug')('mongodb-data-service:instance-detail-helper'); -function getReadPreferenceOptions(db) { - // `db.command` does not use the read preference set on the - // connection, so here we explicitly to specify it in the options. - const readPreference = db.s ? db.s.readPreference : ReadPreference.PRIMARY; - return { readPreference }; -} - /** * aggregates stats across all found databases * @param {Object} results async.auto results @@ -85,7 +78,7 @@ function getBuildInfo(results, done) { }; const adminDb = db.databaseName === 'admin' ? db : db.admin(); - adminDb.command(spec, getReadPreferenceOptions(db), function(err, res) { + adminDb.command(spec, {}, function(err, res) { if (err) { // buildInfo doesn't require any privileges to run, so if it fails, // something really went wrong and we should return the error. @@ -105,7 +98,7 @@ function getCmdLineOpts(results, done) { }; const adminDb = db.databaseName === 'admin' ? db : db.admin(); - adminDb.command(spec, getReadPreferenceOptions(db), function(err, res) { + adminDb.command(spec, {}, function(err, res) { if (err) { debug('getCmdLineOpts failed!', err); return done(null, { errmsg: err.message }); @@ -220,7 +213,7 @@ function getHostInfo(results, done) { }; const adminDb = db.databaseName === 'admin' ? db : db.admin(); - adminDb.command(spec, getReadPreferenceOptions(db), function(err, res) { + adminDb.command(spec, {}, function(err, res) { if (err) { if (isNotAuthorized(err)) { // if the error is that the user is not authorized, silently ignore it @@ -260,14 +253,12 @@ function listDatabases(results, done) { listDatabases: 1 }; - const options = getReadPreferenceOptions(db); - const adminDb = db.databaseName === 'admin' ? db : db.admin(); - adminDb.command(spec, options, function(err, res) { + adminDb.command(spec, {}, function(err, res) { if (err) { if (isNotAuthorized(err)) { // eslint-disable-next-line no-shadow - adminDb.command({connectionStatus: 1, showPrivileges: 1}, options, function(err, res) { + adminDb.command({connectionStatus: 1, showPrivileges: 1}, {}, function(err, res) { if (err) { done(err); return; @@ -352,7 +343,7 @@ function getDatabase(client, db, name, done) { dbStats: 1 }; - const options = getReadPreferenceOptions(db); + const options = {}; debug('running dbStats for `%s`...', name); client.db(name).command(spec, options, function(err, res) { if (err) { @@ -385,15 +376,8 @@ function getDatabases(results, done) { function getUserInfo(results, done) { const db = results.db; - const options = getReadPreferenceOptions(db); - // get the user privileges - db.command({ - connectionStatus: 1, - showPrivileges: true - }, - options, - function( + db.command({ connectionStatus: 1, showPrivileges: true }, {}, function( err, res ) { @@ -410,7 +394,7 @@ function getUserInfo(results, done) { } const user = res.authInfo.authenticatedUsers[0]; - db.command({ usersInfo: user, showPrivileges: true }, options, function( + db.command({ usersInfo: user, showPrivileges: true }, {}, function( _err, _res ) { @@ -507,7 +491,15 @@ function getDatabaseCollections(db, done) { const spec = {}; - db.listCollections(spec, getReadPreferenceOptions(db)).toArray(function(err, res) { + /** + * @note: Durran: For some reason the listCollections call does not take into + * account the read preference that was set on the db instance - it only looks + * in the passed options: https://github.com/mongodb/node-mongodb-native/blob/2.2/lib/db.js#L671 + */ + const rp = db.s ? db.s.readPreference : ReadPreference.PRIMARY; + const options = { readPreference: rp }; + + db.listCollections(spec, options).toArray(function(err, res) { if (err) { if (isNotAuthorized(err) || isMongosLocalException(err)) { // if the error is that the user is not authorized, silently ignore it diff --git a/lib/native-client.js b/lib/native-client.js index 3c53d039..235ff202 100644 --- a/lib/native-client.js +++ b/lib/native-client.js @@ -267,11 +267,7 @@ class NativeClient extends EventEmitter { * @param {Function} callback - The callback. */ listDatabases(callback) { - this.database.admin().command({ - listDatabases: 1 - }, { - readPreference: this.model.readPreference - }, (error, result) => { + this.database.admin().command({ listDatabases: 1 }, {}, (error, result) => { if (error) { return callback(this._translateMessage(error)); } diff --git a/package-lock.json b/package-lock.json index 89f0d543..4300fe19 100644 --- a/package-lock.json +++ b/package-lock.json @@ -4377,9 +4377,9 @@ } }, "mongodb-connection-model": { - "version": "18.0.0", - "resolved": "https://registry.npmjs.org/mongodb-connection-model/-/mongodb-connection-model-18.0.0.tgz", - "integrity": "sha512-X5OtFijmCP6HNfsCETEYiBH3M+hvW+p+E7gHqA2CQcVaXaeoojYpX2Nsr7YKwW1oKUbQURfM4szt1Ds9rBAzvw==", + "version": "17.3.0", + "resolved": "https://registry.npmjs.org/mongodb-connection-model/-/mongodb-connection-model-17.3.0.tgz", + "integrity": "sha512-loaAlu6xoEyz7th+f4lkFd07BZZawZYl2Q4EvImAp45kZxo8AH9RmuPCCUsxUlcTHP35i5pDHnCVxzAOb+LB3A==", "dev": true, "requires": { "ampersand-model": "^8.0.0", @@ -4387,6 +4387,7 @@ "async": "^3.1.0", "debug": "^4.1.1", "lodash": "^4.17.15", + "mongodb": "^3.6.3", "raf": "^3.4.1", "ssh2": "^0.8.7", "storage-mixin": "^3.3.4" diff --git a/package.json b/package.json index f1900da3..67ec0a26 100644 --- a/package.json +++ b/package.json @@ -20,8 +20,8 @@ "check": "mongodb-js-precommit" }, "peerDependencies": { - "mongodb": "3.x", - "mongodb-connection-model": "*" + "mongodb": "^3.6.3", + "mongodb-connection-model": "^17.3.0" }, "dependencies": { "async": "^3.2.0", @@ -41,7 +41,7 @@ "mocha": "^8.2.1", "mock-require": "^3.0.3", "mongodb": "^3.6.3", - "mongodb-connection-model": "^18.0.0", + "mongodb-connection-model": "^17.3.0", "mongodb-js-precommit": "^2.2.1", "mongodb-runner": "^4.8.0", "xvfb-maybe": "^0.2.1" diff --git a/test/instance-detail-helper-mocked.test.js b/test/instance-detail-helper-mocked.test.js index d6d236b6..2bbd4e55 100644 --- a/test/instance-detail-helper-mocked.test.js +++ b/test/instance-detail-helper-mocked.test.js @@ -1,5 +1,4 @@ const assert = require('assert'); -const { ReadPreference } = require('mongodb'); const { getAllowedCollections, getAllowedDatabases, @@ -384,43 +383,6 @@ describe('instance-detail-helper-mocked', function() { done(); }); }); - it('should pass the read preference explicity when it is set', function(done) { - let passedOptions; - results.db = makeMockDB(function(times, command, options, callback) { - if (command.listDatabases) { - passedOptions = options; - return callback(null, { databases: [] }); - } - return callback(null, {}); - }); - results.db.s = { - readPreference: ReadPreference.SECONDARY - }; - - listDatabases(results, function() { - assert.deepEqual(passedOptions, { - readPreference: 'secondary' - }); - done(); - }); - }); - it('defaults to passing the read preference as PRIMARY', function(done) { - let passedOptions; - results.db = makeMockDB(function(times, command, options, callback) { - if (command.listDatabases) { - passedOptions = options; - return callback(null, { databases: [] }); - } - return callback(null, {}); - }); - - listDatabases(results, function() { - assert.deepEqual(passedOptions, { - readPreference: ReadPreference.PRIMARY - }); - done(); - }); - }); it('should pass on other errors from the listDatabases command', function(done) { // instead of the real db handle, pass in the mocked one results.db = makeMockDB(