fix(idempotency): record validation not using hash (#1502)

* fix: record validation * tests: update unit * chore: updated node types
aws-powertools · Jun 15, 2023 · f475bd0 · f475bd0
1 parent fb932be
commit f475bd0
Show file tree

Hide file tree

Showing 6 changed files with 67 additions and 23 deletions.
diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -52,7 +52,7 @@
     "@middy/core": "^3.6.2",
     "@types/aws-lambda": "^8.10.109",
     "@types/jest": "^29.2.4",
-    "@types/node": "^18.11.15",
+    "@types/node": "^18.16.18",
     "@types/uuid": "^9.0.0",
     "@typescript-eslint/eslint-plugin": "^5.46.1",
     "@typescript-eslint/parser": "^5.46.1",

diff --git a/packages/idempotency/src/persistence/BasePersistenceLayer.ts b/packages/idempotency/src/persistence/BasePersistenceLayer.ts
@@ -132,7 +132,7 @@ abstract class BasePersistenceLayer implements BasePersistenceLayerInterface {
       idempotencyKey: this.getHashedIdempotencyKey(data),
       status: IdempotencyRecordStatus.INPROGRESS,
       expiryTimestamp: this.getExpiryTimestamp(),
-      payloadHash: this.generateHash(JSON.stringify(data)),
+      payloadHash: this.getHashedPayload(data),
     });
 
     if (remainingTimeInMillis) {
@@ -167,7 +167,7 @@ abstract class BasePersistenceLayer implements BasePersistenceLayerInterface {
       status: IdempotencyRecordStatus.COMPLETED,
       expiryTimestamp: this.getExpiryTimestamp(),
       responseData: result,
-      payloadHash: this.generateHash(JSON.stringify(data)),
+      payloadHash: this.getHashedPayload(data),
     });
 
     await this._updateRecord(idempotencyRecord);
