diff --git a/lib/core/sdam/topology_description.js b/lib/core/sdam/topology_description.js index 91939c7ade..6e01d65bf0 100644 --- a/lib/core/sdam/topology_description.js +++ b/lib/core/sdam/topology_description.js @@ -72,12 +72,30 @@ 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 => s.isReadable); - this.logicalSessionTimeoutMinutes = readableServers.reduce((result, server) => { - if (server.logicalSessionTimeoutMinutes == null) return null; - if (result == null) return server.logicalSessionTimeoutMinutes; - return Math.min(result, server.logicalSessionTimeoutMinutes); - }, null); + this.logicalSessionTimeoutMinutes = null; + for (const addressServerTuple of this.servers) { + const server = addressServerTuple[1]; + if (server.isReadable) { + if (server.logicalSessionTimeoutMinutes == null) { + // If any of the servers have a null logicalSessionsTimeout, then the whole topology does + this.logicalSessionTimeoutMinutes = null; + break; + } + + if (this.logicalSessionTimeoutMinutes == null) { + // 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 4fa8def73c..0eb40836d4 100644 --- a/test/unit/core/common.js +++ b/test/unit/core/common.js @@ -11,30 +11,39 @@ 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() { @@ -58,6 +67,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 48f3712570..7d9005a816 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('mongodb-mock-server'); +const ReplSetFixture = require('../core/common').ReplSetFixture; const test = {}; describe('Sessions', function() { @@ -37,6 +38,72 @@ describe('Sessions', function() { } }); + it('should throw an exception if sessions are not supported on some servers', { + metadata: { requires: { topology: 'single' } }, + test() { + const replicaSetMock = new ReplSetFixture(); + 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 => { + expect(client.topology.s.description.logicalSessionTimeoutMinutes).to.not.exist; + expect(() => { + client.startSession(); + }).to.throw(/Current topology does not support sessions/); + return client.close(); + }); + } + }); it('should return a client session when requested if the topology supports it', { metadata: { requires: { topology: 'single' } },