fix(lib-dynamodb): preserve collections when serializing class instan…

…ces to map (#5826) * fix(lib-dynamodb): preserve collections when serializing class instances to map * test(lib-dynamodb): new e2e scenario for class conversion
aws · Feb 26, 2024 · e1ba507 · e1ba507
1 parent 10fe8be
commit e1ba507
Show file tree

Hide file tree

Showing 4 changed files with 207 additions and 77 deletions.
diff --git 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
@@ -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
@@ -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<string, UpdateCommandOutput>,
     updateReadBack: {} as Record<string, GetCommandOutput>,
     delete: {} as Record<string, DeleteItemCommandOutput>,
+    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
@@ -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)),
 });