fix: db.command to not inherit options from parent

Thomas Reggi · web-flow · commit f7a50e2f4337 · 2020-07-14T10:11:21.000-04:00
Fixes `db.command` method to not inherit readPreference, writeConcern, readConcern from parent. centralizes readPreference helpers translate, resolve, fromOptions to the ReadPreference class. Adds Aspect "NO_INHERIT_OPTIONS" to command operations for flagging specific operations. Creates `RunCommand` operation to wrap `CommandV2`, updating `db.command` to use `CommandV2`. NODE-2649
diff --git a/lib/collection.js b/lib/collection.js
@@ -16,7 +16,6 @@ const unordered = require('./bulk/unordered');
 const ordered = require('./bulk/ordered');
 const ChangeStream = require('./change_stream');
 const executeLegacyOperation = require('./utils').executeLegacyOperation;
-const resolveReadPreference = require('./utils').resolveReadPreference;
 const WriteConcern = require('./write_concern');
 const ReadConcern = require('./read_concern');
 const MongoDBNamespace = require('./utils').MongoDBNamespace;
@@ -399,7 +398,7 @@ Collection.prototype.find = deprecateOptions(
     newOptions.slaveOk = options.slaveOk != null ? options.slaveOk : this.s.db.slaveOk;
 
     // Add read preference if needed
-    newOptions.readPreference = resolveReadPreference(this, newOptions);
+    newOptions.readPreference = ReadPreference.resolve(this, newOptions);
 
     // Set slave ok to true if read preference different from primary
     if (
@@ -1978,7 +1977,7 @@ Collection.prototype.parallelCollectionScan = deprecate(function(options, callba
 
   options = Object.assign({}, options);
   // Ensure we have the right read preference inheritance
-  options.readPreference = resolveReadPreference(this, options);
+  options.readPreference = ReadPreference.resolve(this, options);
 
   // Add a promiseLibrary
   options.promiseLibrary = this.s.promiseLibrary;
diff --git a/lib/core/sdam/topology.js b/lib/core/sdam/topology.js
@@ -275,7 +275,7 @@ class Topology extends EventEmitter {
     // connect all known servers, then attempt server selection to connect
     connectServers(this, Array.from(this.s.description.servers.values()));
 
-    translateReadPreference(options);
+    ReadPreference.translate(options);
     const readPreference = options.readPreference || ReadPreference.primary;
     this.selectServer(readPreferenceServerSelector(readPreference), options, err => {
       if (err) {
@@ -381,7 +381,7 @@ class Topology extends EventEmitter {
         } else if (typeof selector === 'string') {
           readPreference = new ReadPreference(selector);
         } else {
-          translateReadPreference(options);
+          ReadPreference.translate(options);
           readPreference = options.readPreference || ReadPreference.primary;
         }
 
@@ -647,7 +647,7 @@ class Topology extends EventEmitter {
       (callback = options), (options = {}), (options = options || {});
     }
 
-    translateReadPreference(options);
+    ReadPreference.translate(options);
     const readPreference = options.readPreference || ReadPreference.primary;
 
     this.selectServer(readPreferenceServerSelector(readPreference), options, (err, server) => {
@@ -708,7 +708,7 @@ class Topology extends EventEmitter {
     options = options || {};
     const topology = options.topology || this;
     const CursorClass = options.cursorFactory || this.s.Cursor;
-    translateReadPreference(options);
+    ReadPreference.translate(options);
 
     return new CursorClass(topology, ns, cmd, options);
   }
@@ -939,28 +939,6 @@ function executeWriteOperation(args, options, callback) {
   });
 }
 
-function translateReadPreference(options) {
-  if (options.readPreference == null) {
-    return;
-  }
-
-  let r = options.readPreference;
-  if (typeof r === 'string') {
-    options.readPreference = new ReadPreference(r);
-  } else if (r && !(r instanceof ReadPreference) && typeof r === 'object') {
-    const mode = r.mode || r.preference;
-    if (mode && typeof mode === 'string') {
-      options.readPreference = new ReadPreference(mode, r.tags, {
-        maxStalenessSeconds: r.maxStalenessSeconds
-      });
-    }
-  } else if (!(r instanceof ReadPreference)) {
-    throw new TypeError('Invalid read preference: ' + r);
-  }
-
-  return options;
-}
-
 function srvPollingHandler(topology) {
   return function handleSrvPolling(ev) {
     const previousTopologyDescription = topology.s.description;
diff --git a/lib/core/topologies/read_preference.js b/lib/core/topologies/read_preference.js
@@ -112,6 +112,60 @@ ReadPreference.fromOptions = function(options) {
   return readPreference;
 };
 
+/**
+ * Resolves a read preference based on well-defined inheritance rules. This method will not only
+ * determine the read preference (if there is one), but will also ensure the returned value is a
+ * properly constructed instance of `ReadPreference`.
+ *
+ * @param {Collection|Db|MongoClient} parent The parent of the operation on which to determine the read
+ * preference, used for determining the inherited read preference.
+ * @param {object} options The options passed into the method, potentially containing a read preference
+ * @returns {(ReadPreference|null)} The resolved read preference
+ */
+ReadPreference.resolve = function(parent, options) {
+  options = options || {};
+  const session = options.session;
+
+  const inheritedReadPreference = parent && parent.readPreference;
+
+  let readPreference;
+  if (options.readPreference) {
+    readPreference = ReadPreference.fromOptions(options);
+  } else if (session && session.inTransaction() && session.transaction.options.readPreference) {
+    // The transaction’s read preference MUST override all other user configurable read preferences.
+    readPreference = session.transaction.options.readPreference;
+  } else if (inheritedReadPreference != null) {
+    readPreference = inheritedReadPreference;
+  } else {
+    readPreference = ReadPreference.primary;
+  }
+
+  return typeof readPreference === 'string' ? new ReadPreference(readPreference) : readPreference;
+};
+
+/**
+ * Replaces options.readPreference with a ReadPreference instance
+ */
+ReadPreference.translate = function(options) {
+  if (options.readPreference == null) return options;
+  const r = options.readPreference;
+
+  if (typeof r === 'string') {
+    options.readPreference = new ReadPreference(r);
+  } else if (r && !(r instanceof ReadPreference) && typeof r === 'object') {
+    const mode = r.mode || r.preference;
+    if (mode && typeof mode === 'string') {
+      options.readPreference = new ReadPreference(mode, r.tags, {
+        maxStalenessSeconds: r.maxStalenessSeconds
+      });
+    }
+  } else if (!(r instanceof ReadPreference)) {
+    throw new TypeError('Invalid read preference: ' + r);
+  }
+
+  return options;
+};
+
 /**
  * Validate if a mode is legal
  *
diff --git a/lib/core/wireprotocol/command.js b/lib/core/wireprotocol/command.js
@@ -66,12 +66,7 @@ function _command(server, ns, cmd, options, callback) {
     finalCmd.$clusterTime = clusterTime;
   }
 
-  if (
-    isSharded(server) &&
-    !shouldUseOpMsg &&
-    readPreference &&
-    readPreference.preference !== 'primary'
-  ) {
+  if (isSharded(server) && !shouldUseOpMsg && readPreference && readPreference.mode !== 'primary') {
     finalCmd = {
       $query: finalCmd,
       $readPreference: readPreference.toJSON()
diff --git a/lib/db.js b/lib/db.js
@@ -14,7 +14,6 @@ const Logger = require('./core').Logger;
 const Collection = require('./collection');
 const mergeOptionsAndWriteConcern = require('./utils').mergeOptionsAndWriteConcern;
 const executeLegacyOperation = require('./utils').executeLegacyOperation;
-const resolveReadPreference = require('./utils').resolveReadPreference;
 const ChangeStream = require('./change_stream');
 const deprecate = require('util').deprecate;
 const deprecateOptions = require('./utils').deprecateOptions;
@@ -35,6 +34,7 @@ const AggregateOperation = require('./operations/aggregate');
 const AddUserOperation = require('./operations/add_user');
 const CollectionsOperation = require('./operations/collections');
 const CommandOperation = require('./operations/command');
+const RunCommandOperation = require('./operations/run_command');
 const CreateCollectionOperation = require('./operations/create_collection');
 const CreateIndexOperation = require('./operations/create_index');
 const DropCollectionOperation = require('./operations/drop').DropCollectionOperation;
@@ -290,7 +290,7 @@ Db.prototype.command = function(command, options, callback) {
   if (typeof options === 'function') (callback = options), (options = {});
   options = Object.assign({}, options);
 
-  const commandOperation = new CommandOperation(this, options, null, command);
+  const commandOperation = new RunCommandOperation(this, command, options);
 
   return executeOperation(this.s.topology, commandOperation, callback);
 };
@@ -709,7 +709,7 @@ Db.prototype.collections = function(options, callback) {
 Db.prototype.executeDbAdminCommand = function(selector, options, callback) {
   if (typeof options === 'function') (callback = options), (options = {});
   options = options || {};
-  options.readPreference = resolveReadPreference(this, options);
+  options.readPreference = ReadPreference.resolve(this, options);
 
   const executeDbAdminCommandOperation = new ExecuteDbAdminCommandOperation(
     this,
diff --git a/lib/operations/collection_ops.js b/lib/operations/collection_ops.js
@@ -8,7 +8,6 @@ const decorateWithReadConcern = require('../utils').decorateWithReadConcern;
 const ensureIndexDb = require('./db_ops').ensureIndex;
 const evaluate = require('./db_ops').evaluate;
 const executeCommand = require('./db_ops').executeCommand;
-const resolveReadPreference = require('../utils').resolveReadPreference;
 const handleCallback = require('../utils').handleCallback;
 const indexInformationDb = require('./db_ops').indexInformation;
 const Long = require('../core').BSON.Long;
@@ -188,7 +187,7 @@ function group(coll, keys, condition, initial, reduce, finalize, command, option
 
     options = Object.assign({}, options);
     // Ensure we have the right read preference inheritance
-    options.readPreference = resolveReadPreference(coll, options);
+    options.readPreference = ReadPreference.resolve(coll, options);
 
     // Do we have a readConcern specified
     decorateWithReadConcern(selector, coll, options);
diff --git a/lib/operations/command.js b/lib/operations/command.js
@@ -7,7 +7,6 @@ const debugOptions = require('../utils').debugOptions;
 const handleCallback = require('../utils').handleCallback;
 const MongoError = require('../core').MongoError;
 const ReadPreference = require('../core').ReadPreference;
-const resolveReadPreference = require('../utils').resolveReadPreference;
 const MongoDBNamespace = require('../utils').MongoDBNamespace;
 
 const debugFields = [
@@ -38,9 +37,9 @@ class CommandOperation extends OperationBase {
 
     if (!this.hasAspect(Aspect.WRITE_OPERATION)) {
       if (collection != null) {
-        this.options.readPreference = resolveReadPreference(collection, options);
+        this.options.readPreference = ReadPreference.resolve(collection, options);
       } else {
-        this.options.readPreference = resolveReadPreference(db, options);
+        this.options.readPreference = ReadPreference.resolve(db, options);
       }
     } else {
       if (collection != null) {
diff --git a/lib/operations/command_v2.js b/lib/operations/command_v2.js
@@ -2,7 +2,7 @@
 
 const Aspect = require('./operation').Aspect;
 const OperationBase = require('./operation').OperationBase;
-const resolveReadPreference = require('../utils').resolveReadPreference;
+const ReadPreference = require('../core').ReadPreference;
 const ReadConcern = require('../read_concern');
 const WriteConcern = require('../write_concern');
 const maxWireVersion = require('../core/utils').maxWireVersion;
@@ -16,9 +16,10 @@ class CommandOperationV2 extends OperationBase {
     super(options);
 
     this.ns = parent.s.namespace.withCollection('$cmd');
-    this.readPreference = resolveReadPreference(parent, this.options);
-    this.readConcern = resolveReadConcern(parent, this.options);
-    this.writeConcern = resolveWriteConcern(parent, this.options);
+    const propertyProvider = this.hasAspect(Aspect.NO_INHERIT_OPTIONS) ? undefined : parent;
+    this.readPreference = ReadPreference.resolve(propertyProvider, this.options);
+    this.readConcern = resolveReadConcern(propertyProvider, this.options);
+    this.writeConcern = resolveWriteConcern(propertyProvider, this.options);
     this.explain = false;
 
     if (operationOptions && typeof operationOptions.fullResponse === 'boolean') {
@@ -97,11 +98,11 @@ class CommandOperationV2 extends OperationBase {
 }
 
 function resolveWriteConcern(parent, options) {
-  return WriteConcern.fromOptions(options) || parent.writeConcern;
+  return WriteConcern.fromOptions(options) || (parent && parent.writeConcern);
 }
 
 function resolveReadConcern(parent, options) {
-  return ReadConcern.fromOptions(options) || parent.readConcern;
+  return ReadConcern.fromOptions(options) || (parent && parent.readConcern);
 }
 
 module.exports = CommandOperationV2;
diff --git a/lib/operations/db_ops.js b/lib/operations/db_ops.js
@@ -2,7 +2,6 @@
 
 const applyWriteConcern = require('../utils').applyWriteConcern;
 const Code = require('../core').BSON.Code;
-const resolveReadPreference = require('../utils').resolveReadPreference;
 const debugOptions = require('../utils').debugOptions;
 const handleCallback = require('../utils').handleCallback;
 const MongoError = require('../core').MongoError;
@@ -225,7 +224,7 @@ function executeCommand(db, command, options, callback) {
   const dbName = options.dbName || options.authdb || db.databaseName;
 
   // Convert the readPreference if its not a write
-  options.readPreference = resolveReadPreference(db, options);
+  options.readPreference = ReadPreference.resolve(db, options);
 
   // Debug information
   if (db.s.logger.isDebug())
diff --git a/lib/operations/find.js b/lib/operations/find.js
@@ -3,15 +3,15 @@
 const OperationBase = require('./operation').OperationBase;
 const Aspect = require('./operation').Aspect;
 const defineAspects = require('./operation').defineAspects;
-const resolveReadPreference = require('../utils').resolveReadPreference;
+const ReadPreference = require('../core').ReadPreference;
 
 class FindOperation extends OperationBase {
   constructor(collection, ns, command, options) {
     super(options);
 
     this.ns = ns;
     this.cmd = command;
-    this.readPreference = resolveReadPreference(collection, this.options);
+    this.readPreference = ReadPreference.resolve(collection, this.options);
   }
 
   execute(server, callback) {
diff --git a/lib/operations/geo_haystack_search.js b/lib/operations/geo_haystack_search.js
@@ -7,7 +7,7 @@ const decorateCommand = require('../utils').decorateCommand;
 const decorateWithReadConcern = require('../utils').decorateWithReadConcern;
 const executeCommand = require('./db_ops').executeCommand;
 const handleCallback = require('../utils').handleCallback;
-const resolveReadPreference = require('../utils').resolveReadPreference;
+const ReadPreference = require('../core').ReadPreference;
 const toError = require('../utils').toError;
 
 /**
@@ -58,7 +58,7 @@ class GeoHaystackSearchOperation extends OperationBase {
 
     options = Object.assign({}, options);
     // Ensure we have the right read preference inheritance
-    options.readPreference = resolveReadPreference(coll, options);
+    options.readPreference = ReadPreference.resolve(coll, options);
 
     // Do we have a readConcern specified
     decorateWithReadConcern(commandObject, coll, options);
diff --git a/lib/operations/map_reduce.js b/lib/operations/map_reduce.js
@@ -9,7 +9,7 @@ const handleCallback = require('../utils').handleCallback;
 const isObject = require('../utils').isObject;
 const loadDb = require('../dynamic_loaders').loadDb;
 const OperationBase = require('./operation').OperationBase;
-const resolveReadPreference = require('../utils').resolveReadPreference;
+const ReadPreference = require('../core').ReadPreference;
 const toError = require('../utils').toError;
 
 const exclusionList = [
@@ -80,7 +80,7 @@ class MapReduceOperation extends OperationBase {
     options = Object.assign({}, options);
 
     // Ensure we have the right read preference inheritance
-    options.readPreference = resolveReadPreference(coll, options);
+    options.readPreference = ReadPreference.resolve(coll, options);
 
     // If we have a read preference and inline is not set as output fail hard
     if (
diff --git a/lib/operations/operation.js b/lib/operations/operation.js
@@ -4,7 +4,8 @@ const Aspect = {
   READ_OPERATION: Symbol('READ_OPERATION'),
   WRITE_OPERATION: Symbol('WRITE_OPERATION'),
   RETRYABLE: Symbol('RETRYABLE'),
-  EXECUTE_WITH_SELECTION: Symbol('EXECUTE_WITH_SELECTION')
+  EXECUTE_WITH_SELECTION: Symbol('EXECUTE_WITH_SELECTION'),
+  NO_INHERIT_OPTIONS: Symbol('NO_INHERIT_OPTIONS')
 };
 
 /**
diff --git a/lib/operations/run_command.js b/lib/operations/run_command.js
@@ -0,0 +1,19 @@
+'use strict';
+
+const CommandOperationV2 = require('./command_v2');
+const defineAspects = require('./operation').defineAspects;
+const Aspect = require('./operation').Aspect;
+
+class RunCommandOperation extends CommandOperationV2 {
+  constructor(parent, command, options) {
+    super(parent, options);
+    this.command = command;
+  }
+  execute(server, callback) {
+    const command = this.command;
+    this.executeCommand(server, command, callback);
+  }
+}
+defineAspects(RunCommandOperation, [Aspect.EXECUTE_WITH_SELECTION, Aspect.NO_INHERIT_OPTIONS]);
+
+module.exports = RunCommandOperation;
diff --git a/lib/topologies/topology_base.js b/lib/topologies/topology_base.js
diff --git a/lib/utils.js b/lib/utils.js
