Skip to content

Commit fd58eec

Browse files
feat(NODE-3881): require hello command + OP_MSG when 'loadBalanced=True' (#3907)
Co-authored-by: Durran Jordan <durran@gmail.com>
1 parent bb5fa43 commit fd58eec

File tree

3 files changed

+157
-24
lines changed

3 files changed

+157
-24
lines changed

src/cmap/connect.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -214,7 +214,7 @@ export async function prepareHandshakeDocument(
214214
const { serverApi } = authContext.connection;
215215

216216
const handshakeDoc: HandshakeDocument = {
217-
[serverApi?.version ? 'hello' : LEGACY_HELLO_COMMAND]: 1,
217+
[serverApi?.version || options.loadBalanced === true ? 'hello' : LEGACY_HELLO_COMMAND]: 1,
218218
helloOk: true,
219219
client: options.metadata,
220220
compression: compressors

test/unit/cmap/connect.test.ts

Lines changed: 47 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -234,42 +234,68 @@ describe('Connect Tests', function () {
234234

235235
context('loadBalanced option', () => {
236236
context('when loadBalanced is not set as an option', () => {
237+
const authContext = {
238+
connection: {},
239+
options: {}
240+
};
241+
237242
it('does not set loadBalanced on the handshake document', async () => {
238-
const options = {};
239-
const authContext = {
240-
connection: {},
241-
options
242-
};
243243
const handshakeDocument = await prepareHandshakeDocument(authContext);
244244
expect(handshakeDocument).not.to.have.property('loadBalanced');
245245
});
246+
247+
it('does not set hello on the handshake document', async () => {
248+
const handshakeDocument = await prepareHandshakeDocument(authContext);
249+
expect(handshakeDocument).not.to.have.property('hello');
250+
});
251+
252+
it('sets LEGACY_HELLO_COMMAND on the handshake document', async () => {
253+
const handshakeDocument = await prepareHandshakeDocument(authContext);
254+
expect(handshakeDocument).to.have.property(LEGACY_HELLO_COMMAND, 1);
255+
});
246256
});
247257

248258
context('when loadBalanced is set to false', () => {
259+
const authContext = {
260+
connection: {},
261+
options: { loadBalanced: false }
262+
};
263+
249264
it('does not set loadBalanced on the handshake document', async () => {
250-
const options = {
251-
loadBalanced: false
252-
};
253-
const authContext = {
254-
connection: {},
255-
options
256-
};
257265
const handshakeDocument = await prepareHandshakeDocument(authContext);
258266
expect(handshakeDocument).not.to.have.property('loadBalanced');
259267
});
268+
269+
it('does not set hello on the handshake document', async () => {
270+
const handshakeDocument = await prepareHandshakeDocument(authContext);
271+
expect(handshakeDocument).not.to.have.property('hello');
272+
});
273+
274+
it('sets LEGACY_HELLO_COMMAND on the handshake document', async () => {
275+
const handshakeDocument = await prepareHandshakeDocument(authContext);
276+
expect(handshakeDocument).to.have.property(LEGACY_HELLO_COMMAND, 1);
277+
});
260278
});
261279

262280
context('when loadBalanced is set to true', () => {
263-
it('does set loadBalanced on the handshake document', async () => {
264-
const options = {
265-
loadBalanced: true
266-
};
267-
const authContext = {
268-
connection: {},
269-
options
270-
};
281+
const authContext = {
282+
connection: {},
283+
options: { loadBalanced: true }
284+
};
285+
286+
it('sets loadBalanced on the handshake document', async () => {
287+
const handshakeDocument = await prepareHandshakeDocument(authContext);
288+
expect(handshakeDocument).to.have.property('loadBalanced');
289+
});
290+
291+
it('sets hello on the handshake document', async () => {
292+
const handshakeDocument = await prepareHandshakeDocument(authContext);
293+
expect(handshakeDocument).to.have.property('hello');
294+
});
295+
296+
it('does not set LEGACY_HELLO_COMMAND on the handshake document', async () => {
271297
const handshakeDocument = await prepareHandshakeDocument(authContext);
272-
expect(handshakeDocument).to.have.property('loadBalanced', true);
298+
expect(handshakeDocument).not.have.property(LEGACY_HELLO_COMMAND, 1);
273299
});
274300
});
275301
});

test/unit/cmap/connection.test.ts

Lines changed: 109 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,14 @@ import {
1414
hasSessionSupport,
1515
type HostAddress,
1616
isHello,
17-
type MessageStream,
17+
MessageStream,
1818
MongoNetworkError,
1919
MongoNetworkTimeoutError,
2020
MongoRuntimeError,
21+
Msg,
2122
ns,
22-
type OperationDescription
23+
type OperationDescription,
24+
Query
2325
} from '../../mongodb';
2426
import * as mock from '../../tools/mongodb-mock/index';
2527
import { generateOpMsgBuffer, getSymbolFrom } from '../../tools/utils';
@@ -1027,4 +1029,109 @@ describe('new Connection()', function () {
10271029
});
10281030
});
10291031
});
1032+
1033+
describe('when load-balanced', () => {
1034+
const CONNECT_DEFAULTS = {
1035+
id: 1,
1036+
tls: false,
1037+
generation: 1,
1038+
monitorCommands: false,
1039+
metadata: {} as ClientMetadata
1040+
};
1041+
let server;
1042+
let connectOptions;
1043+
let connection: Connection;
1044+
let writeCommandSpy;
1045+
1046+
beforeEach(async () => {
1047+
server = await mock.createServer();
1048+
server.setMessageHandler(request => {
1049+
request.reply(mock.HELLO);
1050+
});
1051+
writeCommandSpy = sinon.spy(MessageStream.prototype, 'writeCommand');
1052+
});
1053+
1054+
afterEach(async () => {
1055+
connection?.destroy({ force: true });
1056+
sinon.restore();
1057+
await mock.cleanup();
1058+
});
1059+
1060+
it('sends the first command as OP_MSG', async () => {
1061+
try {
1062+
connectOptions = {
1063+
...CONNECT_DEFAULTS,
1064+
hostAddress: server.hostAddress() as HostAddress,
1065+
socketTimeoutMS: 100,
1066+
loadBalanced: true
1067+
};
1068+
1069+
connection = await promisify<Connection>(callback =>
1070+
//@ts-expect-error: Callbacks do not have mutual exclusion for error/result existence
1071+
connect(connectOptions, callback)
1072+
)();
1073+
1074+
await promisify(callback =>
1075+
connection.command(ns('admin.$cmd'), { hello: 1 }, {}, callback)
1076+
)();
1077+
} catch (error) {
1078+
/** Connection timeouts, but the handshake message is sent. */
1079+
}
1080+
1081+
expect(writeCommandSpy).to.have.been.called;
1082+
expect(writeCommandSpy.firstCall.args[0] instanceof Msg).to.equal(true);
1083+
});
1084+
});
1085+
1086+
describe('when not load-balanced', () => {
1087+
const CONNECT_DEFAULTS = {
1088+
id: 1,
1089+
tls: false,
1090+
generation: 1,
1091+
monitorCommands: false,
1092+
metadata: {} as ClientMetadata
1093+
};
1094+
let server;
1095+
let connectOptions;
1096+
let connection: Connection;
1097+
let writeCommandSpy;
1098+
1099+
beforeEach(async () => {
1100+
server = await mock.createServer();
1101+
server.setMessageHandler(request => {
1102+
request.reply(mock.HELLO);
1103+
});
1104+
writeCommandSpy = sinon.spy(MessageStream.prototype, 'writeCommand');
1105+
});
1106+
1107+
afterEach(async () => {
1108+
connection?.destroy({ force: true });
1109+
sinon.restore();
1110+
await mock.cleanup();
1111+
});
1112+
1113+
it('sends the first command as OP_QUERY', async () => {
1114+
try {
1115+
connectOptions = {
1116+
...CONNECT_DEFAULTS,
1117+
hostAddress: server.hostAddress() as HostAddress,
1118+
socketTimeoutMS: 100
1119+
};
1120+
1121+
connection = await promisify<Connection>(callback =>
1122+
//@ts-expect-error: Callbacks do not have mutual exclusion for error/result existence
1123+
connect(connectOptions, callback)
1124+
)();
1125+
1126+
await promisify(callback =>
1127+
connection.command(ns('admin.$cmd'), { hello: 1 }, {}, callback)
1128+
)();
1129+
} catch (error) {
1130+
/** Connection timeouts, but the handshake message is sent. */
1131+
}
1132+
1133+
expect(writeCommandSpy).to.have.been.called;
1134+
expect(writeCommandSpy.firstCall.args[0] instanceof Query).to.equal(true);
1135+
});
1136+
});
10301137
});

0 commit comments

Comments
 (0)