Skip to content

Commit f475bd0

Browse files
dreamorosidreamorosi
authoredJun 15, 2023
fix(idempotency): record validation not using hash (#1502)
* fix: record validation * tests: update unit * chore: updated node types
·
v2.16.0v1.10.0
1 parent fb932be commit f475bd0

File tree

6 files changed

+67
-23
lines changed

6 files changed

+67
-23
lines changed
 

‎package-lock.json

+16-3
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎package.json

+1-1
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@
5252
"@middy/core": "^3.6.2",
5353
"@types/aws-lambda": "^8.10.109",
5454
"@types/jest": "^29.2.4",
55-
"@types/node": "^18.11.15",
55+
"@types/node": "^18.16.18",
5656
"@types/uuid": "^9.0.0",
5757
"@typescript-eslint/eslint-plugin": "^5.46.1",
5858
"@typescript-eslint/parser": "^5.46.1",

‎packages/idempotency/src/persistence/BasePersistenceLayer.ts

+8-8
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,7 @@ abstract class BasePersistenceLayer implements BasePersistenceLayerInterface {
132132
idempotencyKey: this.getHashedIdempotencyKey(data),
133133
status: IdempotencyRecordStatus.INPROGRESS,
134134
expiryTimestamp: this.getExpiryTimestamp(),
135-
payloadHash: this.generateHash(JSON.stringify(data)),
135+
payloadHash: this.getHashedPayload(data),
136136
});
137137

138138
if (remainingTimeInMillis) {
@@ -167,7 +167,7 @@ abstract class BasePersistenceLayer implements BasePersistenceLayerInterface {
167167
status: IdempotencyRecordStatus.COMPLETED,
168168
expiryTimestamp: this.getExpiryTimestamp(),
169169
responseData: result,
170-
payloadHash: this.generateHash(JSON.stringify(data)),
170+
payloadHash: this.getHashedPayload(data),
171171
});
172172

