added tests

added tests - no docker tests removed extraneous export and newline removed extraneous export and newline removed unnecesary helper funcs removed unnecesary let, changed to const added more tests to comply w kickoff - no docker tests still cleared up logic
mongodb · Feb 29, 2024 · 7140454 · 7140454
1 parent 7c4ce27
commit 7140454
Show file tree

Hide file tree

Showing 8 changed files with 82 additions and 70 deletions.
diff --git a/src/cmap/connect.ts b/src/cmap/connect.ts
@@ -25,7 +25,7 @@ import {
   type ConnectionOptions,
   CryptoConnection
 } from './connection';
-import { addAllEnvClientMetadata, type ClientMetadata } from './handshake/client_metadata';
+import { addContainerMetadata } from './handshake/client_metadata';
 import {
   MAX_SUPPORTED_SERVER_VERSION,
   MAX_SUPPORTED_WIRE_VERSION,
@@ -183,7 +183,7 @@ export interface HandshakeDocument extends Document {
   ismaster?: boolean;
   hello?: boolean;
   helloOk?: boolean;
-  client: ClientMetadata;
+  client: Document;
   compression: string[];
   saslSupportedMechs?: string;
   loadBalanced?: boolean;
@@ -200,11 +200,12 @@ export async function prepareHandshakeDocument(
   const options = authContext.options;
   const compressors = options.compressors ? options.compressors : [];
   const { serverApi } = authContext.connection;
-  const clientMetadata = await addAllEnvClientMetadata(options.internalMetadata);
+  const clientMetadata = await addContainerMetadata(options.metadata);
+
   const handshakeDoc: HandshakeDocument = {
     [serverApi?.version || options.loadBalanced === true ? 'hello' : LEGACY_HELLO_COMMAND]: 1,
     helloOk: true,
-    client: clientMetadata.toObject(),
+    client: clientMetadata,
     compression: compressors
   };
 

diff --git a/src/cmap/connection.ts b/src/cmap/connection.ts
@@ -57,7 +57,7 @@ import {
   type WriteProtocolMessageType
 } from './commands';
 import type { Stream } from './connect';
-import type { ClientMetadata, LimitedSizeDocument } from './handshake/client_metadata';
+import type { ClientMetadata } from './handshake/client_metadata';
 import { StreamDescription, type StreamDescriptionOptions } from './stream_description';
 import { type CompressorName, decompressResponse } from './wire_protocol/compression';
 import { onData } from './wire_protocol/on_data';
@@ -119,8 +119,6 @@ export interface ConnectionOptions
   cancellationToken?: CancellationToken;
   metadata: ClientMetadata;
   /** @internal */
-  internalMetadata: LimitedSizeDocument;
-  /** @internal */
   mongoLogger?: MongoLogger | undefined;
 }
 

diff --git a/src/cmap/handshake/client_metadata.ts b/src/cmap/handshake/client_metadata.ts
@@ -2,7 +2,7 @@ import { promises as fs } from 'fs';
 import * as os from 'os';
 import * as process from 'process';
 
-import { BSON, Int32 } from '../../bson';
+import { BSON, type Document, Int32 } from '../../bson';
 import { MongoInvalidArgumentError } from '../../error';
 import type { MongoOptions } from '../../mongo_client';
 
@@ -72,13 +72,13 @@ export class LimitedSizeDocument {
     return true;
   }
 
-  toObject(): ClientMetadata {
+  toObject(): Document {
     return BSON.deserialize(BSON.serialize(this.document), {
       promoteLongs: false,
       promoteBuffers: false,
       promoteValues: false,
       useBigInt64: false
-    }) as ClientMetadata;
+    });
   }
 }
 
@@ -92,17 +92,8 @@ type MakeClientMetadataOptions = Pick<MongoOptions, 'appName' | 'driverInfo'>;
  * 4. Truncate `platform`. -- special we do not truncate this field
  */
 export function makeClientMetadata(options: MakeClientMetadataOptions): ClientMetadata {
-  let metadataDocument = new LimitedSizeDocument(512);
-  metadataDocument = addNonEnvClientMetadata(options, metadataDocument);
-  metadataDocument = addFAASOnlyEnvClientMetadata(metadataDocument);
-  return metadataDocument.toObject();
-}
+  const metadataDocument = new LimitedSizeDocument(512);
 
-/** @internal */
-export function addNonEnvClientMetadata(
-  options: MakeClientMetadataOptions,
-  metadataDocument: LimitedSizeDocument
-): LimitedSizeDocument {
   const { appName = '' } = options;
   // Add app name first, it must be sent
   if (appName.length > 0) {
@@ -152,11 +143,6 @@ export function addNonEnvClientMetadata(
     }
   }
 
-  return metadataDocument;
-}
-
-/** @internal */
-function addFAASOnlyEnvClientMetadata(metadataDocument: LimitedSizeDocument): LimitedSizeDocument {
   const faasEnv = getFAASEnv();
   if (faasEnv != null) {
     if (!metadataDocument.ifItFitsItSits('env', faasEnv)) {
@@ -167,15 +153,13 @@ function addFAASOnlyEnvClientMetadata(metadataDocument: LimitedSizeDocument): Li
       }
     }
   }
