From b5d0f0286046383dafce9ab4b2278e0a3978dcac Mon Sep 17 00:00:00 2001 From: Alena Khineika Date: Wed, 28 Feb 2024 20:31:13 +0100 Subject: [PATCH 01/21] fix(NODE-5817): read preference not applied to commands properly --- src/cmap/commands.ts | 6 - src/cmap/connection.ts | 35 ++- src/cmap/wire_protocol/shared.ts | 8 +- .../run-command/run_command.spec.test.ts | 7 +- .../server-selection/readpreference.test.js | 266 ++++++++++++++++++ test/tools/runner/config.ts | 10 +- 6 files changed, 300 insertions(+), 32 deletions(-) diff --git a/src/cmap/commands.ts b/src/cmap/commands.ts index 165e33a2f07..f67a4c9b956 100644 --- a/src/cmap/commands.ts +++ b/src/cmap/commands.ts @@ -51,7 +51,6 @@ export interface OpQueryOptions extends CommandOptions { requestId?: number; moreToCome?: boolean; exhaustAllowed?: boolean; - readPreference?: ReadPreference; } /************************************************************** @@ -77,7 +76,6 @@ export class OpQueryRequest { awaitData: boolean; exhaust: boolean; partial: boolean; - documentsReturnedIn?: string; constructor(public databaseName: string, public query: Document, options: OpQueryOptions) { // Basic options needed to be passed in @@ -503,10 +501,6 @@ export class OpMsgRequest { // Basic options this.command.$db = databaseName; - if (options.readPreference && options.readPreference.mode !== ReadPreference.PRIMARY) { - this.command.$readPreference = options.readPreference.toJSON(); - } - // Ensure empty options this.options = options ?? {}; diff --git a/src/cmap/connection.ts b/src/cmap/connection.ts index e33b4f835f6..53a07677864 100644 --- a/src/cmap/connection.ts +++ b/src/cmap/connection.ts @@ -27,7 +27,8 @@ import type { ServerApi, SupportedNodeConnectionOptions } from '../mongo_client' import { type MongoClientAuthProviders } from '../mongo_client_auth_providers'; import { MongoLoggableComponent, type MongoLogger, SeverityLevel } from '../mongo_logger'; import { type CancellationToken, TypedEventEmitter } from '../mongo_types'; -import type { ReadPreferenceLike } from '../read_preference'; +import { ReadPreference, type ReadPreferenceLike } from '../read_preference'; +import { ServerType } from '../sdam/common'; import { applySession, type ClientSession, updateSessionFromResponse } from '../sessions'; import { BufferPool, @@ -363,6 +364,7 @@ export class Connection extends TypedEventEmitter { let cmd = { ...command }; const readPreference = getReadPreference(options); + const session = options?.session; let clusterTime = this.clusterTime; @@ -394,16 +396,26 @@ export class Connection extends TypedEventEmitter { cmd.$clusterTime = clusterTime; } - if ( - isSharded(this) && - !this.supportsOpMsg && - readPreference && - readPreference.mode !== 'primary' + // For mongos and load balancers in 'primary' mode, drivers MUST NOT set $readPreference. + if ((isSharded(this) || this.description.loadBalanced) && readPreference?.mode !== 'primary') { + // When sending a read operation via OP_QUERY and any $readPreference modifie is used, + // the query MUST be provided using the $query modifier. + cmd = this.supportsOpMsg + ? { ...cmd, $readPreference: readPreference.toJSON() } + : { + $query: cmd, + $readPreference: readPreference.toJSON() + }; + } else if ( + this.supportsOpMsg && + this.description.type !== ServerType.Standalone && + options.session?.clientOptions?.directConnection === true ) { - cmd = { - $query: cmd, - $readPreference: readPreference.toJSON() - }; + // For standalone, drivers MUST NOT set $readPreference. + cmd.$readPreference = + readPreference?.mode === 'primary' + ? ReadPreference.primaryPreferred.toJSON() + : readPreference.toJSON(); } const commandOptions = { @@ -412,8 +424,7 @@ export class Connection extends TypedEventEmitter { checkKeys: false, // This value is not overridable secondaryOk: readPreference.secondaryOk(), - ...options, - readPreference // ensure we pass in ReadPreference instance + ...options }; const message = this.supportsOpMsg diff --git a/src/cmap/wire_protocol/shared.ts b/src/cmap/wire_protocol/shared.ts index 53375d8ec57..98e0149001d 100644 --- a/src/cmap/wire_protocol/shared.ts +++ b/src/cmap/wire_protocol/shared.ts @@ -13,12 +13,8 @@ export interface ReadPreferenceOption { } export function getReadPreference(options?: ReadPreferenceOption): ReadPreference { - // Default to command version of the readPreference + // Default to command version of the readPreference. let readPreference = options?.readPreference ?? ReadPreference.primary; - // If we have an option readPreference override the command one - if (options?.readPreference) { - readPreference = options.readPreference; - } if (typeof readPreference === 'string') { readPreference = ReadPreference.fromString(readPreference); @@ -43,7 +39,7 @@ export function isSharded(topologyOrServer?: Topology | Server | Connection): bo } // NOTE: This is incredibly inefficient, and should be removed once command construction - // happens based on `Server` not `Topology`. + // happens based on `Server` not `Topology`. if (topologyOrServer.description && topologyOrServer.description instanceof TopologyDescription) { const servers: ServerDescription[] = Array.from(topologyOrServer.description.servers.values()); return servers.some((server: ServerDescription) => server.type === ServerType.Mongos); diff --git a/test/integration/run-command/run_command.spec.test.ts b/test/integration/run-command/run_command.spec.test.ts index a4a8a522d5e..c2ca5e91b5f 100644 --- a/test/integration/run-command/run_command.spec.test.ts +++ b/test/integration/run-command/run_command.spec.test.ts @@ -2,10 +2,5 @@ import { loadSpecTests } from '../../spec'; import { runUnifiedSuite } from '../../tools/unified-spec-runner/runner'; describe('RunCommand spec', () => { - runUnifiedSuite(loadSpecTests('run-command'), test => { - if (test.description === 'does not attach $readPreference to given command on standalone') { - return 'TODO(NODE-5263): Do not send $readPreference to standalone servers'; - } - return false; - }); + runUnifiedSuite(loadSpecTests('run-command')); }); diff --git a/test/integration/server-selection/readpreference.test.js b/test/integration/server-selection/readpreference.test.js index ba1e8602a84..d8f5878f45c 100644 --- a/test/integration/server-selection/readpreference.test.js +++ b/test/integration/server-selection/readpreference.test.js @@ -463,4 +463,270 @@ describe('ReadPreference', function () { await client.close(); } }); + + context('when connecting to a secondary in a replica set with a direct connection', () => { + context('when readPreference is primary', () => { + it('should attach a read preference of primaryPreferred to the read command for replicaset', { + metadata: { requires: { topology: 'replicaset', mongodb: '>=3.6' } }, + test: async function () { + for (const server of this.configuration.options.hostAddresses) { + const { host, port } = server.toHostPort(); + const client = this.configuration.newClient( + { + readPreference: 'primary', + directConnection: true, + host, + port + }, + { + monitorCommands: true + } + ); + const events = []; + client.on('commandStarted', event => { + if (event.commandName === 'find') { + events.push(event); + } + }); + let serverError; + try { + await client.db('test').collection('test').findOne({ a: 1 }); + } catch (error) { + serverError = error; + } + + expect(serverError).to.not.exist; + expect(events[0]).to.containSubset({ + commandName: 'find', + command: { + $readPreference: { mode: 'primaryPreferred' } + } + }); + + await client.close(); + } + } + }); + + it('should not attach a read preference to the read command for loadbalanced', { + metadata: { requires: { topology: 'load-balanced', mongodb: '>=3.6' } }, + test: async function () { + for (const server of this.configuration.options.hostAddresses) { + const { host, port } = server.toHostPort(); + const client = this.configuration.newClient( + { + readPreference: 'primary', + directConnection: true, + host, + port + }, + { + monitorCommands: true + } + ); + const events = []; + client.on('commandStarted', event => { + if (event.commandName === 'find') { + events.push(event); + } + }); + let serverError; + try { + await client.db('test').collection('test').findOne({ a: 1 }); + } catch (error) { + serverError = error; + } + + expect(serverError).to.not.exist; + expect(events[0]).to.not.containSubset({ + commandName: 'find', + command: { + $readPreference: { mode: 'primaryPreferred' } + } + }); + expect(events[0]).to.not.containSubset({ + commandName: 'find', + command: { + $readPreference: { mode: 'primary' } + } + }); + + await client.close(); + } + } + }); + + it('should not attach a read preference to the read command for standalone', { + metadata: { requires: { topology: 'single', mongodb: '>=3.6' } }, + test: async function () { + const client = this.configuration.newClient( + { + readPreference: 'primary', + directConnection: true + }, + { + monitorCommands: true + } + ); + const events = []; + client.on('commandStarted', event => { + if (event.commandName === 'find') { + events.push(event); + } + }); + let serverError; + try { + await client.db('test').collection('test').findOne({ a: 1 }); + } catch (error) { + serverError = error; + } + + expect(serverError).to.not.exist; + expect(events[0]).to.not.containSubset({ + commandName: 'find', + command: { + $readPreference: { mode: 'primaryPreferred' } + } + }); + expect(events[0]).to.not.containSubset({ + commandName: 'find', + command: { + $readPreference: { mode: 'primary' } + } + }); + + await client.close(); + } + }); + }); + + context('when readPreference is secondary', () => { + it('should attach a read preference of secondary to the read command for replicaset', { + metadata: { requires: { topology: 'replicaset', mongodb: '>=3.6' } }, + test: async function () { + for (const server of this.configuration.options.hostAddresses) { + const { host, port } = server.toHostPort(); + const client = this.configuration.newClient( + { + readPreference: 'secondary', + directConnection: true, + host, + port + }, + { + monitorCommands: true + } + ); + const events = []; + client.on('commandStarted', event => { + if (event.commandName === 'find') { + events.push(event); + } + }); + let serverError; + try { + await client.db('test').collection('test').findOne({ a: 1 }); + } catch (error) { + serverError = error; + } + + expect(serverError).to.not.exist; + expect(events[0]).to.containSubset({ + commandName: 'find', + command: { + $readPreference: { mode: 'secondary' } + } + }); + + await client.close(); + } + } + }); + + it('should attach a read preference of secondary to the read command for loadbalanced', { + metadata: { requires: { topology: 'load-balanced', mongodb: '>=3.6' } }, + test: async function () { + for (const server of this.configuration.options.hostAddresses) { + const { host, port } = server.toHostPort(); + const client = this.configuration.newClient( + { + readPreference: 'secondary', + directConnection: true, + host, + port + }, + { + monitorCommands: true + } + ); + const events = []; + client.on('commandStarted', event => { + if (event.commandName === 'find') { + events.push(event); + } + }); + let serverError; + try { + await client.db('test').collection('test').findOne({ a: 1 }); + } catch (error) { + serverError = error; + } + + expect(serverError).to.not.exist; + expect(events[0]).to.not.containSubset({ + commandName: 'find', + command: { + $readPreference: { mode: 'primaryPreferred' } + } + }); + expect(events[0]).to.not.containSubset({ + commandName: 'find', + command: { + $readPreference: { mode: 'primary' } + } + }); + + await client.close(); + } + } + }); + + it('should not attach a read preference to the read command for standalone', { + metadata: { requires: { topology: 'single', mongodb: '>=3.6' } }, + test: async function () { + const client = this.configuration.newClient( + { + readPreference: 'secondary', + directConnection: true + }, + { + monitorCommands: true + } + ); + const events = []; + client.on('commandStarted', event => { + if (event.commandName === 'find') { + events.push(event); + } + }); + let serverError; + try { + await client.db('test').collection('test').findOne({ a: 1 }); + } catch (error) { + serverError = error; + } + + expect(serverError).to.not.exist; + expect(events[0]).to.not.containSubset({ + commandName: 'find', + command: { + $readPreference: { mode: 'secondary' } + } + }); + + await client.close(); + } + }); + }); + }); }); diff --git a/test/tools/runner/config.ts b/test/tools/runner/config.ts index e4f3dd52a3f..6a43ae6b297 100644 --- a/test/tools/runner/config.ts +++ b/test/tools/runner/config.ts @@ -39,6 +39,10 @@ interface UrlOptions { useMultipleMongoses?: boolean; /** Parameters for configuring a proxy connection */ proxyURIParams?: ProxyParams; + /** Host to overwrite that is provided in url */ + host?: string; + /** Port to overwrite that is provided in url */ + port?: number; } function convertToConnStringMap(obj: Record) { @@ -173,11 +177,13 @@ export class TestConfiguration { const queryOptions = urlOrQueryOptions || {}; // Fall back. - let dbHost = serverOptions.host || this.options.host; + let dbHost = queryOptions.host || this.options.host; if (dbHost.indexOf('.sock') !== -1) { dbHost = qs.escape(dbHost); } - const dbPort = serverOptions.port || this.options.port; + delete queryOptions.host; + const dbPort = queryOptions.port || this.options.port; + delete queryOptions.port; if (this.options.authMechanism && !serverOptions.authMechanism) { Object.assign(queryOptions, { From 1e4c377315cde48a69f96d00ac55ee47bb02fa08 Mon Sep 17 00:00:00 2001 From: Alena Khineika Date: Wed, 28 Feb 2024 20:32:02 +0100 Subject: [PATCH 02/21] test: remove a test case --- .../server-selection/readpreference.test.js | 96 ------------------- 1 file changed, 96 deletions(-) diff --git a/test/integration/server-selection/readpreference.test.js b/test/integration/server-selection/readpreference.test.js index d8f5878f45c..a0a0864fd72 100644 --- a/test/integration/server-selection/readpreference.test.js +++ b/test/integration/server-selection/readpreference.test.js @@ -508,54 +508,6 @@ describe('ReadPreference', function () { } }); - it('should not attach a read preference to the read command for loadbalanced', { - metadata: { requires: { topology: 'load-balanced', mongodb: '>=3.6' } }, - test: async function () { - for (const server of this.configuration.options.hostAddresses) { - const { host, port } = server.toHostPort(); - const client = this.configuration.newClient( - { - readPreference: 'primary', - directConnection: true, - host, - port - }, - { - monitorCommands: true - } - ); - const events = []; - client.on('commandStarted', event => { - if (event.commandName === 'find') { - events.push(event); - } - }); - let serverError; - try { - await client.db('test').collection('test').findOne({ a: 1 }); - } catch (error) { - serverError = error; - } - - expect(serverError).to.not.exist; - expect(events[0]).to.not.containSubset({ - commandName: 'find', - command: { - $readPreference: { mode: 'primaryPreferred' } - } - }); - expect(events[0]).to.not.containSubset({ - commandName: 'find', - command: { - $readPreference: { mode: 'primary' } - } - }); - - await client.close(); - } - } - }); - it('should not attach a read preference to the read command for standalone', { metadata: { requires: { topology: 'single', mongodb: '>=3.6' } }, test: async function () { @@ -643,54 +595,6 @@ describe('ReadPreference', function () { } }); - it('should attach a read preference of secondary to the read command for loadbalanced', { - metadata: { requires: { topology: 'load-balanced', mongodb: '>=3.6' } }, - test: async function () { - for (const server of this.configuration.options.hostAddresses) { - const { host, port } = server.toHostPort(); - const client = this.configuration.newClient( - { - readPreference: 'secondary', - directConnection: true, - host, - port - }, - { - monitorCommands: true - } - ); - const events = []; - client.on('commandStarted', event => { - if (event.commandName === 'find') { - events.push(event); - } - }); - let serverError; - try { - await client.db('test').collection('test').findOne({ a: 1 }); - } catch (error) { - serverError = error; - } - - expect(serverError).to.not.exist; - expect(events[0]).to.not.containSubset({ - commandName: 'find', - command: { - $readPreference: { mode: 'primaryPreferred' } - } - }); - expect(events[0]).to.not.containSubset({ - commandName: 'find', - command: { - $readPreference: { mode: 'primary' } - } - }); - - await client.close(); - } - } - }); - it('should not attach a read preference to the read command for standalone', { metadata: { requires: { topology: 'single', mongodb: '>=3.6' } }, test: async function () { From 541b5d5c3b67b711fc9304fa12a6c18939a46613 Mon Sep 17 00:00:00 2001 From: Alena Khineika Date: Wed, 28 Feb 2024 20:32:30 +0100 Subject: [PATCH 03/21] docs: update comments --- test/tools/runner/config.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/tools/runner/config.ts b/test/tools/runner/config.ts index 6a43ae6b297..258bbc776c8 100644 --- a/test/tools/runner/config.ts +++ b/test/tools/runner/config.ts @@ -39,9 +39,9 @@ interface UrlOptions { useMultipleMongoses?: boolean; /** Parameters for configuring a proxy connection */ proxyURIParams?: ProxyParams; - /** Host to overwrite that is provided in url */ + /** Host overwriting the one provided in the url. */ host?: string; - /** Port to overwrite that is provided in url */ + /** Port overwriting the one provided in the url. */ port?: number; } From 3304317b224d803a348c59b54092c55a90f6748e Mon Sep 17 00:00:00 2001 From: Alena Khineika Date: Thu, 29 Feb 2024 16:15:50 +0100 Subject: [PATCH 04/21] feat: pass directConnection from connection pool --- src/cmap/connection.ts | 53 +++---- src/cmap/connection_pool.ts | 5 +- src/cmap/stream_description.ts | 5 +- src/mongo_client.ts | 1 + ...ference.test.js => readpreference.test.ts} | 132 +++++++++--------- 5 files changed, 101 insertions(+), 95 deletions(-) rename test/integration/server-selection/{readpreference.test.js => readpreference.test.ts} (84%) diff --git a/src/cmap/connection.ts b/src/cmap/connection.ts index 53a07677864..c390147b894 100644 --- a/src/cmap/connection.ts +++ b/src/cmap/connection.ts @@ -121,6 +121,7 @@ export interface ConnectionOptions metadata: ClientMetadata; /** @internal */ mongoLogger?: MongoLogger | undefined; + dierctConnection?: boolean; } /** @internal */ @@ -277,6 +278,10 @@ export class Connection extends TypedEventEmitter { return this.description.loadBalanced; } + public get directConnection(): boolean { + return this.description.directConnection; + } + public get idleTime(): number { return calculateDurationInMs(this.lastUseTime); } @@ -361,10 +366,8 @@ export class Connection extends TypedEventEmitter { } private prepareCommand(db: string, command: Document, options: CommandOptions) { - let cmd = { ...command }; - + const cmd = { ...command }; const readPreference = getReadPreference(options); - const session = options?.session; let clusterTime = this.clusterTime; @@ -391,31 +394,33 @@ export class Connection extends TypedEventEmitter { throw new MongoCompatibilityError('Current topology does not support sessions'); } - // if we have a known cluster time, gossip it + // if we have a known cluster time, gossip it. if (clusterTime) { cmd.$clusterTime = clusterTime; } - // For mongos and load balancers in 'primary' mode, drivers MUST NOT set $readPreference. - if ((isSharded(this) || this.description.loadBalanced) && readPreference?.mode !== 'primary') { - // When sending a read operation via OP_QUERY and any $readPreference modifie is used, - // the query MUST be provided using the $query modifier. - cmd = this.supportsOpMsg - ? { ...cmd, $readPreference: readPreference.toJSON() } - : { - $query: cmd, - $readPreference: readPreference.toJSON() - }; - } else if ( - this.supportsOpMsg && - this.description.type !== ServerType.Standalone && - options.session?.clientOptions?.directConnection === true - ) { - // For standalone, drivers MUST NOT set $readPreference. - cmd.$readPreference = - readPreference?.mode === 'primary' - ? ReadPreference.primaryPreferred.toJSON() - : readPreference.toJSON(); + // Driver does not use legacy hello for anything other than the first handshake + // (we do not support servers that only speak OP_QUERY), + // therefore $readPreference is not relevant for servers that do not support OP_MSG. + + // For standalone, drivers MUST NOT set $readPreference. + if (this.supportsOpMsg && this.description.type !== ServerType.Standalone) { + // For mongos and load balancers with a direct connection and 'primary' mode, + // drivers MUST NOT set $readPreference. + if (isSharded(this) || this.description.loadBalanced) { + if (this.description.directConnection !== true || readPreference?.mode !== 'primary') { + cmd.$readPreference = readPreference.toJSON(); + } + } else { + // For all other types with a direct connection, if the read preference is 'primary' + // (driver sets 'primary' as default if no read preference is configured), + // the $readPreference MUST be set to 'primaryPreferred' + // to ensure that any server type can handle the request. + cmd.$readPreference = + this.description.directConnection !== true || readPreference?.mode !== 'primary' + ? readPreference.toJSON() + : ReadPreference.primaryPreferred.toJSON(); + } } const commandOptions = { diff --git a/src/cmap/connection_pool.ts b/src/cmap/connection_pool.ts index 435b66936d5..4c046abb202 100644 --- a/src/cmap/connection_pool.ts +++ b/src/cmap/connection_pool.ts @@ -100,6 +100,8 @@ export interface ConnectionPoolOptions extends Omit { generation: this[kGeneration], cancellationToken: this[kCancellationToken], mongoLogger: this.mongoLogger, - authProviders: this[kServer].topology.client.s.authProviders + authProviders: this[kServer].topology.client.s.authProviders, + dierctConnection: this[kServer].topology.client.options.directConnection === true }; this[kPending]++; diff --git a/src/cmap/stream_description.ts b/src/cmap/stream_description.ts index 85decc2a091..43cb2c9af91 100644 --- a/src/cmap/stream_description.ts +++ b/src/cmap/stream_description.ts @@ -17,12 +17,13 @@ export interface StreamDescriptionOptions { compressors?: CompressorName[]; logicalSessionTimeoutMinutes?: number; loadBalanced: boolean; + directConnection: boolean; } /** @public */ export class StreamDescription { address: string; - type: string; + type: ServerType; minWireVersion?: number; maxWireVersion?: number; maxBsonObjectSize: number; @@ -32,6 +33,7 @@ export class StreamDescription { compressor?: CompressorName; logicalSessionTimeoutMinutes?: number; loadBalanced: boolean; + directConnection: boolean; __nodejs_mock_server__?: boolean; @@ -55,6 +57,7 @@ export class StreamDescription { ? options.compressors : []; this.serverConnectionId = null; + this.directConnection = !!options?.directConnection; } receiveResponse(response: Document | null): void { diff --git a/src/mongo_client.ts b/src/mongo_client.ts index be039944a4f..0cdb0186b88 100644 --- a/src/mongo_client.ts +++ b/src/mongo_client.ts @@ -821,6 +821,7 @@ export interface MongoOptions readPreference: ReadPreference; readConcern: ReadConcern; loadBalanced: boolean; + directConnection: boolean; serverApi: ServerApi; compressors: CompressorName[]; writeConcern: WriteConcern; diff --git a/test/integration/server-selection/readpreference.test.js b/test/integration/server-selection/readpreference.test.ts similarity index 84% rename from test/integration/server-selection/readpreference.test.js rename to test/integration/server-selection/readpreference.test.ts index a0a0864fd72..00d02aa9842 100644 --- a/test/integration/server-selection/readpreference.test.js +++ b/test/integration/server-selection/readpreference.test.ts @@ -1,16 +1,12 @@ -'use strict'; +import { expect } from 'chai'; -const { ReadPreference } = require('../../mongodb'); -const { Topology } = require('../../mongodb'); -const chai = require('chai'); -chai.use(require('chai-subset')); +import { ReadPreference, Topology } from '../../mongodb'; +import { assert as test, setupDatabase } from '../shared'; -const { assert: test, setupDatabase } = require('../shared'); - -const expect = chai.expect; - -describe('ReadPreference', function () { +describe.only('ReadPreference', function () { let client; + let events; + beforeEach(async function () { client = this.configuration.newClient({ monitorCommands: true }); }); @@ -27,20 +23,19 @@ describe('ReadPreference', function () { metadata: { requires: { mongodb: '>=2.6.0', topology: ['single', 'ssl'] } }, test: function (done) { - var configuration = this.configuration; - var client = configuration.newClient(configuration.writeConcernMax(), { maxPoolSize: 1 }); + const configuration = this.configuration; + const client = configuration.newClient(configuration.writeConcernMax(), { maxPoolSize: 1 }); client.connect(function (err, client) { - var db = client.db(configuration.db); + const db = client.db(configuration.db); expect(err).to.not.exist; // Set read preference - var collection = db.collection('read_pref_1', { + const collection = db.collection('read_pref_1', { readPreference: ReadPreference.SECONDARY_PREFERRED }); // Save checkout function - var command = client.topology.command; + const command = client.topology.command; // Set up our checker method - client.topology.command = function () { - var args = Array.prototype.slice.call(arguments, 0); + client.topology.command = function (...args) { if (args[0] === 'integration_tests.$cmd') { test.equal(ReadPreference.SECONDARY_PREFERRED, args[2].readPreference.mode); } @@ -63,20 +58,19 @@ describe('ReadPreference', function () { metadata: { requires: { mongodb: '>=2.6.0', topology: ['single', 'ssl'] } }, test: function (done) { - var configuration = this.configuration; - var client = configuration.newClient(configuration.writeConcernMax(), { maxPoolSize: 1 }); + const configuration = this.configuration; + const client = configuration.newClient(configuration.writeConcernMax(), { maxPoolSize: 1 }); client.connect(function (err, client) { - var db = client.db(configuration.db); + const db = client.db(configuration.db); expect(err).to.not.exist; // Set read preference - var collection = db.collection('read_pref_1', { + const collection = db.collection('read_pref_1', { readPreference: ReadPreference.SECONDARY_PREFERRED }); // Save checkout function - var command = client.topology.command; + const command = client.topology.command; // Set up our checker method - client.topology.command = function () { - var args = Array.prototype.slice.call(arguments, 0); + client.topology.command = function (...args) { if (args[0] === 'integration_tests.$cmd') { test.equal(ReadPreference.SECONDARY_PREFERRED, args[2].readPreference.mode); } @@ -114,18 +108,17 @@ describe('ReadPreference', function () { metadata: { requires: { mongodb: '>=2.6.0', topology: ['single', 'ssl'] } }, test: function (done) { - var configuration = this.configuration; - var client = configuration.newClient( + const configuration = this.configuration; + const client = configuration.newClient( { w: 1, readPreference: 'secondary' }, { maxPoolSize: 1 } ); client.connect(function (err, client) { - var db = client.db(configuration.db); + const db = client.db(configuration.db); // Save checkout function - var command = client.topology.command; + const command = client.topology.command; // Set up our checker method - client.topology.command = function () { - var args = Array.prototype.slice.call(arguments, 0); + client.topology.command = function (...args) { if (args[0] === 'integration_tests.$cmd') { test.equal(ReadPreference.SECONDARY, args[2].readPreference.mode); } @@ -136,8 +129,7 @@ describe('ReadPreference', function () { db.command({ dbStats: true }, function (err) { expect(err).to.not.exist; - client.topology.command = function () { - var args = Array.prototype.slice.call(arguments, 0); + client.topology.command = function (...args) { if (args[0] === 'integration_tests.$cmd') { test.equal(ReadPreference.SECONDARY_PREFERRED, args[2].readPreference.mode); } @@ -159,13 +151,13 @@ describe('ReadPreference', function () { metadata: { requires: { mongodb: '>=2.6.0', topology: ['single', 'ssl'] } }, test: function (done) { - var configuration = this.configuration; - var client = configuration.newClient(configuration.writeConcernMax(), { maxPoolSize: 1 }); + const configuration = this.configuration; + const client = configuration.newClient(configuration.writeConcernMax(), { maxPoolSize: 1 }); client.connect(function (err, client) { - var db = client.db(configuration.db); + const db = client.db(configuration.db); expect(err).to.not.exist; // Create read preference object. - var mySecondaryPreferred = { mode: 'secondaryPreferred', tags: [] }; + const mySecondaryPreferred = { mode: 'secondaryPreferred', tags: [] }; db.command({ dbStats: true }, { readPreference: mySecondaryPreferred }, function (err) { expect(err).to.not.exist; client.close(done); @@ -178,13 +170,13 @@ describe('ReadPreference', function () { metadata: { requires: { mongodb: '>=2.6.0', topology: ['single', 'ssl'] } }, test: function (done) { - var configuration = this.configuration; - var client = configuration.newClient(configuration.writeConcernMax(), { maxPoolSize: 1 }); + const configuration = this.configuration; + const client = configuration.newClient(configuration.writeConcernMax(), { maxPoolSize: 1 }); client.connect(function (err, client) { - var db = client.db(configuration.db); + const db = client.db(configuration.db); expect(err).to.not.exist; // Create read preference object. - var mySecondaryPreferred = { mode: 'secondaryPreferred', tags: [] }; + const mySecondaryPreferred = { mode: 'secondaryPreferred', tags: [] }; db.listCollections({}, { readPreference: mySecondaryPreferred }).toArray(function (err) { expect(err).to.not.exist; client.close(done); @@ -197,14 +189,14 @@ describe('ReadPreference', function () { metadata: { requires: { mongodb: '>=2.6.0', topology: ['single', 'ssl'] } }, test: function (done) { - var configuration = this.configuration; - var client = configuration.newClient(configuration.writeConcernMax(), { maxPoolSize: 1 }); + const configuration = this.configuration; + const client = configuration.newClient(configuration.writeConcernMax(), { maxPoolSize: 1 }); client.connect(function (err, client) { - var db = client.db(configuration.db); + const db = client.db(configuration.db); expect(err).to.not.exist; // Create read preference object. - var mySecondaryPreferred = { mode: 'secondaryPreferred', tags: [] }; - var cursor = db.collection('test').find({}, { readPreference: mySecondaryPreferred }); + const mySecondaryPreferred = { mode: 'secondaryPreferred', tags: [] }; + const cursor = db.collection('test').find({}, { readPreference: mySecondaryPreferred }); cursor.toArray(function (err) { expect(err).to.not.exist; client.close(done); @@ -217,12 +209,12 @@ describe('ReadPreference', function () { metadata: { requires: { mongodb: '>=2.6.0', topology: ['single', 'ssl'] } }, test: function (done) { - var configuration = this.configuration; - var client = configuration.newClient(configuration.writeConcernMax(), { maxPoolSize: 1 }); + const configuration = this.configuration; + const client = configuration.newClient(configuration.writeConcernMax(), { maxPoolSize: 1 }); client.connect(function (err, client) { - var db = client.db(configuration.db); + const db = client.db(configuration.db); expect(err).to.not.exist; - var cursor = db + const cursor = db .collection('test', { readPreference: ReadPreference.SECONDARY_PREFERRED }) .listIndexes(); test.equal(cursor.readPreference.mode, 'secondaryPreferred'); @@ -247,15 +239,15 @@ describe('ReadPreference', function () { context('hedge', function () { it('should set hedge using [find option & empty hedge]', { - metadata: { requires: { mongodb: '>=3.6.0' } }, + metadata: { requires: { topology: 'replicaset', mongodb: '>=3.6.0' } }, test: function (done) { - const events = []; + events = []; client.on('commandStarted', event => { if (event.commandName === 'find') { events.push(event); } }); - const rp = new ReadPreference(ReadPreference.SECONDARY, null, { hedge: {} }); + const rp = new ReadPreference(ReadPreference.SECONDARY, undefined, { hedge: {} }); client .db(this.configuration.db) .collection('test') @@ -270,15 +262,15 @@ describe('ReadPreference', function () { }); it('should set hedge using [.withReadPreference & empty hedge] ', { - metadata: { requires: { mongodb: '>=3.6.0' } }, + metadata: { requires: { topology: 'replicaset', mongodb: '>=3.6.0' } }, test: function (done) { - const events = []; + events = []; client.on('commandStarted', event => { if (event.commandName === 'find') { events.push(event); } }); - const rp = new ReadPreference(ReadPreference.SECONDARY, null, { hedge: {} }); + const rp = new ReadPreference(ReadPreference.SECONDARY, undefined, { hedge: {} }); client .db(this.configuration.db) .collection('test') @@ -294,15 +286,17 @@ describe('ReadPreference', function () { }); it('should set hedge using [.withReadPreference & enabled hedge] ', { - metadata: { requires: { mongodb: '>=3.6.0' } }, + metadata: { requires: { topology: 'replicaset', mongodb: '>=3.6.0' } }, test: function (done) { - const events = []; + events = []; client.on('commandStarted', event => { if (event.commandName === 'find') { events.push(event); } }); - const rp = new ReadPreference(ReadPreference.SECONDARY, null, { hedge: { enabled: true } }); + const rp = new ReadPreference(ReadPreference.SECONDARY, undefined, { + hedge: { enabled: true } + }); client .db(this.configuration.db) .collection('test') @@ -318,15 +312,15 @@ describe('ReadPreference', function () { }); it('should set hedge using [.withReadPreference & disabled hedge] ', { - metadata: { requires: { mongodb: '>=3.6.0' } }, + metadata: { requires: { topology: 'replicaset', mongodb: '>=3.6.0' } }, test: function (done) { - const events = []; + events = []; client.on('commandStarted', event => { if (event.commandName === 'find') { events.push(event); } }); - const rp = new ReadPreference(ReadPreference.SECONDARY, null, { + const rp = new ReadPreference(ReadPreference.SECONDARY, undefined, { hedge: { enabled: false } }); client @@ -344,15 +338,15 @@ describe('ReadPreference', function () { }); it('should set hedge using [.withReadPreference & undefined hedge] ', { - metadata: { requires: { mongodb: '>=3.6.0' } }, + metadata: { requires: { topology: 'replicaset', mongodb: '>=3.6.0' } }, test: function (done) { - const events = []; + events = []; client.on('commandStarted', event => { if (event.commandName === 'find') { events.push(event); } }); - const rp = new ReadPreference(ReadPreference.SECONDARY, null); + const rp = new ReadPreference(ReadPreference.SECONDARY); client .db(this.configuration.db) .collection('test') @@ -443,7 +437,7 @@ describe('ReadPreference', function () { readPreference: 'secondary', monitorCommands: true }); - const events = []; + events = []; client.on('commandStarted', event => { if (event.commandName === 'find') { events.push(event); @@ -482,7 +476,7 @@ describe('ReadPreference', function () { monitorCommands: true } ); - const events = []; + events = []; client.on('commandStarted', event => { if (event.commandName === 'find') { events.push(event); @@ -520,7 +514,7 @@ describe('ReadPreference', function () { monitorCommands: true } ); - const events = []; + events = []; client.on('commandStarted', event => { if (event.commandName === 'find') { events.push(event); @@ -569,7 +563,7 @@ describe('ReadPreference', function () { monitorCommands: true } ); - const events = []; + events = []; client.on('commandStarted', event => { if (event.commandName === 'find') { events.push(event); @@ -607,7 +601,7 @@ describe('ReadPreference', function () { monitorCommands: true } ); - const events = []; + events = []; client.on('commandStarted', event => { if (event.commandName === 'find') { events.push(event); From 8b875ef21c71905e3c98660eca04c39dbd004262 Mon Sep 17 00:00:00 2001 From: Alena Khineika Date: Thu, 29 Feb 2024 16:17:00 +0100 Subject: [PATCH 05/21] test: remove only --- test/integration/server-selection/readpreference.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/server-selection/readpreference.test.ts b/test/integration/server-selection/readpreference.test.ts index 00d02aa9842..8043759ef0a 100644 --- a/test/integration/server-selection/readpreference.test.ts +++ b/test/integration/server-selection/readpreference.test.ts @@ -3,7 +3,7 @@ import { expect } from 'chai'; import { ReadPreference, Topology } from '../../mongodb'; import { assert as test, setupDatabase } from '../shared'; -describe.only('ReadPreference', function () { +describe('ReadPreference', function () { let client; let events; From 2d033a3c11acfad3037dedee9af94cf53d46ebfd Mon Sep 17 00:00:00 2001 From: Alena Khineika Date: Fri, 1 Mar 2024 12:23:51 +0100 Subject: [PATCH 06/21] fix: import type --- src/cmap/commands.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cmap/commands.ts b/src/cmap/commands.ts index f67a4c9b956..57a605cf248 100644 --- a/src/cmap/commands.ts +++ b/src/cmap/commands.ts @@ -1,7 +1,7 @@ import type { BSONSerializeOptions, Document, Long } from '../bson'; import * as BSON from '../bson'; import { MongoInvalidArgumentError, MongoRuntimeError } from '../error'; -import { ReadPreference } from '../read_preference'; +import { type ReadPreference } from '../read_preference'; import type { ClientSession } from '../sessions'; import type { CommandOptions } from './connection'; import { From 325cb58a545f4a315c3e0e3919a6c15fd42acbbb Mon Sep 17 00:00:00 2001 From: Alena Khineika Date: Fri, 1 Mar 2024 13:37:38 +0100 Subject: [PATCH 07/21] fix: add options to unit tests --- test/unit/cmap/connection_pool.test.js | 3 ++- test/unit/sdam/monitor.test.ts | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/test/unit/cmap/connection_pool.test.js b/test/unit/cmap/connection_pool.test.js index b6e408e3d56..6aacf75d5f4 100644 --- a/test/unit/cmap/connection_pool.test.js +++ b/test/unit/cmap/connection_pool.test.js @@ -22,7 +22,8 @@ describe('Connection Pool', function () { }, s: { authProviders: new MongoClientAuthProviders() - } + }, + options: {} } } }; diff --git a/test/unit/sdam/monitor.test.ts b/test/unit/sdam/monitor.test.ts index c5eb826ddfa..325b411de24 100644 --- a/test/unit/sdam/monitor.test.ts +++ b/test/unit/sdam/monitor.test.ts @@ -35,7 +35,8 @@ class MockServer { debug: function (_v: any, _x: any) { return; } - } + }, + options: {} } }; } From d65cd3a7c8837c0dc745c2ceb4e432c648e6c9d9 Mon Sep 17 00:00:00 2001 From: Alena Khineika Date: Fri, 1 Mar 2024 14:03:19 +0100 Subject: [PATCH 08/21] test: sync run-command spec tests --- src/cmap/connection.ts | 2 +- test/spec/run-command/runCommand.json | 3 +-- test/spec/run-command/runCommand.yml | 4 ++-- 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/cmap/connection.ts b/src/cmap/connection.ts index c390147b894..8083abea844 100644 --- a/src/cmap/connection.ts +++ b/src/cmap/connection.ts @@ -408,7 +408,7 @@ export class Connection extends TypedEventEmitter { // For mongos and load balancers with a direct connection and 'primary' mode, // drivers MUST NOT set $readPreference. if (isSharded(this) || this.description.loadBalanced) { - if (this.description.directConnection !== true || readPreference?.mode !== 'primary') { + if (this.description.directConnection !== true && readPreference?.mode !== 'primary') { cmd.$readPreference = readPreference.toJSON(); } } else { diff --git a/test/spec/run-command/runCommand.json b/test/spec/run-command/runCommand.json index 007e514bd73..fde9de92e60 100644 --- a/test/spec/run-command/runCommand.json +++ b/test/spec/run-command/runCommand.json @@ -229,7 +229,6 @@ { "topologies": [ "replicaset", - "sharded-replicaset", "load-balanced", "sharded" ] @@ -493,7 +492,7 @@ { "minServerVersion": "4.2", "topologies": [ - "sharded-replicaset", + "sharded", "load-balanced" ] } diff --git a/test/spec/run-command/runCommand.yml b/test/spec/run-command/runCommand.yml index eaa12eff231..9b0bf1ad63f 100644 --- a/test/spec/run-command/runCommand.yml +++ b/test/spec/run-command/runCommand.yml @@ -119,7 +119,7 @@ tests: - description: attaches the provided $readPreference to given command runOnRequirements: # Exclude single topology, which is most likely a standalone server - - topologies: [ replicaset, sharded-replicaset, load-balanced, sharded ] + - topologies: [ replicaset, load-balanced, sharded ] operations: - name: runCommand object: *db @@ -250,7 +250,7 @@ tests: - minServerVersion: "4.0" topologies: [ replicaset ] - minServerVersion: "4.2" - topologies: [ sharded-replicaset, load-balanced ] + topologies: [ sharded, load-balanced ] operations: - name: withTransaction object: *session From f591d2eaeaf51b27a3f74bea2795240aabc05114 Mon Sep 17 00:00:00 2001 From: Alena Khineika Date: Fri, 1 Mar 2024 14:20:13 +0100 Subject: [PATCH 09/21] fix: do not check direct connection --- src/cmap/connection.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cmap/connection.ts b/src/cmap/connection.ts index 8083abea844..0e9e50b33b7 100644 --- a/src/cmap/connection.ts +++ b/src/cmap/connection.ts @@ -408,7 +408,7 @@ export class Connection extends TypedEventEmitter { // For mongos and load balancers with a direct connection and 'primary' mode, // drivers MUST NOT set $readPreference. if (isSharded(this) || this.description.loadBalanced) { - if (this.description.directConnection !== true && readPreference?.mode !== 'primary') { + if (readPreference?.mode !== 'primary') { cmd.$readPreference = readPreference.toJSON(); } } else { From 9883c45adfab988a7840e556d7d5604693d223f9 Mon Sep 17 00:00:00 2001 From: Alena Khineika Date: Fri, 1 Mar 2024 14:20:50 +0100 Subject: [PATCH 10/21] docs: update comment --- src/cmap/connection.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/cmap/connection.ts b/src/cmap/connection.ts index 0e9e50b33b7..418e9e470ff 100644 --- a/src/cmap/connection.ts +++ b/src/cmap/connection.ts @@ -405,8 +405,7 @@ export class Connection extends TypedEventEmitter { // For standalone, drivers MUST NOT set $readPreference. if (this.supportsOpMsg && this.description.type !== ServerType.Standalone) { - // For mongos and load balancers with a direct connection and 'primary' mode, - // drivers MUST NOT set $readPreference. + // For mongos and load balancers with 'primary' mode, drivers MUST NOT set $readPreference. if (isSharded(this) || this.description.loadBalanced) { if (readPreference?.mode !== 'primary') { cmd.$readPreference = readPreference.toJSON(); From 2de4e4d914e0353c31e3d49e781565625e888dfa Mon Sep 17 00:00:00 2001 From: Alena Khineika Date: Fri, 1 Mar 2024 15:20:14 +0100 Subject: [PATCH 11/21] fix: check for primary for all topologies --- src/cmap/connection.ts | 29 ++++++++++++++++------------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/src/cmap/connection.ts b/src/cmap/connection.ts index 418e9e470ff..4cc5860b567 100644 --- a/src/cmap/connection.ts +++ b/src/cmap/connection.ts @@ -406,19 +406,22 @@ export class Connection extends TypedEventEmitter { // For standalone, drivers MUST NOT set $readPreference. if (this.supportsOpMsg && this.description.type !== ServerType.Standalone) { // For mongos and load balancers with 'primary' mode, drivers MUST NOT set $readPreference. - if (isSharded(this) || this.description.loadBalanced) { - if (readPreference?.mode !== 'primary') { - cmd.$readPreference = readPreference.toJSON(); - } - } else { - // For all other types with a direct connection, if the read preference is 'primary' - // (driver sets 'primary' as default if no read preference is configured), - // the $readPreference MUST be set to 'primaryPreferred' - // to ensure that any server type can handle the request. - cmd.$readPreference = - this.description.directConnection !== true || readPreference?.mode !== 'primary' - ? readPreference.toJSON() - : ReadPreference.primaryPreferred.toJSON(); + // For all other types with a direct connection, if the read preference is 'primary' + // (driver sets 'primary' as default if no read preference is configured), + // the $readPreference MUST be set to 'primaryPreferred' + // to ensure that any server type can handle the request. + if ( + !isSharded(this) && + !this.description.loadBalanced && + this.description.directConnection === true && + readPreference?.mode === 'primary' + ) { + cmd.$readPreference = ReadPreference.primaryPreferred.toJSON(); + } else if (readPreference?.mode !== 'primary') { + // For mode 'primary', drivers MUST NOT set $readPreference. + // For all other read preference modes (i.e. 'secondary', 'primaryPreferred', ...), + // drivers MUST set $readPreference + cmd.$readPreference = readPreference.toJSON(); } } From 4193f7dc5f0bb1e5bbd934c510ee1d41c70e5d0b Mon Sep 17 00:00:00 2001 From: Alena Khineika Date: Tue, 5 Mar 2024 17:03:20 +0100 Subject: [PATCH 12/21] refactor: try to revert query --- src/cmap/connection.ts | 26 +-- .../max-staleness/max_staleness.test.js | 155 ++++++++++-------- 2 files changed, 98 insertions(+), 83 deletions(-) diff --git a/src/cmap/connection.ts b/src/cmap/connection.ts index 4cc5860b567..903fda2bc04 100644 --- a/src/cmap/connection.ts +++ b/src/cmap/connection.ts @@ -366,7 +366,7 @@ export class Connection extends TypedEventEmitter { } private prepareCommand(db: string, command: Document, options: CommandOptions) { - const cmd = { ...command }; + let cmd = { ...command }; const readPreference = getReadPreference(options); const session = options?.session; @@ -399,24 +399,28 @@ export class Connection extends TypedEventEmitter { cmd.$clusterTime = clusterTime; } - // Driver does not use legacy hello for anything other than the first handshake - // (we do not support servers that only speak OP_QUERY), - // therefore $readPreference is not relevant for servers that do not support OP_MSG. - // For standalone, drivers MUST NOT set $readPreference. - if (this.supportsOpMsg && this.description.type !== ServerType.Standalone) { - // For mongos and load balancers with 'primary' mode, drivers MUST NOT set $readPreference. - // For all other types with a direct connection, if the read preference is 'primary' - // (driver sets 'primary' as default if no read preference is configured), - // the $readPreference MUST be set to 'primaryPreferred' - // to ensure that any server type can handle the request. + if (this.description.type !== ServerType.Standalone) { if ( !isSharded(this) && !this.description.loadBalanced && + this.supportsOpMsg && this.description.directConnection === true && readPreference?.mode === 'primary' ) { + // For mongos and load balancers with 'primary' mode, drivers MUST NOT set $readPreference. + // For all other types with a direct connection, if the read preference is 'primary' + // (driver sets 'primary' as default if no read preference is configured), + // the $readPreference MUST be set to 'primaryPreferred' + // to ensure that any server type can handle the request. cmd.$readPreference = ReadPreference.primaryPreferred.toJSON(); + } else if (isSharded(this) && !this.supportsOpMsg && readPreference?.mode !== 'primary') { + // When sending a read operation via OP_QUERY and the $readPreference modifier, + // the query MUST be provided using the $query modifier. + cmd = { + $query: cmd, + $readPreference: readPreference.toJSON() + }; } else if (readPreference?.mode !== 'primary') { // For mode 'primary', drivers MUST NOT set $readPreference. // For all other read preference modes (i.e. 'secondary', 'primaryPreferred', ...), diff --git a/test/integration/max-staleness/max_staleness.test.js b/test/integration/max-staleness/max_staleness.test.js index d4dbf413683..3b559979f83 100644 --- a/test/integration/max-staleness/max_staleness.test.js +++ b/test/integration/max-staleness/max_staleness.test.js @@ -18,7 +18,7 @@ describe('Max Staleness', function () { // Primary server states const serverIsPrimary = [Object.assign({}, defaultFields)]; server.setMessageHandler(request => { - var doc = request.document; + const doc = request.document; if (isHello(doc)) { request.reply(serverIsPrimary[0]); return; @@ -46,33 +46,31 @@ describe('Max Staleness', function () { metadata: { requires: { generators: true, - topology: 'single' + topology: 'replicaset' } }, - test: function (done) { - var self = this; + test: function () { + const self = this; const configuration = this.configuration; const client = configuration.newClient( `mongodb://${test.server.uri()}/test?readPreference=secondary&maxStalenessSeconds=250`, { serverApi: null } // TODO(NODE-3807): remove resetting serverApi when the usage of mongodb mock server is removed ); - client.connect(function (err, client) { + client.connect(async function (err, client) { expect(err).to.not.exist; - var db = client.db(self.configuration.db); - - db.collection('test') - .find({}) - .toArray(function (err) { - expect(err).to.not.exist; - expect(test.checkCommand).to.containSubset({ - $query: { find: 'test', filter: {} }, - $readPreference: { mode: 'secondary', maxStalenessSeconds: 250 } - }); - - client.close(done); - }); + const db = client.db(self.configuration.db); + + try { + await db.collection('test').find({}).toArray(); + } catch (err) { + expect(err).to.not.exist; + } + + expect(test.checkCommand).to.containSubset({ + $readPreference: { mode: 'secondary', maxStalenessSeconds: 250 } + }); }); } }); @@ -81,36 +79,38 @@ describe('Max Staleness', function () { metadata: { requires: { generators: true, - topology: 'single' + topology: 'replicaset' } }, - test: function (done) { + test: async function () { const configuration = this.configuration; const client = configuration.newClient(`mongodb://${test.server.uri()}/test`, { serverApi: null // TODO(NODE-3807): remove resetting serverApi when the usage of mongodb mock server is removed }); - client.connect(function (err, client) { + + try { + await client.connect(); + } catch (err) { expect(err).to.not.exist; + } - // Get a db with a new readPreference - var db1 = client.db('test', { - readPreference: new ReadPreference('secondary', null, { maxStalenessSeconds: 250 }) - }); + // Get a db with a new readPreference + const db1 = client.db('test', { + readPreference: new ReadPreference('secondary', null, { maxStalenessSeconds: 250 }) + }); - db1 - .collection('test') - .find({}) - .toArray(function (err) { - expect(err).to.not.exist; - expect(test.checkCommand).to.containSubset({ - $query: { find: 'test', filter: {} }, - $readPreference: { mode: 'secondary', maxStalenessSeconds: 250 } - }); - - client.close(done); - }); + try { + await db1.collection('test').find({}).toArray(); + } catch (err) { + expect(err).to.not.exist; + } + + expect(test.checkCommand).to.containSubset({ + $readPreference: { mode: 'secondary', maxStalenessSeconds: 250 } }); + + client.close(); } }); @@ -120,35 +120,42 @@ describe('Max Staleness', function () { metadata: { requires: { generators: true, - topology: 'single' + topology: 'replicaset' } }, - test: function (done) { - var self = this; + test: async function () { + const self = this; const configuration = this.configuration; const client = configuration.newClient(`mongodb://${test.server.uri()}/test`, { serverApi: null // TODO(NODE-3807): remove resetting serverApi when the usage of mongodb mock server is removed }); - client.connect(function (err, client) { + + try { + await client.connect(); + } catch (err) { expect(err).to.not.exist; - var db = client.db(self.configuration.db); + } + const db = client.db(self.configuration.db); + + try { // Get a db with a new readPreference - db.collection('test', { - readPreference: new ReadPreference('secondary', null, { maxStalenessSeconds: 250 }) - }) + await db + .collection('test', { + readPreference: new ReadPreference('secondary', null, { maxStalenessSeconds: 250 }) + }) .find({}) - .toArray(function (err) { - expect(err).to.not.exist; - expect(test.checkCommand).to.containSubset({ - $query: { find: 'test', filter: {} }, - $readPreference: { mode: 'secondary', maxStalenessSeconds: 250 } - }); - - client.close(done); - }); + .toArray(); + } catch (err) { + expect(err).to.not.exist; + } + + expect(test.checkCommand).to.containSubset({ + $readPreference: { mode: 'secondary', maxStalenessSeconds: 250 } }); + + client.close(); } } ); @@ -157,35 +164,39 @@ describe('Max Staleness', function () { metadata: { requires: { generators: true, - topology: 'single' + topology: 'replicaset' } }, - test: function (done) { - var self = this; + test: async function () { + const self = this; const configuration = this.configuration; const client = configuration.newClient(`mongodb://${test.server.uri()}/test`, { serverApi: null // TODO(NODE-3807): remove resetting serverApi when the usage of mongodb mock server is removed }); - client.connect(function (err, client) { + + try { + await client.connect(); + } catch (err) { expect(err).to.not.exist; - var db = client.db(self.configuration.db); - var readPreference = new ReadPreference('secondary', null, { maxStalenessSeconds: 250 }); + } + + const db = client.db(self.configuration.db); + const readPreference = new ReadPreference('secondary', null, { maxStalenessSeconds: 250 }); + try { // Get a db with a new readPreference - db.collection('test') - .find({}) - .withReadPreference(readPreference) - .toArray(function (err) { - expect(err).to.not.exist; - expect(test.checkCommand).to.containSubset({ - $query: { find: 'test', filter: {} }, - $readPreference: { mode: 'secondary', maxStalenessSeconds: 250 } - }); - - client.close(done); - }); + await db.collection('test').find({}).withReadPreference(readPreference).toArray(); + } catch (err) { + expect(err).to.not.exist; + } + + expect(test.checkCommand).to.containSubset({ + $query: { find: 'test', filter: {} }, + $readPreference: { mode: 'secondary', maxStalenessSeconds: 250 } }); + + client.close(); } }); }); From 40a48476238330fd0175288bf9c12ba3ac49fe42 Mon Sep 17 00:00:00 2001 From: Alena Khineika Date: Tue, 5 Mar 2024 20:57:10 +0100 Subject: [PATCH 13/21] test: exclude arbiter --- .../server-selection/readpreference.test.ts | 76 ++++++++++++++----- 1 file changed, 56 insertions(+), 20 deletions(-) diff --git a/test/integration/server-selection/readpreference.test.ts b/test/integration/server-selection/readpreference.test.ts index 8043759ef0a..acc6ba9503b 100644 --- a/test/integration/server-selection/readpreference.test.ts +++ b/test/integration/server-selection/readpreference.test.ts @@ -463,6 +463,8 @@ describe('ReadPreference', function () { it('should attach a read preference of primaryPreferred to the read command for replicaset', { metadata: { requires: { topology: 'replicaset', mongodb: '>=3.6' } }, test: async function () { + let checkedPrimary = false; + for (const server of this.configuration.options.hostAddresses) { const { host, port } = server.toHostPort(); const client = this.configuration.newClient( @@ -482,23 +484,39 @@ describe('ReadPreference', function () { events.push(event); } }); - let serverError; + let serverStatus; + try { - await client.db('test').collection('test').findOne({ a: 1 }); - } catch (error) { - serverError = error; + const admin = client.db().admin(); + serverStatus = await admin.serverStatus(); + } catch (serverStatusError) { + expect(serverStatusError).to.not.exist; } - expect(serverError).to.not.exist; - expect(events[0]).to.containSubset({ - commandName: 'find', - command: { - $readPreference: { mode: 'primaryPreferred' } + if (serverStatus.repl.ismaster) { + let serverError; + + try { + await client.db('test').collection('test').findOne({ a: 1 }); + } catch (error) { + serverError = error; } - }); + + expect(serverError).to.not.exist; + expect(events[0]).to.containSubset({ + commandName: 'find', + command: { + $readPreference: { mode: 'primaryPreferred' } + } + }); + + checkedPrimary = true; + } await client.close(); } + + expect(checkedPrimary).to.be.equal(true); } }); @@ -550,6 +568,8 @@ describe('ReadPreference', function () { it('should attach a read preference of secondary to the read command for replicaset', { metadata: { requires: { topology: 'replicaset', mongodb: '>=3.6' } }, test: async function () { + let checkedSecondary = false; + for (const server of this.configuration.options.hostAddresses) { const { host, port } = server.toHostPort(); const client = this.configuration.newClient( @@ -569,23 +589,39 @@ describe('ReadPreference', function () { events.push(event); } }); - let serverError; + let serverStatus; + try { - await client.db('test').collection('test').findOne({ a: 1 }); - } catch (error) { - serverError = error; + const admin = client.db().admin(); + serverStatus = await admin.serverStatus(); + } catch (serverStatusError) { + expect(serverStatusError).to.not.exist; } - expect(serverError).to.not.exist; - expect(events[0]).to.containSubset({ - commandName: 'find', - command: { - $readPreference: { mode: 'secondary' } + if (serverStatus.repl.secondary) { + let serverError; + + try { + await client.db('test').collection('test').findOne({ a: 1 }); + } catch (error) { + serverError = error; } - }); + + expect(serverError).to.not.exist; + expect(events[0]).to.containSubset({ + commandName: 'find', + command: { + $readPreference: { mode: 'secondary' } + } + }); + + checkedSecondary = true; + } await client.close(); } + + expect(checkedSecondary).to.be.equal(true); } }); From 1806f30cbc2686bbca8e314486ac0ae86870408e Mon Sep 17 00:00:00 2001 From: Alena Khineika Date: Tue, 5 Mar 2024 21:11:47 +0100 Subject: [PATCH 14/21] test: debug --- .../server-selection/readpreference.test.ts | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/test/integration/server-selection/readpreference.test.ts b/test/integration/server-selection/readpreference.test.ts index acc6ba9503b..1ebd7c31fe9 100644 --- a/test/integration/server-selection/readpreference.test.ts +++ b/test/integration/server-selection/readpreference.test.ts @@ -3,7 +3,7 @@ import { expect } from 'chai'; import { ReadPreference, Topology } from '../../mongodb'; import { assert as test, setupDatabase } from '../shared'; -describe('ReadPreference', function () { +describe.only('ReadPreference', function () { let client; let events; @@ -489,7 +489,15 @@ describe('ReadPreference', function () { try { const admin = client.db().admin(); serverStatus = await admin.serverStatus(); + + console.log('serverStatus.repl----------------------'); + console.log(serverStatus.repl); + console.log('----------------------'); } catch (serverStatusError) { + + console.log('serverStatusError----------------------'); + console.log(serverStatusError); + console.log('----------------------'); expect(serverStatusError).to.not.exist; } From bb79b9bac898f032aa20e8e81b6e14e0487970e9 Mon Sep 17 00:00:00 2001 From: Alena Khineika Date: Tue, 5 Mar 2024 21:14:42 +0100 Subject: [PATCH 15/21] test: skip when is not ReplicaSetWithPrimary --- .../server-selection/readpreference.test.ts | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/test/integration/server-selection/readpreference.test.ts b/test/integration/server-selection/readpreference.test.ts index 1ebd7c31fe9..996d0d77b2e 100644 --- a/test/integration/server-selection/readpreference.test.ts +++ b/test/integration/server-selection/readpreference.test.ts @@ -3,7 +3,7 @@ import { expect } from 'chai'; import { ReadPreference, Topology } from '../../mongodb'; import { assert as test, setupDatabase } from '../shared'; -describe.only('ReadPreference', function () { +describe('ReadPreference', function () { let client; let events; @@ -463,6 +463,10 @@ describe.only('ReadPreference', function () { it('should attach a read preference of primaryPreferred to the read command for replicaset', { metadata: { requires: { topology: 'replicaset', mongodb: '>=3.6' } }, test: async function () { + if (this.configuration.topologyType !== 'ReplicaSetWithPrimary') { + return this.skip(); + } + let checkedPrimary = false; for (const server of this.configuration.options.hostAddresses) { @@ -489,15 +493,7 @@ describe.only('ReadPreference', function () { try { const admin = client.db().admin(); serverStatus = await admin.serverStatus(); - - console.log('serverStatus.repl----------------------'); - console.log(serverStatus.repl); - console.log('----------------------'); } catch (serverStatusError) { - - console.log('serverStatusError----------------------'); - console.log(serverStatusError); - console.log('----------------------'); expect(serverStatusError).to.not.exist; } From 250451e51accda553bceec34a8ee1450b5d57f26 Mon Sep 17 00:00:00 2001 From: Alena Khineika Date: Tue, 5 Mar 2024 21:58:40 +0100 Subject: [PATCH 16/21] test: debug --- test/integration/server-selection/readpreference.test.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/test/integration/server-selection/readpreference.test.ts b/test/integration/server-selection/readpreference.test.ts index 996d0d77b2e..6c11c874bfa 100644 --- a/test/integration/server-selection/readpreference.test.ts +++ b/test/integration/server-selection/readpreference.test.ts @@ -493,8 +493,16 @@ describe('ReadPreference', function () { try { const admin = client.db().admin(); serverStatus = await admin.serverStatus(); + + console.log('serverStatus.repl----------------------'); + console.log(serverStatus.repl); + console.log('----------------------'); } catch (serverStatusError) { expect(serverStatusError).to.not.exist; + + console.log('serverStatusError----------------------'); + console.log(serverStatusError); + console.log('----------------------'); } if (serverStatus.repl.ismaster) { From cd9ecdfd0c1493f41db03f8f4625c5229a9c0316 Mon Sep 17 00:00:00 2001 From: Alena Khineika Date: Tue, 5 Mar 2024 22:17:50 +0100 Subject: [PATCH 17/21] test: compare with primary as a string --- .../server-selection/readpreference.test.ts | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/test/integration/server-selection/readpreference.test.ts b/test/integration/server-selection/readpreference.test.ts index 6c11c874bfa..bf55480f57f 100644 --- a/test/integration/server-selection/readpreference.test.ts +++ b/test/integration/server-selection/readpreference.test.ts @@ -493,19 +493,11 @@ describe('ReadPreference', function () { try { const admin = client.db().admin(); serverStatus = await admin.serverStatus(); - - console.log('serverStatus.repl----------------------'); - console.log(serverStatus.repl); - console.log('----------------------'); } catch (serverStatusError) { expect(serverStatusError).to.not.exist; - - console.log('serverStatusError----------------------'); - console.log(serverStatusError); - console.log('----------------------'); } - if (serverStatus.repl.ismaster) { + if (server.toString() === serverStatus.repl.primary) { let serverError; try { From 4c82bb8e7dc31d7fb880669bdfbc73bbd5fa8ea4 Mon Sep 17 00:00:00 2001 From: Alena Khineika Date: Thu, 7 Mar 2024 12:04:59 +0100 Subject: [PATCH 18/21] test: adress pr comments --- src/cmap/connection.ts | 3 +- src/cmap/connection_pool.ts | 4 +- .../max-staleness/max_staleness.test.js | 85 ++++--------- .../server-selection/readpreference.test.ts | 115 ++++-------------- 4 files changed, 52 insertions(+), 155 deletions(-) diff --git a/src/cmap/connection.ts b/src/cmap/connection.ts index 903fda2bc04..8c33cf550fd 100644 --- a/src/cmap/connection.ts +++ b/src/cmap/connection.ts @@ -121,7 +121,8 @@ export interface ConnectionOptions metadata: ClientMetadata; /** @internal */ mongoLogger?: MongoLogger | undefined; - dierctConnection?: boolean; + /** @internal */ + directConnection?: boolean; } /** @internal */ diff --git a/src/cmap/connection_pool.ts b/src/cmap/connection_pool.ts index 4c046abb202..d9f71bdb529 100644 --- a/src/cmap/connection_pool.ts +++ b/src/cmap/connection_pool.ts @@ -100,7 +100,7 @@ export interface ConnectionPoolOptions extends Omit { cancellationToken: this[kCancellationToken], mongoLogger: this.mongoLogger, authProviders: this[kServer].topology.client.s.authProviders, - dierctConnection: this[kServer].topology.client.options.directConnection === true + directConnection: this[kServer].topology.client.options.directConnection === true }; this[kPending]++; diff --git a/test/integration/max-staleness/max_staleness.test.js b/test/integration/max-staleness/max_staleness.test.js index 3b559979f83..b93b343fee0 100644 --- a/test/integration/max-staleness/max_staleness.test.js +++ b/test/integration/max-staleness/max_staleness.test.js @@ -50,7 +50,7 @@ describe('Max Staleness', function () { } }, - test: function () { + test: async function () { const self = this; const configuration = this.configuration; const client = configuration.newClient( @@ -58,20 +58,13 @@ describe('Max Staleness', function () { { serverApi: null } // TODO(NODE-3807): remove resetting serverApi when the usage of mongodb mock server is removed ); - client.connect(async function (err, client) { - expect(err).to.not.exist; - const db = client.db(self.configuration.db); - - try { - await db.collection('test').find({}).toArray(); - } catch (err) { - expect(err).to.not.exist; - } - - expect(test.checkCommand).to.containSubset({ - $readPreference: { mode: 'secondary', maxStalenessSeconds: 250 } - }); + await client.connect(); + const db = client.db(self.configuration.db); + await db.collection('test').find({}).toArray(); + expect(test.checkCommand).to.containSubset({ + $readPreference: { mode: 'secondary', maxStalenessSeconds: 250 } }); + await client.close(); } }); @@ -89,28 +82,17 @@ describe('Max Staleness', function () { serverApi: null // TODO(NODE-3807): remove resetting serverApi when the usage of mongodb mock server is removed }); - try { - await client.connect(); - } catch (err) { - expect(err).to.not.exist; - } + await client.connect(); // Get a db with a new readPreference const db1 = client.db('test', { readPreference: new ReadPreference('secondary', null, { maxStalenessSeconds: 250 }) }); - - try { - await db1.collection('test').find({}).toArray(); - } catch (err) { - expect(err).to.not.exist; - } - + await db1.collection('test').find({}).toArray(); expect(test.checkCommand).to.containSubset({ $readPreference: { mode: 'secondary', maxStalenessSeconds: 250 } }); - - client.close(); + await client.close(); } }); @@ -131,31 +113,20 @@ describe('Max Staleness', function () { serverApi: null // TODO(NODE-3807): remove resetting serverApi when the usage of mongodb mock server is removed }); - try { - await client.connect(); - } catch (err) { - expect(err).to.not.exist; - } - + await client.connect(); const db = client.db(self.configuration.db); - try { - // Get a db with a new readPreference - await db - .collection('test', { - readPreference: new ReadPreference('secondary', null, { maxStalenessSeconds: 250 }) - }) - .find({}) - .toArray(); - } catch (err) { - expect(err).to.not.exist; - } - + // Get a db with a new readPreference + await db + .collection('test', { + readPreference: new ReadPreference('secondary', null, { maxStalenessSeconds: 250 }) + }) + .find({}) + .toArray(); expect(test.checkCommand).to.containSubset({ $readPreference: { mode: 'secondary', maxStalenessSeconds: 250 } }); - - client.close(); + await client.close(); } } ); @@ -175,28 +146,18 @@ describe('Max Staleness', function () { serverApi: null // TODO(NODE-3807): remove resetting serverApi when the usage of mongodb mock server is removed }); - try { - await client.connect(); - } catch (err) { - expect(err).to.not.exist; - } - + await client.connect(); const db = client.db(self.configuration.db); const readPreference = new ReadPreference('secondary', null, { maxStalenessSeconds: 250 }); - try { - // Get a db with a new readPreference - await db.collection('test').find({}).withReadPreference(readPreference).toArray(); - } catch (err) { - expect(err).to.not.exist; - } + // Get a db with a new readPreference + await db.collection('test').find({}).withReadPreference(readPreference).toArray(); expect(test.checkCommand).to.containSubset({ $query: { find: 'test', filter: {} }, $readPreference: { mode: 'secondary', maxStalenessSeconds: 250 } }); - - client.close(); + await client.close(); } }); }); diff --git a/test/integration/server-selection/readpreference.test.ts b/test/integration/server-selection/readpreference.test.ts index bf55480f57f..1b837598b37 100644 --- a/test/integration/server-selection/readpreference.test.ts +++ b/test/integration/server-selection/readpreference.test.ts @@ -431,7 +431,7 @@ describe('ReadPreference', function () { }); it('should respect readPreference from uri', { - metadata: { requires: { topology: 'replicaset', mongodb: '>=3.6' } }, + metadata: { requires: { topology: 'replicaset' } }, test: async function () { const client = this.configuration.newClient({ readPreference: 'secondary', @@ -458,12 +458,13 @@ describe('ReadPreference', function () { } }); - context('when connecting to a secondary in a replica set with a direct connection', () => { + context('when connecting to a secondary in a replica set with a direct connection', function () { context('when readPreference is primary', () => { it('should attach a read preference of primaryPreferred to the read command for replicaset', { - metadata: { requires: { topology: 'replicaset', mongodb: '>=3.6' } }, + metadata: { requires: { topology: 'replicaset' } }, test: async function () { if (this.configuration.topologyType !== 'ReplicaSetWithPrimary') { + this.skipReason = 'This test is supposed to run on the replicaset with primary'; return this.skip(); } @@ -488,44 +489,26 @@ describe('ReadPreference', function () { events.push(event); } }); - let serverStatus; - try { - const admin = client.db().admin(); - serverStatus = await admin.serverStatus(); - } catch (serverStatusError) { - expect(serverStatusError).to.not.exist; - } + const admin = client.db().admin(); + const serverStatus = await admin.serverStatus(); if (server.toString() === serverStatus.repl.primary) { - let serverError; - - try { - await client.db('test').collection('test').findOne({ a: 1 }); - } catch (error) { - serverError = error; - } - - expect(serverError).to.not.exist; - expect(events[0]).to.containSubset({ - commandName: 'find', - command: { - $readPreference: { mode: 'primaryPreferred' } - } + await client.db('test').collection('test').findOne({ a: 1 }); + expect(events[0]).to.have.property('commandName', 'find'); + expect(events[0]).to.have.deep.nested.property('command.$readPreference', { + mode: 'primaryPreferred' }); - checkedPrimary = true; } - await client.close(); } - expect(checkedPrimary).to.be.equal(true); } }); it('should not attach a read preference to the read command for standalone', { - metadata: { requires: { topology: 'single', mongodb: '>=3.6' } }, + metadata: { requires: { topology: 'single' } }, test: async function () { const client = this.configuration.newClient( { @@ -542,27 +525,9 @@ describe('ReadPreference', function () { events.push(event); } }); - let serverError; - try { - await client.db('test').collection('test').findOne({ a: 1 }); - } catch (error) { - serverError = error; - } - - expect(serverError).to.not.exist; - expect(events[0]).to.not.containSubset({ - commandName: 'find', - command: { - $readPreference: { mode: 'primaryPreferred' } - } - }); - expect(events[0]).to.not.containSubset({ - commandName: 'find', - command: { - $readPreference: { mode: 'primary' } - } - }); - + await client.db('test').collection('test').findOne({ a: 1 }); + expect(events[0]).to.have.property('commandName', 'find'); + expect(events[0]).to.not.have.deep.nested.property('command.$readPreference'); await client.close(); } }); @@ -570,7 +535,7 @@ describe('ReadPreference', function () { context('when readPreference is secondary', () => { it('should attach a read preference of secondary to the read command for replicaset', { - metadata: { requires: { topology: 'replicaset', mongodb: '>=3.6' } }, + metadata: { requires: { topology: 'replicaset' } }, test: async function () { let checkedSecondary = false; @@ -593,44 +558,26 @@ describe('ReadPreference', function () { events.push(event); } }); - let serverStatus; - try { - const admin = client.db().admin(); - serverStatus = await admin.serverStatus(); - } catch (serverStatusError) { - expect(serverStatusError).to.not.exist; - } + const admin = client.db().admin(); + const serverStatus = await admin.serverStatus(); if (serverStatus.repl.secondary) { - let serverError; - - try { - await client.db('test').collection('test').findOne({ a: 1 }); - } catch (error) { - serverError = error; - } - - expect(serverError).to.not.exist; - expect(events[0]).to.containSubset({ - commandName: 'find', - command: { - $readPreference: { mode: 'secondary' } - } + await client.db('test').collection('test').findOne({ a: 1 }); + expect(events[0]).to.have.property('commandName', 'find'); + expect(events[0]).to.have.deep.nested.property('command.$readPreference', { + mode: 'secondary' }); - checkedSecondary = true; } - await client.close(); } - expect(checkedSecondary).to.be.equal(true); } }); it('should not attach a read preference to the read command for standalone', { - metadata: { requires: { topology: 'single', mongodb: '>=3.6' } }, + metadata: { requires: { topology: 'single' } }, test: async function () { const client = this.configuration.newClient( { @@ -647,21 +594,9 @@ describe('ReadPreference', function () { events.push(event); } }); - let serverError; - try { - await client.db('test').collection('test').findOne({ a: 1 }); - } catch (error) { - serverError = error; - } - - expect(serverError).to.not.exist; - expect(events[0]).to.not.containSubset({ - commandName: 'find', - command: { - $readPreference: { mode: 'secondary' } - } - }); - + await client.db('test').collection('test').findOne({ a: 1 }); + expect(events[0]).to.have.property('commandName', 'find'); + expect(events[0]).to.not.have.deep.nested.property('command.$readPreference'); await client.close(); } }); From 4c41b4e8ed7ee9f4fa1176bcbe023f6eaafa2371 Mon Sep 17 00:00:00 2001 From: Alena Khineika Date: Thu, 7 Mar 2024 12:12:48 +0100 Subject: [PATCH 19/21] fix: type --- src/cmap/connection.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cmap/connection.ts b/src/cmap/connection.ts index ddc7158bb36..998ca725490 100644 --- a/src/cmap/connection.ts +++ b/src/cmap/connection.ts @@ -124,7 +124,7 @@ export interface ConnectionOptions /** @internal */ mongoLogger?: MongoLogger | undefined; /** @internal */ - directConnection?: boolean; + directConnection: boolean; } /** @internal */ From 61635b001fbc26e0021eb9a93c522be1b2768569 Mon Sep 17 00:00:00 2001 From: Bailey Pearson Date: Mon, 11 Mar 2024 09:33:02 -0600 Subject: [PATCH 20/21] refactor(no-story): don't store directConnection on the connection pool and stream description (#4027) --- src/cmap/connection.ts | 10 +++------- src/cmap/connection_pool.ts | 5 +---- src/cmap/stream_description.ts | 3 --- src/sdam/server.ts | 5 ++++- 4 files changed, 8 insertions(+), 15 deletions(-) diff --git a/src/cmap/connection.ts b/src/cmap/connection.ts index 998ca725490..cb76e9dd69a 100644 --- a/src/cmap/connection.ts +++ b/src/cmap/connection.ts @@ -85,6 +85,8 @@ export interface CommandOptions extends BSONSerializeOptions { willRetryWrite?: boolean; writeConcern?: WriteConcern; + + directConnection?: boolean; } /** @public */ @@ -123,8 +125,6 @@ export interface ConnectionOptions extendedMetadata: Promise; /** @internal */ mongoLogger?: MongoLogger | undefined; - /** @internal */ - directConnection: boolean; } /** @internal */ @@ -281,10 +281,6 @@ export class Connection extends TypedEventEmitter { return this.description.loadBalanced; } - public get directConnection(): boolean { - return this.description.directConnection; - } - public get idleTime(): number { return calculateDurationInMs(this.lastUseTime); } @@ -408,7 +404,7 @@ export class Connection extends TypedEventEmitter { !isSharded(this) && !this.description.loadBalanced && this.supportsOpMsg && - this.description.directConnection === true && + options.directConnection === true && readPreference?.mode === 'primary' ) { // For mongos and load balancers with 'primary' mode, drivers MUST NOT set $readPreference. diff --git a/src/cmap/connection_pool.ts b/src/cmap/connection_pool.ts index 585e9145a8a..64b89ee1200 100644 --- a/src/cmap/connection_pool.ts +++ b/src/cmap/connection_pool.ts @@ -100,8 +100,6 @@ export interface ConnectionPoolOptions extends Omit { generation: this[kGeneration], cancellationToken: this[kCancellationToken], mongoLogger: this.mongoLogger, - authProviders: this[kServer].topology.client.s.authProviders, - directConnection: this[kServer].topology.client.options.directConnection === true + authProviders: this[kServer].topology.client.s.authProviders }; this[kPending]++; diff --git a/src/cmap/stream_description.ts b/src/cmap/stream_description.ts index 43cb2c9af91..88411c92c49 100644 --- a/src/cmap/stream_description.ts +++ b/src/cmap/stream_description.ts @@ -17,7 +17,6 @@ export interface StreamDescriptionOptions { compressors?: CompressorName[]; logicalSessionTimeoutMinutes?: number; loadBalanced: boolean; - directConnection: boolean; } /** @public */ @@ -33,7 +32,6 @@ export class StreamDescription { compressor?: CompressorName; logicalSessionTimeoutMinutes?: number; loadBalanced: boolean; - directConnection: boolean; __nodejs_mock_server__?: boolean; @@ -57,7 +55,6 @@ export class StreamDescription { ? options.compressors : []; this.serverConnectionId = null; - this.directConnection = !!options?.directConnection; } receiveResponse(response: Document | null): void { diff --git a/src/sdam/server.ts b/src/sdam/server.ts index 13cadd9f7fc..4e719192890 100644 --- a/src/sdam/server.ts +++ b/src/sdam/server.ts @@ -290,7 +290,10 @@ export class Server extends TypedEventEmitter { } // Clone the options - const finalOptions = Object.assign({}, options, { wireProtocolCommand: false }); + const finalOptions = Object.assign({}, options, { + wireProtocolCommand: false, + directConnection: this.topology.s.options.directConnection + }); // There are cases where we need to flag the read preference not to get sent in // the command, such as pre-5.0 servers attempting to perform an aggregate write From 4bee076fe85f5bded65c643b62cb552304246add Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Mon, 11 Mar 2024 11:40:33 -0400 Subject: [PATCH 21/21] undo whitespace changes --- src/cmap/connection.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/cmap/connection.ts b/src/cmap/connection.ts index 0860b8df04a..7ea71727e2f 100644 --- a/src/cmap/connection.ts +++ b/src/cmap/connection.ts @@ -341,6 +341,7 @@ export class Connection extends TypedEventEmitter { private prepareCommand(db: string, command: Document, options: CommandOptions) { let cmd = { ...command }; + const readPreference = getReadPreference(options); const session = options?.session; @@ -368,7 +369,7 @@ export class Connection extends TypedEventEmitter { throw new MongoCompatibilityError('Current topology does not support sessions'); } - // if we have a known cluster time, gossip it. + // if we have a known cluster time, gossip it if (clusterTime) { cmd.$clusterTime = clusterTime; }