From 0752e9bd930862a8ae77bcd60870d445eea74da9 Mon Sep 17 00:00:00 2001 From: Alena Khineika Date: Tue, 31 Oct 2023 15:56:42 +0100 Subject: [PATCH 01/14] feat(NODE-3881): require hello command + OP_MSG when 'loadBalanced=True' --- src/cmap/connect.ts | 2 +- src/cmap/connection.ts | 14 +++++++++++--- test/unit/cmap/connect.test.ts | 15 ++++++++++++++- 3 files changed, 26 insertions(+), 5 deletions(-) diff --git a/src/cmap/connect.ts b/src/cmap/connect.ts index 3cf6db7447..58a7affddc 100644 --- a/src/cmap/connect.ts +++ b/src/cmap/connect.ts @@ -214,7 +214,7 @@ export async function prepareHandshakeDocument( const { serverApi } = authContext.connection; const handshakeDoc: HandshakeDocument = { - [serverApi?.version ? 'hello' : LEGACY_HELLO_COMMAND]: 1, + [serverApi?.version || options.loadBalanced ? 'hello' : LEGACY_HELLO_COMMAND]: 1, helloOk: true, client: options.metadata, compression: compressors diff --git a/src/cmap/connection.ts b/src/cmap/connection.ts index 67dc42ee7f..598a91a216 100644 --- a/src/cmap/connection.ts +++ b/src/cmap/connection.ts @@ -638,12 +638,20 @@ export function hasSessionSupport(conn: Connection): boolean { } function supportsOpMsg(conn: Connection) { - const description = conn.description; - if (description == null) { + const { description, serverApi, loadBalanced } = conn; + + if (description == null || description.__nodejs_mock_server__) { return false; } - return maxWireVersion(conn) >= 6 && !description.__nodejs_mock_server__; + // Handshake spec requires us to use hello command + OP_MSG for the + // initial handshake in load balanced or versioned api mode. + if (serverApi || loadBalanced) { + return true; + } + + // Use OP_MSG for all future commands. + return maxWireVersion(conn) >= 6; } function streamIdentifier(stream: Stream, options: ConnectionOptions): string { diff --git a/test/unit/cmap/connect.test.ts b/test/unit/cmap/connect.test.ts index c3327a8646..28efa2a12a 100644 --- a/test/unit/cmap/connect.test.ts +++ b/test/unit/cmap/connect.test.ts @@ -219,7 +219,20 @@ describe('Connect Tests', function () { }); }); - context('when serverApi is not present', () => { + context('when loadBalanced is true', () => { + const options = { loadBalanced: true }; + const authContext = { + connection: {}, + options + }; + + it('sets the hello parameter to 1', async () => { + const handshakeDocument = await prepareHandshakeDocument(authContext); + expect(handshakeDocument).to.have.property('hello', 1); + }); + }); + + context('when serverApi and loadBalanced are not present', () => { const options = {}; const authContext = { connection: {}, From 8f25b3828ca11dbb9339a53159b348bb9a24d433 Mon Sep 17 00:00:00 2001 From: Alena Khineika Date: Wed, 1 Nov 2023 16:27:30 +0100 Subject: [PATCH 02/14] test: add supportsOpMsg unit tests --- src/cmap/connect.ts | 2 +- src/cmap/connection.ts | 29 +++++----- test/unit/cmap/connection.test.ts | 90 ++++++++++++++++++++++++++++++- 3 files changed, 107 insertions(+), 14 deletions(-) diff --git a/src/cmap/connect.ts b/src/cmap/connect.ts index 58a7affddc..d3839a5a03 100644 --- a/src/cmap/connect.ts +++ b/src/cmap/connect.ts @@ -214,7 +214,7 @@ export async function prepareHandshakeDocument( const { serverApi } = authContext.connection; const handshakeDoc: HandshakeDocument = { - [serverApi?.version || options.loadBalanced ? 'hello' : LEGACY_HELLO_COMMAND]: 1, + [serverApi?.version || options.loadBalanced === true ? 'hello' : LEGACY_HELLO_COMMAND]: 1, helloOk: true, client: options.metadata, compression: compressors diff --git a/src/cmap/connection.ts b/src/cmap/connection.ts index 598a91a216..ff9d421eef 100644 --- a/src/cmap/connection.ts +++ b/src/cmap/connection.ts @@ -487,7 +487,7 @@ export class Connection extends TypedEventEmitter { let cmd = { ...command }; const readPreference = getReadPreference(options); - const shouldUseOpMsg = supportsOpMsg(this); + const shouldUseOpMsg = supportsOpMsg(this, { loadBalanced: cmd.loadBalanced }); const session = options?.session; let clusterTime = this.clusterTime; @@ -637,21 +637,26 @@ export function hasSessionSupport(conn: Connection): boolean { return description.logicalSessionTimeoutMinutes != null; } -function supportsOpMsg(conn: Connection) { - const { description, serverApi, loadBalanced } = conn; - - if (description == null || description.__nodejs_mock_server__) { - return false; +/** @internal */ +export function supportsOpMsg(conn: Connection, options: { loadBalanced?: boolean }) { + // If server API versioning has been requested or loadBalanced is true, + // then we MUST send the initial hello command using OP_MSG. + // Since this is the first message and hello/legacy hello hasn't been sent yet, + // conn.description will be null and we can't rely on the server check to determine if + // the server supports OP_MSG. + if (conn.serverApi?.version || options.loadBalanced === true) { + return true; } - // Handshake spec requires us to use hello command + OP_MSG for the - // initial handshake in load balanced or versioned api mode. - if (serverApi || loadBalanced) { - return true; + // If server API versioning and loadBalanced are not requested, + // we MUST use legacy hello for the first message of the initial handshake with the OP_QUERY protocol + // before switching to OP_MSG if the maxWireVersion indicates compatibility. + const description = conn.description; + if (description == null) { + return false; } - // Use OP_MSG for all future commands. - return maxWireVersion(conn) >= 6; + return maxWireVersion(conn) >= 6 && !description.__nodejs_mock_server__; } function streamIdentifier(stream: Stream, options: ConnectionOptions): string { diff --git a/test/unit/cmap/connection.test.ts b/test/unit/cmap/connection.test.ts index 42c4dfae9d..b1eb6ef99f 100644 --- a/test/unit/cmap/connection.test.ts +++ b/test/unit/cmap/connection.test.ts @@ -19,7 +19,8 @@ import { MongoNetworkTimeoutError, MongoRuntimeError, ns, - type OperationDescription + type OperationDescription, + supportsOpMsg } from '../../mongodb'; import * as mock from '../../tools/mongodb-mock/index'; import { generateOpMsgBuffer, getSymbolFrom } from '../../tools/utils'; @@ -894,6 +895,93 @@ describe('new Connection()', function () { }); }); + describe('supportsOpMsg', function () { + let connection; + + context('serverApi versioning is requested', function () { + beforeEach(function () { + connection = { + serverApi: { version: '1' } + }; + }); + + it('returns true', function () { + expect(supportsOpMsg(connection, {})).to.be.true; + }); + }); + + context('loadBalanced option is true', function () { + beforeEach(function () { + connection = {}; + }); + + it('returns true', function () { + expect(supportsOpMsg(connection, { loadBalanced: true })).to.be.true; + }); + }); + + context( + 'serverApi and loadBalanced are not requested, the first message has not been sent', + function () { + beforeEach(function () { + connection = {}; + }); + + it('returns false', function () { + expect(supportsOpMsg(connection, {})).to.be.false; + }); + } + ); + + context( + 'serverApi and loadBalanced are not requested, the first message has been sent already, maxWireVersion matches the compatibile version', + function () { + beforeEach(function () { + connection = { + description: new InputStream(), + hello: { maxWireVersion: 6 } + }; + }); + + it('returns true', function () { + expect(supportsOpMsg(connection, {})).to.be.true; + }); + } + ); + + context( + 'serverApi and loadBalanced are not requested, the first message has been sent already, maxWireVersion is above the compatibile version', + function () { + beforeEach(function () { + connection = { + description: new InputStream(), + hello: { maxWireVersion: 7 } + }; + }); + + it('returns true', function () { + expect(supportsOpMsg(connection, {})).to.be.true; + }); + } + ); + + context( + 'serverApi and loadBalanced are not requested, the first message has been sent already, maxWireVersion is not compatibile', + function () { + beforeEach(function () { + connection = { + description: new InputStream(), + hello: { maxWireVersion: 5 } + }; + }); + + it('returns false', function () { + expect(supportsOpMsg(connection, {})).to.be.false; + }); + } + ); + }); + describe('destroy()', () => { let connection: sinon.SinonSpiedInstance; let clock: sinon.SinonFakeTimers; From dd29305c1f5acde81ed3bc088e33f044c08a2dec Mon Sep 17 00:00:00 2001 From: Alena Khineika Date: Wed, 1 Nov 2023 17:24:36 +0100 Subject: [PATCH 03/14] test: move test case to another context --- test/unit/cmap/connect.test.ts | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-) diff --git a/test/unit/cmap/connect.test.ts b/test/unit/cmap/connect.test.ts index 28efa2a12a..57eedecedc 100644 --- a/test/unit/cmap/connect.test.ts +++ b/test/unit/cmap/connect.test.ts @@ -219,20 +219,7 @@ describe('Connect Tests', function () { }); }); - context('when loadBalanced is true', () => { - const options = { loadBalanced: true }; - const authContext = { - connection: {}, - options - }; - - it('sets the hello parameter to 1', async () => { - const handshakeDocument = await prepareHandshakeDocument(authContext); - expect(handshakeDocument).to.have.property('hello', 1); - }); - }); - - context('when serverApi and loadBalanced are not present', () => { + context('when serverApi is not present', () => { const options = {}; const authContext = { connection: {}, @@ -255,6 +242,7 @@ describe('Connect Tests', function () { }; const handshakeDocument = await prepareHandshakeDocument(authContext); expect(handshakeDocument).not.to.have.property('loadBalanced'); + expect(handshakeDocument).to.have.property(LEGACY_HELLO_COMMAND, 1); }); }); @@ -269,6 +257,7 @@ describe('Connect Tests', function () { }; const handshakeDocument = await prepareHandshakeDocument(authContext); expect(handshakeDocument).not.to.have.property('loadBalanced'); + expect(handshakeDocument).to.have.property(LEGACY_HELLO_COMMAND, 1); }); }); @@ -283,6 +272,7 @@ describe('Connect Tests', function () { }; const handshakeDocument = await prepareHandshakeDocument(authContext); expect(handshakeDocument).to.have.property('loadBalanced', true); + expect(handshakeDocument).to.have.property('hello', 1); }); }); }); From da3fd621901fc844961a96cf265334b92ec0d364 Mon Sep 17 00:00:00 2001 From: Alena Khineika Date: Thu, 2 Nov 2023 01:05:28 +0100 Subject: [PATCH 04/14] refactor: narrow down to loadBalanced usage --- src/cmap/connection.ts | 10 +++------- test/unit/cmap/connection.test.ts | 12 ------------ 2 files changed, 3 insertions(+), 19 deletions(-) diff --git a/src/cmap/connection.ts b/src/cmap/connection.ts index ff9d421eef..c4d6fa3d26 100644 --- a/src/cmap/connection.ts +++ b/src/cmap/connection.ts @@ -639,19 +639,15 @@ export function hasSessionSupport(conn: Connection): boolean { /** @internal */ export function supportsOpMsg(conn: Connection, options: { loadBalanced?: boolean }) { - // If server API versioning has been requested or loadBalanced is true, - // then we MUST send the initial hello command using OP_MSG. + // If loadBalanced is true, then we MUST send the initial hello command using OP_MSG. // Since this is the first message and hello/legacy hello hasn't been sent yet, // conn.description will be null and we can't rely on the server check to determine if // the server supports OP_MSG. - if (conn.serverApi?.version || options.loadBalanced === true) { + const description = conn.description; + if (options.loadBalanced === true && description == null) { return true; } - // If server API versioning and loadBalanced are not requested, - // we MUST use legacy hello for the first message of the initial handshake with the OP_QUERY protocol - // before switching to OP_MSG if the maxWireVersion indicates compatibility. - const description = conn.description; if (description == null) { return false; } diff --git a/test/unit/cmap/connection.test.ts b/test/unit/cmap/connection.test.ts index b1eb6ef99f..fe51d84826 100644 --- a/test/unit/cmap/connection.test.ts +++ b/test/unit/cmap/connection.test.ts @@ -898,18 +898,6 @@ describe('new Connection()', function () { describe('supportsOpMsg', function () { let connection; - context('serverApi versioning is requested', function () { - beforeEach(function () { - connection = { - serverApi: { version: '1' } - }; - }); - - it('returns true', function () { - expect(supportsOpMsg(connection, {})).to.be.true; - }); - }); - context('loadBalanced option is true', function () { beforeEach(function () { connection = {}; From a180dcb88f07fb180b96690ff0375efde7f8b36d Mon Sep 17 00:00:00 2001 From: Alena Khineika Date: Thu, 2 Nov 2023 11:36:37 +0100 Subject: [PATCH 05/14] test: update descriptions --- test/unit/cmap/connection.test.ts | 25 +++++++++++-------------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/test/unit/cmap/connection.test.ts b/test/unit/cmap/connection.test.ts index fe51d84826..62c9b99b39 100644 --- a/test/unit/cmap/connection.test.ts +++ b/test/unit/cmap/connection.test.ts @@ -908,21 +908,18 @@ describe('new Connection()', function () { }); }); - context( - 'serverApi and loadBalanced are not requested, the first message has not been sent', - function () { - beforeEach(function () { - connection = {}; - }); + context('loadBalanced is not requested and this is the first message', function () { + beforeEach(function () { + connection = {}; // The first message, therefore no description. + }); - it('returns false', function () { - expect(supportsOpMsg(connection, {})).to.be.false; - }); - } - ); + it('returns false', function () { + expect(supportsOpMsg(connection, {})).to.be.false; + }); + }); context( - 'serverApi and loadBalanced are not requested, the first message has been sent already, maxWireVersion matches the compatibile version', + 'the first message has been sent and maxWireVersion matches the compatible version', function () { beforeEach(function () { connection = { @@ -938,7 +935,7 @@ describe('new Connection()', function () { ); context( - 'serverApi and loadBalanced are not requested, the first message has been sent already, maxWireVersion is above the compatibile version', + 'the first message has been sent and maxWireVersion is above the compatible version', function () { beforeEach(function () { connection = { @@ -954,7 +951,7 @@ describe('new Connection()', function () { ); context( - 'serverApi and loadBalanced are not requested, the first message has been sent already, maxWireVersion is not compatibile', + 'the first message has been sent and maxWireVersion is below the compatible version', function () { beforeEach(function () { connection = { From 46acca897edf8f7fad6df2abe1003a38af8520e1 Mon Sep 17 00:00:00 2001 From: Alena Khineika Date: Thu, 2 Nov 2023 15:16:00 +0100 Subject: [PATCH 06/14] Update test/unit/cmap/connection.test.ts Co-authored-by: Durran Jordan --- test/unit/cmap/connection.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unit/cmap/connection.test.ts b/test/unit/cmap/connection.test.ts index 62c9b99b39..3755da773d 100644 --- a/test/unit/cmap/connection.test.ts +++ b/test/unit/cmap/connection.test.ts @@ -895,7 +895,7 @@ describe('new Connection()', function () { }); }); - describe('supportsOpMsg', function () { + describe('.supportsOpMsg', function () { let connection; context('loadBalanced option is true', function () { From de61753cd46ffcac591d74e3233c042c0a196173 Mon Sep 17 00:00:00 2001 From: Alena Khineika Date: Thu, 2 Nov 2023 21:50:54 +0100 Subject: [PATCH 07/14] test: try test in CI --- src/cmap/connection.ts | 13 +--- .../mongodb-handshake.test.ts | 21 +++++- test/unit/cmap/connection.test.ts | 75 +------------------ 3 files changed, 23 insertions(+), 86 deletions(-) diff --git a/src/cmap/connection.ts b/src/cmap/connection.ts index c4d6fa3d26..67dc42ee7f 100644 --- a/src/cmap/connection.ts +++ b/src/cmap/connection.ts @@ -487,7 +487,7 @@ export class Connection extends TypedEventEmitter { let cmd = { ...command }; const readPreference = getReadPreference(options); - const shouldUseOpMsg = supportsOpMsg(this, { loadBalanced: cmd.loadBalanced }); + const shouldUseOpMsg = supportsOpMsg(this); const session = options?.session; let clusterTime = this.clusterTime; @@ -637,17 +637,8 @@ export function hasSessionSupport(conn: Connection): boolean { return description.logicalSessionTimeoutMinutes != null; } -/** @internal */ -export function supportsOpMsg(conn: Connection, options: { loadBalanced?: boolean }) { - // If loadBalanced is true, then we MUST send the initial hello command using OP_MSG. - // Since this is the first message and hello/legacy hello hasn't been sent yet, - // conn.description will be null and we can't rely on the server check to determine if - // the server supports OP_MSG. +function supportsOpMsg(conn: Connection) { const description = conn.description; - if (options.loadBalanced === true && description == null) { - return true; - } - if (description == null) { return false; } diff --git a/test/integration/mongodb-handshake/mongodb-handshake.test.ts b/test/integration/mongodb-handshake/mongodb-handshake.test.ts index 2e00d0e2e9..b0d4521749 100644 --- a/test/integration/mongodb-handshake/mongodb-handshake.test.ts +++ b/test/integration/mongodb-handshake/mongodb-handshake.test.ts @@ -6,7 +6,8 @@ import { Connection, LEGACY_HELLO_COMMAND, MongoServerError, - MongoServerSelectionError + MongoServerSelectionError, + Msg } from '../../mongodb'; describe('MongoDB Handshake', () => { @@ -62,4 +63,22 @@ describe('MongoDB Handshake', () => { expect(handshakeDoc).to.have.property('compression').to.deep.equal(['snappy']); }); }); + + context('when load-balanced', function () { + let spy: Sinon.SinonSpy; + before(() => { + spy = sinon.spy(Msg.prototype, 'makeDocumentSegment'); + }); + + after(() => 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(spy.called).to.be.true; + } + }); + }); }); diff --git a/test/unit/cmap/connection.test.ts b/test/unit/cmap/connection.test.ts index 3755da773d..42c4dfae9d 100644 --- a/test/unit/cmap/connection.test.ts +++ b/test/unit/cmap/connection.test.ts @@ -19,8 +19,7 @@ import { MongoNetworkTimeoutError, MongoRuntimeError, ns, - type OperationDescription, - supportsOpMsg + type OperationDescription } from '../../mongodb'; import * as mock from '../../tools/mongodb-mock/index'; import { generateOpMsgBuffer, getSymbolFrom } from '../../tools/utils'; @@ -895,78 +894,6 @@ describe('new Connection()', function () { }); }); - describe('.supportsOpMsg', function () { - let connection; - - context('loadBalanced option is true', function () { - beforeEach(function () { - connection = {}; - }); - - it('returns true', function () { - expect(supportsOpMsg(connection, { loadBalanced: true })).to.be.true; - }); - }); - - context('loadBalanced is not requested and this is the first message', function () { - beforeEach(function () { - connection = {}; // The first message, therefore no description. - }); - - it('returns false', function () { - expect(supportsOpMsg(connection, {})).to.be.false; - }); - }); - - context( - 'the first message has been sent and maxWireVersion matches the compatible version', - function () { - beforeEach(function () { - connection = { - description: new InputStream(), - hello: { maxWireVersion: 6 } - }; - }); - - it('returns true', function () { - expect(supportsOpMsg(connection, {})).to.be.true; - }); - } - ); - - context( - 'the first message has been sent and maxWireVersion is above the compatible version', - function () { - beforeEach(function () { - connection = { - description: new InputStream(), - hello: { maxWireVersion: 7 } - }; - }); - - it('returns true', function () { - expect(supportsOpMsg(connection, {})).to.be.true; - }); - } - ); - - context( - 'the first message has been sent and maxWireVersion is below the compatible version', - function () { - beforeEach(function () { - connection = { - description: new InputStream(), - hello: { maxWireVersion: 5 } - }; - }); - - it('returns false', function () { - expect(supportsOpMsg(connection, {})).to.be.false; - }); - } - ); - }); - describe('destroy()', () => { let connection: sinon.SinonSpiedInstance; let clock: sinon.SinonFakeTimers; From 867168d90a2f980368b516e8f1843d406f6bfe43 Mon Sep 17 00:00:00 2001 From: Alena Khineika Date: Thu, 2 Nov 2023 22:37:44 +0100 Subject: [PATCH 08/14] test: split load balanced test cases --- .../mongodb-handshake.test.ts | 18 +++++ test/unit/cmap/connect.test.ts | 67 +++++++++++++------ 2 files changed, 63 insertions(+), 22 deletions(-) diff --git a/test/integration/mongodb-handshake/mongodb-handshake.test.ts b/test/integration/mongodb-handshake/mongodb-handshake.test.ts index b0d4521749..c12344a38b 100644 --- a/test/integration/mongodb-handshake/mongodb-handshake.test.ts +++ b/test/integration/mongodb-handshake/mongodb-handshake.test.ts @@ -81,4 +81,22 @@ describe('MongoDB Handshake', () => { } }); }); + + context('when not load-balanced', function () { + let spy: Sinon.SinonSpy; + before(() => { + spy = sinon.spy(Msg.prototype, 'makeDocumentSegment'); + }); + + after(() => sinon.restore()); + + it('should send the legacy hello command as OP_QUERY', { + metadata: { requires: { topology: '!load-balanced', mongodb: '<6.0.x' } }, + test: async function () { + client = this.configuration.newClient({ loadBalanced: true }); + await client.connect(); + expect(spy.called).to.be.false; + } + }); + }); }); diff --git a/test/unit/cmap/connect.test.ts b/test/unit/cmap/connect.test.ts index 57eedecedc..1638456078 100644 --- a/test/unit/cmap/connect.test.ts +++ b/test/unit/cmap/connect.test.ts @@ -234,45 +234,68 @@ describe('Connect Tests', function () { context('loadBalanced option', () => { context('when loadBalanced is not set as an option', () => { + const authContext = { + connection: {}, + options: {} + }; + it('does not set loadBalanced on the handshake document', async () => { - const options = {}; - const authContext = { - connection: {}, - options - }; const handshakeDocument = await prepareHandshakeDocument(authContext); expect(handshakeDocument).not.to.have.property('loadBalanced'); + }); + + it('does not set hello on the handshake document', async () => { + const handshakeDocument = await prepareHandshakeDocument(authContext); + expect(handshakeDocument).not.to.have.property('hello'); + }); + + it('sets LEGACY_HELLO_COMMAND on the handshake document', async () => { + const handshakeDocument = await prepareHandshakeDocument(authContext); expect(handshakeDocument).to.have.property(LEGACY_HELLO_COMMAND, 1); }); }); context('when loadBalanced is set to false', () => { + const authContext = { + connection: {}, + options: { loadBalanced: false } + }; + it('does not set loadBalanced on the handshake document', async () => { - const options = { - loadBalanced: false - }; - const authContext = { - connection: {}, - options - }; const handshakeDocument = await prepareHandshakeDocument(authContext); expect(handshakeDocument).not.to.have.property('loadBalanced'); + }); + + it('does not set hello on the handshake document', async () => { + const handshakeDocument = await prepareHandshakeDocument(authContext); + expect(handshakeDocument).not.to.have.property('hello'); + }); + + it('sets LEGACY_HELLO_COMMAND on the handshake document', async () => { + const handshakeDocument = await prepareHandshakeDocument(authContext); expect(handshakeDocument).to.have.property(LEGACY_HELLO_COMMAND, 1); }); }); context('when loadBalanced is set to true', () => { - it('does set loadBalanced on the handshake document', async () => { - const options = { - loadBalanced: true - }; - const authContext = { - connection: {}, - options - }; + const authContext = { + connection: {}, + options: { loadBalanced: true } + }; + + it('sets loadBalanced on the handshake document', async () => { + const handshakeDocument = await prepareHandshakeDocument(authContext); + expect(handshakeDocument).to.have.property('loadBalanced'); + }); + + it('sets hello on the handshake document', async () => { + const handshakeDocument = await prepareHandshakeDocument(authContext); + expect(handshakeDocument).to.have.property('hello'); + }); + + it('does not set LEGACY_HELLO_COMMAND on the handshake document', async () => { const handshakeDocument = await prepareHandshakeDocument(authContext); - expect(handshakeDocument).to.have.property('loadBalanced', true); - expect(handshakeDocument).to.have.property('hello', 1); + expect(handshakeDocument).not.have.property(LEGACY_HELLO_COMMAND, 1); }); }); }); From a633333a6eba2ef59d5626b3136d4bbf5fd80ec9 Mon Sep 17 00:00:00 2001 From: Alena Khineika Date: Thu, 2 Nov 2023 23:01:42 +0100 Subject: [PATCH 09/14] test: remove option --- test/integration/mongodb-handshake/mongodb-handshake.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/mongodb-handshake/mongodb-handshake.test.ts b/test/integration/mongodb-handshake/mongodb-handshake.test.ts index c12344a38b..0711f76a68 100644 --- a/test/integration/mongodb-handshake/mongodb-handshake.test.ts +++ b/test/integration/mongodb-handshake/mongodb-handshake.test.ts @@ -93,7 +93,7 @@ describe('MongoDB Handshake', () => { it('should send the legacy hello command as OP_QUERY', { metadata: { requires: { topology: '!load-balanced', mongodb: '<6.0.x' } }, test: async function () { - client = this.configuration.newClient({ loadBalanced: true }); + client = this.configuration.newClient(); await client.connect(); expect(spy.called).to.be.false; } From 8ab159d874f72aa01b52413729ad9e23f3e6c62c Mon Sep 17 00:00:00 2001 From: Alena Khineika Date: Fri, 3 Nov 2023 00:28:29 +0100 Subject: [PATCH 10/14] test: remove test case --- .../mongodb-handshake.test.ts | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/test/integration/mongodb-handshake/mongodb-handshake.test.ts b/test/integration/mongodb-handshake/mongodb-handshake.test.ts index 0711f76a68..b0d4521749 100644 --- a/test/integration/mongodb-handshake/mongodb-handshake.test.ts +++ b/test/integration/mongodb-handshake/mongodb-handshake.test.ts @@ -81,22 +81,4 @@ describe('MongoDB Handshake', () => { } }); }); - - context('when not load-balanced', function () { - let spy: Sinon.SinonSpy; - before(() => { - spy = sinon.spy(Msg.prototype, 'makeDocumentSegment'); - }); - - after(() => sinon.restore()); - - it('should send the legacy hello command as OP_QUERY', { - metadata: { requires: { topology: '!load-balanced', mongodb: '<6.0.x' } }, - test: async function () { - client = this.configuration.newClient(); - await client.connect(); - expect(spy.called).to.be.false; - } - }); - }); }); From 25c86eeb23e5cbdd0fb90833628ca3632736ff93 Mon Sep 17 00:00:00 2001 From: Alena Khineika Date: Fri, 3 Nov 2023 14:30:39 +0100 Subject: [PATCH 11/14] test: use unit tests to check instance of the first message --- test/unit/cmap/connection.test.ts | 105 ++++++++++++++++++++++++++++++ 1 file changed, 105 insertions(+) diff --git a/test/unit/cmap/connection.test.ts b/test/unit/cmap/connection.test.ts index 42c4dfae9d..2047d0487e 100644 --- a/test/unit/cmap/connection.test.ts +++ b/test/unit/cmap/connection.test.ts @@ -1027,4 +1027,109 @@ 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] instanceof Msg).to.equal(true); + }); + }); + + 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] instanceof Query).to.equal(true); + }); + }); }); From 9644c4df7653362ce0d7eab299a60b267996ba41 Mon Sep 17 00:00:00 2001 From: Alena Khineika Date: Fri, 3 Nov 2023 14:31:49 +0100 Subject: [PATCH 12/14] test: remove ex test --- .../mongodb-handshake.test.ts | 21 +------------------ 1 file changed, 1 insertion(+), 20 deletions(-) diff --git a/test/integration/mongodb-handshake/mongodb-handshake.test.ts b/test/integration/mongodb-handshake/mongodb-handshake.test.ts index b0d4521749..2e00d0e2e9 100644 --- a/test/integration/mongodb-handshake/mongodb-handshake.test.ts +++ b/test/integration/mongodb-handshake/mongodb-handshake.test.ts @@ -6,8 +6,7 @@ import { Connection, LEGACY_HELLO_COMMAND, MongoServerError, - MongoServerSelectionError, - Msg + MongoServerSelectionError } from '../../mongodb'; describe('MongoDB Handshake', () => { @@ -63,22 +62,4 @@ describe('MongoDB Handshake', () => { expect(handshakeDoc).to.have.property('compression').to.deep.equal(['snappy']); }); }); - - context('when load-balanced', function () { - let spy: Sinon.SinonSpy; - before(() => { - spy = sinon.spy(Msg.prototype, 'makeDocumentSegment'); - }); - - after(() => 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(spy.called).to.be.true; - } - }); - }); }); From 8a0ce07bab91cf7d1cf7b26f61e99ffc0cf7a80c Mon Sep 17 00:00:00 2001 From: Alena Khineika Date: Fri, 3 Nov 2023 15:08:42 +0100 Subject: [PATCH 13/14] refactor: import class --- test/unit/cmap/connection.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unit/cmap/connection.test.ts b/test/unit/cmap/connection.test.ts index 2047d0487e..7c0aa44601 100644 --- a/test/unit/cmap/connection.test.ts +++ b/test/unit/cmap/connection.test.ts @@ -14,7 +14,7 @@ import { hasSessionSupport, type HostAddress, isHello, - type MessageStream, + MessageStream, MongoNetworkError, MongoNetworkTimeoutError, MongoRuntimeError, From 9e7b9ed481537819c24b052d212fae59eafe6ac2 Mon Sep 17 00:00:00 2001 From: Alena Khineika Date: Fri, 3 Nov 2023 15:25:33 +0100 Subject: [PATCH 14/14] refactor: import more classes --- test/unit/cmap/connection.test.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/unit/cmap/connection.test.ts b/test/unit/cmap/connection.test.ts index 7c0aa44601..1a8ada7af6 100644 --- a/test/unit/cmap/connection.test.ts +++ b/test/unit/cmap/connection.test.ts @@ -18,8 +18,10 @@ import { MongoNetworkError, MongoNetworkTimeoutError, MongoRuntimeError, + Msg, ns, - type OperationDescription + type OperationDescription, + Query } from '../../mongodb'; import * as mock from '../../tools/mongodb-mock/index'; import { generateOpMsgBuffer, getSymbolFrom } from '../../tools/utils';