Skip to content

Commit 8bd285d

Browse files
mcasimirmcasimir
authored
fix(connection-model): default to direct connection for single host (#2314)
* fix(connection-model): default to direct connection for single host * fix tests * fix failing test
1 parent 6644e40 commit 8bd285d

File tree

6 files changed

+103
-50
lines changed

6 files changed

+103
-50
lines changed

‎packages/compass-connect/test/renderer/stores/index.spec.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1052,7 +1052,7 @@ describe('Store', () => {
10521052

10531053
it('sets the driverUrl', (done) => {
10541054
const driverUrl =
1055-
'mongodb://server.example.com:27017/?readPreference=primary&ssl=false';
1055+
'mongodb://server.example.com:27017/?readPreference=primary&directConnection=true&ssl=false';
10561056
const unsubscribe = Store.listen((state) => {
10571057
try {
10581058
unsubscribe();

‎packages/connection-model/lib/connect.js‎

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,22 @@ async function waitForTunnelError(tunnel) {
5656
throw error;
5757
}
5858

59+
function addDirectConnectionWhenNeeded(options, model) {
60+
if (
61+
model.directConnection === undefined &&
62+
model.hosts.length === 1 &&
63+
!model.isSrvRecord &&
64+
(model.replicaSet === undefined || model.replicaSet === '')
65+
) {
66+
return {
67+
...options,
68+
directConnection: true
69+
};
70+
}
71+
72+
return options;
73+
}
74+
5975
async function connect(model, setupListeners) {
6076
if (model.serialize === undefined) {
6177
// note this is only here for testing reasons and would not be
@@ -68,7 +84,7 @@ async function connect(model, setupListeners) {
6884

6985
const url = removeGssapiServiceName(model.driverUrlWithSsh);
7086
const options = {
71-
...model.driverOptions
87+
...addDirectConnectionWhenNeeded(model.driverOptions, model)
7288
};
7389

7490
// if `auth` is passed then username and password must be present,
@@ -124,3 +140,4 @@ module.exports = (model, setupListeners, done) => connect(model, setupListeners)
124140
(res) => process.nextTick(() => done(null, ...res)),
125141
(err) => process.nextTick(() => done(err))
126142
);
143+

‎packages/connection-model/lib/model.js‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -169,9 +169,7 @@ const CONNECTION_STRING_OPTIONS = {
169169
],
170170
default: undefined
171171
},
172-
// Driver brought this behaviour back in v3.6.3+ (but will remove in v4), we don't need to handle directConnection ourselves
173-
// See https://github.com/mongodb/node-mongodb-native/pull/2719
174-
// directConnection: { type: 'boolean', default: undefined }
172+
directConnection: { type: 'boolean', default: undefined }
175173
};
176174

177175
assign(props, CONNECTION_STRING_OPTIONS);

‎packages/connection-model/test/connect.test.js‎

Lines changed: 40 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -27,22 +27,31 @@ describe('connection model connector', () => {
2727
model,
2828
setupListeners,
2929
(connectErr, client, _tunnel, { url, options }) => {
30-
if (connectErr) throw connectErr;
31-
32-
assert.strictEqual(
33-
url,
34-
'mongodb://localhost:27018/?readPreference=primary&ssl=false'
35-
);
36-
37-
assert.deepStrictEqual(options, {
38-
// Driver brought this behaviour back in v3.6.3+ (but will remove in v4), we don't need to handle directConnection ourselves
39-
// See https://github.com/mongodb/node-mongodb-native/pull/2719
40-
// directConnection: true,
41-
readPreference: 'primary'
42-
});
43-
44-
client.close(true);
45-
done();
30+
if (connectErr) {
31+
return done(connectErr);
32+
}
33+
34+
try {
35+
assert.strictEqual(
36+
url,
37+
'mongodb://localhost:27018/?readPreference=primary&directConnection=true&ssl=false'
38+
);
39+
40+
assert.deepStrictEqual(options, {
41+
// this is never truly added at the moment since Connection.from
42+
// already adds this to the model and query string when necessary:
43+
// directConnection: true,
44+
readPreference: 'primary'
45+
});
46+
47+
done();
48+
} catch (e) {
49+
done(e);
50+
} finally {
51+
if (client) {
52+
client.close(true);
53+
}
54+
}
4655
}
4756
);
4857
});
@@ -51,20 +60,28 @@ describe('connection model connector', () => {
5160
it('should connect to `localhost:27018 with model`', (done) => {
5261
Connection.from('mongodb://localhost:27018', (parseErr, model) => {
5362
assert.equal(parseErr, null);
54-
connect(model, setupListeners, (connectErr, client) => {
55-
assert.equal(connectErr, null);
56-
client.close(true);
57-
done();
58-
});
63+
connect(model, setupListeners,
64+
(connectErr, client) => {
65+
if (connectErr) {
66+
return done(connectErr);
67+
}
68+
69+
client.close(true);
70+
done();
71+
}
72+
);
5973
});
6074
});
6175

6276
it('should connect to `localhost:27018 with object`', (done) => {
6377
connect(
6478
{ port: 27018, host: 'localhost' },
6579
setupListeners,
66-
(err, client) => {
67-
assert.equal(err, null);
80+
(connectErr, client) => {
81+
if (connectErr) {
82+
return done(connectErr);
83+
}
84+
6885
client.close(true);
6986
done();
7087
}

‎packages/connection-model/test/parse-and-build-uri.js‎

Lines changed: 18 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -31,20 +31,20 @@ const tests = [
3131
description: 'with socketTimeoutMS',
3232
connectionString:
3333
'mongodb://localhost:27017/sampleDb' +
34-
'?socketTimeoutMS=30000&w=majority&readPreference=primary&ssl=false'
34+
'?socketTimeoutMS=30000&w=majority&readPreference=primary&directConnection=true&ssl=false'
3535
// only: true // Uncomment this line to run only one test debugging purpose
3636
},
3737
{
3838
description: 'with compression',
3939
connectionString:
4040
'mongodb://localhost:27017/?compressors=snappy%2Czlib&' +
41-
'readPreference=primary&ssl=false'
41+
'readPreference=primary&directConnection=true&ssl=false'
4242
},
4343
{
4444
description: 'with zlibCompressionLevel',
4545
connectionString:
4646
'mongodb://localhost:27017/?zlibCompressionLevel=9&' +
47-
'readPreference=primary&ssl=false'
47+
'readPreference=primary&directConnection=true&ssl=false'
4848
},
4949
{
5050
description:
@@ -59,7 +59,7 @@ const tests = [
5959
connectionString:
6060
'mongodb://localhost:27017/test?' +
6161
'maxPoolSize=50&minPoolSize=5&maxIdleTimeMS=1000&waitQueueMultiple=200&waitQueueTimeoutMS=100&' +
62-
'w=1&wTimeoutMS=2000&journal=true&readPreference=primary&ssl=false'
62+
'w=1&wTimeoutMS=2000&journal=true&readPreference=primary&directConnection=true&ssl=false'
6363
},
6464
{
6565
description: 'with readConcernLevel',
@@ -84,13 +84,13 @@ const tests = [
8484
description: 'with authSource and authMechanism (SCRAM-SHA-1)',
8585
connectionString:
8686
'mongodb://%40rlo:w%40of@localhost:27017/dogdb?authSource=catdb&' +
87-
'readPreference=primary&authMechanism=SCRAM-SHA-1&ssl=false'
87+
'readPreference=primary&authMechanism=SCRAM-SHA-1&directConnection=true&ssl=false'
8888
},
8989
{
9090
description: 'with authSource and authMechanism (SCRAM-SHA-256)',
9191
connectionString:
9292
'mongodb://%40rlo:w%40of@localhost:27017/dogdb?authSource=catdb&' +
93-
'authMechanism=SCRAM-SHA-256&readPreference=primary&ssl=false'
93+
'authMechanism=SCRAM-SHA-256&readPreference=primary&directConnection=true&ssl=false'
9494
},
9595
{
9696
description: 'with password which is ignored for GSSAPI',
@@ -101,15 +101,15 @@ const tests = [
101101
expectedConnectionString:
102102
'mongodb://%40rlo@localhost:27017/?' +
103103
'authMechanism=GSSAPI&readPreference=primary&' +
104-
'authSource=%24external&ssl=false&authSource=$external'
104+
'authSource=%24external&directConnection=true&ssl=false&authSource=$external'
105105
},
106106
{
107107
description: 'with authMechanismProperties and gssapiServiceName',
108108
connectionString:
109109
'mongodb://%40rlo@localhost:27017/?' +
110110
'authMechanism=GSSAPI&readPreference=primary&' +
111111
'authSource=%24external&authMechanismProperties=CANONICALIZE_HOST_NAME%3Atrue&' +
112-
'gssapiCanonicalizeHostName=true&ssl=false&authSource=$external'
112+
'gssapiCanonicalizeHostName=true&directConnection=true&ssl=false&authSource=$external'
113113
},
114114
{
115115
description:
@@ -123,22 +123,22 @@ const tests = [
123123
description: 'with serverSelectionTryOnce',
124124
connectionString:
125125
'mongodb://a:27017/?readPreference=primary&' +
126-
'serverSelectionTryOnce=false&ssl=false'
126+
'serverSelectionTryOnce=false&directConnection=true&ssl=false'
127127
},
128128
{
129129
description: 'with appName',
130130
connectionString:
131-
'mongodb://localhost:27017/?readPreference=primary&appname=foo&ssl=false'
131+
'mongodb://localhost:27017/?readPreference=primary&appname=foo&directConnection=true&ssl=false'
132132
},
133133
{
134134
description: 'with retryWrites',
135135
connectionString:
136-
'mongodb://hostname:27017/?readPreference=primary&retryWrites=true&ssl=false'
136+
'mongodb://hostname:27017/?readPreference=primary&retryWrites=true&directConnection=true&ssl=false'
137137
},
138138
{
139139
description: 'with uuidRepresentation',
140140
connectionString:
141-
'mongodb://foo:27017/?readPreference=primary&uuidRepresentation=csharpLegacy&ssl=false'
141+
'mongodb://foo:27017/?readPreference=primary&uuidRepresentation=csharpLegacy&directConnection=true&ssl=false'
142142
}
143143
];
144144

@@ -151,8 +151,12 @@ describe('connection model', () => {
151151

152152
const c = new Connection(result.toJSON());
153153

154-
expect(c.driverUrl).to.be.equal(test.expectedConnectionString || test.connectionString);
155-
done();
154+
try {
155+
expect(c.driverUrl).to.be.equal(test.expectedConnectionString || test.connectionString);
156+
done();
157+
} catch (e) {
158+
done(e);
159+
}
156160
});
157161
const runMode = test.only ? it.only : it;
158162
runMode(test.description, runTest);

‎packages/connection-model/test/parse-uri-components.test.js‎

Lines changed: 25 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -549,16 +549,33 @@ describe('connection model partser should parse URI components such as', () => {
549549
);
550550
});
551551

552-
// Driver brought this behaviour back in v3.6.3+ (but will remove in v4), we don't need to handle directConnection ourselves
553-
// See https://github.com/mongodb/node-mongodb-native/pull/2719
554-
describe.skip('directConnection', () => {
555-
it('defaults directConnection undefined', (done) => {
552+
describe('directConnection', () => {
553+
it('defaults directConnection undefined for srv records', (done) => {
556554
Connection.from(
557-
'mongodb://localhost:27017',
555+
'mongodb+srv://user:[email protected]',
558556
(error, result) => {
559-
expect(error).to.not.exist;
560-
expect(result.directConnection).to.be.equal(undefined);
561-
done();
557+
try {
558+
expect(error).to.not.exist;
559+
expect(result.directConnection).to.be.equal(undefined);
560+
done();
561+
} catch (e) {
562+
done(e);
563+
}
564+
}
565+
);
566+
});
567+
568+
it('defaults directConnection undefined for multiple hosts', (done) => {
569+
Connection.from(
570+
'mongodb://host1,host2,host3',
571+
(error, result) => {
572+
try {
573+
expect(error).to.not.exist;
574+
expect(result.directConnection).to.be.equal(undefined);
575+
done();
576+
} catch (e) {
577+
done(e);
578+
}
562579
}
563580
);
564581
});

0 commit comments

Comments
 (0)