fix: session support detection spec compliance [PORT: 4.0] (#2733)

Author: nbbeeken

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
+        );
+      }
+    }
   }
 
   /**

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,

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' } },