From 85fbb89dd6f97cd81759124d834dc6029473b0f4 Mon Sep 17 00:00:00 2001 From: Bailey Pearson Date: Wed, 29 Mar 2023 11:44:51 -0400 Subject: [PATCH 01/12] fix metadata issue --- src/cmap/handshake/client_metadata.ts | 94 +++++++++++++++++++ src/connection_string.ts | 18 ++-- .../cmap/handshake/client_metadata.test.ts | 83 ++++++++++++++++ 3 files changed, 183 insertions(+), 12 deletions(-) create mode 100644 src/cmap/handshake/client_metadata.ts create mode 100644 test/unit/cmap/handshake/client_metadata.test.ts diff --git a/src/cmap/handshake/client_metadata.ts b/src/cmap/handshake/client_metadata.ts new file mode 100644 index 00000000000..f52a75de073 --- /dev/null +++ b/src/cmap/handshake/client_metadata.ts @@ -0,0 +1,94 @@ +import * as os from 'os'; + +import type { MongoOptions } from '../../mongo_client'; + +// eslint-disable-next-line @typescript-eslint/no-var-requires +const NODE_DRIVER_VERSION = require('../../../package.json').version; + +/** + * FaaS environment metadata. + * @public + */ +export interface FaasMetadata { + /** All metadata has a name */ + name: string; + /** Lambda/GCP/Vercel */ + region?: string; + /** Lambda/GCP */ + memoryMb?: string; + /** GCP */ + timeoutSec?: string; + /** Vercel */ + url?: string; +} + +/** @public */ +export interface ClientMetadata { + driver: { + name: string; + version: string; + }; + os: { + type: string; + name: NodeJS.Platform; + architecture: string; + version: string; + }; + platform: string; + version?: string; + application?: { + name: string; + }; + env?: FaasMetadata; +} + +/** @public */ +export interface ClientMetadataOptions { + driverInfo?: { + name?: string; + version?: string; + platform?: string; + }; + appName?: string; +} + +export function makeClientMetadata(options: MongoOptions): ClientMetadata { + const metadata: ClientMetadata = { + driver: { + name: 'nodejs', + version: NODE_DRIVER_VERSION + }, + os: { + type: os.type(), + name: process.platform, + architecture: process.arch, + version: os.release() + }, + platform: `Node.js ${process.version}, ${os.endianness()} (unified)` + }; + + // support optionally provided wrapping driver info + if (options.driverInfo) { + if (options.driverInfo.name) { + metadata.driver.name = `${metadata.driver.name}|${options.driverInfo.name}`; + } + + if (options.driverInfo.version) { + metadata.version = `${metadata.driver.version}|${options.driverInfo.version}`; + } + + if (options.driverInfo.platform) { + metadata.platform = `${metadata.platform}|${options.driverInfo.platform}`; + } + } + + if (options.appName) { + // MongoDB requires the appName not exceed a byte length of 128 + const buffer = Buffer.from(options.appName); + metadata.application = { + name: buffer.byteLength > 128 ? buffer.slice(0, 128).toString('utf8') : options.appName + }; + } + + return metadata; +} diff --git a/src/connection_string.ts b/src/connection_string.ts index 096af0545a1..cd59306518d 100644 --- a/src/connection_string.ts +++ b/src/connection_string.ts @@ -15,7 +15,6 @@ import { MongoParseError } from './error'; import { - DriverInfo, MongoClient, MongoClientOptions, MongoOptions, @@ -543,6 +542,8 @@ export function parseOptions( loggerClientOptions ); + mongoOptions.metadata = makeClientMetadata(mongoOptions); + return mongoOptions; } @@ -644,10 +645,7 @@ interface OptionDescriptor { export const OPTIONS = { appName: { - target: 'metadata', - transform({ options, values: [value] }): DriverInfo { - return makeClientMetadata({ ...options.driverInfo, appName: String(value) }); - } + type: 'string' }, auth: { target: 'credentials', @@ -798,14 +796,10 @@ export const OPTIONS = { type: 'boolean' }, driverInfo: { - target: 'metadata', - default: makeClientMetadata(), - transform({ options, values: [value] }) { + default: {}, + transform({ values: [value] }) { if (!isRecord(value)) throw new MongoParseError('DriverInfo must be an object'); - return makeClientMetadata({ - driverInfo: value, - appName: options.metadata?.application?.name - }); + return value; } }, enableUtf8Validation: { type: 'boolean', default: true }, diff --git a/test/unit/cmap/handshake/client_metadata.test.ts b/test/unit/cmap/handshake/client_metadata.test.ts new file mode 100644 index 00000000000..a0fb11de614 --- /dev/null +++ b/test/unit/cmap/handshake/client_metadata.test.ts @@ -0,0 +1,83 @@ +import { expect } from 'chai'; +import * as os from 'os'; + +import { makeClientMetadata } from '../../../mongodb'; + +// eslint-disable-next-line @typescript-eslint/no-var-requires +const NODE_DRIVER_VERSION = require('../../../../package.json').version; + +describe('ClientMetadata [module]', function () { + describe('.makeClientMetadata', function () { + context('when options are provided', function () { + context('when driver info is provided', function () { + const options = { + driverInfo: { platform: 'myPlatform', name: 'myName', version: 'myVersion' } + }; + const metadata = makeClientMetadata(options); + + it('appends the driver info to the metadata', function () { + expect(metadata).to.deep.equal({ + driver: { + name: 'nodejs|myName', + version: NODE_DRIVER_VERSION + }, + os: { + type: os.type(), + name: process.platform, + architecture: process.arch, + version: os.release() + }, + platform: `Node.js ${process.version}, ${os.endianness()} (unified)|myPlatform`, + version: `${NODE_DRIVER_VERSION}|myVersion` + }); + }); + }); + + context('when driver info is not provided', function () { + const metadata = makeClientMetadata({}); + + it('does not append the driver info to the metadata', function () { + expect(metadata).to.deep.equal({ + driver: { + name: 'nodejs', + version: NODE_DRIVER_VERSION + }, + os: { + type: os.type(), + name: process.platform, + architecture: process.arch, + version: os.release() + }, + platform: `Node.js ${process.version}, ${os.endianness()} (unified)` + }); + }); + }); + + context('when app name is provided', function () { + context('when the app name is over 128 bytes', function () { + const longString = new Array(300).join('a'); + const exactString = new Array(128 + 1).join('a'); + const options = { + appName: longString + }; + const metadata = makeClientMetadata(options); + + it('truncates the application name to 128 bytes', function () { + expect(metadata.application?.name).to.equal(exactString); + }); + }); + + context('when the app name is under 128 bytes', function () { + const options = { + appName: 'myApplication' + }; + const metadata = makeClientMetadata(options); + + it('sets the application name to the value', function () { + expect(metadata.application?.name).to.equal('myApplication'); + }); + }); + }); + }); + }); +}); From cf909bf1be54131de13878a72030f7800318af90 Mon Sep 17 00:00:00 2001 From: Bailey Pearson Date: Wed, 29 Mar 2023 12:15:45 -0400 Subject: [PATCH 02/12] fix metadata duplication --- src/cmap/connect.ts | 4 +- src/cmap/handshake/client_metadata.ts | 94 --------------------------- src/connection_string.ts | 5 +- src/mongo_client.ts | 1 + src/utils.ts | 6 +- 5 files changed, 6 insertions(+), 104 deletions(-) delete mode 100644 src/cmap/handshake/client_metadata.ts diff --git a/src/cmap/connect.ts b/src/cmap/connect.ts index 231adc53f12..7b2866adf2d 100644 --- a/src/cmap/connect.ts +++ b/src/cmap/connect.ts @@ -17,7 +17,7 @@ import { MongoRuntimeError, needsRetryableWriteLabel } from '../error'; -import { Callback, ClientMetadata, HostAddress, makeClientMetadata, ns } from '../utils'; +import { Callback, ClientMetadata, HostAddress, ns } from '../utils'; import { AuthContext, AuthProvider } from './auth/auth_provider'; import { GSSAPI } from './auth/gssapi'; import { MongoCR } from './auth/mongocr'; @@ -213,7 +213,7 @@ export async function prepareHandshakeDocument( const handshakeDoc: HandshakeDocument = { [serverApi?.version ? 'hello' : LEGACY_HELLO_COMMAND]: 1, helloOk: true, - client: options.metadata || makeClientMetadata(options), + client: options.metadata, compression: compressors }; diff --git a/src/cmap/handshake/client_metadata.ts b/src/cmap/handshake/client_metadata.ts deleted file mode 100644 index f52a75de073..00000000000 --- a/src/cmap/handshake/client_metadata.ts +++ /dev/null @@ -1,94 +0,0 @@ -import * as os from 'os'; - -import type { MongoOptions } from '../../mongo_client'; - -// eslint-disable-next-line @typescript-eslint/no-var-requires -const NODE_DRIVER_VERSION = require('../../../package.json').version; - -/** - * FaaS environment metadata. - * @public - */ -export interface FaasMetadata { - /** All metadata has a name */ - name: string; - /** Lambda/GCP/Vercel */ - region?: string; - /** Lambda/GCP */ - memoryMb?: string; - /** GCP */ - timeoutSec?: string; - /** Vercel */ - url?: string; -} - -/** @public */ -export interface ClientMetadata { - driver: { - name: string; - version: string; - }; - os: { - type: string; - name: NodeJS.Platform; - architecture: string; - version: string; - }; - platform: string; - version?: string; - application?: { - name: string; - }; - env?: FaasMetadata; -} - -/** @public */ -export interface ClientMetadataOptions { - driverInfo?: { - name?: string; - version?: string; - platform?: string; - }; - appName?: string; -} - -export function makeClientMetadata(options: MongoOptions): ClientMetadata { - const metadata: ClientMetadata = { - driver: { - name: 'nodejs', - version: NODE_DRIVER_VERSION - }, - os: { - type: os.type(), - name: process.platform, - architecture: process.arch, - version: os.release() - }, - platform: `Node.js ${process.version}, ${os.endianness()} (unified)` - }; - - // support optionally provided wrapping driver info - if (options.driverInfo) { - if (options.driverInfo.name) { - metadata.driver.name = `${metadata.driver.name}|${options.driverInfo.name}`; - } - - if (options.driverInfo.version) { - metadata.version = `${metadata.driver.version}|${options.driverInfo.version}`; - } - - if (options.driverInfo.platform) { - metadata.platform = `${metadata.platform}|${options.driverInfo.platform}`; - } - } - - if (options.appName) { - // MongoDB requires the appName not exceed a byte length of 128 - const buffer = Buffer.from(options.appName); - metadata.application = { - name: buffer.byteLength > 128 ? buffer.slice(0, 128).toString('utf8') : options.appName - }; - } - - return metadata; -} diff --git a/src/connection_string.ts b/src/connection_string.ts index cd59306518d..9b554372739 100644 --- a/src/connection_string.ts +++ b/src/connection_string.ts @@ -797,10 +797,7 @@ export const OPTIONS = { }, driverInfo: { default: {}, - transform({ values: [value] }) { - if (!isRecord(value)) throw new MongoParseError('DriverInfo must be an object'); - return value; - } + type: 'record' }, enableUtf8Validation: { type: 'boolean', default: true }, family: { diff --git a/src/mongo_client.ts b/src/mongo_client.ts index 7e73b4c9b5e..784ff6b9122 100644 --- a/src/mongo_client.ts +++ b/src/mongo_client.ts @@ -667,6 +667,7 @@ export interface MongoOptions extends Required< Pick< MongoClientOptions, + | 'appName' | 'autoEncryption' | 'connectTimeoutMS' | 'directConnection' diff --git a/src/utils.ts b/src/utils.ts index 4cbd88e7e4b..c59800e9b6f 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -20,7 +20,7 @@ import { MongoRuntimeError } from './error'; import type { Explain } from './explain'; -import type { MongoClient } from './mongo_client'; +import type { MongoClient, MongoOptions } from './mongo_client'; import type { CommandOperationOptions, OperationParent } from './operations/command'; import type { Hint, OperationOptions } from './operations/operation'; import { ReadConcern } from './read_concern'; @@ -545,9 +545,7 @@ export interface ClientMetadataOptions { // eslint-disable-next-line @typescript-eslint/no-var-requires const NODE_DRIVER_VERSION = require('../package.json').version; -export function makeClientMetadata(options?: ClientMetadataOptions): ClientMetadata { - options = options ?? {}; - +export function makeClientMetadata(options: MongoOptions): ClientMetadata { const metadata: ClientMetadata = { driver: { name: 'nodejs', From c55dcf24605bd68a55d85326ff0f410d6d132d80 Mon Sep 17 00:00:00 2001 From: Bailey Pearson Date: Wed, 29 Mar 2023 14:28:05 -0400 Subject: [PATCH 03/12] address comments --- src/cmap/auth/auth_provider.ts | 8 +- src/index.ts | 2 +- src/mongo_client.ts | 2 +- src/utils.ts | 49 +++--- test/tools/cmap_spec_runner.ts | 2 +- .../cmap/handshake/client_metadata.test.ts | 144 ++++++++++-------- test/unit/sdam/topology.test.js | 3 +- 7 files changed, 112 insertions(+), 98 deletions(-) diff --git a/src/cmap/auth/auth_provider.ts b/src/cmap/auth/auth_provider.ts index 98c93669f8b..37a47889b91 100644 --- a/src/cmap/auth/auth_provider.ts +++ b/src/cmap/auth/auth_provider.ts @@ -1,13 +1,9 @@ import type { Document } from '../../bson'; import { MongoRuntimeError } from '../../error'; -import type { ClientMetadataOptions } from '../../utils'; import type { HandshakeDocument } from '../connect'; import type { Connection, ConnectionOptions } from '../connection'; import type { MongoCredentials } from './mongo_credentials'; -/** @internal */ -export type AuthContextOptions = ConnectionOptions & ClientMetadataOptions; - /** * Context used during authentication * @internal @@ -20,7 +16,7 @@ export class AuthContext { /** If the context is for reauthentication. */ reauthenticating = false; /** The options passed to the `connect` method */ - options: AuthContextOptions; + options: ConnectionOptions; /** A response from an initial auth attempt, only some mechanisms use this (e.g, SCRAM) */ response?: Document; @@ -30,7 +26,7 @@ export class AuthContext { constructor( connection: Connection, credentials: MongoCredentials | undefined, - options: AuthContextOptions + options: ConnectionOptions ) { this.connection = connection; this.credentials = credentials; diff --git a/src/index.ts b/src/index.ts index 2ea5b261240..fac3c9c95ba 100644 --- a/src/index.ts +++ b/src/index.ts @@ -197,7 +197,7 @@ export type { ResumeToken, UpdateDescription } from './change_stream'; -export type { AuthContext, AuthContextOptions } from './cmap/auth/auth_provider'; +export type { AuthContext } from './cmap/auth/auth_provider'; export type { AuthMechanismProperties, MongoCredentials, diff --git a/src/mongo_client.ts b/src/mongo_client.ts index 784ff6b9122..885c980fbf9 100644 --- a/src/mongo_client.ts +++ b/src/mongo_client.ts @@ -667,7 +667,6 @@ export interface MongoOptions extends Required< Pick< MongoClientOptions, - | 'appName' | 'autoEncryption' | 'connectTimeoutMS' | 'directConnection' @@ -701,6 +700,7 @@ export interface MongoOptions > >, SupportedNodeConnectionOptions { + appName?: string; hosts: HostAddress[]; srvHost?: string; credentials?: MongoCredentials; diff --git a/src/utils.ts b/src/utils.ts index c59800e9b6f..be461df4a23 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -526,13 +526,18 @@ export interface ClientMetadata { version: string; }; platform: string; + /** */ version?: string; application?: { name: string; }; } -/** @public */ +/** + * @public + * + * @deprecated This type will be removed in the next major version. + */ export interface ClientMetadataOptions { driverInfo?: { name?: string; @@ -545,11 +550,21 @@ export interface ClientMetadataOptions { // eslint-disable-next-line @typescript-eslint/no-var-requires const NODE_DRIVER_VERSION = require('../package.json').version; -export function makeClientMetadata(options: MongoOptions): ClientMetadata { +export function makeClientMetadata( + options: Pick +): ClientMetadata { + const name = options.driverInfo.name ? `nodejs|${options.driverInfo.name}` : 'nodejs'; + const version = options.driverInfo.version + ? `${NODE_DRIVER_VERSION}|${options.driverInfo.version}` + : NODE_DRIVER_VERSION; + const platform = options.driverInfo.platform + ? `Node.js ${process.version}, ${os.endianness()} (unified)|${options.driverInfo.platform}` + : `Node.js ${process.version}, ${os.endianness()} (unified)`; + const metadata: ClientMetadata = { driver: { - name: 'nodejs', - version: NODE_DRIVER_VERSION + name, + version }, os: { type: os.type(), @@ -557,30 +572,16 @@ export function makeClientMetadata(options: MongoOptions): ClientMetadata { architecture: process.arch, version: os.release() }, - platform: `Node.js ${process.version}, ${os.endianness()} (unified)` + platform }; - // support optionally provided wrapping driver info - if (options.driverInfo) { - if (options.driverInfo.name) { - metadata.driver.name = `${metadata.driver.name}|${options.driverInfo.name}`; - } - - if (options.driverInfo.version) { - metadata.version = `${metadata.driver.version}|${options.driverInfo.version}`; - } - - if (options.driverInfo.platform) { - metadata.platform = `${metadata.platform}|${options.driverInfo.platform}`; - } - } - if (options.appName) { // MongoDB requires the appName not exceed a byte length of 128 - const buffer = Buffer.from(options.appName); - metadata.application = { - name: buffer.byteLength > 128 ? buffer.slice(0, 128).toString('utf8') : options.appName - }; + const name = + Buffer.byteLength(options.appName, 'utf8') <= 128 + ? options.appName + : Buffer.from(options.appName, 'utf8').subarray(0, 128).toString('utf8'); + metadata.application = { name }; } return metadata; diff --git a/test/tools/cmap_spec_runner.ts b/test/tools/cmap_spec_runner.ts index 3232c180aa4..1172b8cf804 100644 --- a/test/tools/cmap_spec_runner.ts +++ b/test/tools/cmap_spec_runner.ts @@ -372,7 +372,7 @@ async function runCmapTest(test: CmapTest, threadContext: ThreadContext) { let metadata; if (poolOptions.appName) { - metadata = makeClientMetadata({ appName: poolOptions.appName }); + metadata = makeClientMetadata({ appName: poolOptions.appName, driverInfo: {} }); delete poolOptions.appName; } diff --git a/test/unit/cmap/handshake/client_metadata.test.ts b/test/unit/cmap/handshake/client_metadata.test.ts index a0fb11de614..cb09f6f18bd 100644 --- a/test/unit/cmap/handshake/client_metadata.test.ts +++ b/test/unit/cmap/handshake/client_metadata.test.ts @@ -6,77 +6,93 @@ import { makeClientMetadata } from '../../../mongodb'; // eslint-disable-next-line @typescript-eslint/no-var-requires const NODE_DRIVER_VERSION = require('../../../../package.json').version; -describe('ClientMetadata [module]', function () { - describe('.makeClientMetadata', function () { - context('when options are provided', function () { - context('when driver info is provided', function () { - const options = { - driverInfo: { platform: 'myPlatform', name: 'myName', version: 'myVersion' } - }; - const metadata = makeClientMetadata(options); - - it('appends the driver info to the metadata', function () { - expect(metadata).to.deep.equal({ - driver: { - name: 'nodejs|myName', - version: NODE_DRIVER_VERSION - }, - os: { - type: os.type(), - name: process.platform, - architecture: process.arch, - version: os.release() - }, - platform: `Node.js ${process.version}, ${os.endianness()} (unified)|myPlatform`, - version: `${NODE_DRIVER_VERSION}|myVersion` - }); - }); - }); +describe('makeClientMetadata()', () => { + context('when driverInfo.platform is provided', () => { + it('appends driverInfo.platform to the platform field', () => { + const options = { + driverInfo: { platform: 'myPlatform' } + }; + const metadata = makeClientMetadata(options); + expect(metadata).to.have.property( + 'platform', + `Node.js ${process.version}, ${os.endianness()} (unified)|myPlatform` + ); + }); + }); + context('when driverInfo.name is provided', () => { + it('appends driverInfo.name to the driver.name field', () => { + const options = { + driverInfo: { name: 'myName' } + }; + const metadata = makeClientMetadata(options); + expect(metadata).to.have.nested.property('driver.name', `nodejs|myName`); + }); + }); + context('when driverInfo.version is provided', () => { + it('appends driverInfo.version to the version field', () => { + const options = { + driverInfo: { version: 'myVersion' } + }; + const metadata = makeClientMetadata(options); + expect(metadata).to.have.nested.property( + 'driver.version', + `${NODE_DRIVER_VERSION}|myVersion` + ); + }); + }); - context('when driver info is not provided', function () { - const metadata = makeClientMetadata({}); + context('when no custom driverInto is provided', () => { + const metadata = makeClientMetadata({ driverInfo: {} }); - it('does not append the driver info to the metadata', function () { - expect(metadata).to.deep.equal({ - driver: { - name: 'nodejs', - version: NODE_DRIVER_VERSION - }, - os: { - type: os.type(), - name: process.platform, - architecture: process.arch, - version: os.release() - }, - platform: `Node.js ${process.version}, ${os.endianness()} (unified)` - }); - }); + it('does not append the driver info to the metadata', () => { + expect(metadata).to.deep.equal({ + driver: { + name: 'nodejs', + version: NODE_DRIVER_VERSION + }, + os: { + type: os.type(), + name: process.platform, + architecture: process.arch, + version: os.release() + }, + platform: `Node.js ${process.version}, ${os.endianness()} (unified)` }); + }); - context('when app name is provided', function () { - context('when the app name is over 128 bytes', function () { - const longString = new Array(300).join('a'); - const exactString = new Array(128 + 1).join('a'); - const options = { - appName: longString - }; - const metadata = makeClientMetadata(options); + it('does not set the application field', () => { + expect(metadata).not.to.have.property('application'); + }); + }); - it('truncates the application name to 128 bytes', function () { - expect(metadata.application?.name).to.equal(exactString); - }); - }); + context('when app name is provided', () => { + context('when the app name is over 128 bytes', () => { + const longString = 'a'.repeat(300); + const exactString = 'a'.repeat(128); + const options = { + appName: longString, + driverInfo: {} + }; + const metadata = makeClientMetadata(options); + + it('truncates the application name to 128 bytes', () => { + expect(metadata.application?.name).to.equal(exactString); + // the above assertion fails if `metadata.application?.name` is undefined, so + // we can safely assert that it exists + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + expect(Buffer.byteLength(metadata.application!.name, 'utf8')).to.equal(128); + }); + }); - context('when the app name is under 128 bytes', function () { - const options = { - appName: 'myApplication' - }; - const metadata = makeClientMetadata(options); + context('when the app name is under 128 bytes', () => { + const options = { + appName: 'myApplication', + driverInfo: {} + }; + const metadata = makeClientMetadata(options); - it('sets the application name to the value', function () { - expect(metadata.application?.name).to.equal('myApplication'); - }); - }); + it('sets the application name to the value', () => { + expect(metadata.application?.name).to.equal('myApplication'); }); }); }); diff --git a/test/unit/sdam/topology.test.js b/test/unit/sdam/topology.test.js index a9896438e9c..f3d8a623824 100644 --- a/test/unit/sdam/topology.test.js +++ b/test/unit/sdam/topology.test.js @@ -37,7 +37,8 @@ describe('Topology (unit)', function () { it('should correctly pass appname', function (done) { const server = new Topology([`localhost:27017`], { metadata: makeClientMetadata({ - appName: 'My application name' + appName: 'My application name', + driverInfo: {} }) }); From 8cb32a57ba3328a2e9f03dd4690f512198563303 Mon Sep 17 00:00:00 2001 From: Bailey Pearson Date: Wed, 29 Mar 2023 14:34:51 -0400 Subject: [PATCH 04/12] fix client metadata interface --- src/utils.ts | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/utils.ts b/src/utils.ts index be461df4a23..f7fd92fc1ce 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -513,7 +513,10 @@ export function makeStateMachine(stateTable: StateTable): StateTransitionFunctio }; } -/** @public */ +/** + * @public + * https://github.com/mongodb/specifications/blob/master/source/mongodb-handshake/handshake.rst#hello-command + */ export interface ClientMetadata { driver: { name: string; @@ -526,8 +529,6 @@ export interface ClientMetadata { version: string; }; platform: string; - /** */ - version?: string; application?: { name: string; }; @@ -535,8 +536,6 @@ export interface ClientMetadata { /** * @public - * - * @deprecated This type will be removed in the next major version. */ export interface ClientMetadataOptions { driverInfo?: { From e7e4a8fdb985d7890c3639f829394cb6535867b3 Mon Sep 17 00:00:00 2001 From: Bailey Pearson Date: Wed, 29 Mar 2023 20:02:59 -0400 Subject: [PATCH 05/12] address comments --- src/utils.ts | 10 +++---- .../cmap/handshake/client_metadata.test.ts | 29 +++++++++++++++---- 2 files changed, 28 insertions(+), 11 deletions(-) diff --git a/src/utils.ts b/src/utils.ts index f7fd92fc1ce..dee4bc51ae3 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -515,7 +515,7 @@ export function makeStateMachine(stateTable: StateTable): StateTransitionFunctio /** * @public - * https://github.com/mongodb/specifications/blob/master/source/mongodb-handshake/handshake.rst#hello-command + * @see https://github.com/mongodb/specifications/blob/master/source/mongodb-handshake/handshake.rst#hello-command */ export interface ClientMetadata { driver: { @@ -534,9 +534,7 @@ export interface ClientMetadata { }; } -/** - * @public - */ +/** @public */ export interface ClientMetadataOptions { driverInfo?: { name?: string; @@ -557,8 +555,8 @@ export function makeClientMetadata( ? `${NODE_DRIVER_VERSION}|${options.driverInfo.version}` : NODE_DRIVER_VERSION; const platform = options.driverInfo.platform - ? `Node.js ${process.version}, ${os.endianness()} (unified)|${options.driverInfo.platform}` - : `Node.js ${process.version}, ${os.endianness()} (unified)`; + ? `Node.js ${process.version}, ${os.endianness()}|${options.driverInfo.platform}` + : `Node.js ${process.version}, ${os.endianness()}`; const metadata: ClientMetadata = { driver: { diff --git a/test/unit/cmap/handshake/client_metadata.test.ts b/test/unit/cmap/handshake/client_metadata.test.ts index cb09f6f18bd..2cb0f8cf7f4 100644 --- a/test/unit/cmap/handshake/client_metadata.test.ts +++ b/test/unit/cmap/handshake/client_metadata.test.ts @@ -15,7 +15,7 @@ describe('makeClientMetadata()', () => { const metadata = makeClientMetadata(options); expect(metadata).to.have.property( 'platform', - `Node.js ${process.version}, ${os.endianness()} (unified)|myPlatform` + `Node.js ${process.version}, ${os.endianness()}|myPlatform` ); }); }); @@ -56,7 +56,7 @@ describe('makeClientMetadata()', () => { architecture: process.arch, version: os.release() }, - platform: `Node.js ${process.version}, ${os.endianness()} (unified)` + platform: `Node.js ${process.version}, ${os.endianness()}` }); }); @@ -68,15 +68,14 @@ describe('makeClientMetadata()', () => { context('when app name is provided', () => { context('when the app name is over 128 bytes', () => { const longString = 'a'.repeat(300); - const exactString = 'a'.repeat(128); const options = { appName: longString, driverInfo: {} }; const metadata = makeClientMetadata(options); - it('truncates the application name to 128 bytes', () => { - expect(metadata.application?.name).to.equal(exactString); + it('truncates the application name to <=128 bytes', () => { + expect(metadata.application?.name).to.be.a('string'); // the above assertion fails if `metadata.application?.name` is undefined, so // we can safely assert that it exists // eslint-disable-next-line @typescript-eslint/no-non-null-assertion @@ -84,6 +83,26 @@ describe('makeClientMetadata()', () => { }); }); + context( + 'TODO(NODE-5150): fix appName truncation when multi-byte unicode charaters straddle byte 128', + () => { + const longString = '€'.repeat(300); + const options = { + appName: longString, + driverInfo: {} + }; + const metadata = makeClientMetadata(options); + + it('truncates the application name to 129 bytes', () => { + expect(metadata.application?.name).to.be.a('string'); + // the above assertion fails if `metadata.application?.name` is undefined, so + // we can safely assert that it exists + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + expect(Buffer.byteLength(metadata.application!.name, 'utf8')).to.equal(129); + }); + } + ); + context('when the app name is under 128 bytes', () => { const options = { appName: 'myApplication', From a9adccff8a2b2d46a2fea26eb1c6dc783958c223 Mon Sep 17 00:00:00 2001 From: Bailey Pearson Date: Thu, 30 Mar 2023 08:04:04 -0400 Subject: [PATCH 06/12] beef up tests --- .../cmap/handshake/client_metadata.test.ts | 50 +++++++++++++++---- 1 file changed, 41 insertions(+), 9 deletions(-) diff --git a/test/unit/cmap/handshake/client_metadata.test.ts b/test/unit/cmap/handshake/client_metadata.test.ts index 2cb0f8cf7f4..9fd720ec9bf 100644 --- a/test/unit/cmap/handshake/client_metadata.test.ts +++ b/test/unit/cmap/handshake/client_metadata.test.ts @@ -13,31 +13,63 @@ describe('makeClientMetadata()', () => { driverInfo: { platform: 'myPlatform' } }; const metadata = makeClientMetadata(options); - expect(metadata).to.have.property( - 'platform', - `Node.js ${process.version}, ${os.endianness()}|myPlatform` - ); + expect(metadata).to.deep.equal({ + driver: { + name: 'nodejs', + version: NODE_DRIVER_VERSION + }, + os: { + type: os.type(), + name: process.platform, + architecture: process.arch, + version: os.release() + }, + platform: `Node.js ${process.version}, ${os.endianness()}|myPlatform` + }); }); }); + context('when driverInfo.name is provided', () => { it('appends driverInfo.name to the driver.name field', () => { const options = { driverInfo: { name: 'myName' } }; const metadata = makeClientMetadata(options); - expect(metadata).to.have.nested.property('driver.name', `nodejs|myName`); + expect(metadata).to.deep.equal({ + driver: { + name: 'nodejs|myName', + version: NODE_DRIVER_VERSION + }, + os: { + type: os.type(), + name: process.platform, + architecture: process.arch, + version: os.release() + }, + platform: `Node.js ${process.version}, ${os.endianness()}` + }); }); }); + context('when driverInfo.version is provided', () => { it('appends driverInfo.version to the version field', () => { const options = { driverInfo: { version: 'myVersion' } }; const metadata = makeClientMetadata(options); - expect(metadata).to.have.nested.property( - 'driver.version', - `${NODE_DRIVER_VERSION}|myVersion` - ); + expect(metadata).to.deep.equal({ + driver: { + name: 'nodejs', + version: `${NODE_DRIVER_VERSION}|myVersion` + }, + os: { + type: os.type(), + name: process.platform, + architecture: process.arch, + version: os.release() + }, + platform: `Node.js ${process.version}, ${os.endianness()}` + }); }); }); From e6af1b1404acd9fffe9643aaabe8d0bad84568c6 Mon Sep 17 00:00:00 2001 From: Bailey Pearson Date: Thu, 30 Mar 2023 10:31:23 -0400 Subject: [PATCH 07/12] fix topology test --- test/unit/sdam/topology.test.js | 23 +++++++++-------------- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/test/unit/sdam/topology.test.js b/test/unit/sdam/topology.test.js index f3d8a623824..8f9dd6e9849 100644 --- a/test/unit/sdam/topology.test.js +++ b/test/unit/sdam/topology.test.js @@ -46,7 +46,7 @@ describe('Topology (unit)', function () { done(); }); - it('should report the correct platform in client metadata', function (done) { + it('should report the correct platform in client metadata', async function () { const helloRequests = []; mockServer.setMessageHandler(request => { const doc = request.document; @@ -59,22 +59,17 @@ describe('Topology (unit)', function () { }); client = new MongoClient(`mongodb://${mockServer.uri()}/`); - client.connect(err => { - expect(err).to.not.exist; - client.db().command({ ping: 1 }, err => { - expect(err).to.not.exist; + await client.connect(); - expect(helloRequests).to.have.length.greaterThan(1); - helloRequests.forEach(helloRequest => - expect(helloRequest) - .nested.property('client.platform') - .to.match(/unified/) - ); + await client.db().command({ ping: 1 }); - done(); - }); - }); + expect(helloRequests).to.have.length.greaterThan(1); + for (const request of helloRequests) { + expect(request) + .nested.property('client.platform') + .to.match(/Node.js /); + } }); }); From 847249442a8e71a8a19fdbfa19b726457e2f7ab0 Mon Sep 17 00:00:00 2001 From: Bailey Pearson Date: Thu, 30 Mar 2023 17:36:06 -0400 Subject: [PATCH 08/12] fix cmap tests --- test/tools/cmap_spec_runner.ts | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/test/tools/cmap_spec_runner.ts b/test/tools/cmap_spec_runner.ts index 1172b8cf804..7f21a8bc34e 100644 --- a/test/tools/cmap_spec_runner.ts +++ b/test/tools/cmap_spec_runner.ts @@ -370,11 +370,8 @@ async function runCmapTest(test: CmapTest, threadContext: ThreadContext) { delete poolOptions.backgroundThreadIntervalMS; } - let metadata; - if (poolOptions.appName) { - metadata = makeClientMetadata({ appName: poolOptions.appName, driverInfo: {} }); - delete poolOptions.appName; - } + const metadata = makeClientMetadata({ appName: poolOptions.appName, driverInfo: {} }); + delete poolOptions.appName; const operations = test.operations; const expectedError = test.error; From eddc01e7dd6dd91f209249e4f65df43956b06b69 Mon Sep 17 00:00:00 2001 From: Bailey Pearson Date: Fri, 31 Mar 2023 10:12:10 -0400 Subject: [PATCH 09/12] fix tests --- .../connection.test.ts | 27 +++++++++++-------- .../node-specific/topology.test.js | 5 +++- 2 files changed, 20 insertions(+), 12 deletions(-) diff --git a/test/integration/connection-monitoring-and-pooling/connection.test.ts b/test/integration/connection-monitoring-and-pooling/connection.test.ts index b13ee006b33..4ee973ea00b 100644 --- a/test/integration/connection-monitoring-and-pooling/connection.test.ts +++ b/test/integration/connection-monitoring-and-pooling/connection.test.ts @@ -3,7 +3,9 @@ import { expect } from 'chai'; import { connect, Connection, + ConnectionOptions, LEGACY_HELLO_COMMAND, + makeClientMetadata, MongoClient, MongoServerError, ns, @@ -31,12 +33,13 @@ describe('Connection', function () { it('should execute a command against a server', { metadata: { requires: { apiVersion: false, topology: '!load-balanced' } }, test: function (done) { - const connectOptions = Object.assign( - { connectionType: Connection }, - this.configuration.options - ); + const connectOptions: Partial = { + connectionType: Connection, + ...this.configuration.options, + metadata: makeClientMetadata({ driverInfo: {} }) + }; - connect(connectOptions, (err, conn) => { + connect(connectOptions as any as ConnectionOptions, (err, conn) => { expect(err).to.not.exist; this.defer(_done => conn.destroy(_done)); @@ -53,12 +56,14 @@ describe('Connection', function () { it('should emit command monitoring events', { metadata: { requires: { apiVersion: false, topology: '!load-balanced' } }, test: function (done) { - const connectOptions = Object.assign( - { connectionType: Connection, monitorCommands: true }, - this.configuration.options - ); - - connect(connectOptions, (err, conn) => { + const connectOptions: Partial = { + connectionType: Connection, + monitorCommands: true, + ...this.configuration.options, + metadata: makeClientMetadata({ driverInfo: {} }) + }; + + connect(connectOptions as any as ConnectionOptions, (err, conn) => { expect(err).to.not.exist; this.defer(_done => conn.destroy(_done)); diff --git a/test/integration/node-specific/topology.test.js b/test/integration/node-specific/topology.test.js index 912c1443c49..ee4393efcd6 100644 --- a/test/integration/node-specific/topology.test.js +++ b/test/integration/node-specific/topology.test.js @@ -1,11 +1,14 @@ 'use strict'; const { expect } = require('chai'); +const { makeClientMetadata } = require('../../mongodb'); describe('Topology', function () { it('should correctly track states of a topology', { metadata: { requires: { apiVersion: false, topology: '!load-balanced' } }, // apiVersion not supported by newTopology() test: function (done) { - const topology = this.configuration.newTopology(); + const topology = this.configuration.newTopology({ + metadata: makeClientMetadata({ driverInfo: {} }) + }); const states = []; topology.on('stateChanged', (_, newState) => states.push(newState)); From bf59a94a08fe4965d85ece144209ce40ad82465f Mon Sep 17 00:00:00 2001 From: Bailey Pearson Date: Fri, 31 Mar 2023 14:09:07 -0400 Subject: [PATCH 10/12] chore: fix noauth test --- .../connection.test.ts | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/test/integration/connection-monitoring-and-pooling/connection.test.ts b/test/integration/connection-monitoring-and-pooling/connection.test.ts index 4ee973ea00b..2b86f9bd9e8 100644 --- a/test/integration/connection-monitoring-and-pooling/connection.test.ts +++ b/test/integration/connection-monitoring-and-pooling/connection.test.ts @@ -83,18 +83,19 @@ describe('Connection', function () { } }); - it('should support calling back multiple times on exhaust commands', { + it.only('should support calling back multiple times on exhaust commands', { metadata: { requires: { apiVersion: false, mongodb: '>=4.2.0', topology: ['single'] } }, test: function (done) { const namespace = ns(`${this.configuration.db}.$cmd`); - const connectOptions = Object.assign( - { connectionType: Connection }, - this.configuration.options - ); + const connectOptions: Partial = { + connectionType: Connection, + ...this.configuration.options, + metadata: makeClientMetadata({ driverInfo: {} }) + }; - connect(connectOptions, (err, conn) => { + connect(connectOptions as any as ConnectionOptions, (err, conn) => { expect(err).to.not.exist; this.defer(_done => conn.destroy(_done)); From 357cb5d89a81f8c11ae328f7aaa869deba1b8fb0 Mon Sep 17 00:00:00 2001 From: Bailey Pearson Date: Fri, 31 Mar 2023 14:41:53 -0400 Subject: [PATCH 11/12] Update test/unit/cmap/handshake/client_metadata.test.ts Co-authored-by: Warren James --- test/unit/cmap/handshake/client_metadata.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unit/cmap/handshake/client_metadata.test.ts b/test/unit/cmap/handshake/client_metadata.test.ts index 9fd720ec9bf..f75c9cecfe5 100644 --- a/test/unit/cmap/handshake/client_metadata.test.ts +++ b/test/unit/cmap/handshake/client_metadata.test.ts @@ -73,7 +73,7 @@ describe('makeClientMetadata()', () => { }); }); - context('when no custom driverInto is provided', () => { + context('when no custom driverInfo is provided', () => { const metadata = makeClientMetadata({ driverInfo: {} }); it('does not append the driver info to the metadata', () => { From 5f2e2297a305e6c82187b31d7c71c638ed9bfab3 Mon Sep 17 00:00:00 2001 From: Bailey Pearson Date: Fri, 31 Mar 2023 14:42:38 -0400 Subject: [PATCH 12/12] remove .only --- .../connection-monitoring-and-pooling/connection.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/connection-monitoring-and-pooling/connection.test.ts b/test/integration/connection-monitoring-and-pooling/connection.test.ts index 2b86f9bd9e8..7ad5bb5c598 100644 --- a/test/integration/connection-monitoring-and-pooling/connection.test.ts +++ b/test/integration/connection-monitoring-and-pooling/connection.test.ts @@ -83,7 +83,7 @@ describe('Connection', function () { } }); - it.only('should support calling back multiple times on exhaust commands', { + it('should support calling back multiple times on exhaust commands', { metadata: { requires: { apiVersion: false, mongodb: '>=4.2.0', topology: ['single'] } },