fix: honor journal=true in connection string

Ensure the journal option returned byparseQueryString 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
mongodb · May 7, 2020 · 41d291a · 41d291a
1 parent d3bd81f
commit 41d291a
Show file tree

Hide file tree

Showing 3 changed files with 79 additions and 2 deletions.
diff --git a/lib/operations/connect.js b/lib/operations/connect.js
@@ -213,6 +213,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
@@ -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,13 +210,29 @@ class EventCollector {
   }
 }
 
-function withMonitoredClient(commands, callback) {
+/**
+ * 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({ monitorCommands: true });
+    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) => {
@@ -222,6 +245,13 @@ function withMonitoredClient(commands, callback) {
   };
 }
 
+/**
+ * @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,

diff --git a/test/functional/write_concern.test.js b/test/functional/write_concern.test.js
@@ -1,8 +1,11 @@
 'use strict';
 
+const chai = require('chai');
+const expect = chai.expect;
 const TestRunnerContext = require('./spec-runner').TestRunnerContext;
 const generateTopologyTests = require('./spec-runner').generateTopologyTests;
 const loadSpecTests = require('../spec').loadSpecTests;
+const { withMonitoredClient } = require('./shared');
 
 describe('Write Concern', function() {
   describe('spec tests', function() {
@@ -16,4 +19,42 @@ describe('Write Concern', function() {
 
     generateTopologyTests(testSuites, testContext);
   });
+
+  // TODO: once `read-write-concern/connection-string` spec tests are implemented these can likely be removed
+  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).to.containSubset({ j: 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)
+    );
+  });
 });