Skip to content
This repository was archived by the owner on May 17, 2021. It is now read-only.

Commit bd8c323

Browse files
authored
Revert "fix: Explicitly pass read preference with db.command commands COMPASS-4539 (#282)" (#283)
This reverts commit f2bd36d.
1 parent 94f5344 commit bd8c323

File tree

5 files changed

+25
-74
lines changed

5 files changed

+25
-74
lines changed

lib/instance-detail-helper.js

Lines changed: 17 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -20,13 +20,6 @@ const {
2020

2121
const debug = require('debug')('mongodb-data-service:instance-detail-helper');
2222

23-
function getReadPreferenceOptions(db) {
24-
// `db.command` does not use the read preference set on the
25-
// connection, so here we explicitly to specify it in the options.
26-
const readPreference = db.s ? db.s.readPreference : ReadPreference.PRIMARY;
27-
return { readPreference };
28-
}
29-
3023
/**
3124
* aggregates stats across all found databases
3225
* @param {Object} results async.auto results
@@ -85,7 +78,7 @@ function getBuildInfo(results, done) {
8578
};
8679

8780
const adminDb = db.databaseName === 'admin' ? db : db.admin();
88-
adminDb.command(spec, getReadPreferenceOptions(db), function(err, res) {
81+
adminDb.command(spec, {}, function(err, res) {
8982
if (err) {
9083
// buildInfo doesn't require any privileges to run, so if it fails,
9184
// something really went wrong and we should return the error.
@@ -105,7 +98,7 @@ function getCmdLineOpts(results, done) {
10598
};
10699

107100
const adminDb = db.databaseName === 'admin' ? db : db.admin();
108-
adminDb.command(spec, getReadPreferenceOptions(db), function(err, res) {
101+
adminDb.command(spec, {}, function(err, res) {
109102
if (err) {
110103
debug('getCmdLineOpts failed!', err);
111104
return done(null, { errmsg: err.message });
@@ -220,7 +213,7 @@ function getHostInfo(results, done) {
220213
};
221214

222215
const adminDb = db.databaseName === 'admin' ? db : db.admin();
223-
adminDb.command(spec, getReadPreferenceOptions(db), function(err, res) {
216+
adminDb.command(spec, {}, function(err, res) {
224217
if (err) {
225218
if (isNotAuthorized(err)) {
226219
// if the error is that the user is not authorized, silently ignore it
@@ -260,14 +253,12 @@ function listDatabases(results, done) {
260253
listDatabases: 1
261254
};
262255

263-
const options = getReadPreferenceOptions(db);
264-
265256
const adminDb = db.databaseName === 'admin' ? db : db.admin();
266-
adminDb.command(spec, options, function(err, res) {
257+
adminDb.command(spec, {}, function(err, res) {
267258
if (err) {
268259
if (isNotAuthorized(err)) {
269260
// eslint-disable-next-line no-shadow
270-
adminDb.command({connectionStatus: 1, showPrivileges: 1}, options, function(err, res) {
261+
adminDb.command({connectionStatus: 1, showPrivileges: 1}, {}, function(err, res) {
271262
if (err) {
272263
done(err);
273264
return;
@@ -352,7 +343,7 @@ function getDatabase(client, db, name, done) {
352343
dbStats: 1
353344
};
354345

355-
const options = getReadPreferenceOptions(db);
346+
const options = {};
356347
debug('running dbStats for `%s`...', name);
357348
client.db(name).command(spec, options, function(err, res) {
358349
if (err) {
@@ -385,15 +376,8 @@ function getDatabases(results, done) {
385376
function getUserInfo(results, done) {
386377
const db = results.db;
387378

388-
const options = getReadPreferenceOptions(db);
389-
390379
// get the user privileges
391-
db.command({
392-
connectionStatus: 1,
393-
showPrivileges: true
394-
},
395-
options,
396-
function(
380+
db.command({ connectionStatus: 1, showPrivileges: true }, {}, function(
397381
err,
398382
res
399383
) {
@@ -410,7 +394,7 @@ function getUserInfo(results, done) {
410394
}
411395
const user = res.authInfo.authenticatedUsers[0];
412396

413-
db.command({ usersInfo: user, showPrivileges: true }, options, function(
397+
db.command({ usersInfo: user, showPrivileges: true }, {}, function(
414398
_err,
415399
_res
416400
) {
@@ -507,7 +491,15 @@ function getDatabaseCollections(db, done) {
507491

508492
const spec = {};
509493

510-
db.listCollections(spec, getReadPreferenceOptions(db)).toArray(function(err, res) {
494+
/**
495+
* @note: Durran: For some reason the listCollections call does not take into
496+
* account the read preference that was set on the db instance - it only looks
497+
* in the passed options: https://github.com/mongodb/node-mongodb-native/blob/2.2/lib/db.js#L671
498+
*/
499+
const rp = db.s ? db.s.readPreference : ReadPreference.PRIMARY;
500+
const options = { readPreference: rp };
501+
502+
db.listCollections(spec, options).toArray(function(err, res) {
511503
if (err) {
512504
if (isNotAuthorized(err) || isMongosLocalException(err)) {
513505
// if the error is that the user is not authorized, silently ignore it

lib/native-client.js

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -267,11 +267,7 @@ class NativeClient extends EventEmitter {
267267
* @param {Function} callback - The callback.
268268
*/
269269
listDatabases(callback) {
270-
this.database.admin().command({
271-
listDatabases: 1
272-
}, {
273-
readPreference: this.model.readPreference
274-
}, (error, result) => {
270+
this.database.admin().command({ listDatabases: 1 }, {}, (error, result) => {
275271
if (error) {
276272
return callback(this._translateMessage(error));
277273
}

package-lock.json

Lines changed: 4 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

package.json

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,8 @@
2020
"check": "mongodb-js-precommit"
2121
},
2222
"peerDependencies": {
23-
"mongodb": "3.x",
24-
"mongodb-connection-model": "*"
23+
"mongodb": "^3.6.3",
24+
"mongodb-connection-model": "^17.3.0"
2525
},
2626
"dependencies": {
2727
"async": "^3.2.0",
@@ -41,7 +41,7 @@
4141
"mocha": "^8.2.1",
4242
"mock-require": "^3.0.3",
4343
"mongodb": "^3.6.3",
44-
"mongodb-connection-model": "^18.0.0",
44+
"mongodb-connection-model": "^17.3.0",
4545
"mongodb-js-precommit": "^2.2.1",
4646
"mongodb-runner": "^4.8.0",
4747
"xvfb-maybe": "^0.2.1"

test/instance-detail-helper-mocked.test.js

Lines changed: 0 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
const assert = require('assert');
2-
const { ReadPreference } = require('mongodb');
32
const {
43
getAllowedCollections,
54
getAllowedDatabases,
@@ -384,43 +383,6 @@ describe('instance-detail-helper-mocked', function() {
384383
done();
385384
});
386385
});
387-
it('should pass the read preference explicity when it is set', function(done) {
388-
let passedOptions;
389-
results.db = makeMockDB(function(times, command, options, callback) {
390-
if (command.listDatabases) {
391-
passedOptions = options;
392-
return callback(null, { databases: [] });
393-
}
394-
return callback(null, {});
395-
});
396-
results.db.s = {
397-
readPreference: ReadPreference.SECONDARY
398-
};
399-
400-
listDatabases(results, function() {
401-
assert.deepEqual(passedOptions, {
402-
readPreference: 'secondary'
403-
});
404-
done();
405-
});
406-
});
407-
it('defaults to passing the read preference as PRIMARY', function(done) {
408-
let passedOptions;
409-
results.db = makeMockDB(function(times, command, options, callback) {
410-
if (command.listDatabases) {
411-
passedOptions = options;
412-
return callback(null, { databases: [] });
413-
}
414-
return callback(null, {});
415-
});
416-
417-
listDatabases(results, function() {
418-
assert.deepEqual(passedOptions, {
419-
readPreference: ReadPreference.PRIMARY
420-
});
421-
done();
422-
});
423-
});
424386
it('should pass on other errors from the listDatabases command', function(done) {
425387
// instead of the real db handle, pass in the mocked one
426388
results.db = makeMockDB(

0 commit comments

Comments
 (0)