Repository: jeremydaly/dynamodb-toolbox
Pull request #677 (2024-02-12 00:57:11)

fix: remove attributes that depend on attributes that can't be resolved
Addresses #678 

naorpeled: Hey @ojongerius,
sorry for the huge delay,
LGTM!!!

Thanks so much for this fix.

Patch:
diff --git a/src/lib/normalizeData.ts b/src/lib/normalizeData.ts
index 243881e7..6ad36b97 100644
--- a/src/lib/normalizeData.ts
+++ b/src/lib/normalizeData.ts
@@ -5,8 +5,6 @@
  */
 
 
-import { DynamoDBDocumentClient } from '@aws-sdk/lib-dynamodb'
-
 import validateTypes from './validateTypes.js'
 import { error, transformAttr } from './utils.js'
 
@@ -34,6 +32,8 @@ export default () => (
           if (typeof map[dependent] === 'function') {
             // Resolve the dependency graph
             map = dependsOn(map, dependent)
+          } if (typeof map[dependent] === 'undefined') {
+            throw new DependsOnUndefined(`'${dependent}' is undefined, bailing`)
           }
         } else {
           error(`'${dependent}' is not a valid attribute or alias name`)
@@ -48,7 +48,7 @@ export default () => (
         if (schema[attr].alias) map[schema[attr].alias] = map[attr]
         if (schema[attr].map) map[schema[attr].map] = map[attr]
       } catch (e) {
-        // TODO: Find a better way to prevent this for missing fields
+        // noop
       }
       return map
     }
@@ -80,8 +80,15 @@ export default () => (
   const defaults: Record<string, any> = Object.keys(defaultMap).reduce((acc, attr) => {
     // If a function, resolve the dependency graph
     if (typeof defaultMap[attr] === 'function') {
-      const map = dependsOn(defaultMap, attr)
-      defaultMap = map
+      try {
+        const map = dependsOn(defaultMap, attr)
+        defaultMap = map
+      } catch (e) {
+        if (e instanceof DependsOnUndefined && (schema[attr].partitionKey || schema[attr].sortKey || schema[attr].required)) {
+          throw new Error(`Required field '${attr}' depends on attribute(s), one or more of which can't be resolved (${schema[attr].dependsOn})`)
+        }
+        delete defaultMap[attr]
+      }
     }
     return Object.assign(acc, { [attr]: defaultMap[attr] })
   }, {})
@@ -108,14 +115,8 @@ export default () => (
       })
       .filter((x: any) => x !== null)
 
-    // TODO: add required fields
-    // if (values.length > 0 && values.length !== linked[field].length) {
-    //   error(`${linked[field].join(', ')} are all required for composite key`)
-    // } else
-
     if (values.length === linked[attr].length) {
       return Object.assign(acc, {
-        //[field]: `${schema[attr].prefix || ''}${values.join(schema[attr].delimiter || '#')}${schema[attr].suffix || ''}`
         [field]: values.join(schema[attr].delimiter || '#')
       })
     } else {
@@ -127,17 +128,8 @@ export default () => (
   return Object.assign(composites, _data)
 }
 
-// Generate final data and evaluate function expressions
-// let _data = Object.keys(dataMap.data).reduce((acc,field) => {
-//   return Object.assign(acc, {
-//     [field]: typeof dataMap.data[field] === 'function' ? dataMap.data[field](defaults) : dataMap.data[field]
-//   })
-// },{})
-
-// console.log(_data)
-
-// map[schema[attr].dependsOn] = typeof map[schema[attr].dependsOn] === 'function' ?
-//   dependsOn(map,schema[attr].dependsOn) // map[schema[attr].dependsOn](map)
-//   : map[schema[attr].dependsOn]
-// return { map, val: map[attr](map) }
-// return dependsOn(map,attr)
+class DependsOnUndefined extends Error {
+  constructor(message: string) {
+    super(message)
+  }
+}


Test patch:
diff --git a/src/__tests__/normalizeData.unit.test.ts b/src/__tests__/normalizeData.unit.test.ts
index f957b56e..d3eee91a 100644
--- a/src/__tests__/normalizeData.unit.test.ts
+++ b/src/__tests__/normalizeData.unit.test.ts
@@ -29,8 +29,6 @@ new Entity({
   table: DefaultTable
 } as const)
 
-// console.log(DefaultTable.User);
-
 const attributes = DefaultTable.User.schema.attributes
 const linked = DefaultTable.User.linked
 
@@ -80,4 +78,22 @@ describe('normalizeData', () => {
       })
     }).toThrow(`Field 'notAField' does not have a mapping or alias`)
   })