-  return metadataDocument;
+  return metadataDocument.toObject() as ClientMetadata;
 }
 
 let isDocker: boolean;
 let dockerPromise: any;
 /** @internal */
-export async function addAllEnvClientMetadata(metadataDocument: LimitedSizeDocument) {
-  const faasEnv = getFAASEnv();
-
+export async function addContainerMetadata(originalMetadata: ClientMetadata) {
   async function getContainerMetadata() {
     const containerMetadata: Record<string, any> = {};
     if (isDocker == null) {
@@ -189,30 +173,33 @@ export async function addAllEnvClientMetadata(metadataDocument: LimitedSizeDocum
     }
     const isKubernetes = process.env.KUBERNETES_SERVICE_HOST ? true : false;
 
-    if (isDocker || isKubernetes) {
-      if (isDocker) {
-        containerMetadata['runtime'] = 'docker';
-      }
-      if (isKubernetes) {
-        containerMetadata['orchestrator'] = 'kubernetes';
-      }
-    }
+    if (isDocker) containerMetadata['runtime'] = 'docker';
+    if (isKubernetes) containerMetadata['orchestrator'] = 'kubernetes';
+
     return containerMetadata;
   }
 
   const containerMetadata = await getContainerMetadata();
-  const envMetadata = faasEnv ?? new Map();
-  envMetadata.set('container', containerMetadata);
-  if (envMetadata != null) {
-    if (!metadataDocument.ifItFitsItSits('env', envMetadata)) {
-      for (const key of envMetadata.keys()) {
-        envMetadata.delete(key);
-        if (envMetadata.size === 0) break;
-        if (metadataDocument.ifItFitsItSits('env', envMetadata)) break;
+  if (Object.keys(containerMetadata).length === 0) return originalMetadata;
+
+  const extendedMetadata = new LimitedSizeDocument(512);
+  const envMetadata = { ...originalMetadata?.env, container: containerMetadata };
+
+  for (const [key, val] of Object.entries(originalMetadata)) {
+    if (key !== 'env') {
+      extendedMetadata.ifItFitsItSits(key, val);
+    } else {
+      if (!extendedMetadata.ifItFitsItSits('env', envMetadata)) {
+        extendedMetadata.ifItFitsItSits('env', val);
       }
     }
   }
-  return metadataDocument;
+
+  if (!('env' in originalMetadata)) {
+    extendedMetadata.ifItFitsItSits('env', envMetadata);
+  }
+
+  return extendedMetadata.toObject();
 }
 
 /**

diff --git a/src/connection_string.ts b/src/connection_string.ts
@@ -5,7 +5,7 @@ import { URLSearchParams } from 'url';
 import type { Document } from './bson';
 import { MongoCredentials } from './cmap/auth/mongo_credentials';
 import { AUTH_MECHS_AUTH_SRC_EXTERNAL, AuthMechanism } from './cmap/auth/providers';
-import { LimitedSizeDocument, addNonEnvClientMetadata, makeClientMetadata } from './cmap/handshake/client_metadata';
+import { makeClientMetadata } from './cmap/handshake/client_metadata';
 import { Compressor, type CompressorName } from './cmap/wire_protocol/compression';
 import { Encrypter } from './encrypter';
 import {
@@ -544,7 +544,6 @@ export function parseOptions(
   );
 
   mongoOptions.metadata = makeClientMetadata(mongoOptions);
-  mongoOptions.internalMetadata = addNonEnvClientMetadata(mongoOptions, new LimitedSizeDocument(512));
 
   return mongoOptions;
 }

diff --git a/src/index.ts b/src/index.ts
@@ -287,11 +287,7 @@ export type {
   WaitQueueMember,
   WithConnectionCallback
 } from './cmap/connection_pool';
-export type {
-  ClientMetadata,
-  ClientMetadataOptions,
-  LimitedSizeDocument
-} from './cmap/handshake/client_metadata';
+export type { ClientMetadata, ClientMetadataOptions } from './cmap/handshake/client_metadata';
 export type { ConnectionPoolMetrics } from './cmap/metrics';
 export type { StreamDescription, StreamDescriptionOptions } from './cmap/stream_description';
 export type { CompressorName } from './cmap/wire_protocol/compression';

diff --git a/src/mongo_client.ts b/src/mongo_client.ts
@@ -14,7 +14,7 @@ import {
 import { AuthMechanism } from './cmap/auth/providers';
 import type { LEGAL_TCP_SOCKET_OPTIONS, LEGAL_TLS_SOCKET_OPTIONS } from './cmap/connect';
 import type { Connection } from './cmap/connection';
-import type { ClientMetadata, LimitedSizeDocument } from './cmap/handshake/client_metadata';
+import type { ClientMetadata } from './cmap/handshake/client_metadata';
 import type { CompressorName } from './cmap/wire_protocol/compression';
 import { parseOptions, resolveSRVRecord } from './connection_string';
 import { MONGO_CLIENT_EVENTS } from './constants';
@@ -897,11 +897,4 @@ export interface MongoOptions
    * TODO: NODE-5671 - remove internal flag
    */
   mongodbLogPath?: 'stderr' | 'stdout' | MongoDBLogWritable;
-
-  /**
-   * @internal
-   * Metadata as BSON bytes.
-   * Does not contain any environment information.
-   */
-  internalMetadata: LimitedSizeDocument;
 }
diff --git a/src/sdam/topology.ts b/src/sdam/topology.ts
@@ -4,7 +4,7 @@ import type { BSONSerializeOptions, Document } from '../bson';
 import type { MongoCredentials } from '../cmap/auth/mongo_credentials';
 import type { ConnectionEvents, DestroyOptions } from '../cmap/connection';
 import type { CloseOptions, ConnectionPoolEvents } from '../cmap/connection_pool';
-import type { ClientMetadata, LimitedSizeDocument } from '../cmap/handshake/client_metadata';
+import type { ClientMetadata } from '../cmap/handshake/client_metadata';
 import { DEFAULT_OPTIONS, FEATURE_FLAGS } from '../connection_string';
 import {
   CLOSE,
@@ -158,7 +158,6 @@ export interface TopologyOptions extends BSONSerializeOptions, ServerOptions {
   directConnection: boolean;
   loadBalanced: boolean;
   metadata: ClientMetadata;
-  internalMetadata: LimitedSizeDocument;
   serverMonitoringMode: ServerMonitoringMode;
   /** MongoDB server API version */
   serverApi?: ServerApi;

diff --git a/test/unit/cmap/connect.test.ts b/test/unit/cmap/connect.test.ts
@@ -26,7 +26,7 @@ const CONNECT_DEFAULTS = {
   loadBalanced: false
 };
 
-describe('Connect Tests', function () {
+describe('Connect Tests', async function () {
   context('when PLAIN auth enabled', () => {
     const test: {
       server?: any;
@@ -185,9 +185,48 @@ describe('Connect Tests', function () {
     expect(error).to.be.instanceOf(MongoNetworkError);
   });
 
-  context('prepareHandshakeDocument', () => {
+  context('prepareHandshakeDocument', async () => {
+    context('when container is present', async () => {
+      const authContext = {
+        connection: {},
+        options: { ...CONNECT_DEFAULTS }
+      };
+
+      context('when only kubernetes is present', async () => {
+        beforeEach(() => {
+          process.env.KUBERNETES_SERVICE_HOST = 'I exist';
+        });
+
+        afterEach(() => {
+          process.env.KUBERNETES_SERVICE_HOST = '';
+        });
+
+        it(`should include { orchestrator: 'kubernetes'} in client.env.container`, async () => {
+          const handshakeDocument = await prepareHandshakeDocument(authContext);
+          expect(handshakeDocument.client.env.container.orchestrator).to.equal('kubernetes');
+        });
+
+        it(`should not have 'name' property in client.env `, async () => {
+          const handshakeDocument = await prepareHandshakeDocument(authContext);
+          expect(handshakeDocument.client.env).to.not.have.property('name');
+        });
+      });
+    });
+
+    context('when container nor FAAS env is not present', async () => {
+      const authContext = {
+        connection: {},
+        options: { ...CONNECT_DEFAULTS }
+      };
+
+      it(`should not have 'env' property in client`, async () => {
+        const handshakeDocument = await prepareHandshakeDocument(authContext);
+        expect(handshakeDocument.client).to.not.have.property('env');
+      });
+    });
+
     context('when serverApi.version is present', () => {
-      const options = {};
+      const options = { ...CONNECT_DEFAULTS };
       const authContext = {
         connection: { serverApi: { version: '1' } },
         options
@@ -200,7 +239,7 @@ describe('Connect Tests', function () {
     });
 
     context('when serverApi is not present', () => {
-      const options = {};
+      const options = { ...CONNECT_DEFAULTS };
       const authContext = {
         connection: {},
         options
@@ -216,7 +255,7 @@ describe('Connect Tests', function () {
       context('when loadBalanced is not set as an option', () => {
         const authContext = {
           connection: {},
-          options: {}
+          options: { ...CONNECT_DEFAULTS }
         };
 
         it('does not set loadBalanced on the handshake document', async () => {
@@ -238,7 +277,7 @@ describe('Connect Tests', function () {
       context('when loadBalanced is set to false', () => {
         const authContext = {
           connection: {},
-          options: { loadBalanced: false }
+          options: { ...CONNECT_DEFAULTS, loadBalanced: false }
         };
 
         it('does not set loadBalanced on the handshake document', async () => {
@@ -260,7 +299,7 @@ describe('Connect Tests', function () {
       context('when loadBalanced is set to true', () => {
         const authContext = {
           connection: {},
-          options: { loadBalanced: true }
+          options: { ...CONNECT_DEFAULTS, loadBalanced: true }
         };
 
         it('sets loadBalanced on the handshake document', async () => {