From baf11b14d41222d66561fd54819f84036173db49 Mon Sep 17 00:00:00 2001 From: Theophilus Ajayi Date: Thu, 2 Nov 2023 10:50:42 +0100 Subject: [PATCH 01/12] created custom error with existingRecord field --- packages/idempotency/src/errors.ts | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/packages/idempotency/src/errors.ts b/packages/idempotency/src/errors.ts index da96692bc8..30223a4564 100644 --- a/packages/idempotency/src/errors.ts +++ b/packages/idempotency/src/errors.ts @@ -1,7 +1,18 @@ +import { AttributeValue } from '@aws-sdk/client-dynamodb'; /** * Item attempting to be inserted into persistence store already exists and is not expired */ -class IdempotencyItemAlreadyExistsError extends Error {} +class IdempotencyItemAlreadyExistsError extends Error { + public existingRecord: Record | undefined; + + public constructor( + message: string, + existingRecord: Record | undefined + ) { + super(message); + this.existingRecord = existingRecord; + } +} /** * Item does not exist in persistence store From 7dccc57bb35a785cb85efe606d7104d06ea369c1 Mon Sep 17 00:00:00 2001 From: Theophilus Ajayi Date: Thu, 2 Nov 2023 11:08:14 +0100 Subject: [PATCH 02/12] fetch items in DB on ReturnValuesOnConditionCheckFailure --- .../src/persistence/DynamoDBPersistenceLayer.ts | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/packages/idempotency/src/persistence/DynamoDBPersistenceLayer.ts b/packages/idempotency/src/persistence/DynamoDBPersistenceLayer.ts index e13bdebdfd..91e75f7cbf 100644 --- a/packages/idempotency/src/persistence/DynamoDBPersistenceLayer.ts +++ b/packages/idempotency/src/persistence/DynamoDBPersistenceLayer.ts @@ -6,6 +6,7 @@ import { IdempotencyRecordStatus } from '../constants'; import type { DynamoDBPersistenceOptions } from '../types'; import { AttributeValue, + ConditionalCheckFailedException, DeleteItemCommand, DynamoDBClient, DynamoDBClientConfig, @@ -202,15 +203,13 @@ class DynamoDBPersistenceLayer extends BasePersistenceLayer { ); } catch (error) { if (error instanceof DynamoDBServiceException) { - if (error.name === 'ConditionalCheckFailedException') { + if (error instanceof ConditionalCheckFailedException) { throw new IdempotencyItemAlreadyExistsError( - `Failed to put record for already existing idempotency key: ${record.idempotencyKey}` + `Failed to put record for already existing idempotency key: ${record.idempotencyKey}`, + error.Item ); } } - - throw error; - } } protected async _updateRecord(record: IdempotencyRecord): Promise { From 11b2bf05aa89babe6b47002eeb3a91e817be4180 Mon Sep 17 00:00:00 2001 From: Theophilus Ajayi Date: Thu, 2 Nov 2023 09:26:47 +0000 Subject: [PATCH 03/12] custom error field to fetch original Item from failed Item from Db --- packages/idempotency/src/errors.ts | 1 + .../src/persistence/DynamoDBPersistenceLayer.ts | 14 +++++++++----- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/packages/idempotency/src/errors.ts b/packages/idempotency/src/errors.ts index 30223a4564..f460aa4ee1 100644 --- a/packages/idempotency/src/errors.ts +++ b/packages/idempotency/src/errors.ts @@ -1,4 +1,5 @@ import { AttributeValue } from '@aws-sdk/client-dynamodb'; +//import { IdempotencyRecord } from './persistence'; /** * Item attempting to be inserted into persistence store already exists and is not expired */ diff --git a/packages/idempotency/src/persistence/DynamoDBPersistenceLayer.ts b/packages/idempotency/src/persistence/DynamoDBPersistenceLayer.ts index 91e75f7cbf..972a30c5a6 100644 --- a/packages/idempotency/src/persistence/DynamoDBPersistenceLayer.ts +++ b/packages/idempotency/src/persistence/DynamoDBPersistenceLayer.ts @@ -199,17 +199,21 @@ class DynamoDBPersistenceLayer extends BasePersistenceLayer { ':inprogress': IdempotencyRecordStatus.INPROGRESS, }), ConditionExpression: conditionExpression, + ReturnValuesOnConditionCheckFailure: 'ALL_OLD', }) ); } catch (error) { if (error instanceof DynamoDBServiceException) { - if (error instanceof ConditionalCheckFailedException) { - throw new IdempotencyItemAlreadyExistsError( - `Failed to put record for already existing idempotency key: ${record.idempotencyKey}`, - error.Item - ); + if (error.name === 'ConditionalCheckFailedException') { + if (error instanceof ConditionalCheckFailedException) { + throw new IdempotencyItemAlreadyExistsError( + `Failed to put record for already existing idempotency key: ${record.idempotencyKey}`, + error.Item + ); + } } } + } } protected async _updateRecord(record: IdempotencyRecord): Promise { From 393685a046a45ad40c4ddcd85fbd041cdaed23cd Mon Sep 17 00:00:00 2001 From: Theophilus Ajayi Date: Thu, 2 Nov 2023 09:32:57 +0000 Subject: [PATCH 04/12] set base persitence layer to undefined. --- packages/idempotency/src/persistence/BasePersistenceLayer.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/idempotency/src/persistence/BasePersistenceLayer.ts b/packages/idempotency/src/persistence/BasePersistenceLayer.ts index e90118d042..4f52cf5ae2 100644 --- a/packages/idempotency/src/persistence/BasePersistenceLayer.ts +++ b/packages/idempotency/src/persistence/BasePersistenceLayer.ts @@ -154,7 +154,7 @@ abstract class BasePersistenceLayer implements BasePersistenceLayerInterface { } if (this.getFromCache(idempotencyRecord.idempotencyKey)) { - throw new IdempotencyItemAlreadyExistsError(); + throw new IdempotencyItemAlreadyExistsError('', undefined); } await this._putRecord(idempotencyRecord); From bc600d0add29ed644180ad7ded9a719d9bf2f0eb Mon Sep 17 00:00:00 2001 From: Theophilus Ajayi Date: Thu, 9 Nov 2023 20:17:35 +0000 Subject: [PATCH 05/12] converted error type to IdempotencyRecord --- .../idempotency/src/IdempotencyHandler.ts | 9 ++++---- packages/idempotency/src/errors.ts | 10 +++------ .../persistence/DynamoDBPersistenceLayer.ts | 21 +++++++++++++------ 3 files changed, 23 insertions(+), 17 deletions(-) diff --git a/packages/idempotency/src/IdempotencyHandler.ts b/packages/idempotency/src/IdempotencyHandler.ts index 7deb2c53f7..f2d97aef24 100644 --- a/packages/idempotency/src/IdempotencyHandler.ts +++ b/packages/idempotency/src/IdempotencyHandler.ts @@ -313,10 +313,11 @@ export class IdempotencyHandler { ); } catch (e) { if (e instanceof IdempotencyItemAlreadyExistsError) { - const idempotencyRecord: IdempotencyRecord = - await this.#persistenceStore.getRecord( - this.#functionPayloadToBeHashed - ); + const idempotencyRecord: IdempotencyRecord = e.existingRecord; + // const idempotencyRecord: IdempotencyRecord = + // await this.#persistenceStore.getRecord( + // this.#functionPayloadToBeHashed + // ); return IdempotencyHandler.determineResultFromIdempotencyRecord( idempotencyRecord diff --git a/packages/idempotency/src/errors.ts b/packages/idempotency/src/errors.ts index f460aa4ee1..0e0f0434d3 100644 --- a/packages/idempotency/src/errors.ts +++ b/packages/idempotency/src/errors.ts @@ -1,15 +1,11 @@ -import { AttributeValue } from '@aws-sdk/client-dynamodb'; -//import { IdempotencyRecord } from './persistence'; +import { IdempotencyRecord } from './persistence'; /** * Item attempting to be inserted into persistence store already exists and is not expired */ class IdempotencyItemAlreadyExistsError extends Error { - public existingRecord: Record | undefined; + public existingRecord: IdempotencyRecord; - public constructor( - message: string, - existingRecord: Record | undefined - ) { + public constructor(message: string, existingRecord: IdempotencyRecord) { super(message); this.existingRecord = existingRecord; } diff --git a/packages/idempotency/src/persistence/DynamoDBPersistenceLayer.ts b/packages/idempotency/src/persistence/DynamoDBPersistenceLayer.ts index 972a30c5a6..d12799bd2b 100644 --- a/packages/idempotency/src/persistence/DynamoDBPersistenceLayer.ts +++ b/packages/idempotency/src/persistence/DynamoDBPersistenceLayer.ts @@ -204,13 +204,22 @@ class DynamoDBPersistenceLayer extends BasePersistenceLayer { ); } catch (error) { if (error instanceof DynamoDBServiceException) { - if (error.name === 'ConditionalCheckFailedException') { - if (error instanceof ConditionalCheckFailedException) { - throw new IdempotencyItemAlreadyExistsError( - `Failed to put record for already existing idempotency key: ${record.idempotencyKey}`, - error.Item - ); + if (error instanceof ConditionalCheckFailedException) { + if (!error.Item) { + throw new Error('Item is undefined'); } + const Item = unmarshall(error.Item); + throw new IdempotencyItemAlreadyExistsError( + `Failed to put record for already existing idempotency key: ${record.idempotencyKey}`, + new IdempotencyRecord({ + idempotencyKey: Item[this.keyAttr], + status: Item[this.statusAttr], + expiryTimestamp: Item[this.expiryAttr], + inProgressExpiryTimestamp: Item[this.inProgressExpiryAttr], + responseData: Item[this.dataAttr], + payloadHash: Item[this.validationKeyAttr], + }) + ); } } } From 2e2c79f0a032803248a10ad1efe2c142b7079a29 Mon Sep 17 00:00:00 2001 From: Theophilus Ajayi Date: Sun, 12 Nov 2023 13:54:43 +0000 Subject: [PATCH 06/12] dev branch commit and unit tests --- .../idempotency/src/IdempotencyHandler.ts | 10 ++-- .../src/persistence/BasePersistenceLayer.ts | 5 +- .../tests/unit/IdempotencyHandler.test.ts | 29 ++++++++++-- .../tests/unit/idempotencyDecorator.test.ts | 25 ++++++++-- .../tests/unit/makeHandlerIdempotent.test.ts | 42 +++++++++++++++-- .../tests/unit/makeIdempotent.test.ts | 42 +++++++++++++++-- .../persistence/BasePersistenceLayer.test.ts | 12 ++++- .../DynamoDbPersistenceLayer.test.ts | 47 +++++++++++++++++-- 8 files changed, 186 insertions(+), 26 deletions(-) diff --git a/packages/idempotency/src/IdempotencyHandler.ts b/packages/idempotency/src/IdempotencyHandler.ts index f2d97aef24..862e9888b9 100644 --- a/packages/idempotency/src/IdempotencyHandler.ts +++ b/packages/idempotency/src/IdempotencyHandler.ts @@ -313,11 +313,11 @@ export class IdempotencyHandler { ); } catch (e) { if (e instanceof IdempotencyItemAlreadyExistsError) { - const idempotencyRecord: IdempotencyRecord = e.existingRecord; - // const idempotencyRecord: IdempotencyRecord = - // await this.#persistenceStore.getRecord( - // this.#functionPayloadToBeHashed - // ); + //const idempotencyRecord: IdempotencyRecord = e.existingRecord; + const idempotencyRecord: IdempotencyRecord = + await this.#persistenceStore.getRecord( + this.#functionPayloadToBeHashed + ); return IdempotencyHandler.determineResultFromIdempotencyRecord( idempotencyRecord diff --git a/packages/idempotency/src/persistence/BasePersistenceLayer.ts b/packages/idempotency/src/persistence/BasePersistenceLayer.ts index 4f52cf5ae2..108a7c0adf 100644 --- a/packages/idempotency/src/persistence/BasePersistenceLayer.ts +++ b/packages/idempotency/src/persistence/BasePersistenceLayer.ts @@ -154,7 +154,10 @@ abstract class BasePersistenceLayer implements BasePersistenceLayerInterface { } if (this.getFromCache(idempotencyRecord.idempotencyKey)) { - throw new IdempotencyItemAlreadyExistsError('', undefined); + throw new IdempotencyItemAlreadyExistsError( + `Failed to put record for already existing idempotency key: ${idempotencyRecord.idempotencyKey}`, + idempotencyRecord + ); } await this._putRecord(idempotencyRecord); diff --git a/packages/idempotency/tests/unit/IdempotencyHandler.test.ts b/packages/idempotency/tests/unit/IdempotencyHandler.test.ts index deeccee875..4c322eab9d 100644 --- a/packages/idempotency/tests/unit/IdempotencyHandler.test.ts +++ b/packages/idempotency/tests/unit/IdempotencyHandler.test.ts @@ -56,7 +56,6 @@ describe('Class IdempotencyHandler', () => { expiryTimestamp: Date.now() + 1000, // should be in the future inProgressExpiryTimestamp: 0, // less than current time in milliseconds responseData: { responseData: 'responseData' }, - payloadHash: 'payloadHash', status: IdempotencyRecordStatus.INPROGRESS, }); @@ -75,7 +74,6 @@ describe('Class IdempotencyHandler', () => { expiryTimestamp: Date.now() + 1000, // should be in the future inProgressExpiryTimestamp: new Date().getUTCMilliseconds() - 1000, // should be in the past responseData: { responseData: 'responseData' }, - payloadHash: 'payloadHash', status: IdempotencyRecordStatus.INPROGRESS, }); @@ -94,7 +92,6 @@ describe('Class IdempotencyHandler', () => { expiryTimestamp: new Date().getUTCMilliseconds() - 1000, // should be in the past inProgressExpiryTimestamp: 0, // less than current time in milliseconds responseData: { responseData: 'responseData' }, - payloadHash: 'payloadHash', status: IdempotencyRecordStatus.EXPIRED, }); @@ -109,10 +106,22 @@ describe('Class IdempotencyHandler', () => { describe('Method: handle', () => { test('when IdempotencyAlreadyInProgressError is thrown, it retries once', async () => { + // Arrange + const existingRecord = new IdempotencyRecord({ + idempotencyKey: 'idempotence-key', + status: 'COMPLETED', + expiryTimestamp: Date.now() / 1000 - 1, + responseData: { test: 'data' }, + }); // Prepare const saveInProgressSpy = jest .spyOn(persistenceStore, 'saveInProgress') - .mockRejectedValueOnce(new IdempotencyItemAlreadyExistsError()); + .mockRejectedValueOnce( + new IdempotencyItemAlreadyExistsError( + 'Failed to put record for already existing idempotency key: idempotence-key', + existingRecord + ) + ); // Act & Assess await expect(idempotentHandler.handle()).rejects.toThrow(); @@ -123,7 +132,17 @@ describe('Class IdempotencyHandler', () => { // Prepare const mockProcessIdempotency = jest .spyOn(persistenceStore, 'saveInProgress') - .mockRejectedValue(new IdempotencyItemAlreadyExistsError()); + .mockRejectedValue( + new IdempotencyItemAlreadyExistsError( + 'Failed to put record for already existing idempotency key: my-lambda-function#mocked-hash', + new IdempotencyRecord({ + idempotencyKey: 'my-lambda-function#mocked-hash', + status: IdempotencyRecordStatus.EXPIRED, + payloadHash: 'different-hash', + expiryTimestamp: Date.now() / 1000 - 1, + }) + ) + ); jest.spyOn(persistenceStore, 'getRecord').mockResolvedValue( new IdempotencyRecord({ status: IdempotencyRecordStatus.EXPIRED, diff --git a/packages/idempotency/tests/unit/idempotencyDecorator.test.ts b/packages/idempotency/tests/unit/idempotencyDecorator.test.ts index 1a293d8569..a3431168e3 100644 --- a/packages/idempotency/tests/unit/idempotencyDecorator.test.ts +++ b/packages/idempotency/tests/unit/idempotencyDecorator.test.ts @@ -124,7 +124,13 @@ describe('Given a class with a function to decorate', (classWithLambdaHandler = let resultingError: Error; beforeEach(async () => { mockSaveInProgress.mockRejectedValue( - new IdempotencyItemAlreadyExistsError() + new IdempotencyItemAlreadyExistsError( + 'Failed to put record for already existing idempotency key: key', + new IdempotencyRecord({ + idempotencyKey: 'key', + status: IdempotencyRecordStatus.INPROGRESS, + }) + ) ); const idempotencyOptions: IdempotencyRecordOptions = { idempotencyKey: 'key', @@ -164,7 +170,13 @@ describe('Given a class with a function to decorate', (classWithLambdaHandler = let resultingError: Error; beforeEach(async () => { mockSaveInProgress.mockRejectedValue( - new IdempotencyItemAlreadyExistsError() + new IdempotencyItemAlreadyExistsError( + 'Failed to put record for already existing idempotency key: key', + new IdempotencyRecord({ + idempotencyKey: 'key', + status: IdempotencyRecordStatus.EXPIRED, + }) + ) ); const idempotencyOptions: IdempotencyRecordOptions = { idempotencyKey: 'key', @@ -203,7 +215,14 @@ describe('Given a class with a function to decorate', (classWithLambdaHandler = describe('When wrapping a function with previous execution that is COMPLETED', () => { beforeEach(async () => { mockSaveInProgress.mockRejectedValue( - new IdempotencyItemAlreadyExistsError() + new IdempotencyItemAlreadyExistsError( + 'Failed to put record for already existing idempotency key: key', + new IdempotencyRecord({ + idempotencyKey: 'key', + status: IdempotencyRecordStatus.COMPLETED, + responseData: 'Hi', + }) + ) ); const idempotencyOptions: IdempotencyRecordOptions = { idempotencyKey: 'key', diff --git a/packages/idempotency/tests/unit/makeHandlerIdempotent.test.ts b/packages/idempotency/tests/unit/makeHandlerIdempotent.test.ts index db064e6f38..90efa418e4 100644 --- a/packages/idempotency/tests/unit/makeHandlerIdempotent.test.ts +++ b/packages/idempotency/tests/unit/makeHandlerIdempotent.test.ts @@ -159,7 +159,19 @@ describe('Middleware: makeHandlerIdempotent', () => { ).use(makeHandlerIdempotent(mockIdempotencyOptions)); jest .spyOn(mockIdempotencyOptions.persistenceStore, 'saveInProgress') - .mockRejectedValue(new IdempotencyItemAlreadyExistsError()); + .mockRejectedValue( + new IdempotencyItemAlreadyExistsError( + 'Failed to put record for already existing idempotency key: idempotencyKey', + new IdempotencyRecord({ + idempotencyKey: 'idempotencyKey', + expiryTimestamp: Date.now() + 10000, + inProgressExpiryTimestamp: 0, + responseData: { response: false }, + payloadHash: 'payloadHash', + status: IdempotencyRecordStatus.COMPLETED, + }) + ) + ); const stubRecord = new IdempotencyRecord({ idempotencyKey: 'idempotencyKey', expiryTimestamp: Date.now() + 10000, @@ -187,7 +199,19 @@ describe('Middleware: makeHandlerIdempotent', () => { ).use(makeHandlerIdempotent(mockIdempotencyOptions)); jest .spyOn(mockIdempotencyOptions.persistenceStore, 'saveInProgress') - .mockRejectedValue(new IdempotencyItemAlreadyExistsError()); + .mockRejectedValue( + new IdempotencyItemAlreadyExistsError( + 'Failed to put record for already existing idempotency key: idempotencyKey', + new IdempotencyRecord({ + idempotencyKey: 'idempotencyKey', + expiryTimestamp: Date.now() + 10000, + inProgressExpiryTimestamp: 0, + responseData: { response: false }, + payloadHash: 'payloadHash', + status: IdempotencyRecordStatus.EXPIRED, + }) + ) + ); const stubRecordInconsistent = new IdempotencyRecord({ idempotencyKey: 'idempotencyKey', expiryTimestamp: Date.now() + 10000, @@ -223,7 +247,19 @@ describe('Middleware: makeHandlerIdempotent', () => { ).use(makeHandlerIdempotent(mockIdempotencyOptions)); jest .spyOn(mockIdempotencyOptions.persistenceStore, 'saveInProgress') - .mockRejectedValue(new IdempotencyItemAlreadyExistsError()); + .mockRejectedValue( + new IdempotencyItemAlreadyExistsError( + 'Failed to put record for already existing idempotency key: idempotencyKey', + new IdempotencyRecord({ + idempotencyKey: 'idempotencyKey', + expiryTimestamp: Date.now() + 10000, + inProgressExpiryTimestamp: 0, + responseData: { response: false }, + payloadHash: 'payloadHash', + status: IdempotencyRecordStatus.EXPIRED, + }) + ) + ); const stubRecordInconsistent = new IdempotencyRecord({ idempotencyKey: 'idempotencyKey', expiryTimestamp: Date.now() + 10000, diff --git a/packages/idempotency/tests/unit/makeIdempotent.test.ts b/packages/idempotency/tests/unit/makeIdempotent.test.ts index d46bc7ad6b..2d5a95b4ab 100644 --- a/packages/idempotency/tests/unit/makeIdempotent.test.ts +++ b/packages/idempotency/tests/unit/makeIdempotent.test.ts @@ -177,7 +177,19 @@ describe('Function: makeIdempotent', () => { ); jest .spyOn(mockIdempotencyOptions.persistenceStore, 'saveInProgress') - .mockRejectedValue(new IdempotencyItemAlreadyExistsError()); + .mockRejectedValue( + new IdempotencyItemAlreadyExistsError( + 'Failed to put record for already existing idempotency key: idempotencyKey', + new IdempotencyRecord({ + idempotencyKey: 'idempotencyKey', + expiryTimestamp: Date.now() + 10000, + inProgressExpiryTimestamp: 0, + responseData: { response: false }, + payloadHash: 'payloadHash', + status: IdempotencyRecordStatus.COMPLETED, + }) + ) + ); const stubRecord = new IdempotencyRecord({ idempotencyKey: 'idempotencyKey', expiryTimestamp: Date.now() + 10000, @@ -209,7 +221,19 @@ describe('Function: makeIdempotent', () => { ); jest .spyOn(mockIdempotencyOptions.persistenceStore, 'saveInProgress') - .mockRejectedValue(new IdempotencyItemAlreadyExistsError()); + .mockRejectedValue( + new IdempotencyItemAlreadyExistsError( + 'Failed to put record for already existing idempotency key: idempotencyKey', + new IdempotencyRecord({ + idempotencyKey: 'idempotencyKey', + expiryTimestamp: Date.now() + 10000, + inProgressExpiryTimestamp: 0, + responseData: { response: false }, + payloadHash: 'payloadHash', + status: IdempotencyRecordStatus.COMPLETED, + }) + ) + ); const stubRecordInconsistent = new IdempotencyRecord({ idempotencyKey: 'idempotencyKey', expiryTimestamp: Date.now() + 10000, @@ -249,7 +273,19 @@ describe('Function: makeIdempotent', () => { ); jest .spyOn(mockIdempotencyOptions.persistenceStore, 'saveInProgress') - .mockRejectedValue(new IdempotencyItemAlreadyExistsError()); + .mockRejectedValue( + new IdempotencyItemAlreadyExistsError( + 'Failed to put record for already existing idempotency key: idempotencyKey', + new IdempotencyRecord({ + idempotencyKey: 'idempotencyKey', + expiryTimestamp: Date.now() + 10000, + inProgressExpiryTimestamp: 0, + responseData: { response: false }, + payloadHash: 'payloadHash', + status: IdempotencyRecordStatus.EXPIRED, + }) + ) + ); const stubRecordInconsistent = new IdempotencyRecord({ idempotencyKey: 'idempotencyKey', expiryTimestamp: Date.now() + 10000, diff --git a/packages/idempotency/tests/unit/persistence/BasePersistenceLayer.test.ts b/packages/idempotency/tests/unit/persistence/BasePersistenceLayer.test.ts index 3224c0661b..b26423a0d9 100644 --- a/packages/idempotency/tests/unit/persistence/BasePersistenceLayer.test.ts +++ b/packages/idempotency/tests/unit/persistence/BasePersistenceLayer.test.ts @@ -407,7 +407,17 @@ describe('Class: BasePersistenceLayer', () => { // Act & Assess await expect( persistenceLayer.saveInProgress({ foo: 'bar' }) - ).rejects.toThrow(new IdempotencyItemAlreadyExistsError()); + ).rejects.toThrow( + new IdempotencyItemAlreadyExistsError( + 'Failed to put record for already existing idempotency key: my-lambda-function#mocked-hash', + new IdempotencyRecord({ + idempotencyKey: 'my-lambda-function#mocked-hash', + status: IdempotencyRecordStatus.EXPIRED, + payloadHash: 'different-hash', + expiryTimestamp: Date.now() / 1000 - 1, + }) + ) + ); expect(putRecordSpy).toHaveBeenCalledTimes(0); }); diff --git a/packages/idempotency/tests/unit/persistence/DynamoDbPersistenceLayer.test.ts b/packages/idempotency/tests/unit/persistence/DynamoDbPersistenceLayer.test.ts index aa817e7b77..66461fccb1 100644 --- a/packages/idempotency/tests/unit/persistence/DynamoDbPersistenceLayer.test.ts +++ b/packages/idempotency/tests/unit/persistence/DynamoDbPersistenceLayer.test.ts @@ -13,11 +13,12 @@ import type { DynamoDBPersistenceOptions } from '../../../src/types'; import { IdempotencyRecordStatus } from '../../../src'; import { DynamoDBClient, - DynamoDBServiceException, + //DynamoDBServiceException, PutItemCommand, GetItemCommand, UpdateItemCommand, DeleteItemCommand, + ConditionalCheckFailedException, } from '@aws-sdk/client-dynamodb'; import { marshall } from '@aws-sdk/util-dynamodb'; import { mockClient } from 'aws-sdk-client-mock'; @@ -395,19 +396,32 @@ describe('Class: DynamoDBPersistenceLayer', () => { expiryTimestamp: 0, }); client.on(PutItemCommand).rejects( - new DynamoDBServiceException({ - $fault: 'client', + new ConditionalCheckFailedException({ + //$fault: 'client', $metadata: { httpStatusCode: 400, requestId: 'someRequestId', }, - name: 'ConditionalCheckFailedException', + message: 'Conditional check failed', + Item: { + id: { S: 'test-key' }, + status: { S: 'INPROGRESS' }, + expiration: { N: Date.now().toString() }, + }, + //name: 'ConditionalCheckFailedException', }) ); // Act & Assess await expect(persistenceLayer._putRecord(record)).rejects.toThrowError( - IdempotencyItemAlreadyExistsError + new IdempotencyItemAlreadyExistsError( + `Failed to put record for already existing idempotency key: ${record.idempotencyKey}`, + new IdempotencyRecord({ + idempotencyKey: record.idempotencyKey, + status: IdempotencyRecordStatus.EXPIRED, + expiryTimestamp: Date.now() / 1000 - 1, + }) + ) ); }); @@ -676,4 +690,27 @@ describe('Class: DynamoDBPersistenceLayer', () => { }); }); }); + + //write test for when Item is undefined + test('_putRecord throws Error when Item is undefined', async () => { + // Prepare + const persistenceLayer = new TestDynamoDBPersistenceLayer({ + tableName: dummyTableName, + }); + const mockRecord = new IdempotencyRecord({ + idempotencyKey: 'test-key', + status: 'INPROGRESS', + expiryTimestamp: Date.now(), + }); + + DynamoDBClient.prototype.send = jest.fn().mockRejectedValueOnce( + new ConditionalCheckFailedException({ + message: 'Conditional check failed', + $metadata: {}, + }) + ); + await expect(persistenceLayer._putRecord(mockRecord)).rejects.toThrowError( + 'Item is undefined' + ); + }); }); From e601d9d72dd3ba61a8a5c75e291d6f4e9725b1e9 Mon Sep 17 00:00:00 2001 From: Theophilus Ajayi Date: Thu, 16 Nov 2023 09:33:19 +0000 Subject: [PATCH 07/12] pull request corrections --- packages/idempotency/src/errors.ts | 2 +- .../persistence/DynamoDBPersistenceLayer.ts | 33 +++++++++---------- .../DynamoDbPersistenceLayer.test.ts | 7 ++-- 3 files changed, 18 insertions(+), 24 deletions(-) diff --git a/packages/idempotency/src/errors.ts b/packages/idempotency/src/errors.ts index 0e0f0434d3..cfabaa6558 100644 --- a/packages/idempotency/src/errors.ts +++ b/packages/idempotency/src/errors.ts @@ -1,4 +1,4 @@ -import { IdempotencyRecord } from './persistence'; +import type { IdempotencyRecord } from './persistence'; /** * Item attempting to be inserted into persistence store already exists and is not expired */ diff --git a/packages/idempotency/src/persistence/DynamoDBPersistenceLayer.ts b/packages/idempotency/src/persistence/DynamoDBPersistenceLayer.ts index d12799bd2b..3e17971688 100644 --- a/packages/idempotency/src/persistence/DynamoDBPersistenceLayer.ts +++ b/packages/idempotency/src/persistence/DynamoDBPersistenceLayer.ts @@ -10,7 +10,6 @@ import { DeleteItemCommand, DynamoDBClient, DynamoDBClientConfig, - DynamoDBServiceException, GetItemCommand, PutItemCommand, UpdateItemCommand, @@ -203,24 +202,22 @@ class DynamoDBPersistenceLayer extends BasePersistenceLayer { }) ); } catch (error) { - if (error instanceof DynamoDBServiceException) { - if (error instanceof ConditionalCheckFailedException) { - if (!error.Item) { - throw new Error('Item is undefined'); - } - const Item = unmarshall(error.Item); - throw new IdempotencyItemAlreadyExistsError( - `Failed to put record for already existing idempotency key: ${record.idempotencyKey}`, - new IdempotencyRecord({ - idempotencyKey: Item[this.keyAttr], - status: Item[this.statusAttr], - expiryTimestamp: Item[this.expiryAttr], - inProgressExpiryTimestamp: Item[this.inProgressExpiryAttr], - responseData: Item[this.dataAttr], - payloadHash: Item[this.validationKeyAttr], - }) - ); + if (error instanceof ConditionalCheckFailedException) { + if (!error.Item) { + throw new Error('item is undefined'); } + const item = unmarshall(error.Item); + throw new IdempotencyItemAlreadyExistsError( + `Failed to put record for already existing idempotency key: ${record.idempotencyKey}`, + new IdempotencyRecord({ + idempotencyKey: item[this.keyAttr], + status: item[this.statusAttr], + expiryTimestamp: item[this.expiryAttr], + inProgressExpiryTimestamp: item[this.inProgressExpiryAttr], + responseData: item[this.dataAttr], + payloadHash: item[this.validationKeyAttr], + }) + ); } } } diff --git a/packages/idempotency/tests/unit/persistence/DynamoDbPersistenceLayer.test.ts b/packages/idempotency/tests/unit/persistence/DynamoDbPersistenceLayer.test.ts index 66461fccb1..04d6640885 100644 --- a/packages/idempotency/tests/unit/persistence/DynamoDbPersistenceLayer.test.ts +++ b/packages/idempotency/tests/unit/persistence/DynamoDbPersistenceLayer.test.ts @@ -12,13 +12,12 @@ import { IdempotencyRecord } from '../../../src/persistence'; import type { DynamoDBPersistenceOptions } from '../../../src/types'; import { IdempotencyRecordStatus } from '../../../src'; import { + ConditionalCheckFailedException, DynamoDBClient, - //DynamoDBServiceException, PutItemCommand, GetItemCommand, UpdateItemCommand, DeleteItemCommand, - ConditionalCheckFailedException, } from '@aws-sdk/client-dynamodb'; import { marshall } from '@aws-sdk/util-dynamodb'; import { mockClient } from 'aws-sdk-client-mock'; @@ -397,7 +396,6 @@ describe('Class: DynamoDBPersistenceLayer', () => { }); client.on(PutItemCommand).rejects( new ConditionalCheckFailedException({ - //$fault: 'client', $metadata: { httpStatusCode: 400, requestId: 'someRequestId', @@ -408,7 +406,6 @@ describe('Class: DynamoDBPersistenceLayer', () => { status: { S: 'INPROGRESS' }, expiration: { N: Date.now().toString() }, }, - //name: 'ConditionalCheckFailedException', }) ); @@ -710,7 +707,7 @@ describe('Class: DynamoDBPersistenceLayer', () => { }) ); await expect(persistenceLayer._putRecord(mockRecord)).rejects.toThrowError( - 'Item is undefined' + 'item is undefined' ); }); }); From 44f8ce58aa49e9711c51f68b67b774c5305efb0f Mon Sep 17 00:00:00 2001 From: Andrea Amorosi Date: Tue, 19 Dec 2023 19:12:50 +0800 Subject: [PATCH 08/12] chore: remove comment --- packages/idempotency/src/IdempotencyHandler.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/idempotency/src/IdempotencyHandler.ts b/packages/idempotency/src/IdempotencyHandler.ts index 862e9888b9..7deb2c53f7 100644 --- a/packages/idempotency/src/IdempotencyHandler.ts +++ b/packages/idempotency/src/IdempotencyHandler.ts @@ -313,7 +313,6 @@ export class IdempotencyHandler { ); } catch (e) { if (e instanceof IdempotencyItemAlreadyExistsError) { - //const idempotencyRecord: IdempotencyRecord = e.existingRecord; const idempotencyRecord: IdempotencyRecord = await this.#persistenceStore.getRecord( this.#functionPayloadToBeHashed From d98794f4627cb8177379f8d08715f63694fcd1d5 Mon Sep 17 00:00:00 2001 From: Andrea Amorosi Date: Tue, 19 Dec 2023 19:21:28 +0800 Subject: [PATCH 09/12] chore: simplify conditional item hydration --- .../src/persistence/DynamoDBPersistenceLayer.ts | 5 +---- .../unit/persistence/DynamoDbPersistenceLayer.test.ts | 7 +++---- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/packages/idempotency/src/persistence/DynamoDBPersistenceLayer.ts b/packages/idempotency/src/persistence/DynamoDBPersistenceLayer.ts index 3e17971688..da63b3c097 100644 --- a/packages/idempotency/src/persistence/DynamoDBPersistenceLayer.ts +++ b/packages/idempotency/src/persistence/DynamoDBPersistenceLayer.ts @@ -202,10 +202,7 @@ class DynamoDBPersistenceLayer extends BasePersistenceLayer { }) ); } catch (error) { - if (error instanceof ConditionalCheckFailedException) { - if (!error.Item) { - throw new Error('item is undefined'); - } + if (error instanceof ConditionalCheckFailedException && error.Item) { const item = unmarshall(error.Item); throw new IdempotencyItemAlreadyExistsError( `Failed to put record for already existing idempotency key: ${record.idempotencyKey}`, diff --git a/packages/idempotency/tests/unit/persistence/DynamoDbPersistenceLayer.test.ts b/packages/idempotency/tests/unit/persistence/DynamoDbPersistenceLayer.test.ts index 04d6640885..4a0195c19e 100644 --- a/packages/idempotency/tests/unit/persistence/DynamoDbPersistenceLayer.test.ts +++ b/packages/idempotency/tests/unit/persistence/DynamoDbPersistenceLayer.test.ts @@ -688,7 +688,6 @@ describe('Class: DynamoDBPersistenceLayer', () => { }); }); - //write test for when Item is undefined test('_putRecord throws Error when Item is undefined', async () => { // Prepare const persistenceLayer = new TestDynamoDBPersistenceLayer({ @@ -706,8 +705,8 @@ describe('Class: DynamoDBPersistenceLayer', () => { $metadata: {}, }) ); - await expect(persistenceLayer._putRecord(mockRecord)).rejects.toThrowError( - 'item is undefined' - ); + await expect( + persistenceLayer._putRecord(mockRecord) + ).rejects.toThrowError(); }); }); From 587da380591704554c7f7a3d7c02f24f4856dab7 Mon Sep 17 00:00:00 2001 From: Andrea Amorosi Date: Tue, 19 Dec 2023 19:33:52 +0800 Subject: [PATCH 10/12] chore: rebase --- .../idempotency/src/persistence/DynamoDBPersistenceLayer.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/idempotency/src/persistence/DynamoDBPersistenceLayer.ts b/packages/idempotency/src/persistence/DynamoDBPersistenceLayer.ts index da63b3c097..b3ecd947c5 100644 --- a/packages/idempotency/src/persistence/DynamoDBPersistenceLayer.ts +++ b/packages/idempotency/src/persistence/DynamoDBPersistenceLayer.ts @@ -216,6 +216,8 @@ class DynamoDBPersistenceLayer extends BasePersistenceLayer { }) ); } + + throw error; } } From eb132dd98c639eeb19e60561d536b9733fe15cb3 Mon Sep 17 00:00:00 2001 From: Andrea Amorosi Date: Wed, 20 Dec 2023 00:35:55 +0800 Subject: [PATCH 11/12] fix: use returned items + tests --- .../idempotency/src/IdempotencyHandler.ts | 5 ++- packages/idempotency/src/errors.ts | 5 ++- .../persistence/DynamoDBPersistenceLayer.ts | 13 +++--- .../tests/unit/IdempotencyHandler.test.ts | 43 ++++++++++--------- .../tests/unit/idempotencyDecorator.test.ts | 25 ++--------- .../tests/unit/makeHandlerIdempotent.test.ts | 42 ++---------------- .../tests/unit/makeIdempotent.test.ts | 42 ++---------------- .../persistence/BasePersistenceLayer.test.ts | 12 +----- 8 files changed, 47 insertions(+), 140 deletions(-) diff --git a/packages/idempotency/src/IdempotencyHandler.ts b/packages/idempotency/src/IdempotencyHandler.ts index 7deb2c53f7..0808edf999 100644 --- a/packages/idempotency/src/IdempotencyHandler.ts +++ b/packages/idempotency/src/IdempotencyHandler.ts @@ -314,9 +314,10 @@ export class IdempotencyHandler { } catch (e) { if (e instanceof IdempotencyItemAlreadyExistsError) { const idempotencyRecord: IdempotencyRecord = - await this.#persistenceStore.getRecord( + e.existingRecord || + (await this.#persistenceStore.getRecord( this.#functionPayloadToBeHashed - ); + )); return IdempotencyHandler.determineResultFromIdempotencyRecord( idempotencyRecord diff --git a/packages/idempotency/src/errors.ts b/packages/idempotency/src/errors.ts index cfabaa6558..674e05d6a6 100644 --- a/packages/idempotency/src/errors.ts +++ b/packages/idempotency/src/errors.ts @@ -1,11 +1,12 @@ import type { IdempotencyRecord } from './persistence'; + /** * Item attempting to be inserted into persistence store already exists and is not expired */ class IdempotencyItemAlreadyExistsError extends Error { - public existingRecord: IdempotencyRecord; + public existingRecord?: IdempotencyRecord; - public constructor(message: string, existingRecord: IdempotencyRecord) { + public constructor(message?: string, existingRecord?: IdempotencyRecord) { super(message); this.existingRecord = existingRecord; } diff --git a/packages/idempotency/src/persistence/DynamoDBPersistenceLayer.ts b/packages/idempotency/src/persistence/DynamoDBPersistenceLayer.ts index b3ecd947c5..f30fa83b57 100644 --- a/packages/idempotency/src/persistence/DynamoDBPersistenceLayer.ts +++ b/packages/idempotency/src/persistence/DynamoDBPersistenceLayer.ts @@ -202,10 +202,10 @@ class DynamoDBPersistenceLayer extends BasePersistenceLayer { }) ); } catch (error) { - if (error instanceof ConditionalCheckFailedException && error.Item) { - const item = unmarshall(error.Item); - throw new IdempotencyItemAlreadyExistsError( - `Failed to put record for already existing idempotency key: ${record.idempotencyKey}`, + if (error instanceof ConditionalCheckFailedException) { + const item = error.Item && unmarshall(error.Item); + const idempotencyRecord = + item && new IdempotencyRecord({ idempotencyKey: item[this.keyAttr], status: item[this.statusAttr], @@ -213,7 +213,10 @@ class DynamoDBPersistenceLayer extends BasePersistenceLayer { inProgressExpiryTimestamp: item[this.inProgressExpiryAttr], responseData: item[this.dataAttr], payloadHash: item[this.validationKeyAttr], - }) + }); + throw new IdempotencyItemAlreadyExistsError( + `Failed to put record for already existing idempotency key: ${record.idempotencyKey}`, + idempotencyRecord ); } diff --git a/packages/idempotency/tests/unit/IdempotencyHandler.test.ts b/packages/idempotency/tests/unit/IdempotencyHandler.test.ts index 4c322eab9d..1ebb14e0be 100644 --- a/packages/idempotency/tests/unit/IdempotencyHandler.test.ts +++ b/packages/idempotency/tests/unit/IdempotencyHandler.test.ts @@ -56,6 +56,7 @@ describe('Class IdempotencyHandler', () => { expiryTimestamp: Date.now() + 1000, // should be in the future inProgressExpiryTimestamp: 0, // less than current time in milliseconds responseData: { responseData: 'responseData' }, + payloadHash: 'payloadHash', status: IdempotencyRecordStatus.INPROGRESS, }); @@ -74,6 +75,7 @@ describe('Class IdempotencyHandler', () => { expiryTimestamp: Date.now() + 1000, // should be in the future inProgressExpiryTimestamp: new Date().getUTCMilliseconds() - 1000, // should be in the past responseData: { responseData: 'responseData' }, + payloadHash: 'payloadHash', status: IdempotencyRecordStatus.INPROGRESS, }); @@ -92,6 +94,7 @@ describe('Class IdempotencyHandler', () => { expiryTimestamp: new Date().getUTCMilliseconds() - 1000, // should be in the past inProgressExpiryTimestamp: 0, // less than current time in milliseconds responseData: { responseData: 'responseData' }, + payloadHash: 'payloadHash', status: IdempotencyRecordStatus.EXPIRED, }); @@ -106,43 +109,43 @@ describe('Class IdempotencyHandler', () => { describe('Method: handle', () => { test('when IdempotencyAlreadyInProgressError is thrown, it retries once', async () => { - // Arrange - const existingRecord = new IdempotencyRecord({ - idempotencyKey: 'idempotence-key', - status: 'COMPLETED', - expiryTimestamp: Date.now() / 1000 - 1, - responseData: { test: 'data' }, - }); + // Prepare + const saveInProgressSpy = jest + .spyOn(persistenceStore, 'saveInProgress') + .mockRejectedValueOnce(new IdempotencyItemAlreadyExistsError()); + + // Act & Assess + await expect(idempotentHandler.handle()).rejects.toThrow(); + expect(saveInProgressSpy).toHaveBeenCalledTimes(1); + }); + + test('when IdempotencyAlreadyInProgressError is thrown and it contains the existing item, it returns it directly', async () => { // Prepare const saveInProgressSpy = jest .spyOn(persistenceStore, 'saveInProgress') .mockRejectedValueOnce( new IdempotencyItemAlreadyExistsError( 'Failed to put record for already existing idempotency key: idempotence-key', - existingRecord + new IdempotencyRecord({ + idempotencyKey: 'key', + status: IdempotencyRecordStatus.COMPLETED, + responseData: 'Hi', + }) ) ); + const getRecordSpy = jest.spyOn(persistenceStore, 'getRecord'); // Act & Assess - await expect(idempotentHandler.handle()).rejects.toThrow(); + await expect(idempotentHandler.handle()).resolves.toEqual('Hi'); expect(saveInProgressSpy).toHaveBeenCalledTimes(1); + expect(getRecordSpy).toHaveBeenCalledTimes(0); }); test('when IdempotencyInconsistentStateError is thrown, it retries until max retries are exhausted', async () => { // Prepare const mockProcessIdempotency = jest .spyOn(persistenceStore, 'saveInProgress') - .mockRejectedValue( - new IdempotencyItemAlreadyExistsError( - 'Failed to put record for already existing idempotency key: my-lambda-function#mocked-hash', - new IdempotencyRecord({ - idempotencyKey: 'my-lambda-function#mocked-hash', - status: IdempotencyRecordStatus.EXPIRED, - payloadHash: 'different-hash', - expiryTimestamp: Date.now() / 1000 - 1, - }) - ) - ); + .mockRejectedValue(new IdempotencyItemAlreadyExistsError()); jest.spyOn(persistenceStore, 'getRecord').mockResolvedValue( new IdempotencyRecord({ status: IdempotencyRecordStatus.EXPIRED, diff --git a/packages/idempotency/tests/unit/idempotencyDecorator.test.ts b/packages/idempotency/tests/unit/idempotencyDecorator.test.ts index a3431168e3..1a293d8569 100644 --- a/packages/idempotency/tests/unit/idempotencyDecorator.test.ts +++ b/packages/idempotency/tests/unit/idempotencyDecorator.test.ts @@ -124,13 +124,7 @@ describe('Given a class with a function to decorate', (classWithLambdaHandler = let resultingError: Error; beforeEach(async () => { mockSaveInProgress.mockRejectedValue( - new IdempotencyItemAlreadyExistsError( - 'Failed to put record for already existing idempotency key: key', - new IdempotencyRecord({ - idempotencyKey: 'key', - status: IdempotencyRecordStatus.INPROGRESS, - }) - ) + new IdempotencyItemAlreadyExistsError() ); const idempotencyOptions: IdempotencyRecordOptions = { idempotencyKey: 'key', @@ -170,13 +164,7 @@ describe('Given a class with a function to decorate', (classWithLambdaHandler = let resultingError: Error; beforeEach(async () => { mockSaveInProgress.mockRejectedValue( - new IdempotencyItemAlreadyExistsError( - 'Failed to put record for already existing idempotency key: key', - new IdempotencyRecord({ - idempotencyKey: 'key', - status: IdempotencyRecordStatus.EXPIRED, - }) - ) + new IdempotencyItemAlreadyExistsError() ); const idempotencyOptions: IdempotencyRecordOptions = { idempotencyKey: 'key', @@ -215,14 +203,7 @@ describe('Given a class with a function to decorate', (classWithLambdaHandler = describe('When wrapping a function with previous execution that is COMPLETED', () => { beforeEach(async () => { mockSaveInProgress.mockRejectedValue( - new IdempotencyItemAlreadyExistsError( - 'Failed to put record for already existing idempotency key: key', - new IdempotencyRecord({ - idempotencyKey: 'key', - status: IdempotencyRecordStatus.COMPLETED, - responseData: 'Hi', - }) - ) + new IdempotencyItemAlreadyExistsError() ); const idempotencyOptions: IdempotencyRecordOptions = { idempotencyKey: 'key', diff --git a/packages/idempotency/tests/unit/makeHandlerIdempotent.test.ts b/packages/idempotency/tests/unit/makeHandlerIdempotent.test.ts index 90efa418e4..db064e6f38 100644 --- a/packages/idempotency/tests/unit/makeHandlerIdempotent.test.ts +++ b/packages/idempotency/tests/unit/makeHandlerIdempotent.test.ts @@ -159,19 +159,7 @@ describe('Middleware: makeHandlerIdempotent', () => { ).use(makeHandlerIdempotent(mockIdempotencyOptions)); jest .spyOn(mockIdempotencyOptions.persistenceStore, 'saveInProgress') - .mockRejectedValue( - new IdempotencyItemAlreadyExistsError( - 'Failed to put record for already existing idempotency key: idempotencyKey', - new IdempotencyRecord({ - idempotencyKey: 'idempotencyKey', - expiryTimestamp: Date.now() + 10000, - inProgressExpiryTimestamp: 0, - responseData: { response: false }, - payloadHash: 'payloadHash', - status: IdempotencyRecordStatus.COMPLETED, - }) - ) - ); + .mockRejectedValue(new IdempotencyItemAlreadyExistsError()); const stubRecord = new IdempotencyRecord({ idempotencyKey: 'idempotencyKey', expiryTimestamp: Date.now() + 10000, @@ -199,19 +187,7 @@ describe('Middleware: makeHandlerIdempotent', () => { ).use(makeHandlerIdempotent(mockIdempotencyOptions)); jest .spyOn(mockIdempotencyOptions.persistenceStore, 'saveInProgress') - .mockRejectedValue( - new IdempotencyItemAlreadyExistsError( - 'Failed to put record for already existing idempotency key: idempotencyKey', - new IdempotencyRecord({ - idempotencyKey: 'idempotencyKey', - expiryTimestamp: Date.now() + 10000, - inProgressExpiryTimestamp: 0, - responseData: { response: false }, - payloadHash: 'payloadHash', - status: IdempotencyRecordStatus.EXPIRED, - }) - ) - ); + .mockRejectedValue(new IdempotencyItemAlreadyExistsError()); const stubRecordInconsistent = new IdempotencyRecord({ idempotencyKey: 'idempotencyKey', expiryTimestamp: Date.now() + 10000, @@ -247,19 +223,7 @@ describe('Middleware: makeHandlerIdempotent', () => { ).use(makeHandlerIdempotent(mockIdempotencyOptions)); jest .spyOn(mockIdempotencyOptions.persistenceStore, 'saveInProgress') - .mockRejectedValue( - new IdempotencyItemAlreadyExistsError( - 'Failed to put record for already existing idempotency key: idempotencyKey', - new IdempotencyRecord({ - idempotencyKey: 'idempotencyKey', - expiryTimestamp: Date.now() + 10000, - inProgressExpiryTimestamp: 0, - responseData: { response: false }, - payloadHash: 'payloadHash', - status: IdempotencyRecordStatus.EXPIRED, - }) - ) - ); + .mockRejectedValue(new IdempotencyItemAlreadyExistsError()); const stubRecordInconsistent = new IdempotencyRecord({ idempotencyKey: 'idempotencyKey', expiryTimestamp: Date.now() + 10000, diff --git a/packages/idempotency/tests/unit/makeIdempotent.test.ts b/packages/idempotency/tests/unit/makeIdempotent.test.ts index 2d5a95b4ab..d46bc7ad6b 100644 --- a/packages/idempotency/tests/unit/makeIdempotent.test.ts +++ b/packages/idempotency/tests/unit/makeIdempotent.test.ts @@ -177,19 +177,7 @@ describe('Function: makeIdempotent', () => { ); jest .spyOn(mockIdempotencyOptions.persistenceStore, 'saveInProgress') - .mockRejectedValue( - new IdempotencyItemAlreadyExistsError( - 'Failed to put record for already existing idempotency key: idempotencyKey', - new IdempotencyRecord({ - idempotencyKey: 'idempotencyKey', - expiryTimestamp: Date.now() + 10000, - inProgressExpiryTimestamp: 0, - responseData: { response: false }, - payloadHash: 'payloadHash', - status: IdempotencyRecordStatus.COMPLETED, - }) - ) - ); + .mockRejectedValue(new IdempotencyItemAlreadyExistsError()); const stubRecord = new IdempotencyRecord({ idempotencyKey: 'idempotencyKey', expiryTimestamp: Date.now() + 10000, @@ -221,19 +209,7 @@ describe('Function: makeIdempotent', () => { ); jest .spyOn(mockIdempotencyOptions.persistenceStore, 'saveInProgress') - .mockRejectedValue( - new IdempotencyItemAlreadyExistsError( - 'Failed to put record for already existing idempotency key: idempotencyKey', - new IdempotencyRecord({ - idempotencyKey: 'idempotencyKey', - expiryTimestamp: Date.now() + 10000, - inProgressExpiryTimestamp: 0, - responseData: { response: false }, - payloadHash: 'payloadHash', - status: IdempotencyRecordStatus.COMPLETED, - }) - ) - ); + .mockRejectedValue(new IdempotencyItemAlreadyExistsError()); const stubRecordInconsistent = new IdempotencyRecord({ idempotencyKey: 'idempotencyKey', expiryTimestamp: Date.now() + 10000, @@ -273,19 +249,7 @@ describe('Function: makeIdempotent', () => { ); jest .spyOn(mockIdempotencyOptions.persistenceStore, 'saveInProgress') - .mockRejectedValue( - new IdempotencyItemAlreadyExistsError( - 'Failed to put record for already existing idempotency key: idempotencyKey', - new IdempotencyRecord({ - idempotencyKey: 'idempotencyKey', - expiryTimestamp: Date.now() + 10000, - inProgressExpiryTimestamp: 0, - responseData: { response: false }, - payloadHash: 'payloadHash', - status: IdempotencyRecordStatus.EXPIRED, - }) - ) - ); + .mockRejectedValue(new IdempotencyItemAlreadyExistsError()); const stubRecordInconsistent = new IdempotencyRecord({ idempotencyKey: 'idempotencyKey', expiryTimestamp: Date.now() + 10000, diff --git a/packages/idempotency/tests/unit/persistence/BasePersistenceLayer.test.ts b/packages/idempotency/tests/unit/persistence/BasePersistenceLayer.test.ts index b26423a0d9..b615434c9f 100644 --- a/packages/idempotency/tests/unit/persistence/BasePersistenceLayer.test.ts +++ b/packages/idempotency/tests/unit/persistence/BasePersistenceLayer.test.ts @@ -407,17 +407,7 @@ describe('Class: BasePersistenceLayer', () => { // Act & Assess await expect( persistenceLayer.saveInProgress({ foo: 'bar' }) - ).rejects.toThrow( - new IdempotencyItemAlreadyExistsError( - 'Failed to put record for already existing idempotency key: my-lambda-function#mocked-hash', - new IdempotencyRecord({ - idempotencyKey: 'my-lambda-function#mocked-hash', - status: IdempotencyRecordStatus.EXPIRED, - payloadHash: 'different-hash', - expiryTimestamp: Date.now() / 1000 - 1, - }) - ) - ); + ).rejects.toThrow(IdempotencyItemAlreadyExistsError); expect(putRecordSpy).toHaveBeenCalledTimes(0); }); From 1c35d85ef2d5448f45519449b5a325150b5553bc Mon Sep 17 00:00:00 2001 From: Alexander Schueren Date: Thu, 4 Jan 2024 07:47:11 +0100 Subject: [PATCH 12/12] add info about ReturnValuesOnConditionCheckFailure and conditional expression --- docs/utilities/idempotency.md | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/docs/utilities/idempotency.md b/docs/utilities/idempotency.md index 65852c06ca..d45607db69 100644 --- a/docs/utilities/idempotency.md +++ b/docs/utilities/idempotency.md @@ -109,10 +109,11 @@ If you're not [changing the default configuration for the DynamoDB persistence l Larger items cannot be written to DynamoDB and will cause exceptions. ???+ info "Info: DynamoDB" - Each function invocation will generally make 2 requests to DynamoDB. If the - result returned by your Lambda is less than 1kb, you can expect 2 WCUs per invocation. For retried invocations, you will - see 1WCU and 1RCU. Review the [DynamoDB pricing documentation](https://aws.amazon.com/dynamodb/pricing/){target="_blank"} to - estimate the cost. + Each function invocation will make only 1 request to DynamoDB by using DynamoDB's [conditional expressions](https://docs.aws.amazon.com/amazondynamodb/latest/developerguide/Expressions.ConditionExpressions.html){target="_blank"} to ensure that we don't overwrite existing records, + and [ReturnValuesOnConditionCheckFailure](https://docs.aws.amazon.com/amazondynamodb/latest/APIReference/API_PutItem.html#DDB-PutItem-request-ReturnValuesOnConditionCheckFailure){target="_blank"} to return the record if it exists. + See [AWS Blog post on handling conditional write errors](https://aws.amazon.com/blogs/database/handle-conditional-write-errors-in-high-concurrency-scenarios-with-amazon-dynamodb/) for more details. + For retried invocations, you will see 1WCU and 1RCU. + Review the [DynamoDB pricing documentation](https://aws.amazon.com/dynamodb/pricing/){target="_blank"} to estimate the cost. ### MakeIdempotent function wrapper