@@ -264,13 +264,13 @@ abstract class BasePersistenceLayer implements BasePersistenceLayerInterface {
    * @param data payload
    */
   private getHashedPayload(data: Record<string, unknown>): string {
-    // This method is only called when payload validation is enabled.
-    // For payload validation to be enabled, the validation key jmespath must be set.
-    // Therefore, the assertion is safe.
-    // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
-    data = search(data, this.validationKeyJmesPath!);
+    if (this.isPayloadValidationEnabled() && this.validationKeyJmesPath) {
+      data = search(data, this.validationKeyJmesPath);
 
-    return this.generateHash(JSON.stringify(data));
+      return this.generateHash(JSON.stringify(data));
+    } else {
+      return '';
+    }
   }
 
   private static isMissingIdempotencyKey(

diff --git a/packages/idempotency/src/persistence/DynamoDBPersistenceLayer.ts b/packages/idempotency/src/persistence/DynamoDBPersistenceLayer.ts
@@ -48,7 +48,7 @@ class DynamoDBPersistenceLayer extends BasePersistenceLayer {
     this.statusAttr = config.statusAttr ?? 'status';
     this.expiryAttr = config.expiryAttr ?? 'expiration';
     this.inProgressExpiryAttr =
-      config.inProgressExpiryAttr ?? 'in_progress_expiry_attr';
+      config.inProgressExpiryAttr ?? 'in_progress_expiration';
     this.dataAttr = config.dataAttr ?? 'data';
     this.validationKeyAttr = config.validationKeyAttr ?? 'validation';
     if (config.sortKeyAttr === this.keyAttr) {
@@ -106,6 +106,7 @@ class DynamoDBPersistenceLayer extends BasePersistenceLayer {
       expiryTimestamp: item[this.expiryAttr],
       inProgressExpiryTimestamp: item[this.inProgressExpiryAttr],
       responseData: item[this.dataAttr],
+      payloadHash: item[this.validationKeyAttr],
     });
   }
 

diff --git a/packages/idempotency/tests/unit/persistence/BasePersistenceLayer.test.ts b/packages/idempotency/tests/unit/persistence/BasePersistenceLayer.test.ts
@@ -369,7 +369,7 @@ describe('Class: BasePersistenceLayer', () => {
           idempotencyKey: 'my-lambda-function#mocked-hash',
           status: IdempotencyRecordStatus.INPROGRESS,
           expiryTimestamp: Date.now() / 1000 + 3600,
-          payloadHash: 'mocked-hash',
+          payloadHash: '',
           inProgressExpiryTimestamp: Date.now() + remainingTimeInMs,
           responseData: undefined,
         })
@@ -455,7 +455,7 @@ describe('Class: BasePersistenceLayer', () => {
           idempotencyKey: 'my-lambda-function#mocked-hash',
           status: IdempotencyRecordStatus.COMPLETED,
           expiryTimestamp: Date.now() / 1000 + 3600,
-          payloadHash: 'mocked-hash',
+          payloadHash: '',
           inProgressExpiryTimestamp: undefined,
           responseData: result,
         })

diff --git a/packages/idempotency/tests/unit/persistence/DynamoDbPersistenceLayer.test.ts b/packages/idempotency/tests/unit/persistence/DynamoDbPersistenceLayer.test.ts
@@ -82,7 +82,7 @@ describe('Class: DynamoDBPersistenceLayer', () => {
           keyAttr: 'id',
           statusAttr: 'status',
           expiryAttr: 'expiration',
-          inProgressExpiryAttr: 'in_progress_expiry_attr',
+          inProgressExpiryAttr: 'in_progress_expiration',
           dataAttr: 'data',
           validationKeyAttr: 'validation',
           staticPkValue: 'idempotency#my-lambda-function',
@@ -230,7 +230,7 @@ describe('Class: DynamoDBPersistenceLayer', () => {
           '#id': 'id',
           '#expiry': 'expiration',
           '#status': 'status',
-          '#in_progress_expiry': 'in_progress_expiry_attr',
+          '#in_progress_expiry': 'in_progress_expiration',
         },
         ExpressionAttributeValues: marshall({
           ':now': Date.now() / 1000,
@@ -273,7 +273,7 @@ describe('Class: DynamoDBPersistenceLayer', () => {
           '#id': 'id',
           '#expiry': 'expiration',
           '#status': 'status',
-          '#in_progress_expiry': 'in_progress_expiry_attr',
+          '#in_progress_expiry': 'in_progress_expiration',
         },
         ExpressionAttributeValues: marshall({
           ':now': Date.now() / 1000,
@@ -310,13 +310,13 @@ describe('Class: DynamoDBPersistenceLayer', () => {
           id: dummyKey,
           expiration: expiryTimestamp,
           status,
-          in_progress_expiry_attr: inProgressExpiryTimestamp,
+          in_progress_expiration: inProgressExpiryTimestamp,
         }),
         ExpressionAttributeNames: {
           '#id': 'id',
           '#expiry': 'expiration',
           '#status': 'status',
-          '#in_progress_expiry': 'in_progress_expiry_attr',
+          '#in_progress_expiry': 'in_progress_expiration',
         },
         ExpressionAttributeValues: marshall({
           ':now': Date.now() / 1000,
@@ -361,7 +361,7 @@ describe('Class: DynamoDBPersistenceLayer', () => {
           '#id': 'id',
           '#expiry': 'expiration',
           '#status': 'status',
-          '#in_progress_expiry': 'in_progress_expiry_attr',
+          '#in_progress_expiry': 'in_progress_expiration',
         },
         ExpressionAttributeValues: marshall({
           ':now': Date.now() / 1000,
@@ -433,7 +433,7 @@ describe('Class: DynamoDBPersistenceLayer', () => {
           id: dummyKey,
           status: IdempotencyRecordStatus.INPROGRESS,
           expiration: getFutureTimestamp(15),
-          in_progress_expiry_attr: getFutureTimestamp(10),
+          in_progress_expiration: getFutureTimestamp(10),
           data: {},
         }),
       });
@@ -465,7 +465,7 @@ describe('Class: DynamoDBPersistenceLayer', () => {
           id: dummyKey,
           status,
           expiration: expiryTimestamp,
-          in_progress_expiry_attr: inProgressExpiryTimestamp,
+          in_progress_expiration: inProgressExpiryTimestamp,
           data: responseData,
         }),
       });
@@ -526,6 +526,36 @@ describe('Class: DynamoDBPersistenceLayer', () => {
         ConsistentRead: true,
       });
     });
+
+    test('when called with a record that had the ', async () => {
+      // Prepare
+      const persistenceLayer = new TestDynamoDBPersistenceLayer({
+        tableName: dummyTableName,
+        staticPkValue: 'idempotency#my-lambda-function',
+        sortKeyAttr: 'sortKey',
+      });
+      client.on(GetItemCommand).resolves({
+        Item: marshall({
+          id: dummyKey,
+          status: IdempotencyRecordStatus.INPROGRESS,
+          expiration: getFutureTimestamp(15),
+          in_progress_expiration: getFutureTimestamp(10),
+          data: {},
+          validation: 'someHash',
+        }),
+      });
+
+      // Act
+      const record = await persistenceLayer._getRecord(dummyKey);
+
+      // Assess
+      expect(record.idempotencyKey).toEqual(dummyKey);
+      expect(record.getStatus()).toEqual(IdempotencyRecordStatus.INPROGRESS);
+      expect(record.expiryTimestamp).toEqual(getFutureTimestamp(15));
+      expect(record.inProgressExpiryTimestamp).toEqual(getFutureTimestamp(10));
+      expect(record.responseData).toStrictEqual({});
+      expect(record.payloadHash).toEqual('someHash');
+    });
   });
 
   describe('Method: _updateRecord', () => {