From e1ba507fc84d5ae526fe0ee0a26ea4f039b63d03 Mon Sep 17 00:00:00 2001 From: George Fu Date: Mon, 26 Feb 2024 16:05:43 -0500 Subject: [PATCH] fix(lib-dynamodb): preserve collections when serializing class instances to map (#5826) * fix(lib-dynamodb): preserve collections when serializing class instances to map * test(lib-dynamodb): new e2e scenario for class conversion --- lib/lib-dynamodb/src/commands/utils.spec.ts | 146 +++++++++++++----- lib/lib-dynamodb/src/commands/utils.ts | 56 +++---- .../src/test/lib-dynamodb.e2e.spec.ts | 74 +++++++++ packages/util-dynamodb/src/convertToAttr.ts | 8 +- 4 files changed, 207 insertions(+), 77 deletions(-) diff --git a/lib/lib-dynamodb/src/commands/utils.spec.ts b/lib/lib-dynamodb/src/commands/utils.spec.ts index f9a50a2ff8c5..d2d044900070 100644 --- a/lib/lib-dynamodb/src/commands/utils.spec.ts +++ b/lib/lib-dynamodb/src/commands/utils.spec.ts @@ -152,32 +152,31 @@ describe("object with function property", () => { const keyNodes = { Item: {} }; const nativeAttrObj = { Item: { id: 1, func: () => {} }, ...notAttrValue }; const attrObj = { Item: { id: { N: "1" } }, ...notAttrValue }; + it("should remove functions", () => { expect( marshallInput(nativeAttrObj, keyNodes, { convertTopLevelContainer: true, convertClassInstanceToMap: true }) ).toEqual(attrObj); }); - // List of functions - const listOfFunctions = { Item: { id: 1, funcs: [() => {}, () => {}] }, ...notAttrValue }; it("should remove functions from lists", () => { + const listOfFunctions = { Item: { id: 1, funcs: [() => {}, () => {}] }, ...notAttrValue }; expect( marshallInput(listOfFunctions, keyNodes, { convertTopLevelContainer: true, convertClassInstanceToMap: true }) ).toEqual({ Item: { id: { N: "1" }, funcs: { L: [] } }, ...notAttrValue }); }); - // Nested list of functions - const nestedListOfFunctions = { - Item: { - id: 1, - funcs: [ - [() => {}, () => {}], - [() => {}, () => {}], - ], - }, - ...notAttrValue, - }; it("should remove functions from nested lists", () => { + const nestedListOfFunctions = { + Item: { + id: 1, + funcs: [ + [() => {}, () => {}], + [() => {}, () => {}], + ], + }, + ...notAttrValue, + }; expect( marshallInput(nestedListOfFunctions, keyNodes, { convertTopLevelContainer: true, @@ -186,25 +185,103 @@ describe("object with function property", () => { ).toEqual({ Item: { id: { N: "1" }, funcs: { L: [{ L: [] }, { L: [] }] } }, ...notAttrValue }); }); - // Nested list of functions 3 levels down - const nestedListOfFunctions3Levels = { - Item: { - id: 1, - funcs: [ - [ - [() => {}, () => {}], - [() => {}, () => {}], - ], - [ - [() => {}, () => {}], - [() => {}, () => {}], - ], - ], - }, - ...notAttrValue, - }; + it("should convert data class objects without affecting known data collection objects", () => { + const nestedListOfFunctions3Levels = { + Item: { + id: 1, + x: { + map: new Map([ + [1, 1], + [2, 2], + [3, 3], + ]), + set: new Set([1, 2, 3]), + binary: new Uint8Array([1, 2, 3]), + myPojo: new (class { + public a = 1; + public b = 2; + public c = 3; + public method() { + return "method"; + } + public get getter() { + return "getter"; + } + public arrowFn = () => "arrowFn"; + public ownFunction = function () { + return "ownFunction"; + }; + })(), + }, + }, + ...notAttrValue, + }; + expect( + marshallInput(nestedListOfFunctions3Levels, keyNodes, { + convertTopLevelContainer: true, + convertClassInstanceToMap: true, + }) + ).toEqual({ + Item: { + id: { N: "1" }, + x: { + M: { + binary: { + B: new Uint8Array([1, 2, 3]), + }, + map: { + M: { + "1": { + N: "1", + }, + "2": { + N: "2", + }, + "3": { + N: "3", + }, + }, + }, + myPojo: { + M: { + a: { + N: "1", + }, + b: { + N: "2", + }, + c: { + N: "3", + }, + }, + }, + set: { + NS: ["1", "2", "3"], + }, + }, + }, + }, + ...notAttrValue, + }); + }); it("should remove functions from a nested list of depth 3", () => { + const nestedListOfFunctions3Levels = { + Item: { + id: 1, + funcs: [ + [ + [() => {}, () => {}], + [() => {}, () => {}], + ], + [ + [() => {}, () => {}], + [() => {}, () => {}], + ], + ], + }, + ...notAttrValue, + }; expect( marshallInput(nestedListOfFunctions3Levels, keyNodes, { convertTopLevelContainer: true, @@ -227,13 +304,4 @@ describe("object with function property", () => { ...notAttrValue, }); }); - it("should throw when recursion depth has exceeded", () => { - const obj = {} as any; - obj.SELF = obj; - expect(() => marshallInput(obj, {}, { convertClassInstanceToMap: true })).toThrow( - new Error( - "Recursive copy depth exceeded 1000. Please set options.convertClassInstanceToMap to false and manually remove functions from your data object." - ) - ); - }); }); diff --git a/lib/lib-dynamodb/src/commands/utils.ts b/lib/lib-dynamodb/src/commands/utils.ts index 8f15fdc57239..8f9982e32702 100644 --- a/lib/lib-dynamodb/src/commands/utils.ts +++ b/lib/lib-dynamodb/src/commands/utils.ts @@ -50,7 +50,9 @@ const processObj = (obj: any, processFunc: Function, keyNodes?: KeyNodes): any = return processAllKeysInObj(obj, processFunc, SELF); } else if (goToNextLevel) { return Object.entries(obj ?? {}).reduce((acc, [k, v]) => { - acc[k] = processObj(v, processFunc, keyNodes[NEXT_LEVEL]); + if (typeof v !== "function") { + acc[k] = processObj(v, processFunc, keyNodes[NEXT_LEVEL]); + } return acc; }, (Array.isArray(obj) ? [] : {}) as any); } @@ -62,14 +64,22 @@ const processObj = (obj: any, processFunc: Function, keyNodes?: KeyNodes): any = const processKeysInObj = (obj: any, processFunc: Function, keyNodes: KeyNodeChildren) => { let accumulator: any; if (Array.isArray(obj)) { - accumulator = [...obj]; + accumulator = [...obj].filter((item) => typeof item !== "function"); } else { - accumulator = { ...obj }; + accumulator = {}; + for (const [k, v] of Object.entries(obj)) { + if (typeof v !== "function") { + accumulator[k] = v; + } + } } for (const [nodeKey, nodes] of Object.entries(keyNodes)) { + if (typeof obj[nodeKey] === "function") { + continue; + } const processedValue = processObj(obj[nodeKey], processFunc, nodes); - if (processedValue !== undefined) { + if (processedValue !== undefined && typeof processedValue !== "function") { accumulator[nodeKey] = processedValue; } } @@ -79,52 +89,26 @@ const processKeysInObj = (obj: any, processFunc: Function, keyNodes: KeyNodeChil const processAllKeysInObj = (obj: any, processFunc: Function, keyNodes: KeyNodes): any => { if (Array.isArray(obj)) { - return obj.map((item) => processObj(item, processFunc, keyNodes)); + return obj.filter((item) => typeof item !== "function").map((item) => processObj(item, processFunc, keyNodes)); } return Object.entries(obj).reduce((acc, [key, value]) => { + if (typeof value === "function") { + return acc; + } const processedValue = processObj(value, processFunc, keyNodes); - if (processedValue !== undefined) { + if (processedValue !== undefined && typeof processedValue !== "function") { acc[key] = processedValue; } return acc; }, {} as any); }; -function copyWithoutFunctions(o: any, depth = 0): any { - if (depth > 1000) { - throw new Error( - "Recursive copy depth exceeded 1000. Please set options.convertClassInstanceToMap to false and manually remove functions from your data object." - ); - } - if (typeof o === "object" || typeof o === "function") { - if (Array.isArray(o)) { - return o.filter((item) => typeof item !== "function").map((item) => copyWithoutFunctions(item, depth + 1)); - } - if (o === null) { - return null; - } - const copy = {} as any; - for (const [key, value] of Object.entries(o)) { - if (typeof value !== "function") { - copy[key] = copyWithoutFunctions(value, depth + 1); - } - } - return copy; - } else { - return o; - } -} - /** * @internal */ export const marshallInput = (obj: any, keyNodes: KeyNodeChildren, options?: marshallOptions) => { - let _obj = obj; - if (options?.convertClassInstanceToMap) { - _obj = copyWithoutFunctions(obj); - } const marshallFunc = (toMarshall: any) => marshall(toMarshall, options); - return processKeysInObj(_obj, marshallFunc, keyNodes); + return processKeysInObj(obj, marshallFunc, keyNodes); }; /** diff --git a/lib/lib-dynamodb/src/test/lib-dynamodb.e2e.spec.ts b/lib/lib-dynamodb/src/test/lib-dynamodb.e2e.spec.ts index a6a5792a57b9..7c26c54bc1a5 100644 --- a/lib/lib-dynamodb/src/test/lib-dynamodb.e2e.spec.ts +++ b/lib/lib-dynamodb/src/test/lib-dynamodb.e2e.spec.ts @@ -32,6 +32,7 @@ describe(DynamoDBDocument.name, () => { const doc = DynamoDBDocument.from(dynamodb, { marshallOptions: { convertTopLevelContainer: true, + convertClassInstanceToMap: true, }, unmarshallOptions: { wrapNumbers: true, @@ -75,6 +76,10 @@ describe(DynamoDBDocument.name, () => { update: {} as Record, updateReadBack: {} as Record, delete: {} as Record, + classInstanceConversion: { + write: null as null | PutCommandOutput, + read: null as null | GetCommandOutput, + }, }; const data = { @@ -439,6 +444,57 @@ describe(DynamoDBDocument.name, () => { }); })().catch(passError); } + + log.classInstanceConversion.write = await doc + .put({ + TableName, + Item: { + id: "classInstance", + data: { + a: new (class { + public a = 1; + public b = 2; + public c = 3; + public method() { + return "method"; + } + public get getter() { + return "getter"; + } + public arrowFn = () => "arrowFn"; + public ownFunction = function () { + return "ownFunction"; + }; + })(), + b: new (class { + public a = 4; + public b = 5; + public c = 6; + public method() { + return "method"; + } + public get getter() { + return "getter"; + } + public arrowFn = () => "arrowFn"; + public ownFunction = function () { + return "ownFunction"; + }; + })(), + }, + }, + }) + .catch(passError); + + log.classInstanceConversion.read = await doc + .get({ + ConsistentRead: true, + TableName, + Key: { + id: "classInstance", + }, + }) + .catch(passError); }); afterAll(async () => { @@ -635,4 +691,22 @@ describe(DynamoDBDocument.name, () => { expect(log.delete[key].$metadata).toBeDefined(); }); } + + it("can serialize class instances as maps", async () => { + expect(log.classInstanceConversion.read?.Item).toEqual({ + id: "classInstance", + data: { + a: { + a: NumberValue.from(1), + b: NumberValue.from(2), + c: NumberValue.from(3), + }, + b: { + a: NumberValue.from(4), + b: NumberValue.from(5), + c: NumberValue.from(6), + }, + }, + }); + }); }); diff --git a/packages/util-dynamodb/src/convertToAttr.ts b/packages/util-dynamodb/src/convertToAttr.ts index e7e26c8bc45f..a617d21ca215 100644 --- a/packages/util-dynamodb/src/convertToAttr.ts +++ b/packages/util-dynamodb/src/convertToAttr.ts @@ -1,7 +1,7 @@ import { AttributeValue } from "@aws-sdk/client-dynamodb"; import { marshallOptions } from "./marshall"; -import { NativeAttributeBinary, NativeAttributeValue, NativeScalarAttributeValue } from "./models"; +import { NativeAttributeBinary, NativeAttributeValue } from "./models"; import { NumberValue } from "./NumberValue"; /** @@ -57,7 +57,11 @@ export const convertToAttr = (data: NativeAttributeValue, options?: marshallOpti const convertToListAttr = (data: NativeAttributeValue[], options?: marshallOptions): { L: AttributeValue[] } => ({ L: data - .filter((item) => !options?.removeUndefinedValues || (options?.removeUndefinedValues && item !== undefined)) + .filter( + (item) => + typeof item !== "function" && + (!options?.removeUndefinedValues || (options?.removeUndefinedValues && item !== undefined)) + ) .map((item) => convertToAttr(item, options)), });