fix(NODE-3521): remove session support checks

durran · durran · commit a5c8fb39095b · 2022-02-24T19:37:14.000+01:00
diff --git a/src/cmap/connection.ts b/src/cmap/connection.ts
@@ -402,7 +402,7 @@ export class Connection extends TypedEventEmitter<ConnectionEvents> {
       if (deprecationErrors != null) finalCmd.apiDeprecationErrors = deprecationErrors;
     }
 
-    if (hasSessionSupport(this) && session) {
+    if (session) {
       if (
         session.clusterTime &&
         clusterTime &&
@@ -683,12 +683,6 @@ export class CryptoConnection extends Connection {
   }
 }
 
-/** @internal */
-export function hasSessionSupport(conn: Connection): boolean {
-  const description = conn.description;
-  return description.logicalSessionTimeoutMinutes != null || !!description.loadBalanced;
-}
-
 function supportsOpMsg(conn: Connection) {
   const description = conn.description;
   if (description == null) {
diff --git a/src/cursor/abstract_cursor.ts b/src/cursor/abstract_cursor.ts
@@ -653,7 +653,7 @@ function next<T>(cursor: AbstractCursor, blocking: boolean, callback: Callback<T
 
   if (cursorId == null) {
     // All cursors must operate within a session, one must be made implicitly if not explicitly provided
-    if (cursor[kSession] == null && cursor[kTopology].hasSessionSupport()) {
+    if (cursor[kSession] == null) {
       cursor[kSession] = cursor[kTopology].startSession({ owner: cursor, explicit: false });
     }
 
diff --git a/src/operations/execute_operation.ts b/src/operations/execute_operation.ts
@@ -89,19 +89,13 @@ export function executeOperation<
     // that are not explicitly provided with a session.
     let session: ClientSession | undefined = operation.session;
     let owner: symbol | undefined;
-    if (topology.hasSessionSupport()) {
-      if (session == null) {
-        owner = Symbol();
-        session = topology.startSession({ owner, explicit: false });
-      } else if (session.hasEnded) {
-        return cb(new MongoExpiredSessionError('Use of expired sessions is not permitted'));
-      } else if (session.snapshotEnabled && !topology.capabilities.supportsSnapshotReads) {
-        return cb(new MongoCompatibilityError('Snapshot reads require MongoDB 5.0 or later'));
-      }
-    } else if (session) {
-      // If the user passed an explicit session and we are still, after server selection,
-      // trying to run against a topology that doesn't support sessions we error out.
-      return cb(new MongoCompatibilityError('Current topology does not support sessions'));
+    if (session == null) {
+      owner = Symbol();
+      session = topology.startSession({ owner, explicit: false });
+    } else if (session.hasEnded) {
+      return cb(new MongoExpiredSessionError('Use of expired sessions is not permitted'));
+    } else if (session.snapshotEnabled && !topology.capabilities.supportsSnapshotReads) {
+      return cb(new MongoCompatibilityError('Snapshot reads require MongoDB 5.0 or later'));
     }
 
     try {
diff --git a/src/sdam/topology.ts b/src/sdam/topology.ts
@@ -641,13 +641,6 @@ export class Topology extends TypedEventEmitter<TopologyEvents> {
     return !this.description.hasDataBearingServers;
   }
 
-  /**
-   * @returns Whether sessions are supported on the current topology
-   */
-  hasSessionSupport(): boolean {
-    return this.loadBalanced || this.description.logicalSessionTimeoutMinutes != null;
-  }
-
   /** Start a logical session */
   startSession(options: ClientSessionOptions, clientOptions?: MongoOptions): ClientSession {
     const session = new ClientSession(this, this.s.sessionPool, options, clientOptions);
diff --git a/src/utils.ts b/src/utils.ts
@@ -229,7 +229,7 @@ export function executeLegacyOperation(
   let session: ClientSession;
   let opOptions: any;
   let owner: any;
-  if (!options.skipSessions && topology.hasSessionSupport()) {
+  if (!options.skipSessions) {
     opOptions = args[args.length - 2];
     if (opOptions == null || opOptions.session == null) {
       owner = Symbol();
diff --git a/test/unit/assorted/sessions_client.test.js b/test/unit/assorted/sessions_client.test.js
@@ -16,95 +16,7 @@ describe('Sessions - client/unit', function () {
       });
     });
 
-    it('should not throw a synchronous exception if sessions are not supported', function () {
-      test.server.setMessageHandler(request => {
-        var doc = request.document;
-        if (isHello(doc)) {
-          request.reply(Object.assign({}, mock.HELLO));
-        } else if (doc.endSessions) {
-          request.reply({ ok: 1 });
-        }
-      });
-
-      const client = new MongoClient(`mongodb://${test.server.uri()}/test`);
-      return client.connect().then(() => {
-        expect(() => client.startSession()).to.not.throw(
-          'Current topology does not support sessions'
-        );
-        return client.close();
-      });
-    });
-
-    it('should throw an exception if sessions are not supported on some servers', function () {
-      const replicaSetMock = new ReplSetFixture();
-      let testClient;
-      return replicaSetMock
-        .setup({ doNotInitHandlers: true })
-        .then(() => {
-          replicaSetMock.firstSecondaryServer.setMessageHandler(request => {
-            var doc = request.document;
-            if (isHello(doc)) {
-              const hello = replicaSetMock.firstSecondaryStates[0];
-              hello.logicalSessionTimeoutMinutes = 20;
-              request.reply(hello);
-            } else if (doc.endSessions) {
-              request.reply({ ok: 1 });
-            }
-          });
-
-          replicaSetMock.secondSecondaryServer.setMessageHandler(request => {
-            var doc = request.document;
-            if (isHello(doc)) {
-              const hello = replicaSetMock.secondSecondaryStates[0];
-              hello.logicalSessionTimeoutMinutes = 10;
-              request.reply(hello);
-            } else if (doc.endSessions) {
-              request.reply({ ok: 1 });
-            }
-          });
-
-          replicaSetMock.arbiterServer.setMessageHandler(request => {
-            var doc = request.document;
-            if (isHello(doc)) {
-              const hello = replicaSetMock.arbiterStates[0];
-              hello.logicalSessionTimeoutMinutes = 30;
-              request.reply(hello);
-            } else if (doc.endSessions) {
-              request.reply({ ok: 1 });
-            }
-          });
-
-          replicaSetMock.primaryServer.setMessageHandler(request => {
-            var doc = request.document;
-            if (isHello(doc)) {
-              const hello = replicaSetMock.primaryStates[0];
-              hello.logicalSessionTimeoutMinutes = null;
-              request.reply(hello);
-            } else if (doc.endSessions) {
-              request.reply({ ok: 1 });
-            }
-          });
-
-          return replicaSetMock.uri();
-        })
-        .then(uri => {
-          testClient = new MongoClient(uri);
-          return testClient.connect();
-        })
-        .then(client => {
-          const session = client.startSession();
-          return client.db().collection('t').insertOne({ a: 1 }, { session });
-        })
-        .then(() => {
-          expect.fail('Expected an error to be thrown about not supporting sessions');
-        })
-        .catch(error => {
-          expect(error.message).to.equal('Current topology does not support sessions');
-        })
-        .finally(() => (testClient ? testClient.close() : null));
-    });
-
-    it('should return a client session when requested if the topology supports it', function (done) {
+    it('should return a client session when requested', function (done) {
       test.server.setMessageHandler(request => {
         var doc = request.document;
         if (isHello(doc)) {
diff --git a/test/unit/cmap/connection.test.js b/test/unit/cmap/connection.test.js
@@ -2,9 +2,8 @@
 
 const mock = require('../../tools/mongodb-mock/index');
 const { connect } = require('../../../src/cmap/connect');
-const { Connection, hasSessionSupport } = require('../../../src/cmap/connection');
+const { Connection } = require('../../../src/cmap/connection');
 const { expect } = require('chai');
-const { Socket } = require('net');
 const { ns, isHello } = require('../../../src/utils');
 const { getSymbolFrom } = require('../../tools/utils');
 
@@ -107,49 +106,4 @@ describe('Connection - unit/cmap', function () {
       done();
     });
   });
-
-  describe('.hasSessionSupport', function () {
-    let connection;
-    const stream = new Socket();
-
-    context('when logicalSessionTimeoutMinutes is present', function () {
-      beforeEach(function () {
-        connection = new Connection(stream, {
-          hostAddress: server.hostAddress(),
-          logicalSessionTimeoutMinutes: 5
-        });
-      });
-
-      it('returns true', function () {
-        expect(hasSessionSupport(connection)).to.be.true;
-      });
-    });
-
-    context('when logicalSessionTimeoutMinutes is not present', function () {
-      context('when in load balancing mode', function () {
-        beforeEach(function () {
-          connection = new Connection(stream, {
-            hostAddress: server.hostAddress(),
-            loadBalanced: true
-          });
-        });
-
-        it('returns true', function () {
-          expect(hasSessionSupport(connection)).to.be.true;
-        });
-      });
-
-      context('when not in load balancing mode', function () {
-        beforeEach(function () {
-          connection = new Connection(stream, {
-            hostAddress: server.hostAddress()
-          });
-        });
-
-        it('returns false', function () {
-          expect(hasSessionSupport(connection)).to.be.false;
-        });
-      });
-    });
-  });
 });

Original file line number	Diff line number	Diff line change
`@@ -402,7 +402,7 @@ export class Connection extends TypedEventEmitter<ConnectionEvents> {`
`402`	`402`	`if (deprecationErrors != null) finalCmd.apiDeprecationErrors = deprecationErrors;`
`403`	`403`	`}`
`404`	`404`
`405`		`- if (hasSessionSupport(this) && session) {`
	`405`	`+ if (session) {`
`406`	`406`	`if (`
`407`	`407`	`session.clusterTime &&`
`408`	`408`	`clusterTime &&`
`@@ -683,12 +683,6 @@ export class CryptoConnection extends Connection {`
`683`	`683`	`}`
`684`	`684`	`}`
`685`	`685`
`686`		`-/** @internal */`
`687`		`-export function hasSessionSupport(conn: Connection): boolean {`
`688`		`- const description = conn.description;`
`689`		`- return description.logicalSessionTimeoutMinutes != null \|\| !!description.loadBalanced;`
`690`		`-}`
`691`		`-`
`692`	`686`	`function supportsOpMsg(conn: Connection) {`
`693`	`687`	`const description = conn.description;`
`694`	`688`	`if (description == null) {`
Original file line number	Diff line number	Diff line change
`@@ -653,7 +653,7 @@ function next<T>(cursor: AbstractCursor, blocking: boolean, callback: Callback<T`
`653`	`653`
`654`	`654`	`if (cursorId == null) {`
`655`	`655`	`// All cursors must operate within a session, one must be made implicitly if not explicitly provided`
`656`		`- if (cursor[kSession] == null && cursor[kTopology].hasSessionSupport()) {`
	`656`	`+ if (cursor[kSession] == null) {`
`657`	`657`	`cursor[kSession] = cursor[kTopology].startSession({ owner: cursor, explicit: false });`
`658`	`658`	`}`
`659`	`659`