Skip to content

Commit 8d157c8

Browse files
committed
fix(topology): correct logic for checking for sessions support
The logic for checking for sessions support was flawed, in that if it was connect to a single server it would check the conditions for single server session support and, failing that, would back off to the generalized support check. These cases have been split up, and unit tests were added. Additionally, a typeo was fixed when determining if a topology had known servers. NODE-2342
1 parent 6801d21 commit 8d157c8

File tree

3 files changed

+79
-5
lines changed

3 files changed

+79
-5
lines changed

lib/core/sdam/topology.js

+5-4
Original file line numberDiff line numberDiff line change
@@ -448,10 +448,11 @@ class Topology extends EventEmitter {
448448
* @return Whether the topology should initiate selection to determine session support
449449
*/
450450
shouldCheckForSessionSupport() {
451-
return (
452-
(this.description.type === TopologyType.Single && !this.description.hasKnownServers) ||
453-
!this.description.hasDataBearingServers
454-
);
451+
if (this.description.type === TopologyType.Single) {
452+
return !this.description.hasKnownServers;
453+
}
454+
455+
return !this.description.hasDataBearingServers;
455456
}
456457

457458
/**

lib/core/sdam/topology_description.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -254,7 +254,7 @@ class TopologyDescription {
254254
* Determines if the topology description has any known servers
255255
*/
256256
get hasKnownServers() {
257-
return Array.from(this.servers.values()).some(sd => sd.type !== ServerDescription.Unknown);
257+
return Array.from(this.servers.values()).some(sd => sd.type !== ServerType.Unknown);
258258
}
259259

260260
/**

test/unit/sdam/topology_tests.js

+73
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
'use strict';
2+
const Topology = require('../../../lib/core/sdam/topology').Topology;
3+
const Server = require('../../../lib/core/sdam/server').Server;
4+
const ServerDescription = require('../../../lib/core/sdam/server_description').ServerDescription;
5+
const expect = require('chai').expect;
6+
const sinon = require('sinon');
7+
8+
describe('Topology (unit)', function() {
9+
describe('shouldCheckForSessionSupport', function() {
10+
beforeEach(function() {
11+
this.sinon = sinon.sandbox.create();
12+
13+
// these are mocks we want across all tests
14+
this.sinon.stub(Server.prototype, 'monitor');
15+
this.sinon
16+
.stub(Topology.prototype, 'selectServer')
17+
.callsFake(function(selector, options, callback) {
18+
const server = Array.from(this.s.servers.values())[0];
19+
callback(null, server);
20+
});
21+
});
22+
23+
afterEach(function() {
24+
this.sinon.restore();
25+
});
26+
27+
it('should check for sessions if connected to a single server and has no known servers', function(done) {
28+
const topology = new Topology('someserver:27019');
29+
this.sinon.stub(Server.prototype, 'connect').callsFake(function() {
30+
this.emit('connect');
31+
});
32+
33+
topology.connect(() => {
34+
expect(topology.shouldCheckForSessionSupport()).to.be.true;
35+
topology.close(done);
36+
});
37+
});
38+
39+
it('should not check for sessions if connected to a single server', function(done) {
40+
const topology = new Topology('someserver:27019');
41+
this.sinon.stub(Server.prototype, 'connect').callsFake(function() {
42+
this.emit(
43+
'descriptionReceived',
44+
new ServerDescription('someserver:27019', { ok: 1, maxWireVersion: 5 })
45+
);
46+
47+
this.emit('connect');
48+
});
49+
50+
topology.connect(() => {
51+
expect(topology.shouldCheckForSessionSupport()).to.be.false;
52+
topology.close(done);
53+
});
54+
});
55+
56+
it('should check for sessions if there are no data-bearing nodes', function(done) {
57+
const topology = new Topology('mongos:27019,mongos:27018,mongos:27017');
58+
this.sinon.stub(Server.prototype, 'connect').callsFake(function() {
59+
this.emit(
60+
'descriptionReceived',
61+
new ServerDescription(this.name, { ok: 1, msg: 'isdbgrid', maxWireVersion: 5 })
62+
);
63+
64+
this.emit('connect');
65+
});
66+
67+
topology.connect(() => {
68+
expect(topology.shouldCheckForSessionSupport()).to.be.false;
69+
topology.close(done);
70+
});
71+
});
72+
});
73+
});

0 commit comments

Comments
 (0)