Skip to content

Commit 93c27ec

Browse files
blvablva
committed
chore: add arg validation test coverage
1 parent dd36b1a commit 93c27ec

File tree

8 files changed

+183
-93
lines changed

8 files changed

+183
-93
lines changed

‎src/tools/mongodb/mongodbTool.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,11 @@ import type { CallToolResult } from "@modelcontextprotocol/sdk/types.js";
66
import { ErrorCodes, MongoDBError } from "../../common/errors.js";
77
import { LogId } from "../../common/logger.js";
88
import type { Server } from "../../server.js";
9+
import { CommonArgs } from "../args.js";
910

1011
export const DbOperationArgs = {
11-
database: z.string().describe("Database name"),
12-
collection: z.string().describe("Collection name"),
12+
database: CommonArgs.string().describe("Database name"),
13+
collection: CommonArgs.string().describe("Collection name"),
1314
};
1415

1516
export abstract class MongoDBToolBase extends ToolBase {

‎tests/integration/helpers.ts‎

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,8 +96,10 @@ export function setupIntegrationTest(
9696
keychain: new Keychain(),
9797
});
9898

99-
// Mock hasValidAccessToken for tests
99+
// Mock API Client for tests
100100
if (!userConfig.apiClientId && !userConfig.apiClientSecret) {
101+
userConfig.apiClientId = "test";
102+
userConfig.apiClientSecret = "test";
101103
const mockFn = vi.fn().mockResolvedValue(true);
102104
session.apiClient.validateAccessToken = mockFn;
103105
}
@@ -235,6 +237,29 @@ export const databaseCollectionParameters: ParameterInfo[] = [
235237
{ name: "collection", type: "string", description: "Collection name", required: true },
236238
];
237239