+
+  it('fails when partion, sortkey or other required fields have depend attributes not provided', () => {
+    const ent = new Entity({
+      name: 'Test',
+      attributes: {
+        pk: { type: 'string', partitionKey: true, default: 'pk'},
+        sk: { type: 'string', sortKey: true, dependsOn: ['parent'],
+          default: (data: { parent: string }) => `parent#${data.parent}`,
+        },
+        parent: { type: 'string'},
+      },
+      table: DefaultTable
+    } as const)
+
+    expect(() => ent.updateParams({})).toThrow(
+      `Required field 'sk' depends on attribute(s), one or more of which can't be resolved (parent)`
+    )
+  })
 })
diff --git a/src/__tests__/type-infering.unit.test.ts b/src/__tests__/type-infering.unit.test.ts
index 8b2d1092..be094cfd 100644
--- a/src/__tests__/type-infering.unit.test.ts
+++ b/src/__tests__/type-infering.unit.test.ts
@@ -1668,25 +1668,14 @@ describe('Entity', () => {
         type TestGetItem3 = A.Equals<GetItem3, ExpectedItem | undefined>
         const testGetItem3: TestGetItem3 = 1
         testGetItem3
-
-        // Using SK "dependsOn": skMap1 is not required as well
-        const ck4 = { pkMap1 }
-        ent.getParams(ck4)
-        const getPromise4 = () => ent.get(ck4)
-        type GetItem4 = Awaited<ReturnType<typeof getPromise4>>['Item']
-        type TestGetItem4 = A.Equals<GetItem4, ExpectedItem | undefined>
-        const testGetItem4: TestGetItem4 = 1
-        testGetItem4
       })
 
       it('throws when primary key is incomplete', () => {
-        // 🔨 TOIMPROVE: Not sure this is expected behavior: Missing partition key doesn't throw
-        // @ts-expect-error
-        ent.getParams({ sk })
+        // @ts-expect-error Missing partition
+        expect(() => ent.getParams({ sk })).toThrow()
 
-        // 🔨 TOIMPROVE: Not sure this is expected behavior: Partition key dependsOn incomplete
-        // @ts-expect-error
-        ent.getParams({ pkMap2, sk })
+        // @ts-expect-error Partition key dependsOn incomplete
+        expect(() => ent.getParams({ pkMap2, sk })).toThrow()
       })
     })
 
@@ -1707,31 +1696,29 @@ describe('Entity', () => {
         const deletePromise3 = () => ent.delete(ck3)
         deletePromise3
 
-        const ck4 = { pkMap1 }
+        const ck4 = { pkMap1, skMap1 }
         ent.deleteParams(ck4)
         const deletePromise4 = () => ent.delete(ck4)
         deletePromise4
       })
 
       it('throws when primary key is incomplete', () => {
-        // 🔨 TOIMPROVE: Not sure this is expected behavior: Missing partition key doesn't throw
-        // @ts-expect-error
-        ent.deleteParams({ sk })
+        // @ts-expect-error Missing sort key
+        expect(() => ent.deleteParams({ sk })).toThrow()
 
-        // 🔨 TOIMPROVE: Not sure this is expected behavior: Partition key dependsOn incomplete
         // @ts-expect-error: Partition key dependsOn incomplete
-        ent.deleteParams({ pkMap2, sk })
+        expect(() => ent.deleteParams({ pkMap2, sk })).toThrow()
       })
     })
 
     describe('put method', () => {
       it('nominal case', () => {
-        const item1 = { pkMap1, pkMap2, skMap2 }
+        const item1 = { pkMap1, skMap1, pkMap2, skMap2 }
         ent.putParams(item1)
         const putPromise1 = () => ent.put(item1)
         putPromise1
 
-        const item2 = { pkMap1 }
+        const item2 = { pkMap1, skMap1 }
         ent.putParams(item2)
         const putPromise2 = () => ent.put(item2)
         putPromise2
@@ -1748,25 +1735,24 @@ describe('Entity', () => {
 
     describe('update method', () => {
       it('nominal case', () => {
-        const item1 = { pkMap1, pkMap2, skMap2 }
+        const item1 = { pkMap1, pkMap2, skMap1, skMap2 }
+
         ent.updateParams(item1)
         const updatePromise1 = () => ent.update(item1)
         updatePromise1
 
-        const item2 = { pkMap1 }
+        const item2 = { pkMap1, skMap1 }
         ent.updateParams(item2)
         const updatePromise2 = () => ent.update(item2)
         updatePromise2
       })
 
       it('throws when primary key is incomplete', () => {
-        // 🔨 TOIMPROVE: Not sure this is expected behavior: Missing partition key doesn't throw
-        // @ts-expect-error
-        ent.updateParams({ sk })
+        // @ts-expect-error Missing partition key
+        expect(() => ent.updateParams({ sk })).toThrow()
 
-        // 🔨 TOIMPROVE: Not sure this is expected behavior: Partition key dependsOn incomplete
-        // @ts-expect-error
-        ent.updateParams({ pkMap2, sk })
+        // @ts-expect-error Partition key dependsOn incomplete
+        expect(() => ent.updateParams({ pkMap2, sk })).toThrow()
       })
     })
   })