From 1e925377035e327dc83f3049085b2909ab9a5f14 Mon Sep 17 00:00:00 2001 From: Andrea Amorosi Date: Mon, 3 Jul 2023 19:27:58 +0200 Subject: [PATCH 1/4] fix: misc fixes from feedback --- packages/idempotency/README.md | 16 ++++++-------- .../persistence/DynamoDBPersistenceLayer.ts | 21 +++++++++++++++++-- .../src/types/DynamoDBPersistence.ts | 2 +- 3 files changed, 26 insertions(+), 13 deletions(-) diff --git a/packages/idempotency/README.md b/packages/idempotency/README.md index 63833d664d..5fb4acd204 100644 --- a/packages/idempotency/README.md +++ b/packages/idempotency/README.md @@ -1,8 +1,8 @@ # Powertools for AWS Lambda (TypeScript) - Idempotency Utility -| ⚠️ **WARNING: Do not use this utility in production just yet!** ⚠️ | -| :------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | +| ⚠️ **WARNING: Do not use this utility in production just yet!** ⚠️ | +| :--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | **This utility is currently released as beta developer preview** and is intended strictly for feedback and testing purposes **and not for production workloads**.. The version and all future versions tagged with the `-beta` suffix should be treated as not stable. Up until before the [General Availability release](https://github.com/aws-powertools/powertools-lambda-typescript/milestone/10) we might introduce significant breaking changes and improvements in response to customers feedback. | _ | @@ -29,7 +29,7 @@ You can use the package in both TypeScript and JavaScript code bases. ## Intro This package provides a utility to implement idempotency in your Lambda functions. -You can either use it to wrapp a function, or as Middy middleware to make your AWS Lambda handler idempotent. +You can either use it to wrap a function, or as Middy middleware to make your AWS Lambda handler idempotent. The current implementation provides a persistence layer for Amazon DynamoDB, which offers a variety of configuration options. You can also bring your own persistence layer by extending the `BasePersistenceLayer` class. @@ -59,7 +59,7 @@ The function wrapper takes a reference to the function to be made idempotent as ```ts import { makeFunctionIdempotent } from '@aws-lambda-powertools/idempotency'; -import { DynamoDBPersistenceLayer } from '@aws-lambda-powertools/idempotency/persistence'; +import { DynamoDBPersistenceLayer } from '@aws-lambda-powertools/idempotency/dynamodb'; import type { Context, SQSEvent, SQSRecord } from 'aws-lambda'; const persistenceStore = new DynamoDBPersistenceLayer({ @@ -75,7 +75,7 @@ export const handler = async ( _context: Context ): Promise => { for (const record of event.Records) { - await makeFunctionIdempotent(proccessingFunction, { + await makeFunctionIdempotent(processingFunction, { dataKeywordArgument: 'transactionId', persistenceStore, }); @@ -96,7 +96,7 @@ By default, the Idempotency utility will use the full event payload to create an ```ts import { IdempotencyConfig } from '@aws-lambda-powertools/idempotency'; import { makeHandlerIdempotent } from '@aws-lambda-powertools/idempotency/middleware'; -import { DynamoDBPersistenceLayer } from '@aws-lambda-powertools/idempotency/persistence'; +import { DynamoDBPersistenceLayer } from '@aws-lambda-powertools/idempotency/dynamodb'; import middy from '@middy/core'; import type { Context, APIGatewayProxyEvent } from 'aws-lambda'; @@ -111,10 +111,6 @@ const config = new IdempotencyConfig({ eventKeyJmesPath: 'headers.idempotency-key', }); -const processingFunction = async (payload: SQSRecord): Promise => { - // your code goes here here -}; - export const handler = middy( async (event: APIGatewayProxyEvent, _context: Context): Promise => { // your code goes here here diff --git a/packages/idempotency/src/persistence/DynamoDBPersistenceLayer.ts b/packages/idempotency/src/persistence/DynamoDBPersistenceLayer.ts index 57b76bb596..6d5705fa25 100644 --- a/packages/idempotency/src/persistence/DynamoDBPersistenceLayer.ts +++ b/packages/idempotency/src/persistence/DynamoDBPersistenceLayer.ts @@ -19,10 +19,27 @@ import { IdempotencyRecord } from './IdempotencyRecord'; import { BasePersistenceLayer } from './BasePersistenceLayer'; /** - * DynamoDB persistence layer for idempotency records. This class will use the AWS SDK V3 to write and read idempotency records from DynamoDB. + * DynamoDB persistence layer for idempotency records. + * + * This class uses the AWS SDK for JavaScript v3 to write and read idempotency records from DynamoDB. + * * There are various options to configure the persistence layer, such as the table name, the key attribute, the status attribute, etc. - * With default configuration you don't need to create the table beforehand, the persistence layer will create it for you. + * + * With default configuration you don't need to create the client beforehand, the persistence layer will create it for you. * You can also bring your own AWS SDK V3 client, or configure the client with the `clientConfig` option. + * + * See the {@link https://docs.powertools.aws.dev/lambda/python/latest/utilities/idempotency/ Idempotency documentation} for more details + * on the IAM permissions and DynamoDB table configuration. + * + * @example + * ```ts + * import { DynamoDBPersistenceLayer } from '@aws-lambda-powertools/idempotency/dynamodb'; + * + * const persistence = new DynamoDBPersistenceLayer({ + * tableName: 'my-idempotency-table', + * }); + * ``` + * * @see https://docs.aws.amazon.com/AWSJavaScriptSDK/v3/latest/clients/client-dynamodb/index.html * @category Persistence Layer * @implements {BasePersistenceLayer} diff --git a/packages/idempotency/src/types/DynamoDBPersistence.ts b/packages/idempotency/src/types/DynamoDBPersistence.ts index 7a1284d692..a3a753b9da 100644 --- a/packages/idempotency/src/types/DynamoDBPersistence.ts +++ b/packages/idempotency/src/types/DynamoDBPersistence.ts @@ -48,7 +48,7 @@ interface DynamoDBPersistenceOptionsWithClientConfig * * @interface * @extends DynamoDBPersistenceOptionsBase - * @property {DynamoDBClient} [awsSdkV3Client] - Optional AWS SDK v3 client to pass during AppConfigProvider class instantiation + * @property {DynamoDBClient} [awsSdkV3Client] - Optional AWS SDK v3 client to pass during DynamoDB client instantiation * @property {never} [clientConfig] - This property should never be passed. */ interface DynamoDBPersistenceOptionsWithClientInstance From 770cec2b174b4e2972cfb23861af2b420f3e6da2 Mon Sep 17 00:00:00 2001 From: Andrea Amorosi Date: Tue, 4 Jul 2023 18:58:45 +0200 Subject: [PATCH 2/4] chore: add makeIdempotent wrapper to jest config (it was excluded) --- packages/idempotency/jest.config.js | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/packages/idempotency/jest.config.js b/packages/idempotency/jest.config.js index f0890d6314..7a512f0d83 100644 --- a/packages/idempotency/jest.config.js +++ b/packages/idempotency/jest.config.js @@ -14,11 +14,7 @@ module.exports = { roots: ['/src', '/tests'], testPathIgnorePatterns: ['/node_modules/'], testEnvironment: 'node', - coveragePathIgnorePatterns: [ - '/node_modules/', - '/types/', - 'src/makeFunctionIdempotent.ts', // TODO: remove this once makeFunctionIdempotent is implemented - ], + coveragePathIgnorePatterns: ['/node_modules/', '/types/'], coverageThreshold: { global: { statements: 100, From e73137264eb6cc9ab3a79e9b2958347936a405d2 Mon Sep 17 00:00:00 2001 From: Andrea Amorosi Date: Tue, 4 Jul 2023 19:00:45 +0200 Subject: [PATCH 3/4] chore: made error cause mandatory --- packages/idempotency/src/errors.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/packages/idempotency/src/errors.ts b/packages/idempotency/src/errors.ts index 21812ece6a..da96692bc8 100644 --- a/packages/idempotency/src/errors.ts +++ b/packages/idempotency/src/errors.ts @@ -34,10 +34,8 @@ class IdempotencyInconsistentStateError extends Error {} class IdempotencyPersistenceLayerError extends Error { public readonly cause: Error | undefined; - public constructor(message: string, cause?: Error) { - const errorMessage = cause - ? `${message}. This error was caused by: ${cause.message}.` - : message; + public constructor(message: string, cause: Error) { + const errorMessage = `${message}. This error was caused by: ${cause.message}.`; super(errorMessage); this.cause = cause; } From 02740b81a3338c8cf8e67e17b69dfa10f072d28a Mon Sep 17 00:00:00 2001 From: Andrea Amorosi Date: Tue, 4 Jul 2023 19:27:50 +0200 Subject: [PATCH 4/4] chore: renamed wrapper function, fixed arguments handling, fixed types --- .../idempotency/src/IdempotencyHandler.ts | 103 +++-- packages/idempotency/src/index.ts | 2 +- .../idempotency/src/makeFunctionIdempotent.ts | 87 ---- packages/idempotency/src/makeIdempotent.ts | 108 +++++ .../src/middleware/makeHandlerIdempotent.ts | 17 +- .../src/persistence/BasePersistenceLayer.ts | 27 +- .../src/persistence/IdempotencyRecord.ts | 5 +- packages/idempotency/src/types/AnyFunction.ts | 15 - .../src/types/IdempotencyOptions.ts | 142 ++++++- .../src/types/IdempotencyRecord.ts | 4 +- packages/idempotency/src/types/index.ts | 1 - .../tests/unit/IdempotencyHandler.test.ts | 37 +- .../tests/unit/makeFunctionIdempotent.test.ts | 220 ---------- .../tests/unit/makeHandlerIdempotent.test.ts | 22 +- .../tests/unit/makeIdempotent.test.ts | 401 ++++++++++++++++++ 15 files changed, 763 insertions(+), 428 deletions(-) delete mode 100644 packages/idempotency/src/makeFunctionIdempotent.ts create mode 100644 packages/idempotency/src/makeIdempotent.ts delete mode 100644 packages/idempotency/src/types/AnyFunction.ts delete mode 100644 packages/idempotency/tests/unit/makeFunctionIdempotent.test.ts create mode 100644 packages/idempotency/tests/unit/makeIdempotent.test.ts diff --git a/packages/idempotency/src/IdempotencyHandler.ts b/packages/idempotency/src/IdempotencyHandler.ts index 1caeeef6dc..06387ed0c8 100644 --- a/packages/idempotency/src/IdempotencyHandler.ts +++ b/packages/idempotency/src/IdempotencyHandler.ts @@ -1,4 +1,5 @@ -import type { AnyFunctionWithRecord, IdempotencyHandlerOptions } from './types'; +import type { JSONValue } from '@aws-lambda-powertools/commons'; +import type { AnyFunction, IdempotencyHandlerOptions } from './types'; import { IdempotencyRecordStatus } from './types'; import { IdempotencyAlreadyInProgressError, @@ -13,31 +14,57 @@ import { search } from 'jmespath'; /** * @internal + * + * Class that handles the idempotency lifecycle. + * + * This class is used under the hood by the Idempotency utility + * and provides several methods that are called at different stages + * to orchestrate the idempotency logic. */ -export class IdempotencyHandler { - private readonly fullFunctionPayload: Record; - private readonly functionPayloadToBeHashed: Record; - private readonly functionToMakeIdempotent: AnyFunctionWithRecord; - private readonly idempotencyConfig: IdempotencyConfig; - private readonly persistenceStore: BasePersistenceLayer; +export class IdempotencyHandler { + /** + * The arguments passed to the function. + * + * For example, if the function is `foo(a, b)`, then `functionArguments` will be `[a, b]`. + * We need to keep track of the arguments so that we can pass them to the function when we call it. + */ + readonly #functionArguments: unknown[]; + /** + * The payload to be hashed. + * + * This is the argument that is used for the idempotency. + */ + readonly #functionPayloadToBeHashed: JSONValue; + /** + * Reference to the function to be made idempotent. + */ + readonly #functionToMakeIdempotent: AnyFunction; + /** + * Idempotency configuration options. + */ + readonly #idempotencyConfig: IdempotencyConfig; + /** + * Persistence layer used to store the idempotency records. + */ + readonly #persistenceStore: BasePersistenceLayer; - public constructor(options: IdempotencyHandlerOptions) { + public constructor(options: IdempotencyHandlerOptions) { const { functionToMakeIdempotent, functionPayloadToBeHashed, idempotencyConfig, - fullFunctionPayload, + functionArguments, persistenceStore, } = options; - this.functionToMakeIdempotent = functionToMakeIdempotent; - this.functionPayloadToBeHashed = functionPayloadToBeHashed; - this.idempotencyConfig = idempotencyConfig; - this.fullFunctionPayload = fullFunctionPayload; + this.#functionToMakeIdempotent = functionToMakeIdempotent; + this.#functionPayloadToBeHashed = functionPayloadToBeHashed; + this.#idempotencyConfig = idempotencyConfig; + this.#functionArguments = functionArguments; - this.persistenceStore = persistenceStore; + this.#persistenceStore = persistenceStore; - this.persistenceStore.configure({ - config: this.idempotencyConfig, + this.#persistenceStore.configure({ + config: this.#idempotencyConfig, }); } @@ -69,14 +96,14 @@ export class IdempotencyHandler { return idempotencyRecord.getResponse(); } - public async getFunctionResult(): Promise { - let result: U; + public async getFunctionResult(): Promise> { + let result; try { - result = await this.functionToMakeIdempotent(this.fullFunctionPayload); + result = await this.#functionToMakeIdempotent(...this.#functionArguments); } catch (e) { try { - await this.persistenceStore.deleteRecord( - this.functionPayloadToBeHashed + await this.#persistenceStore.deleteRecord( + this.#functionPayloadToBeHashed ); } catch (e) { throw new IdempotencyPersistenceLayerError( @@ -87,9 +114,9 @@ export class IdempotencyHandler { throw e; } try { - await this.persistenceStore.saveSuccess( - this.functionPayloadToBeHashed, - result as Record + await this.#persistenceStore.saveSuccess( + this.#functionPayloadToBeHashed, + result ); } catch (e) { throw new IdempotencyPersistenceLayerError( @@ -108,7 +135,7 @@ export class IdempotencyHandler { * window, we might get an `IdempotencyInconsistentStateError`. In such * cases we can safely retry the handling a few times. */ - public async handle(): Promise { + public async handle(): Promise> { let e; for (let retryNo = 0; retryNo <= MAX_RETRIES; retryNo++) { try { @@ -129,34 +156,36 @@ export class IdempotencyHandler { throw e; } - public async processIdempotency(): Promise { + public async processIdempotency(): Promise> { // early return if we should skip idempotency completely if ( IdempotencyHandler.shouldSkipIdempotency( - this.idempotencyConfig.eventKeyJmesPath, - this.idempotencyConfig.throwOnNoIdempotencyKey, - this.fullFunctionPayload + this.#idempotencyConfig.eventKeyJmesPath, + this.#idempotencyConfig.throwOnNoIdempotencyKey, + this.#functionPayloadToBeHashed ) ) { - return await this.functionToMakeIdempotent(this.fullFunctionPayload); + return await this.#functionToMakeIdempotent(...this.#functionArguments); } try { - await this.persistenceStore.saveInProgress( - this.functionPayloadToBeHashed, - this.idempotencyConfig.lambdaContext?.getRemainingTimeInMillis() + await this.#persistenceStore.saveInProgress( + this.#functionPayloadToBeHashed, + this.#idempotencyConfig.lambdaContext?.getRemainingTimeInMillis() ); } catch (e) { if (e instanceof IdempotencyItemAlreadyExistsError) { const idempotencyRecord: IdempotencyRecord = - await this.persistenceStore.getRecord(this.functionPayloadToBeHashed); + await this.#persistenceStore.getRecord( + this.#functionPayloadToBeHashed + ); return IdempotencyHandler.determineResultFromIdempotencyRecord( idempotencyRecord - ) as U; + ) as ReturnType; } else { throw new IdempotencyPersistenceLayerError( - 'Failed to save record in progress', + 'Failed to save in progress record to idempotency store', e as Error ); } @@ -177,7 +206,7 @@ export class IdempotencyHandler { public static shouldSkipIdempotency( eventKeyJmesPath: string, throwOnNoIdempotencyKey: boolean, - fullFunctionPayload: Record + fullFunctionPayload: JSONValue ): boolean { return (eventKeyJmesPath && !throwOnNoIdempotencyKey && diff --git a/packages/idempotency/src/index.ts b/packages/idempotency/src/index.ts index bc5289a9ed..d7c77cc459 100644 --- a/packages/idempotency/src/index.ts +++ b/packages/idempotency/src/index.ts @@ -1,3 +1,3 @@ export * from './errors'; export * from './IdempotencyConfig'; -export * from './makeFunctionIdempotent'; +export * from './makeIdempotent'; diff --git a/packages/idempotency/src/makeFunctionIdempotent.ts b/packages/idempotency/src/makeFunctionIdempotent.ts deleted file mode 100644 index 9c11733274..0000000000 --- a/packages/idempotency/src/makeFunctionIdempotent.ts +++ /dev/null @@ -1,87 +0,0 @@ -import type { Context } from 'aws-lambda'; -import type { - AnyFunctionWithRecord, - AnyIdempotentFunction, - IdempotencyFunctionOptions, -} from './types'; -import { IdempotencyHandler } from './IdempotencyHandler'; -import { IdempotencyConfig } from './IdempotencyConfig'; - -const isContext = (arg: unknown): arg is Context => { - return ( - arg !== undefined && - arg !== null && - typeof arg === 'object' && - 'getRemainingTimeInMillis' in arg - ); -}; - -/** - * Use function wrapper to make your function idempotent. - * @example - * ```ts - * // this is your processing function with an example record { transactionId: '123', foo: 'bar' } - * const processRecord = (record: Record): any => { - * // you custom processing logic - * return result; - * }; - * - * // we use wrapper to make processing function idempotent with DynamoDBPersistenceLayer - * const processIdempotently = makeFunctionIdempotent(processRecord, { - * persistenceStore: new DynamoDBPersistenceLayer() - * dataKeywordArgument: 'transactionId', // keyword argument to hash the payload and the result - * }); - * - * export const handler = async ( - * _event: EventRecords, - * _context: Context - * ): Promise => { - * for (const record of _event.records) { - * const result = await processIdempotently(record); - * // do something with the result - * } - * - * return Promise.resolve(); - * }; - * - * ``` - */ -const makeFunctionIdempotent = function ( - fn: AnyFunctionWithRecord, - options: IdempotencyFunctionOptions -): AnyIdempotentFunction | AnyFunctionWithRecord { - const idempotencyConfig = options.config - ? options.config - : new IdempotencyConfig({}); - - const wrappedFn: AnyIdempotentFunction = function ( - ...args: Parameters> - ): Promise { - const payload = args[0]; - const context = args[1]; - - if (options.dataKeywordArgument === undefined) { - throw new Error( - `Missing data keyword argument ${options.dataKeywordArgument}` - ); - } - if (isContext(context)) { - idempotencyConfig.registerLambdaContext(context); - } - const idempotencyHandler: IdempotencyHandler = new IdempotencyHandler( - { - functionToMakeIdempotent: fn, - functionPayloadToBeHashed: payload[options.dataKeywordArgument], - idempotencyConfig: idempotencyConfig, - persistenceStore: options.persistenceStore, - fullFunctionPayload: payload, - } - ); - - return idempotencyHandler.handle(); - }; - if (idempotencyConfig.isEnabled()) return wrappedFn; - else return fn; -}; - -export { makeFunctionIdempotent }; diff --git a/packages/idempotency/src/makeIdempotent.ts b/packages/idempotency/src/makeIdempotent.ts new file mode 100644 index 0000000000..a44079d493 --- /dev/null +++ b/packages/idempotency/src/makeIdempotent.ts @@ -0,0 +1,108 @@ +import type { Context, Handler } from 'aws-lambda'; +import type { + AnyFunction, + ItempotentFunctionOptions, + IdempotencyLambdaHandlerOptions, +} from './types'; +import { IdempotencyHandler } from './IdempotencyHandler'; +import { IdempotencyConfig } from './IdempotencyConfig'; + +const isContext = (arg: unknown): arg is Context => { + return ( + arg !== undefined && + arg !== null && + typeof arg === 'object' && + 'getRemainingTimeInMillis' in arg + ); +}; + +const isFnHandler = ( + fn: AnyFunction, + args: Parameters +): fn is Handler => { + // get arguments of function + return ( + fn !== undefined && + fn !== null && + typeof fn === 'function' && + isContext(args[1]) + ); +}; + +const isOptionsWithDataIndexArgument = ( + options: unknown +): options is IdempotencyLambdaHandlerOptions & { + dataIndexArgument: number; +} => { + return ( + options !== undefined && + options !== null && + typeof options === 'object' && + 'dataIndexArgument' in options + ); +}; + +/** + * Use function wrapper to make your function idempotent. + * @example + * ```ts + * // this is your processing function with an example record { transactionId: '123', foo: 'bar' } + * const processRecord = (record: Record): any => { + * // you custom processing logic + * return result; + * }; + * + * // we use wrapper to make processing function idempotent with DynamoDBPersistenceLayer + * const processIdempotently = makeFunctionIdempotent(processRecord, { + * persistenceStore: new DynamoDBPersistenceLayer() + * dataKeywordArgument: 'transactionId', // keyword argument to hash the payload and the result + * }); + * + * export const handler = async ( + * _event: EventRecords, + * _context: Context + * ): Promise => { + * for (const record of _event.records) { + * const result = await processIdempotently(record); + * // do something with the result + * } + * + * return Promise.resolve(); + * }; + * + * ``` + */ +const makeIdempotent = ( + fn: Func, + options: ItempotentFunctionOptions> +): ((...args: Parameters) => ReturnType) => { + const { persistenceStore, config } = options; + const idempotencyConfig = config ? config : new IdempotencyConfig({}); + + if (!idempotencyConfig.isEnabled()) return fn; + + return (...args: Parameters): ReturnType => { + let functionPayloadToBeHashed; + + if (isFnHandler(fn, args)) { + idempotencyConfig.registerLambdaContext(args[1]); + functionPayloadToBeHashed = args[0]; + } else { + if (isOptionsWithDataIndexArgument(options)) { + functionPayloadToBeHashed = args[options.dataIndexArgument]; + } else { + functionPayloadToBeHashed = args[0]; + } + } + + return new IdempotencyHandler({ + functionToMakeIdempotent: fn, + idempotencyConfig: idempotencyConfig, + persistenceStore: persistenceStore, + functionArguments: args, + functionPayloadToBeHashed, + }).handle() as ReturnType; + }; +}; + +export { makeIdempotent }; diff --git a/packages/idempotency/src/middleware/makeHandlerIdempotent.ts b/packages/idempotency/src/middleware/makeHandlerIdempotent.ts index b0f574cb20..f0edd0c692 100644 --- a/packages/idempotency/src/middleware/makeHandlerIdempotent.ts +++ b/packages/idempotency/src/middleware/makeHandlerIdempotent.ts @@ -11,6 +11,7 @@ import { MAX_RETRIES } from '../constants'; import type { MiddlewareLikeObj, MiddyLikeRequest, + JSONValue, } from '@aws-lambda-powertools/commons'; import type { IdempotencyLambdaHandlerOptions } from '../types'; @@ -79,7 +80,7 @@ const makeHandlerIdempotent = ( IdempotencyHandler.shouldSkipIdempotency( idempotencyConfig.eventKeyJmesPath, idempotencyConfig.throwOnNoIdempotencyKey, - request.event as Record + request.event as JSONValue ) ) { // set the flag to skip checks in after and onError @@ -89,15 +90,13 @@ const makeHandlerIdempotent = ( } try { await persistenceStore.saveInProgress( - request.event as Record, + request.event as JSONValue, request.context.getRemainingTimeInMillis() ); } catch (error) { if (error instanceof IdempotencyItemAlreadyExistsError) { const idempotencyRecord: IdempotencyRecord = - await persistenceStore.getRecord( - request.event as Record - ); + await persistenceStore.getRecord(request.event as JSONValue); try { const response = @@ -145,8 +144,8 @@ const makeHandlerIdempotent = ( } try { await persistenceStore.saveSuccess( - request.event as Record, - request.response as Record + request.event as JSONValue, + request.response as JSONValue ); } catch (e) { throw new IdempotencyPersistenceLayerError( @@ -169,9 +168,7 @@ const makeHandlerIdempotent = ( return; } try { - await persistenceStore.deleteRecord( - request.event as Record - ); + await persistenceStore.deleteRecord(request.event as JSONValue); } catch (error) { throw new IdempotencyPersistenceLayerError( 'Failed to delete record from idempotency store', diff --git a/packages/idempotency/src/persistence/BasePersistenceLayer.ts b/packages/idempotency/src/persistence/BasePersistenceLayer.ts index 94b235c004..153f2b2471 100644 --- a/packages/idempotency/src/persistence/BasePersistenceLayer.ts +++ b/packages/idempotency/src/persistence/BasePersistenceLayer.ts @@ -10,6 +10,7 @@ import { IdempotencyValidationError, } from '../errors'; import { LRUCache } from './LRUCache'; +import type { JSONValue } from '@aws-lambda-powertools/commons'; /** * Base class for all persistence layers. This class provides the basic functionality for @@ -79,7 +80,7 @@ abstract class BasePersistenceLayer implements BasePersistenceLayerInterface { * * @param data - the data payload that will be hashed to create the hash portion of the idempotency key */ - public async deleteRecord(data: Record): Promise { + public async deleteRecord(data: JSONValue): Promise { const idempotencyRecord = new IdempotencyRecord({ idempotencyKey: this.getHashedIdempotencyKey(data), status: IdempotencyRecordStatus.EXPIRED, @@ -95,9 +96,7 @@ abstract class BasePersistenceLayer implements BasePersistenceLayerInterface { * * @param data - the data payload that will be hashed to create the hash portion of the idempotency key */ - public async getRecord( - data: Record - ): Promise { + public async getRecord(data: JSONValue): Promise { const idempotencyKey = this.getHashedIdempotencyKey(data); const cachedRecord = this.getFromCache(idempotencyKey); @@ -125,7 +124,7 @@ abstract class BasePersistenceLayer implements BasePersistenceLayerInterface { * @param remainingTimeInMillis - the remaining time left in the lambda execution context */ public async saveInProgress( - data: Record, + data: JSONValue, remainingTimeInMillis?: number ): Promise { const idempotencyRecord = new IdempotencyRecord({ @@ -158,10 +157,7 @@ abstract class BasePersistenceLayer implements BasePersistenceLayerInterface { * @param data - the data payload that will be hashed to create the hash portion of the idempotency key * @param result - the result of the successfully completed function */ - public async saveSuccess( - data: Record, - result: Record - ): Promise { + public async saveSuccess(data: JSONValue, result: JSONValue): Promise { const idempotencyRecord = new IdempotencyRecord({ idempotencyKey: this.getHashedIdempotencyKey(data), status: IdempotencyRecordStatus.COMPLETED, @@ -242,7 +238,7 @@ abstract class BasePersistenceLayer implements BasePersistenceLayerInterface { * @param data the data payload that will be hashed to create the hash portion of the idempotency key * @returns the idempotency key */ - private getHashedIdempotencyKey(data: Record): string { + private getHashedIdempotencyKey(data: JSONValue): string { if (this.eventKeyJmesPath) { data = search(data, this.eventKeyJmesPath); } @@ -266,7 +262,7 @@ abstract class BasePersistenceLayer implements BasePersistenceLayerInterface { * * @param data payload */ - private getHashedPayload(data: Record): string { + private getHashedPayload(data: JSONValue): string { if (this.isPayloadValidationEnabled() && this.validationKeyJmesPath) { data = search(data, this.validationKeyJmesPath); @@ -276,9 +272,7 @@ abstract class BasePersistenceLayer implements BasePersistenceLayerInterface { } } - private static isMissingIdempotencyKey( - data: Record - ): boolean { + private static isMissingIdempotencyKey(data: JSONValue): boolean { if (Array.isArray(data) || typeof data === 'object') { if (data === null) return true; for (const value of Object.values(data)) { @@ -307,10 +301,7 @@ abstract class BasePersistenceLayer implements BasePersistenceLayerInterface { this.cache?.add(record.idempotencyKey, record); } - private validatePayload( - data: Record, - record: IdempotencyRecord - ): void { + private validatePayload(data: JSONValue, record: IdempotencyRecord): void { if (this.payloadValidationEnabled) { const hashedPayload: string = this.getHashedPayload(data); if (hashedPayload !== record.payloadHash) { diff --git a/packages/idempotency/src/persistence/IdempotencyRecord.ts b/packages/idempotency/src/persistence/IdempotencyRecord.ts index 98ec57da21..de1d2ddac0 100644 --- a/packages/idempotency/src/persistence/IdempotencyRecord.ts +++ b/packages/idempotency/src/persistence/IdempotencyRecord.ts @@ -1,3 +1,4 @@ +import type { JSONValue } from '@aws-lambda-powertools/commons'; import type { IdempotencyRecordOptions } from '../types'; import { IdempotencyRecordStatus } from '../types'; import { IdempotencyInvalidStatusError } from '../errors'; @@ -26,7 +27,7 @@ class IdempotencyRecord { /** * The response data of the request, this will be returned if the payload hash matches. */ - public responseData?: Record; + public responseData?: JSONValue; /** * The idempotency record status can be COMPLETED, IN_PROGRESS or EXPIRED. * We check the status during idempotency processing to make sure we don't process an expired record and handle concurrent requests. @@ -47,7 +48,7 @@ class IdempotencyRecord { /** * Get the response data of the record. */ - public getResponse(): Record | undefined { + public getResponse(): JSONValue { return this.responseData; } diff --git a/packages/idempotency/src/types/AnyFunction.ts b/packages/idempotency/src/types/AnyFunction.ts deleted file mode 100644 index 097deab39a..0000000000 --- a/packages/idempotency/src/types/AnyFunction.ts +++ /dev/null @@ -1,15 +0,0 @@ -// TODO: Find a better way to type this -// eslint-disable-next-line @typescript-eslint/no-explicit-any -type GenericTempRecord = Record; - -type AnyFunctionWithRecord = ( - payload: GenericTempRecord, - ...args: unknown[] -) => Promise | U; - -type AnyIdempotentFunction = ( - payload: GenericTempRecord, - ...args: unknown[] -) => Promise; - -export { GenericTempRecord, AnyFunctionWithRecord, AnyIdempotentFunction }; diff --git a/packages/idempotency/src/types/IdempotencyOptions.ts b/packages/idempotency/src/types/IdempotencyOptions.ts index 830659eaa6..959d014321 100644 --- a/packages/idempotency/src/types/IdempotencyOptions.ts +++ b/packages/idempotency/src/types/IdempotencyOptions.ts @@ -1,23 +1,144 @@ import type { Context } from 'aws-lambda'; import { BasePersistenceLayer } from '../persistence/BasePersistenceLayer'; -import { AnyFunctionWithRecord } from 'types/AnyFunction'; import { IdempotencyConfig } from '../IdempotencyConfig'; +import type { JSONValue } from '@aws-lambda-powertools/commons'; +/** + * Configuration options for the idempotency utility. + * + * When making a function idempotent you should always set + * a persistence store (i.e. {@link DynamoDBPersistenceLayer}). + * + * Optionally, you can also pass a custom configuration object, + * this allows you to customize the behavior of the idempotency utility. + * + */ type IdempotencyLambdaHandlerOptions = { persistenceStore: BasePersistenceLayer; config?: IdempotencyConfig; }; -type IdempotencyFunctionOptions = IdempotencyLambdaHandlerOptions & { - dataKeywordArgument: string; -}; +/** + * This generic type is used to represent any function with any number of arguments and any return type. + * + * It's left intentionally open to allow for any function to be wrapped. + */ +// eslint-disable-next-line @typescript-eslint/no-explicit-any +type AnyFunction = (...args: Array) => any; -type IdempotencyHandlerOptions = { - functionToMakeIdempotent: AnyFunctionWithRecord; - functionPayloadToBeHashed: Record; - persistenceStore: BasePersistenceLayer; +/** + * This is a conditional type that represents the options that can be passed to the `makeIdempotent` function. + * + * Depending on the function being wrapped, the options will be different. For example, + * if the function being wrapped is a Lambda handler (as indicated by the presence of a `Context` + * object as the second argument), then the options will be the standard `IdempotencyLambdaHandlerOptions`. + * + * If instead the function being wrapped is an arbitrary function, then the options can include a + * `dataIndexArgument` property to indicate the index of the argument that contains the data to be hashed. + * + * The reasoning behind this is that the `makeIdempotent` function needs to know where to find the function + * payload, and while we can make assumptions about the structure of a Lambda handler, we can't be certain + * for an arbitrary function. + * + * When the function being wrapped is a Lambda handler the `event` object, which is always the first argument + * of the handler, is used as idempoency payload. For this reason, you don't need to specify the `dataIndexArgument`. + * + * @example + * ```ts + * import type { + * APIGatewayProxyEvent, + * Context, + * APIGatewayProxyResult, + * } from 'aws-lambda'; + * import { makeIdempotent } from '@aws-lambda-powertools/idempotency'; + * import { DynamoDBPersistenceLayer } from '@aws-lambda-powertools/idempotency/dynamodb'; + * + * const myHandler = async ( + * event: APIGatewayProxyEvent, + * context: Context + * ): Promise => { + * // your code goes here + * }; + * + * // Since the function being wrapped is a Lambda handler, the `event` object is used as idempotency payload + * export const handler = makeIdempotent(myHandler, { + * persistenceStore: new DynamoDBPersistenceLayer({ tableName: 'my-table' }), + * }); + * ``` + * + * On the other hand, when the function being wrapped is an arbitray function, the data to be hashed can be extracted + * from any argument of the function. In JavaScript, functions can be called with any number of arguments, and the + * `dataIndexArgument` property is used to indicate the index of the argument that contains the payload. + * + * By default, if the `dataIndexArgument` property is not specified, the first argument is used as idempotency payload. + * However, you can specify a different argument by setting the `dataIndexArgument` property. Note that the index of the + * argument is zero-based, so the first argument has an index of `0`. + * + * @example + * ```ts + * import { makeIdempotent } from '@aws-lambda-powertools/idempotency'; + * import { DynamoDBPersistenceLayer } from '@aws-lambda-powertools/idempotency/dynamodb'; + * + * const myFunction = (data: string, bar: number): string => { + * // your code goes here + * }; + * + * // This will use the `data` argument (first) as idempotency payload + * const idempotentMyFunction = makeIdempotent(myFunction, { + * persistenceStore: new DynamoDBPersistenceLayer({ tableName: 'my-table' }), + * }); + * + * // This will use the `bar` argument as idempotency payload + * const idempotentMyFunction = makeIdempotent(myFunction, { + * persistenceStore: new DynamoDBPersistenceLayer({ tableName: 'my-table' }), + * dataIndexArgument: 1, + * }); + * ``` + * + * If instead you want the Idempotency utility to use only part of your payload as idempotency payload, you can use + * the `eventKeyJmesPath` property to indicate the JMESPath expression to use to extract part of the payload. + * + */ +// eslint-disable-next-line @typescript-eslint/no-explicit-any +type ItempotentFunctionOptions> = T[1] extends Context + ? IdempotencyLambdaHandlerOptions + : IdempotencyLambdaHandlerOptions & { + dataIndexArgument?: number; + }; + +/** + * @internal + * Options to configure the behavior of the idempotency logic. + * + * This is an internal type that is used by the Idempotency utility to + * configure {@link IdempotencyHandler}. + */ +type IdempotencyHandlerOptions = { + /** + * The arguments passed to the function. + * + * For example, if the function is `foo(a, b)`, then `functionArguments` will be `[a, b]`. + * We need to keep track of the arguments so that we can pass them to the function when we call it. + */ + functionArguments: unknown[]; + /** + * The payload to be hashed. + * + * This is the argument that is used for the idempotency. + */ + functionPayloadToBeHashed: JSONValue; + /** + * Reference to the function to be made idempotent. + */ + functionToMakeIdempotent: AnyFunction; + /** + * Idempotency configuration options. + */ idempotencyConfig: IdempotencyConfig; - fullFunctionPayload: Record; + /** + * Persistence layer used to store the idempotency records. + */ + persistenceStore: BasePersistenceLayer; }; /** @@ -59,8 +180,9 @@ type IdempotencyConfigOptions = { }; export { + AnyFunction, IdempotencyConfigOptions, - IdempotencyFunctionOptions, + ItempotentFunctionOptions, IdempotencyLambdaHandlerOptions, IdempotencyHandlerOptions, }; diff --git a/packages/idempotency/src/types/IdempotencyRecord.ts b/packages/idempotency/src/types/IdempotencyRecord.ts index a9fa880caf..14a93748f6 100644 --- a/packages/idempotency/src/types/IdempotencyRecord.ts +++ b/packages/idempotency/src/types/IdempotencyRecord.ts @@ -1,3 +1,5 @@ +import type { JSONValue } from '@aws-lambda-powertools/commons'; + const IdempotencyRecordStatus = { INPROGRESS: 'INPROGRESS', COMPLETED: 'COMPLETED', @@ -12,7 +14,7 @@ type IdempotencyRecordOptions = { status: IdempotencyRecordStatus; expiryTimestamp?: number; inProgressExpiryTimestamp?: number; - responseData?: Record; + responseData?: JSONValue; payloadHash?: string; }; diff --git a/packages/idempotency/src/types/index.ts b/packages/idempotency/src/types/index.ts index 4cde1c7882..741a6fa409 100644 --- a/packages/idempotency/src/types/index.ts +++ b/packages/idempotency/src/types/index.ts @@ -1,4 +1,3 @@ -export * from './AnyFunction'; export * from './IdempotencyRecord'; export * from './BasePersistenceLayer'; export * from './IdempotencyOptions'; diff --git a/packages/idempotency/tests/unit/IdempotencyHandler.test.ts b/packages/idempotency/tests/unit/IdempotencyHandler.test.ts index 4650e12543..977623e14b 100644 --- a/packages/idempotency/tests/unit/IdempotencyHandler.test.ts +++ b/packages/idempotency/tests/unit/IdempotencyHandler.test.ts @@ -10,19 +10,13 @@ import { IdempotencyPersistenceLayerError, } from '../../src/errors'; import { IdempotencyRecordStatus } from '../../src/types'; -import { BasePersistenceLayer, IdempotencyRecord } from '../../src/persistence'; +import { IdempotencyRecord } from '../../src/persistence'; import { IdempotencyHandler } from '../../src/IdempotencyHandler'; import { IdempotencyConfig } from '../../src/'; import { MAX_RETRIES } from '../../src/constants'; +import { PersistenceLayerTestClass } from '../helpers/idempotencyUtils'; import { Context } from 'aws-lambda'; -class PersistenceLayerTestClass extends BasePersistenceLayer { - protected _deleteRecord = jest.fn(); - protected _getRecord = jest.fn(); - protected _putRecord = jest.fn(); - protected _updateRecord = jest.fn(); -} - const mockFunctionToMakeIdempotent = jest.fn(); const mockFunctionPayloadToBeHashed = {}; const mockIdempotencyOptions = { @@ -30,18 +24,30 @@ const mockIdempotencyOptions = { dataKeywordArgument: 'testKeywordArgument', config: new IdempotencyConfig({}), }; -const mockFullFunctionPayload = {}; const idempotentHandler = new IdempotencyHandler({ functionToMakeIdempotent: mockFunctionToMakeIdempotent, functionPayloadToBeHashed: mockFunctionPayloadToBeHashed, persistenceStore: mockIdempotencyOptions.persistenceStore, - fullFunctionPayload: mockFullFunctionPayload, + functionArguments: [], idempotencyConfig: mockIdempotencyOptions.config, }); describe('Class IdempotencyHandler', () => { - beforeEach(() => jest.resetAllMocks()); + const ENVIRONMENT_VARIABLES = process.env; + + beforeEach(() => { + jest.clearAllMocks(); + jest.restoreAllMocks(); + process.env = { ...ENVIRONMENT_VARIABLES }; + jest.spyOn(console, 'debug').mockImplementation(() => null); + jest.spyOn(console, 'warn').mockImplementation(() => null); + jest.spyOn(console, 'error').mockImplementation(() => null); + }); + + afterAll(() => { + process.env = ENVIRONMENT_VARIABLES; + }); describe('Method: determineResultFromIdempotencyRecord', () => { test('when record is in progress and within expiry window, it rejects with IdempotencyAlreadyInProgressError', async () => { @@ -174,7 +180,7 @@ describe('Class IdempotencyHandler', () => { .mockImplementation(() => 'result'); await expect(idempotentHandler.processIdempotency()).rejects.toThrow( new IdempotencyPersistenceLayerError( - 'Failed to save record in progress', + 'Failed to save in progress record to idempotency store', innerError ) ); @@ -220,7 +226,7 @@ describe('Class IdempotencyHandler', () => { functionToMakeIdempotent: mockFunctionToMakeIdempotent, functionPayloadToBeHashed: mockFunctionPayloadToBeHashed, persistenceStore: mockIdempotencyOptions.persistenceStore, - fullFunctionPayload: mockFullFunctionPayload, + functionArguments: [], idempotencyConfig: new IdempotencyConfig({ throwOnNoIdempotencyKey: false, eventKeyJmesPath: 'idempotencyKey', @@ -268,7 +274,7 @@ describe('Class IdempotencyHandler', () => { functionToMakeIdempotent: mockFunctionToMakeIdempotent, functionPayloadToBeHashed: mockFunctionPayloadToBeHashed, persistenceStore: mockIdempotencyOptions.persistenceStore, - fullFunctionPayload: mockFullFunctionPayload, + functionArguments: [], idempotencyConfig: new IdempotencyConfig({ lambdaContext: mockLambaContext, }), @@ -328,7 +334,8 @@ describe('Class IdempotencyHandler', () => { await expect(idempotentHandler.getFunctionResult()).rejects.toThrow( new IdempotencyPersistenceLayerError( - 'Failed to delete record from idempotency store. This error was caused by: Some error.' + 'Failed to delete record from idempotency store', + new Error('Some error') ) ); expect(mockDeleteInProgress).toHaveBeenCalledTimes(1); diff --git a/packages/idempotency/tests/unit/makeFunctionIdempotent.test.ts b/packages/idempotency/tests/unit/makeFunctionIdempotent.test.ts deleted file mode 100644 index 8ae1a1687a..0000000000 --- a/packages/idempotency/tests/unit/makeFunctionIdempotent.test.ts +++ /dev/null @@ -1,220 +0,0 @@ -/** - * Test Function Wrapper - * - * @group unit/idempotency/makeFunctionIdempotent - */ -import { IdempotencyFunctionOptions } from '../../src/types/'; -import { BasePersistenceLayer, IdempotencyRecord } from '../../src/persistence'; -import { makeFunctionIdempotent } from '../../src'; -import type { - AnyIdempotentFunction, - IdempotencyRecordOptions, -} from '../../src/types'; -import { IdempotencyRecordStatus } from '../../src/types'; -import { - IdempotencyAlreadyInProgressError, - IdempotencyInconsistentStateError, - IdempotencyItemAlreadyExistsError, - IdempotencyPersistenceLayerError, -} from '../../src/errors'; -import { IdempotencyConfig } from '../../src'; -import { Context } from 'aws-lambda'; - -const mockSaveInProgress = jest - .spyOn(BasePersistenceLayer.prototype, 'saveInProgress') - .mockImplementation(); -const mockGetRecord = jest - .spyOn(BasePersistenceLayer.prototype, 'getRecord') - .mockImplementation(); - -const mockLambaContext: Context = { - getRemainingTimeInMillis(): number { - return 1000; // we expect this number to be passed to saveInProgress - }, -} as Context; - -class PersistenceLayerTestClass extends BasePersistenceLayer { - protected _deleteRecord = jest.fn(); - protected _getRecord = jest.fn(); - protected _putRecord = jest.fn(); - protected _updateRecord = jest.fn(); -} - -describe('Given a function to wrap', (functionToWrap = jest.fn()) => { - beforeEach(() => jest.clearAllMocks()); - describe('Given options for idempotency', (options: IdempotencyFunctionOptions = { - persistenceStore: new PersistenceLayerTestClass(), - dataKeywordArgument: 'testingKey', - config: new IdempotencyConfig({ lambdaContext: mockLambaContext }), - }) => { - const keyValueToBeSaved = 'thisWillBeSaved'; - const inputRecord = { - testingKey: keyValueToBeSaved, - otherKey: 'thisWillNot', - }; - describe('When wrapping a function with no previous executions', () => { - let resultingFunction: AnyIdempotentFunction; - beforeEach(async () => { - resultingFunction = makeFunctionIdempotent(functionToWrap, options); - await resultingFunction(inputRecord); - }); - - test('Then it will save the record to INPROGRESS', () => { - expect(mockSaveInProgress).toBeCalledWith( - keyValueToBeSaved, - mockLambaContext.getRemainingTimeInMillis() - ); - }); - - test('Then it will call the function that was wrapped with the whole input record', () => { - expect(functionToWrap).toBeCalledWith(inputRecord); - }); - }); - - describe('When wrapping a function with previous execution that is INPROGRESS', () => { - let resultingFunction: AnyIdempotentFunction; - let resultingError: Error; - beforeEach(async () => { - mockSaveInProgress.mockRejectedValue( - new IdempotencyItemAlreadyExistsError() - ); - const idempotencyOptions: IdempotencyRecordOptions = { - idempotencyKey: 'key', - status: IdempotencyRecordStatus.INPROGRESS, - }; - mockGetRecord.mockResolvedValue( - new IdempotencyRecord(idempotencyOptions) - ); - resultingFunction = makeFunctionIdempotent(functionToWrap, options); - try { - await resultingFunction(inputRecord); - } catch (e) { - resultingError = e as Error; - } - }); - - test('Then it will attempt to save the record to INPROGRESS', () => { - expect(mockSaveInProgress).toBeCalledWith( - keyValueToBeSaved, - mockLambaContext.getRemainingTimeInMillis() - ); - }); - - test('Then it will get the previous execution record', () => { - expect(mockGetRecord).toBeCalledWith(keyValueToBeSaved); - }); - - test('Then the function that was wrapped is not called again', () => { - expect(functionToWrap).not.toBeCalled(); - }); - - test('Then an IdempotencyAlreadyInProgressError is thrown', () => { - expect(resultingError).toBeInstanceOf( - IdempotencyAlreadyInProgressError - ); - }); - }); - - describe('When wrapping a function with previous execution that is EXPIRED', () => { - let resultingFunction: AnyIdempotentFunction; - let resultingError: Error; - beforeEach(async () => { - mockSaveInProgress.mockRejectedValue( - new IdempotencyItemAlreadyExistsError() - ); - const idempotencyOptions: IdempotencyRecordOptions = { - idempotencyKey: 'key', - status: IdempotencyRecordStatus.EXPIRED, - }; - mockGetRecord.mockResolvedValue( - new IdempotencyRecord(idempotencyOptions) - ); - resultingFunction = makeFunctionIdempotent(functionToWrap, options); - try { - await resultingFunction(inputRecord); - } catch (e) { - resultingError = e as Error; - } - }); - - test('Then it will attempt to save the record to INPROGRESS', () => { - expect(mockSaveInProgress).toBeCalledWith( - keyValueToBeSaved, - mockLambaContext.getRemainingTimeInMillis() - ); - }); - - test('Then it will get the previous execution record', () => { - expect(mockGetRecord).toBeCalledWith(keyValueToBeSaved); - }); - - test('Then the function that was wrapped is not called again', () => { - expect(functionToWrap).not.toBeCalled(); - }); - - test('Then an IdempotencyInconsistentStateError is thrown', () => { - expect(resultingError).toBeInstanceOf( - IdempotencyInconsistentStateError - ); - }); - }); - - describe('When wrapping a function with previous execution that is COMPLETED', () => { - let resultingFunction: AnyIdempotentFunction; - beforeEach(async () => { - mockSaveInProgress.mockRejectedValue( - new IdempotencyItemAlreadyExistsError() - ); - const idempotencyOptions: IdempotencyRecordOptions = { - idempotencyKey: 'key', - status: IdempotencyRecordStatus.COMPLETED, - }; - mockGetRecord.mockResolvedValue( - new IdempotencyRecord(idempotencyOptions) - ); - resultingFunction = makeFunctionIdempotent(functionToWrap, options); - await resultingFunction(inputRecord); - }); - - test('Then it will attempt to save the record to INPROGRESS', () => { - expect(mockSaveInProgress).toBeCalledWith( - keyValueToBeSaved, - mockLambaContext.getRemainingTimeInMillis() - ); - }); - - test('Then it will get the previous execution record', () => { - expect(mockGetRecord).toBeCalledWith(keyValueToBeSaved); - }); - - test('Then it will not call the function that was wrapped with the whole input record', () => { - expect(functionToWrap).not.toBeCalledWith(inputRecord); - }); - }); - - describe('When wrapping a function with issues saving the record', () => { - let resultingFunction: AnyIdempotentFunction; - let resultingError: Error; - beforeEach(async () => { - mockSaveInProgress.mockRejectedValue(new Error('RandomError')); - resultingFunction = makeFunctionIdempotent(functionToWrap, options); - try { - await resultingFunction(inputRecord); - } catch (e) { - resultingError = e as Error; - } - }); - - test('Then it will attempt to save the record to INPROGRESS', () => { - expect(mockSaveInProgress).toBeCalledWith( - keyValueToBeSaved, - mockLambaContext.getRemainingTimeInMillis() - ); - }); - - test('Then an IdempotencyPersistenceLayerError is thrown', () => { - expect(resultingError).toBeInstanceOf(IdempotencyPersistenceLayerError); - }); - }); - }); -}); diff --git a/packages/idempotency/tests/unit/makeHandlerIdempotent.test.ts b/packages/idempotency/tests/unit/makeHandlerIdempotent.test.ts index 09c0c22f80..bb5d890c59 100644 --- a/packages/idempotency/tests/unit/makeHandlerIdempotent.test.ts +++ b/packages/idempotency/tests/unit/makeHandlerIdempotent.test.ts @@ -22,14 +22,11 @@ import type { Context } from 'aws-lambda'; const mockIdempotencyOptions = { persistenceStore: new PersistenceLayerTestClass(), }; -const remainingTImeInMillis = 10000; +const remainingTImeInMillis = 1234; describe('Middleware: makeHandlerIdempotent', () => { const ENVIRONMENT_VARIABLES = process.env; const context = dummyContext; - context.getRemainingTimeInMillis = jest - .fn() - .mockReturnValue(remainingTImeInMillis); const event = dummyEvent.CustomEvent; beforeEach(() => { @@ -48,7 +45,7 @@ describe('Middleware: makeHandlerIdempotent', () => { it('handles a successful execution', async () => { // Prepare const handler = middy( - async (_event: unknown, _context: Context): Promise => true + async (_event: unknown, context: Context) => context.awsRequestId ).use( makeHandlerIdempotent({ ...mockIdempotencyOptions, @@ -68,14 +65,14 @@ describe('Middleware: makeHandlerIdempotent', () => { const result = await handler(event, context); // Assess - expect(result).toBe(true); + expect(result).toBe(context.awsRequestId); expect(saveInProgressSpy).toHaveBeenCalledTimes(1); expect(saveInProgressSpy).toHaveBeenCalledWith( event, remainingTImeInMillis ); expect(saveSuccessSpy).toHaveBeenCalledTimes(1); - expect(saveSuccessSpy).toHaveBeenCalledWith(event, true); + expect(saveSuccessSpy).toHaveBeenCalledWith(event, context.awsRequestId); }); it('handles an execution that throws an error', async () => { // Prepare @@ -115,7 +112,8 @@ describe('Middleware: makeHandlerIdempotent', () => { // Act && Assess await expect(handler(event, context)).rejects.toThrowError( new IdempotencyPersistenceLayerError( - 'Failed to save in progress record to idempotency store. This error was caused by: Something went wrong.' + 'Failed to save in progress record to idempotency store', + new Error('Something went wrong') ) ); }); @@ -131,7 +129,8 @@ describe('Middleware: makeHandlerIdempotent', () => { // Act && Assess await expect(handler(event, context)).rejects.toThrowError( new IdempotencyPersistenceLayerError( - 'Failed to update success record to idempotency store. This error was caused by: Something went wrong.' + 'Failed to update success record to idempotency store', + new Error('Something went wrong') ) ); }); @@ -149,7 +148,8 @@ describe('Middleware: makeHandlerIdempotent', () => { // Act && Assess await expect(handler(event, context)).rejects.toThrow( new IdempotencyPersistenceLayerError( - 'Failed to delete record from idempotency store. This error was caused by: Something went wrong.' + 'Failed to delete record from idempotency store', + new Error('Something went wrong') ) ); }); @@ -300,7 +300,7 @@ describe('Middleware: makeHandlerIdempotent', () => { expect(saveSuccessSpy).toHaveBeenCalledTimes(0); }); - it(' skips idempotency if error is thrown in the middleware', async () => { + it('skips idempotency if error is thrown in the middleware', async () => { const handler = middy( async (_event: unknown, _context: Context): Promise => { throw new Error('Something went wrong'); diff --git a/packages/idempotency/tests/unit/makeIdempotent.test.ts b/packages/idempotency/tests/unit/makeIdempotent.test.ts new file mode 100644 index 0000000000..0eb229e002 --- /dev/null +++ b/packages/idempotency/tests/unit/makeIdempotent.test.ts @@ -0,0 +1,401 @@ +/** + * Test Function Wrapper + * + * @group unit/idempotency/makeIdempotent + */ +import { IdempotencyRecord } from '../../src/persistence'; +import { makeIdempotent } from '../../src'; +import { IdempotencyRecordStatus } from '../../src/types'; +import { + IdempotencyInconsistentStateError, + IdempotencyItemAlreadyExistsError, + IdempotencyPersistenceLayerError, +} from '../../src/errors'; +import { IdempotencyConfig } from '../../src'; +import { helloworldContext as dummyContext } from '../../../commons/src/samples/resources/contexts'; +import { Custom as dummyEvent } from '../../../commons/src/samples/resources/events'; +import { MAX_RETRIES } from '../../src/constants'; +import { PersistenceLayerTestClass } from '../helpers/idempotencyUtils'; +import type { Context } from 'aws-lambda'; + +const mockIdempotencyOptions = { + persistenceStore: new PersistenceLayerTestClass(), +}; +const remainingTImeInMillis = 1234; + +describe('Function: makeIdempotent', () => { + const ENVIRONMENT_VARIABLES = process.env; + const context = dummyContext; + const event = dummyEvent.CustomEvent; + + beforeEach(() => { + jest.clearAllMocks(); + jest.restoreAllMocks(); + process.env = { ...ENVIRONMENT_VARIABLES }; + jest.spyOn(console, 'debug').mockImplementation(() => null); + jest.spyOn(console, 'warn').mockImplementation(() => null); + jest.spyOn(console, 'error').mockImplementation(() => null); + }); + + afterAll(() => { + process.env = ENVIRONMENT_VARIABLES; + }); + + it('handles a successful execution', async () => { + // Prepare + const handler = makeIdempotent( + async (_event: unknown, context: Context) => context.awsRequestId, + { + ...mockIdempotencyOptions, + config: new IdempotencyConfig({}), + } + ); + const saveInProgressSpy = jest.spyOn( + mockIdempotencyOptions.persistenceStore, + 'saveInProgress' + ); + const saveSuccessSpy = jest.spyOn( + mockIdempotencyOptions.persistenceStore, + 'saveSuccess' + ); + + // Act + const result = await handler(event, context); + + // Assess + expect(result).toBe(context.awsRequestId); + expect(saveInProgressSpy).toHaveBeenCalledTimes(1); + expect(saveInProgressSpy).toHaveBeenCalledWith( + event, + remainingTImeInMillis + ); + expect(saveSuccessSpy).toHaveBeenCalledTimes(1); + expect(saveSuccessSpy).toHaveBeenCalledWith(event, context.awsRequestId); + }); + it('handles an execution that throws an error', async () => { + // Prepare + const handler = makeIdempotent( + async (_event: unknown, _context: Context) => { + throw new Error('Something went wrong'); + }, + { + ...mockIdempotencyOptions, + config: new IdempotencyConfig({}), + } + ); + const saveInProgressSpy = jest.spyOn( + mockIdempotencyOptions.persistenceStore, + 'saveInProgress' + ); + const deleteRecordSpy = jest.spyOn( + mockIdempotencyOptions.persistenceStore, + 'deleteRecord' + ); + + // Act && Assess + await expect(handler(event, context)).rejects.toThrow(); + expect(saveInProgressSpy).toHaveBeenCalledTimes(1); + expect(saveInProgressSpy).toHaveBeenCalledWith( + event, + remainingTImeInMillis + ); + expect(deleteRecordSpy).toHaveBeenCalledTimes(1); + expect(deleteRecordSpy).toHaveBeenCalledWith(event); + }); + it('thows an error if the persistence layer throws an error when saving in progress', async () => { + // Prepare + const handler = makeIdempotent( + async (_event: unknown, _context: Context) => true, + { + ...mockIdempotencyOptions, + config: new IdempotencyConfig({}), + } + ); + jest + .spyOn(mockIdempotencyOptions.persistenceStore, 'saveInProgress') + .mockRejectedValue(new Error('Something went wrong')); + + // Act && Assess + await expect(handler(event, context)).rejects.toThrowError( + new IdempotencyPersistenceLayerError( + 'Failed to save in progress record to idempotency store', + new Error('Something went wrong') + ) + ); + }); + it('thows an error if the persistence layer throws an error when saving a successful operation', async () => { + // Prepare + const handler = makeIdempotent( + async (_event: unknown, _context: Context) => true, + { + ...mockIdempotencyOptions, + config: new IdempotencyConfig({}), + } + ); + jest + .spyOn(mockIdempotencyOptions.persistenceStore, 'saveSuccess') + .mockRejectedValue(new Error('Something went wrong')); + + // Act && Assess + await expect(handler(event, context)).rejects.toThrowError( + new IdempotencyPersistenceLayerError( + 'Failed to update success record to idempotency store', + new Error('Something went wrong') + ) + ); + }); + it('thows an error if the persistence layer throws an error when deleting a record', async () => { + // Prepare + const handler = makeIdempotent( + async (_event: unknown, _context: Context) => { + throw new Error('Something went wrong'); + }, + { + ...mockIdempotencyOptions, + config: new IdempotencyConfig({}), + } + ); + jest + .spyOn(mockIdempotencyOptions.persistenceStore, 'deleteRecord') + .mockRejectedValue(new Error('Something went wrong')); + + // Act && Assess + await expect(handler(event, context)).rejects.toThrow( + new IdempotencyPersistenceLayerError( + 'Failed to delete record from idempotency store', + new Error('Something went wrong') + ) + ); + }); + it('returns the stored response if the operation has already been executed', async () => { + // Prepare + const handler = makeIdempotent( + async (_event: unknown, _context: Context) => true, + { + ...mockIdempotencyOptions, + config: new IdempotencyConfig({}), + } + ); + jest + .spyOn(mockIdempotencyOptions.persistenceStore, 'saveInProgress') + .mockRejectedValue(new IdempotencyItemAlreadyExistsError()); + const stubRecord = new IdempotencyRecord({ + idempotencyKey: 'idempotencyKey', + expiryTimestamp: Date.now() + 10000, + inProgressExpiryTimestamp: 0, + responseData: { response: false }, + payloadHash: 'payloadHash', + status: IdempotencyRecordStatus.COMPLETED, + }); + const getRecordSpy = jest + .spyOn(mockIdempotencyOptions.persistenceStore, 'getRecord') + .mockResolvedValue(stubRecord); + + // Act + const result = await handler(event, context); + + // Assess + expect(result).toStrictEqual({ response: false }); + expect(getRecordSpy).toHaveBeenCalledTimes(1); + expect(getRecordSpy).toHaveBeenCalledWith(event); + }); + it('retries if the record is in an inconsistent state', async () => { + // Prepare + const handler = makeIdempotent( + async (_event: unknown, _context: Context) => true, + { + ...mockIdempotencyOptions, + config: new IdempotencyConfig({}), + } + ); + jest + .spyOn(mockIdempotencyOptions.persistenceStore, 'saveInProgress') + .mockRejectedValue(new IdempotencyItemAlreadyExistsError()); + const stubRecordInconsistent = new IdempotencyRecord({ + idempotencyKey: 'idempotencyKey', + expiryTimestamp: Date.now() + 10000, + inProgressExpiryTimestamp: 0, + responseData: { response: false }, + payloadHash: 'payloadHash', + status: IdempotencyRecordStatus.EXPIRED, + }); + const stubRecord = new IdempotencyRecord({ + idempotencyKey: 'idempotencyKey', + expiryTimestamp: Date.now() + 10000, + inProgressExpiryTimestamp: 0, + responseData: { response: false }, + payloadHash: 'payloadHash', + status: IdempotencyRecordStatus.COMPLETED, + }); + const getRecordSpy = jest + .spyOn(mockIdempotencyOptions.persistenceStore, 'getRecord') + .mockResolvedValueOnce(stubRecordInconsistent) + .mockResolvedValueOnce(stubRecord); + + // Act + const result = await handler(event, context); + + // Assess + expect(result).toStrictEqual({ response: false }); + expect(getRecordSpy).toHaveBeenCalledTimes(2); + }); + it('throws after all the retries have been exhausted if the record is in an inconsistent state', async () => { + // Prepare + const handler = makeIdempotent( + async (_event: unknown, _context: Context) => true, + { + ...mockIdempotencyOptions, + config: new IdempotencyConfig({}), + } + ); + jest + .spyOn(mockIdempotencyOptions.persistenceStore, 'saveInProgress') + .mockRejectedValue(new IdempotencyItemAlreadyExistsError()); + const stubRecordInconsistent = new IdempotencyRecord({ + idempotencyKey: 'idempotencyKey', + expiryTimestamp: Date.now() + 10000, + inProgressExpiryTimestamp: 0, + responseData: { response: false }, + payloadHash: 'payloadHash', + status: IdempotencyRecordStatus.EXPIRED, + }); + const getRecordSpy = jest + .spyOn(mockIdempotencyOptions.persistenceStore, 'getRecord') + .mockResolvedValue(stubRecordInconsistent); + + // Act & Assess + await expect(handler(event, context)).rejects.toThrowError( + new IdempotencyInconsistentStateError( + 'Item has expired during processing and may not longer be valid.' + ) + ); + expect(getRecordSpy).toHaveBeenCalledTimes(MAX_RETRIES + 1); + }); + it('does not do anything if idempotency is disabled', async () => { + // Prepare + process.env.POWERTOOLS_IDEMPOTENCY_DISABLED = 'true'; + const handler = makeIdempotent( + async (_event: unknown, _context: Context) => true, + { + ...mockIdempotencyOptions, + } + ); + const saveInProgressSpy = jest.spyOn( + mockIdempotencyOptions.persistenceStore, + 'saveInProgress' + ); + const saveSuccessSpy = jest.spyOn( + mockIdempotencyOptions.persistenceStore, + 'saveSuccess' + ); + + // Act + const result = await handler(event, context); + + // Assess + expect(result).toBe(true); + expect(saveInProgressSpy).toHaveBeenCalledTimes(0); + expect(saveSuccessSpy).toHaveBeenCalledTimes(0); + }); + + it('skips idempotency if no idempotency key is provided and throwOnNoIdempotencyKey is false', async () => { + // Prepare + const handler = makeIdempotent( + async (_event: unknown, _context: Context) => true, + { + ...mockIdempotencyOptions, + config: new IdempotencyConfig({ + eventKeyJmesPath: 'idempotencyKey', + throwOnNoIdempotencyKey: false, + }), + } + ); + const saveInProgressSpy = jest.spyOn( + mockIdempotencyOptions.persistenceStore, + 'saveInProgress' + ); + const saveSuccessSpy = jest.spyOn( + mockIdempotencyOptions.persistenceStore, + 'saveSuccess' + ); + + // Act + const result = await handler(event, context); + + // Assess + expect(result).toBe(true); + expect(saveInProgressSpy).toHaveBeenCalledTimes(0); + expect(saveSuccessSpy).toHaveBeenCalledTimes(0); + }); + it('when wrapping an arbitrary function it uses the first argument as payload by default', async () => { + // Prepare + const config = new IdempotencyConfig({}); + config.registerLambdaContext(context); + + const arbitraryFn = makeIdempotent( + async (foo: { bar: string }, baz: string) => `${foo.bar}${baz}`, + { + ...mockIdempotencyOptions, + config, + } + ); + const saveInProgressSpy = jest.spyOn( + mockIdempotencyOptions.persistenceStore, + 'saveInProgress' + ); + const saveSuccessSpy = jest.spyOn( + mockIdempotencyOptions.persistenceStore, + 'saveSuccess' + ); + const event = { bar: '123' }; + + // Act + const result = await arbitraryFn(event, '456'); + + // Assess + expect(result).toBe('123456'); + expect(saveInProgressSpy).toHaveBeenCalledTimes(1); + expect(saveInProgressSpy).toHaveBeenCalledWith( + event, + remainingTImeInMillis + ); + expect(saveSuccessSpy).toHaveBeenCalledTimes(1); + expect(saveSuccessSpy).toHaveBeenCalledWith(event, '123456'); + }); + it('when wrapping an arbitrary function it uses the argument specified as payload by default', async () => { + // Prepare + const config = new IdempotencyConfig({}); + config.registerLambdaContext(context); + + const arbitraryFn = makeIdempotent( + async (foo: { bar: string }, baz: string) => `${foo.bar}${baz}`, + { + ...mockIdempotencyOptions, + config, + dataIndexArgument: 1, + } + ); + const saveInProgressSpy = jest.spyOn( + mockIdempotencyOptions.persistenceStore, + 'saveInProgress' + ); + const saveSuccessSpy = jest.spyOn( + mockIdempotencyOptions.persistenceStore, + 'saveSuccess' + ); + const event = { bar: '123' }; + + // Act + const result = await arbitraryFn(event, '456'); + + // Assess + expect(result).toBe('123456'); + expect(saveInProgressSpy).toHaveBeenCalledTimes(1); + expect(saveInProgressSpy).toHaveBeenCalledWith( + '456', + remainingTImeInMillis + ); + expect(saveSuccessSpy).toHaveBeenCalledTimes(1); + expect(saveSuccessSpy).toHaveBeenCalledWith('456', '123456'); + }); +});