fix(utils): only set retryWrites to true for valid operations

Fixes NODE-1641

Author: kvwalker

lib/bulk/common.js

@@ -4,8 +4,8 @@ const Long = require('mongodb-core').BSON.Long;
 const MongoError = require('mongodb-core').MongoError;
 const toError = require('../utils').toError;
 const handleCallback = require('../utils').handleCallback;
+const applyRetryableWrites = require('../utils').applyRetryableWrites;
 const applyWriteConcern = require('../utils').applyWriteConcern;
-const shallowClone = require('../utils').shallowClone;
 const ObjectID = require('mongodb-core').BSON.ObjectID;
 const BSON = require('mongodb-core').BSON;
 
@@ -702,13 +702,11 @@ class BulkOperationBase {
     const maxWriteBatchSize =
       isMaster && isMaster.maxWriteBatchSize ? isMaster.maxWriteBatchSize : 1000;
 
-    // Get the write concern
-    let writeConcern = applyWriteConcern(
-      shallowClone(options),
-      { collection: collection },
-      options
-    );
-    writeConcern = writeConcern.writeConcern;
+    // Final options for retryable writes and write concern
+    let finalOptions = Object.assign({}, options);
+    finalOptions = applyRetryableWrites(finalOptions, collection.s.db);
+    finalOptions = applyWriteConcern(finalOptions, { collection: collection }, options);
+    const writeConcern = finalOptions.writeConcern;
 
     // Get the promiseLibrary
     const promiseLibrary = options.promiseLibrary || Promise;
@@ -754,7 +752,7 @@ class BulkOperationBase {
       // Topology
       topology: topology,
       // Options
-      options: options,
+      options: finalOptions,
       // Current operation
       currentOp: currentOp,
       // Executed

lib/operations/collection_ops.js

@@ -1,6 +1,7 @@
 'use strict';
 
 const applyWriteConcern = require('../utils').applyWriteConcern;
+const applyRetryableWrites = require('../utils').applyRetryableWrites;
 const checkCollectionName = require('../utils').checkCollectionName;
 const Code = require('mongodb-core').BSON.Code;
 const createIndexDb = require('./db_ops').createIndex;
@@ -97,12 +98,10 @@ function bulkWrite(coll, operations, options, callback) {
     return callback(err, null);
   }
 
-  // Final options for write concern
-  const finalOptions = applyWriteConcern(
-    Object.assign({}, options),
-    { db: coll.s.db, collection: coll },
-    options
-  );
+  // Final options for retryable writes and write concern
+  let finalOptions = Object.assign({}, options);
+  finalOptions = applyRetryableWrites(finalOptions, coll.s.db);
+  finalOptions = applyWriteConcern(finalOptions, { db: coll.s.db, collection: coll }, options);
 
   const writeCon = finalOptions.writeConcern ? finalOptions.writeConcern : {};
   const capabilities = coll.s.topology.capabilities();
@@ -504,8 +503,10 @@ function findAndModify(coll, query, sort, doc, options, callback) {
   // No check on the documents
   options.checkKeys = false;
 
-  // Get the write concern settings
-  const finalOptions = applyWriteConcern(options, { db: coll.s.db, collection: coll }, options);
+  // Final options for retryable writes and write concern
+  let finalOptions = Object.assign({}, options);
+  finalOptions = applyRetryableWrites(finalOptions, coll.s.db);
+  finalOptions = applyWriteConcern(finalOptions, { db: coll.s.db, collection: coll }, options);
 
   // Decorate the findAndModify command with the write Concern
   if (finalOptions.writeConcern) {
@@ -805,12 +806,10 @@ function insertDocuments(coll, docs, options, callback) {
   // Ensure we are operating on an array op docs
   docs = Array.isArray(docs) ? docs : [docs];
 
-  // Get the write concern options
-  const finalOptions = applyWriteConcern(
-    Object.assign({}, options),
-    { db: coll.s.db, collection: coll },
-    options
-  );
+  // Final options for retryable writes and write concern
+  let finalOptions = Object.assign({}, options);
+  finalOptions = applyRetryableWrites(finalOptions, coll.s.db);
+  finalOptions = applyWriteConcern(finalOptions, { db: coll.s.db, collection: coll }, options);
 
   // If keep going set unordered
   if (finalOptions.keepGoing === true) finalOptions.ordered = false;
@@ -1138,12 +1137,10 @@ function removeDocuments(coll, selector, options, callback) {
   // Create an empty options object if the provided one is null
   options = options || {};
 
-  // Get the write concern options
-  const finalOptions = applyWriteConcern(
-    Object.assign({}, options),
-    { db: coll.s.db, collection: coll },
-    options
-  );
+  // Final options for retryable writes and write concern
+  let finalOptions = Object.assign({}, options);
+  finalOptions = applyRetryableWrites(finalOptions, coll.s.db);
+  finalOptions = applyWriteConcern(finalOptions, { db: coll.s.db, collection: coll }, options);
 
   // If selector is null set empty
   if (selector == null) selector = {};
@@ -1336,12 +1333,10 @@ function updateDocuments(coll, selector, document, options, callback) {
   if (document == null || typeof document !== 'object')
     return callback(toError('document must be a valid JavaScript object'));
 
-  // Get the write concern options
-  const finalOptions = applyWriteConcern(
-    Object.assign({}, options),
-    { db: coll.s.db, collection: coll },
-    options
-  );
+  // Final options for retryable writes and write concern
+  let finalOptions = Object.assign({}, options);
+  finalOptions = applyRetryableWrites(finalOptions, coll.s.db);
+  finalOptions = applyWriteConcern(finalOptions, { db: coll.s.db, collection: coll }, options);
 
   // Do we return the actual result document
   // Either use override on the function, or go back to default on either the collection

lib/utils.js

@@ -441,6 +441,20 @@ const executeOperation = (topology, operation, args, options) => {
   });
 };
 
+/**
+ * Applies retryWrites: true to a command if retryWrites is set on the command's database.
+ *
+ * @param {object} target The target command to which we will apply retryWrites.
+ * @param {object} db The database from which we can inherit a retryWrites value.
+ */
+function applyRetryableWrites(target, db) {
+  if (db && db.s.options.retryWrites) {
+    target.retryWrites = true;
+  }
+
+  return target;
+}
+
 /**
  * Applies a write concern to a command based on well defined inheritance rules, optionally
  * detecting support for the write concern in the first place.
@@ -455,11 +469,6 @@ function applyWriteConcern(target, sources, options) {
   const db = sources.db;
   const coll = sources.collection;
 
-  // NOTE: there is probably a much better place for this
-  if (db && db.s.options.retryWrites) {
-    target.retryWrites = true;
-  }
-
   if (options.session && options.session.inTransaction()) {
     // writeConcern is not allowed within a multi-statement transaction
     if (target.writeConcern) {
@@ -701,6 +710,7 @@ module.exports = {
   mergeOptionsAndWriteConcern,
   translateReadPreference,
   executeOperation,
+  applyRetryableWrites,
   applyWriteConcern,
   resolveReadPreference,
   isPromiseLike,