refactor(wire-protocol): reuse command where possible

Most of the 2.6 and 3.6 wire protocol implementations are using commands, but they don't internally reuse the available `command` method. This does that as much as possible, reducing a large amount of duplicate code, and improving readability. - converted `killCursors` to reuse `command` (3.2 only) - simplify `command` options handling - converted `getMore` to use `command (3.2 only) - fix mongos sharded tests to pass read preference in as option not in the command - converted write commands to use `command` internally (2.6+3.2) ` converted `query` to reuse `command` (3.2 only) - remove needless copy of options in `Query` class (fixed apm too) - refactor `databaseNamespace` and `collectionNamespace` (2.6+3.2)
mongodb-js · Jan 3, 2019 · e883c8d · e883c8d
1 parent 00c631e
commit e883c8d
Show file tree

Hide file tree

Showing 7 changed files with 158 additions and 225 deletions.
diff --git a/lib/connection/apm.js b/lib/connection/apm.js
@@ -90,8 +90,7 @@ const extractCommand = command => {
     }
 
     Object.keys(LEGACY_FIND_OPTIONS_MAP).forEach(key => {
-      if (typeof command.options[key] !== 'undefined')
-        result[LEGACY_FIND_OPTIONS_MAP[key]] = command.options[key];
+      if (typeof command[key] !== 'undefined') result[LEGACY_FIND_OPTIONS_MAP[key]] = command[key];
     });
 
     OP_QUERY_KEYS.forEach(key => {

diff --git a/lib/connection/commands.js b/lib/connection/commands.js
@@ -46,9 +46,6 @@ var Query = function(bson, ns, query, options) {
   this.ns = ns;
   this.query = query;
 
-  // Ensure empty options
-  this.options = options || {};
-
   // Additional options
   this.numberToSkip = options.numberToSkip || 0;
   this.numberToReturn = options.numberToReturn || 0;

diff --git a/lib/wireprotocol/2_6_support.js b/lib/wireprotocol/2_6_support.js
@@ -8,21 +8,23 @@ const MongoError = require('../error').MongoError;
 const getReadPreference = require('./shared').getReadPreference;
 const applyCommonQueryOptions = require('./shared').applyCommonQueryOptions;
 const isMongos = require('./shared').isMongos;
+const databaseNamespace = require('./shared').databaseNamespace;
+const collectionNamespace = require('./shared').collectionNamespace;
 
 const BSON = retrieveBSON();
 const Long = BSON.Long;
 
 class WireProtocol {
   insert(server, ns, ops, options, callback) {
-    executeWrite(server, 'insert', 'documents', ns, ops, options, callback);
+    executeWrite(this, server, 'insert', 'documents', ns, ops, options, callback);
   }
 
   update(server, ns, ops, options, callback) {
-    executeWrite(server, 'update', 'updates', ns, ops, options, callback);
+    executeWrite(this, server, 'update', 'updates', ns, ops, options, callback);
   }
 
   remove(server, ns, ops, options, callback) {
-    executeWrite(server, 'delete', 'deletes', ns, ops, options, callback);
+    executeWrite(this, server, 'delete', 'deletes', ns, ops, options, callback);
   }
 
   killCursor(server, ns, cursorState, callback) {
@@ -102,25 +104,21 @@ class WireProtocol {
     const bson = server.s.bson;
     const pool = server.s.pool;
     const readPreference = getReadPreference(cmd, options);
-    const parts = ns.split(/\./);
 
     let finalCmd = Object.assign({}, cmd);
-    const serializeFunctions =
-      typeof options.serializeFunctions === 'boolean' ? options.serializeFunctions : false;
-    const ignoreUndefined =
-      typeof options.ignoreUndefined === 'boolean' ? options.ignoreUndefined : false;
-
-    if (cmd.readConcern && cmd.readConcern.level !== 'local') {
-      return callback(
-        new MongoError(
-          `server ${JSON.stringify(cmd)} command does not support a readConcern level of ${
-            cmd.readConcern.level
-          }`
-        )
-      );
-    }
+    if (finalCmd.readConcern) {
+      if (finalCmd.readConcern.level !== 'local') {
+        return callback(
+          new MongoError(
+            `server ${JSON.stringify(finalCmd)} command does not support a readConcern level of ${
+              finalCmd.readConcern.level
+            }`
+          )
+        );
+      }
 
-    if (cmd.readConcern) delete cmd['readConcern'];
+      delete finalCmd['readConcern'];
+    }
 
     if (isMongos(server) && readPreference && readPreference.preference !== 'primary') {
       finalCmd = {
@@ -129,42 +127,39 @@ class WireProtocol {
       };
     }
 
-    const query = new Query(bson, `${parts.shift()}.$cmd`, finalCmd, {
-      numberToSkip: 0,
-      numberToReturn: -1,
-      checkKeys: false,
-      serializeFunctions: serializeFunctions,
-      ignoreUndefined: ignoreUndefined
-    });
-
-    query.slaveOk = readPreference.slaveOk();
+    const commandOptions = Object.assign(
+      {
+        command: true,
+        slaveOk: readPreference.slaveOk(),
+        numberToSkip: 0,
+        numberToReturn: -1,
+        checkKeys: false
+      },
+      options
+    );
 
-    const queryOptions = applyCommonQueryOptions({ command: true }, options);
-    if (typeof query.documentsReturnedIn === 'string') {
-      queryOptions.documentsReturnedIn = query.documentsReturnedIn;
+    try {
+      const query = new Query(bson, `${databaseNamespace(ns)}.$cmd`, finalCmd, commandOptions);
+      pool.write(query, commandOptions, callback);
+    } catch (err) {
+      callback(err);
     }
-
-    pool.write(query, queryOptions, callback);
   }
 }
 
-function executeWrite(server, type, opsField, ns, ops, options, callback) {
+function executeWrite(handler, server, type, opsField, ns, ops, options, callback) {
   if (ops.length === 0) throw new MongoError('insert must contain at least one document');
   if (typeof options === 'function') {
     callback = options;
     options = {};
     options = options || {};
   }
 
-  const bson = server.s.bson;
-  const pool = server.s.pool;
-  const p = ns.split('.');
-  const d = p.shift();
   const ordered = typeof options.ordered === 'boolean' ? options.ordered : true;
   const writeConcern = options.writeConcern;
 
   const writeCommand = {};
-  writeCommand[type] = p.join('.');
+  writeCommand[type] = collectionNamespace(ns);
   writeCommand[opsField] = ops;
   writeCommand.ordered = ordered;
 
@@ -176,20 +171,15 @@ function executeWrite(server, type, opsField, ns, ops, options, callback) {
     writeCommand.bypassDocumentValidation = options.bypassDocumentValidation;
   }
 
-  const opts = { command: true };
-  if (typeof options.session !== 'undefined') opts.session = options.session;
-  const queryOptions = { checkKeys: false, numberToSkip: 0, numberToReturn: 1 };
-  if (type === 'insert') queryOptions.checkKeys = true;
-  if (typeof options.checkKeys === 'boolean') queryOptions.checkKeys = options.checkKeys;
-  if (options.serializeFunctions) queryOptions.serializeFunctions = options.serializeFunctions;
-  if (options.ignoreUndefined) queryOptions.ignoreUndefined = options.ignoreUndefined;
-
-  try {
-    const cmd = new Query(bson, `${d}.$cmd`, writeCommand, queryOptions);
-    pool.write(cmd, opts, callback);
-  } catch (err) {
-    callback(err);
-  }
+  const commandOptions = Object.assign(
+    {
+      checkKeys: type === 'insert',
+      numberToReturn: 1
+    },
+    options
+  );
+
+  handler.command(server, ns, writeCommand, commandOptions, callback);
 }
 
 function setupClassicFind(server, ns, cmd, cursorState, options) {