From 8b85d13fe756f5bc9b9a81ab487c7f84315cad9f Mon Sep 17 00:00:00 2001 From: Alena Khineika Date: Thu, 30 Nov 2023 11:10:19 +0100 Subject: [PATCH 01/15] feat(NODE-3689): require hello command for connection handshake to use OP_MSG disallowing OP_QUERY --- src/sdam/topology.ts | 4 + src/utils.ts | 2 +- .../mongodb-handshake.test.ts | 23 +++- test/unit/cmap/connection.test.ts | 105 ++++++++++++++++++ 4 files changed, 132 insertions(+), 2 deletions(-) diff --git a/src/sdam/topology.ts b/src/sdam/topology.ts index c65a5f22b74..3a73dac70d4 100644 --- a/src/sdam/topology.ts +++ b/src/sdam/topology.ts @@ -382,6 +382,10 @@ export class Topology extends TypedEventEmitter { return this.s.options.loadBalanced; } + get serverApi(): ServerApi | undefined { + return this.s.options.serverApi; + } + get capabilities(): ServerCapabilities { return new ServerCapabilities(this.lastHello()); } diff --git a/src/utils.ts b/src/utils.ts index 8bf4425c18b..513ed94747b 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -371,7 +371,7 @@ export function uuidV4(): Buffer { */ export function maxWireVersion(topologyOrServer?: Connection | Topology | Server): number { if (topologyOrServer) { - if (topologyOrServer.loadBalanced) { + if (topologyOrServer.loadBalanced || topologyOrServer.serverApi) { // Since we do not have a monitor, we assume the load balanced server is always // pointed at the latest mongodb version. There is a risk that for on-prem // deployments that don't upgrade immediately that this could alert to the diff --git a/test/integration/mongodb-handshake/mongodb-handshake.test.ts b/test/integration/mongodb-handshake/mongodb-handshake.test.ts index 2e00d0e2e96..0e6620f217e 100644 --- a/test/integration/mongodb-handshake/mongodb-handshake.test.ts +++ b/test/integration/mongodb-handshake/mongodb-handshake.test.ts @@ -5,8 +5,10 @@ import * as sinon from 'sinon'; import { Connection, LEGACY_HELLO_COMMAND, + MessageStream, MongoServerError, - MongoServerSelectionError + MongoServerSelectionError, + OpMsgRequest } from '../../mongodb'; describe('MongoDB Handshake', () => { @@ -62,4 +64,23 @@ describe('MongoDB Handshake', () => { expect(handshakeDoc).to.have.property('compression').to.deep.equal(['snappy']); }); }); + + context('when load-balanced', function () { + let writeCommandSpy: Sinon.SinonSpy; + beforeEach(() => { + writeCommandSpy = sinon.spy(MessageStream.prototype, 'writeCommand'); + }); + + afterEach(() => sinon.restore()); + + it('should send the hello command as OP_MSG', { + metadata: { requires: { topology: 'load-balanced' } }, + test: async function () { + client = this.configuration.newClient({ loadBalanced: true }); + await client.connect(); + expect(writeCommandSpy).to.have.been.called; + expect(writeCommandSpy.firstCall.args[0] instanceof OpMsgRequest).to.equal(true); + } + }); + }); }); diff --git a/test/unit/cmap/connection.test.ts b/test/unit/cmap/connection.test.ts index 9c0de2e542a..114d6de68c3 100644 --- a/test/unit/cmap/connection.test.ts +++ b/test/unit/cmap/connection.test.ts @@ -1135,4 +1135,109 @@ describe('new Connection()', function () { expect(writeCommandSpy.firstCall.args[0] instanceof OpQueryRequest).to.equal(true); }); }); + + describe('when serverApi.version is present', () => { + const CONNECT_DEFAULTS = { + id: 1, + tls: false, + generation: 1, + monitorCommands: false, + metadata: {} as ClientMetadata + }; + let server; + let connectOptions; + let connection: Connection; + let writeCommandSpy; + + beforeEach(async () => { + server = await mock.createServer(); + server.setMessageHandler(request => { + request.reply(mock.HELLO); + }); + writeCommandSpy = sinon.spy(MessageStream.prototype, 'writeCommand'); + }); + + afterEach(async () => { + connection?.destroy({ force: true }); + sinon.restore(); + await mock.cleanup(); + }); + + it('sends the first command as OP_MSG', async () => { + try { + connectOptions = { + ...CONNECT_DEFAULTS, + hostAddress: server.hostAddress() as HostAddress, + socketTimeoutMS: 100, + serverApi: { version: '1' } + }; + + connection = await promisify(callback => + //@ts-expect-error: Callbacks do not have mutual exclusion for error/result existence + connect(connectOptions, callback) + )(); + + await promisify(callback => + connection.command(ns('admin.$cmd'), { hello: 1 }, {}, callback) + )(); + } catch (error) { + /** Connection timeouts, but the handshake message is sent. */ + } + + expect(writeCommandSpy).to.have.been.called; + expect(writeCommandSpy.firstCall.args[0] instanceof OpMsgRequest).to.equal(true); + }); + }); + + describe('when serverApi.version is not present', () => { + const CONNECT_DEFAULTS = { + id: 1, + tls: false, + generation: 1, + monitorCommands: false, + metadata: {} as ClientMetadata + }; + let server; + let connectOptions; + let connection: Connection; + let writeCommandSpy; + + beforeEach(async () => { + server = await mock.createServer(); + server.setMessageHandler(request => { + request.reply(mock.HELLO); + }); + writeCommandSpy = sinon.spy(MessageStream.prototype, 'writeCommand'); + }); + + afterEach(async () => { + connection?.destroy({ force: true }); + sinon.restore(); + await mock.cleanup(); + }); + + it('sends the first command as OP_MSG', async () => { + try { + connectOptions = { + ...CONNECT_DEFAULTS, + hostAddress: server.hostAddress() as HostAddress, + socketTimeoutMS: 100 + }; + + connection = await promisify(callback => + //@ts-expect-error: Callbacks do not have mutual exclusion for error/result existence + connect(connectOptions, callback) + )(); + + await promisify(callback => + connection.command(ns('admin.$cmd'), { hello: 1 }, {}, callback) + )(); + } catch (error) { + /** Connection timeouts, but the handshake message is sent. */ + } + + expect(writeCommandSpy).to.have.been.called; + expect(writeCommandSpy.firstCall.args[0] instanceof OpQueryRequest).to.equal(true); + }); + }); }); From 140ec6d56dba73ac85235e3671dc880b98a3e8a2 Mon Sep 17 00:00:00 2001 From: Alena Khineika Date: Fri, 1 Dec 2023 14:45:32 +0100 Subject: [PATCH 02/15] test: ping server in noauth mode and use proper env in the testing readme --- .../mongodb-handshake/mongodb-handshake.test.ts | 8 ++++++++ test/readme.md | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/test/integration/mongodb-handshake/mongodb-handshake.test.ts b/test/integration/mongodb-handshake/mongodb-handshake.test.ts index 0e6620f217e..1bde8fc8932 100644 --- a/test/integration/mongodb-handshake/mongodb-handshake.test.ts +++ b/test/integration/mongodb-handshake/mongodb-handshake.test.ts @@ -59,6 +59,10 @@ describe('MongoDB Handshake', () => { it('constructs a handshake with the specified compressors', async function () { client = this.configuration.newClient({ compressors: ['snappy'] }); await client.connect(); + // The load-balanced mode doesn’t perform SDAM, + // so `connect` doesn’t do anything unless authentication is enabled. + // Force the driver to send a command to the server in the noauth mode. + await client.db('admin').command({ ping: 1 }); expect(spy.called).to.be.true; const handshakeDoc = spy.getCall(0).args[1]; expect(handshakeDoc).to.have.property('compression').to.deep.equal(['snappy']); @@ -78,6 +82,10 @@ describe('MongoDB Handshake', () => { test: async function () { client = this.configuration.newClient({ loadBalanced: true }); await client.connect(); + // The load-balanced mode doesn’t perform SDAM, + // so `connect` doesn’t do anything unless authentication is enabled. + // Force the driver to send a command to the server in the noauth mode. + await client.db('admin').command({ ping: 1 }); expect(writeCommandSpy).to.have.been.called; expect(writeCommandSpy.firstCall.args[0] instanceof OpMsgRequest).to.equal(true); } diff --git a/test/readme.md b/test/readme.md index 448be5aca18..117cf70972b 100644 --- a/test/readme.md +++ b/test/readme.md @@ -330,7 +330,7 @@ The following steps will walk you through how to start and test a load balancer. A new file name `lb.env` is automatically created. 1. Source the environment variables using a command like `source lb.env`. 1. Export **each** of the environment variables that were created in `lb.env`. For example: `export SINGLE_MONGOS_LB_URI`. -1. Export the `LOAD_BALANCED` environment variable to 'true': `export LOAD_BALANCED='true'` +1. Export the `LOAD_BALANCER` environment variable to 'true': `export LOAD_BALANCER='true'` 1. Disable auth for tests: `export AUTH='noauth'` 1. Run the test suite as you normally would: ```sh From 35785962b3e1bc6c350b9d26a523928b6703575d Mon Sep 17 00:00:00 2001 From: Alena Khineika Date: Fri, 1 Dec 2023 15:15:20 +0100 Subject: [PATCH 03/15] test: add integration tests for serverApi and the regular topology --- .../mongodb-handshake.test.ts | 54 ++++++++++++++++++- test/unit/cmap/connection.test.ts | 4 +- 2 files changed, 55 insertions(+), 3 deletions(-) diff --git a/test/integration/mongodb-handshake/mongodb-handshake.test.ts b/test/integration/mongodb-handshake/mongodb-handshake.test.ts index 1bde8fc8932..500639d7aaa 100644 --- a/test/integration/mongodb-handshake/mongodb-handshake.test.ts +++ b/test/integration/mongodb-handshake/mongodb-handshake.test.ts @@ -8,7 +8,9 @@ import { MessageStream, MongoServerError, MongoServerSelectionError, - OpMsgRequest + OpMsgRequest, + OpQueryRequest, + ServerApiVersion } from '../../mongodb'; describe('MongoDB Handshake', () => { @@ -91,4 +93,54 @@ describe('MongoDB Handshake', () => { } }); }); + + context('when serverApi version is present', function () { + let writeCommandSpy: Sinon.SinonSpy; + beforeEach(() => { + writeCommandSpy = sinon.spy(MessageStream.prototype, 'writeCommand'); + }); + + afterEach(() => sinon.restore()); + + it('should send the hello command as OP_MSG', { + metadata: { requires: { topology: '!load-balanced', mongodb: '>=5.0' } }, + test: async function () { + client = this.configuration.newClient({}, { serverApi: { version: ServerApiVersion.v1 } }); + await client.connect(); + // The load-balanced mode doesn’t perform SDAM, + // so `connect` doesn’t do anything unless authentication is enabled. + // Force the driver to send a command to the server in the noauth mode. + await client.db('admin').command({ ping: 1 }); + expect(writeCommandSpy).to.have.been.called; + expect(writeCommandSpy.firstCall.args[0] instanceof OpMsgRequest).to.equal(true); + } + }); + }); + + context('when not load-balanced and serverApi version is not present', function () { + let writeCommandSpy: Sinon.SinonSpy; + beforeEach(() => { + writeCommandSpy = sinon.spy(MessageStream.prototype, 'writeCommand'); + }); + + afterEach(() => sinon.restore()); + + it('should send the hello command as OP_MSG', { + metadata: { requires: { topology: '!load-balanced', mongodb: '>=5.0' } }, + test: async function () { + client = this.configuration.newClient(); + await client.connect(); + // The load-balanced mode doesn’t perform SDAM, + // so `connect` doesn’t do anything unless authentication is enabled. + // Force the driver to send a command to the server in the noauth mode. + await client.db('admin').command({ ping: 1 }); + expect(writeCommandSpy).to.have.been.called; + + const opRequests = writeCommandSpy.getCalls().map(items => items.args[0]); + expect(opRequests[0] instanceof OpQueryRequest).to.equal(true); + const isOpMsgRequestSent = !!opRequests.find(op => op instanceof OpMsgRequest); + expect(isOpMsgRequestSent).to.equal(true); + } + }); + }); }); diff --git a/test/unit/cmap/connection.test.ts b/test/unit/cmap/connection.test.ts index 114d6de68c3..efc26ed50a3 100644 --- a/test/unit/cmap/connection.test.ts +++ b/test/unit/cmap/connection.test.ts @@ -1136,7 +1136,7 @@ describe('new Connection()', function () { }); }); - describe('when serverApi.version is present', () => { + describe('when serverApi version is present', () => { const CONNECT_DEFAULTS = { id: 1, tls: false, @@ -1189,7 +1189,7 @@ describe('new Connection()', function () { }); }); - describe('when serverApi.version is not present', () => { + describe('when serverApi version is not present', () => { const CONNECT_DEFAULTS = { id: 1, tls: false, From 21221a2e275489a0d46673d16a95023c19f96d50 Mon Sep 17 00:00:00 2001 From: Alena Khineika Date: Fri, 1 Dec 2023 15:30:50 +0100 Subject: [PATCH 04/15] docs: update comment in maxWireVersion --- src/utils.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/utils.ts b/src/utils.ts index 513ed94747b..ddc3dc4a725 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -372,8 +372,9 @@ export function uuidV4(): Buffer { export function maxWireVersion(topologyOrServer?: Connection | Topology | Server): number { if (topologyOrServer) { if (topologyOrServer.loadBalanced || topologyOrServer.serverApi) { - // Since we do not have a monitor, we assume the load balanced server is always - // pointed at the latest mongodb version. There is a risk that for on-prem + // Since we do not have a monitor, we assume + // when a server API version or load-balanced topology are requested + // the server is always pointed at the latest mongodb version. There is a risk that for on-prem // deployments that don't upgrade immediately that this could alert to the // application that a feature is available that is actually not. return MAX_SUPPORTED_WIRE_VERSION; From f09e423a83fa942052e8e5966caac5949fac3efe Mon Sep 17 00:00:00 2001 From: Alena Khineika Date: Fri, 1 Dec 2023 15:32:41 +0100 Subject: [PATCH 05/15] refactor: check for version --- src/utils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utils.ts b/src/utils.ts index ddc3dc4a725..c6a7a972f75 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -371,7 +371,7 @@ export function uuidV4(): Buffer { */ export function maxWireVersion(topologyOrServer?: Connection | Topology | Server): number { if (topologyOrServer) { - if (topologyOrServer.loadBalanced || topologyOrServer.serverApi) { + if (topologyOrServer.loadBalanced || topologyOrServer.serverApi?.version) { // Since we do not have a monitor, we assume // when a server API version or load-balanced topology are requested // the server is always pointed at the latest mongodb version. There is a risk that for on-prem From 40b30366eb05ee7e9cf2b35f65e0bf541123544c Mon Sep 17 00:00:00 2001 From: Alena Khineika Date: Fri, 1 Dec 2023 18:10:07 +0100 Subject: [PATCH 06/15] test: skip test on latest-server-v1-api --- .../mongodb-handshake/mongodb-handshake.test.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/test/integration/mongodb-handshake/mongodb-handshake.test.ts b/test/integration/mongodb-handshake/mongodb-handshake.test.ts index 500639d7aaa..18d35d83090 100644 --- a/test/integration/mongodb-handshake/mongodb-handshake.test.ts +++ b/test/integration/mongodb-handshake/mongodb-handshake.test.ts @@ -52,6 +52,7 @@ describe('MongoDB Handshake', () => { context('when compressors are provided on the mongo client', () => { let spy: Sinon.SinonSpy; + before(() => { spy = sinon.spy(Connection.prototype, 'command'); }); @@ -73,6 +74,7 @@ describe('MongoDB Handshake', () => { context('when load-balanced', function () { let writeCommandSpy: Sinon.SinonSpy; + beforeEach(() => { writeCommandSpy = sinon.spy(MessageStream.prototype, 'writeCommand'); }); @@ -96,6 +98,7 @@ describe('MongoDB Handshake', () => { context('when serverApi version is present', function () { let writeCommandSpy: Sinon.SinonSpy; + beforeEach(() => { writeCommandSpy = sinon.spy(MessageStream.prototype, 'writeCommand'); }); @@ -119,6 +122,7 @@ describe('MongoDB Handshake', () => { context('when not load-balanced and serverApi version is not present', function () { let writeCommandSpy: Sinon.SinonSpy; + beforeEach(() => { writeCommandSpy = sinon.spy(MessageStream.prototype, 'writeCommand'); }); @@ -128,6 +132,10 @@ describe('MongoDB Handshake', () => { it('should send the hello command as OP_MSG', { metadata: { requires: { topology: '!load-balanced', mongodb: '>=5.0' } }, test: async function () { + if (this.configuration.serverApi) { + this.currentTest.skipReason = 'Test requires serverApi to NOT be enabled'; + return this.skip(); + } client = this.configuration.newClient(); await client.connect(); // The load-balanced mode doesn’t perform SDAM, From e11601b09817d3ae89cee3637a4c251dc5b2847b Mon Sep 17 00:00:00 2001 From: Alena Khineika Date: Fri, 1 Dec 2023 18:41:34 +0100 Subject: [PATCH 07/15] refactor: try to move serverApi to supportsOpMsg --- src/cmap/connection.ts | 4 +++- src/utils.ts | 7 +++---- .../mongodb-handshake/mongodb-handshake.test.ts | 2 +- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/cmap/connection.ts b/src/cmap/connection.ts index d505496519e..895a595d902 100644 --- a/src/cmap/connection.ts +++ b/src/cmap/connection.ts @@ -655,7 +655,9 @@ function supportsOpMsg(conn: Connection) { return false; } - return maxWireVersion(conn) >= 6 && !description.__nodejs_mock_server__; + return ( + (conn.serverApi?.version || maxWireVersion(conn) >= 6) && !description.__nodejs_mock_server__ + ); } function streamIdentifier(stream: Stream, options: ConnectionOptions): string { diff --git a/src/utils.ts b/src/utils.ts index c6a7a972f75..8bf4425c18b 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -371,10 +371,9 @@ export function uuidV4(): Buffer { */ export function maxWireVersion(topologyOrServer?: Connection | Topology | Server): number { if (topologyOrServer) { - if (topologyOrServer.loadBalanced || topologyOrServer.serverApi?.version) { - // Since we do not have a monitor, we assume - // when a server API version or load-balanced topology are requested - // the server is always pointed at the latest mongodb version. There is a risk that for on-prem + if (topologyOrServer.loadBalanced) { + // Since we do not have a monitor, we assume the load balanced server is always + // pointed at the latest mongodb version. There is a risk that for on-prem // deployments that don't upgrade immediately that this could alert to the // application that a feature is available that is actually not. return MAX_SUPPORTED_WIRE_VERSION; diff --git a/test/integration/mongodb-handshake/mongodb-handshake.test.ts b/test/integration/mongodb-handshake/mongodb-handshake.test.ts index 18d35d83090..9362a5168f9 100644 --- a/test/integration/mongodb-handshake/mongodb-handshake.test.ts +++ b/test/integration/mongodb-handshake/mongodb-handshake.test.ts @@ -133,7 +133,7 @@ describe('MongoDB Handshake', () => { metadata: { requires: { topology: '!load-balanced', mongodb: '>=5.0' } }, test: async function () { if (this.configuration.serverApi) { - this.currentTest.skipReason = 'Test requires serverApi to NOT be enabled'; + this.skipReason = 'Test requires serverApi to NOT be enabled'; return this.skip(); } client = this.configuration.newClient(); From 289a072fb6e6124580773a6de15e622bbfd14f51 Mon Sep 17 00:00:00 2001 From: Alena Khineika Date: Fri, 1 Dec 2023 19:05:38 +0100 Subject: [PATCH 08/15] refactor: revert --- src/cmap/connection.ts | 4 +--- src/utils.ts | 7 ++++--- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/src/cmap/connection.ts b/src/cmap/connection.ts index 895a595d902..d505496519e 100644 --- a/src/cmap/connection.ts +++ b/src/cmap/connection.ts @@ -655,9 +655,7 @@ function supportsOpMsg(conn: Connection) { return false; } - return ( - (conn.serverApi?.version || maxWireVersion(conn) >= 6) && !description.__nodejs_mock_server__ - ); + return maxWireVersion(conn) >= 6 && !description.__nodejs_mock_server__; } function streamIdentifier(stream: Stream, options: ConnectionOptions): string { diff --git a/src/utils.ts b/src/utils.ts index 8bf4425c18b..c6a7a972f75 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -371,9 +371,10 @@ export function uuidV4(): Buffer { */ export function maxWireVersion(topologyOrServer?: Connection | Topology | Server): number { if (topologyOrServer) { - if (topologyOrServer.loadBalanced) { - // Since we do not have a monitor, we assume the load balanced server is always - // pointed at the latest mongodb version. There is a risk that for on-prem + if (topologyOrServer.loadBalanced || topologyOrServer.serverApi?.version) { + // Since we do not have a monitor, we assume + // when a server API version or load-balanced topology are requested + // the server is always pointed at the latest mongodb version. There is a risk that for on-prem // deployments that don't upgrade immediately that this could alert to the // application that a feature is available that is actually not. return MAX_SUPPORTED_WIRE_VERSION; From fce2eca5896f3583a72f5060660fff64fd9dce83 Mon Sep 17 00:00:00 2001 From: Alena Khineika Date: Fri, 1 Dec 2023 19:57:49 +0100 Subject: [PATCH 09/15] test: change clients to mocked servers configured with an API version to not set a serverApi --- test/integration/change-streams/change_stream.test.ts | 4 +++- .../integration/change-streams/change_streams.prose.test.ts | 5 ++++- test/integration/max-staleness/max_staleness.test.js | 3 ++- .../integration/mongodb-handshake/mongodb-handshake.test.ts | 6 +----- 4 files changed, 10 insertions(+), 8 deletions(-) diff --git a/test/integration/change-streams/change_stream.test.ts b/test/integration/change-streams/change_stream.test.ts index 5a0a632187c..21a8e1c54fd 100644 --- a/test/integration/change-streams/change_stream.test.ts +++ b/test/integration/change-streams/change_stream.test.ts @@ -1262,7 +1262,9 @@ describe('Change Streams', function () { } req.reply({ ok: 1 }); }); - const client = this.configuration.newClient(`mongodb://${mockServer.uri()}/`); + const client = this.configuration.newClient(`mongodb://${mockServer.uri()}/`, { + serverApi: null + }); client.connect(err => { expect(err).to.not.exist; const collection = client.db('cs').collection('test'); diff --git a/test/integration/change-streams/change_streams.prose.test.ts b/test/integration/change-streams/change_streams.prose.test.ts index 85c5c4794ec..86a8504ee92 100644 --- a/test/integration/change-streams/change_streams.prose.test.ts +++ b/test/integration/change-streams/change_streams.prose.test.ts @@ -332,7 +332,10 @@ describe('Change Stream prose tests', function () { } request.reply(this.applyOpTime(response)); }); - this.client = this.config.newClient(this.mongodbURI, { monitorCommands: true }); + this.client = this.config.newClient(this.mongodbURI, { + monitorCommands: true, + serverApi: null + }); this.apm = { started: [], succeeded: [], failed: [] }; ( diff --git a/test/integration/max-staleness/max_staleness.test.js b/test/integration/max-staleness/max_staleness.test.js index 049e2714d73..eda69ecbecb 100644 --- a/test/integration/max-staleness/max_staleness.test.js +++ b/test/integration/max-staleness/max_staleness.test.js @@ -54,7 +54,8 @@ describe('Max Staleness', function () { var self = this; const configuration = this.configuration; const client = configuration.newClient( - `mongodb://${test.server.uri()}/test?readPreference=secondary&maxStalenessSeconds=250` + `mongodb://${test.server.uri()}/test?readPreference=secondary&maxStalenessSeconds=250`, + { serverApi: null } ); client.connect(function (err, client) { diff --git a/test/integration/mongodb-handshake/mongodb-handshake.test.ts b/test/integration/mongodb-handshake/mongodb-handshake.test.ts index 9362a5168f9..0d241003257 100644 --- a/test/integration/mongodb-handshake/mongodb-handshake.test.ts +++ b/test/integration/mongodb-handshake/mongodb-handshake.test.ts @@ -132,11 +132,7 @@ describe('MongoDB Handshake', () => { it('should send the hello command as OP_MSG', { metadata: { requires: { topology: '!load-balanced', mongodb: '>=5.0' } }, test: async function () { - if (this.configuration.serverApi) { - this.skipReason = 'Test requires serverApi to NOT be enabled'; - return this.skip(); - } - client = this.configuration.newClient(); + client = this.configuration.newClient({}, { serverApi: null }); await client.connect(); // The load-balanced mode doesn’t perform SDAM, // so `connect` doesn’t do anything unless authentication is enabled. From ed257ec6219f45bc3efac969d9f5f4972911a831 Mon Sep 17 00:00:00 2001 From: Alena Khineika Date: Fri, 1 Dec 2023 21:02:49 +0100 Subject: [PATCH 10/15] test: disable serverApi for rest on cases --- .../collection-management/collection.test.ts | 4 +++- test/integration/max-staleness/max_staleness.test.js | 12 +++++++++--- .../mongodb-handshake/mongodb-handshake.test.ts | 6 +++++- 3 files changed, 17 insertions(+), 5 deletions(-) diff --git a/test/integration/collection-management/collection.test.ts b/test/integration/collection-management/collection.test.ts index 3dcbdeeacc5..622a09bc1f2 100644 --- a/test/integration/collection-management/collection.test.ts +++ b/test/integration/collection-management/collection.test.ts @@ -474,7 +474,9 @@ describe('Collection', function () { afterEach(() => mock.cleanup()); function testCountDocMock(testConfiguration, config, done) { - const client = testConfiguration.newClient(`mongodb://${server.uri()}/test`); + const client = testConfiguration.newClient(`mongodb://${server.uri()}/test`, { + serverApi: null + }); const close = e => client.close(() => done(e)); server.setMessageHandler(request => { diff --git a/test/integration/max-staleness/max_staleness.test.js b/test/integration/max-staleness/max_staleness.test.js index eda69ecbecb..54da9787dff 100644 --- a/test/integration/max-staleness/max_staleness.test.js +++ b/test/integration/max-staleness/max_staleness.test.js @@ -87,7 +87,9 @@ describe('Max Staleness', function () { test: function (done) { const configuration = this.configuration; - const client = configuration.newClient(`mongodb://${test.server.uri()}/test`); + const client = configuration.newClient(`mongodb://${test.server.uri()}/test`, { + serverApi: null + }); client.connect(function (err, client) { expect(err).to.not.exist; @@ -125,7 +127,9 @@ describe('Max Staleness', function () { test: function (done) { var self = this; const configuration = this.configuration; - const client = configuration.newClient(`mongodb://${test.server.uri()}/test`); + const client = configuration.newClient(`mongodb://${test.server.uri()}/test`, { + serverApi: null + }); client.connect(function (err, client) { expect(err).to.not.exist; var db = client.db(self.configuration.db); @@ -160,7 +164,9 @@ describe('Max Staleness', function () { test: function (done) { var self = this; const configuration = this.configuration; - const client = configuration.newClient(`mongodb://${test.server.uri()}/test`); + const client = configuration.newClient(`mongodb://${test.server.uri()}/test`, { + serverApi: null + }); client.connect(function (err, client) { expect(err).to.not.exist; var db = client.db(self.configuration.db); diff --git a/test/integration/mongodb-handshake/mongodb-handshake.test.ts b/test/integration/mongodb-handshake/mongodb-handshake.test.ts index 0d241003257..9362a5168f9 100644 --- a/test/integration/mongodb-handshake/mongodb-handshake.test.ts +++ b/test/integration/mongodb-handshake/mongodb-handshake.test.ts @@ -132,7 +132,11 @@ describe('MongoDB Handshake', () => { it('should send the hello command as OP_MSG', { metadata: { requires: { topology: '!load-balanced', mongodb: '>=5.0' } }, test: async function () { - client = this.configuration.newClient({}, { serverApi: null }); + if (this.configuration.serverApi) { + this.skipReason = 'Test requires serverApi to NOT be enabled'; + return this.skip(); + } + client = this.configuration.newClient(); await client.connect(); // The load-balanced mode doesn’t perform SDAM, // so `connect` doesn’t do anything unless authentication is enabled. From a48dbb3973fbb83ea65ba604013476dbf9f73437 Mon Sep 17 00:00:00 2001 From: Alena Khineika Date: Fri, 1 Dec 2023 21:34:47 +0100 Subject: [PATCH 11/15] docs: add todo to fixed --- test/integration/change-streams/change_stream.test.ts | 2 +- .../change-streams/change_streams.prose.test.ts | 2 +- test/integration/collection-management/collection.test.ts | 2 +- test/integration/max-staleness/max_staleness.test.js | 8 ++++---- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/test/integration/change-streams/change_stream.test.ts b/test/integration/change-streams/change_stream.test.ts index 21a8e1c54fd..473b7879025 100644 --- a/test/integration/change-streams/change_stream.test.ts +++ b/test/integration/change-streams/change_stream.test.ts @@ -1263,7 +1263,7 @@ describe('Change Streams', function () { req.reply({ ok: 1 }); }); const client = this.configuration.newClient(`mongodb://${mockServer.uri()}/`, { - serverApi: null + serverApi: null // TODO(NODE-3807): remove resetting serverApi when the usage of mongodb mock server is removed }); client.connect(err => { expect(err).to.not.exist; diff --git a/test/integration/change-streams/change_streams.prose.test.ts b/test/integration/change-streams/change_streams.prose.test.ts index 86a8504ee92..63c22582b7f 100644 --- a/test/integration/change-streams/change_streams.prose.test.ts +++ b/test/integration/change-streams/change_streams.prose.test.ts @@ -334,7 +334,7 @@ describe('Change Stream prose tests', function () { }); this.client = this.config.newClient(this.mongodbURI, { monitorCommands: true, - serverApi: null + serverApi: null // TODO(NODE-3807): remove resetting serverApi when the usage of mongodb mock server is removed }); this.apm = { started: [], succeeded: [], failed: [] }; diff --git a/test/integration/collection-management/collection.test.ts b/test/integration/collection-management/collection.test.ts index 622a09bc1f2..5d645c46aa0 100644 --- a/test/integration/collection-management/collection.test.ts +++ b/test/integration/collection-management/collection.test.ts @@ -475,7 +475,7 @@ describe('Collection', function () { function testCountDocMock(testConfiguration, config, done) { const client = testConfiguration.newClient(`mongodb://${server.uri()}/test`, { - serverApi: null + serverApi: null // TODO(NODE-3807): remove resetting serverApi when the usage of mongodb mock server is removed }); const close = e => client.close(() => done(e)); diff --git a/test/integration/max-staleness/max_staleness.test.js b/test/integration/max-staleness/max_staleness.test.js index 54da9787dff..d4dbf413683 100644 --- a/test/integration/max-staleness/max_staleness.test.js +++ b/test/integration/max-staleness/max_staleness.test.js @@ -55,7 +55,7 @@ describe('Max Staleness', function () { const configuration = this.configuration; const client = configuration.newClient( `mongodb://${test.server.uri()}/test?readPreference=secondary&maxStalenessSeconds=250`, - { serverApi: null } + { serverApi: null } // TODO(NODE-3807): remove resetting serverApi when the usage of mongodb mock server is removed ); client.connect(function (err, client) { @@ -88,7 +88,7 @@ describe('Max Staleness', function () { test: function (done) { const configuration = this.configuration; const client = configuration.newClient(`mongodb://${test.server.uri()}/test`, { - serverApi: null + serverApi: null // TODO(NODE-3807): remove resetting serverApi when the usage of mongodb mock server is removed }); client.connect(function (err, client) { expect(err).to.not.exist; @@ -128,7 +128,7 @@ describe('Max Staleness', function () { var self = this; const configuration = this.configuration; const client = configuration.newClient(`mongodb://${test.server.uri()}/test`, { - serverApi: null + serverApi: null // TODO(NODE-3807): remove resetting serverApi when the usage of mongodb mock server is removed }); client.connect(function (err, client) { expect(err).to.not.exist; @@ -165,7 +165,7 @@ describe('Max Staleness', function () { var self = this; const configuration = this.configuration; const client = configuration.newClient(`mongodb://${test.server.uri()}/test`, { - serverApi: null + serverApi: null // TODO(NODE-3807): remove resetting serverApi when the usage of mongodb mock server is removed }); client.connect(function (err, client) { expect(err).to.not.exist; From 8d4fd462a47db38b8792136fd4a7509b8d720c87 Mon Sep 17 00:00:00 2001 From: Alena Khineika Date: Mon, 4 Dec 2023 16:20:18 +0100 Subject: [PATCH 12/15] refactor: address pr comments --- src/utils.ts | 9 +++++---- .../mongodb-handshake/mongodb-handshake.test.ts | 12 ++++++------ test/unit/cmap/connection.test.ts | 8 ++++---- 3 files changed, 15 insertions(+), 14 deletions(-) diff --git a/src/utils.ts b/src/utils.ts index c6a7a972f75..e2dda3a4ee7 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -372,11 +372,12 @@ export function uuidV4(): Buffer { export function maxWireVersion(topologyOrServer?: Connection | Topology | Server): number { if (topologyOrServer) { if (topologyOrServer.loadBalanced || topologyOrServer.serverApi?.version) { - // Since we do not have a monitor, we assume - // when a server API version or load-balanced topology are requested - // the server is always pointed at the latest mongodb version. There is a risk that for on-prem - // deployments that don't upgrade immediately that this could alert to the + // Since we do not have a monitor in the load balanced mode, + // we assume the load-balanced server is always pointed at the latest mongodb version. + // There is a risk that for on-prem deployments + // that don't upgrade immediately that this could alert to the // application that a feature is available that is actually not. + // We also return the max supported wire version for serverAPI. return MAX_SUPPORTED_WIRE_VERSION; } if (topologyOrServer.hello) { diff --git a/test/integration/mongodb-handshake/mongodb-handshake.test.ts b/test/integration/mongodb-handshake/mongodb-handshake.test.ts index 9362a5168f9..4f65beaf988 100644 --- a/test/integration/mongodb-handshake/mongodb-handshake.test.ts +++ b/test/integration/mongodb-handshake/mongodb-handshake.test.ts @@ -81,7 +81,7 @@ describe('MongoDB Handshake', () => { afterEach(() => sinon.restore()); - it('should send the hello command as OP_MSG', { + it('sends the hello command as OP_MSG', { metadata: { requires: { topology: 'load-balanced' } }, test: async function () { client = this.configuration.newClient({ loadBalanced: true }); @@ -91,7 +91,7 @@ describe('MongoDB Handshake', () => { // Force the driver to send a command to the server in the noauth mode. await client.db('admin').command({ ping: 1 }); expect(writeCommandSpy).to.have.been.called; - expect(writeCommandSpy.firstCall.args[0] instanceof OpMsgRequest).to.equal(true); + expect(writeCommandSpy.firstCall.args[0]).to.be.an.instanceof(OpMsgRequest); } }); }); @@ -105,7 +105,7 @@ describe('MongoDB Handshake', () => { afterEach(() => sinon.restore()); - it('should send the hello command as OP_MSG', { + it('sends the hello command as OP_MSG', { metadata: { requires: { topology: '!load-balanced', mongodb: '>=5.0' } }, test: async function () { client = this.configuration.newClient({}, { serverApi: { version: ServerApiVersion.v1 } }); @@ -115,7 +115,7 @@ describe('MongoDB Handshake', () => { // Force the driver to send a command to the server in the noauth mode. await client.db('admin').command({ ping: 1 }); expect(writeCommandSpy).to.have.been.called; - expect(writeCommandSpy.firstCall.args[0] instanceof OpMsgRequest).to.equal(true); + expect(writeCommandSpy.firstCall.args[0]).to.be.an.instanceof(OpMsgRequest); } }); }); @@ -129,7 +129,7 @@ describe('MongoDB Handshake', () => { afterEach(() => sinon.restore()); - it('should send the hello command as OP_MSG', { + it('sends the hello command as OP_MSG', { metadata: { requires: { topology: '!load-balanced', mongodb: '>=5.0' } }, test: async function () { if (this.configuration.serverApi) { @@ -147,7 +147,7 @@ describe('MongoDB Handshake', () => { const opRequests = writeCommandSpy.getCalls().map(items => items.args[0]); expect(opRequests[0] instanceof OpQueryRequest).to.equal(true); const isOpMsgRequestSent = !!opRequests.find(op => op instanceof OpMsgRequest); - expect(isOpMsgRequestSent).to.equal(true); + expect(isOpMsgRequestSent).to.be.true; } }); }); diff --git a/test/unit/cmap/connection.test.ts b/test/unit/cmap/connection.test.ts index efc26ed50a3..d4f267432cc 100644 --- a/test/unit/cmap/connection.test.ts +++ b/test/unit/cmap/connection.test.ts @@ -1080,7 +1080,7 @@ describe('new Connection()', function () { } expect(writeCommandSpy).to.have.been.called; - expect(writeCommandSpy.firstCall.args[0] instanceof OpMsgRequest).to.equal(true); + expect(writeCommandSpy.firstCall.args[0]).to.be.an.instanceof(OpMsgRequest); }); }); @@ -1132,7 +1132,7 @@ describe('new Connection()', function () { } expect(writeCommandSpy).to.have.been.called; - expect(writeCommandSpy.firstCall.args[0] instanceof OpQueryRequest).to.equal(true); + expect(writeCommandSpy.firstCall.args[0]).to.be.an.instanceof(OpQueryRequest); }); }); @@ -1185,7 +1185,7 @@ describe('new Connection()', function () { } expect(writeCommandSpy).to.have.been.called; - expect(writeCommandSpy.firstCall.args[0] instanceof OpMsgRequest).to.equal(true); + expect(writeCommandSpy.firstCall.args[0]).to.be.an.instanceof(OpMsgRequest); }); }); @@ -1237,7 +1237,7 @@ describe('new Connection()', function () { } expect(writeCommandSpy).to.have.been.called; - expect(writeCommandSpy.firstCall.args[0] instanceof OpQueryRequest).to.equal(true); + expect(writeCommandSpy.firstCall.args[0]).to.be.an.instanceof(OpQueryRequest); }); }); }); From 30715776318cf6694c71fd0f4154d714d0a45687 Mon Sep 17 00:00:00 2001 From: Alena Khineika Date: Tue, 5 Dec 2023 18:48:19 +0100 Subject: [PATCH 13/15] test: spy on toBin --- .../mongodb-handshake.test.ts | 40 ++-- test/unit/cmap/connection.test.ts | 216 +----------------- 2 files changed, 17 insertions(+), 239 deletions(-) diff --git a/test/integration/mongodb-handshake/mongodb-handshake.test.ts b/test/integration/mongodb-handshake/mongodb-handshake.test.ts index 4f65beaf988..1660d8d29bf 100644 --- a/test/integration/mongodb-handshake/mongodb-handshake.test.ts +++ b/test/integration/mongodb-handshake/mongodb-handshake.test.ts @@ -5,7 +5,6 @@ import * as sinon from 'sinon'; import { Connection, LEGACY_HELLO_COMMAND, - MessageStream, MongoServerError, MongoServerSelectionError, OpMsgRequest, @@ -73,10 +72,10 @@ describe('MongoDB Handshake', () => { }); context('when load-balanced', function () { - let writeCommandSpy: Sinon.SinonSpy; + let opMsgRequestToBinSpy: Sinon.SinonSpy; beforeEach(() => { - writeCommandSpy = sinon.spy(MessageStream.prototype, 'writeCommand'); + opMsgRequestToBinSpy = sinon.spy(OpMsgRequest.prototype, 'toBin'); }); afterEach(() => sinon.restore()); @@ -85,22 +84,21 @@ describe('MongoDB Handshake', () => { metadata: { requires: { topology: 'load-balanced' } }, test: async function () { client = this.configuration.newClient({ loadBalanced: true }); - await client.connect(); // The load-balanced mode doesn’t perform SDAM, // so `connect` doesn’t do anything unless authentication is enabled. // Force the driver to send a command to the server in the noauth mode. await client.db('admin').command({ ping: 1 }); - expect(writeCommandSpy).to.have.been.called; - expect(writeCommandSpy.firstCall.args[0]).to.be.an.instanceof(OpMsgRequest); + expect(opMsgRequestToBinSpy).to.have.been.called; + expect(opMsgRequestToBinSpy.firstCall.args[0]).to.be.an.instanceof(OpMsgRequest); } }); }); context('when serverApi version is present', function () { - let writeCommandSpy: Sinon.SinonSpy; + let opMsgRequestToBinSpy: Sinon.SinonSpy; beforeEach(() => { - writeCommandSpy = sinon.spy(MessageStream.prototype, 'writeCommand'); + opMsgRequestToBinSpy = sinon.spy(OpMsgRequest.prototype, 'toBin'); }); afterEach(() => sinon.restore()); @@ -110,21 +108,19 @@ describe('MongoDB Handshake', () => { test: async function () { client = this.configuration.newClient({}, { serverApi: { version: ServerApiVersion.v1 } }); await client.connect(); - // The load-balanced mode doesn’t perform SDAM, - // so `connect` doesn’t do anything unless authentication is enabled. - // Force the driver to send a command to the server in the noauth mode. - await client.db('admin').command({ ping: 1 }); - expect(writeCommandSpy).to.have.been.called; - expect(writeCommandSpy.firstCall.args[0]).to.be.an.instanceof(OpMsgRequest); + expect(opMsgRequestToBinSpy).to.have.been.called; + expect(opMsgRequestToBinSpy.firstCall.args[0]).to.be.an.instanceof(OpMsgRequest); } }); }); context('when not load-balanced and serverApi version is not present', function () { - let writeCommandSpy: Sinon.SinonSpy; + let opQueryRequestToBinSpy: Sinon.SinonSpy; + let opMsgRequestToBinSpy: Sinon.SinonSpy; beforeEach(() => { - writeCommandSpy = sinon.spy(MessageStream.prototype, 'writeCommand'); + opQueryRequestToBinSpy = sinon.spy(OpQueryRequest.prototype, 'toBin'); + opMsgRequestToBinSpy = sinon.spy(OpMsgRequest.prototype, 'toBin'); }); afterEach(() => sinon.restore()); @@ -138,16 +134,10 @@ describe('MongoDB Handshake', () => { } client = this.configuration.newClient(); await client.connect(); - // The load-balanced mode doesn’t perform SDAM, - // so `connect` doesn’t do anything unless authentication is enabled. - // Force the driver to send a command to the server in the noauth mode. await client.db('admin').command({ ping: 1 }); - expect(writeCommandSpy).to.have.been.called; - - const opRequests = writeCommandSpy.getCalls().map(items => items.args[0]); - expect(opRequests[0] instanceof OpQueryRequest).to.equal(true); - const isOpMsgRequestSent = !!opRequests.find(op => op instanceof OpMsgRequest); - expect(isOpMsgRequestSent).to.be.true; + expect(opQueryRequestToBinSpy).to.have.been.called; + expect(opMsgRequestToBinSpy).to.have.been.called; + opMsgRequestToBinSpy.calledAfter(opQueryRequestToBinSpy); } }); }); diff --git a/test/unit/cmap/connection.test.ts b/test/unit/cmap/connection.test.ts index d4f267432cc..d3c33193d61 100644 --- a/test/unit/cmap/connection.test.ts +++ b/test/unit/cmap/connection.test.ts @@ -14,15 +14,13 @@ import { type HostAddress, isHello, type MessageHeader, - MessageStream, + type MessageStream, MongoNetworkError, MongoNetworkTimeoutError, MongoRuntimeError, ns, type OperationDescription, - OpMsgRequest, - OpMsgResponse, - OpQueryRequest + OpMsgResponse } from '../../mongodb'; import * as mock from '../../tools/mongodb-mock/index'; import { generateOpMsgBuffer, getSymbolFrom } from '../../tools/utils'; @@ -1030,214 +1028,4 @@ describe('new Connection()', function () { }); }); }); - - describe('when load-balanced', () => { - const CONNECT_DEFAULTS = { - id: 1, - tls: false, - generation: 1, - monitorCommands: false, - metadata: {} as ClientMetadata - }; - let server; - let connectOptions; - let connection: Connection; - let writeCommandSpy; - - beforeEach(async () => { - server = await mock.createServer(); - server.setMessageHandler(request => { - request.reply(mock.HELLO); - }); - writeCommandSpy = sinon.spy(MessageStream.prototype, 'writeCommand'); - }); - - afterEach(async () => { - connection?.destroy({ force: true }); - sinon.restore(); - await mock.cleanup(); - }); - - it('sends the first command as OP_MSG', async () => { - try { - connectOptions = { - ...CONNECT_DEFAULTS, - hostAddress: server.hostAddress() as HostAddress, - socketTimeoutMS: 100, - loadBalanced: true - }; - - connection = await promisify(callback => - //@ts-expect-error: Callbacks do not have mutual exclusion for error/result existence - connect(connectOptions, callback) - )(); - - await promisify(callback => - connection.command(ns('admin.$cmd'), { hello: 1 }, {}, callback) - )(); - } catch (error) { - /** Connection timeouts, but the handshake message is sent. */ - } - - expect(writeCommandSpy).to.have.been.called; - expect(writeCommandSpy.firstCall.args[0]).to.be.an.instanceof(OpMsgRequest); - }); - }); - - describe('when not load-balanced', () => { - const CONNECT_DEFAULTS = { - id: 1, - tls: false, - generation: 1, - monitorCommands: false, - metadata: {} as ClientMetadata - }; - let server; - let connectOptions; - let connection: Connection; - let writeCommandSpy; - - beforeEach(async () => { - server = await mock.createServer(); - server.setMessageHandler(request => { - request.reply(mock.HELLO); - }); - writeCommandSpy = sinon.spy(MessageStream.prototype, 'writeCommand'); - }); - - afterEach(async () => { - connection?.destroy({ force: true }); - sinon.restore(); - await mock.cleanup(); - }); - - it('sends the first command as OP_QUERY', async () => { - try { - connectOptions = { - ...CONNECT_DEFAULTS, - hostAddress: server.hostAddress() as HostAddress, - socketTimeoutMS: 100 - }; - - connection = await promisify(callback => - //@ts-expect-error: Callbacks do not have mutual exclusion for error/result existence - connect(connectOptions, callback) - )(); - - await promisify(callback => - connection.command(ns('admin.$cmd'), { hello: 1 }, {}, callback) - )(); - } catch (error) { - /** Connection timeouts, but the handshake message is sent. */ - } - - expect(writeCommandSpy).to.have.been.called; - expect(writeCommandSpy.firstCall.args[0]).to.be.an.instanceof(OpQueryRequest); - }); - }); - - describe('when serverApi version is present', () => { - const CONNECT_DEFAULTS = { - id: 1, - tls: false, - generation: 1, - monitorCommands: false, - metadata: {} as ClientMetadata - }; - let server; - let connectOptions; - let connection: Connection; - let writeCommandSpy; - - beforeEach(async () => { - server = await mock.createServer(); - server.setMessageHandler(request => { - request.reply(mock.HELLO); - }); - writeCommandSpy = sinon.spy(MessageStream.prototype, 'writeCommand'); - }); - - afterEach(async () => { - connection?.destroy({ force: true }); - sinon.restore(); - await mock.cleanup(); - }); - - it('sends the first command as OP_MSG', async () => { - try { - connectOptions = { - ...CONNECT_DEFAULTS, - hostAddress: server.hostAddress() as HostAddress, - socketTimeoutMS: 100, - serverApi: { version: '1' } - }; - - connection = await promisify(callback => - //@ts-expect-error: Callbacks do not have mutual exclusion for error/result existence - connect(connectOptions, callback) - )(); - - await promisify(callback => - connection.command(ns('admin.$cmd'), { hello: 1 }, {}, callback) - )(); - } catch (error) { - /** Connection timeouts, but the handshake message is sent. */ - } - - expect(writeCommandSpy).to.have.been.called; - expect(writeCommandSpy.firstCall.args[0]).to.be.an.instanceof(OpMsgRequest); - }); - }); - - describe('when serverApi version is not present', () => { - const CONNECT_DEFAULTS = { - id: 1, - tls: false, - generation: 1, - monitorCommands: false, - metadata: {} as ClientMetadata - }; - let server; - let connectOptions; - let connection: Connection; - let writeCommandSpy; - - beforeEach(async () => { - server = await mock.createServer(); - server.setMessageHandler(request => { - request.reply(mock.HELLO); - }); - writeCommandSpy = sinon.spy(MessageStream.prototype, 'writeCommand'); - }); - - afterEach(async () => { - connection?.destroy({ force: true }); - sinon.restore(); - await mock.cleanup(); - }); - - it('sends the first command as OP_MSG', async () => { - try { - connectOptions = { - ...CONNECT_DEFAULTS, - hostAddress: server.hostAddress() as HostAddress, - socketTimeoutMS: 100 - }; - - connection = await promisify(callback => - //@ts-expect-error: Callbacks do not have mutual exclusion for error/result existence - connect(connectOptions, callback) - )(); - - await promisify(callback => - connection.command(ns('admin.$cmd'), { hello: 1 }, {}, callback) - )(); - } catch (error) { - /** Connection timeouts, but the handshake message is sent. */ - } - - expect(writeCommandSpy).to.have.been.called; - expect(writeCommandSpy.firstCall.args[0]).to.be.an.instanceof(OpQueryRequest); - }); - }); }); From a952ceab8117354e0266746b5807f4a065ed4d38 Mon Sep 17 00:00:00 2001 From: Alena Khineika Date: Wed, 6 Dec 2023 09:28:48 +0100 Subject: [PATCH 14/15] test: clean up --- test/integration/mongodb-handshake/mongodb-handshake.test.ts | 4 ---- 1 file changed, 4 deletions(-) diff --git a/test/integration/mongodb-handshake/mongodb-handshake.test.ts b/test/integration/mongodb-handshake/mongodb-handshake.test.ts index 1660d8d29bf..3671764ccd0 100644 --- a/test/integration/mongodb-handshake/mongodb-handshake.test.ts +++ b/test/integration/mongodb-handshake/mongodb-handshake.test.ts @@ -60,7 +60,6 @@ describe('MongoDB Handshake', () => { it('constructs a handshake with the specified compressors', async function () { client = this.configuration.newClient({ compressors: ['snappy'] }); - await client.connect(); // The load-balanced mode doesn’t perform SDAM, // so `connect` doesn’t do anything unless authentication is enabled. // Force the driver to send a command to the server in the noauth mode. @@ -89,7 +88,6 @@ describe('MongoDB Handshake', () => { // Force the driver to send a command to the server in the noauth mode. await client.db('admin').command({ ping: 1 }); expect(opMsgRequestToBinSpy).to.have.been.called; - expect(opMsgRequestToBinSpy.firstCall.args[0]).to.be.an.instanceof(OpMsgRequest); } }); }); @@ -109,7 +107,6 @@ describe('MongoDB Handshake', () => { client = this.configuration.newClient({}, { serverApi: { version: ServerApiVersion.v1 } }); await client.connect(); expect(opMsgRequestToBinSpy).to.have.been.called; - expect(opMsgRequestToBinSpy.firstCall.args[0]).to.be.an.instanceof(OpMsgRequest); } }); }); @@ -134,7 +131,6 @@ describe('MongoDB Handshake', () => { } client = this.configuration.newClient(); await client.connect(); - await client.db('admin').command({ ping: 1 }); expect(opQueryRequestToBinSpy).to.have.been.called; expect(opMsgRequestToBinSpy).to.have.been.called; opMsgRequestToBinSpy.calledAfter(opQueryRequestToBinSpy); From 85e788bdde3218c15b06e01f8903efe7627db703 Mon Sep 17 00:00:00 2001 From: Alena Khineika Date: Wed, 6 Dec 2023 10:41:14 +0100 Subject: [PATCH 15/15] tests: fix noauth --- test/integration/mongodb-handshake/mongodb-handshake.test.ts | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/test/integration/mongodb-handshake/mongodb-handshake.test.ts b/test/integration/mongodb-handshake/mongodb-handshake.test.ts index 3671764ccd0..4e7399fe182 100644 --- a/test/integration/mongodb-handshake/mongodb-handshake.test.ts +++ b/test/integration/mongodb-handshake/mongodb-handshake.test.ts @@ -83,9 +83,6 @@ describe('MongoDB Handshake', () => { metadata: { requires: { topology: 'load-balanced' } }, test: async function () { client = this.configuration.newClient({ loadBalanced: true }); - // The load-balanced mode doesn’t perform SDAM, - // so `connect` doesn’t do anything unless authentication is enabled. - // Force the driver to send a command to the server in the noauth mode. await client.db('admin').command({ ping: 1 }); expect(opMsgRequestToBinSpy).to.have.been.called; } @@ -130,7 +127,7 @@ describe('MongoDB Handshake', () => { return this.skip(); } client = this.configuration.newClient(); - await client.connect(); + await client.db('admin').command({ ping: 1 }); expect(opQueryRequestToBinSpy).to.have.been.called; expect(opMsgRequestToBinSpy).to.have.been.called; opMsgRequestToBinSpy.calledAfter(opQueryRequestToBinSpy);