240+
export const projectIdParameters: ParameterInfo[] = [
241+
{ name: "projectId", type: "string", description: "Atlas project ID", required: true },
242+
];
243+
244+
export const createClusterParameters: ParameterInfo[] = [
245+
{ name: "name", type: "string", description: "Name of the cluster", required: true },
246+
{ name: "projectId", type: "string", description: "Atlas project ID to create the cluster in", required: true },
247+
{ name: "region", type: "string", description: "Region of the cluster", required: false },
248+
];
249+
250+
export const createDbUserParameters: ParameterInfo[] = [
251+
{
252+
name: "projectId",
253+
type: "string",
254+
description: "Atlas project ID to create the database user in",
255+
required: true,
256+
},
257+
{ name: "username", type: "string", description: "Username of the database user", required: true },
258+
{ name: "password", type: "string", description: "Password of the database user", required: false },
259+
{ name: "roles", type: "array", description: "Roles of the database user", required: true },
260+
{ name: "scopes", type: "array", description: "Scopes of the database user", required: false },
261+
];
262+
238263
export const databaseCollectionInvalidArgs = [
239264
{},
240265
{ database: "test" },
@@ -245,6 +270,14 @@ export const databaseCollectionInvalidArgs = [
245270
{ database: "test", collection: [] },
246271
];
247272

273+
export const projectIdInvalidArgs = [
274+
{},
275+
{ projectId: 123 },
276+
{ projectId: [] },
277+
{ projectId: "!✅invalid" },
278+
{ projectId: "invalid-test-project-id" },
279+
];
280+
248281
export const databaseInvalidArgs = [{}, { database: 123 }, { database: [] }];
249282

250283
export function validateToolMetadata(

‎tests/integration/tools/atlas/alerts.test.ts‎

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,24 @@
1-
import { expectDefined, getResponseElements } from "../../helpers.js";
1+
import {
2+
getResponseElements,
3+
projectIdInvalidArgs,
4+
validateThrowsForInvalidArguments,
5+
validateToolMetadata,
6+
} from "../../helpers.js";
27
import { parseTable, describeWithAtlas, withProject } from "./atlasHelpers.js";
3-
import { expect, it } from "vitest";
8+
import { expect, it, describe } from "vitest";
49

510
describeWithAtlas("atlas-list-alerts", (integration) => {
6-
it("should have correct metadata", async () => {
7-
const { tools } = await integration.mcpClient().listTools();
8-
const listAlerts = tools.find((tool) => tool.name === "atlas-list-alerts");
9-
expectDefined(listAlerts);
10-
expect(listAlerts.inputSchema.type).toBe("object");
11-
expectDefined(listAlerts.inputSchema.properties);
12-
expect(listAlerts.inputSchema.properties).toHaveProperty("projectId");
11+
describe("should have correct metadata and validate invalid arguments", () => {
12+
validateToolMetadata(integration, "atlas-list-alerts", "List MongoDB Atlas alerts", [
13+
{
14+
name: "projectId",
15+
type: "string",
16+
description: "Atlas project ID to list alerts for",
17+
required: true,
18+
},
19+
]);
20+
21+
validateThrowsForInvalidArguments(integration, "atlas-list-alerts", projectIdInvalidArgs);
1322
});
1423

1524
withProject(integration, ({ getProjectId }) => {

‎tests/integration/tools/atlas/atlasHelpers.ts‎

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,7 @@ import { afterAll, beforeAll, describe } from "vitest";
99
export type IntegrationTestFunction = (integration: IntegrationTest) => void;
1010

1111
export function describeWithAtlas(name: string, fn: IntegrationTestFunction): void {
12-
const describeFn =
13-
!process.env.MDB_MCP_API_CLIENT_ID?.length || !process.env.MDB_MCP_API_CLIENT_SECRET?.length
14-
? describe.skip
15-
: describe;
16-
describeFn(name, () => {
12+
describe(name, () => {
1713
const integration = setupIntegrationTest(
1814
() => ({
1915
...defaultTestConfig,
@@ -34,8 +30,23 @@ interface ProjectTestArgs {
3430

3531
type ProjectTestFunction = (args: ProjectTestArgs) => void;
3632

33+
export function withCredentials(integration: IntegrationTest, fn: IntegrationTestFunction): SuiteCollector<object> {
34+
const describeFn =
35+
!process.env.MDB_MCP_API_CLIENT_ID?.length || !process.env.MDB_MCP_API_CLIENT_SECRET?.length
36+
? describe.skip
37+
: describe;
38+
return describeFn("with credentials", () => {
39+
fn(integration);
40+
});
41+
}
42+
3743
export function withProject(integration: IntegrationTest, fn: ProjectTestFunction): SuiteCollector<object> {
38-
return describe("with project", () => {
44+
const describeFn =
45+
!process.env.MDB_MCP_API_CLIENT_ID?.length || !process.env.MDB_MCP_API_CLIENT_SECRET?.length
46+
? describe.skip
47+
: describe;
48+
49+
return describeFn("with project", () => {
3950
let projectId: string = "";
4051
let ipAddress: string = "";
4152

‎tests/integration/tools/atlas/clusters.test.ts‎

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,13 @@
11
import type { Session } from "../../../../src/common/session.js";
2-
import { expectDefined, getDataFromUntrustedContent, getResponseElements } from "../../helpers.js";
2+
import {
3+
expectDefined,
4+
getResponseElements,
5+
getDataFromUntrustedContent,
6+
createClusterParameters,
7+
projectIdInvalidArgs,
8+
validateThrowsForInvalidArguments,
9+
validateToolMetadata,
10+
} from "../../helpers.js";
311
import { describeWithAtlas, withProject, randomId, parseTable } from "./atlasHelpers.js";
412
import type { ClusterDescription20240805 } from "../../../../src/common/atlas/openapi.js";
513
import { afterAll, beforeAll, describe, expect, it } from "vitest";
@@ -57,6 +65,19 @@ async function waitCluster(
5765
}
5866

5967
describeWithAtlas("clusters", (integration) => {
68+
describe("should have correct metadata and validate invalid arguments", () => {
69+
validateToolMetadata(
70+
integration,
71+
"atlas-create-free-cluster",
72+
"Create a free MongoDB Atlas cluster",
73+
createClusterParameters
74+
);
75+
76+
expect(() => {
77+
validateThrowsForInvalidArguments(integration, "atlas-create-free-cluster", projectIdInvalidArgs);
78+
}).not.toThrow();
79+
});
80+
6081
withProject(integration, ({ getProjectId, getIpAddress }) => {
6182
const clusterName = "ClusterTest-" + randomId;
6283

‎tests/integration/tools/atlas/dbUsers.test.ts‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,21 @@
11
import { describeWithAtlas, withProject, randomId } from "./atlasHelpers.js";
2-
import { expectDefined, getResponseElements } from "../../helpers.js";
2+
import {
3+
expectDefined,
4+
getResponseElements,
5+
projectIdInvalidArgs,
6+
validateThrowsForInvalidArguments,
7+
validateToolMetadata,
8+
} from "../../helpers.js";
39
import { ApiClientError } from "../../../../src/common/atlas/apiClientError.js";
410
import { afterEach, beforeEach, describe, expect, it } from "vitest";
511
import { Keychain } from "../../../../src/common/keychain.js";
612

713
describeWithAtlas("db users", (integration) => {
14+
describe("should have correct metadata and validate invalid arguments", () => {
15+
validateToolMetadata(integration, "atlas-create-db-user", "Create a database user", createDbUserParameters);
16+
validateThrowsForInvalidArguments(integration, "atlas-create-db-user", projectIdInvalidArgs);
17+
});
18+
819
withProject(integration, ({ getProjectId }) => {
920
let userName: string;
1021
beforeEach(() => {

‎tests/integration/tools/atlas/orgs.test.ts‎

Lines changed: 17 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,25 @@
11
import { expectDefined, getDataFromUntrustedContent, getResponseElements } from "../../helpers.js";
2-
import { parseTable, describeWithAtlas } from "./atlasHelpers.js";
2+
import { parseTable, describeWithAtlas, withCredentials } from "./atlasHelpers.js";
33
import { describe, expect, it } from "vitest";
44

55
describeWithAtlas("orgs", (integration) => {
6-
describe("atlas-list-orgs", () => {
7-
it("should have correct metadata", async () => {
8-
const { tools } = await integration.mcpClient().listTools();
9-
const listOrgs = tools.find((tool) => tool.name === "atlas-list-orgs");
10-
expectDefined(listOrgs);
11-
});
6+
withCredentials(integration, () => {
7+
describe("atlas-list-orgs", () => {
8+
it("should have correct metadata", async () => {
9+
const { tools } = await integration.mcpClient().listTools();
10+
const listOrgs = tools.find((tool) => tool.name === "atlas-list-orgs");
11+
expectDefined(listOrgs);
12+
});
1213

13-
it("returns org names", async () => {
14-
const response = await integration.mcpClient().callTool({ name: "atlas-list-orgs", arguments: {} });
15-
const elements = getResponseElements(response);
16-
expect(elements[0]?.text).toContain("Found 1 organizations");
17-
expect(elements[1]?.text).toContain("<untrusted-user-data-");
18-
const data = parseTable(getDataFromUntrustedContent(elements[1]?.text ?? ""));
19-
expect(data).toHaveLength(1);
20-
expect(data[0]?.["Organization Name"]).toEqual("MongoDB MCP Test");
14+
it("returns org names", async () => {
15+
const response = await integration.mcpClient().callTool({ name: "atlas-list-orgs", arguments: {} });
16+
const elements = getResponseElements(response);
17+
expect(elements[0]?.text).toContain("Found 1 organizations");
18+
expect(elements[1]?.text).toContain("<untrusted-user-data-");
19+
const data = parseTable(getDataFromUntrustedContent(elements[1]?.text ?? ""));
20+
expect(data).toHaveLength(1);
21+
expect(data[0]?.["Organization Name"]).toEqual("MongoDB MCP Test");
22+
});
2123
});
2224
});
2325
});

‎tests/integration/tools/atlas/projects.test.ts‎

Lines changed: 60 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -1,79 +1,81 @@
11
import { ObjectId } from "mongodb";
2-
import { parseTable, describeWithAtlas } from "./atlasHelpers.js";
2+
import { parseTable, describeWithAtlas, withCredentials } from "./atlasHelpers.js";
33
import { expectDefined, getDataFromUntrustedContent, getResponseElements } from "../../helpers.js";
44
import { afterAll, describe, expect, it } from "vitest";
55

66
const randomId = new ObjectId().toString();
77

88
describeWithAtlas("projects", (integration) => {
9-
const projName = "testProj-" + randomId;
9+
withCredentials(integration, () => {
10+
const projName = "testProj-" + randomId;
1011

11-
afterAll(async () => {
12-
const session = integration.mcpServer().session;
12+
afterAll(async () => {
13+
const session = integration.mcpServer().session;
1314

14-
const projects = await session.apiClient.listProjects();
15-
for (const project of projects?.results || []) {
16-
if (project.name === projName) {
17-
await session.apiClient.deleteProject({
18-
params: {
19-
path: {
20-
groupId: project.id || "",
15+
const projects = await session.apiClient.listProjects();
16+
for (const project of projects?.results || []) {
17+
if (project.name === projName) {
18+
await session.apiClient.deleteProject({
19+
params: {
20+
path: {
21+
groupId: project.id || "",
22+
},
2123
},
22-
},
23-
});
24-
break;
24+
});
25+
break;
26+
}
2527
}
26-
}
27-
});
28-
29-
describe("atlas-create-project", () => {
30-
it("should have correct metadata", async () => {
31-
const { tools } = await integration.mcpClient().listTools();
32-
const createProject = tools.find((tool) => tool.name === "atlas-create-project");
33-
expectDefined(createProject);
34-
expect(createProject.inputSchema.type).toBe("object");
35-
expectDefined(createProject.inputSchema.properties);
36-
expect(createProject.inputSchema.properties).toHaveProperty("projectName");
37-
expect(createProject.inputSchema.properties).toHaveProperty("organizationId");
3828
});
39-
it("should create a project", async () => {
40-
const response = await integration.mcpClient().callTool({
41-
name: "atlas-create-project",
42-
arguments: { projectName: projName },
29+
30+
describe("atlas-create-project", () => {
31+
it("should have correct metadata", async () => {
32+
const { tools } = await integration.mcpClient().listTools();
33+
const createProject = tools.find((tool) => tool.name === "atlas-create-project");
34+
expectDefined(createProject);
35+
expect(createProject.inputSchema.type).toBe("object");
36+
expectDefined(createProject.inputSchema.properties);
37+
expect(createProject.inputSchema.properties).toHaveProperty("projectName");
38+
expect(createProject.inputSchema.properties).toHaveProperty("organizationId");
4339
});
40+
it("should create a project", async () => {
41+
const response = await integration.mcpClient().callTool({
42+
name: "atlas-create-project",
43+
arguments: { projectName: projName },
44+
});
4445

45-
const elements = getResponseElements(response);
46-
expect(elements).toHaveLength(1);
47-
expect(elements[0]?.text).toContain(projName);
48-
});
49-
});
50-
describe("atlas-list-projects", () => {
51-
it("should have correct metadata", async () => {
52-
const { tools } = await integration.mcpClient().listTools();
53-
const listProjects = tools.find((tool) => tool.name === "atlas-list-projects");
54-
expectDefined(listProjects);
55-
expect(listProjects.inputSchema.type).toBe("object");
56-
expectDefined(listProjects.inputSchema.properties);
57-
expect(listProjects.inputSchema.properties).toHaveProperty("orgId");
46+
const elements = getResponseElements(response);
47+
expect(elements).toHaveLength(1);
48+
expect(elements[0]?.text).toContain(projName);
49+
});
5850
});
51+
describe("atlas-list-projects", () => {
52+
it("should have correct metadata", async () => {
53+
const { tools } = await integration.mcpClient().listTools();
54+
const listProjects = tools.find((tool) => tool.name === "atlas-list-projects");
55+
expectDefined(listProjects);
56+
expect(listProjects.inputSchema.type).toBe("object");
57+
expectDefined(listProjects.inputSchema.properties);
58+
expect(listProjects.inputSchema.properties).toHaveProperty("orgId");
59+
});
5960

60-
it("returns project names", async () => {
61-
const response = await integration.mcpClient().callTool({ name: "atlas-list-projects", arguments: {} });
62-
const elements = getResponseElements(response);
63-
expect(elements).toHaveLength(2);
64-
expect(elements[1]?.text).toContain("<untrusted-user-data-");
65-
expect(elements[1]?.text).toContain(projName);
66-
const data = parseTable(getDataFromUntrustedContent(elements[1]?.text ?? ""));
67-
expect(data.length).toBeGreaterThan(0);
68-
let found = false;
69-
for (const project of data) {
70-
if (project["Project Name"] === projName) {
71-
found = true;
61+
it("returns project names", async () => {
62+
const response = await integration.mcpClient().callTool({ name: "atlas-list-projects", arguments: {} });
63+
const elements = getResponseElements(response);
64+
expect(elements).toHaveLength(2);
65+
expect(elements[1]?.text).toContain("<untrusted-user-data-");
66+
expect(elements[1]?.text).toContain(projName);
67+
const data = parseTable(getDataFromUntrustedContent(elements[1]?.text ?? ""));
68+
expect(data.length).toBeGreaterThan(0);
69+
let found = false;
70+
for (const project of data) {
71+
if (project["Project Name"] === projName) {
72+
found = true;
73+
}
7274
}
73-
}
74-
expect(found).toBe(true);
75+
expect(found).toBe(true);
7576

76-
expect(elements[0]?.text).toBe(`Found ${data.length} projects`);
77+
expect(elements[0]?.text).toBe(`Found ${data.length} projects`);
78+
});
7779
});
7880
});
7981
});

0 commit comments

Comments
 (0)