diff --git a/src/sdam/topology_description.ts b/src/sdam/topology_description.ts index 02f92eada47..c17920eb570 100644 --- a/src/sdam/topology_description.ts +++ b/src/sdam/topology_description.ts @@ -95,18 +95,29 @@ export class TopologyDescription { // value among ServerDescriptions of all data-bearing server types. If any have a null // logicalSessionTimeoutMinutes, then TopologyDescription.logicalSessionTimeoutMinutes MUST be // set to null. - const readableServers = Array.from(this.servers.values()).filter( - (s: ServerDescription) => s.isReadable - ); - - this.logicalSessionTimeoutMinutes = readableServers.reduce( - (result: number | undefined, server: ServerDescription) => { - if (server.logicalSessionTimeoutMinutes == null) return; - if (result == null) return server.logicalSessionTimeoutMinutes; - return Math.min(result, server.logicalSessionTimeoutMinutes); - }, - undefined - ); + this.logicalSessionTimeoutMinutes = undefined; + for (const [, server] of this.servers) { + if (server.isReadable) { + if (server.logicalSessionTimeoutMinutes == null) { + // If any of the servers have a null logicalSessionsTimeout, then the whole topology does + this.logicalSessionTimeoutMinutes = undefined; + break; + } + + if (this.logicalSessionTimeoutMinutes === undefined) { + // First server with a non null logicalSessionsTimeout + this.logicalSessionTimeoutMinutes = server.logicalSessionTimeoutMinutes; + continue; + } + + // Always select the smaller of the: + // current server logicalSessionsTimeout and the topologies logicalSessionsTimeout + this.logicalSessionTimeoutMinutes = Math.min( + this.logicalSessionTimeoutMinutes, + server.logicalSessionTimeoutMinutes + ); + } + } } /** diff --git a/test/unit/core/common.js b/test/unit/core/common.js index 49019c7e52e..75f46c26bd8 100644 --- a/test/unit/core/common.js +++ b/test/unit/core/common.js @@ -8,30 +8,40 @@ class ReplSetFixture { this.electionIds = [new ObjectId(), new ObjectId()]; } + uri(dbName) { + return `mongodb://${this.primaryServer.uri()},${this.firstSecondaryServer.uri()},${this.secondSecondaryServer.uri()}/${ + dbName || 'test' + }?replicaSet=rs`; + } + setup(options) { options = options || {}; const ismaster = options.ismaster ? options.ismaster : mock.DEFAULT_ISMASTER_36; - return Promise.all([mock.createServer(), mock.createServer(), mock.createServer()]).then( - servers => { - this.servers = servers; - this.primaryServer = servers[0]; - this.firstSecondaryServer = servers[1]; - this.arbiterServer = servers[2]; - - this.defaultFields = Object.assign({}, ismaster, { - __nodejs_mock_server__: true, - setName: 'rs', - setVersion: 1, - electionId: this.electionIds[0], - hosts: this.servers.map(server => server.uri()), - arbiters: [this.arbiterServer.uri()] - }); - - this.defineReplSetStates(); - this.configureMessageHandlers(); - } - ); + return Promise.all([ + mock.createServer(), + mock.createServer(), + mock.createServer(), + mock.createServer() + ]).then(servers => { + this.servers = servers; + this.primaryServer = servers[0]; + this.firstSecondaryServer = servers[1]; + this.secondSecondaryServer = servers[2]; + this.arbiterServer = servers[3]; + + this.defaultFields = Object.assign({}, ismaster, { + __nodejs_mock_server__: true, + setName: 'rs', + setVersion: 1, + electionId: this.electionIds[0], + hosts: this.servers.map(server => server.uri()), + arbiters: [this.arbiterServer.uri()] + }); + + if (!options.doNotInitStates) this.defineReplSetStates(); + if (!options.doNotInitHandlers) this.configureMessageHandlers(); + }); } defineReplSetStates() { @@ -55,6 +65,16 @@ class ReplSetFixture { }) ]; + this.secondSecondaryStates = [ + Object.assign({}, this.defaultFields, { + ismaster: false, + secondary: true, + me: this.secondSecondaryServer.uri(), + primary: this.primaryServer.uri(), + tags: { loc: 'la' } + }) + ]; + this.arbiterStates = [ Object.assign({}, this.defaultFields, { ismaster: false, diff --git a/test/unit/sessions/client.test.js b/test/unit/sessions/client.test.js index 2cd4c091017..23de6b27523 100644 --- a/test/unit/sessions/client.test.js +++ b/test/unit/sessions/client.test.js @@ -2,6 +2,7 @@ const expect = require('chai').expect; const mock = require('../../tools/mock'); +const { ReplSetFixture } = require('../core/common'); const test = {}; describe('Sessions - client/unit', function () { @@ -37,6 +38,75 @@ describe('Sessions - client/unit', function () { } }); + it('should throw an exception if sessions are not supported on some servers', { + metadata: { requires: { topology: 'single' } }, + test() { + const replicaSetMock = new ReplSetFixture(); + let testClient; + return replicaSetMock + .setup({ doNotInitHandlers: true }) + .then(() => { + replicaSetMock.firstSecondaryServer.setMessageHandler(request => { + var doc = request.document; + if (doc.ismaster) { + const ismaster = replicaSetMock.firstSecondaryStates[0]; + ismaster.logicalSessionTimeoutMinutes = 20; + request.reply(ismaster); + } else if (doc.endSessions) { + request.reply({ ok: 1 }); + } + }); + + replicaSetMock.secondSecondaryServer.setMessageHandler(request => { + var doc = request.document; + if (doc.ismaster) { + const ismaster = replicaSetMock.secondSecondaryStates[0]; + ismaster.logicalSessionTimeoutMinutes = 10; + request.reply(ismaster); + } else if (doc.endSessions) { + request.reply({ ok: 1 }); + } + }); + + replicaSetMock.arbiterServer.setMessageHandler(request => { + var doc = request.document; + if (doc.ismaster) { + const ismaster = replicaSetMock.arbiterStates[0]; + ismaster.logicalSessionTimeoutMinutes = 30; + request.reply(ismaster); + } else if (doc.endSessions) { + request.reply({ ok: 1 }); + } + }); + + replicaSetMock.primaryServer.setMessageHandler(request => { + var doc = request.document; + if (doc.ismaster) { + const ismaster = replicaSetMock.primaryStates[0]; + ismaster.logicalSessionTimeoutMinutes = null; + request.reply(ismaster); + } else if (doc.endSessions) { + request.reply({ ok: 1 }); + } + }); + + return replicaSetMock.uri(); + }) + .then(uri => { + const client = this.configuration.newClient(uri); + return client.connect(); + }) + .then(client => { + testClient = client; + expect(client.topology.s.description.logicalSessionTimeoutMinutes).to.not.exist; + expect(() => { + client.startSession(); + }).to.throw(/Current topology does not support sessions/); + }) + .finally(() => (testClient ? testClient.close() : null)); + } + }); + it('should return a client session when requested if the topology supports it', { metadata: { requires: { topology: 'single' } },