173173
await this._updateRecord(idempotencyRecord);
@@ -264,13 +264,13 @@ abstract class BasePersistenceLayer implements BasePersistenceLayerInterface {
264264
* @param data payload
265265
*/
266266
private getHashedPayload(data: Record<string, unknown>): string {
267-
// This method is only called when payload validation is enabled.
268-
// For payload validation to be enabled, the validation key jmespath must be set.
269-
// Therefore, the assertion is safe.
270-
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
271-
data = search(data, this.validationKeyJmesPath!);
267+
if (this.isPayloadValidationEnabled() && this.validationKeyJmesPath) {
268+
data = search(data, this.validationKeyJmesPath);
272269

273-
return this.generateHash(JSON.stringify(data));
270+
return this.generateHash(JSON.stringify(data));
271+
} else {
272+
return '';
273+
}
274274
}
275275

276276
private static isMissingIdempotencyKey(

‎packages/idempotency/src/persistence/DynamoDBPersistenceLayer.ts

+2-1
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ class DynamoDBPersistenceLayer extends BasePersistenceLayer {
4848
this.statusAttr = config.statusAttr ?? 'status';
4949
this.expiryAttr = config.expiryAttr ?? 'expiration';
5050
this.inProgressExpiryAttr =
51-
config.inProgressExpiryAttr ?? 'in_progress_expiry_attr';
51+
config.inProgressExpiryAttr ?? 'in_progress_expiration';
5252
this.dataAttr = config.dataAttr ?? 'data';
5353
this.validationKeyAttr = config.validationKeyAttr ?? 'validation';
5454
if (config.sortKeyAttr === this.keyAttr) {
@@ -106,6 +106,7 @@ class DynamoDBPersistenceLayer extends BasePersistenceLayer {
106106
expiryTimestamp: item[this.expiryAttr],
107107
inProgressExpiryTimestamp: item[this.inProgressExpiryAttr],
108108
responseData: item[this.dataAttr],
109+
payloadHash: item[this.validationKeyAttr],
109110
});
110111
}
111112

‎packages/idempotency/tests/unit/persistence/BasePersistenceLayer.test.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -369,7 +369,7 @@ describe('Class: BasePersistenceLayer', () => {
369369
idempotencyKey: 'my-lambda-function#mocked-hash',
370370
status: IdempotencyRecordStatus.INPROGRESS,
371371
expiryTimestamp: Date.now() / 1000 + 3600,
372-
payloadHash: 'mocked-hash',
372+
payloadHash: '',
373373
inProgressExpiryTimestamp: Date.now() + remainingTimeInMs,
374374
responseData: undefined,
375375
})
@@ -455,7 +455,7 @@ describe('Class: BasePersistenceLayer', () => {
455455
idempotencyKey: 'my-lambda-function#mocked-hash',
456456
status: IdempotencyRecordStatus.COMPLETED,
457457
expiryTimestamp: Date.now() / 1000 + 3600,
458-
payloadHash: 'mocked-hash',
458+
payloadHash: '',
459459
inProgressExpiryTimestamp: undefined,
460460
responseData: result,
461461
})

‎packages/idempotency/tests/unit/persistence/DynamoDbPersistenceLayer.test.ts

+38-8
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ describe('Class: DynamoDBPersistenceLayer', () => {
8282
keyAttr: 'id',
8383
statusAttr: 'status',
8484
expiryAttr: 'expiration',
85-
inProgressExpiryAttr: 'in_progress_expiry_attr',
85+
inProgressExpiryAttr: 'in_progress_expiration',
8686
dataAttr: 'data',
8787
validationKeyAttr: 'validation',
8888
staticPkValue: 'idempotency#my-lambda-function',
@@ -230,7 +230,7 @@ describe('Class: DynamoDBPersistenceLayer', () => {
230230
'#id': 'id',
231231
'#expiry': 'expiration',
232232
'#status': 'status',
233-
'#in_progress_expiry': 'in_progress_expiry_attr',
233+
'#in_progress_expiry': 'in_progress_expiration',
234234
},
235235
ExpressionAttributeValues: marshall({
236236
':now': Date.now() / 1000,
@@ -273,7 +273,7 @@ describe('Class: DynamoDBPersistenceLayer', () => {
273273
'#id': 'id',
274274
'#expiry': 'expiration',
275275
'#status': 'status',
276-
'#in_progress_expiry': 'in_progress_expiry_attr',
276+
'#in_progress_expiry': 'in_progress_expiration',
277277
},
278278
ExpressionAttributeValues: marshall({
279279
':now': Date.now() / 1000,
@@ -310,13 +310,13 @@ describe('Class: DynamoDBPersistenceLayer', () => {
310310
id: dummyKey,
311311
expiration: expiryTimestamp,
312312
status,
313-
in_progress_expiry_attr: inProgressExpiryTimestamp,
313+
in_progress_expiration: inProgressExpiryTimestamp,
314314
}),
315315
ExpressionAttributeNames: {
316316
'#id': 'id',
317317
'#expiry': 'expiration',
318318
'#status': 'status',
319-
'#in_progress_expiry': 'in_progress_expiry_attr',
319+
'#in_progress_expiry': 'in_progress_expiration',
320320
},
321321
ExpressionAttributeValues: marshall({
322322
':now': Date.now() / 1000,
@@ -361,7 +361,7 @@ describe('Class: DynamoDBPersistenceLayer', () => {
361361
'#id': 'id',
362362
'#expiry': 'expiration',
363363
'#status': 'status',
364-
'#in_progress_expiry': 'in_progress_expiry_attr',
364+
'#in_progress_expiry': 'in_progress_expiration',
365365
},
366366
ExpressionAttributeValues: marshall({
367367
':now': Date.now() / 1000,
@@ -433,7 +433,7 @@ describe('Class: DynamoDBPersistenceLayer', () => {
433433
id: dummyKey,
434434
status: IdempotencyRecordStatus.INPROGRESS,
435435
expiration: getFutureTimestamp(15),
436-
in_progress_expiry_attr: getFutureTimestamp(10),
436+
in_progress_expiration: getFutureTimestamp(10),
437437
data: {},
438438
}),
439439
});
@@ -465,7 +465,7 @@ describe('Class: DynamoDBPersistenceLayer', () => {
465465
id: dummyKey,
466466
status,
467467
expiration: expiryTimestamp,
468-
in_progress_expiry_attr: inProgressExpiryTimestamp,
468+
in_progress_expiration: inProgressExpiryTimestamp,
469469
data: responseData,
470470
}),
471471
});
@@ -526,6 +526,36 @@ describe('Class: DynamoDBPersistenceLayer', () => {
526526
ConsistentRead: true,
527527
});
528528
});
529+
530+
test('when called with a record that had the ', async () => {
531+
// Prepare
532+
const persistenceLayer = new TestDynamoDBPersistenceLayer({
533+
tableName: dummyTableName,
534+
staticPkValue: 'idempotency#my-lambda-function',
535+
sortKeyAttr: 'sortKey',
536+
});
537+
client.on(GetItemCommand).resolves({
538+
Item: marshall({
539+
id: dummyKey,
540+
status: IdempotencyRecordStatus.INPROGRESS,
541+
expiration: getFutureTimestamp(15),
542+
in_progress_expiration: getFutureTimestamp(10),
543+
data: {},
544+
validation: 'someHash',
545+
}),
546+
});
547+
548+
// Act
549+
const record = await persistenceLayer._getRecord(dummyKey);
550+
551+
// Assess
552+
expect(record.idempotencyKey).toEqual(dummyKey);
553+
expect(record.getStatus()).toEqual(IdempotencyRecordStatus.INPROGRESS);
554+
expect(record.expiryTimestamp).toEqual(getFutureTimestamp(15));
555+
expect(record.inProgressExpiryTimestamp).toEqual(getFutureTimestamp(10));
556+
expect(record.responseData).toStrictEqual({});
557+
expect(record.payloadHash).toEqual('someHash');
558+
});
529559
});
530560

531561
describe('Method: _updateRecord', () => {

0 commit comments

Comments
 (0)
Please sign in to comment.