Repository: jeremydaly/dynamodb-toolbox (ts)

Issue:
Conditions on nested fields do not work
If you try and set a condition on a nested field it does not work, it tries to do the condition check on a field "my.nestedValue" instead of "my"."nestedValue"

Pull request:
Allow nested attr paths (dot notation) for exists conditions
Fixes #214 by allowing nested `attr` paths (dot notation) for `exists` conditions.

e.g. currently this:

```js
{
  conditions: {
    attr: 'a.b.c',
    exists: true
  }
}
```

Generates this (which doesn't work):

```js
{
  ExpressionAttributeNames: {
    '#attr1': 'a.b.c'
  },
  ConditionExpression: 'attribute_exists(#attr_1)'
}
```

With this PR, it will instead generate this (which does work):

```js
{
  ExpressionAttributeNames: {
    '#attr1_0': 'a',
    '#attr1_1': 'b',
    '#attr1_2': 'c'
  },
  ConditionExpression: 'attribute_exists(#attr_1_0.#attr_1_1.#attr_1_2)'
}
```

The [AWS docs](https://docs.aws.amazon.com/amazondynamodb/latest/developerguide/Expressions.ConditionExpressions.html) suggest that `attribute_exists(a.b.c)` should work but it doesn't, at least not via the JS SDK. I found out about the above syntax and workaround on Stack Overflow!

Although I only need this fixed for the `exists` conditions, for completeness it looks like I probably should implement this for the `between`, `in`, `begins_with`, `contains`, `attribute_type` and `size` conditions/operators as well.

Keen to hear thoughts!

Just to add - I considered using arrays for nested `attr` paths, e.g.

```js
{
  conditions: {
    attr: ['a', 'b', 'c'],
    exists: true
  }
}
```

As technically, you could have a valid (non-map) attribute key that is `'a.b.c'` and want to check if that exists.

However, I don't *think* many people use that syntax, and it seems dot notation is assumed as the convention for nested attributes in [`checkAttribute()`](https://github.com/jeremydaly/dynamodb-toolbox/blob/74d179af95bf110c450aebeaf9c5a55f4f46a063/src/lib/checkAttribute.ts#L9-L13), and I didn't think it made sense to introduce a different convention here.

Patch:
diff --git a/src/lib/expressionBuilder.ts b/src/lib/expressionBuilder.ts
index 5d693687..a85cb3e0 100644
--- a/src/lib/expressionBuilder.ts
+++ b/src/lib/expressionBuilder.ts
@@ -76,9 +76,9 @@ const buildExpression = <
 
       // Else process the clause
     } else {
-     // Make sure TS knows this is a FilterExpression; copy to avoid mutation
-      const exp = {...clauses[id]} as FilterExpression
-      
+      // Make sure TS knows this is a FilterExpression; copy to avoid mutation
+      const exp = { ...clauses[id] } as FilterExpression
+
       // Increment group counter for each clause
       group++ // guarantees uniqueness
 
@@ -165,13 +165,18 @@ const parseClause = <EntityTable extends TableDef | undefined = undefined>(
   )
     error(`'entity' value of '${entity}' must be a string and a valid table Entity name`)
 
-  // Add filter attribute to names
-  names[`#attr${grp}`] =
-    typeof attr === 'string'
-      ? checkAttribute(attr, entity ? table[entity].schema.attributes : table.Table.attributes)
-      : typeof size === 'string'
-      ? checkAttribute(size, entity ? table[entity].schema.attributes : table.Table.attributes)
-      : error(`A string for 'attr' or 'size' is required for condition expressions`)
+  if (typeof attr === 'string' && attr.includes('.')) {
+    checkAttribute(attr, entity ? table[entity].schema.attributes : table.Table.attributes)
+    attr.split('.').forEach((part, i) => (names[`#attr${grp}_${i}`] = part))
+  } else {
+    // Add filter attribute to names
+    names[`#attr${grp}`] =
+      typeof attr === 'string'
+        ? checkAttribute(attr, entity ? table[entity].schema.attributes : table.Table.attributes)
+        : typeof size === 'string'
+        ? checkAttribute(size, entity ? table[entity].schema.attributes : table.Table.attributes)
+        : error(`A string for 'attr' or 'size' is required for condition expressions`)
+  }
 
   // Parse clause operator and value
   let operator, value, f
@@ -267,7 +272,14 @@ const parseClause = <EntityTable extends TableDef | undefined = undefined>(
       }
     } else if (operator === 'EXISTS') {
       if (!attr) error(`'exists' conditions require an 'attr'.`)
-      clause = value ? `attribute_exists(#attr${grp})` : `attribute_not_exists(#attr${grp})`
+      const path =
+        typeof attr === 'string' && attr.includes('.')
+          ? attr
+              .split('.')
+              .map((_, i) => `#attr${grp}_${i}`)
+              .join('.')
+          : `#attr${grp}`
+      clause = value ? `attribute_exists(${path})` : `attribute_not_exists(${path})`
     } else {
       // Add value
       values[`:attr${grp}`] = value


Test patch:
diff --git a/src/__tests__/expressionBuilder.unit.test.ts b/src/__tests__/expressionBuilder.unit.test.ts
index f3bce1b5..91235e63 100644
--- a/src/__tests__/expressionBuilder.unit.test.ts
+++ b/src/__tests__/expressionBuilder.unit.test.ts
@@ -284,12 +284,24 @@ describe('expressionBuilder', () => {
     expect(result.names).toEqual({ '#attr1': 'a' })
   })
 
+  it(`generates an 'exists' clause for a nested attribute`, () => {
+    let result = expressionBuilder({ attr: 'a.b.c', exists: true }, TestTable, 'TestEntity')
+    expect(result.expression).toBe('attribute_exists(#attr1_0.#attr1_1.#attr1_2)')
+    expect(result.names).toEqual({ '#attr1_0': 'a', '#attr1_1': 'b', '#attr1_2': 'c' })
+  })
+
   it(`generates a 'not exists' clause`, () => {
     let result = expressionBuilder({ attr: 'a', exists: false }, TestTable, 'TestEntity')
     expect(result.expression).toBe('attribute_not_exists(#attr1)')
     expect(result.names).toEqual({ '#attr1': 'a' })
   })
 
+  it(`generates an 'not exists' clause for a nested attribute`, () => {
+    let result = expressionBuilder({ attr: 'a.b.c', exists: false }, TestTable, 'TestEntity')
+    expect(result.expression).toBe('attribute_not_exists(#attr1_0.#attr1_1.#attr1_2)')
+    expect(result.names).toEqual({ '#attr1_0': 'a', '#attr1_1': 'b', '#attr1_2': 'c' })
+  })
+
   it(`generates a 'contains' clause`, () => {
     let result = expressionBuilder({ attr: 'a', contains: 'b' }, TestTable, 'TestEntity')
     expect(result.expression).toBe('contains(#attr1,:attr1)')
@@ -379,8 +391,8 @@ describe('expressionBuilder', () => {
   it("doesn't mutate input expression", () => {
     let expObj = { attr: 'a', eq: 'b' }
     let expArr = [{ attr: 'a', eq: 'b' }]
-    expressionBuilder(expObj,TestTable,'TestEntity')
-    expressionBuilder(expArr,TestTable,'TestEntity')
+    expressionBuilder(expObj, TestTable, 'TestEntity')
+    expressionBuilder(expArr, TestTable, 'TestEntity')
     expect(expObj).toEqual({ attr: 'a', eq: 'b' })
     expect(expArr).toEqual([{ attr: 'a', eq: 'b' }])
   })