Fixes scan filters (#336)

tywalch · Dec 19, 2023 · 346a15a · 346a15a
1 parent 88491a8
commit 346a15a
Show file tree

Hide file tree

Showing 4 changed files with 163 additions and 13 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -498,4 +498,8 @@ All notable changes to this project will be documented in this file. Breaking ch
 
 ## [2.12.1] - 2023-11-29
 ### Fixed
-- Adds more sophisticated custom attribute type extraction. Patch provided by github user @wentsul with an assist by @adriancooney via [PR #332](https://github.com/tywalch/electrodb/pull/334). Thank you both for this great addition!
+- Adds more sophisticated custom attribute type extraction. Patch provided by github user @wentsul with an assist by @adriancooney via [PR #332](https://github.com/tywalch/electrodb/pull/334). Thank you both for this great addition!
+
+## [2.12.2] - 2023-12-18
+### Fixed
+- Fixes bug where `scan` appended invalid filters if some cases. In cases where [attributes are used as keys](https://electrodb.dev/en/modeling/indexes/#attributes-as-indexes) or [composite templates contain no prefixes](https://electrodb.dev/en/modeling/indexes/#composite-attribute-templates) the `scan` operation would append invalid filters to parameters. This bug was identified by discord user @engi22, thank you!  
diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "electrodb",
-  "version": "2.12.1",
+  "version": "2.12.2",
   "description": "A library to more easily create and interact with multiple entities and heretical relationships in dynamodb",
   "main": "index.js",
   "scripts": {

diff --git a/src/entity.js b/src/entity.js
@@ -2211,30 +2211,61 @@ class Entity {
     let { pk, sk } = this._makeIndexKeys({
       index: indexBase,
     });
+
     let keys = this._makeParameterKey(indexBase, pk, ...sk);
+    // trim empty key values (this can occur when keys are defined by users)
+    for (let key in keys) {
+      if (keys[key] === undefined || keys[key] === '') {
+        delete keys[key];
+      }
+    }
+
     let keyExpressions = this._expressionAttributeBuilder(keys);
-    let params = {
-      TableName: this.getTableName(),
-      ExpressionAttributeNames: this._mergeExpressionsAttributes(
+
+    const expressionAttributeNames = this._mergeExpressionsAttributes(
         filter.getNames(),
         keyExpressions.ExpressionAttributeNames,
-      ),
-      ExpressionAttributeValues: this._mergeExpressionsAttributes(
+    );
+
+    const expressionAttributeValues = this._mergeExpressionsAttributes(
         filter.getValues(),
         keyExpressions.ExpressionAttributeValues,
-      ),
-      FilterExpression: `begins_with(#${pkField}, :${pkField})`,
+    );
+
+
+    let params = {
+      TableName: this.getTableName(),
     };
 
+    if (Object.keys(expressionAttributeNames).length) {
+      params['ExpressionAttributeNames'] = expressionAttributeNames;
+    }
+
+    if (Object.keys(expressionAttributeValues).length) {
+      params['ExpressionAttributeValues'] = expressionAttributeValues;
+    }
+
+    let filterExpressions = [];
+
+    if (keys[pkField]) {
+      filterExpressions.push(`begins_with(#${pkField}, :${pkField})`);
+    }
+
     if (hasSortKey) {
       let skField = this.model.indexes[accessPattern].sk.field;
-      params.FilterExpression = `${params.FilterExpression} AND begins_with(#${skField}, :${skField})`;
+      if (keys[skField]) {
+        filterExpressions.push(`begins_with(#${skField}, :${skField})`);
+      }
     }
+
     if (filter.build()) {
-      params.FilterExpression = `${
-        params.FilterExpression
-      } AND ${filter.build()}`;
+      filterExpressions.push(filter.build());
     }
+
+    if (filterExpressions.length) {
+      params.FilterExpression = filterExpressions.join(' AND ');
+    }
+
     return params;
   }
 

diff --git a/test/ts_connected.crud.spec.ts b/test/ts_connected.crud.spec.ts
@@ -3400,6 +3400,121 @@ describe("Entity", () => {
       });
     });
 
+    it('should create valid scan params for entities with key field and attribute values', async () => {
+      const TestEntity = new Entity(
+          {
+            model: {
+              entity: 'test',
+              version: '1',
+              service: 'testing',
+            },
+            attributes: {
+              uuid: {
+                type: 'string',
+                required: true,
+              },
+              id: {
+                type: 'number',
+                required: true,
+              },
+              name: {
+                type: 'string'
+              }
+            },
+            indexes: {
+              primary: {
+                pk: {
+                  field: 'id',
+                  composite: ['id'],
+                },
+                sk: {
+                  field: 'uuid',
+                  composite: ['uuid'],
+                  casing: 'none',
+                },
+              },
+            },
+          },
+          { table, client },
+      );
+
+      const params = TestEntity.scan.params();
+
+      expect(params).to.deep.equal({
+        "TableName": "electro",
+        "ExpressionAttributeNames": {
+          "#__edb_e__": "__edb_e__",
+          "#__edb_v__": "__edb_v__"
+        },
+        "ExpressionAttributeValues": {
+          ":__edb_e__0": "test",
+          ":__edb_v__0": "1"
+        },
+        "FilterExpression": "(#__edb_e__ = :__edb_e__0) AND #__edb_v__ = :__edb_v__0"
+      });
+
+      // should not throw
+      await TestEntity.scan.go()
+    });
+
+    it('should create valid scan params for entities with templated indexes', async () => {
+      const TestEntity = new Entity(
+          {
+            model: {
+              entity: 'test',
+              version: '1',
+              service: 'testing',
+            },
+            attributes: {
+              uuid: {
+                type: 'string',
+                required: true,
+              },
+              id: {
+                type: 'number',
+                required: true,
+              },
+              name: {
+                type: 'string'
+              }
+            },
+            indexes: {
+              primary: {
+                pk: {
+                  field: 'pk',
+                  composite: ['id'],
+                  template: '${id}'
+                },
+                sk: {
+                  field: 'sk',
+                  composite: ['uuid'],
+                  template: '${uuid}'
+                },
+              },
+            },
+          },
+          { table, client },
+      );
+
+      const params = TestEntity.scan.params();
+
+      expect(params).to.deep.equal({
+        "TableName": "electro",
+        "ExpressionAttributeNames": {
+          "#__edb_e__": "__edb_e__",
+          "#__edb_v__": "__edb_v__"
+        },
+        "ExpressionAttributeValues": {
+          ":__edb_e__0": "test",
+          ":__edb_v__0": "1"
+        },
+        "FilterExpression": "(#__edb_e__ = :__edb_e__0) AND #__edb_v__ = :__edb_v__0"
+      });
+
+      // should not throw
+      await TestEntity.scan.go();
+    });
+
     it("should parse the response from a scan", async () => {
       const prop1 = uuid();
       const prop2 = uuid();