From 246669f748ff1bc42265ba80b53e2c8b46a9f8bc Mon Sep 17 00:00:00 2001
From: Eric Adum <eric.adum@mongodb.com>
Date: Mon, 11 May 2020 08:55:08 -0400
Subject: [PATCH] fix: honor journal=true in connection string (#2359)

Ensure the journal option returned by parseQueryString is converted
into j during the connect operation. This fixes the issue of the
write concern not being set on commands where journal is only
specified in the connection string.

NODE-2422
---
 lib/operations/connect.js             |  6 ++++
 test/functional/shared.js             | 50 +++++++++++++++++++++++++++
 test/functional/write_concern.test.js | 45 ++++++++++++++++++++++++
 3 files changed, 101 insertions(+)
 create mode 100644 test/functional/write_concern.test.js

diff --git a/lib/operations/connect.js b/lib/operations/connect.js
index bcdd3eb5b5..178077e11f 100644
--- a/lib/operations/connect.js
+++ b/lib/operations/connect.js
@@ -290,6 +290,12 @@ function connect(mongoClient, url, options, callback) {
       delete _finalOptions.db_options.auth;
     }
 
+    // `journal` should be translated to `j` for the driver
+    if (_finalOptions.journal != null) {
+      _finalOptions.j = _finalOptions.journal;
+      _finalOptions.journal = undefined;
+    }
+
     // resolve tls options if needed
     resolveTLSOptions(_finalOptions);
 
diff --git a/test/functional/shared.js b/test/functional/shared.js
index 106f234742..7b03073e43 100644
--- a/test/functional/shared.js
+++ b/test/functional/shared.js
@@ -86,6 +86,13 @@ function withTempDb(name, options, client, operation, errorHandler) {
   );
 }
 
+/**
+ * Safely perform a test with provided MongoClient, ensuring client won't leak.
+ *
+ * @param {MongoClient} client
+ * @param {Function|Promise} operation
+ * @param {Function|Promise} [errorHandler]
+ */
 function withClient(client, operation, errorHandler) {
   const cleanup = makeCleanupFn(client);
 
@@ -203,12 +210,55 @@ class EventCollector {
   }
 }
 
+/**
+ * Perform a test with a monitored MongoClient that will filter for certain commands.
+ *
+ * @param {string|Array} commands commands to filter for
+ * @param {object} [options] options to pass on to configuration.newClient
+ * @param {object} [options.queryOptions] connection string options
+ * @param {object} [options.clientOptions] MongoClient options
+ * @param {withMonitoredClientCallback} callback the test function
+ */
+function withMonitoredClient(commands, options, callback) {
+  if (arguments.length === 2) {
+    callback = options;
+    options = {};
+  }
+  if (!Object.prototype.hasOwnProperty.call(callback, 'prototype')) {
+    throw new Error('withMonitoredClient callback can not be arrow function');
+  }
+  return function(done) {
+    const configuration = this.configuration;
+    const client = configuration.newClient(
+      Object.assign({}, options.queryOptions),
+      Object.assign({ monitorCommands: true }, options.clientOptions)
+    );
+    const events = [];
+    client.on('commandStarted', filterForCommands(commands, events));
+    client.connect((err, client) => {
+      expect(err).to.not.exist;
+      function _done(err) {
+        client.close(err2 => done(err || err2));
+      }
+      callback.bind(this)(client, events, _done);
+    });
+  };
+}
+
+/**
+ * @callback withMonitoredClientCallback
+ * @param {MongoClient} client monitored client
+ * @param {Array} events record of monitored commands
+ * @param {Function} done trigger end of test and cleanup
+ */
+
 module.exports = {
   connectToDb,
   setupDatabase,
   assert,
   delay,
   withClient,
+  withMonitoredClient,
   withTempDb,
   filterForCommands,
   filterOutCommands,
diff --git a/test/functional/write_concern.test.js b/test/functional/write_concern.test.js
new file mode 100644
index 0000000000..4e7baa5f3f
--- /dev/null
+++ b/test/functional/write_concern.test.js
@@ -0,0 +1,45 @@
+'use strict';
+
+const chai = require('chai');
+const expect = chai.expect;
+chai.use(require('chai-subset'));
+const withMonitoredClient = require('./shared').withMonitoredClient;
+
+describe('Write Concern', function() {
+  describe('test journal connection string option', function() {
+    function journalOptionTest(client, events, done) {
+      expect(client).to.have.nested.property('s.options');
+      const clientOptions = client.s.options;
+      expect(clientOptions).property('j').to.be.true;
+      client
+        .db('test')
+        .collection('test')
+        .insertOne({ a: 1 }, (err, result) => {
+          expect(err).to.not.exist;
+          expect(result).to.exist;
+          expect(events)
+            .to.be.an('array')
+            .with.lengthOf(1);
+          expect(events[0]).to.containSubset({
+            commandName: 'insert',
+            command: {
+              writeConcern: { j: true }
+            }
+          });
+          done();
+        });
+    }
+
+    // baseline to confirm client option is working
+    it(
+      'should set write concern with j: true client option',
+      withMonitoredClient('insert', { clientOptions: { j: true } }, journalOptionTest)
+    );
+
+    // ensure query option in connection string passes through
+    it(
+      'should set write concern with journal=true connection string option',
+      withMonitoredClient('insert', { queryOptions: { journal: true } }, journalOptionTest)
+    );
+  });
+});