From a2fd0cf5ddcae48c98937a400d3083ce3b85a965 Mon Sep 17 00:00:00 2001 From: Nikola Irinchev Date: Mon, 5 May 2025 17:14:02 +0200 Subject: [PATCH 1/6] Update connection string app name if not present --- package-lock.json | 2 + package.json | 2 + src/common/atlas/apiClient.ts | 2 +- src/helpers/connectionOptions.ts | 27 ++++++ src/{ => helpers}/deferred-promise.ts | 0 src/{ => helpers}/packageInfo.ts | 2 +- src/index.ts | 2 +- src/server.ts | 6 +- src/session.ts | 14 ++- src/telemetry/constants.ts | 2 +- src/telemetry/telemetry.ts | 2 +- src/tools/atlas/metadata/connectCluster.ts | 2 +- src/tools/mongodb/mongodbTool.ts | 2 +- src/types/mongodb-connection-string-url.d.ts | 69 ++++++++++++++ tests/unit/deferred-promise.test.ts | 2 +- tests/unit/telemetry.test.ts | 96 ++++++++++++++++++++ 16 files changed, 222 insertions(+), 10 deletions(-) create mode 100644 src/helpers/connectionOptions.ts rename src/{ => helpers}/deferred-promise.ts (100%) rename src/{ => helpers}/packageInfo.ts (61%) create mode 100644 src/types/mongodb-connection-string-url.d.ts diff --git a/package-lock.json b/package-lock.json index e71274d2..2a910a4a 100644 --- a/package-lock.json +++ b/package-lock.json @@ -15,6 +15,8 @@ "bson": "^6.10.3", "lru-cache": "^11.1.0", "mongodb": "^6.15.0", + "mongodb-build-info": "^1.7.2", + "mongodb-connection-string-url": "^3.0.2", "mongodb-log-writer": "^2.4.1", "mongodb-redact": "^1.1.6", "mongodb-schema": "^12.6.2", diff --git a/package.json b/package.json index 6e77412f..5720fc47 100644 --- a/package.json +++ b/package.json @@ -66,6 +66,8 @@ "bson": "^6.10.3", "lru-cache": "^11.1.0", "mongodb": "^6.15.0", + "mongodb-build-info": "^1.7.2", + "mongodb-connection-string-url": "^3.0.2", "mongodb-log-writer": "^2.4.1", "mongodb-redact": "^1.1.6", "mongodb-schema": "^12.6.2", diff --git a/src/common/atlas/apiClient.ts b/src/common/atlas/apiClient.ts index 7f74f578..13272127 100644 --- a/src/common/atlas/apiClient.ts +++ b/src/common/atlas/apiClient.ts @@ -4,7 +4,7 @@ import { AccessToken, ClientCredentials } from "simple-oauth2"; import { ApiClientError } from "./apiClientError.js"; import { paths, operations } from "./openapi.js"; import { CommonProperties, TelemetryEvent } from "../../telemetry/types.js"; -import { packageInfo } from "../../packageInfo.js"; +import { packageInfo } from "../../helpers/packageInfo.js"; const ATLAS_API_VERSION = "2025-03-12"; diff --git a/src/helpers/connectionOptions.ts b/src/helpers/connectionOptions.ts new file mode 100644 index 00000000..0ad4024c --- /dev/null +++ b/src/helpers/connectionOptions.ts @@ -0,0 +1,27 @@ +import { MongoClientOptions } from "mongodb"; +import ConnectionString from "mongodb-connection-string-url"; +import { isAtlas } from "mongodb-build-info"; + +export function setAppNameParamIfMissing({ + connectionString, + defaultAppName, + telemetryAnonymousId, +}: { + connectionString: string; + defaultAppName?: string; + telemetryAnonymousId?: string; +}): string { + const connectionStringUrl = new ConnectionString(connectionString); + + const searchParams = connectionStringUrl.typedSearchParams(); + + if (!searchParams.has("appName") && defaultAppName !== undefined) { + const appName = isAtlas(connectionString) + ? `${defaultAppName}${telemetryAnonymousId ? `-${telemetryAnonymousId}` : ""}` + : defaultAppName; + + searchParams.set("appName", appName); + } + + return connectionStringUrl.toString(); +} diff --git a/src/deferred-promise.ts b/src/helpers/deferred-promise.ts similarity index 100% rename from src/deferred-promise.ts rename to src/helpers/deferred-promise.ts diff --git a/src/packageInfo.ts b/src/helpers/packageInfo.ts similarity index 61% rename from src/packageInfo.ts rename to src/helpers/packageInfo.ts index dea9214b..6c075dc0 100644 --- a/src/packageInfo.ts +++ b/src/helpers/packageInfo.ts @@ -1,4 +1,4 @@ -import packageJson from "../package.json" with { type: "json" }; +import packageJson from "../../package.json" with { type: "json" }; export const packageInfo = { version: packageJson.version, diff --git a/src/index.ts b/src/index.ts index 20a60e53..f91db447 100644 --- a/src/index.ts +++ b/src/index.ts @@ -6,7 +6,7 @@ import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; import { config } from "./config.js"; import { Session } from "./session.js"; import { Server } from "./server.js"; -import { packageInfo } from "./packageInfo.js"; +import { packageInfo } from "./helpers/packageInfo.js"; import { Telemetry } from "./telemetry/telemetry.js"; try { diff --git a/src/server.ts b/src/server.ts index 76f73826..30493e62 100644 --- a/src/server.ts +++ b/src/server.ts @@ -187,7 +187,11 @@ export class Server { if (this.userConfig.connectionString) { try { - await this.session.connectToMongoDB(this.userConfig.connectionString, this.userConfig.connectOptions); + await this.session.connectToMongoDB( + this.userConfig.connectionString, + this.userConfig.connectOptions, + this.telemetry + ); } catch (error) { console.error( "Failed to connect to MongoDB instance using the connection string from the config: ", diff --git a/src/session.ts b/src/session.ts index 6f219c41..fd6504b6 100644 --- a/src/session.ts +++ b/src/session.ts @@ -4,6 +4,9 @@ import { Implementation } from "@modelcontextprotocol/sdk/types.js"; import logger, { LogId } from "./logger.js"; import EventEmitter from "events"; import { ConnectOptions } from "./config.js"; +import { setAppNameParamIfMissing } from "./helpers/connectionOptions.js"; +import { packageInfo } from "./helpers/packageInfo.js"; +import { Telemetry } from "./telemetry/telemetry.js"; export interface SessionOptions { apiBaseUrl: string; @@ -97,7 +100,16 @@ export class Session extends EventEmitter<{ this.emit("close"); } - async connectToMongoDB(connectionString: string, connectOptions: ConnectOptions): Promise { + async connectToMongoDB( + connectionString: string, + connectOptions: ConnectOptions, + telemetry: Telemetry + ): Promise { + connectionString = setAppNameParamIfMissing({ + connectionString, + defaultAppName: `${packageInfo.mcpServerName} ${packageInfo.version}`, + telemetryAnonymousId: await telemetry.deviceIdPromise, + }); const provider = await NodeDriverServiceProvider.connect(connectionString, { productDocsLink: "https://docs.mongodb.com/todo-mcp", productName: "MongoDB MCP", diff --git a/src/telemetry/constants.ts b/src/telemetry/constants.ts index 998f6e24..9dd1cc76 100644 --- a/src/telemetry/constants.ts +++ b/src/telemetry/constants.ts @@ -1,4 +1,4 @@ -import { packageInfo } from "../packageInfo.js"; +import { packageInfo } from "../helpers/packageInfo.js"; import { type CommonStaticProperties } from "./types.js"; /** diff --git a/src/telemetry/telemetry.ts b/src/telemetry/telemetry.ts index 30a0363b..5670b3a4 100644 --- a/src/telemetry/telemetry.ts +++ b/src/telemetry/telemetry.ts @@ -7,7 +7,7 @@ import { MACHINE_METADATA } from "./constants.js"; import { EventCache } from "./eventCache.js"; import { createHmac } from "crypto"; import nodeMachineId from "node-machine-id"; -import { DeferredPromise } from "../deferred-promise.js"; +import { DeferredPromise } from "../helpers/deferred-promise.js"; type EventResult = { success: boolean; diff --git a/src/tools/atlas/metadata/connectCluster.ts b/src/tools/atlas/metadata/connectCluster.ts index 8280406a..a3059c00 100644 --- a/src/tools/atlas/metadata/connectCluster.ts +++ b/src/tools/atlas/metadata/connectCluster.ts @@ -99,7 +99,7 @@ export class ConnectClusterTool extends AtlasToolBase { for (let i = 0; i < 20; i++) { try { - await this.session.connectToMongoDB(connectionString, this.config.connectOptions); + await this.session.connectToMongoDB(connectionString, this.config.connectOptions, this.telemetry); lastError = undefined; break; } catch (err: unknown) { diff --git a/src/tools/mongodb/mongodbTool.ts b/src/tools/mongodb/mongodbTool.ts index 2ef1aee0..079dd117 100644 --- a/src/tools/mongodb/mongodbTool.ts +++ b/src/tools/mongodb/mongodbTool.ts @@ -71,7 +71,7 @@ export abstract class MongoDBToolBase extends ToolBase { } protected connectToMongoDB(connectionString: string): Promise { - return this.session.connectToMongoDB(connectionString, this.config.connectOptions); + return this.session.connectToMongoDB(connectionString, this.config.connectOptions, this.telemetry); } protected resolveTelemetryMetadata( diff --git a/src/types/mongodb-connection-string-url.d.ts b/src/types/mongodb-connection-string-url.d.ts new file mode 100644 index 00000000..01a0cff2 --- /dev/null +++ b/src/types/mongodb-connection-string-url.d.ts @@ -0,0 +1,69 @@ +declare module "mongodb-connection-string-url" { + import { URL } from "whatwg-url"; + import { redactConnectionString, ConnectionStringRedactionOptions } from "./redact"; + export { redactConnectionString, ConnectionStringRedactionOptions }; + declare class CaseInsensitiveMap extends Map { + delete(name: K): boolean; + get(name: K): string | undefined; + has(name: K): boolean; + set(name: K, value: any): this; + _normalizeKey(name: any): K; + } + declare abstract class URLWithoutHost extends URL { + abstract get host(): never; + abstract set host(value: never); + abstract get hostname(): never; + abstract set hostname(value: never); + abstract get port(): never; + abstract set port(value: never); + abstract get href(): string; + abstract set href(value: string); + } + export interface ConnectionStringParsingOptions { + looseValidation?: boolean; + } + export declare class ConnectionString extends URLWithoutHost { + _hosts: string[]; + constructor(uri: string, options?: ConnectionStringParsingOptions); + get host(): never; + set host(_ignored: never); + get hostname(): never; + set hostname(_ignored: never); + get port(): never; + set port(_ignored: never); + get href(): string; + set href(_ignored: string); + get isSRV(): boolean; + get hosts(): string[]; + set hosts(list: string[]); + toString(): string; + clone(): ConnectionString; + redact(options?: ConnectionStringRedactionOptions): ConnectionString; + typedSearchParams(): { + append(name: keyof T & string, value: any): void; + delete(name: keyof T & string): void; + get(name: keyof T & string): string | null; + getAll(name: keyof T & string): string[]; + has(name: keyof T & string): boolean; + set(name: keyof T & string, value: any): void; + keys(): IterableIterator; + values(): IterableIterator; + entries(): IterableIterator<[keyof T & string, string]>; + _normalizeKey(name: keyof T & string): string; + [Symbol.iterator](): IterableIterator<[keyof T & string, string]>; + sort(): void; + forEach( + callback: (this: THIS_ARG, value: string, name: string, searchParams: any) => void, + thisArg?: THIS_ARG | undefined + ): void; + readonly [Symbol.toStringTag]: "URLSearchParams"; + }; + } + export declare class CommaAndColonSeparatedRecord< + K extends {} = Record, + > extends CaseInsensitiveMap { + constructor(from?: string | null); + toString(): string; + } + export default ConnectionString; +} diff --git a/tests/unit/deferred-promise.test.ts b/tests/unit/deferred-promise.test.ts index c6011af1..5fdaba7d 100644 --- a/tests/unit/deferred-promise.test.ts +++ b/tests/unit/deferred-promise.test.ts @@ -1,4 +1,4 @@ -import { DeferredPromise } from "../../src/deferred-promise.js"; +import { DeferredPromise } from "../../src/helpers/deferred-promise.js"; import { jest } from "@jest/globals"; describe("DeferredPromise", () => { diff --git a/tests/unit/telemetry.test.ts b/tests/unit/telemetry.test.ts index 969a4ee8..e79d471a 100644 --- a/tests/unit/telemetry.test.ts +++ b/tests/unit/telemetry.test.ts @@ -7,6 +7,10 @@ import { config } from "../../src/config.js"; import { jest } from "@jest/globals"; import logger, { LogId } from "../../src/logger.js"; import { createHmac } from "crypto"; +import { NodeDriverServiceProvider } from "@mongosh/service-provider-node-driver"; + +jest.mock("@mongosh/service-provider-node-driver"); +const MockNodeDriverServiceProvider = NodeDriverServiceProvider as jest.MockedClass; // Mock the ApiClient to avoid real API calls jest.mock("../../src/common/atlas/apiClient.js"); @@ -315,4 +319,96 @@ describe("Telemetry", () => { }); }); }); + + describe("connectToMongoDB", () => { + beforeEach(() => { + session = new Session({ + apiClientId: "test-client-id", + apiBaseUrl: "https://api.test.com", + }); + + MockNodeDriverServiceProvider.connect = jest.fn(() => + Promise.resolve({} as unknown as NodeDriverServiceProvider) + ); + }); + + const testCases: { + connectionString: string; + isAtlas: boolean; + expectAppName: boolean; + expectTelemetryId: boolean; + name: string; + disableTelemetry?: boolean; + }[] = [ + { + connectionString: "mongodb://localhost:27017", + isAtlas: false, + expectAppName: true, + expectTelemetryId: false, + name: "local db", + }, + { + connectionString: "mongodb+srv://test.mongodb.net/test?retryWrites=true&w=majority", + isAtlas: true, + expectAppName: true, + expectTelemetryId: true, + name: "atlas db", + }, + { + connectionString: "mongodb://localhost:27017?appName=CustomAppName", + isAtlas: false, + expectAppName: false, + expectTelemetryId: false, + name: "local db with custom appName", + }, + { + connectionString: + "mongodb+srv://test.mongodb.net/test?retryWrites=true&w=majority&appName=CustomAppName", + isAtlas: true, + expectAppName: false, + expectTelemetryId: false, + name: "atlas db with custom appName", + }, + + { + connectionString: "mongodb+srv://test.mongodb.net/test?retryWrites=true&w=majority", + isAtlas: true, + expectAppName: true, + expectTelemetryId: false, + name: "atlas db with telemetry disabled", + disableTelemetry: true, + }, + ]; + + for (const testCase of testCases) { + it(`should update connection string for ${testCase.name}`, async () => { + if (testCase.disableTelemetry) { + config.telemetry = "disabled"; + telemetry = Telemetry.create(session, config, { + getRawMachineId: () => Promise.resolve(machineId), + }); + } + await session.connectToMongoDB(testCase.connectionString, config.connectOptions, telemetry); + expect(session.serviceProvider).toBeDefined(); + expect(MockNodeDriverServiceProvider.connect).toHaveBeenCalledOnce(); + const connectionString = MockNodeDriverServiceProvider.connect.mock.calls[0][0]; + if (testCase.expectAppName) { + expect(connectionString).toContain("appName=MongoDB+MCP+Server"); + } else { + expect(connectionString).not.toContain("appName=MongoDB+MCP+Server"); + } + + if (testCase.disableTelemetry) { + expect(connectionString).not.toMatch(/appName=[^-]*-[^&]*/); + } else { + const telemetryId = await telemetry.deviceIdPromise; + if (testCase.isAtlas && testCase.expectTelemetryId) { + expect(connectionString).toContain(telemetryId); + } else { + expect(connectionString).not.toContain(telemetryId); + } + } + }); + } + }); }); From d53ba26d2e400324fd5a7e79181bbdaa4c879f19 Mon Sep 17 00:00:00 2001 From: Nikola Irinchev Date: Mon, 5 May 2025 17:36:49 +0200 Subject: [PATCH 2/6] fix lint --- eslint.config.js | 1 + src/telemetry/telemetry.ts | 1 - tests/integration/telemetry.test.ts | 1 - tests/unit/telemetry.test.ts | 7 +++++-- 4 files changed, 6 insertions(+), 4 deletions(-) diff --git a/eslint.config.js b/eslint.config.js index b42518a5..e6dd1af0 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -49,6 +49,7 @@ export default defineConfig([ "global.d.ts", "eslint.config.js", "jest.config.ts", + "src/types/*.d.ts", ]), eslintPluginPrettierRecommended, ]); diff --git a/src/telemetry/telemetry.ts b/src/telemetry/telemetry.ts index 5670b3a4..5f8554e6 100644 --- a/src/telemetry/telemetry.ts +++ b/src/telemetry/telemetry.ts @@ -40,7 +40,6 @@ export class Telemetry { commonProperties = { ...MACHINE_METADATA }, eventCache = EventCache.getInstance(), - // eslint-disable-next-line @typescript-eslint/no-unsafe-return, @typescript-eslint/no-unsafe-call, @typescript-eslint/no-unsafe-member-access getRawMachineId = () => nodeMachineId.machineId(true), }: { eventCache?: EventCache; diff --git a/tests/integration/telemetry.test.ts b/tests/integration/telemetry.test.ts index fe8e51ff..522c1154 100644 --- a/tests/integration/telemetry.test.ts +++ b/tests/integration/telemetry.test.ts @@ -6,7 +6,6 @@ import nodeMachineId from "node-machine-id"; describe("Telemetry", () => { it("should resolve the actual machine ID", async () => { - // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment, @typescript-eslint/no-unsafe-call, @typescript-eslint/no-unsafe-member-access const actualId: string = await nodeMachineId.machineId(true); const actualHashedId = createHmac("sha256", actualId.toUpperCase()).update("atlascli").digest("hex"); diff --git a/tests/unit/telemetry.test.ts b/tests/unit/telemetry.test.ts index e79d471a..603a6874 100644 --- a/tests/unit/telemetry.test.ts +++ b/tests/unit/telemetry.test.ts @@ -390,8 +390,11 @@ describe("Telemetry", () => { } await session.connectToMongoDB(testCase.connectionString, config.connectOptions, telemetry); expect(session.serviceProvider).toBeDefined(); - expect(MockNodeDriverServiceProvider.connect).toHaveBeenCalledOnce(); - const connectionString = MockNodeDriverServiceProvider.connect.mock.calls[0][0]; + + // eslint-disable-next-line @typescript-eslint/unbound-method + const connectMock = MockNodeDriverServiceProvider.connect as jest.Mock; + expect(connectMock).toHaveBeenCalledOnce(); + const connectionString = connectMock.mock.calls[0][0]; if (testCase.expectAppName) { expect(connectionString).toContain("appName=MongoDB+MCP+Server"); } else { From 52c8c0c9cb520fecae8c522760b397a8aff3a7b0 Mon Sep 17 00:00:00 2001 From: Nikola Irinchev Date: Mon, 5 May 2025 17:52:52 +0200 Subject: [PATCH 3/6] fix tests --- tests/unit/telemetry.test.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tests/unit/telemetry.test.ts b/tests/unit/telemetry.test.ts index 603a6874..4fb81d10 100644 --- a/tests/unit/telemetry.test.ts +++ b/tests/unit/telemetry.test.ts @@ -307,7 +307,7 @@ describe("Telemetry", () => { }); afterEach(() => { - process.env.DO_NOT_TRACK = originalEnv; + delete process.env.DO_NOT_TRACK; }); it("should not send events", async () => { @@ -332,6 +332,10 @@ describe("Telemetry", () => { ); }); + afterEach(() => { + config.telemetry = "enabled"; + }); + const testCases: { connectionString: string; isAtlas: boolean; @@ -388,6 +392,7 @@ describe("Telemetry", () => { getRawMachineId: () => Promise.resolve(machineId), }); } + await session.connectToMongoDB(testCase.connectionString, config.connectOptions, telemetry); expect(session.serviceProvider).toBeDefined(); From 4da79adc945a25500f813138990c5654287d9cd0 Mon Sep 17 00:00:00 2001 From: Nikola Irinchev Date: Tue, 6 May 2025 13:29:37 +0200 Subject: [PATCH 4/6] Restore original DO_NOT_TRACK variable. --- tests/unit/telemetry.test.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/unit/telemetry.test.ts b/tests/unit/telemetry.test.ts index 4fb81d10..8818a3ce 100644 --- a/tests/unit/telemetry.test.ts +++ b/tests/unit/telemetry.test.ts @@ -307,7 +307,11 @@ describe("Telemetry", () => { }); afterEach(() => { - delete process.env.DO_NOT_TRACK; + if (originalEnv) { + process.env.DO_NOT_TRACK = originalEnv; + } else { + delete process.env.DO_NOT_TRACK; + } }); it("should not send events", async () => { From 8cab5a1065e1e106fa478ec37fa37a20fa1c750d Mon Sep 17 00:00:00 2001 From: Nikola Irinchev Date: Tue, 6 May 2025 14:42:35 +0200 Subject: [PATCH 5/6] Remove telemetry anonymous id for now --- package-lock.json | 1 - package.json | 1 - src/helpers/connectionOptions.ts | 9 +- src/server.ts | 6 +- src/session.ts | 8 +- src/tools/atlas/metadata/connectCluster.ts | 2 +- src/tools/mongodb/mongodbTool.ts | 2 +- tests/unit/session.test.ts | 66 +++++++++++++ tests/unit/telemetry.test.ts | 104 --------------------- 9 files changed, 71 insertions(+), 128 deletions(-) create mode 100644 tests/unit/session.test.ts diff --git a/package-lock.json b/package-lock.json index 98e40e9a..9d01e564 100644 --- a/package-lock.json +++ b/package-lock.json @@ -15,7 +15,6 @@ "bson": "^6.10.3", "lru-cache": "^11.1.0", "mongodb": "^6.15.0", - "mongodb-build-info": "^1.7.2", "mongodb-connection-string-url": "^3.0.2", "mongodb-log-writer": "^2.4.1", "mongodb-redact": "^1.1.6", diff --git a/package.json b/package.json index 5720fc47..d8ce1f40 100644 --- a/package.json +++ b/package.json @@ -66,7 +66,6 @@ "bson": "^6.10.3", "lru-cache": "^11.1.0", "mongodb": "^6.15.0", - "mongodb-build-info": "^1.7.2", "mongodb-connection-string-url": "^3.0.2", "mongodb-log-writer": "^2.4.1", "mongodb-redact": "^1.1.6", diff --git a/src/helpers/connectionOptions.ts b/src/helpers/connectionOptions.ts index 0ad4024c..10b1ecc8 100644 --- a/src/helpers/connectionOptions.ts +++ b/src/helpers/connectionOptions.ts @@ -1,26 +1,19 @@ import { MongoClientOptions } from "mongodb"; import ConnectionString from "mongodb-connection-string-url"; -import { isAtlas } from "mongodb-build-info"; export function setAppNameParamIfMissing({ connectionString, defaultAppName, - telemetryAnonymousId, }: { connectionString: string; defaultAppName?: string; - telemetryAnonymousId?: string; }): string { const connectionStringUrl = new ConnectionString(connectionString); const searchParams = connectionStringUrl.typedSearchParams(); if (!searchParams.has("appName") && defaultAppName !== undefined) { - const appName = isAtlas(connectionString) - ? `${defaultAppName}${telemetryAnonymousId ? `-${telemetryAnonymousId}` : ""}` - : defaultAppName; - - searchParams.set("appName", appName); + searchParams.set("appName", defaultAppName); } return connectionStringUrl.toString(); diff --git a/src/server.ts b/src/server.ts index 30493e62..76f73826 100644 --- a/src/server.ts +++ b/src/server.ts @@ -187,11 +187,7 @@ export class Server { if (this.userConfig.connectionString) { try { - await this.session.connectToMongoDB( - this.userConfig.connectionString, - this.userConfig.connectOptions, - this.telemetry - ); + await this.session.connectToMongoDB(this.userConfig.connectionString, this.userConfig.connectOptions); } catch (error) { console.error( "Failed to connect to MongoDB instance using the connection string from the config: ", diff --git a/src/session.ts b/src/session.ts index fd6504b6..a7acabb1 100644 --- a/src/session.ts +++ b/src/session.ts @@ -6,7 +6,6 @@ import EventEmitter from "events"; import { ConnectOptions } from "./config.js"; import { setAppNameParamIfMissing } from "./helpers/connectionOptions.js"; import { packageInfo } from "./helpers/packageInfo.js"; -import { Telemetry } from "./telemetry/telemetry.js"; export interface SessionOptions { apiBaseUrl: string; @@ -100,15 +99,10 @@ export class Session extends EventEmitter<{ this.emit("close"); } - async connectToMongoDB( - connectionString: string, - connectOptions: ConnectOptions, - telemetry: Telemetry - ): Promise { + async connectToMongoDB(connectionString: string, connectOptions: ConnectOptions): Promise { connectionString = setAppNameParamIfMissing({ connectionString, defaultAppName: `${packageInfo.mcpServerName} ${packageInfo.version}`, - telemetryAnonymousId: await telemetry.deviceIdPromise, }); const provider = await NodeDriverServiceProvider.connect(connectionString, { productDocsLink: "https://docs.mongodb.com/todo-mcp", diff --git a/src/tools/atlas/metadata/connectCluster.ts b/src/tools/atlas/metadata/connectCluster.ts index a3059c00..8280406a 100644 --- a/src/tools/atlas/metadata/connectCluster.ts +++ b/src/tools/atlas/metadata/connectCluster.ts @@ -99,7 +99,7 @@ export class ConnectClusterTool extends AtlasToolBase { for (let i = 0; i < 20; i++) { try { - await this.session.connectToMongoDB(connectionString, this.config.connectOptions, this.telemetry); + await this.session.connectToMongoDB(connectionString, this.config.connectOptions); lastError = undefined; break; } catch (err: unknown) { diff --git a/src/tools/mongodb/mongodbTool.ts b/src/tools/mongodb/mongodbTool.ts index 079dd117..2ef1aee0 100644 --- a/src/tools/mongodb/mongodbTool.ts +++ b/src/tools/mongodb/mongodbTool.ts @@ -71,7 +71,7 @@ export abstract class MongoDBToolBase extends ToolBase { } protected connectToMongoDB(connectionString: string): Promise { - return this.session.connectToMongoDB(connectionString, this.config.connectOptions, this.telemetry); + return this.session.connectToMongoDB(connectionString, this.config.connectOptions); } protected resolveTelemetryMetadata( diff --git a/tests/unit/session.test.ts b/tests/unit/session.test.ts new file mode 100644 index 00000000..b43e46b9 --- /dev/null +++ b/tests/unit/session.test.ts @@ -0,0 +1,66 @@ +import { jest } from "@jest/globals"; +import { NodeDriverServiceProvider } from "@mongosh/service-provider-node-driver"; +import { Session } from "../../src/session.js"; +import { config } from "../../src/config.js"; + +jest.mock("@mongosh/service-provider-node-driver"); +const MockNodeDriverServiceProvider = NodeDriverServiceProvider as jest.MockedClass; + +describe("Session", () => { + let session: Session; + beforeEach(() => { + session = new Session({ + apiClientId: "test-client-id", + apiBaseUrl: "https://api.test.com", + }); + + MockNodeDriverServiceProvider.connect = jest.fn(() => + Promise.resolve({} as unknown as NodeDriverServiceProvider) + ); + }); + + describe("connectToMongoDB", () => { + const testCases: { + connectionString: string; + expectAppName: boolean; + name: string; + }[] = [ + { + connectionString: "mongodb://localhost:27017", + expectAppName: true, + name: "db without appName", + }, + { + connectionString: "mongodb://localhost:27017?appName=CustomAppName", + expectAppName: false, + name: "db with custom appName", + }, + { + connectionString: + "mongodb+srv://test.mongodb.net/test?retryWrites=true&w=majority&appName=CustomAppName", + expectAppName: false, + name: "atlas db with custom appName", + }, + ]; + + for (const testCase of testCases) { + it(`should update connection string for ${testCase.name}`, async () => { + await session.connectToMongoDB(testCase.connectionString, config.connectOptions); + expect(session.serviceProvider).toBeDefined(); + + // eslint-disable-next-line @typescript-eslint/unbound-method + const connectMock = MockNodeDriverServiceProvider.connect as jest.Mock< + ReturnType, + Parameters + >; + expect(connectMock).toHaveBeenCalledOnce(); + const connectionString = connectMock.mock.calls[0][0]; + if (testCase.expectAppName) { + expect(connectionString).toContain("appName=MongoDB+MCP+Server"); + } else { + expect(connectionString).not.toContain("appName=MongoDB+MCP+Server"); + } + }); + } + }); +}); diff --git a/tests/unit/telemetry.test.ts b/tests/unit/telemetry.test.ts index 8818a3ce..c1ae28ea 100644 --- a/tests/unit/telemetry.test.ts +++ b/tests/unit/telemetry.test.ts @@ -7,10 +7,6 @@ import { config } from "../../src/config.js"; import { jest } from "@jest/globals"; import logger, { LogId } from "../../src/logger.js"; import { createHmac } from "crypto"; -import { NodeDriverServiceProvider } from "@mongosh/service-provider-node-driver"; - -jest.mock("@mongosh/service-provider-node-driver"); -const MockNodeDriverServiceProvider = NodeDriverServiceProvider as jest.MockedClass; // Mock the ApiClient to avoid real API calls jest.mock("../../src/common/atlas/apiClient.js"); @@ -323,104 +319,4 @@ describe("Telemetry", () => { }); }); }); - - describe("connectToMongoDB", () => { - beforeEach(() => { - session = new Session({ - apiClientId: "test-client-id", - apiBaseUrl: "https://api.test.com", - }); - - MockNodeDriverServiceProvider.connect = jest.fn(() => - Promise.resolve({} as unknown as NodeDriverServiceProvider) - ); - }); - - afterEach(() => { - config.telemetry = "enabled"; - }); - - const testCases: { - connectionString: string; - isAtlas: boolean; - expectAppName: boolean; - expectTelemetryId: boolean; - name: string; - disableTelemetry?: boolean; - }[] = [ - { - connectionString: "mongodb://localhost:27017", - isAtlas: false, - expectAppName: true, - expectTelemetryId: false, - name: "local db", - }, - { - connectionString: "mongodb+srv://test.mongodb.net/test?retryWrites=true&w=majority", - isAtlas: true, - expectAppName: true, - expectTelemetryId: true, - name: "atlas db", - }, - { - connectionString: "mongodb://localhost:27017?appName=CustomAppName", - isAtlas: false, - expectAppName: false, - expectTelemetryId: false, - name: "local db with custom appName", - }, - { - connectionString: - "mongodb+srv://test.mongodb.net/test?retryWrites=true&w=majority&appName=CustomAppName", - isAtlas: true, - expectAppName: false, - expectTelemetryId: false, - name: "atlas db with custom appName", - }, - - { - connectionString: "mongodb+srv://test.mongodb.net/test?retryWrites=true&w=majority", - isAtlas: true, - expectAppName: true, - expectTelemetryId: false, - name: "atlas db with telemetry disabled", - disableTelemetry: true, - }, - ]; - - for (const testCase of testCases) { - it(`should update connection string for ${testCase.name}`, async () => { - if (testCase.disableTelemetry) { - config.telemetry = "disabled"; - telemetry = Telemetry.create(session, config, { - getRawMachineId: () => Promise.resolve(machineId), - }); - } - - await session.connectToMongoDB(testCase.connectionString, config.connectOptions, telemetry); - expect(session.serviceProvider).toBeDefined(); - - // eslint-disable-next-line @typescript-eslint/unbound-method - const connectMock = MockNodeDriverServiceProvider.connect as jest.Mock; - expect(connectMock).toHaveBeenCalledOnce(); - const connectionString = connectMock.mock.calls[0][0]; - if (testCase.expectAppName) { - expect(connectionString).toContain("appName=MongoDB+MCP+Server"); - } else { - expect(connectionString).not.toContain("appName=MongoDB+MCP+Server"); - } - - if (testCase.disableTelemetry) { - expect(connectionString).not.toMatch(/appName=[^-]*-[^&]*/); - } else { - const telemetryId = await telemetry.deviceIdPromise; - if (testCase.isAtlas && testCase.expectTelemetryId) { - expect(connectionString).toContain(telemetryId); - } else { - expect(connectionString).not.toContain(telemetryId); - } - } - }); - } - }); }); From aea9657488941b183c64563b4f6abcabfb9933de Mon Sep 17 00:00:00 2001 From: Nikola Irinchev Date: Tue, 6 May 2025 15:22:58 +0200 Subject: [PATCH 6/6] fix generic args --- tests/unit/session.test.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/unit/session.test.ts b/tests/unit/session.test.ts index b43e46b9..f60feca1 100644 --- a/tests/unit/session.test.ts +++ b/tests/unit/session.test.ts @@ -50,8 +50,7 @@ describe("Session", () => { // eslint-disable-next-line @typescript-eslint/unbound-method const connectMock = MockNodeDriverServiceProvider.connect as jest.Mock< - ReturnType, - Parameters + typeof NodeDriverServiceProvider.connect >; expect(connectMock).toHaveBeenCalledOnce(); const connectionString = connectMock.mock.calls[0][0];