From d9f6a236b798845e40444ac76a204b573747dff0 Mon Sep 17 00:00:00 2001 From: Nataly Carbonell Date: Tue, 23 Sep 2025 12:37:08 -0400 Subject: [PATCH 01/12] feat(compass-collection): CLOUDP-347048 LLM Output Validation Followup - Mock Data Generator --- .../faker-mapping-selector.tsx | 94 +++++++++- .../faker-schema-editor-screen.tsx | 2 + .../mock-data-generator-modal.spec.tsx | 39 ++++ .../script-generation-utils.ts | 7 +- .../mock-data-generator-modal/utils.spec.ts | 171 ++++++++++++++++++ .../mock-data-generator-modal/utils.ts | 137 ++++++++++++++ .../src/modules/collection-tab.ts | 45 +---- 7 files changed, 458 insertions(+), 37 deletions(-) create mode 100644 packages/compass-collection/src/components/mock-data-generator-modal/utils.spec.ts create mode 100644 packages/compass-collection/src/components/mock-data-generator-modal/utils.ts diff --git a/packages/compass-collection/src/components/mock-data-generator-modal/faker-mapping-selector.tsx b/packages/compass-collection/src/components/mock-data-generator-modal/faker-mapping-selector.tsx index b6c1aa726b0..e977b28e487 100644 --- a/packages/compass-collection/src/components/mock-data-generator-modal/faker-mapping-selector.tsx +++ b/packages/compass-collection/src/components/mock-data-generator-modal/faker-mapping-selector.tsx @@ -7,10 +7,12 @@ import { palette, Select, spacing, + TextInput, } from '@mongodb-js/compass-components'; import React from 'react'; import { UNRECOGNIZED_FAKER_METHOD } from '../../modules/collection-tab'; import type { MongoDBFieldType } from '@mongodb-js/compass-generative-ai'; +import type { FakerArg } from './script-generation-utils'; const fieldMappingSelectorsStyles = css({ width: '50%', @@ -24,16 +26,106 @@ const labelStyles = css({ fontWeight: 600, }); +/** + * Renders read-only TextInput components for each key-value pair in a faker arguments object. + */ +const getFakerArgsInputFromObject = (fakerArgsObject: Record) => { + return Object.entries(fakerArgsObject).map(([key, item]: [string, any]) => { + if (typeof item === 'string' || typeof item === 'boolean') { + return ( + + ); + } else if (typeof item === 'number') { + return ( + + ); + } else if ( + Array.isArray(item) && + item.length > 0 && + typeof item[0] === 'string' + ) { + return ( + + ); + } + return null; + }); +}; + +/** + * Renders TextInput components for each faker argument based on its type. + */ +const getFakerArgsInput = (fakerArgs: FakerArg[]) => { + return fakerArgs.map((arg, idx) => { + if (typeof arg === 'string' || typeof arg === 'boolean') { + return ( + + ); + } else if (typeof arg === 'number') { + return ( + + ); + } else if (typeof arg === 'object' && 'json' in arg) { + // parse the object + let parsedArg; + try { + parsedArg = JSON.parse(arg.json); + } catch { + // If parsing fails, skip rendering this arg + return null; + } + if (typeof parsedArg === 'object') { + return getFakerArgsInputFromObject(parsedArg); + } + } + }); +}; + interface Props { activeJsonType: string; activeFakerFunction: string; onJsonTypeSelect: (jsonType: MongoDBFieldType) => void; + activeFakerArgs: FakerArg[]; onFakerFunctionSelect: (fakerFunction: string) => void; } const FakerMappingSelector = ({ activeJsonType, activeFakerFunction, + activeFakerArgs, onJsonTypeSelect, onFakerFunctionSelect, }: Props) => { @@ -72,7 +164,7 @@ const FakerMappingSelector = ({ string "Unrecognized" )} - {/* TODO(CLOUDP-344400): Render faker function parameters once we have a way to validate them. */} + {getFakerArgsInput(activeFakerArgs)} ); }; diff --git a/packages/compass-collection/src/components/mock-data-generator-modal/faker-schema-editor-screen.tsx b/packages/compass-collection/src/components/mock-data-generator-modal/faker-schema-editor-screen.tsx index 39b6a5d7a27..55ebe80bd0b 100644 --- a/packages/compass-collection/src/components/mock-data-generator-modal/faker-schema-editor-screen.tsx +++ b/packages/compass-collection/src/components/mock-data-generator-modal/faker-schema-editor-screen.tsx @@ -64,6 +64,7 @@ const FakerSchemaEditorContent = ({ const activeJsonType = fakerSchemaFormValues[activeField]?.mongoType; const activeFakerFunction = fakerSchemaFormValues[activeField]?.fakerMethod; + const activeFakerArgs = fakerSchemaFormValues[activeField]?.fakerArgs; const resetIsSchemaConfirmed = () => { onSchemaConfirmed(false); @@ -109,6 +110,7 @@ const FakerSchemaEditorContent = ({ diff --git a/packages/compass-collection/src/components/mock-data-generator-modal/mock-data-generator-modal.spec.tsx b/packages/compass-collection/src/components/mock-data-generator-modal/mock-data-generator-modal.spec.tsx index e758e4dbac6..c2ec74f6522 100644 --- a/packages/compass-collection/src/components/mock-data-generator-modal/mock-data-generator-modal.spec.tsx +++ b/packages/compass-collection/src/components/mock-data-generator-modal/mock-data-generator-modal.spec.tsx @@ -62,6 +62,7 @@ describe('MockDataGeneratorModal', () => { }, }; + // @ts-expect-error // TODO: fix ts error const store = createStore( collectionTabReducer, initialState, @@ -538,6 +539,44 @@ describe('MockDataGeneratorModal', () => { ); }); + it('does not show faker args that are too large', async () => { + const largeLengthArgs = Array.from({ length: 11 }, () => 'test'); + const mockServices = createMockServices(); + mockServices.atlasAiService.getMockDataSchema = () => + Promise.resolve({ + fields: [ + { + fieldPath: 'name', + mongoType: 'String', + fakerMethod: 'person.firstName', + fakerArgs: [JSON.stringify(largeLengthArgs)], + isArray: false, + probability: 1.0, + }, + ], + }); + + await renderModal({ + mockServices, + schemaAnalysis: { + ...defaultSchemaAnalysisState, + processedSchema: { + name: { + type: 'String', + probability: 1.0, + }, + }, + sampleDocument: { name: 'Peaches' }, + }, + }); + + // advance to the schema editor step + userEvent.click(screen.getByText('Confirm')); + await waitFor(() => { + expect(screen.getByTestId('faker-schema-editor')).to.exist; + }); + }); + it('disables the Next button when the faker schema mapping is not confirmed', async () => { await renderModal({ mockServices: mockServicesWithMockDataResponse, diff --git a/packages/compass-collection/src/components/mock-data-generator-modal/script-generation-utils.ts b/packages/compass-collection/src/components/mock-data-generator-modal/script-generation-utils.ts index 6d465e6628a..6dd44a36eda 100644 --- a/packages/compass-collection/src/components/mock-data-generator-modal/script-generation-utils.ts +++ b/packages/compass-collection/src/components/mock-data-generator-modal/script-generation-utils.ts @@ -1,7 +1,12 @@ import type { MongoDBFieldType } from '@mongodb-js/compass-generative-ai'; import type { FakerFieldMapping } from './types'; -export type FakerArg = string | number | boolean | { json: string }; +export type FakerArg = + | string + | number + | boolean + | { json: string } + | FakerArg[]; const DEFAULT_ARRAY_LENGTH = 3; const INDENT_SIZE = 2; diff --git a/packages/compass-collection/src/components/mock-data-generator-modal/utils.spec.ts b/packages/compass-collection/src/components/mock-data-generator-modal/utils.spec.ts new file mode 100644 index 00000000000..c669cfe4b64 --- /dev/null +++ b/packages/compass-collection/src/components/mock-data-generator-modal/utils.spec.ts @@ -0,0 +1,171 @@ +import { expect } from 'chai'; +import { areFakerArgsValid, isValidFakerMethod } from './utils'; + +describe('Mock Data Generator Utils', function () { + describe('areFakerArgsValid', () => { + it('returns true for empty array', () => { + expect(areFakerArgsValid([])).to.be.true; + }); + + it('returns false if array length exceeds max faker args length', () => { + const arr = Array(11).fill(1); + expect(areFakerArgsValid(arr)).to.be.false; + }); + + it('returns true for valid numbers, strings, booleans', () => { + expect(areFakerArgsValid([1, 'foo', true, false, 0])).to.be.true; + }); + + it('returns false for non-finite numbers', () => { + expect(areFakerArgsValid([Infinity])).to.be.false; + expect(areFakerArgsValid([NaN])).to.be.false; + }); + + it('returns false for strings exceeding max faker string length', () => { + const longStr = 'a'.repeat(1001); + expect(areFakerArgsValid([longStr])).to.be.false; + }); + + it('returns true for nested valid arrays', () => { + expect( + areFakerArgsValid([ + [1, 'foo', true], + [2, false], + ]) + ).to.be.true; + }); + + it('returns false for nested arrays exceeding max faker args length', () => { + const nested = [Array(11).fill(1)]; + expect(areFakerArgsValid(nested)).to.be.false; + }); + + it('returns true for valid object with json property', () => { + const obj = { json: JSON.stringify({ a: 1, b: 'foo', c: true }) }; + expect(areFakerArgsValid([obj])).to.be.true; + }); + + it('returns false for object with invalid json property', () => { + const obj = { json: '{invalid json}' }; + expect(areFakerArgsValid([obj])).to.be.false; + }); + + it('returns false for object with json property containing invalid values', () => { + const obj = { json: JSON.stringify({ a: Infinity }) }; + expect(areFakerArgsValid([obj])).to.be.false; + }); + + it('returns false for unrecognized argument types', () => { + expect(areFakerArgsValid([undefined as any])).to.be.false; + expect(areFakerArgsValid([null as any])).to.be.false; + expect(areFakerArgsValid([(() => {}) as any])).to.be.false; + }); + + it('returns true for deeply nested valid structures', () => { + const obj = { + json: JSON.stringify({ + a: [1, { json: JSON.stringify({ b: 'foo' }) }], + }), + }; + expect(areFakerArgsValid([obj])).to.be.true; + }); + + it('returns false for deeply nested invalid structures', () => { + const obj = { + json: JSON.stringify({ + a: [1, { json: JSON.stringify({ b: Infinity }) }], + }), + }; + expect(areFakerArgsValid([obj])).to.be.false; + }); + + describe('isValidFakerMethod', () => { + it('returns false for invalid method format', () => { + expect(isValidFakerMethod('invalidMethod', [])).to.deep.equal({ + isValid: false, + fakerArgs: [], + }); + expect(isValidFakerMethod('internet.email.extra', [])).to.deep.equal({ + isValid: false, + fakerArgs: [], + }); + }); + + it('returns false for non-existent faker module', () => { + expect(isValidFakerMethod('notamodule.email', [])).to.deep.equal({ + isValid: false, + fakerArgs: [], + }); + }); + + it('returns false for non-existent faker method', () => { + expect(isValidFakerMethod('internet.notamethod', [])).to.deep.equal({ + isValid: false, + fakerArgs: [], + }); + }); + + it('returns true for valid method without arguments', () => { + const result = isValidFakerMethod('internet.email', []); + expect(result.isValid).to.be.true; + expect(result.fakerArgs).to.deep.equal([]); + }); + + it('returns true for valid method with valid arguments', () => { + // name.firstName takes optional gender argument + const result = isValidFakerMethod('name.firstName', ['female']); + expect(result.isValid).to.be.true; + expect(result.fakerArgs).to.deep.equal(['female']); + }); + + it('returns true for valid method with no args if args are invalid but fallback works', () => { + // internet.email does not take arguments, so passing one should fallback to [] + const result = isValidFakerMethod('internet.email', []); + expect(result.isValid).to.be.true; + expect(result.fakerArgs).to.deep.equal([]); + }); + + it('returns false for valid method with invalid arguments and fallback fails', () => { + // date.month expects at most one argument, passing an object will fail both attempts + const result = isValidFakerMethod('date.month', [ + { foo: 'bar' } as any, + ]); + expect(result.isValid).to.be.false; + expect(result.fakerArgs).to.deep.equal([]); + }); + + it('returns false for helpers methods except arrayElement', () => { + expect(isValidFakerMethod('helpers.fake', [])).to.deep.equal({ + isValid: false, + fakerArgs: [], + }); + expect(isValidFakerMethod('helpers.slugify', [])).to.deep.equal({ + isValid: false, + fakerArgs: [], + }); + }); + + it('returns true for helpers.arrayElement with valid arguments', () => { + const arr = ['a', 'b', 'c']; + const result = isValidFakerMethod('helpers.arrayElement', [arr]); + expect(result.isValid).to.be.true; + expect(result.fakerArgs).to.deep.equal([arr]); + }); + + it('returns false for helpers.arrayElement with invalid arguments', () => { + // Exceeding max args length + const arr = Array(11).fill('x'); + const result = isValidFakerMethod('helpers.arrayElement', [arr]); + expect(result.isValid).to.be.false; + expect(result.fakerArgs).to.deep.equal([]); + }); + + it('returns false for method with invalid fakerArgs', () => { + // Passing Infinity as argument + const result = isValidFakerMethod('name.firstName', [Infinity]); + expect(result.isValid).to.be.false; + expect(result.fakerArgs).to.deep.equal([]); + }); + }); + }); +}); diff --git a/packages/compass-collection/src/components/mock-data-generator-modal/utils.ts b/packages/compass-collection/src/components/mock-data-generator-modal/utils.ts new file mode 100644 index 00000000000..ca98da12e4e --- /dev/null +++ b/packages/compass-collection/src/components/mock-data-generator-modal/utils.ts @@ -0,0 +1,137 @@ +import type { FakerArg } from './script-generation-utils'; +import { faker } from '@faker-js/faker/locale/en'; + +const MAX_FAKER_ARGS_LENGTH = 10; +const MAX_FAKER_STRING_LENGTH = 1000; + +/** + * Checks if the provided faker arguments are valid. + * - Numbers must be finite + * - Strings must not exceed max length + * - Arrays must not exceed max length and all elements must be valid + * - Objects must have a 'json' property that is a valid JSON string and all values must be valid + */ +export function areFakerArgsValid(fakerArgs: FakerArg[]): boolean { + if (fakerArgs.length === 0) { + return true; + } + if (fakerArgs.length > MAX_FAKER_ARGS_LENGTH) { + return false; + } + for (const arg of fakerArgs) { + if (arg === null || arg === undefined) { + return false; + } + if (typeof arg === 'number') { + if (!Number.isFinite(arg)) { + return false; + } + } else if (typeof arg === 'string') { + if (arg.length > MAX_FAKER_STRING_LENGTH) { + return false; + } + } else if (typeof arg === 'boolean') { + // booleans are always valid, continue + } else if (Array.isArray(arg)) { + if (!areFakerArgsValid(arg)) { + return false; + } + } else if (typeof arg === 'object' && typeof arg.json === 'string') { + try { + const parsedJson = JSON.parse(arg.json); + if (!areFakerArgsValid(Object.values(parsedJson))) { + return false; + } + } catch { + return false; + } + } else { + // Unrecognized argument type + return false; + } + } + return true; +} + +/** + * Checks if the method exists and is callable on the faker object. + * + * Note: Only supports the format `module.method` (e.g., `internet.email`). + * - Nested modules are not supported. + * - Most methods in the `helpers` module are not supported due to their complex argument requirements. @see {@link https://fakerjs.dev/api/helpers.html} + * - If the method call with provided args fails, it retries without args before marking as invalid. + * + * @see {@link https://fakerjs.dev/api/} + */ +export function isValidFakerMethod( + fakerMethod: string, + fakerArgs: FakerArg[] +): { + isValid: boolean; + fakerArgs: FakerArg[]; +} { + const moduleAndMethod = getFakerModuleAndMethod(fakerMethod); + if (!moduleAndMethod) { + return { isValid: false, fakerArgs: [] }; + } + const { moduleName, methodName, fakerModule } = moduleAndMethod; + + if ( + isAllowedHelper(moduleName, methodName) && + canInvokeFakerMethod(fakerModule, methodName) + ) { + const callableFakerMethod = (fakerModule as Record)[ + methodName + ]; + return tryInvokeFakerMethod(callableFakerMethod, fakerArgs); + } else { + return { isValid: false, fakerArgs: [] }; + } +} + +function getFakerModuleAndMethod(method: string) { + const parts = method.split('.'); + if (parts.length !== 2) { + return null; + } + const [moduleName, methodName] = parts; + const fakerModule = (faker as unknown as Record)[moduleName]; + return { moduleName, methodName, fakerModule }; +} + +function isAllowedHelper(moduleName: string, methodName: string) { + return moduleName !== 'helpers' || methodName === 'arrayElement'; +} + +function canInvokeFakerMethod(fakerModule: any, methodName: string) { + return ( + fakerModule !== null && + fakerModule !== undefined && + typeof fakerModule === 'object' && + typeof fakerModule[methodName] === 'function' + ); +} + +function tryInvokeFakerMethod( + callable: (...args: any[]) => any, + args: FakerArg[] +) { + try { + if (areFakerArgsValid(args)) { + callable(...args); + return { isValid: true, fakerArgs: args }; + } + } catch { + // Intentionally ignored: error may be due to invalid arguments, will retry without args. + if (args.length > 0) { + try { + callable(); + return { isValid: true, fakerArgs: [] }; + } catch { + // Intentionally ignored: method is invalid without arguments as well. + return { isValid: false, fakerArgs: [] }; + } + } + } + return { isValid: false, fakerArgs: [] }; +} diff --git a/packages/compass-collection/src/modules/collection-tab.ts b/packages/compass-collection/src/modules/collection-tab.ts index 77720bd4bf3..c5db1ac3ee2 100644 --- a/packages/compass-collection/src/modules/collection-tab.ts +++ b/packages/compass-collection/src/modules/collection-tab.ts @@ -41,7 +41,7 @@ import type { MockDataGeneratorState, } from '../components/mock-data-generator-modal/types'; -import { faker } from '@faker-js/faker/locale/en'; +import { isValidFakerMethod } from '../components/mock-data-generator-modal/utils'; const DEFAULT_SAMPLE_SIZE = 100; @@ -716,38 +716,6 @@ function transformFakerSchemaToObject( return result; } -/** - * Checks if the method exists and is callable on the faker object. - * - * Note: Only supports the format `module.method` (e.g., `internet.email`). - * Nested modules or other formats are not supported. - * @see {@link https://fakerjs.dev/api/} - */ -function isValidFakerMethod(fakerMethod: string): boolean { - const parts = fakerMethod.split('.'); - - // Validate format: exactly module.method - if (parts.length !== 2) { - return false; - } - - const [moduleName, methodName] = parts; - - try { - const fakerModule = (faker as unknown as Record)[ - moduleName - ]; - return ( - fakerModule !== null && - fakerModule !== undefined && - typeof fakerModule === 'object' && - typeof (fakerModule as Record)[methodName] === 'function' - ); - } catch { - return false; - } -} - /** * Validates a given faker schema against an input schema. * @@ -775,8 +743,15 @@ const validateFakerSchema = ( probability: inputSchema[fieldPath].probability, }; // Validate the faker method - if (isValidFakerMethod(fakerMapping.fakerMethod)) { - result[fieldPath] = fakerMapping; + const { isValid: isValidMethod, fakerArgs } = isValidFakerMethod( + fakerMapping.fakerMethod, + fakerMapping.fakerArgs + ); + if (isValidMethod) { + result[fieldPath] = { + ...fakerMapping, + fakerArgs, + }; } else { logger.log.warn( mongoLogId(1_001_000_372), From 406aa2bd9875e6155724067760e205b3a7ab961c Mon Sep 17 00:00:00 2001 From: Nataly Carbonell Date: Tue, 23 Sep 2025 13:26:56 -0400 Subject: [PATCH 02/12] call faker method without args if invalid --- .../src/components/mock-data-generator-modal/utils.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/compass-collection/src/components/mock-data-generator-modal/utils.ts b/packages/compass-collection/src/components/mock-data-generator-modal/utils.ts index ca98da12e4e..908b9e91746 100644 --- a/packages/compass-collection/src/components/mock-data-generator-modal/utils.ts +++ b/packages/compass-collection/src/components/mock-data-generator-modal/utils.ts @@ -120,6 +120,9 @@ function tryInvokeFakerMethod( if (areFakerArgsValid(args)) { callable(...args); return { isValid: true, fakerArgs: args }; + } else { + callable(); + return { isValid: true, fakerArgs: [] }; } } catch { // Intentionally ignored: error may be due to invalid arguments, will retry without args. From 0b079fbb2f7fbaaf361f5d9273f6d74e447ffc52 Mon Sep 17 00:00:00 2001 From: Nataly Carbonell Date: Tue, 23 Sep 2025 17:10:11 -0400 Subject: [PATCH 03/12] update faker argument handling and improve validation logging --- .../faker-mapping-selector.tsx | 2 +- .../mock-data-generator-modal/utils.spec.ts | 207 +++++++++++------- .../mock-data-generator-modal/utils.ts | 77 ++++--- .../src/modules/collection-tab.ts | 3 +- 4 files changed, 184 insertions(+), 105 deletions(-) diff --git a/packages/compass-collection/src/components/mock-data-generator-modal/faker-mapping-selector.tsx b/packages/compass-collection/src/components/mock-data-generator-modal/faker-mapping-selector.tsx index e977b28e487..0fe0e9f0498 100644 --- a/packages/compass-collection/src/components/mock-data-generator-modal/faker-mapping-selector.tsx +++ b/packages/compass-collection/src/components/mock-data-generator-modal/faker-mapping-selector.tsx @@ -84,7 +84,7 @@ const getFakerArgsInput = (fakerArgs: FakerArg[]) => { key={`faker-arg-${idx}`} type="text" label="Faker Arg" - required + readOnly value={arg.toString()} /> ); diff --git a/packages/compass-collection/src/components/mock-data-generator-modal/utils.spec.ts b/packages/compass-collection/src/components/mock-data-generator-modal/utils.spec.ts index c669cfe4b64..b1054c93e1c 100644 --- a/packages/compass-collection/src/components/mock-data-generator-modal/utils.spec.ts +++ b/packages/compass-collection/src/components/mock-data-generator-modal/utils.spec.ts @@ -1,7 +1,18 @@ import { expect } from 'chai'; import { areFakerArgsValid, isValidFakerMethod } from './utils'; -describe('Mock Data Generator Utils', function () { +import Sinon from 'sinon'; +import { faker } from '@faker-js/faker/locale/en'; +import { createNoopLogger } from '@mongodb-js/compass-logging/provider'; + +describe('Mock Data Generator Utils', () => { + const sandbox = Sinon.createSandbox(); + const logger = createNoopLogger(); + + afterEach(() => { + sandbox.restore(); + }); + describe('areFakerArgsValid', () => { it('returns true for empty array', () => { expect(areFakerArgsValid([])).to.be.true; @@ -79,93 +90,141 @@ describe('Mock Data Generator Utils', function () { expect(areFakerArgsValid([obj])).to.be.false; }); - describe('isValidFakerMethod', () => { - it('returns false for invalid method format', () => { - expect(isValidFakerMethod('invalidMethod', [])).to.deep.equal({ - isValid: false, - fakerArgs: [], - }); - expect(isValidFakerMethod('internet.email.extra', [])).to.deep.equal({ - isValid: false, - fakerArgs: [], - }); - }); + it('returns false for deeply nested invalid structures with max depth', () => { + const obj = { + json: JSON.stringify({ + a: [ + 1, + { + json: JSON.stringify({ + b: 2, + c: { + json: JSON.stringify({ + d: 3, + }), + }, + }), + }, + ], + }), + }; + expect(areFakerArgsValid([obj])).to.be.false; + }); + }); - it('returns false for non-existent faker module', () => { - expect(isValidFakerMethod('notamodule.email', [])).to.deep.equal({ - isValid: false, - fakerArgs: [], - }); - }); + describe('isValidFakerMethod', () => { + it('returns false for invalid method format', () => { + sandbox.stub(faker.internet, 'email'); - it('returns false for non-existent faker method', () => { - expect(isValidFakerMethod('internet.notamethod', [])).to.deep.equal({ - isValid: false, - fakerArgs: [], - }); + expect(isValidFakerMethod('invalidMethod', [], logger)).to.deep.equal({ + isValid: false, + fakerArgs: [], }); - - it('returns true for valid method without arguments', () => { - const result = isValidFakerMethod('internet.email', []); - expect(result.isValid).to.be.true; - expect(result.fakerArgs).to.deep.equal([]); + expect( + isValidFakerMethod('internet.email.extra', [], logger) + ).to.deep.equal({ + isValid: false, + fakerArgs: [], }); + }); - it('returns true for valid method with valid arguments', () => { - // name.firstName takes optional gender argument - const result = isValidFakerMethod('name.firstName', ['female']); - expect(result.isValid).to.be.true; - expect(result.fakerArgs).to.deep.equal(['female']); + it('returns false for non-existent faker module', () => { + expect(isValidFakerMethod('notamodule.email', [], logger)).to.deep.equal({ + isValid: false, + fakerArgs: [], }); + }); - it('returns true for valid method with no args if args are invalid but fallback works', () => { - // internet.email does not take arguments, so passing one should fallback to [] - const result = isValidFakerMethod('internet.email', []); - expect(result.isValid).to.be.true; - expect(result.fakerArgs).to.deep.equal([]); + it('returns false for non-existent faker method', () => { + expect( + isValidFakerMethod('internet.notamethod', [], logger) + ).to.deep.equal({ + isValid: false, + fakerArgs: [], }); + }); - it('returns false for valid method with invalid arguments and fallback fails', () => { - // date.month expects at most one argument, passing an object will fail both attempts - const result = isValidFakerMethod('date.month', [ - { foo: 'bar' } as any, - ]); - expect(result.isValid).to.be.false; - expect(result.fakerArgs).to.deep.equal([]); - }); + it('returns true for valid method without arguments', () => { + sandbox.stub(faker.internet, 'email').returns('test@test.com'); - it('returns false for helpers methods except arrayElement', () => { - expect(isValidFakerMethod('helpers.fake', [])).to.deep.equal({ - isValid: false, - fakerArgs: [], - }); - expect(isValidFakerMethod('helpers.slugify', [])).to.deep.equal({ - isValid: false, - fakerArgs: [], - }); - }); + const result = isValidFakerMethod('internet.email', [], logger); + expect(result.isValid).to.be.true; + expect(result.fakerArgs).to.deep.equal([]); + }); - it('returns true for helpers.arrayElement with valid arguments', () => { - const arr = ['a', 'b', 'c']; - const result = isValidFakerMethod('helpers.arrayElement', [arr]); - expect(result.isValid).to.be.true; - expect(result.fakerArgs).to.deep.equal([arr]); - }); + it('returns true for valid method with valid arguments', () => { + sandbox.stub(faker.person, 'firstName'); - it('returns false for helpers.arrayElement with invalid arguments', () => { - // Exceeding max args length - const arr = Array(11).fill('x'); - const result = isValidFakerMethod('helpers.arrayElement', [arr]); - expect(result.isValid).to.be.false; - expect(result.fakerArgs).to.deep.equal([]); - }); + const result = isValidFakerMethod('person.firstName', ['female'], logger); + expect(result.isValid).to.be.true; + expect(result.fakerArgs).to.deep.equal(['female']); + }); + + it('returns true for valid method with no args if args are invalid but fallback works', () => { + sandbox.stub(faker.internet, 'email').returns('test@test.com'); + + const result = isValidFakerMethod('internet.email', [], logger); + expect(result.isValid).to.be.true; + expect(result.fakerArgs).to.deep.equal([]); + }); - it('returns false for method with invalid fakerArgs', () => { - // Passing Infinity as argument - const result = isValidFakerMethod('name.firstName', [Infinity]); - expect(result.isValid).to.be.false; - expect(result.fakerArgs).to.deep.equal([]); + it('returns true valid method with invalid arguments and strips args', () => { + sandbox.stub(faker.date, 'month').returns('February'); + + const result = isValidFakerMethod( + 'date.month', + [{ foo: 'bar' } as any], + logger + ); + expect(result.isValid).to.be.true; + expect(result.fakerArgs).to.deep.equal([]); + }); + + it('returns false for helpers methods except arrayElement', () => { + expect(isValidFakerMethod('helpers.fake', [], logger)).to.deep.equal({ + isValid: false, + fakerArgs: [], }); + expect(isValidFakerMethod('helpers.slugify', [], logger)).to.deep.equal({ + isValid: false, + fakerArgs: [], + }); + }); + + it('returns true for helpers.arrayElement with valid arguments', () => { + sandbox.stub(faker.helpers, 'arrayElement').returns('a'); + + const arr = ['a', 'b', 'c']; + const result = isValidFakerMethod('helpers.arrayElement', [arr], logger); + expect(result.isValid).to.be.true; + expect(result.fakerArgs).to.deep.equal([arr]); + }); + + it('returns false for helpers.arrayElement with invalid arguments', () => { + // Exceeding max args length + const arr = Array(11).fill('x'); + const result = isValidFakerMethod('helpers.arrayElement', [arr], logger); + expect(result.isValid).to.be.false; + expect(result.fakerArgs).to.deep.equal([]); + }); + + it('returns true for valid method with invalid fakerArgs and strips args', () => { + sandbox.stub(faker.person, 'firstName').returns('a'); + + // Passing Infinity as argument + const result = isValidFakerMethod('person.firstName', [Infinity], logger); + expect(result.isValid).to.be.true; + expect(result.fakerArgs).to.deep.equal([]); + }); + + it('returns false when calling a faker method fails', () => { + sandbox + .stub(faker.person, 'firstName') + .throws(new Error('Invalid faker method')); + + const result = isValidFakerMethod('person.firstName', [], logger); + expect(result.isValid).to.be.false; + expect(result.fakerArgs).to.deep.equal([]); }); }); }); diff --git a/packages/compass-collection/src/components/mock-data-generator-modal/utils.ts b/packages/compass-collection/src/components/mock-data-generator-modal/utils.ts index 908b9e91746..8a1a41ca628 100644 --- a/packages/compass-collection/src/components/mock-data-generator-modal/utils.ts +++ b/packages/compass-collection/src/components/mock-data-generator-modal/utils.ts @@ -1,8 +1,10 @@ +import { type Logger, mongoLogId } from '@mongodb-js/compass-logging/provider'; import type { FakerArg } from './script-generation-utils'; import { faker } from '@faker-js/faker/locale/en'; const MAX_FAKER_ARGS_LENGTH = 10; const MAX_FAKER_STRING_LENGTH = 1000; +const MAX_FAKER_ARGS_DEPTH = 3; /** * Checks if the provided faker arguments are valid. @@ -11,7 +13,13 @@ const MAX_FAKER_STRING_LENGTH = 1000; * - Arrays must not exceed max length and all elements must be valid * - Objects must have a 'json' property that is a valid JSON string and all values must be valid */ -export function areFakerArgsValid(fakerArgs: FakerArg[]): boolean { +export function areFakerArgsValid( + fakerArgs: FakerArg[], + depth: number = 0 +): boolean { + if (depth > MAX_FAKER_ARGS_DEPTH) { + return false; + } if (fakerArgs.length === 0) { return true; } @@ -33,13 +41,13 @@ export function areFakerArgsValid(fakerArgs: FakerArg[]): boolean { } else if (typeof arg === 'boolean') { // booleans are always valid, continue } else if (Array.isArray(arg)) { - if (!areFakerArgsValid(arg)) { + if (!areFakerArgsValid(arg, depth + 1)) { return false; } } else if (typeof arg === 'object' && typeof arg.json === 'string') { try { const parsedJson = JSON.parse(arg.json); - if (!areFakerArgsValid(Object.values(parsedJson))) { + if (!areFakerArgsValid(Object.values(parsedJson), depth + 1)) { return false; } } catch { @@ -65,7 +73,8 @@ export function areFakerArgsValid(fakerArgs: FakerArg[]): boolean { */ export function isValidFakerMethod( fakerMethod: string, - fakerArgs: FakerArg[] + fakerArgs: FakerArg[], + logger: Logger ): { isValid: boolean; fakerArgs: FakerArg[]; @@ -80,10 +89,15 @@ export function isValidFakerMethod( isAllowedHelper(moduleName, methodName) && canInvokeFakerMethod(fakerModule, methodName) ) { - const callableFakerMethod = (fakerModule as Record)[ - methodName - ]; - return tryInvokeFakerMethod(callableFakerMethod, fakerArgs); + const callableFakerMethod = ( + fakerModule as Record any> + )[methodName]; + return tryInvokeFakerMethod( + callableFakerMethod, + fakerMethod, + fakerArgs, + logger + ); } else { return { isValid: false, fakerArgs: [] }; } @@ -103,38 +117,43 @@ function isAllowedHelper(moduleName: string, methodName: string) { return moduleName !== 'helpers' || methodName === 'arrayElement'; } -function canInvokeFakerMethod(fakerModule: any, methodName: string) { +function canInvokeFakerMethod(fakerModule: unknown, methodName: string) { return ( fakerModule !== null && fakerModule !== undefined && typeof fakerModule === 'object' && - typeof fakerModule[methodName] === 'function' + typeof (fakerModule as Record)[methodName] === 'function' ); } function tryInvokeFakerMethod( - callable: (...args: any[]) => any, - args: FakerArg[] + callable: (...args: readonly unknown[]) => unknown, + fakerMethod: string, + args: FakerArg[], + logger: Logger ) { - try { - if (areFakerArgsValid(args)) { + // If args are present and safe, try calling with args + if (args.length > 0 && areFakerArgsValid(args)) { + try { callable(...args); return { isValid: true, fakerArgs: args }; - } else { - callable(); - return { isValid: true, fakerArgs: [] }; - } - } catch { - // Intentionally ignored: error may be due to invalid arguments, will retry without args. - if (args.length > 0) { - try { - callable(); - return { isValid: true, fakerArgs: [] }; - } catch { - // Intentionally ignored: method is invalid without arguments as well. - return { isValid: false, fakerArgs: [] }; - } + } catch { + // Call with args failed. Fall through to trying without args } } - return { isValid: false, fakerArgs: [] }; + + // Try without args (either because args were invalid or args failed) + try { + callable(); + return { isValid: true, fakerArgs: [] }; + } catch (error) { + // Calling the method without arguments failed. + logger.log.warn( + mongoLogId(1_001_000_377), + 'Collection', + 'Invalid faker method', + { error, fakerMethod, fakerArgs: args } + ); + return { isValid: false, fakerArgs: [] }; + } } diff --git a/packages/compass-collection/src/modules/collection-tab.ts b/packages/compass-collection/src/modules/collection-tab.ts index c5db1ac3ee2..1c6869effcc 100644 --- a/packages/compass-collection/src/modules/collection-tab.ts +++ b/packages/compass-collection/src/modules/collection-tab.ts @@ -745,7 +745,8 @@ const validateFakerSchema = ( // Validate the faker method const { isValid: isValidMethod, fakerArgs } = isValidFakerMethod( fakerMapping.fakerMethod, - fakerMapping.fakerArgs + fakerMapping.fakerArgs, + logger ); if (isValidMethod) { result[fieldPath] = { From df49911800747f9660a1882ca68c542b9898491b Mon Sep 17 00:00:00 2001 From: Nataly Carbonell Date: Tue, 23 Sep 2025 17:28:38 -0400 Subject: [PATCH 04/12] resolve TypeScript error by casting initialState to any --- .../mock-data-generator-modal.spec.tsx | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/compass-collection/src/components/mock-data-generator-modal/mock-data-generator-modal.spec.tsx b/packages/compass-collection/src/components/mock-data-generator-modal/mock-data-generator-modal.spec.tsx index c2ec74f6522..52bfc7f05b5 100644 --- a/packages/compass-collection/src/components/mock-data-generator-modal/mock-data-generator-modal.spec.tsx +++ b/packages/compass-collection/src/components/mock-data-generator-modal/mock-data-generator-modal.spec.tsx @@ -62,10 +62,9 @@ describe('MockDataGeneratorModal', () => { }, }; - // @ts-expect-error // TODO: fix ts error const store = createStore( collectionTabReducer, - initialState, + initialState as any, applyMiddleware(thunk.withExtraArgument(mockServices)) ); From b471586f9d78ff533f6c8eb6f4b88ced1ae18652 Mon Sep 17 00:00:00 2001 From: Nataly Carbonell Date: Thu, 25 Sep 2025 10:58:15 -0400 Subject: [PATCH 05/12] add faker argument validation for large numbers and add sample function call display --- .../faker-mapping-selector.tsx | 113 ++++-------------- .../mock-data-generator-modal/utils.spec.ts | 12 ++ .../mock-data-generator-modal/utils.ts | 18 ++- 3 files changed, 48 insertions(+), 95 deletions(-) diff --git a/packages/compass-collection/src/components/mock-data-generator-modal/faker-mapping-selector.tsx b/packages/compass-collection/src/components/mock-data-generator-modal/faker-mapping-selector.tsx index 0fe0e9f0498..bf3d89d0fc2 100644 --- a/packages/compass-collection/src/components/mock-data-generator-modal/faker-mapping-selector.tsx +++ b/packages/compass-collection/src/components/mock-data-generator-modal/faker-mapping-selector.tsx @@ -2,12 +2,13 @@ import { Banner, BannerVariant, Body, + Code, css, + Label, Option, palette, Select, spacing, - TextInput, } from '@mongodb-js/compass-components'; import React from 'react'; import { UNRECOGNIZED_FAKER_METHOD } from '../../modules/collection-tab'; @@ -26,92 +27,11 @@ const labelStyles = css({ fontWeight: 600, }); -/** - * Renders read-only TextInput components for each key-value pair in a faker arguments object. - */ -const getFakerArgsInputFromObject = (fakerArgsObject: Record) => { - return Object.entries(fakerArgsObject).map(([key, item]: [string, any]) => { - if (typeof item === 'string' || typeof item === 'boolean') { - return ( - - ); - } else if (typeof item === 'number') { - return ( - - ); - } else if ( - Array.isArray(item) && - item.length > 0 && - typeof item[0] === 'string' - ) { - return ( - - ); - } - return null; - }); -}; - -/** - * Renders TextInput components for each faker argument based on its type. - */ -const getFakerArgsInput = (fakerArgs: FakerArg[]) => { - return fakerArgs.map((arg, idx) => { - if (typeof arg === 'string' || typeof arg === 'boolean') { - return ( - - ); - } else if (typeof arg === 'number') { - return ( - - ); - } else if (typeof arg === 'object' && 'json' in arg) { - // parse the object - let parsedArg; - try { - parsedArg = JSON.parse(arg.json); - } catch { - // If parsing fails, skip rendering this arg - return null; - } - if (typeof parsedArg === 'object') { - return getFakerArgsInputFromObject(parsedArg); - } - } - }); +const formatFakerFunctionCallWithArgs = ( + fakerFunction: string, + fakerArgs: FakerArg[] +) => { + return `faker.${fakerFunction}(${fakerArgs.join(', ')})`; }; interface Props { @@ -158,13 +78,28 @@ const FakerMappingSelector = ({ ))} - {activeFakerFunction === UNRECOGNIZED_FAKER_METHOD && ( + {activeFakerFunction === UNRECOGNIZED_FAKER_METHOD ? ( Please select a function or we will default fill this field with the string "Unrecognized" + ) : ( + <> + + + {formatFakerFunctionCallWithArgs( + activeFakerFunction, + activeFakerArgs + )} + + )} - {getFakerArgsInput(activeFakerArgs)} ); }; diff --git a/packages/compass-collection/src/components/mock-data-generator-modal/utils.spec.ts b/packages/compass-collection/src/components/mock-data-generator-modal/utils.spec.ts index b1054c93e1c..c576b0c0773 100644 --- a/packages/compass-collection/src/components/mock-data-generator-modal/utils.spec.ts +++ b/packages/compass-collection/src/components/mock-data-generator-modal/utils.spec.ts @@ -37,6 +37,18 @@ describe('Mock Data Generator Utils', () => { expect(areFakerArgsValid([longStr])).to.be.false; }); + it('returns false for numbers that are too large', () => { + expect(areFakerArgsValid([1001])).to.be.false; + expect(areFakerArgsValid([-1001])).to.be.false; + expect(areFakerArgsValid([{ json: { length: 1001 } } as any])).to.be + .false; + expect(areFakerArgsValid([{ json: { length: -1001 } } as any])).to.be + .false; + expect( + areFakerArgsValid([{ json: { width: 1001, height: 1001 } } as any]) + ).to.be.false; + }); + it('returns true for nested valid arrays', () => { expect( areFakerArgsValid([ diff --git a/packages/compass-collection/src/components/mock-data-generator-modal/utils.ts b/packages/compass-collection/src/components/mock-data-generator-modal/utils.ts index 8a1a41ca628..2da644e04e8 100644 --- a/packages/compass-collection/src/components/mock-data-generator-modal/utils.ts +++ b/packages/compass-collection/src/components/mock-data-generator-modal/utils.ts @@ -5,6 +5,7 @@ import { faker } from '@faker-js/faker/locale/en'; const MAX_FAKER_ARGS_LENGTH = 10; const MAX_FAKER_STRING_LENGTH = 1000; const MAX_FAKER_ARGS_DEPTH = 3; +const MAX_FAKER_NUMBER_SIZE = 1000; /** * Checks if the provided faker arguments are valid. @@ -30,23 +31,28 @@ export function areFakerArgsValid( if (arg === null || arg === undefined) { return false; } - if (typeof arg === 'number') { - if (!Number.isFinite(arg)) { + if (typeof arg === 'boolean') { + // booleans are always valid, continue + continue; + } else if (typeof arg === 'number') { + if (!Number.isFinite(arg) || Math.abs(arg) > MAX_FAKER_NUMBER_SIZE) { return false; } } else if (typeof arg === 'string') { if (arg.length > MAX_FAKER_STRING_LENGTH) { return false; } - } else if (typeof arg === 'boolean') { - // booleans are always valid, continue } else if (Array.isArray(arg)) { if (!areFakerArgsValid(arg, depth + 1)) { return false; } - } else if (typeof arg === 'object' && typeof arg.json === 'string') { + } else if ( + typeof arg === 'object' && + arg !== null && + typeof (arg as { json?: unknown }).json === 'string' + ) { try { - const parsedJson = JSON.parse(arg.json); + const parsedJson = JSON.parse((arg as { json: string }).json); if (!areFakerArgsValid(Object.values(parsedJson), depth + 1)) { return false; } From 06258a86044d631a5dc966378a9a1948dfd38b47 Mon Sep 17 00:00:00 2001 From: Nataly Carbonell Date: Thu, 25 Sep 2025 14:25:20 -0400 Subject: [PATCH 06/12] increase max number size, allow faker.helpers.arrayElements, parse args before call, update ui label --- .../faker-mapping-selector.tsx | 20 ++++++++++++---- .../mock-data-generator-modal/utils.spec.ts | 10 ++++---- .../mock-data-generator-modal/utils.ts | 24 +++++++++++++++---- 3 files changed, 40 insertions(+), 14 deletions(-) diff --git a/packages/compass-collection/src/components/mock-data-generator-modal/faker-mapping-selector.tsx b/packages/compass-collection/src/components/mock-data-generator-modal/faker-mapping-selector.tsx index bf3d89d0fc2..1597a5391a1 100644 --- a/packages/compass-collection/src/components/mock-data-generator-modal/faker-mapping-selector.tsx +++ b/packages/compass-collection/src/components/mock-data-generator-modal/faker-mapping-selector.tsx @@ -27,11 +27,23 @@ const labelStyles = css({ fontWeight: 600, }); +const parseFakerArg = (arg: FakerArg): string => { + if (typeof arg === 'object' && arg !== null && 'json' in arg) { + try { + return JSON.stringify(JSON.parse(arg.json)); + } catch { + return ''; + } + } + return arg.toString(); +}; + const formatFakerFunctionCallWithArgs = ( fakerFunction: string, fakerArgs: FakerArg[] ) => { - return `faker.${fakerFunction}(${fakerArgs.join(', ')})`; + const parsedFakerArgs = fakerArgs.map(parseFakerArg); + return `faker.${fakerFunction}(${parsedFakerArgs.join(', ')})`; }; interface Props { @@ -85,11 +97,11 @@ const FakerMappingSelector = ({ ) : ( <> -