Skip to content

Commit 246669f

Browse files
authored
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
1 parent 1e3b4c9 commit 246669f

File tree

3 files changed

+101
-0
lines changed

3 files changed

+101
-0
lines changed

lib/operations/connect.js

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -290,6 +290,12 @@ function connect(mongoClient, url, options, callback) {
290290
delete _finalOptions.db_options.auth;
291291
}
292292

293+
// `journal` should be translated to `j` for the driver
294+
if (_finalOptions.journal != null) {
295+
_finalOptions.j = _finalOptions.journal;
296+
_finalOptions.journal = undefined;
297+
}
298+
293299
// resolve tls options if needed
294300
resolveTLSOptions(_finalOptions);
295301

test/functional/shared.js

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,13 @@ function withTempDb(name, options, client, operation, errorHandler) {
8686
);
8787
}
8888

89+
/**
90+
* Safely perform a test with provided MongoClient, ensuring client won't leak.
91+
*
92+
* @param {MongoClient} client
93+
* @param {Function|Promise} operation
94+
* @param {Function|Promise} [errorHandler]
95+
*/
8996
function withClient(client, operation, errorHandler) {
9097
const cleanup = makeCleanupFn(client);
9198

@@ -203,12 +210,55 @@ class EventCollector {
203210
}
204211
}
205212

213+
/**
214+
* Perform a test with a monitored MongoClient that will filter for certain commands.
215+
*
216+
* @param {string|Array} commands commands to filter for
217+
* @param {object} [options] options to pass on to configuration.newClient
218+
* @param {object} [options.queryOptions] connection string options
219+
* @param {object} [options.clientOptions] MongoClient options
220+
* @param {withMonitoredClientCallback} callback the test function
221+
*/
222+
function withMonitoredClient(commands, options, callback) {
223+
if (arguments.length === 2) {
224+
callback = options;
225+
options = {};
226+
}
227+
if (!Object.prototype.hasOwnProperty.call(callback, 'prototype')) {
228+
throw new Error('withMonitoredClient callback can not be arrow function');
229+
}
230+
return function(done) {
231+
const configuration = this.configuration;
232+
const client = configuration.newClient(
233+
Object.assign({}, options.queryOptions),
234+
Object.assign({ monitorCommands: true }, options.clientOptions)
235+
);
236+
const events = [];
237+
client.on('commandStarted', filterForCommands(commands, events));
238+
client.connect((err, client) => {
239+
expect(err).to.not.exist;
240+
function _done(err) {
241+
client.close(err2 => done(err || err2));
242+
}
243+
callback.bind(this)(client, events, _done);
244+
});
245+
};
246+
}
247+
248+
/**
249+
* @callback withMonitoredClientCallback
250+
* @param {MongoClient} client monitored client
251+
* @param {Array} events record of monitored commands
252+
* @param {Function} done trigger end of test and cleanup
253+
*/
254+
206255
module.exports = {
207256
connectToDb,
208257
setupDatabase,
209258
assert,
210259
delay,
211260
withClient,
261+
withMonitoredClient,
212262
withTempDb,
213263
filterForCommands,
214264
filterOutCommands,

test/functional/write_concern.test.js

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
'use strict';
2+
3+
const chai = require('chai');
4+
const expect = chai.expect;
5+
chai.use(require('chai-subset'));
6+
const withMonitoredClient = require('./shared').withMonitoredClient;
7+
8+
describe('Write Concern', function() {
9+
describe('test journal connection string option', function() {
10+
function journalOptionTest(client, events, done) {
11+
expect(client).to.have.nested.property('s.options');
12+
const clientOptions = client.s.options;
13+
expect(clientOptions).property('j').to.be.true;
14+
client
15+
.db('test')
16+
.collection('test')
17+
.insertOne({ a: 1 }, (err, result) => {
18+
expect(err).to.not.exist;
19+
expect(result).to.exist;
20+
expect(events)
21+
.to.be.an('array')
22+
.with.lengthOf(1);
23+
expect(events[0]).to.containSubset({
24+
commandName: 'insert',
25+
command: {
26+
writeConcern: { j: true }
27+
}
28+
});
29+
done();
30+
});
31+
}
32+
33+
// baseline to confirm client option is working
34+
it(
35+
'should set write concern with j: true client option',
36+
withMonitoredClient('insert', { clientOptions: { j: true } }, journalOptionTest)
37+
);
38+
39+
// ensure query option in connection string passes through
40+
it(
41+
'should set write concern with journal=true connection string option',
42+
withMonitoredClient('insert', { queryOptions: { journal: true } }, journalOptionTest)
43+
);
44+
});
45+
});

0 commit comments

Comments
 (0)