From 8d157c8bada52aa125110a9f727a385c70255679 Mon Sep 17 00:00:00 2001
From: Matt Broadstone <mbroadst@mongodb.com>
Date: Thu, 21 Nov 2019 08:44:01 -0500
Subject: [PATCH] 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
---
 lib/core/sdam/topology.js             |  9 ++--
 lib/core/sdam/topology_description.js |  2 +-
 test/unit/sdam/topology_tests.js      | 73 +++++++++++++++++++++++++++
 3 files changed, 79 insertions(+), 5 deletions(-)
 create mode 100644 test/unit/sdam/topology_tests.js

diff --git a/lib/core/sdam/topology.js b/lib/core/sdam/topology.js
index f9dbdbb073..95dd38c8bb 100644
--- a/lib/core/sdam/topology.js
+++ b/lib/core/sdam/topology.js
@@ -448,10 +448,11 @@ class Topology extends EventEmitter {
    * @return Whether the topology should initiate selection to determine session support
    */
   shouldCheckForSessionSupport() {
-    return (
-      (this.description.type === TopologyType.Single && !this.description.hasKnownServers) ||
-      !this.description.hasDataBearingServers
-    );
+    if (this.description.type === TopologyType.Single) {
+      return !this.description.hasKnownServers;
+    }
+
+    return !this.description.hasDataBearingServers;
   }
 
   /**
diff --git a/lib/core/sdam/topology_description.js b/lib/core/sdam/topology_description.js
index 97d14364ad..fe1fac6ed6 100644
--- a/lib/core/sdam/topology_description.js
+++ b/lib/core/sdam/topology_description.js
@@ -254,7 +254,7 @@ class TopologyDescription {
    * Determines if the topology description has any known servers
    */
   get hasKnownServers() {
-    return Array.from(this.servers.values()).some(sd => sd.type !== ServerDescription.Unknown);
+    return Array.from(this.servers.values()).some(sd => sd.type !== ServerType.Unknown);
   }
 
   /**
diff --git a/test/unit/sdam/topology_tests.js b/test/unit/sdam/topology_tests.js
new file mode 100644
index 0000000000..79879483bd
--- /dev/null
+++ b/test/unit/sdam/topology_tests.js
@@ -0,0 +1,73 @@
+'use strict';
+const Topology = require('../../../lib/core/sdam/topology').Topology;
+const Server = require('../../../lib/core/sdam/server').Server;
+const ServerDescription = require('../../../lib/core/sdam/server_description').ServerDescription;
+const expect = require('chai').expect;
+const sinon = require('sinon');
+
+describe('Topology (unit)', function() {
+  describe('shouldCheckForSessionSupport', function() {
+    beforeEach(function() {
+      this.sinon = sinon.sandbox.create();
+
+      // these are mocks we want across all tests
+      this.sinon.stub(Server.prototype, 'monitor');
+      this.sinon
+        .stub(Topology.prototype, 'selectServer')
+        .callsFake(function(selector, options, callback) {
+          const server = Array.from(this.s.servers.values())[0];
+          callback(null, server);
+        });
+    });
+
+    afterEach(function() {
+      this.sinon.restore();
+    });
+
+    it('should check for sessions if connected to a single server and has no known servers', function(done) {
+      const topology = new Topology('someserver:27019');
+      this.sinon.stub(Server.prototype, 'connect').callsFake(function() {
+        this.emit('connect');
+      });
+
+      topology.connect(() => {
+        expect(topology.shouldCheckForSessionSupport()).to.be.true;
+        topology.close(done);
+      });
+    });
+
+    it('should not check for sessions if connected to a single server', function(done) {
+      const topology = new Topology('someserver:27019');
+      this.sinon.stub(Server.prototype, 'connect').callsFake(function() {
+        this.emit(
+          'descriptionReceived',
+          new ServerDescription('someserver:27019', { ok: 1, maxWireVersion: 5 })
+        );
+
+        this.emit('connect');
+      });
+
+      topology.connect(() => {
+        expect(topology.shouldCheckForSessionSupport()).to.be.false;
+        topology.close(done);
+      });
+    });
+
+    it('should check for sessions if there are no data-bearing nodes', function(done) {
+      const topology = new Topology('mongos:27019,mongos:27018,mongos:27017');
+      this.sinon.stub(Server.prototype, 'connect').callsFake(function() {
+        this.emit(
+          'descriptionReceived',
+          new ServerDescription(this.name, { ok: 1, msg: 'isdbgrid', maxWireVersion: 5 })
+        );
+
+        this.emit('connect');
+      });
+
+      topology.connect(() => {
+        expect(topology.shouldCheckForSessionSupport()).to.be.false;
+        topology.close(done);
+      });
+    });
+  });
+});