Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(NODE-3521): update session support checks #3151

Merged
merged 12 commits into from
Mar 3, 2022
8 changes: 1 addition & 7 deletions src/cmap/connection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 &&
Expand Down Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion src/cursor/abstract_cursor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 });
}

Expand Down
20 changes: 7 additions & 13 deletions src/operations/execute_operation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
nbbeeken marked this conversation as resolved.
Show resolved Hide resolved
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) {
nbbeeken marked this conversation as resolved.
Show resolved Hide resolved
return cb(new MongoExpiredSessionError('Use of expired sessions is not permitted'));
} else if (session.snapshotEnabled && !topology.capabilities.supportsSnapshotReads) {
nbbeeken marked this conversation as resolved.
Show resolved Hide resolved
return cb(new MongoCompatibilityError('Snapshot reads require MongoDB 5.0 or later'));
}

try {
Expand Down
7 changes: 0 additions & 7 deletions src/sdam/topology.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
91 changes: 1 addition & 90 deletions test/unit/assorted/sessions_client.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

const expect = require('chai').expect;
const mock = require('../../tools/mongodb-mock/index');
const { ReplSetFixture } = require('../../tools/common');
const { isHello } = require('../../../src/utils');
const { MongoClient } = require('../../../src');

Expand All @@ -16,95 +15,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)) {
Expand Down
48 changes: 1 addition & 47 deletions test/unit/cmap/connection.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');

Expand Down Expand Up @@ -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;
});
});
});
});
});