From c0990b0fe101a43cdf4de4fdae3433a2b9d4a786 Mon Sep 17 00:00:00 2001 From: hkobew Date: Wed, 23 Apr 2025 18:40:27 -0400 Subject: [PATCH 1/3] feat: add value length cap to partialClone --- .../amazonq/test/e2e/inline/inline.test.ts | 2 +- packages/core/src/auth/sso/clients.ts | 4 +- .../src/shared/utilities/collectionUtils.ts | 21 +++-- .../src/shared/utilities/textUtilities.ts | 2 +- packages/core/src/shared/vscode/commands2.ts | 2 +- .../shared/utilities/collectionUtils.test.ts | 85 +++++++++++-------- 6 files changed, 71 insertions(+), 45 deletions(-) diff --git a/packages/amazonq/test/e2e/inline/inline.test.ts b/packages/amazonq/test/e2e/inline/inline.test.ts index 57c6e1c4996..43a9f67ab73 100644 --- a/packages/amazonq/test/e2e/inline/inline.test.ts +++ b/packages/amazonq/test/e2e/inline/inline.test.ts @@ -122,7 +122,7 @@ describe('Amazon Q Inline', async function () { .query({ metricName: 'codewhisperer_userTriggerDecision', }) - .map((e) => collectionUtil.partialClone(e, 3, ['credentialStartUrl'], '[omitted]')) + .map((e) => collectionUtil.partialClone(e, 3, ['credentialStartUrl'], { replacement: '[omitted]' })) } for (const [name, invokeCompletion] of [ diff --git a/packages/core/src/auth/sso/clients.ts b/packages/core/src/auth/sso/clients.ts index 01d0e031d04..e050bdc793e 100644 --- a/packages/core/src/auth/sso/clients.ts +++ b/packages/core/src/auth/sso/clients.ts @@ -258,7 +258,7 @@ function addLoggingMiddleware(client: SSOOIDCClient) { args.input as unknown as Record, 3, ['clientSecret', 'accessToken', 'refreshToken'], - '[omitted]' + { replacement: '[omitted]' } ) getLogger().debug('API request (%s %s): %O', hostname, path, input) } @@ -288,7 +288,7 @@ function addLoggingMiddleware(client: SSOOIDCClient) { result.output as unknown as Record, 3, ['clientSecret', 'accessToken', 'refreshToken'], - '[omitted]' + { replacement: '[omitted]' } ) getLogger().debug('API response (%s %s): %O', hostname, path, output) } diff --git a/packages/core/src/shared/utilities/collectionUtils.ts b/packages/core/src/shared/utilities/collectionUtils.ts index 9f9fe9875b9..929a9286dbd 100644 --- a/packages/core/src/shared/utilities/collectionUtils.ts +++ b/packages/core/src/shared/utilities/collectionUtils.ts @@ -7,6 +7,7 @@ import { isWeb } from '../extensionGlobals' import { inspect as nodeInspect } from 'util' import { AsyncCollection, toCollection } from './asyncCollection' import { SharedProp, AccumulableKeys, Coalesce, isNonNullable } from './tsUtils' +import { truncate } from './textUtilities' export function union(a: Iterable, b: Iterable): Set { const result = new Set() @@ -304,26 +305,36 @@ export function assign, U extends Partial>(data: T * @param depth * @param omitKeys Omit properties matching these names (at any depth). * @param replacement Replacement for object whose fields extend beyond `depth`, and properties matching `omitKeys`. + * @param maxLength truncates string values that exceed this threshold. */ -export function partialClone(obj: any, depth: number = 3, omitKeys: string[] = [], replacement?: any): any { +export function partialClone( + obj: any, + depth: number = 3, + omitKeys: string[] = [], + options?: { + replacement?: any + maxLength?: number + } +): any { // Base case: If input is not an object or has no children, return it. if (typeof obj !== 'object' || obj === null || 0 === Object.getOwnPropertyNames(obj).length) { - return obj + const maxLength = options?.maxLength + return typeof obj === 'string' && maxLength !== undefined ? truncate(obj, maxLength, '...') : obj } // Create a new object of the same type as the input object. const clonedObj = Array.isArray(obj) ? [] : {} if (depth === 0) { - return replacement ? replacement : clonedObj + return options?.replacement ? options.replacement : clonedObj } // Recursively clone properties of the input object for (const key in obj) { if (omitKeys.includes(key)) { - ;(clonedObj as any)[key] = replacement ? replacement : Array.isArray(obj) ? [] : {} + ;(clonedObj as any)[key] = options?.replacement ? options.replacement : Array.isArray(obj) ? [] : {} } else if (Object.prototype.hasOwnProperty.call(obj, key)) { - ;(clonedObj as any)[key] = partialClone(obj[key], depth - 1, omitKeys, replacement) + ;(clonedObj as any)[key] = partialClone(obj[key], depth - 1, omitKeys, options) } } diff --git a/packages/core/src/shared/utilities/textUtilities.ts b/packages/core/src/shared/utilities/textUtilities.ts index 53c2e2be32c..ed1e1619122 100644 --- a/packages/core/src/shared/utilities/textUtilities.ts +++ b/packages/core/src/shared/utilities/textUtilities.ts @@ -10,7 +10,7 @@ import { default as stripAnsi } from 'strip-ansi' import { getLogger } from '../logger/logger' /** - * Truncates string `s` if it exceeds `n` chars. + * Truncates string `s` if it has or exceeds `n` chars. * * If `n` is negative, truncates at start instead of end. * diff --git a/packages/core/src/shared/vscode/commands2.ts b/packages/core/src/shared/vscode/commands2.ts index c55cd66cc7a..b40134c2afa 100644 --- a/packages/core/src/shared/vscode/commands2.ts +++ b/packages/core/src/shared/vscode/commands2.ts @@ -653,7 +653,7 @@ async function runCommand(fn: T, info: CommandInfo): Prom logger.debug( `command: running ${label} with arguments: %O`, - partialClone(args, 3, ['clientSecret', 'accessToken', 'refreshToken', 'tooltip'], '[omitted]') + partialClone(args, 3, ['clientSecret', 'accessToken', 'refreshToken', 'tooltip'], { replacement: '[omitted]' }) ) try { diff --git a/packages/core/src/test/shared/utilities/collectionUtils.test.ts b/packages/core/src/test/shared/utilities/collectionUtils.test.ts index 53ddc39eff8..ed5fb707808 100644 --- a/packages/core/src/test/shared/utilities/collectionUtils.test.ts +++ b/packages/core/src/test/shared/utilities/collectionUtils.test.ts @@ -710,8 +710,10 @@ describe('CollectionUtils', async function () { }) describe('partialClone', function () { - it('omits properties by depth', function () { - const testObj = { + let multipleTypedObj: object + + before(async function () { + multipleTypedObj = { a: 34234234234, b: '123456789', c: new Date(2023, 1, 1), @@ -724,56 +726,69 @@ describe('CollectionUtils', async function () { throw Error() }, } + }) - assert.deepStrictEqual(partialClone(testObj, 1), { - ...testObj, + it('omits properties by depth', function () { + assert.deepStrictEqual(partialClone(multipleTypedObj, 1), { + ...multipleTypedObj, d: {}, e: {}, }) - assert.deepStrictEqual(partialClone(testObj, 0, [], '[replaced]'), '[replaced]') - assert.deepStrictEqual(partialClone(testObj, 1, [], '[replaced]'), { - ...testObj, + assert.deepStrictEqual(partialClone(multipleTypedObj, 0, [], { replacement: '[replaced]' }), '[replaced]') + assert.deepStrictEqual(partialClone(multipleTypedObj, 1, [], { replacement: '[replaced]' }), { + ...multipleTypedObj, d: '[replaced]', e: '[replaced]', }) - assert.deepStrictEqual(partialClone(testObj, 3), { - ...testObj, + assert.deepStrictEqual(partialClone(multipleTypedObj, 3), { + ...multipleTypedObj, d: { d1: { d2: {} } }, }) - assert.deepStrictEqual(partialClone(testObj, 4), testObj) + assert.deepStrictEqual(partialClone(multipleTypedObj, 4), multipleTypedObj) }) it('omits properties by name', function () { - const testObj = { - a: 34234234234, - b: '123456789', - c: new Date(2023, 1, 1), - d: { d1: { d2: { d3: 'deep' } } }, - e: { - e1: [4, 3, 7], - e2: 'loooooooooo \n nnnnnnnnnnn \n gggggggg \n string', - }, - f: () => { - throw Error() - }, - } - - assert.deepStrictEqual(partialClone(testObj, 2, ['c', 'e2'], '[omitted]'), { - ...testObj, - c: '[omitted]', - d: { d1: '[omitted]' }, + assert.deepStrictEqual(partialClone(multipleTypedObj, 2, ['c', 'e2'], { replacement: '[replaced]' }), { + ...multipleTypedObj, + c: '[replaced]', + d: { d1: '[replaced]' }, e: { - e1: '[omitted]', - e2: '[omitted]', + e1: '[replaced]', + e2: '[replaced]', }, }) - assert.deepStrictEqual(partialClone(testObj, 3, ['c', 'e2'], '[omitted]'), { - ...testObj, - c: '[omitted]', - d: { d1: { d2: '[omitted]' } }, + assert.deepStrictEqual(partialClone(multipleTypedObj, 3, ['c', 'e2'], { replacement: '[replaced]' }), { + ...multipleTypedObj, + c: '[replaced]', + d: { d1: { d2: '[replaced]' } }, e: { e1: [4, 3, 7], - e2: '[omitted]', + e2: '[replaced]', + }, + }) + }) + + it('truncates properties by maxLength', function () { + const testObj = { + a: '1', + b: '11', + c: '11111', + d: { + e: { + a: '11111', + b: '11', + }, + }, + } + assert.deepStrictEqual(partialClone(testObj, 5, [], { maxLength: 2 }), { + a: '1', + b: '11', + c: '11...', + d: { + e: { + a: '11...', + b: '11', + }, }, }) }) From 41768b09a2b54935aa8054d313b051a42c7aa13a Mon Sep 17 00:00:00 2001 From: hkobew Date: Wed, 23 Apr 2025 19:21:44 -0400 Subject: [PATCH 2/3] style: change paramter name to be more specific --- packages/core/src/shared/utilities/collectionUtils.ts | 6 +++--- .../core/src/test/shared/utilities/collectionUtils.test.ts | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/core/src/shared/utilities/collectionUtils.ts b/packages/core/src/shared/utilities/collectionUtils.ts index 929a9286dbd..8e210974059 100644 --- a/packages/core/src/shared/utilities/collectionUtils.ts +++ b/packages/core/src/shared/utilities/collectionUtils.ts @@ -305,7 +305,7 @@ export function assign, U extends Partial>(data: T * @param depth * @param omitKeys Omit properties matching these names (at any depth). * @param replacement Replacement for object whose fields extend beyond `depth`, and properties matching `omitKeys`. - * @param maxLength truncates string values that exceed this threshold. + * @param maxStringLength truncates string values that exceed this threshold. */ export function partialClone( obj: any, @@ -313,12 +313,12 @@ export function partialClone( omitKeys: string[] = [], options?: { replacement?: any - maxLength?: number + maxStringLength?: number } ): any { // Base case: If input is not an object or has no children, return it. if (typeof obj !== 'object' || obj === null || 0 === Object.getOwnPropertyNames(obj).length) { - const maxLength = options?.maxLength + const maxLength = options?.maxStringLength return typeof obj === 'string' && maxLength !== undefined ? truncate(obj, maxLength, '...') : obj } diff --git a/packages/core/src/test/shared/utilities/collectionUtils.test.ts b/packages/core/src/test/shared/utilities/collectionUtils.test.ts index ed5fb707808..1a6d717dc4b 100644 --- a/packages/core/src/test/shared/utilities/collectionUtils.test.ts +++ b/packages/core/src/test/shared/utilities/collectionUtils.test.ts @@ -780,7 +780,7 @@ describe('CollectionUtils', async function () { }, }, } - assert.deepStrictEqual(partialClone(testObj, 5, [], { maxLength: 2 }), { + assert.deepStrictEqual(partialClone(testObj, 5, [], { maxStringLength: 2 }), { a: '1', b: '11', c: '11...', From c49b272b8912e4e8bf1bb26f38cb30a743bc61cd Mon Sep 17 00:00:00 2001 From: hkobew Date: Thu, 24 Apr 2025 06:52:36 -0400 Subject: [PATCH 3/3] test: add more types to test --- .../src/shared/utilities/collectionUtils.ts | 8 ++-- .../shared/utilities/collectionUtils.test.ts | 38 ++++++++++++------- 2 files changed, 29 insertions(+), 17 deletions(-) diff --git a/packages/core/src/shared/utilities/collectionUtils.ts b/packages/core/src/shared/utilities/collectionUtils.ts index 8e210974059..8a428b8e8b7 100644 --- a/packages/core/src/shared/utilities/collectionUtils.ts +++ b/packages/core/src/shared/utilities/collectionUtils.ts @@ -305,7 +305,7 @@ export function assign, U extends Partial>(data: T * @param depth * @param omitKeys Omit properties matching these names (at any depth). * @param replacement Replacement for object whose fields extend beyond `depth`, and properties matching `omitKeys`. - * @param maxStringLength truncates string values that exceed this threshold. + * @param maxStringLength truncates string values that exceed this threshold (includes values in nested arrays) */ export function partialClone( obj: any, @@ -318,8 +318,10 @@ export function partialClone( ): any { // Base case: If input is not an object or has no children, return it. if (typeof obj !== 'object' || obj === null || 0 === Object.getOwnPropertyNames(obj).length) { - const maxLength = options?.maxStringLength - return typeof obj === 'string' && maxLength !== undefined ? truncate(obj, maxLength, '...') : obj + if (typeof obj === 'string' && options?.maxStringLength) { + return truncate(obj, options?.maxStringLength, '...') + } + return obj } // Create a new object of the same type as the input object. diff --git a/packages/core/src/test/shared/utilities/collectionUtils.test.ts b/packages/core/src/test/shared/utilities/collectionUtils.test.ts index 1a6d717dc4b..34aacb9f28e 100644 --- a/packages/core/src/test/shared/utilities/collectionUtils.test.ts +++ b/packages/core/src/test/shared/utilities/collectionUtils.test.ts @@ -770,26 +770,36 @@ describe('CollectionUtils', async function () { it('truncates properties by maxLength', function () { const testObj = { - a: '1', - b: '11', - c: '11111', - d: { - e: { - a: '11111', - b: '11', + strValue: '1', + boolValue: true, + longString: '11111', + nestedObj: { + nestedObjAgain: { + longNestedStr: '11111', + shortNestedStr: '11', }, }, + nestedObj2: { + functionValue: (_: unknown) => {}, + }, + nestedObj3: { + myArray: ['1', '11111', '1'], + }, + objInArray: [{ shortString: '11', longString: '11111' }], } assert.deepStrictEqual(partialClone(testObj, 5, [], { maxStringLength: 2 }), { - a: '1', - b: '11', - c: '11...', - d: { - e: { - a: '11...', - b: '11', + ...testObj, + longString: '11...', + nestedObj: { + nestedObjAgain: { + longNestedStr: '11...', + shortNestedStr: '11', }, }, + nestedObj3: { + myArray: ['1', '11...', '1'], + }, + objInArray: [{ shortString: '11', longString: '11...' }], }) }) })