Skip to content

Commit

Permalink
fix: honor journal=true in connection string (#2359)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
emadum committed May 11, 2020
1 parent 1e3b4c9 commit 246669f
Show file tree
Hide file tree
Showing 3 changed files with 101 additions and 0 deletions.
6 changes: 6 additions & 0 deletions lib/operations/connect.js
Expand Up @@ -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);

Expand Down
50 changes: 50 additions & 0 deletions test/functional/shared.js
Expand Up @@ -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);

Expand Down Expand Up @@ -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,
Expand Down
45 changes: 45 additions & 0 deletions 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)
);
});
});

0 comments on commit 246669f

Please sign in to comment.