refactor(command): move common command construction to base cmd op

Author: mbroadst

lib/operations/aggregate.js

@@ -61,26 +61,16 @@ class AggregateOperation extends CommandOperationV2 {
   execute(server, callback) {
     const options = this.options;
     const serverWireVersion = maxWireVersion(server);
-    const inTransaction = this.session && this.session.inTransaction();
-
     const command = { aggregate: this.target, pipeline: this.pipeline };
 
-    if (
-      this.readConcern &&
-      (!this.hasWriteStage || serverWireVersion >= MIN_WIRE_VERSION_$OUT_READ_CONCERN_SUPPORT) &&
-      !inTransaction
-    ) {
-      Object.assign(command, { readConcern: this.readConcern });
+    if (this.hasWriteStage && serverWireVersion < MIN_WIRE_VERSION_$OUT_READ_CONCERN_SUPPORT) {
+      this.readConcern = null;
     }
 
     if (serverWireVersion >= 5) {
       if (this.hasWriteStage && this.writeConcern) {
         Object.assign(command, { writeConcern: this.writeConcern });
       }
-
-      if (options.collation && typeof options.collation === 'object') {
-        Object.assign(command, { collation: options.collation });
-      }
     }
 
     if (options.bypassDocumentValidation === true) {
@@ -91,10 +81,6 @@ class AggregateOperation extends CommandOperationV2 {
       command.allowDiskUse = options.allowDiskUse;
     }
 
-    if (typeof options.maxTimeMS === 'number') {
-      command.maxTimeMS = options.maxTimeMS;
-    }
-
     if (options.hint) {
       command.hint = options.hint;
     }
@@ -104,10 +90,6 @@ class AggregateOperation extends CommandOperationV2 {
       command.explain = options.explain;
     }
 
-    if (typeof options.comment === 'string') {
-      command.comment = options.comment;
-    }
-
     command.cursor = options.cursor || {};
     if (options.batchSize && !this.hasWriteStage) {
       command.cursor.batchSize = options.batchSize;

lib/operations/command_v2.js

@@ -1,9 +1,11 @@
 'use strict';
 
+const Aspect = require('./operation').Aspect;
 const OperationBase = require('./operation').OperationBase;
 const resolveReadPreference = require('../utils').resolveReadPreference;
 const ReadConcern = require('../read_concern');
 const WriteConcern = require('../write_concern');
+const maxWireVersion = require('../core/utils').maxWireVersion;
 
 class CommandOperationV2 extends OperationBase {
   constructor(parent, options) {
@@ -31,6 +33,32 @@ class CommandOperationV2 extends OperationBase {
     // TODO: consider making this a non-enumerable property
     this.server = server;
 
+    const options = this.options;
+    const serverWireVersion = maxWireVersion(server);
+    const inTransaction = this.session && this.session.inTransaction();
+
+    if (this.readConcern && this.hasAspect(Aspect.READ_OPERATION) && !inTransaction) {
+      Object.assign(cmd, { readConcern: this.readConcern });
+    }
+
+    if (serverWireVersion >= 5) {
+      if (this.writeConcern && this.hasAspect(Aspect.WRITE_OPERATION)) {
+        Object.assign(cmd, { writeConcern: this.writeConcern });
+      }
+
+      if (options.collation && typeof options.collation === 'object') {
+        Object.assign(cmd, { collation: options.collation });
+      }
+    }
+
+    if (typeof options.maxTimeMS === 'number') {
+      cmd.maxTimeMS = options.maxTimeMS;
+    }
+
+    if (typeof options.comment === 'string') {
+      cmd.comment = options.comment;
+    }
+
     if (this.logger && this.logger.isDebug()) {
       this.logger.debug(`executing command ${JSON.stringify(cmd)} against ${this.ns}`);
     }