Skip to content

Commit 0b079fb

Browse files
ncarbonncarbon
committed
update faker argument handling and improve validation logging
1 parent 406aa2b commit 0b079fb

File tree

4 files changed

+184
-105
lines changed

4 files changed

+184
-105
lines changed

‎packages/compass-collection/src/components/mock-data-generator-modal/faker-mapping-selector.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ const getFakerArgsInput = (fakerArgs: FakerArg[]) => {
8484
key={`faker-arg-${idx}`}
8585
type="text"
8686
label="Faker Arg"
87-
required
87+
readOnly
8888
value={arg.toString()}
8989
/>
9090
);

‎packages/compass-collection/src/components/mock-data-generator-modal/utils.spec.ts‎

Lines changed: 133 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,18 @@
11
import { expect } from 'chai';
22
import { areFakerArgsValid, isValidFakerMethod } from './utils';
33

4-
describe('Mock Data Generator Utils', function () {
4+
import Sinon from 'sinon';
5+
import { faker } from '@faker-js/faker/locale/en';
6+
import { createNoopLogger } from '@mongodb-js/compass-logging/provider';
7+
8+
describe('Mock Data Generator Utils', () => {
9+
const sandbox = Sinon.createSandbox();
10+
const logger = createNoopLogger();
11+
12+
afterEach(() => {
13+
sandbox.restore();
14+
});
15+
516
describe('areFakerArgsValid', () => {
617
it('returns true for empty array', () => {
718
expect(areFakerArgsValid([])).to.be.true;
@@ -79,93 +90,141 @@ describe('Mock Data Generator Utils', function () {
7990
expect(areFakerArgsValid([obj])).to.be.false;
8091
});
8192

82-
describe('isValidFakerMethod', () => {
83-
it('returns false for invalid method format', () => {
84-
expect(isValidFakerMethod('invalidMethod', [])).to.deep.equal({
85-
isValid: false,
86-
fakerArgs: [],
87-
});
88-
expect(isValidFakerMethod('internet.email.extra', [])).to.deep.equal({
89-
isValid: false,
90-
fakerArgs: [],
91-
});
92-
});
93+
it('returns false for deeply nested invalid structures with max depth', () => {
94+
const obj = {
95+
json: JSON.stringify({
96+
a: [
97+
1,
98+
{
99+
json: JSON.stringify({
100+
b: 2,
101+
c: {
102+
json: JSON.stringify({
103+
d: 3,
104+
}),
105+
},
106+
}),
107+
},
108+
],
109+
}),
110+
};
111+
expect(areFakerArgsValid([obj])).to.be.false;
112+
});
113+
});
93114

94-
it('returns false for non-existent faker module', () => {
95-
expect(isValidFakerMethod('notamodule.email', [])).to.deep.equal({
96-
isValid: false,
97-
fakerArgs: [],
98-
});
99-
});
115+
describe('isValidFakerMethod', () => {
116+
it('returns false for invalid method format', () => {
117+
sandbox.stub(faker.internet, 'email');
100118

101-
it('returns false for non-existent faker method', () => {
102-
expect(isValidFakerMethod('internet.notamethod', [])).to.deep.equal({
103-
isValid: false,
104-
fakerArgs: [],
105-
});
119+
expect(isValidFakerMethod('invalidMethod', [], logger)).to.deep.equal({
120+
isValid: false,
121+
fakerArgs: [],
106122
});
107-
108-
it('returns true for valid method without arguments', () => {
109-
const result = isValidFakerMethod('internet.email', []);
110-
expect(result.isValid).to.be.true;
111-
expect(result.fakerArgs).to.deep.equal([]);
123+
expect(
124+
isValidFakerMethod('internet.email.extra', [], logger)
125+
).to.deep.equal({
126+
isValid: false,
127+
fakerArgs: [],
112128
});
129+
});
113130

114-
it('returns true for valid method with valid arguments', () => {
115-
// name.firstName takes optional gender argument
116-
const result = isValidFakerMethod('name.firstName', ['female']);
117-
expect(result.isValid).to.be.true;
118-
expect(result.fakerArgs).to.deep.equal(['female']);
131+
it('returns false for non-existent faker module', () => {
132+
expect(isValidFakerMethod('notamodule.email', [], logger)).to.deep.equal({
133+
isValid: false,
134+
fakerArgs: [],
119135
});
136+
});
120137

121-
it('returns true for valid method with no args if args are invalid but fallback works', () => {
122-
// internet.email does not take arguments, so passing one should fallback to []
123-
const result = isValidFakerMethod('internet.email', []);
124-
expect(result.isValid).to.be.true;
125-
expect(result.fakerArgs).to.deep.equal([]);
138+
it('returns false for non-existent faker method', () => {
139+
expect(
140+
isValidFakerMethod('internet.notamethod', [], logger)
141+
).to.deep.equal({
142+
isValid: false,
143+
fakerArgs: [],
126144
});
145+
});
127146

128-
it('returns false for valid method with invalid arguments and fallback fails', () => {
129-
// date.month expects at most one argument, passing an object will fail both attempts
130-
const result = isValidFakerMethod('date.month', [
131-
{ foo: 'bar' } as any,
132-
]);
133-
expect(result.isValid).to.be.false;
134-
expect(result.fakerArgs).to.deep.equal([]);
135-
});
147+
it('returns true for valid method without arguments', () => {
148+
sandbox.stub(faker.internet, 'email').returns('[email protected]');
136149

137-
it('returns false for helpers methods except arrayElement', () => {
138-
expect(isValidFakerMethod('helpers.fake', [])).to.deep.equal({
139-
isValid: false,
140-
fakerArgs: [],
141-
});
142-
expect(isValidFakerMethod('helpers.slugify', [])).to.deep.equal({
143-
isValid: false,
144-
fakerArgs: [],
145-
});
146-
});
150+
const result = isValidFakerMethod('internet.email', [], logger);
151+
expect(result.isValid).to.be.true;
152+
expect(result.fakerArgs).to.deep.equal([]);
153+
});
147154

148-
it('returns true for helpers.arrayElement with valid arguments', () => {
149-
const arr = ['a', 'b', 'c'];
150-
const result = isValidFakerMethod('helpers.arrayElement', [arr]);
151-
expect(result.isValid).to.be.true;
152-
expect(result.fakerArgs).to.deep.equal([arr]);
153-
});
155+
it('returns true for valid method with valid arguments', () => {
156+
sandbox.stub(faker.person, 'firstName');
154157

155-
it('returns false for helpers.arrayElement with invalid arguments', () => {
156-
// Exceeding max args length
157-
const arr = Array(11).fill('x');
158-
const result = isValidFakerMethod('helpers.arrayElement', [arr]);
159-
expect(result.isValid).to.be.false;
160-
expect(result.fakerArgs).to.deep.equal([]);
161-
});
158+
const result = isValidFakerMethod('person.firstName', ['female'], logger);
159+
expect(result.isValid).to.be.true;
160+
expect(result.fakerArgs).to.deep.equal(['female']);
161+
});
162+
163+
it('returns true for valid method with no args if args are invalid but fallback works', () => {
164+
sandbox.stub(faker.internet, 'email').returns('[email protected]');
165+
166+
const result = isValidFakerMethod('internet.email', [], logger);
167+
expect(result.isValid).to.be.true;
168+
expect(result.fakerArgs).to.deep.equal([]);
169+
});
162170

163-
it('returns false for method with invalid fakerArgs', () => {
164-
// Passing Infinity as argument
165-
const result = isValidFakerMethod('name.firstName', [Infinity]);
166-
expect(result.isValid).to.be.false;
167-
expect(result.fakerArgs).to.deep.equal([]);
171+
it('returns true valid method with invalid arguments and strips args', () => {
172+
sandbox.stub(faker.date, 'month').returns('February');
173+
174+
const result = isValidFakerMethod(
175+
'date.month',
176+
[{ foo: 'bar' } as any],
177+
logger
178+
);
179+
expect(result.isValid).to.be.true;
180+
expect(result.fakerArgs).to.deep.equal([]);
181+
});
182+
183+
it('returns false for helpers methods except arrayElement', () => {
184+
expect(isValidFakerMethod('helpers.fake', [], logger)).to.deep.equal({
185+
isValid: false,
186+
fakerArgs: [],
168187
});
188+
expect(isValidFakerMethod('helpers.slugify', [], logger)).to.deep.equal({
189+
isValid: false,
190+
fakerArgs: [],
191+
});
192+
});
193+
194+
it('returns true for helpers.arrayElement with valid arguments', () => {
195+
sandbox.stub(faker.helpers, 'arrayElement').returns('a');
196+
197+
const arr = ['a', 'b', 'c'];
198+
const result = isValidFakerMethod('helpers.arrayElement', [arr], logger);
199+
expect(result.isValid).to.be.true;
200+
expect(result.fakerArgs).to.deep.equal([arr]);
201+
});
202+
203+
it('returns false for helpers.arrayElement with invalid arguments', () => {
204+
// Exceeding max args length
205+
const arr = Array(11).fill('x');
206+
const result = isValidFakerMethod('helpers.arrayElement', [arr], logger);
207+
expect(result.isValid).to.be.false;
208+
expect(result.fakerArgs).to.deep.equal([]);
209+
});
210+
211+
it('returns true for valid method with invalid fakerArgs and strips args', () => {
212+
sandbox.stub(faker.person, 'firstName').returns('a');
213+
214+
// Passing Infinity as argument
215+
const result = isValidFakerMethod('person.firstName', [Infinity], logger);
216+
expect(result.isValid).to.be.true;
217+
expect(result.fakerArgs).to.deep.equal([]);
218+
});
219+
220+
it('returns false when calling a faker method fails', () => {
221+
sandbox
222+
.stub(faker.person, 'firstName')
223+
.throws(new Error('Invalid faker method'));
224+
225+
const result = isValidFakerMethod('person.firstName', [], logger);
226+
expect(result.isValid).to.be.false;
227+
expect(result.fakerArgs).to.deep.equal([]);
169228
});
170229
});
171230
});

‎packages/compass-collection/src/components/mock-data-generator-modal/utils.ts‎

Lines changed: 48 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
1+
import { type Logger, mongoLogId } from '@mongodb-js/compass-logging/provider';
12
import type { FakerArg } from './script-generation-utils';
23
import { faker } from '@faker-js/faker/locale/en';
34

45
const MAX_FAKER_ARGS_LENGTH = 10;
56
const MAX_FAKER_STRING_LENGTH = 1000;
7+
const MAX_FAKER_ARGS_DEPTH = 3;
68

79
/**
810
* Checks if the provided faker arguments are valid.
@@ -11,7 +13,13 @@ const MAX_FAKER_STRING_LENGTH = 1000;
1113
* - Arrays must not exceed max length and all elements must be valid
1214
* - Objects must have a 'json' property that is a valid JSON string and all values must be valid
1315
*/
14-
export function areFakerArgsValid(fakerArgs: FakerArg[]): boolean {
16+
export function areFakerArgsValid(
17+
fakerArgs: FakerArg[],
18+
depth: number = 0
19+
): boolean {
20+
if (depth > MAX_FAKER_ARGS_DEPTH) {
21+
return false;
22+
}
1523
if (fakerArgs.length === 0) {
1624
return true;
1725
}
@@ -33,13 +41,13 @@ export function areFakerArgsValid(fakerArgs: FakerArg[]): boolean {
3341
} else if (typeof arg === 'boolean') {
3442
// booleans are always valid, continue
3543
} else if (Array.isArray(arg)) {
36-
if (!areFakerArgsValid(arg)) {
44+
if (!areFakerArgsValid(arg, depth + 1)) {
3745
return false;
3846
}
3947
} else if (typeof arg === 'object' && typeof arg.json === 'string') {
4048
try {
4149
const parsedJson = JSON.parse(arg.json);
42-
if (!areFakerArgsValid(Object.values(parsedJson))) {
50+
if (!areFakerArgsValid(Object.values(parsedJson), depth + 1)) {
4351
return false;
4452
}
4553
} catch {
@@ -65,7 +73,8 @@ export function areFakerArgsValid(fakerArgs: FakerArg[]): boolean {
6573
*/
6674
export function isValidFakerMethod(
6775
fakerMethod: string,
68-
fakerArgs: FakerArg[]
76+
fakerArgs: FakerArg[],
77+
logger: Logger
6978
): {
7079
isValid: boolean;
7180
fakerArgs: FakerArg[];
@@ -80,10 +89,15 @@ export function isValidFakerMethod(
8089
isAllowedHelper(moduleName, methodName) &&
8190
canInvokeFakerMethod(fakerModule, methodName)
8291
) {
83-
const callableFakerMethod = (fakerModule as Record<string, any>)[
84-
methodName
85-
];
86-
return tryInvokeFakerMethod(callableFakerMethod, fakerArgs);
92+
const callableFakerMethod = (
93+
fakerModule as Record<string, (...args: any[]) => any>
94+
)[methodName];
95+
return tryInvokeFakerMethod(
96+
callableFakerMethod,
97+
fakerMethod,
98+
fakerArgs,
99+
logger
100+
);
87101
} else {
88102
return { isValid: false, fakerArgs: [] };
89103
}
@@ -103,38 +117,43 @@ function isAllowedHelper(moduleName: string, methodName: string) {
103117
return moduleName !== 'helpers' || methodName === 'arrayElement';
104118
}
105119

106-
function canInvokeFakerMethod(fakerModule: any, methodName: string) {
120+
function canInvokeFakerMethod(fakerModule: unknown, methodName: string) {
107121
return (
108122
fakerModule !== null &&
109123
fakerModule !== undefined &&
110124
typeof fakerModule === 'object' &&
111-
typeof fakerModule[methodName] === 'function'
125+
typeof (fakerModule as Record<string, unknown>)[methodName] === 'function'
112126
);
113127
}
114128

115129
function tryInvokeFakerMethod(
116-
callable: (...args: any[]) => any,
117-
args: FakerArg[]
130+
callable: (...args: readonly unknown[]) => unknown,
131+
fakerMethod: string,
132+
args: FakerArg[],
133+
logger: Logger
118134
) {
119-
try {
120-
if (areFakerArgsValid(args)) {
135+
// If args are present and safe, try calling with args
136+
if (args.length > 0 && areFakerArgsValid(args)) {
137+
try {
121138
callable(...args);
122139
return { isValid: true, fakerArgs: args };
123-
} else {
124-
callable();
125-
return { isValid: true, fakerArgs: [] };
126-
}
127-
} catch {
128-
// Intentionally ignored: error may be due to invalid arguments, will retry without args.
129-
if (args.length > 0) {
130-
try {
131-
callable();
132-
return { isValid: true, fakerArgs: [] };
133-
} catch {
134-
// Intentionally ignored: method is invalid without arguments as well.
135-
return { isValid: false, fakerArgs: [] };
136-
}
140+
} catch {
141+
// Call with args failed. Fall through to trying without args
137142
}
138143
}
139-
return { isValid: false, fakerArgs: [] };
144+
145+
// Try without args (either because args were invalid or args failed)
146+
try {
147+
callable();
148+
return { isValid: true, fakerArgs: [] };
149+
} catch (error) {
150+
// Calling the method without arguments failed.
151+
logger.log.warn(
152+
mongoLogId(1_001_000_377),
153+
'Collection',
154+
'Invalid faker method',
155+
{ error, fakerMethod, fakerArgs: args }
156+
);
157+
return { isValid: false, fakerArgs: [] };
158+
}
140159
}

‎packages/compass-collection/src/modules/collection-tab.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -745,7 +745,8 @@ const validateFakerSchema = (
745745
// Validate the faker method
746746
const { isValid: isValidMethod, fakerArgs } = isValidFakerMethod(
747747
fakerMapping.fakerMethod,
748-
fakerMapping.fakerArgs
748+
fakerMapping.fakerArgs,
749+
logger
749750
);
750751
if (isValidMethod) {
751752
result[fieldPath] = {

0 commit comments

Comments
 (0)