From 3016a84b5d1712cbc8159b4f9ff5787335f0574a Mon Sep 17 00:00:00 2001 From: arnabrahman Date: Sun, 20 Apr 2025 11:21:39 +0600 Subject: [PATCH 01/51] feat: add @redis/client as a peer dependency in idempotency package --- package-lock.json | 39 +++++++++++++++++++++++++++++-- packages/idempotency/package.json | 3 ++- 2 files changed, 39 insertions(+), 3 deletions(-) diff --git a/package-lock.json b/package-lock.json index 9a74dff3e..196eeb644 100644 --- a/package-lock.json +++ b/package-lock.json @@ -11826,6 +11826,21 @@ "node": ">=14" } }, + "node_modules/@redis/client": { + "version": "1.6.0", + "resolved": "https://registry.npmjs.org/@redis/client/-/client-1.6.0.tgz", + "integrity": "sha512-aR0uffYI700OEEH4gYnitAnv3vzVGXCFvYfdpu/CJKvk4pHfLPEy/JSZyrpQ+15WhXe1yJRXLtfQ84s4mEXnPg==", + "license": "MIT", + "peer": true, + "dependencies": { + "cluster-key-slot": "1.1.2", + "generic-pool": "3.9.0", + "yallist": "4.0.0" + }, + "engines": { + "node": ">=14" + } + }, "node_modules/@rollup/rollup-android-arm-eabi": { "version": "4.38.0", "resolved": "https://registry.npmjs.org/@rollup/rollup-android-arm-eabi/-/rollup-android-arm-eabi-4.38.0.tgz", @@ -15405,6 +15420,16 @@ "semver": "bin/semver" } }, + "node_modules/cluster-key-slot": { + "version": "1.1.2", + "resolved": "https://registry.npmjs.org/cluster-key-slot/-/cluster-key-slot-1.1.2.tgz", + "integrity": "sha512-RMr0FhtfXemyinomL4hrWcYJxmX6deFdCxpJzhDttxgO1+bcCnkk+9drydLVDmAMG7NE6aN/fl4F7ucU/90gAA==", + "license": "Apache-2.0", + "peer": true, + "engines": { + "node": ">=0.10.0" + } + }, "node_modules/cmd-shim": { "version": "6.0.1", "resolved": "https://registry.npmjs.org/cmd-shim/-/cmd-shim-6.0.1.tgz", @@ -16813,6 +16838,16 @@ "node": "^12.13.0 || ^14.15.0 || >=16.0.0" } }, + "node_modules/generic-pool": { + "version": "3.9.0", + "resolved": "https://registry.npmjs.org/generic-pool/-/generic-pool-3.9.0.tgz", + "integrity": "sha512-hymDOu5B53XvN4QT9dBmZxPX4CWhBPPLguTZ9MMFeFa/Kg0xWVfylOVNlJji/E7yTZWFd/q9GO5TxDLq156D7g==", + "license": "MIT", + "peer": true, + "engines": { + "node": ">= 4" + } + }, "node_modules/get-caller-file": { "version": "2.0.5", "resolved": "https://registry.npmjs.org/get-caller-file/-/get-caller-file-2.0.5.tgz", @@ -24664,7 +24699,6 @@ "version": "4.0.0", "resolved": "https://registry.npmjs.org/yallist/-/yallist-4.0.0.tgz", "integrity": "sha512-3wdGidZyq5PB084XLES5TpOSRA3wjXAlIWMhum2kRcv/41Sn2emQ0dycQW4uZXLejwKvg6EsvbdlVL+FYEct7A==", - "dev": true, "license": "ISC" }, "node_modules/yaml": { @@ -24831,7 +24865,8 @@ "peerDependencies": { "@aws-sdk/client-dynamodb": ">=3.x", "@aws-sdk/lib-dynamodb": ">=3.x", - "@middy/core": "4.x || 5.x || 6.x" + "@middy/core": "4.x || 5.x || 6.x", + "@redis/client": "^1.6.0" }, "peerDependenciesMeta": { "@aws-sdk/client-dynamodb": { diff --git a/packages/idempotency/package.json b/packages/idempotency/package.json index 537749632..97813db55 100644 --- a/packages/idempotency/package.json +++ b/packages/idempotency/package.json @@ -104,7 +104,8 @@ "peerDependencies": { "@aws-sdk/client-dynamodb": ">=3.x", "@aws-sdk/lib-dynamodb": ">=3.x", - "@middy/core": "4.x || 5.x || 6.x" + "@middy/core": "4.x || 5.x || 6.x", + "@redis/client": "^1.6.0" }, "peerDependenciesMeta": { "@aws-sdk/client-dynamodb": { From 2110332bea9801fb8b7ed432ca018eeec7933f08 Mon Sep 17 00:00:00 2001 From: arnabrahman Date: Sun, 20 Apr 2025 11:34:03 +0600 Subject: [PATCH 02/51] chore: mark @redis/client as an optional dependency --- packages/idempotency/package.json | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/idempotency/package.json b/packages/idempotency/package.json index 97813db55..a6a2b1f2b 100644 --- a/packages/idempotency/package.json +++ b/packages/idempotency/package.json @@ -116,6 +116,9 @@ }, "@middy/core": { "optional": true + }, + "@redis/client": { + "optional": true } }, "keywords": [ From b0ade6d34c9404255e92429e69c4e0c55f69a85f Mon Sep 17 00:00:00 2001 From: arnabrahman Date: Sun, 20 Apr 2025 11:51:31 +0600 Subject: [PATCH 03/51] chore: mark @redis/client and related dependencies as optional --- package-lock.json | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/package-lock.json b/package-lock.json index 196eeb644..f87a62f76 100644 --- a/package-lock.json +++ b/package-lock.json @@ -11831,6 +11831,7 @@ "resolved": "https://registry.npmjs.org/@redis/client/-/client-1.6.0.tgz", "integrity": "sha512-aR0uffYI700OEEH4gYnitAnv3vzVGXCFvYfdpu/CJKvk4pHfLPEy/JSZyrpQ+15WhXe1yJRXLtfQ84s4mEXnPg==", "license": "MIT", + "optional": true, "peer": true, "dependencies": { "cluster-key-slot": "1.1.2", @@ -15425,6 +15426,7 @@ "resolved": "https://registry.npmjs.org/cluster-key-slot/-/cluster-key-slot-1.1.2.tgz", "integrity": "sha512-RMr0FhtfXemyinomL4hrWcYJxmX6deFdCxpJzhDttxgO1+bcCnkk+9drydLVDmAMG7NE6aN/fl4F7ucU/90gAA==", "license": "Apache-2.0", + "optional": true, "peer": true, "engines": { "node": ">=0.10.0" @@ -16843,6 +16845,7 @@ "resolved": "https://registry.npmjs.org/generic-pool/-/generic-pool-3.9.0.tgz", "integrity": "sha512-hymDOu5B53XvN4QT9dBmZxPX4CWhBPPLguTZ9MMFeFa/Kg0xWVfylOVNlJji/E7yTZWFd/q9GO5TxDLq156D7g==", "license": "MIT", + "optional": true, "peer": true, "engines": { "node": ">= 4" @@ -24699,6 +24702,7 @@ "version": "4.0.0", "resolved": "https://registry.npmjs.org/yallist/-/yallist-4.0.0.tgz", "integrity": "sha512-3wdGidZyq5PB084XLES5TpOSRA3wjXAlIWMhum2kRcv/41Sn2emQ0dycQW4uZXLejwKvg6EsvbdlVL+FYEct7A==", + "devOptional": true, "license": "ISC" }, "node_modules/yaml": { @@ -24877,6 +24881,9 @@ }, "@middy/core": { "optional": true + }, + "@redis/client": { + "optional": true } } }, From b55374d4770a51e2b2b129d251bc4ff3077d8b24 Mon Sep 17 00:00:00 2001 From: arnabrahman Date: Sun, 27 Apr 2025 11:07:32 +0600 Subject: [PATCH 04/51] feat: add Redis persistence layer with connection configuration and protocol --- packages/idempotency/package.json | 16 ++ .../src/persistence/BasePersistenceLayer.ts | 2 +- .../src/persistence/RedisConnection.ts | 75 ++++++++++ .../idempotency/src/types/RedisPersistence.ts | 139 ++++++++++++++++++ packages/idempotency/src/types/index.ts | 5 + packages/idempotency/typedoc.json | 3 +- 6 files changed, 238 insertions(+), 2 deletions(-) create mode 100644 packages/idempotency/src/persistence/RedisConnection.ts create mode 100644 packages/idempotency/src/types/RedisPersistence.ts diff --git a/packages/idempotency/package.json b/packages/idempotency/package.json index a6a2b1f2b..ac17c7bda 100644 --- a/packages/idempotency/package.json +++ b/packages/idempotency/package.json @@ -52,6 +52,14 @@ "import": "./lib/esm/types/DynamoDBPersistence.js", "require": "./lib/cjs/types/DynamoDBPersistence.js" }, + "./redis": { + "import": "./lib/esm/persistence/RedisPersistenceLayer.js", + "require": "./lib/cjs/persistence/RedisPersistenceLayer.js" + }, + "./redis/types": { + "import": "./lib/esm/types/RedisPersistence.js", + "require": "./lib/cjs/types/RedisPersistence.js" + }, "./middleware": { "import": "./lib/esm/middleware/makeHandlerIdempotent.js", "require": "./lib/cjs/middleware/makeHandlerIdempotent.js" @@ -75,6 +83,14 @@ "lib/cjs/types/DynamoDBPersistence.d.ts", "lib/esm/types/DynamoDBPersistence.d.ts" ], + "redis": [ + "lib/cjs/persistence/RedisPersistenceLayer.d.ts", + "lib/esm/persistence/RedisPersistenceLayer.d.ts" + ], + "redis/types": [ + "lib/cjs/types/RedisPersistence.d.ts", + "lib/esm/types/RedisPersistence.d.ts" + ], "middleware": [ "lib/cjs/middleware/makeHandlerIdempotent.d.ts", "lib/esm/middleware/makeHandlerIdempotent.d.ts" diff --git a/packages/idempotency/src/persistence/BasePersistenceLayer.ts b/packages/idempotency/src/persistence/BasePersistenceLayer.ts index 0c13a6d75..cc29ac6e4 100644 --- a/packages/idempotency/src/persistence/BasePersistenceLayer.ts +++ b/packages/idempotency/src/persistence/BasePersistenceLayer.ts @@ -30,7 +30,7 @@ abstract class BasePersistenceLayer implements BasePersistenceLayerInterface { // envVarsService is always initialized in the constructor private envVarsService!: EnvironmentVariablesService; private eventKeyJmesPath?: string; - private expiresAfterSeconds: number = 60 * 60; // 1 hour default + protected expiresAfterSeconds: number = 60 * 60; // 1 hour default private hashFunction = 'md5'; private payloadValidationEnabled = false; private throwOnNoIdempotencyKey = false; diff --git a/packages/idempotency/src/persistence/RedisConnection.ts b/packages/idempotency/src/persistence/RedisConnection.ts new file mode 100644 index 000000000..d472a405b --- /dev/null +++ b/packages/idempotency/src/persistence/RedisConnection.ts @@ -0,0 +1,75 @@ +import { createClient, createCluster } from '@redis/client'; +import type { RedisClientType, RedisClusterType } from '@redis/client'; +import type { RedisConnectionConfig } from '../types/RedisPersistence.js'; + +/** + * RedisConnection class is responsible for creating a Redis client based on the provided configuration. + * It supports both standalone and cluster modes. + * The class takes a configuration object as a parameter and initializes the Redis client accordingly. + */ +class RedisConnection { + readonly #mode: 'standalone' | 'cluster'; + readonly #host: string; + readonly #port: number; + readonly #username: string; + readonly #password: string; + readonly #url: string; + readonly #dbIndex: number; + readonly #ssl: boolean; + + public constructor(options: RedisConnectionConfig) { + this.#mode = options.mode ?? 'standalone'; + this.#host = options.host ?? ''; + this.#port = options.port ?? 6379; + this.#username = options.username ?? ''; + this.#password = options.password ?? ''; + this.#url = options.url ?? ''; + this.#dbIndex = options.dbIndex ?? 0; + this.#ssl = options.ssl || true; + } + + /** + * Returns a Redis client based on the connection mode. + * If the mode is 'cluster', it creates a Redis cluster client. + * If the mode is 'standalone', it creates a standalone Redis client. + */ + public getClient(): RedisClientType | RedisClusterType { + return this.#mode === 'cluster' + ? this.#createClusterClient() + : this.#createStandaloneClient(); + } + + /** + * Creates a Redis cluster client. + * The client will connect to the Redis cluster using the provided URL. + */ + #createClusterClient(): RedisClusterType { + return createCluster({ + rootNodes: [{ url: this.#url }], + }); + } + + /** + * Creates a standalone Redis client. + * The client will connect to the Redis server using the provided host, port, username, password, and database index. + * If a URL is provided, it will be used to create the client instead of the other parameters. + */ + #createStandaloneClient(): RedisClientType { + if (this.#url) { + return createClient({ url: this.#url }); + } + + return createClient({ + username: this.#username, + password: this.#password, + socket: { + host: this.#host, + port: this.#port, + tls: this.#ssl, + }, + database: this.#dbIndex, + }); + } +} + +export default RedisConnection; diff --git a/packages/idempotency/src/types/RedisPersistence.ts b/packages/idempotency/src/types/RedisPersistence.ts new file mode 100644 index 000000000..d97fad857 --- /dev/null +++ b/packages/idempotency/src/types/RedisPersistence.ts @@ -0,0 +1,139 @@ +import type { JSONValue } from '@aws-lambda-powertools/commons/types'; + +/** + * Protocol defining the interface for a Redis client. + * This ensures standardization among different Redis client implementations. + */ +interface RedisClientProtocol { + + /** + * Indicates whether the connection to the Redis server is currently open and ready for commands. + * This can be used to check the connection status before sending commands. + */ + isOpen: boolean; + /** + * Retrieves the value associated with the given key. + * @param name The key to get the value for + */ + get(name: string): Promise; + + /** + * Sets the value for the specified key with optional parameters. + * @param name The key to set + * @param value The value to set + * @param options Optional parameters for setting the value + */ + set( + name: string, + value: string, + options?: { + EX?: number; // Expiration time in seconds + PX?: number; // Expiration time in milliseconds + NX?: boolean; // Only set the key if it does not already exist + } + ): Promise; + + /** + * Deletes one or more keys. + * @param keys The key(s) to delete + */ + del(keys: string): Promise; +} + +/** + * Redis client connection configuration parameters + */ +interface RedisConnectionConfig { + /** + * Redis host + */ + host?: string; + + /** + * Redis port (default: 6379) + */ + port?: number; + + /** + * Redis username + */ + username?: string; + + /** + * Redis password + */ + password?: string; + + /** + * Redis connection string URL, overrides host/port if provided + */ + url?: string; + + /** + * Redis database index (default: 0) + */ + dbIndex?: number; + + /** + * Redis client mode (default: 'standalone') + */ + mode?: 'standalone' | 'cluster'; + + /** + * Whether to use SSL for Redis connection (default: true) + */ + ssl?: boolean; +} + +/** + * Options for configuring the Redis persistence layer + */ +interface RedisPersistenceOptions extends RedisConnectionConfig { + /** + * A Redis client that implements the RedisClientProtocol interface. + * If provided, all other connection configuration options will be ignored. + */ + client: RedisClientProtocol; + + /** + * Redis JSON attribute name for expiry timestamp (default: 'expiration') + */ + expiryAttr?: string; + + /** + * Redis JSON attribute name for in-progress expiry timestamp (default: 'in_progress_expiration') + */ + inProgressExpiryAttr?: string; + + /** + * Redis JSON attribute name for status (default: 'status') + */ + statusAttr?: string; + + /** + * Redis JSON attribute name for response data (default: 'data') + */ + dataAttr?: string; + + /** + * Redis JSON attribute name for hashed representation of the parts of the event used for validation + * (default: 'validation') + */ + validationKeyAttr?: string; + + /** + * Function used to serialize JSON data (default: JSON.stringify) + */ + jsonSerializer?: (value: JSONValue) => string; + + /** + * Function used to deserialize JSON data (default: JSON.parse) + */ + jsonDeserializer?: (text: string) => JSONValue; +} + +export type { + RedisClientProtocol, + RedisConnectionConfig, + RedisPersistenceOptions, +}; diff --git a/packages/idempotency/src/types/index.ts b/packages/idempotency/src/types/index.ts index 0158d7855..f7d8fe5d6 100644 --- a/packages/idempotency/src/types/index.ts +++ b/packages/idempotency/src/types/index.ts @@ -20,3 +20,8 @@ export type { DynamoDBPersistenceOptionsWithClientConfig, DynamoDBPersistenceOptionsWithClientInstance, } from './DynamoDBPersistence.js'; +export type { + RedisClientProtocol, + RedisConnectionConfig, + RedisPersistenceOptions, +} from './RedisPersistence.js'; diff --git a/packages/idempotency/typedoc.json b/packages/idempotency/typedoc.json index 938073260..9b7ddb988 100644 --- a/packages/idempotency/typedoc.json +++ b/packages/idempotency/typedoc.json @@ -5,7 +5,8 @@ "./src/types/index.ts", "./src/middleware/makeHandlerIdempotent.ts", "./src/persistence/index.ts", - "./src/persistence/DynamoDBPersistenceLayer.ts" + "./src/persistence/DynamoDBPersistenceLayer.ts", + "./src/persistence/RedisPersistenceLayer.ts", ], "readme": "README.md" } From 5e7e0f2c03a528a4edddaa66e35f54db71a53e38 Mon Sep 17 00:00:00 2001 From: arnabrahman Date: Sun, 27 Apr 2025 16:17:46 +0600 Subject: [PATCH 05/51] fix: update RedisClientProtocol return type for set method --- packages/idempotency/src/types/RedisPersistence.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/idempotency/src/types/RedisPersistence.ts b/packages/idempotency/src/types/RedisPersistence.ts index d97fad857..8c1873191 100644 --- a/packages/idempotency/src/types/RedisPersistence.ts +++ b/packages/idempotency/src/types/RedisPersistence.ts @@ -31,7 +31,7 @@ interface RedisClientProtocol { PX?: number; // Expiration time in milliseconds NX?: boolean; // Only set the key if it does not already exist } - ): Promise; + ): Promise; /** * Deletes one or more keys. From b63fbb7a742e3a2dc799ae5acd1ed01134f098bf Mon Sep 17 00:00:00 2001 From: arnabrahman Date: Sun, 27 Apr 2025 16:18:09 +0600 Subject: [PATCH 06/51] feat: add persistence layer error classes for connection and consistency issues --- packages/idempotency/src/errors.ts | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/packages/idempotency/src/errors.ts b/packages/idempotency/src/errors.ts index ba2a0b77c..dbb9475d6 100644 --- a/packages/idempotency/src/errors.ts +++ b/packages/idempotency/src/errors.ts @@ -113,6 +113,29 @@ class IdempotencyKeyError extends IdempotencyUnknownError { } } +/** + * Error connecting to the persistence layer + */ +class IdempotencyPersistenceConnectionError extends IdempotencyUnknownError { + public constructor(message: string, options?: ErrorOptions) { + super(message, options); + this.name = 'IdempotencyPersistenceConnectionError'; + } +} + +/** + * Error with the persistence layer's consistency, needs to be removed + */ +class IdempotencyPersistenceConsistencyError extends IdempotencyUnknownError { + public constructor( + message: string, + options?: ErrorOptions + ) { + super(message, options); + this.name = 'IdempotencyPersistenceConsistencyError'; + } +} + export { IdempotencyUnknownError, IdempotencyItemAlreadyExistsError, @@ -122,5 +145,7 @@ export { IdempotencyValidationError, IdempotencyInconsistentStateError, IdempotencyPersistenceLayerError, + IdempotencyPersistenceConnectionError, + IdempotencyPersistenceConsistencyError, IdempotencyKeyError, }; From c04db42ebe7cf44e9b00b618e59192e5edef60c9 Mon Sep 17 00:00:00 2001 From: arnabrahman Date: Sun, 27 Apr 2025 16:18:28 +0600 Subject: [PATCH 07/51] feat: implement Redis persistence layer with idempotency record management --- .../src/persistence/RedisPersistenceLayer.ts | 306 ++++++++++++++++++ 1 file changed, 306 insertions(+) create mode 100644 packages/idempotency/src/persistence/RedisPersistenceLayer.ts diff --git a/packages/idempotency/src/persistence/RedisPersistenceLayer.ts b/packages/idempotency/src/persistence/RedisPersistenceLayer.ts new file mode 100644 index 000000000..dbaa909f1 --- /dev/null +++ b/packages/idempotency/src/persistence/RedisPersistenceLayer.ts @@ -0,0 +1,306 @@ +import { + IdempotencyItemAlreadyExistsError, + IdempotencyItemNotFoundError, + IdempotencyPersistenceConnectionError, + IdempotencyPersistenceConsistencyError, +} from '../errors.js'; +import type { + RedisClientProtocol, + RedisPersistenceOptions, +} from '../types/RedisPersistence.js'; +import type { RedisClientType, RedisClusterType } from '@redis/client'; +import type { JSONObject } from '@aws-lambda-powertools/commons/types'; +import { BasePersistenceLayer } from './BasePersistenceLayer.js'; +import { IdempotencyRecord } from './IdempotencyRecord.js'; +import { IdempotencyRecordStatus } from '../constants.js'; +import RedisConnection from './RedisConnection.js'; +import type { IdempotencyRecordStatusValue } from '../types/IdempotencyRecord.js'; + +class RedisPersistenceLayer extends BasePersistenceLayer { + readonly #client: RedisClientProtocol | RedisClientType | RedisClusterType; + readonly #dataAttr: string; + readonly #expiryAttr: string; + readonly #inProgressExpiryAttr: string; + readonly #statusAttr: string; + readonly #validationKeyAttr: string; + readonly #orphanLockTimeout: number; + + public constructor(options: RedisPersistenceOptions) { + super(); + + this.#statusAttr = options.statusAttr ?? 'status'; + this.#expiryAttr = options.expiryAttr ?? 'expiration'; + this.#inProgressExpiryAttr = + options.inProgressExpiryAttr ?? 'in_progress_expiration'; + this.#dataAttr = options.dataAttr ?? 'data'; + this.#validationKeyAttr = options.validationKeyAttr ?? 'validation'; + this.#orphanLockTimeout = Math.min(10, this.expiresAfterSeconds); + if (options.client) { + this.#client = options.client; + } else { + this.#client = new RedisConnection(options).getClient(); + } + } + + /** + * Initializes the Redis client connection. + * This method is called when the persistence layer is created. + * It checks if the client is already open, and if not, it attempts to connect. + * If the connection fails, it throws an IdempotencyPersistenceConnectionError. + */ + public async init() { + if (!this.#client.isOpen) { + try { + await (this.#client as RedisClusterType | RedisClientType).connect(); + } catch (error) { + console.error('Failed to connect to Redis:', error); + throw new IdempotencyPersistenceConnectionError( + 'Could not connect to Redis', + error as Error + ); + } + } + return this; + } + + /** + * Deletes the idempotency record associated with a given record from Redis. + * This function is designed to be called after a Lambda handler invocation has completed processing. + * It ensures that the idempotency key associated with the record is removed from Redis to + * prevent future conflicts and to maintain the idempotency integrity. + * + * Note: it is essential that the idempotency key is not empty, as that would indicate the Lambda + * handler has not been invoked or the key was not properly set. + * + * @param record + */ + protected async _deleteRecord(record: IdempotencyRecord): Promise { + console.debug( + `Deleting record for idempotency key: ${record.idempotencyKey}` + ); + await this.#client.del(record.idempotencyKey); + } + + protected async _putRecord(record: IdempotencyRecord): Promise { + if (record.getStatus() === IdempotencyRecordStatus.INPROGRESS) { + await this.#_putInProgressRecord(record); + } else { + throw new Error( + 'Only INPROGRESS records can be inserted with _putRecord' + ); + } + } + + protected async _getRecord( + idempotencyKey: string + ): Promise { + const response = await this.#client.get(idempotencyKey); + + if (response === null) { + throw new IdempotencyItemNotFoundError( + 'Item does not exist in persistence store' + ); + } + let item: JSONObject; + try { + item = JSON.parse(response); + } catch (error) { + throw new IdempotencyPersistenceConsistencyError( + 'Idempotency persistency consistency error, needs to be removed', + error as Error + ); + } + return new IdempotencyRecord({ + idempotencyKey: idempotencyKey, + status: item[this.#statusAttr] as IdempotencyRecordStatusValue, + expiryTimestamp: item[this.#expiryAttr] as number | undefined, + inProgressExpiryTimestamp: item[this.#inProgressExpiryAttr] as + | number + | undefined, + responseData: item[this.#dataAttr], + payloadHash: item[this.#validationKeyAttr] as string | undefined, + }); + } + + protected async _updateRecord(record: IdempotencyRecord): Promise { + const item: Record = { + [this.#statusAttr]: record.getStatus(), + [this.#expiryAttr]: record.expiryTimestamp, + [this.#dataAttr]: record.responseData, + }; + + const encodedItem = JSON.stringify(item); + const ttl = this.#_getExpirySeconds(record.expiryTimestamp); + // Need to set ttl again, if we don't set `EX` here the record will not have a ttl + await this.#client.set(record.idempotencyKey, encodedItem, { + EX: ttl, + }); + } + + /** + * Put a record in the persistence store with a status of "INPROGRESS". + * The method guards against concurrent execution by using Redis' conditional write operations. + */ + async #_putInProgressRecord(record: IdempotencyRecord): Promise { + const item: Record = { + [this.#statusAttr]: record.getStatus(), + [this.#expiryAttr]: record.expiryTimestamp, + }; + + if (record.inProgressExpiryTimestamp !== undefined) { + item[this.#inProgressExpiryAttr] = record.inProgressExpiryTimestamp; + } + + if (this.isPayloadValidationEnabled() && record.payloadHash !== undefined) { + item[this.#validationKeyAttr] = record.payloadHash; + } + + const encodedItem = JSON.stringify(item); + const ttl = this.#_getExpirySeconds(record.expiryTimestamp); + const now = Date.now(); + + try { + /** + * | LOCKED | RETRY if status = "INPROGRESS" | RETRY + * |----------------|-------------------------------------------------------|-------------> .... (time) + * | Lambda Idempotency Record + * | Timeout Timeout + * | (in_progress_expiry) (expiry) + * + * Conditions to successfully save a record: + * * The idempotency key does not exist: + * - first time that this invocation key is used + * - previous invocation with the same key was deleted due to TTL + * - SET see https://redis.io/commands/set/ + */ + + console.debug( + `Putting record for idempotency key: ${record.idempotencyKey}` + ); + const response = await this.#client.set( + record.idempotencyKey, + encodedItem, + { + EX: ttl, + NX: true, + } + ); + + /** + * If response is not `null`, the redis SET operation was successful and the idempotency key was not + * previously set. This indicates that we can safely proceed to the handler execution phase. + * Most invocations should successfully proceed past this point. + */ + if (response !== null) { + return; + } + + /** + * If response is `null`, it indicates an existing record in Redis for the given idempotency key. + * This could be due to: + * - An active idempotency record from a previous invocation that has not yet expired. + * - An orphan record where a previous invocation has timed out. + * - An expired idempotency record that has not been deleted by Redis. + * + * In any case, we proceed to retrieve the record for further inspection. + */ + const existingRecord = await this._getRecord(record.idempotencyKey); + + /** If the status of the idempotency record is `COMPLETED` and the record has not expired + * (i.e., the expiry timestamp is greater than the current timestamp), then a valid completed + * record exists. We raise an error to prevent duplicate processing of a request that has already + * been completed successfully. + */ + if ( + existingRecord.getStatus() === IdempotencyRecordStatus.COMPLETED && + !existingRecord.isExpired() + ) { + throw new IdempotencyItemAlreadyExistsError( + `Failed to put record for already existing idempotency key: ${record.idempotencyKey}`, + existingRecord + ); + } + + /** If the idempotency record has a status of 'INPROGRESS' and has a valid `inProgressExpiryTimestamp` + * (meaning the timestamp is greater than the current timestamp in milliseconds), then we have encountered + * a valid in-progress record. This indicates that another process is currently handling the request, and + * to maintain idempotency, we raise an error to prevent concurrent processing of the same request. + */ + if ( + existingRecord.getStatus() === IdempotencyRecordStatus.INPROGRESS && + existingRecord.inProgressExpiryTimestamp && + existingRecord.inProgressExpiryTimestamp > now + ) { + throw new IdempotencyItemAlreadyExistsError( + `Failed to put record for in-progress idempotency key: ${record.idempotencyKey}`, + existingRecord + ); + } + + /** Reaching this point indicates that the idempotency record found is an orphan record. An orphan record is + * one that is neither completed nor in-progress within its expected time frame. It may result from a + * previous invocation that has timed out or an expired record that has yet to be cleaned up by Redis. + * We raise an error to handle this exceptional scenario appropriately. + */ + throw new IdempotencyPersistenceConsistencyError( + 'Orphaned record detected' + ); + } catch (error) { + if (error instanceof IdempotencyPersistenceConsistencyError) { + /** Handle an orphan record by attempting to acquire a lock, which by default lasts for 10 seconds. + * The purpose of acquiring the lock is to prevent race conditions with other processes that might + * also be trying to handle the same orphan record. Once the lock is acquired, we set a new value + * for the idempotency record in Redis with the appropriate time-to-live (TTL). + */ + await this.#_acquireLock(record.idempotencyKey); + + console.debug('Lock acquired, updating record'); + await this.#client.set(record.idempotencyKey, encodedItem, { + EX: ttl, + }); + } else { + throw error; + } + } + } + + /** + * Calculates the number of seconds remaining until a specified expiry timestamp + */ + #_getExpirySeconds(expiryTimestamp?: number): number { + if (expiryTimestamp) { + return expiryTimestamp - Math.floor(Date.now() / 1000); + } + return this.getExpiresAfterSeconds(); + } + + /** + * Attempt to acquire a lock for a specified resource name, with a default timeout. + * This method attempts to set a lock using Redis to prevent concurrent + * access to a resource identified by 'idempotencyKey'. It uses the 'NX' flag to ensure that + * the lock is only set if it does not already exist, thereby enforcing mutual exclusion. + * + * @param idempotencyKey - The key to create a lock for + */ + async #_acquireLock(idempotencyKey: string): Promise { + const lockKey = `${idempotencyKey}:lock`; + const lockValue = 'true'; + + console.debug('Acquiring lock to overwrite orphan record'); + const acquired = await this.#client.set(lockKey, lockValue, { + EX: this.#orphanLockTimeout, + NX: true, + }); + + if (acquired) return; + // If the lock acquisition fails, it suggests a race condition has occurred. In this case, instead of + // proceeding, we log the event and raise an error to indicate that the current operation should be + // retried after the lock is released by the process that currently holds it. + console.debug('Lock acquisition failed, raise to retry'); + throw new IdempotencyItemAlreadyExistsError( + 'Lock acquisition failed, raise to retry' + ); + } +} + +export { RedisPersistenceLayer }; From d687674e144d13ff574447e47974dfe0711795a1 Mon Sep 17 00:00:00 2001 From: arnabrahman Date: Sat, 3 May 2025 18:58:37 +0600 Subject: [PATCH 08/51] test: unit tests for `RedisConnection` --- .../unit/persistence/RedisConnection.test.ts | 127 ++++++++++++++++++ 1 file changed, 127 insertions(+) create mode 100644 packages/idempotency/tests/unit/persistence/RedisConnection.test.ts diff --git a/packages/idempotency/tests/unit/persistence/RedisConnection.test.ts b/packages/idempotency/tests/unit/persistence/RedisConnection.test.ts new file mode 100644 index 000000000..d7b99c389 --- /dev/null +++ b/packages/idempotency/tests/unit/persistence/RedisConnection.test.ts @@ -0,0 +1,127 @@ +import { createClient, createCluster } from '@redis/client'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import RedisConnection from '../../../src/persistence/RedisConnection.js'; + +vi.mock('@redis/client', () => ({ + createClient: vi.fn(), + createCluster: vi.fn(), +})); + +describe('Class: RedisConnection', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + afterEach(() => { + vi.resetAllMocks(); + }); + + describe('Method: constructor', () => { + it('creates an instance with default values', () => { + // Prepare & Act + const connection = new RedisConnection({}); + + // Assess + expect(connection).toBeInstanceOf(RedisConnection); + }); + + it('creates an instance with provided values', () => { + // Prepare & Act + const connection = new RedisConnection({ + mode: 'standalone', + host: 'localhost', + port: 6379, + username: 'user', + password: 'pass', + dbIndex: 1, + ssl: false, + }); + + // Assess + expect(connection).toBeInstanceOf(RedisConnection); + }); + }); + + describe('Method: getClient', () => { + const url = 'redis://localhost:6379'; + it('creates a standalone client by default', () => { + // Prepare + const connection = new RedisConnection({}); + + // Act + connection.getClient(); + + // Assess + expect(createClient).toHaveBeenCalledTimes(1); + expect(createCluster).not.toHaveBeenCalled(); + }); + + it('creates a standalone client with URL configuration', () => { + // Prepare + const connection = new RedisConnection({ url }); + + // Act + connection.getClient(); + + // Assess + expect(createClient).toHaveBeenCalledWith({ url }); + expect(createCluster).not.toHaveBeenCalled(); + }); + + it('creates a standalone client with host and port configuration', () => { + // Prepare + const connection = new RedisConnection({ + host: 'localhost', + port: 6380, + username: 'user', + password: 'pass', + dbIndex: 2, + ssl: true, + }); + + // Act + connection.getClient(); + + // Assess + expect(createClient).toHaveBeenCalledWith({ + username: 'user', + password: 'pass', + socket: { + host: 'localhost', + port: 6380, + tls: true, + }, + database: 2, + }); + expect(createCluster).not.toHaveBeenCalled(); + }); + + it('creates a cluster client when mode is set to cluster', () => { + // Prepare + const connection = new RedisConnection({ + mode: 'cluster', + url, + }); + + // Act + connection.getClient(); + + // Assess + expect(createCluster).toHaveBeenCalledTimes(1); + expect(createCluster).toHaveBeenCalledWith({ + rootNodes: [{ url }], + }); + expect(createClient).not.toHaveBeenCalled(); + }); + + it('prioritizes URL over host/port when both are provided', () => { + const connection = new RedisConnection({ + url, + host: 'localhost', + port: 6380, + }); + connection.getClient(); + expect(createClient).toHaveBeenCalledWith({ url }); + }); + }); +}); From ea5578399734e01dc1e9f668bbf10a702c1ff8f0 Mon Sep 17 00:00:00 2001 From: arnabrahman Date: Sat, 3 May 2025 19:00:06 +0600 Subject: [PATCH 09/51] test: dummy test class for `RedisPersistenceLayer` --- .../tests/helpers/idempotencyUtils.ts | 29 ++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/packages/idempotency/tests/helpers/idempotencyUtils.ts b/packages/idempotency/tests/helpers/idempotencyUtils.ts index 3c5728181..b758481cd 100644 --- a/packages/idempotency/tests/helpers/idempotencyUtils.ts +++ b/packages/idempotency/tests/helpers/idempotencyUtils.ts @@ -2,6 +2,7 @@ import { vi } from 'vitest'; import { BasePersistenceLayer } from '../../src/persistence/BasePersistenceLayer.js'; import { DynamoDBPersistenceLayer } from '../../src/persistence/DynamoDBPersistenceLayer.js'; import type { IdempotencyRecord } from '../../src/persistence/IdempotencyRecord.js'; +import { RedisPersistenceLayer } from '../../src/persistence/RedisPersistenceLayer.js'; /** * Dummy class to test the abstract class BasePersistenceLayer. @@ -37,5 +38,31 @@ class DynamoDBPersistenceLayerTestClass extends DynamoDBPersistenceLayer { return super._updateRecord(record); } } +/** + * Dummy class to test the abstract class RedisPersistenceLayer. + * + * This class is used in the unit tests. + */ +class RedisPersistenceLayerTestClass extends RedisPersistenceLayer { + public _deleteRecord(record: IdempotencyRecord): Promise { + return super._deleteRecord(record); + } + + public _getRecord(idempotencyKey: string): Promise { + return super._getRecord(idempotencyKey); + } + + public _putRecord(_record: IdempotencyRecord): Promise { + return super._putRecord(_record); + } + + public _updateRecord(record: IdempotencyRecord): Promise { + return super._updateRecord(record); + } +} -export { PersistenceLayerTestClass, DynamoDBPersistenceLayerTestClass }; +export { + PersistenceLayerTestClass, + DynamoDBPersistenceLayerTestClass, + RedisPersistenceLayerTestClass, +}; From 499cc8d25b7e0e36b0146faa5a946da5530c9cc9 Mon Sep 17 00:00:00 2001 From: arnabrahman Date: Sat, 3 May 2025 19:38:52 +0600 Subject: [PATCH 10/51] fix: make Redis client optional in `RedisPersistenceOptions` interface --- packages/idempotency/src/types/RedisPersistence.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/idempotency/src/types/RedisPersistence.ts b/packages/idempotency/src/types/RedisPersistence.ts index 8c1873191..b6b930dc7 100644 --- a/packages/idempotency/src/types/RedisPersistence.ts +++ b/packages/idempotency/src/types/RedisPersistence.ts @@ -5,7 +5,6 @@ import type { JSONValue } from '@aws-lambda-powertools/commons/types'; * This ensures standardization among different Redis client implementations. */ interface RedisClientProtocol { - /** * Indicates whether the connection to the Redis server is currently open and ready for commands. * This can be used to check the connection status before sending commands. @@ -93,7 +92,7 @@ interface RedisPersistenceOptions extends RedisConnectionConfig { * A Redis client that implements the RedisClientProtocol interface. * If provided, all other connection configuration options will be ignored. */ - client: RedisClientProtocol; + client?: RedisClientProtocol; /** * Redis JSON attribute name for expiry timestamp (default: 'expiration') From 271617c04ab21af01fad2013b47a772164413dc7 Mon Sep 17 00:00:00 2001 From: arnabrahman Date: Sat, 3 May 2025 19:56:53 +0600 Subject: [PATCH 11/51] refactor: update Redis client connection initialization and add client type check --- .../src/persistence/RedisPersistenceLayer.ts | 38 ++++++++++++++----- .../idempotency/src/types/RedisPersistence.ts | 5 --- 2 files changed, 28 insertions(+), 15 deletions(-) diff --git a/packages/idempotency/src/persistence/RedisPersistenceLayer.ts b/packages/idempotency/src/persistence/RedisPersistenceLayer.ts index dbaa909f1..59c8ce98e 100644 --- a/packages/idempotency/src/persistence/RedisPersistenceLayer.ts +++ b/packages/idempotency/src/persistence/RedisPersistenceLayer.ts @@ -1,20 +1,20 @@ +import type { JSONObject } from '@aws-lambda-powertools/commons/types'; +import type { RedisClientType, RedisClusterType } from '@redis/client'; +import { IdempotencyRecordStatus } from '../constants.js'; import { IdempotencyItemAlreadyExistsError, IdempotencyItemNotFoundError, IdempotencyPersistenceConnectionError, IdempotencyPersistenceConsistencyError, } from '../errors.js'; +import type { IdempotencyRecordStatusValue } from '../types/IdempotencyRecord.js'; import type { RedisClientProtocol, RedisPersistenceOptions, } from '../types/RedisPersistence.js'; -import type { RedisClientType, RedisClusterType } from '@redis/client'; -import type { JSONObject } from '@aws-lambda-powertools/commons/types'; import { BasePersistenceLayer } from './BasePersistenceLayer.js'; import { IdempotencyRecord } from './IdempotencyRecord.js'; -import { IdempotencyRecordStatus } from '../constants.js'; import RedisConnection from './RedisConnection.js'; -import type { IdempotencyRecordStatusValue } from '../types/IdempotencyRecord.js'; class RedisPersistenceLayer extends BasePersistenceLayer { readonly #client: RedisClientProtocol | RedisClientType | RedisClusterType; @@ -43,15 +43,18 @@ class RedisPersistenceLayer extends BasePersistenceLayer { } /** - * Initializes the Redis client connection. - * This method is called when the persistence layer is created. - * It checks if the client is already open, and if not, it attempts to connect. - * If the connection fails, it throws an IdempotencyPersistenceConnectionError. + * Initialize Redis client connection if default client is being used. + * + * This method establishes a connection to the Redis server when using the default Redis client. + * For custom clients, this method performs no connection initialization as that's expected to be + * handled externally. + * + * @throws {IdempotencyPersistenceConnectionError} When connection to Redis fails */ public async init() { - if (!this.#client.isOpen) { + if (this.#isDefaultRedisClient(this.#client)) { try { - await (this.#client as RedisClusterType | RedisClientType).connect(); + await this.#client.connect(); } catch (error) { console.error('Failed to connect to Redis:', error); throw new IdempotencyPersistenceConnectionError( @@ -301,6 +304,21 @@ class RedisPersistenceLayer extends BasePersistenceLayer { 'Lock acquisition failed, raise to retry' ); } + + /** + * Determines if the provided Redis client is a default Redis client. + * + * This method checks if the provided client is an instance of the default Redis client + * by verifying the presence of the 'isOpen' property, which is specific to + * RedisClientType or RedisClusterType from the @redis/client package. + * + * @param client - The Redis client to check + */ + #isDefaultRedisClient( + client: RedisClientProtocol | RedisClientType | RedisClusterType + ): client is RedisClientType | RedisClusterType { + return 'isOpen' in client; + } } export { RedisPersistenceLayer }; diff --git a/packages/idempotency/src/types/RedisPersistence.ts b/packages/idempotency/src/types/RedisPersistence.ts index b6b930dc7..b8effef63 100644 --- a/packages/idempotency/src/types/RedisPersistence.ts +++ b/packages/idempotency/src/types/RedisPersistence.ts @@ -5,11 +5,6 @@ import type { JSONValue } from '@aws-lambda-powertools/commons/types'; * This ensures standardization among different Redis client implementations. */ interface RedisClientProtocol { - /** - * Indicates whether the connection to the Redis server is currently open and ready for commands. - * This can be used to check the connection status before sending commands. - */ - isOpen: boolean; /** * Retrieves the value associated with the given key. * @param name The key to get the value for From 46e25a21155a7742353f7f40a9cde57480c341d1 Mon Sep 17 00:00:00 2001 From: arnabrahman Date: Sat, 3 May 2025 20:12:35 +0600 Subject: [PATCH 12/51] fix: ensure Redis connection is only initialized if the default client is not open --- .../src/persistence/RedisPersistenceLayer.ts | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/packages/idempotency/src/persistence/RedisPersistenceLayer.ts b/packages/idempotency/src/persistence/RedisPersistenceLayer.ts index 59c8ce98e..b4d5b96aa 100644 --- a/packages/idempotency/src/persistence/RedisPersistenceLayer.ts +++ b/packages/idempotency/src/persistence/RedisPersistenceLayer.ts @@ -43,16 +43,18 @@ class RedisPersistenceLayer extends BasePersistenceLayer { } /** - * Initialize Redis client connection if default client is being used. + * Initializes the Redis connection if it's the default Redis client and not already open. * - * This method establishes a connection to the Redis server when using the default Redis client. - * For custom clients, this method performs no connection initialization as that's expected to be - * handled externally. + * This method attempts to connect to Redis if necessary. If using a custom Redis client, + * it assumes the client is already initialized. * - * @throws {IdempotencyPersistenceConnectionError} When connection to Redis fails + * @throws {IdempotencyPersistenceConnectionError} When the connection to Redis fails */ public async init() { - if (this.#isDefaultRedisClient(this.#client)) { + if ( + this.#isDefaultRedisClient(this.#client) && + this.#client.isOpen === false + ) { try { await this.#client.connect(); } catch (error) { From 26870f41b7405fc880043ba53f18fe7ec76562d5 Mon Sep 17 00:00:00 2001 From: arnabrahman Date: Sat, 3 May 2025 20:50:27 +0600 Subject: [PATCH 13/51] test: add unit tests for `init` method of `RedisPersistenceLayer` --- .../persistence/RedisPersistenceLayer.test.ts | 118 ++++++++++++++++++ 1 file changed, 118 insertions(+) create mode 100644 packages/idempotency/tests/unit/persistence/RedisPersistenceLayer.test.ts diff --git a/packages/idempotency/tests/unit/persistence/RedisPersistenceLayer.test.ts b/packages/idempotency/tests/unit/persistence/RedisPersistenceLayer.test.ts new file mode 100644 index 000000000..94948349d --- /dev/null +++ b/packages/idempotency/tests/unit/persistence/RedisPersistenceLayer.test.ts @@ -0,0 +1,118 @@ +import { + type Mock, + afterEach, + beforeEach, + describe, + expect, + it, + vi, +} from 'vitest'; +import { IdempotencyPersistenceConnectionError } from '../../../src/errors.js'; +import RedisConnection from '../../../src/persistence/RedisConnection.js'; +import { RedisPersistenceLayer } from '../../../src/persistence/RedisPersistenceLayer.js'; +import type { RedisClientProtocol } from '../../../src/types/RedisPersistence.js'; + +vi.mock('../../../src/persistence/RedisConnection.js'); + +describe('Class: RedisPersistenceLayer', () => { + const mockDefaultClientConnect = vi.fn().mockResolvedValue(undefined); + + const mockDefaultClient = { + isOpen: false, + connect: mockDefaultClientConnect, + }; + + const mockUserProvidedClient: RedisClientProtocol = { + get: vi.fn(), + set: vi.fn(), + del: vi.fn(), + }; + + beforeEach(() => { + vi.clearAllMocks(); + + (RedisConnection as unknown as Mock).mockImplementation(() => ({ + getClient: vi.fn().mockReturnValue(mockDefaultClient), + })); + }); + + afterEach(() => { + vi.clearAllMocks(); + }); + + describe('Method: init', () => { + it('should create a Redis connection and connect when no client is provided', async () => { + // Prepare + const layer = new RedisPersistenceLayer({}); + + // Act + await layer.init(); + + // Assess + expect(RedisConnection).toHaveBeenCalledTimes(1); + expect(mockDefaultClientConnect).toHaveBeenCalledTimes(1); + }); + + it('should throw IdempotencyPersistenceConnectionError when connection fails', async () => { + // Prepare + mockDefaultClientConnect.mockRejectedValueOnce( + new Error('Connection failed') + ); + const layer = new RedisPersistenceLayer({}); + const consoleErrorSpy = vi + .spyOn(console, 'error') + .mockImplementation(() => {}); + + // Act & Assess + await expect(layer.init()).rejects.toThrow( + new IdempotencyPersistenceConnectionError( + 'Could not connect to Redis', + new Error('Connection failed') + ) + ); + expect(mockDefaultClientConnect).toHaveBeenCalledTimes(1); + expect(consoleErrorSpy).toHaveBeenCalledWith( + 'Failed to connect to Redis:', + expect.any(Error) + ); + + // Cleanup + consoleErrorSpy.mockRestore(); + }); + + it('should not attempt to connect if the client is already connected', async () => { + // Prepare + const connectedClient = { + isOpen: true, + connect: vi.fn().mockResolvedValue(undefined), + }; + + (RedisConnection as unknown as Mock).mockImplementation(() => ({ + getClient: vi.fn().mockReturnValue(connectedClient), + })); + + const layer = new RedisPersistenceLayer({}); + + // Act + await layer.init(); + + // Assess + expect(RedisConnection).toHaveBeenCalledTimes(1); + expect(connectedClient.connect).not.toHaveBeenCalled(); + }); + + it('will do nothing if a user-provided client is used', async () => { + // Prepare + const layer = new RedisPersistenceLayer({ + client: mockUserProvidedClient, + }); + + // Act + await layer.init(); + + // Assess + expect(RedisConnection).not.toHaveBeenCalled(); + expect(mockDefaultClientConnect).not.toHaveBeenCalled(); + }); + }); +}); From 479f3830422ecd7028fdff11d50ddb5e0d53bff2 Mon Sep 17 00:00:00 2001 From: arnabrahman Date: Sun, 4 May 2025 10:35:45 +0600 Subject: [PATCH 14/51] test: ` _putRecord` unit tests --- .../persistence/RedisPersistenceLayer.test.ts | 98 +++++++++++++++++-- 1 file changed, 91 insertions(+), 7 deletions(-) diff --git a/packages/idempotency/tests/unit/persistence/RedisPersistenceLayer.test.ts b/packages/idempotency/tests/unit/persistence/RedisPersistenceLayer.test.ts index 94948349d..c3c4a4068 100644 --- a/packages/idempotency/tests/unit/persistence/RedisPersistenceLayer.test.ts +++ b/packages/idempotency/tests/unit/persistence/RedisPersistenceLayer.test.ts @@ -7,19 +7,27 @@ import { it, vi, } from 'vitest'; +import { IdempotencyRecordStatus } from '../../../src/constants.js'; import { IdempotencyPersistenceConnectionError } from '../../../src/errors.js'; +import { IdempotencyRecord } from '../../../src/persistence/IdempotencyRecord.js'; import RedisConnection from '../../../src/persistence/RedisConnection.js'; -import { RedisPersistenceLayer } from '../../../src/persistence/RedisPersistenceLayer.js'; import type { RedisClientProtocol } from '../../../src/types/RedisPersistence.js'; +import { RedisPersistenceLayerTestClass } from '../../helpers/idempotencyUtils.js'; vi.mock('../../../src/persistence/RedisConnection.js'); -describe('Class: RedisPersistenceLayer', () => { - const mockDefaultClientConnect = vi.fn().mockResolvedValue(undefined); +const getFutureTimestamp = (seconds: number): number => + Math.floor(Date.now() / 1000) + seconds; + +describe('Class: RedisPersistenceLayerTestClass', () => { + const mockDefaultClientConnect = vi.fn().mockResolvedValue(true); const mockDefaultClient = { isOpen: false, connect: mockDefaultClientConnect, + set: vi.fn(), + get: vi.fn(), + del: vi.fn().mockResolvedValue(1), }; const mockUserProvidedClient: RedisClientProtocol = { @@ -43,7 +51,7 @@ describe('Class: RedisPersistenceLayer', () => { describe('Method: init', () => { it('should create a Redis connection and connect when no client is provided', async () => { // Prepare - const layer = new RedisPersistenceLayer({}); + const layer = new RedisPersistenceLayerTestClass({}); // Act await layer.init(); @@ -58,7 +66,7 @@ describe('Class: RedisPersistenceLayer', () => { mockDefaultClientConnect.mockRejectedValueOnce( new Error('Connection failed') ); - const layer = new RedisPersistenceLayer({}); + const layer = new RedisPersistenceLayerTestClass({}); const consoleErrorSpy = vi .spyOn(console, 'error') .mockImplementation(() => {}); @@ -91,7 +99,7 @@ describe('Class: RedisPersistenceLayer', () => { getClient: vi.fn().mockReturnValue(connectedClient), })); - const layer = new RedisPersistenceLayer({}); + const layer = new RedisPersistenceLayerTestClass({}); // Act await layer.init(); @@ -103,7 +111,7 @@ describe('Class: RedisPersistenceLayer', () => { it('will do nothing if a user-provided client is used', async () => { // Prepare - const layer = new RedisPersistenceLayer({ + const layer = new RedisPersistenceLayerTestClass({ client: mockUserProvidedClient, }); @@ -115,4 +123,80 @@ describe('Class: RedisPersistenceLayer', () => { expect(mockDefaultClientConnect).not.toHaveBeenCalled(); }); }); + + describe('Method: _putRecord', () => { + const dummyKey = 'someKey'; + + describe('when a user-provided client is used', () => { + it('puts a record with INPROGRESS status into Redis', async () => { + // Prepare + const layer = new RedisPersistenceLayerTestClass({}); + await layer.init(); + const record = new IdempotencyRecord({ + idempotencyKey: dummyKey, + status: IdempotencyRecordStatus.INPROGRESS, + expiryTimestamp: getFutureTimestamp(10), + }); + mockDefaultClient.set.mockResolvedValue('OK'); + + // Act + await layer._putRecord(record); + + // Assess + expect(mockDefaultClient.set).toHaveBeenCalledWith( + dummyKey, + JSON.stringify({ + status: IdempotencyRecordStatus.INPROGRESS, + expiration: record.expiryTimestamp, + }), + expect.objectContaining({ EX: expect.any(Number), NX: true }) + ); + }); + + it('puts the record in Redis when using an in progress expiry timestamp', async () => { + // Prepare + const layer = new RedisPersistenceLayerTestClass({}); + await layer.init(); + const status = IdempotencyRecordStatus.INPROGRESS; + const expiryTimestamp = getFutureTimestamp(10); + const inProgressExpiryTimestamp = getFutureTimestamp(5); + const record = new IdempotencyRecord({ + idempotencyKey: dummyKey, + status, + expiryTimestamp, + inProgressExpiryTimestamp, + }); + + // Act + await layer._putRecord(record); + + // Assess + expect(mockDefaultClient.set).toHaveBeenCalledWith( + dummyKey, + JSON.stringify({ + status, + expiration: expiryTimestamp, + in_progress_expiration: inProgressExpiryTimestamp, + }), + { EX: 10, NX: true } + ); + }); + + it('throws error when trying to put a non-INPROGRESS record', async () => { + // Prepare + const layer = new RedisPersistenceLayerTestClass({}); + await layer.init(); + const record = new IdempotencyRecord({ + idempotencyKey: dummyKey, + status: IdempotencyRecordStatus.COMPLETED, + expiryTimestamp: getFutureTimestamp(10), + }); + + // Act & Assess + await expect(layer._putRecord(record)).rejects.toThrow( + 'Only INPROGRESS records can be inserted with _putRecord' + ); + }); + }); + }); }); From f7022935995218664485dee4d8942ebe15ea0643 Mon Sep 17 00:00:00 2001 From: arnabrahman Date: Sun, 4 May 2025 10:43:58 +0600 Subject: [PATCH 15/51] test: payload hash test case for `putRecord` --- .../persistence/RedisPersistenceLayer.test.ts | 33 ++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-) diff --git a/packages/idempotency/tests/unit/persistence/RedisPersistenceLayer.test.ts b/packages/idempotency/tests/unit/persistence/RedisPersistenceLayer.test.ts index c3c4a4068..7b9be5510 100644 --- a/packages/idempotency/tests/unit/persistence/RedisPersistenceLayer.test.ts +++ b/packages/idempotency/tests/unit/persistence/RedisPersistenceLayer.test.ts @@ -149,7 +149,7 @@ describe('Class: RedisPersistenceLayerTestClass', () => { status: IdempotencyRecordStatus.INPROGRESS, expiration: record.expiryTimestamp, }), - expect.objectContaining({ EX: expect.any(Number), NX: true }) + { EX: 10, NX: true } ); }); @@ -182,6 +182,37 @@ describe('Class: RedisPersistenceLayerTestClass', () => { ); }); + it('puts record in Redis when using payload validation', async () => { + // Prepare + const layer = new RedisPersistenceLayerTestClass({}); + await layer.init(); + const persistenceLayerSpy = vi + .spyOn(layer, 'isPayloadValidationEnabled') + .mockReturnValue(true); + const expiryTimestamp = getFutureTimestamp(10); + const record = new IdempotencyRecord({ + idempotencyKey: dummyKey, + status: IdempotencyRecordStatus.INPROGRESS, + expiryTimestamp, + payloadHash: 'someHash', + }); + + // Act + await layer._putRecord(record); + + // Assess + expect(mockDefaultClient.set).toHaveBeenCalledWith( + dummyKey, + JSON.stringify({ + status: IdempotencyRecordStatus.INPROGRESS, + expiration: expiryTimestamp, + validation: 'someHash', + }), + { EX: 10, NX: true } + ); + persistenceLayerSpy.mockRestore(); + }); + it('throws error when trying to put a non-INPROGRESS record', async () => { // Prepare const layer = new RedisPersistenceLayerTestClass({}); From 955b273a7a73186119bc802c07f9ba66a03ee2f1 Mon Sep 17 00:00:00 2001 From: arnabrahman Date: Sun, 4 May 2025 10:56:17 +0600 Subject: [PATCH 16/51] test: add unit test for `_putRecord` method to verify record insertion with default expiry --- .../persistence/RedisPersistenceLayer.test.ts | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/packages/idempotency/tests/unit/persistence/RedisPersistenceLayer.test.ts b/packages/idempotency/tests/unit/persistence/RedisPersistenceLayer.test.ts index 7b9be5510..94e283413 100644 --- a/packages/idempotency/tests/unit/persistence/RedisPersistenceLayer.test.ts +++ b/packages/idempotency/tests/unit/persistence/RedisPersistenceLayer.test.ts @@ -213,6 +213,29 @@ describe('Class: RedisPersistenceLayerTestClass', () => { persistenceLayerSpy.mockRestore(); }); + it('puts record in Redis with default expiry timestamp', async () => { + // Prepare + const layer = new RedisPersistenceLayerTestClass({}); + await layer.init(); + const status = IdempotencyRecordStatus.INPROGRESS; + const record = new IdempotencyRecord({ + idempotencyKey: dummyKey, + status, + }); + + // Act + await layer._putRecord(record); + + // Assess + expect(mockDefaultClient.set).toHaveBeenCalledWith( + dummyKey, + JSON.stringify({ + status, + }), + { EX: 60 * 60, NX: true } + ); + }); + it('throws error when trying to put a non-INPROGRESS record', async () => { // Prepare const layer = new RedisPersistenceLayerTestClass({}); From e45492d41ed99d1cbabb0e25ca130cd7672498bb Mon Sep 17 00:00:00 2001 From: arnabrahman Date: Sun, 4 May 2025 12:21:00 +0600 Subject: [PATCH 17/51] fix: correct expiry time calculation in `_getExpirySeconds` method --- packages/idempotency/src/persistence/RedisPersistenceLayer.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/idempotency/src/persistence/RedisPersistenceLayer.ts b/packages/idempotency/src/persistence/RedisPersistenceLayer.ts index b4d5b96aa..c03e1d879 100644 --- a/packages/idempotency/src/persistence/RedisPersistenceLayer.ts +++ b/packages/idempotency/src/persistence/RedisPersistenceLayer.ts @@ -274,7 +274,7 @@ class RedisPersistenceLayer extends BasePersistenceLayer { */ #_getExpirySeconds(expiryTimestamp?: number): number { if (expiryTimestamp) { - return expiryTimestamp - Math.floor(Date.now() / 1000); + return Math.floor((expiryTimestamp - Date.now()) / 1000); } return this.getExpiresAfterSeconds(); } From fabfb197d37562a499d98b6ab72f557aa0985187 Mon Sep 17 00:00:00 2001 From: arnabrahman Date: Sun, 4 May 2025 12:24:50 +0600 Subject: [PATCH 18/51] test: `_putRecord` function unit test --- .../persistence/RedisPersistenceLayer.test.ts | 131 +++++++++++++++++- 1 file changed, 130 insertions(+), 1 deletion(-) diff --git a/packages/idempotency/tests/unit/persistence/RedisPersistenceLayer.test.ts b/packages/idempotency/tests/unit/persistence/RedisPersistenceLayer.test.ts index 94e283413..486fb5d12 100644 --- a/packages/idempotency/tests/unit/persistence/RedisPersistenceLayer.test.ts +++ b/packages/idempotency/tests/unit/persistence/RedisPersistenceLayer.test.ts @@ -1,6 +1,8 @@ import { type Mock, + afterAll, afterEach, + beforeAll, beforeEach, describe, expect, @@ -17,7 +19,7 @@ import { RedisPersistenceLayerTestClass } from '../../helpers/idempotencyUtils.j vi.mock('../../../src/persistence/RedisConnection.js'); const getFutureTimestamp = (seconds: number): number => - Math.floor(Date.now() / 1000) + seconds; + new Date().getTime() + seconds * 1000; describe('Class: RedisPersistenceLayerTestClass', () => { const mockDefaultClientConnect = vi.fn().mockResolvedValue(true); @@ -36,6 +38,10 @@ describe('Class: RedisPersistenceLayerTestClass', () => { del: vi.fn(), }; + beforeAll(() => { + vi.useFakeTimers().setSystemTime(new Date()); + }); + beforeEach(() => { vi.clearAllMocks(); @@ -48,6 +54,10 @@ describe('Class: RedisPersistenceLayerTestClass', () => { vi.clearAllMocks(); }); + afterAll(() => { + vi.useRealTimers(); + }); + describe('Method: init', () => { it('should create a Redis connection and connect when no client is provided', async () => { // Prepare @@ -236,6 +246,125 @@ describe('Class: RedisPersistenceLayerTestClass', () => { ); }); + it('handles orphaned records by acquiring a lock and updating', async () => { + // Prepare + const layer = new RedisPersistenceLayerTestClass({}); + await layer.init(); + const record = new IdempotencyRecord({ + idempotencyKey: dummyKey, + status: IdempotencyRecordStatus.INPROGRESS, + expiryTimestamp: getFutureTimestamp(10), + }); + + mockDefaultClient.set + .mockResolvedValueOnce(null) + .mockResolvedValueOnce('OK'); + mockDefaultClient.get.mockResolvedValueOnce( + JSON.stringify({ + status: IdempotencyRecordStatus.INPROGRESS, + expiration: getFutureTimestamp(-10), + }) + ); + const consoleDebugSpy = vi.spyOn(console, 'debug'); + + // Act + await layer._putRecord(record); + + // Assess + expect(consoleDebugSpy).toHaveBeenCalledWith( + 'Acquiring lock to overwrite orphan record' + ); + expect(mockDefaultClient.set).toHaveBeenCalledWith( + `${dummyKey}:lock`, + 'true', + expect.objectContaining({ EX: 10, NX: true }) + ); + expect(consoleDebugSpy).toHaveBeenCalledWith( + 'Lock acquired, updating record' + ); + expect(mockDefaultClient.set).toHaveBeenCalledWith( + dummyKey, + JSON.stringify({ + status: IdempotencyRecordStatus.INPROGRESS, + expiration: record.expiryTimestamp, + }), + { EX: 10 } + ); + }); + + it('handles orphaned records by acquiring a lock but it fails and throw error', async () => { + // Prepare + const layer = new RedisPersistenceLayerTestClass({}); + await layer.init(); + const record = new IdempotencyRecord({ + idempotencyKey: dummyKey, + status: IdempotencyRecordStatus.INPROGRESS, + expiryTimestamp: getFutureTimestamp(10), + }); + + mockDefaultClient.set + .mockResolvedValueOnce(null) // First attempt to set fails + .mockResolvedValueOnce(null); // Lock acquisition fails + mockDefaultClient.get.mockResolvedValueOnce( + JSON.stringify({ + status: IdempotencyRecordStatus.INPROGRESS, + expiration: getFutureTimestamp(-10), + }) + ); + + // Act + await expect(layer._putRecord(record)).rejects.toThrow( + 'Lock acquisition failed, raise to retry' + ); + }); + + it('throws error when item already exists and not expired', async () => { + // Prepare + const layer = new RedisPersistenceLayerTestClass({}); + await layer.init(); + const record = new IdempotencyRecord({ + idempotencyKey: dummyKey, + status: IdempotencyRecordStatus.INPROGRESS, + expiryTimestamp: getFutureTimestamp(10), + }); + mockDefaultClient.set.mockResolvedValue(null); + mockDefaultClient.get.mockResolvedValueOnce( + JSON.stringify({ + status: IdempotencyRecordStatus.COMPLETED, + expiration: getFutureTimestamp(10), + }) + ); + + // Act & Assess + await expect(layer._putRecord(record)).rejects.toThrow( + `Failed to put record for already existing idempotency key: ${dummyKey}` + ); + }); + + it('throws error when item is in progress', async () => { + // Prepare + const layer = new RedisPersistenceLayerTestClass({}); + await layer.init(); + const inProgressExpiryTimestamp = getFutureTimestamp(10); + const record = new IdempotencyRecord({ + idempotencyKey: dummyKey, + status: IdempotencyRecordStatus.INPROGRESS, + expiryTimestamp: getFutureTimestamp(10), + }); + mockDefaultClient.set.mockResolvedValue(null); + mockDefaultClient.get.mockResolvedValueOnce( + JSON.stringify({ + status: IdempotencyRecordStatus.INPROGRESS, + in_progress_expiration: inProgressExpiryTimestamp, + }) + ); + + // Act & Assess + await expect(layer._putRecord(record)).rejects.toThrow( + `Failed to put record for in-progress idempotency key: ${dummyKey}` + ); + }); + it('throws error when trying to put a non-INPROGRESS record', async () => { // Prepare const layer = new RedisPersistenceLayerTestClass({}); From 91afcdf415f1546e85284188be2b7bae06a73e01 Mon Sep 17 00:00:00 2001 From: arnabrahman Date: Sun, 4 May 2025 12:41:41 +0600 Subject: [PATCH 19/51] test: refactor `_putRecord` tests to support both default and user-provided Redis clients --- .../persistence/RedisPersistenceLayer.test.ts | 70 +++++++++++-------- 1 file changed, 40 insertions(+), 30 deletions(-) diff --git a/packages/idempotency/tests/unit/persistence/RedisPersistenceLayer.test.ts b/packages/idempotency/tests/unit/persistence/RedisPersistenceLayer.test.ts index 486fb5d12..efc5c2e5e 100644 --- a/packages/idempotency/tests/unit/persistence/RedisPersistenceLayer.test.ts +++ b/packages/idempotency/tests/unit/persistence/RedisPersistenceLayer.test.ts @@ -13,7 +13,6 @@ import { IdempotencyRecordStatus } from '../../../src/constants.js'; import { IdempotencyPersistenceConnectionError } from '../../../src/errors.js'; import { IdempotencyRecord } from '../../../src/persistence/IdempotencyRecord.js'; import RedisConnection from '../../../src/persistence/RedisConnection.js'; -import type { RedisClientProtocol } from '../../../src/types/RedisPersistence.js'; import { RedisPersistenceLayerTestClass } from '../../helpers/idempotencyUtils.js'; vi.mock('../../../src/persistence/RedisConnection.js'); @@ -21,6 +20,8 @@ vi.mock('../../../src/persistence/RedisConnection.js'); const getFutureTimestamp = (seconds: number): number => new Date().getTime() + seconds * 1000; +const dummyKey = 'someKey'; + describe('Class: RedisPersistenceLayerTestClass', () => { const mockDefaultClientConnect = vi.fn().mockResolvedValue(true); @@ -32,7 +33,7 @@ describe('Class: RedisPersistenceLayerTestClass', () => { del: vi.fn().mockResolvedValue(1), }; - const mockUserProvidedClient: RedisClientProtocol = { + const mockUserProvidedClient = { get: vi.fn(), set: vi.fn(), del: vi.fn(), @@ -135,25 +136,36 @@ describe('Class: RedisPersistenceLayerTestClass', () => { }); describe('Method: _putRecord', () => { - const dummyKey = 'someKey'; + const testScenarios = [ + { + name: 'when default Redis client is used', + client: mockDefaultClient, + getLayerConfig: () => ({}), + }, + { + name: 'when a user-provided client is used', + client: mockUserProvidedClient, + getLayerConfig: () => ({ client: mockUserProvidedClient }), + }, + ]; - describe('when a user-provided client is used', () => { + describe.each(testScenarios)('$name', ({ client, getLayerConfig }) => { it('puts a record with INPROGRESS status into Redis', async () => { // Prepare - const layer = new RedisPersistenceLayerTestClass({}); + const layer = new RedisPersistenceLayerTestClass(getLayerConfig()); await layer.init(); const record = new IdempotencyRecord({ idempotencyKey: dummyKey, status: IdempotencyRecordStatus.INPROGRESS, expiryTimestamp: getFutureTimestamp(10), }); - mockDefaultClient.set.mockResolvedValue('OK'); + client.set.mockResolvedValue('OK'); // Act await layer._putRecord(record); // Assess - expect(mockDefaultClient.set).toHaveBeenCalledWith( + expect(client.set).toHaveBeenCalledWith( dummyKey, JSON.stringify({ status: IdempotencyRecordStatus.INPROGRESS, @@ -165,7 +177,7 @@ describe('Class: RedisPersistenceLayerTestClass', () => { it('puts the record in Redis when using an in progress expiry timestamp', async () => { // Prepare - const layer = new RedisPersistenceLayerTestClass({}); + const layer = new RedisPersistenceLayerTestClass(getLayerConfig()); await layer.init(); const status = IdempotencyRecordStatus.INPROGRESS; const expiryTimestamp = getFutureTimestamp(10); @@ -181,7 +193,7 @@ describe('Class: RedisPersistenceLayerTestClass', () => { await layer._putRecord(record); // Assess - expect(mockDefaultClient.set).toHaveBeenCalledWith( + expect(client.set).toHaveBeenCalledWith( dummyKey, JSON.stringify({ status, @@ -194,7 +206,7 @@ describe('Class: RedisPersistenceLayerTestClass', () => { it('puts record in Redis when using payload validation', async () => { // Prepare - const layer = new RedisPersistenceLayerTestClass({}); + const layer = new RedisPersistenceLayerTestClass(getLayerConfig()); await layer.init(); const persistenceLayerSpy = vi .spyOn(layer, 'isPayloadValidationEnabled') @@ -211,7 +223,7 @@ describe('Class: RedisPersistenceLayerTestClass', () => { await layer._putRecord(record); // Assess - expect(mockDefaultClient.set).toHaveBeenCalledWith( + expect(client.set).toHaveBeenCalledWith( dummyKey, JSON.stringify({ status: IdempotencyRecordStatus.INPROGRESS, @@ -225,7 +237,7 @@ describe('Class: RedisPersistenceLayerTestClass', () => { it('puts record in Redis with default expiry timestamp', async () => { // Prepare - const layer = new RedisPersistenceLayerTestClass({}); + const layer = new RedisPersistenceLayerTestClass(getLayerConfig()); await layer.init(); const status = IdempotencyRecordStatus.INPROGRESS; const record = new IdempotencyRecord({ @@ -237,7 +249,7 @@ describe('Class: RedisPersistenceLayerTestClass', () => { await layer._putRecord(record); // Assess - expect(mockDefaultClient.set).toHaveBeenCalledWith( + expect(client.set).toHaveBeenCalledWith( dummyKey, JSON.stringify({ status, @@ -248,7 +260,7 @@ describe('Class: RedisPersistenceLayerTestClass', () => { it('handles orphaned records by acquiring a lock and updating', async () => { // Prepare - const layer = new RedisPersistenceLayerTestClass({}); + const layer = new RedisPersistenceLayerTestClass(getLayerConfig()); await layer.init(); const record = new IdempotencyRecord({ idempotencyKey: dummyKey, @@ -256,10 +268,8 @@ describe('Class: RedisPersistenceLayerTestClass', () => { expiryTimestamp: getFutureTimestamp(10), }); - mockDefaultClient.set - .mockResolvedValueOnce(null) - .mockResolvedValueOnce('OK'); - mockDefaultClient.get.mockResolvedValueOnce( + client.set.mockResolvedValueOnce(null).mockResolvedValueOnce('OK'); + client.get.mockResolvedValueOnce( JSON.stringify({ status: IdempotencyRecordStatus.INPROGRESS, expiration: getFutureTimestamp(-10), @@ -274,7 +284,7 @@ describe('Class: RedisPersistenceLayerTestClass', () => { expect(consoleDebugSpy).toHaveBeenCalledWith( 'Acquiring lock to overwrite orphan record' ); - expect(mockDefaultClient.set).toHaveBeenCalledWith( + expect(client.set).toHaveBeenCalledWith( `${dummyKey}:lock`, 'true', expect.objectContaining({ EX: 10, NX: true }) @@ -282,7 +292,7 @@ describe('Class: RedisPersistenceLayerTestClass', () => { expect(consoleDebugSpy).toHaveBeenCalledWith( 'Lock acquired, updating record' ); - expect(mockDefaultClient.set).toHaveBeenCalledWith( + expect(client.set).toHaveBeenCalledWith( dummyKey, JSON.stringify({ status: IdempotencyRecordStatus.INPROGRESS, @@ -294,7 +304,7 @@ describe('Class: RedisPersistenceLayerTestClass', () => { it('handles orphaned records by acquiring a lock but it fails and throw error', async () => { // Prepare - const layer = new RedisPersistenceLayerTestClass({}); + const layer = new RedisPersistenceLayerTestClass(getLayerConfig()); await layer.init(); const record = new IdempotencyRecord({ idempotencyKey: dummyKey, @@ -302,10 +312,10 @@ describe('Class: RedisPersistenceLayerTestClass', () => { expiryTimestamp: getFutureTimestamp(10), }); - mockDefaultClient.set + client.set .mockResolvedValueOnce(null) // First attempt to set fails .mockResolvedValueOnce(null); // Lock acquisition fails - mockDefaultClient.get.mockResolvedValueOnce( + client.get.mockResolvedValueOnce( JSON.stringify({ status: IdempotencyRecordStatus.INPROGRESS, expiration: getFutureTimestamp(-10), @@ -320,15 +330,15 @@ describe('Class: RedisPersistenceLayerTestClass', () => { it('throws error when item already exists and not expired', async () => { // Prepare - const layer = new RedisPersistenceLayerTestClass({}); + const layer = new RedisPersistenceLayerTestClass(getLayerConfig()); await layer.init(); const record = new IdempotencyRecord({ idempotencyKey: dummyKey, status: IdempotencyRecordStatus.INPROGRESS, expiryTimestamp: getFutureTimestamp(10), }); - mockDefaultClient.set.mockResolvedValue(null); - mockDefaultClient.get.mockResolvedValueOnce( + client.set.mockResolvedValue(null); + client.get.mockResolvedValueOnce( JSON.stringify({ status: IdempotencyRecordStatus.COMPLETED, expiration: getFutureTimestamp(10), @@ -343,7 +353,7 @@ describe('Class: RedisPersistenceLayerTestClass', () => { it('throws error when item is in progress', async () => { // Prepare - const layer = new RedisPersistenceLayerTestClass({}); + const layer = new RedisPersistenceLayerTestClass(getLayerConfig()); await layer.init(); const inProgressExpiryTimestamp = getFutureTimestamp(10); const record = new IdempotencyRecord({ @@ -351,8 +361,8 @@ describe('Class: RedisPersistenceLayerTestClass', () => { status: IdempotencyRecordStatus.INPROGRESS, expiryTimestamp: getFutureTimestamp(10), }); - mockDefaultClient.set.mockResolvedValue(null); - mockDefaultClient.get.mockResolvedValueOnce( + client.set.mockResolvedValue(null); + client.get.mockResolvedValueOnce( JSON.stringify({ status: IdempotencyRecordStatus.INPROGRESS, in_progress_expiration: inProgressExpiryTimestamp, @@ -367,7 +377,7 @@ describe('Class: RedisPersistenceLayerTestClass', () => { it('throws error when trying to put a non-INPROGRESS record', async () => { // Prepare - const layer = new RedisPersistenceLayerTestClass({}); + const layer = new RedisPersistenceLayerTestClass(getLayerConfig()); await layer.init(); const record = new IdempotencyRecord({ idempotencyKey: dummyKey, From 92a186b3338948088001601423a400c79908f46f Mon Sep 17 00:00:00 2001 From: arnabrahman Date: Sun, 4 May 2025 12:44:35 +0600 Subject: [PATCH 20/51] refactor: simplify test scenarios for `_putRecord` method in RedisPersistenceLayer tests --- .../persistence/RedisPersistenceLayer.test.ts | 28 +++++++++---------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/packages/idempotency/tests/unit/persistence/RedisPersistenceLayer.test.ts b/packages/idempotency/tests/unit/persistence/RedisPersistenceLayer.test.ts index efc5c2e5e..7a684e26e 100644 --- a/packages/idempotency/tests/unit/persistence/RedisPersistenceLayer.test.ts +++ b/packages/idempotency/tests/unit/persistence/RedisPersistenceLayer.test.ts @@ -135,21 +135,19 @@ describe('Class: RedisPersistenceLayerTestClass', () => { }); }); - describe('Method: _putRecord', () => { - const testScenarios = [ - { - name: 'when default Redis client is used', - client: mockDefaultClient, - getLayerConfig: () => ({}), - }, - { - name: 'when a user-provided client is used', - client: mockUserProvidedClient, - getLayerConfig: () => ({ client: mockUserProvidedClient }), - }, - ]; - - describe.each(testScenarios)('$name', ({ client, getLayerConfig }) => { + describe.each([ + { + name: 'when default Redis client is used', + client: mockDefaultClient, + getLayerConfig: () => ({}), + }, + { + name: 'when a user-provided client is used', + client: mockUserProvidedClient, + getLayerConfig: () => ({ client: mockUserProvidedClient }), + }, + ])('$name', ({ client, getLayerConfig }) => { + describe('Method: _putRecord', () => { it('puts a record with INPROGRESS status into Redis', async () => { // Prepare const layer = new RedisPersistenceLayerTestClass(getLayerConfig()); From 952c451e9e03632b72cc081b3960a8728a6dda60 Mon Sep 17 00:00:00 2001 From: arnabrahman Date: Sun, 4 May 2025 12:55:39 +0600 Subject: [PATCH 21/51] test: refactor `_putRecord` tests to initialize layer in beforeEach and add `_deleteRecord` test --- .../persistence/RedisPersistenceLayer.test.ts | 48 +++++++++++-------- 1 file changed, 29 insertions(+), 19 deletions(-) diff --git a/packages/idempotency/tests/unit/persistence/RedisPersistenceLayer.test.ts b/packages/idempotency/tests/unit/persistence/RedisPersistenceLayer.test.ts index 7a684e26e..a3b73eaf2 100644 --- a/packages/idempotency/tests/unit/persistence/RedisPersistenceLayer.test.ts +++ b/packages/idempotency/tests/unit/persistence/RedisPersistenceLayer.test.ts @@ -147,11 +147,16 @@ describe('Class: RedisPersistenceLayerTestClass', () => { getLayerConfig: () => ({ client: mockUserProvidedClient }), }, ])('$name', ({ client, getLayerConfig }) => { + let layer: RedisPersistenceLayerTestClass; + + beforeEach(async () => { + layer = new RedisPersistenceLayerTestClass(getLayerConfig()); + await layer.init(); + }); + describe('Method: _putRecord', () => { it('puts a record with INPROGRESS status into Redis', async () => { // Prepare - const layer = new RedisPersistenceLayerTestClass(getLayerConfig()); - await layer.init(); const record = new IdempotencyRecord({ idempotencyKey: dummyKey, status: IdempotencyRecordStatus.INPROGRESS, @@ -175,8 +180,6 @@ describe('Class: RedisPersistenceLayerTestClass', () => { it('puts the record in Redis when using an in progress expiry timestamp', async () => { // Prepare - const layer = new RedisPersistenceLayerTestClass(getLayerConfig()); - await layer.init(); const status = IdempotencyRecordStatus.INPROGRESS; const expiryTimestamp = getFutureTimestamp(10); const inProgressExpiryTimestamp = getFutureTimestamp(5); @@ -204,8 +207,6 @@ describe('Class: RedisPersistenceLayerTestClass', () => { it('puts record in Redis when using payload validation', async () => { // Prepare - const layer = new RedisPersistenceLayerTestClass(getLayerConfig()); - await layer.init(); const persistenceLayerSpy = vi .spyOn(layer, 'isPayloadValidationEnabled') .mockReturnValue(true); @@ -235,8 +236,6 @@ describe('Class: RedisPersistenceLayerTestClass', () => { it('puts record in Redis with default expiry timestamp', async () => { // Prepare - const layer = new RedisPersistenceLayerTestClass(getLayerConfig()); - await layer.init(); const status = IdempotencyRecordStatus.INPROGRESS; const record = new IdempotencyRecord({ idempotencyKey: dummyKey, @@ -258,8 +257,6 @@ describe('Class: RedisPersistenceLayerTestClass', () => { it('handles orphaned records by acquiring a lock and updating', async () => { // Prepare - const layer = new RedisPersistenceLayerTestClass(getLayerConfig()); - await layer.init(); const record = new IdempotencyRecord({ idempotencyKey: dummyKey, status: IdempotencyRecordStatus.INPROGRESS, @@ -302,8 +299,6 @@ describe('Class: RedisPersistenceLayerTestClass', () => { it('handles orphaned records by acquiring a lock but it fails and throw error', async () => { // Prepare - const layer = new RedisPersistenceLayerTestClass(getLayerConfig()); - await layer.init(); const record = new IdempotencyRecord({ idempotencyKey: dummyKey, status: IdempotencyRecordStatus.INPROGRESS, @@ -320,7 +315,7 @@ describe('Class: RedisPersistenceLayerTestClass', () => { }) ); - // Act + // Act & Assess await expect(layer._putRecord(record)).rejects.toThrow( 'Lock acquisition failed, raise to retry' ); @@ -328,8 +323,6 @@ describe('Class: RedisPersistenceLayerTestClass', () => { it('throws error when item already exists and not expired', async () => { // Prepare - const layer = new RedisPersistenceLayerTestClass(getLayerConfig()); - await layer.init(); const record = new IdempotencyRecord({ idempotencyKey: dummyKey, status: IdempotencyRecordStatus.INPROGRESS, @@ -351,8 +344,6 @@ describe('Class: RedisPersistenceLayerTestClass', () => { it('throws error when item is in progress', async () => { // Prepare - const layer = new RedisPersistenceLayerTestClass(getLayerConfig()); - await layer.init(); const inProgressExpiryTimestamp = getFutureTimestamp(10); const record = new IdempotencyRecord({ idempotencyKey: dummyKey, @@ -375,8 +366,6 @@ describe('Class: RedisPersistenceLayerTestClass', () => { it('throws error when trying to put a non-INPROGRESS record', async () => { // Prepare - const layer = new RedisPersistenceLayerTestClass(getLayerConfig()); - await layer.init(); const record = new IdempotencyRecord({ idempotencyKey: dummyKey, status: IdempotencyRecordStatus.COMPLETED, @@ -389,5 +378,26 @@ describe('Class: RedisPersistenceLayerTestClass', () => { ); }); }); + + describe('Method: _deleteRecord', () => { + it('deletes a record from Redis', async () => { + // Prepare + const record = new IdempotencyRecord({ + idempotencyKey: dummyKey, + status: IdempotencyRecordStatus.COMPLETED, + expiryTimestamp: getFutureTimestamp(15), + }); + const consoleDebugSpy = vi.spyOn(console, 'debug'); + + // Act + await layer._deleteRecord(record); + + // Assess + expect(client.del).toHaveBeenCalledWith(dummyKey); + expect(consoleDebugSpy).toHaveBeenCalledWith( + `Deleting record for idempotency key: ${record.idempotencyKey}` + ); + }); + }); }); }); From d6d421ea5a75e76282420e4fe16a018b463e05a6 Mon Sep 17 00:00:00 2001 From: arnabrahman Date: Sun, 4 May 2025 13:02:39 +0600 Subject: [PATCH 22/51] test: add unit tests for `_getRecord` method in RedisPersistenceLayer --- .../persistence/RedisPersistenceLayer.test.ts | 54 ++++++++++++++++++- 1 file changed, 53 insertions(+), 1 deletion(-) diff --git a/packages/idempotency/tests/unit/persistence/RedisPersistenceLayer.test.ts b/packages/idempotency/tests/unit/persistence/RedisPersistenceLayer.test.ts index a3b73eaf2..8eb7d06da 100644 --- a/packages/idempotency/tests/unit/persistence/RedisPersistenceLayer.test.ts +++ b/packages/idempotency/tests/unit/persistence/RedisPersistenceLayer.test.ts @@ -10,7 +10,11 @@ import { vi, } from 'vitest'; import { IdempotencyRecordStatus } from '../../../src/constants.js'; -import { IdempotencyPersistenceConnectionError } from '../../../src/errors.js'; +import { + IdempotencyItemNotFoundError, + IdempotencyPersistenceConnectionError, + IdempotencyPersistenceConsistencyError, +} from '../../../src/errors.js'; import { IdempotencyRecord } from '../../../src/persistence/IdempotencyRecord.js'; import RedisConnection from '../../../src/persistence/RedisConnection.js'; import { RedisPersistenceLayerTestClass } from '../../helpers/idempotencyUtils.js'; @@ -399,5 +403,53 @@ describe('Class: RedisPersistenceLayerTestClass', () => { ); }); }); + + describe('Method: _getRecord', () => { + it('gets a record from Redis', async () => { + // Prepare + const status = IdempotencyRecordStatus.INPROGRESS; + const expiryTimestamp = getFutureTimestamp(15); + client.get.mockResolvedValue( + JSON.stringify({ + status, + expiration: expiryTimestamp, + in_progress_expiration: expiryTimestamp, + validation: 'someHash', + data: { some: 'data' }, + }) + ); + + // Act + const record = await layer._getRecord(dummyKey); + + // Assess + expect(client.get).toHaveBeenCalledWith(dummyKey); + expect(record.getStatus()).toEqual(status); + expect(record.expiryTimestamp).toEqual(expiryTimestamp); + expect(record.inProgressExpiryTimestamp).toEqual(expiryTimestamp); + expect(record.payloadHash).toEqual('someHash'); + expect(record.getResponse()).toEqual({ some: 'data' }); + }); + + it('throws IdempotencyItemNotFoundError when record does not exist', async () => { + // Prepare + client.get.mockResolvedValue(null); + + // Act & Assess + await expect(layer._getRecord(dummyKey)).rejects.toThrow( + IdempotencyItemNotFoundError + ); + }); + + it('throws IdempotencyPersistenceConsistencyError when record is invalid JSON', async () => { + // Prepare + client.get.mockResolvedValue('invalid-json'); + + // Act & Assess + await expect(layer._getRecord(dummyKey)).rejects.toThrow( + IdempotencyPersistenceConsistencyError + ); + }); + }); }); }); From c7e45c63dbeec63270442c2bcf2c7cc52b1ab03a Mon Sep 17 00:00:00 2001 From: arnabrahman Date: Sun, 4 May 2025 13:08:07 +0600 Subject: [PATCH 23/51] test: add unit tests for `_updateRecord` method in RedisPersistenceLayer --- .../persistence/RedisPersistenceLayer.test.ts | 74 +++++++++++++++++++ 1 file changed, 74 insertions(+) diff --git a/packages/idempotency/tests/unit/persistence/RedisPersistenceLayer.test.ts b/packages/idempotency/tests/unit/persistence/RedisPersistenceLayer.test.ts index 8eb7d06da..49d7f0791 100644 --- a/packages/idempotency/tests/unit/persistence/RedisPersistenceLayer.test.ts +++ b/packages/idempotency/tests/unit/persistence/RedisPersistenceLayer.test.ts @@ -451,5 +451,79 @@ describe('Class: RedisPersistenceLayerTestClass', () => { ); }); }); + + describe('Method: _updateRecord', () => { + it('updates a record in Redis', async () => { + // Prepare + const record = new IdempotencyRecord({ + idempotencyKey: dummyKey, + status: IdempotencyRecordStatus.COMPLETED, + expiryTimestamp: getFutureTimestamp(15), + }); + client.set.mockResolvedValue('OK'); + + // Act + await layer._updateRecord(record); + + // Assess + expect(client.set).toHaveBeenCalledWith( + dummyKey, + JSON.stringify({ + status: 'COMPLETED', + expiration: record.expiryTimestamp, + }), + expect.objectContaining({ EX: expect.any(Number) }) + ); + }); + + it('updates a record with null responseData', async () => { + // Prepare + const record = new IdempotencyRecord({ + idempotencyKey: dummyKey, + status: IdempotencyRecordStatus.COMPLETED, + expiryTimestamp: getFutureTimestamp(15), + responseData: undefined, + }); + client.set.mockResolvedValue('OK'); + + // Act + await layer._updateRecord(record); + + // Assess + expect(client.set).toHaveBeenCalledWith( + dummyKey, + JSON.stringify({ + status: 'COMPLETED', + expiration: record.expiryTimestamp, + }), + expect.objectContaining({ EX: expect.any(Number) }) + ); + }); + + it('updates a record with valid responseData', async () => { + // Prepare + const record = new IdempotencyRecord({ + idempotencyKey: dummyKey, + status: IdempotencyRecordStatus.COMPLETED, + expiryTimestamp: getFutureTimestamp(15), + responseData: { key: 'value' }, + }); + client.set.mockResolvedValue('OK'); + + // Act + await layer._updateRecord(record); + + // Assess + expect(client.set).toHaveBeenCalledWith( + dummyKey, + JSON.stringify({ + status: 'COMPLETED', + expiration: record.expiryTimestamp, + data: record.responseData, + }), + expect.objectContaining({ EX: expect.any(Number) }) + ); + }); + }); }); }); From 1be3cd4df3434af40e3068cbe53d86930eb01563 Mon Sep 17 00:00:00 2001 From: arnabrahman Date: Sun, 4 May 2025 18:33:43 +0600 Subject: [PATCH 24/51] fix: correct expiry timestamp units in IdempotencyRecord and related tests --- .../src/persistence/IdempotencyRecord.ts | 2 +- .../src/persistence/RedisPersistenceLayer.ts | 4 ++-- .../persistence/RedisPersistenceLayer.test.ts | 15 ++++++++++----- 3 files changed, 13 insertions(+), 8 deletions(-) diff --git a/packages/idempotency/src/persistence/IdempotencyRecord.ts b/packages/idempotency/src/persistence/IdempotencyRecord.ts index b0aa037a6..9d83a17bb 100644 --- a/packages/idempotency/src/persistence/IdempotencyRecord.ts +++ b/packages/idempotency/src/persistence/IdempotencyRecord.ts @@ -13,7 +13,7 @@ import type { DynamoDBPersistenceLayer } from './DynamoDBPersistenceLayer.js'; */ class IdempotencyRecord { /** - * The expiry timestamp of the record in milliseconds UTC. + * The expiry timestamp of the record in seconds UTC. */ public expiryTimestamp?: number; /** diff --git a/packages/idempotency/src/persistence/RedisPersistenceLayer.ts b/packages/idempotency/src/persistence/RedisPersistenceLayer.ts index c03e1d879..2a9dfb728 100644 --- a/packages/idempotency/src/persistence/RedisPersistenceLayer.ts +++ b/packages/idempotency/src/persistence/RedisPersistenceLayer.ts @@ -274,9 +274,9 @@ class RedisPersistenceLayer extends BasePersistenceLayer { */ #_getExpirySeconds(expiryTimestamp?: number): number { if (expiryTimestamp) { - return Math.floor((expiryTimestamp - Date.now()) / 1000); + return expiryTimestamp - Math.floor(Date.now() / 1000); } - return this.getExpiresAfterSeconds(); + return this.expiresAfterSeconds; } /** diff --git a/packages/idempotency/tests/unit/persistence/RedisPersistenceLayer.test.ts b/packages/idempotency/tests/unit/persistence/RedisPersistenceLayer.test.ts index 49d7f0791..3c7ebadc8 100644 --- a/packages/idempotency/tests/unit/persistence/RedisPersistenceLayer.test.ts +++ b/packages/idempotency/tests/unit/persistence/RedisPersistenceLayer.test.ts @@ -22,7 +22,9 @@ import { RedisPersistenceLayerTestClass } from '../../helpers/idempotencyUtils.j vi.mock('../../../src/persistence/RedisConnection.js'); const getFutureTimestamp = (seconds: number): number => - new Date().getTime() + seconds * 1000; + Math.floor(Date.now() / 1000) + seconds; +const getFutureTimestampInMillis = (seconds: number): number => + getFutureTimestamp(seconds) * 1000; const dummyKey = 'someKey'; @@ -186,7 +188,7 @@ describe('Class: RedisPersistenceLayerTestClass', () => { // Prepare const status = IdempotencyRecordStatus.INPROGRESS; const expiryTimestamp = getFutureTimestamp(10); - const inProgressExpiryTimestamp = getFutureTimestamp(5); + const inProgressExpiryTimestamp = getFutureTimestampInMillis(5); const record = new IdempotencyRecord({ idempotencyKey: dummyKey, status, @@ -348,7 +350,7 @@ describe('Class: RedisPersistenceLayerTestClass', () => { it('throws error when item is in progress', async () => { // Prepare - const inProgressExpiryTimestamp = getFutureTimestamp(10); + const inProgressExpiryTimestamp = getFutureTimestampInMillis(10); const record = new IdempotencyRecord({ idempotencyKey: dummyKey, status: IdempotencyRecordStatus.INPROGRESS, @@ -409,11 +411,12 @@ describe('Class: RedisPersistenceLayerTestClass', () => { // Prepare const status = IdempotencyRecordStatus.INPROGRESS; const expiryTimestamp = getFutureTimestamp(15); + const inProgressExpiryTimestamp = getFutureTimestampInMillis(15); client.get.mockResolvedValue( JSON.stringify({ status, expiration: expiryTimestamp, - in_progress_expiration: expiryTimestamp, + in_progress_expiration: inProgressExpiryTimestamp, validation: 'someHash', data: { some: 'data' }, }) @@ -426,7 +429,9 @@ describe('Class: RedisPersistenceLayerTestClass', () => { expect(client.get).toHaveBeenCalledWith(dummyKey); expect(record.getStatus()).toEqual(status); expect(record.expiryTimestamp).toEqual(expiryTimestamp); - expect(record.inProgressExpiryTimestamp).toEqual(expiryTimestamp); + expect(record.inProgressExpiryTimestamp).toEqual( + inProgressExpiryTimestamp + ); expect(record.payloadHash).toEqual('someHash'); expect(record.getResponse()).toEqual({ some: 'data' }); }); From dda4cf1d22b755a47883861cd1d1154b0829db49 Mon Sep 17 00:00:00 2001 From: arnabrahman Date: Sun, 4 May 2025 18:45:44 +0600 Subject: [PATCH 25/51] refactor: rename private methods for consistency in RedisPersistenceLayer --- .../src/persistence/RedisPersistenceLayer.ts | 44 +++++++++---------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/packages/idempotency/src/persistence/RedisPersistenceLayer.ts b/packages/idempotency/src/persistence/RedisPersistenceLayer.ts index 2a9dfb728..c04b1dc9b 100644 --- a/packages/idempotency/src/persistence/RedisPersistenceLayer.ts +++ b/packages/idempotency/src/persistence/RedisPersistenceLayer.ts @@ -88,7 +88,7 @@ class RedisPersistenceLayer extends BasePersistenceLayer { protected async _putRecord(record: IdempotencyRecord): Promise { if (record.getStatus() === IdempotencyRecordStatus.INPROGRESS) { - await this.#_putInProgressRecord(record); + await this.#putInProgressRecord(record); } else { throw new Error( 'Only INPROGRESS records can be inserted with _putRecord' @@ -135,18 +135,33 @@ class RedisPersistenceLayer extends BasePersistenceLayer { }; const encodedItem = JSON.stringify(item); - const ttl = this.#_getExpirySeconds(record.expiryTimestamp); + const ttl = this.#getExpirySeconds(record.expiryTimestamp); // Need to set ttl again, if we don't set `EX` here the record will not have a ttl await this.#client.set(record.idempotencyKey, encodedItem, { EX: ttl, }); } + /** + * Determines if the provided Redis client is a default Redis client. + * + * This method checks if the provided client is an instance of the default Redis client + * by verifying the presence of the 'isOpen' property, which is specific to + * RedisClientType or RedisClusterType from the @redis/client package. + * + * @param client - The Redis client to check + */ + #isDefaultRedisClient( + client: RedisClientProtocol | RedisClientType | RedisClusterType + ): client is RedisClientType | RedisClusterType { + return 'isOpen' in client; + } + /** * Put a record in the persistence store with a status of "INPROGRESS". * The method guards against concurrent execution by using Redis' conditional write operations. */ - async #_putInProgressRecord(record: IdempotencyRecord): Promise { + async #putInProgressRecord(record: IdempotencyRecord): Promise { const item: Record = { [this.#statusAttr]: record.getStatus(), [this.#expiryAttr]: record.expiryTimestamp, @@ -161,7 +176,7 @@ class RedisPersistenceLayer extends BasePersistenceLayer { } const encodedItem = JSON.stringify(item); - const ttl = this.#_getExpirySeconds(record.expiryTimestamp); + const ttl = this.#getExpirySeconds(record.expiryTimestamp); const now = Date.now(); try { @@ -257,7 +272,7 @@ class RedisPersistenceLayer extends BasePersistenceLayer { * also be trying to handle the same orphan record. Once the lock is acquired, we set a new value * for the idempotency record in Redis with the appropriate time-to-live (TTL). */ - await this.#_acquireLock(record.idempotencyKey); + await this.#acquireLock(record.idempotencyKey); console.debug('Lock acquired, updating record'); await this.#client.set(record.idempotencyKey, encodedItem, { @@ -272,7 +287,7 @@ class RedisPersistenceLayer extends BasePersistenceLayer { /** * Calculates the number of seconds remaining until a specified expiry timestamp */ - #_getExpirySeconds(expiryTimestamp?: number): number { + #getExpirySeconds(expiryTimestamp?: number): number { if (expiryTimestamp) { return expiryTimestamp - Math.floor(Date.now() / 1000); } @@ -287,7 +302,7 @@ class RedisPersistenceLayer extends BasePersistenceLayer { * * @param idempotencyKey - The key to create a lock for */ - async #_acquireLock(idempotencyKey: string): Promise { + async #acquireLock(idempotencyKey: string): Promise { const lockKey = `${idempotencyKey}:lock`; const lockValue = 'true'; @@ -306,21 +321,6 @@ class RedisPersistenceLayer extends BasePersistenceLayer { 'Lock acquisition failed, raise to retry' ); } - - /** - * Determines if the provided Redis client is a default Redis client. - * - * This method checks if the provided client is an instance of the default Redis client - * by verifying the presence of the 'isOpen' property, which is specific to - * RedisClientType or RedisClusterType from the @redis/client package. - * - * @param client - The Redis client to check - */ - #isDefaultRedisClient( - client: RedisClientProtocol | RedisClientType | RedisClusterType - ): client is RedisClientType | RedisClusterType { - return 'isOpen' in client; - } } export { RedisPersistenceLayer }; From 93706e5ba4c619e0a444f6a36439b10a8b86c81c Mon Sep 17 00:00:00 2001 From: arnabrahman Date: Sun, 4 May 2025 19:02:01 +0600 Subject: [PATCH 26/51] docs: update class documentation for `RedisPersistenceLayer` with usage examples --- .../src/persistence/RedisPersistenceLayer.ts | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/packages/idempotency/src/persistence/RedisPersistenceLayer.ts b/packages/idempotency/src/persistence/RedisPersistenceLayer.ts index c04b1dc9b..ad92136a5 100644 --- a/packages/idempotency/src/persistence/RedisPersistenceLayer.ts +++ b/packages/idempotency/src/persistence/RedisPersistenceLayer.ts @@ -16,6 +16,46 @@ import { BasePersistenceLayer } from './BasePersistenceLayer.js'; import { IdempotencyRecord } from './IdempotencyRecord.js'; import RedisConnection from './RedisConnection.js'; +/** + * Redis persistence layer for idempotency records. + * + * This class uses Redis to write and read idempotency records. It supports both the default Redis client + * from @redis/client package as well as custom Redis clients. + * + * There are various options to configure the persistence layer, such as attribute names for storing + * status, expiry, data, and validation keys in Redis. + * + * With default configuration, you don't need to create the Redis client beforehand, the persistence layer + * will create it for you using the provided options. You can also bring your own Redis client by passing + * it through the `client` option. + * + * See the {@link https://docs.powertools.aws.dev/lambda/typescript/latest/utilities/idempotency/ Idempotency documentation} + * for more details on the Redis configuration and usage patterns. + * + * @example + * ```ts + * import { RedisPersistenceLayer } from '@aws-lambda-powertools/idempotency/redis'; + * + * const persistence = await new RedisPersistenceLayer({ url: 'redis://localhost:6379' }).init(); + * ``` + * + * @example + * ```ts + * // Using your own Redis client + * import { createClient } from '@redis/client'; + * import { RedisPersistenceLayer } from '@aws-lambda-powertools/idempotency/redis'; + * + * const redisClient = createClient({ url: 'redis://localhost:6379' }); + * await redisClient.connect(); + * + * const persistence = new RedisPersistenceLayer({ + * client: redisClient, + * }); + * ``` + * + * @see https://github.com/redis/node-redis/tree/master/packages/client + * @category Persistence Layer + */ class RedisPersistenceLayer extends BasePersistenceLayer { readonly #client: RedisClientProtocol | RedisClientType | RedisClusterType; readonly #dataAttr: string; From 1ca565d31fda38a5510e8ab0e93e0266db977710 Mon Sep 17 00:00:00 2001 From: arnabrahman Date: Sun, 4 May 2025 19:18:00 +0600 Subject: [PATCH 27/51] refactor: remove redundant JSON serialization options from `RedisPersistenceOptions` --- packages/idempotency/src/types/RedisPersistence.ts | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/packages/idempotency/src/types/RedisPersistence.ts b/packages/idempotency/src/types/RedisPersistence.ts index b8effef63..55b20ba02 100644 --- a/packages/idempotency/src/types/RedisPersistence.ts +++ b/packages/idempotency/src/types/RedisPersistence.ts @@ -1,5 +1,3 @@ -import type { JSONValue } from '@aws-lambda-powertools/commons/types'; - /** * Protocol defining the interface for a Redis client. * This ensures standardization among different Redis client implementations. @@ -22,7 +20,6 @@ interface RedisClientProtocol { value: string, options?: { EX?: number; // Expiration time in seconds - PX?: number; // Expiration time in milliseconds NX?: boolean; // Only set the key if it does not already exist } ): Promise; @@ -114,16 +111,6 @@ interface RedisPersistenceOptions extends RedisConnectionConfig { * (default: 'validation') */ validationKeyAttr?: string; - - /** - * Function used to serialize JSON data (default: JSON.stringify) - */ - jsonSerializer?: (value: JSONValue) => string; - - /** - * Function used to deserialize JSON data (default: JSON.parse) - */ - jsonDeserializer?: (text: string) => JSONValue; } export type { From 1740ee045d9126df2320dd162edf59ca028892cd Mon Sep 17 00:00:00 2001 From: arnabrahman Date: Mon, 5 May 2025 18:43:36 +0600 Subject: [PATCH 28/51] feat: upgrade @redis/client --- package-lock.json | 27 +++++++-------------------- packages/idempotency/package.json | 2 +- 2 files changed, 8 insertions(+), 21 deletions(-) diff --git a/package-lock.json b/package-lock.json index 11a3a0d30..6471e1d69 100644 --- a/package-lock.json +++ b/package-lock.json @@ -12073,19 +12073,17 @@ } }, "node_modules/@redis/client": { - "version": "1.6.0", - "resolved": "https://registry.npmjs.org/@redis/client/-/client-1.6.0.tgz", - "integrity": "sha512-aR0uffYI700OEEH4gYnitAnv3vzVGXCFvYfdpu/CJKvk4pHfLPEy/JSZyrpQ+15WhXe1yJRXLtfQ84s4mEXnPg==", + "version": "5.0.1", + "resolved": "https://registry.npmjs.org/@redis/client/-/client-5.0.1.tgz", + "integrity": "sha512-k0EJvlMGEyBqUD3orKe0UMZ66fPtfwqPIr+ZSd853sXj2EyhNtPXSx+J6sENXJNgAlEBhvD+57Dwt0qTisKB0A==", "license": "MIT", "optional": true, "peer": true, "dependencies": { - "cluster-key-slot": "1.1.2", - "generic-pool": "3.9.0", - "yallist": "4.0.0" + "cluster-key-slot": "1.1.2" }, "engines": { - "node": ">=14" + "node": ">= 18" } }, "node_modules/@rollup/rollup-android-arm-eabi": { @@ -17050,17 +17048,6 @@ "node": "^12.13.0 || ^14.15.0 || >=16.0.0" } }, - "node_modules/generic-pool": { - "version": "3.9.0", - "resolved": "https://registry.npmjs.org/generic-pool/-/generic-pool-3.9.0.tgz", - "integrity": "sha512-hymDOu5B53XvN4QT9dBmZxPX4CWhBPPLguTZ9MMFeFa/Kg0xWVfylOVNlJji/E7yTZWFd/q9GO5TxDLq156D7g==", - "license": "MIT", - "optional": true, - "peer": true, - "engines": { - "node": ">= 4" - } - }, "node_modules/get-caller-file": { "version": "2.0.5", "resolved": "https://registry.npmjs.org/get-caller-file/-/get-caller-file-2.0.5.tgz", @@ -24979,7 +24966,7 @@ "version": "4.0.0", "resolved": "https://registry.npmjs.org/yallist/-/yallist-4.0.0.tgz", "integrity": "sha512-3wdGidZyq5PB084XLES5TpOSRA3wjXAlIWMhum2kRcv/41Sn2emQ0dycQW4uZXLejwKvg6EsvbdlVL+FYEct7A==", - "devOptional": true, + "dev": true, "license": "ISC" }, "node_modules/yaml": { @@ -25147,7 +25134,7 @@ "@aws-sdk/client-dynamodb": ">=3.x", "@aws-sdk/lib-dynamodb": ">=3.x", "@middy/core": "4.x || 5.x || 6.x", - "@redis/client": "^1.6.0" + "@redis/client": "^5.0.1" }, "peerDependenciesMeta": { "@aws-sdk/client-dynamodb": { diff --git a/packages/idempotency/package.json b/packages/idempotency/package.json index c550ea871..a6635202f 100644 --- a/packages/idempotency/package.json +++ b/packages/idempotency/package.json @@ -121,7 +121,7 @@ "@aws-sdk/client-dynamodb": ">=3.x", "@aws-sdk/lib-dynamodb": ">=3.x", "@middy/core": "4.x || 5.x || 6.x", - "@redis/client": "^1.6.0" + "@redis/client": "^5.0.1" }, "peerDependenciesMeta": { "@aws-sdk/client-dynamodb": { From a73bea2a32ed6a42e565e88c02967af7191a9148 Mon Sep 17 00:00:00 2001 From: arnabrahman Date: Mon, 5 May 2025 19:23:07 +0600 Subject: [PATCH 29/51] fix: correct SSL option handling in `RedisConnection` --- packages/idempotency/src/persistence/RedisConnection.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/idempotency/src/persistence/RedisConnection.ts b/packages/idempotency/src/persistence/RedisConnection.ts index d472a405b..afd32e675 100644 --- a/packages/idempotency/src/persistence/RedisConnection.ts +++ b/packages/idempotency/src/persistence/RedisConnection.ts @@ -25,7 +25,7 @@ class RedisConnection { this.#password = options.password ?? ''; this.#url = options.url ?? ''; this.#dbIndex = options.dbIndex ?? 0; - this.#ssl = options.ssl || true; + this.#ssl = options.ssl ?? true; } /** @@ -65,7 +65,7 @@ class RedisConnection { socket: { host: this.#host, port: this.#port, - tls: this.#ssl, + ...(this.#ssl ? { tls: true } : {}), }, database: this.#dbIndex, }); From 5cf6c04cf08e5e0d6e24ff4bb66c6c93e3a9183d Mon Sep 17 00:00:00 2001 From: arnabrahman Date: Mon, 5 May 2025 19:27:09 +0600 Subject: [PATCH 30/51] test: add case for standalone client without `tls` when `ssl` is false --- .../unit/persistence/RedisConnection.test.ts | 28 ++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/packages/idempotency/tests/unit/persistence/RedisConnection.test.ts b/packages/idempotency/tests/unit/persistence/RedisConnection.test.ts index d7b99c389..892d7c62c 100644 --- a/packages/idempotency/tests/unit/persistence/RedisConnection.test.ts +++ b/packages/idempotency/tests/unit/persistence/RedisConnection.test.ts @@ -76,7 +76,6 @@ describe('Class: RedisConnection', () => { username: 'user', password: 'pass', dbIndex: 2, - ssl: true, }); // Act @@ -96,6 +95,33 @@ describe('Class: RedisConnection', () => { expect(createCluster).not.toHaveBeenCalled(); }); + it('creates a standalone client without `tls` when `ssl` is false', () => { + // Prepare + const connection = new RedisConnection({ + host: 'localhost', + port: 6380, + username: 'user', + password: 'pass', + dbIndex: 2, + ssl: false, + }); + + // Act + connection.getClient(); + + // Assess + expect(createClient).toHaveBeenCalledWith({ + username: 'user', + password: 'pass', + socket: { + host: 'localhost', + port: 6380, + }, + database: 2, + }); + expect(createCluster).not.toHaveBeenCalled(); + }); + it('creates a cluster client when mode is set to cluster', () => { // Prepare const connection = new RedisConnection({ From 4efa8deb9c9aaabaaa2e867d9ffc04826620ece6 Mon Sep 17 00:00:00 2001 From: arnabrahman Date: Mon, 5 May 2025 20:08:13 +0600 Subject: [PATCH 31/51] fix: update Redis client deletion method to accept an array of keys --- .../src/persistence/RedisPersistenceLayer.ts | 4 ++-- .../idempotency/src/types/RedisPersistence.ts | 15 ++++++++++----- .../persistence/RedisPersistenceLayer.test.ts | 2 +- 3 files changed, 13 insertions(+), 8 deletions(-) diff --git a/packages/idempotency/src/persistence/RedisPersistenceLayer.ts b/packages/idempotency/src/persistence/RedisPersistenceLayer.ts index ad92136a5..8611e9411 100644 --- a/packages/idempotency/src/persistence/RedisPersistenceLayer.ts +++ b/packages/idempotency/src/persistence/RedisPersistenceLayer.ts @@ -86,7 +86,7 @@ class RedisPersistenceLayer extends BasePersistenceLayer { * Initializes the Redis connection if it's the default Redis client and not already open. * * This method attempts to connect to Redis if necessary. If using a custom Redis client, - * it assumes the client is already initialized. + * it assumes the client is already connected. * * @throws {IdempotencyPersistenceConnectionError} When the connection to Redis fails */ @@ -123,7 +123,7 @@ class RedisPersistenceLayer extends BasePersistenceLayer { console.debug( `Deleting record for idempotency key: ${record.idempotencyKey}` ); - await this.#client.del(record.idempotencyKey); + await this.#client.del([record.idempotencyKey]); } protected async _putRecord(record: IdempotencyRecord): Promise { diff --git a/packages/idempotency/src/types/RedisPersistence.ts b/packages/idempotency/src/types/RedisPersistence.ts index 55b20ba02..870440b63 100644 --- a/packages/idempotency/src/types/RedisPersistence.ts +++ b/packages/idempotency/src/types/RedisPersistence.ts @@ -1,6 +1,11 @@ +import type { JSONValue } from '@aws-lambda-powertools/commons/types'; + /** * Protocol defining the interface for a Redis client. - * This ensures standardization among different Redis client implementations. + * + * This protocol outlines the expected behavior of a Redis client, allowing for + * standardization among different implementations and allowing customers to extend it + * in their own implementation. */ interface RedisClientProtocol { /** @@ -17,7 +22,7 @@ interface RedisClientProtocol { */ set( name: string, - value: string, + value: JSONValue, options?: { EX?: number; // Expiration time in seconds NX?: boolean; // Only set the key if it does not already exist @@ -25,10 +30,10 @@ interface RedisClientProtocol { ): Promise; /** - * Deletes one or more keys. - * @param keys The key(s) to delete + * Deletes the specified keys from Redis. + * @param keys The keys to delete */ - del(keys: string): Promise; + del(keys: string[]): Promise; } /** diff --git a/packages/idempotency/tests/unit/persistence/RedisPersistenceLayer.test.ts b/packages/idempotency/tests/unit/persistence/RedisPersistenceLayer.test.ts index 3c7ebadc8..592841e25 100644 --- a/packages/idempotency/tests/unit/persistence/RedisPersistenceLayer.test.ts +++ b/packages/idempotency/tests/unit/persistence/RedisPersistenceLayer.test.ts @@ -399,7 +399,7 @@ describe('Class: RedisPersistenceLayerTestClass', () => { await layer._deleteRecord(record); // Assess - expect(client.del).toHaveBeenCalledWith(dummyKey); + expect(client.del).toHaveBeenCalledWith([dummyKey]); expect(consoleDebugSpy).toHaveBeenCalledWith( `Deleting record for idempotency key: ${record.idempotencyKey}` ); From 9d007dfa2840b091a1f6267d6cbbfaf77978062b Mon Sep 17 00:00:00 2001 From: arnabrahman Date: Mon, 5 May 2025 22:26:02 +0600 Subject: [PATCH 32/51] fix: correct expiry timestamp documentation in IdempotencyRecordOptions --- packages/idempotency/src/types/IdempotencyRecord.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/idempotency/src/types/IdempotencyRecord.ts b/packages/idempotency/src/types/IdempotencyRecord.ts index 2ba7a5a49..ee1fed122 100644 --- a/packages/idempotency/src/types/IdempotencyRecord.ts +++ b/packages/idempotency/src/types/IdempotencyRecord.ts @@ -26,7 +26,7 @@ type IdempotencyRecordOptions = { */ status: IdempotencyRecordStatusValue; /** - * The expiry timestamp of the record in milliseconds UTC. + * The expiry timestamp of the record in seconds UTC. */ expiryTimestamp?: number; /** From 39167e7bb7421fed8c027ea5e6850bc057b16d44 Mon Sep 17 00:00:00 2001 From: arnabrahman Date: Tue, 6 May 2025 10:17:21 +0600 Subject: [PATCH 33/51] fix: throw `IdempotencyUnknownError` for invalid record status in RedisPersistenceLayer --- packages/idempotency/src/persistence/RedisPersistenceLayer.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/idempotency/src/persistence/RedisPersistenceLayer.ts b/packages/idempotency/src/persistence/RedisPersistenceLayer.ts index 8611e9411..463024ca7 100644 --- a/packages/idempotency/src/persistence/RedisPersistenceLayer.ts +++ b/packages/idempotency/src/persistence/RedisPersistenceLayer.ts @@ -6,6 +6,7 @@ import { IdempotencyItemNotFoundError, IdempotencyPersistenceConnectionError, IdempotencyPersistenceConsistencyError, + IdempotencyUnknownError, } from '../errors.js'; import type { IdempotencyRecordStatusValue } from '../types/IdempotencyRecord.js'; import type { @@ -130,7 +131,7 @@ class RedisPersistenceLayer extends BasePersistenceLayer { if (record.getStatus() === IdempotencyRecordStatus.INPROGRESS) { await this.#putInProgressRecord(record); } else { - throw new Error( + throw new IdempotencyUnknownError( 'Only INPROGRESS records can be inserted with _putRecord' ); } From 22e592658c03918fef4e41f928b6cfa7439d5b02 Mon Sep 17 00:00:00 2001 From: arnabrahman Date: Tue, 6 May 2025 10:38:31 +0600 Subject: [PATCH 34/51] fix: improve logging and documentation clarity in `RedisPersistenceLayer` --- .../src/persistence/RedisPersistenceLayer.ts | 44 +++++++++---------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/packages/idempotency/src/persistence/RedisPersistenceLayer.ts b/packages/idempotency/src/persistence/RedisPersistenceLayer.ts index 463024ca7..1a06b84de 100644 --- a/packages/idempotency/src/persistence/RedisPersistenceLayer.ts +++ b/packages/idempotency/src/persistence/RedisPersistenceLayer.ts @@ -218,7 +218,6 @@ class RedisPersistenceLayer extends BasePersistenceLayer { const encodedItem = JSON.stringify(item); const ttl = this.#getExpirySeconds(record.expiryTimestamp); - const now = Date.now(); try { /** @@ -229,14 +228,15 @@ class RedisPersistenceLayer extends BasePersistenceLayer { * | (in_progress_expiry) (expiry) * * Conditions to successfully save a record: - * * The idempotency key does not exist: + * + * The idempotency key does not exist: * - first time that this invocation key is used * - previous invocation with the same key was deleted due to TTL * - SET see https://redis.io/commands/set/ */ console.debug( - `Putting record for idempotency key: ${record.idempotencyKey}` + `Putting record on redis for idempotency key: ${record.idempotencyKey}` ); const response = await this.#client.set( record.idempotencyKey, @@ -268,9 +268,8 @@ class RedisPersistenceLayer extends BasePersistenceLayer { const existingRecord = await this._getRecord(record.idempotencyKey); /** If the status of the idempotency record is `COMPLETED` and the record has not expired - * (i.e., the expiry timestamp is greater than the current timestamp), then a valid completed - * record exists. We raise an error to prevent duplicate processing of a request that has already - * been completed successfully. + * then a valid completed record exists. We raise an error to prevent duplicate processing + * of a request that has already been completed successfully. */ if ( existingRecord.getStatus() === IdempotencyRecordStatus.COMPLETED && @@ -283,14 +282,14 @@ class RedisPersistenceLayer extends BasePersistenceLayer { } /** If the idempotency record has a status of 'INPROGRESS' and has a valid `inProgressExpiryTimestamp` - * (meaning the timestamp is greater than the current timestamp in milliseconds), then we have encountered - * a valid in-progress record. This indicates that another process is currently handling the request, and - * to maintain idempotency, we raise an error to prevent concurrent processing of the same request. + * (meaning the timestamp is greater than the current timestamp in milliseconds), then we have encountered + * a valid in-progress record. This indicates that another process is currently handling the request, and + * to maintain idempotency, we raise an error to prevent concurrent processing of the same request. */ if ( existingRecord.getStatus() === IdempotencyRecordStatus.INPROGRESS && existingRecord.inProgressExpiryTimestamp && - existingRecord.inProgressExpiryTimestamp > now + existingRecord.inProgressExpiryTimestamp > Date.now() ) { throw new IdempotencyItemAlreadyExistsError( `Failed to put record for in-progress idempotency key: ${record.idempotencyKey}`, @@ -299,9 +298,9 @@ class RedisPersistenceLayer extends BasePersistenceLayer { } /** Reaching this point indicates that the idempotency record found is an orphan record. An orphan record is - * one that is neither completed nor in-progress within its expected time frame. It may result from a - * previous invocation that has timed out or an expired record that has yet to be cleaned up by Redis. - * We raise an error to handle this exceptional scenario appropriately. + * one that is neither completed nor in-progress within its expected time frame. It may result from a + * previous invocation that has timed out or an expired record that has yet to be cleaned up by Redis. + * We raise an error to handle this exceptional scenario appropriately. */ throw new IdempotencyPersistenceConsistencyError( 'Orphaned record detected' @@ -309,9 +308,9 @@ class RedisPersistenceLayer extends BasePersistenceLayer { } catch (error) { if (error instanceof IdempotencyPersistenceConsistencyError) { /** Handle an orphan record by attempting to acquire a lock, which by default lasts for 10 seconds. - * The purpose of acquiring the lock is to prevent race conditions with other processes that might - * also be trying to handle the same orphan record. Once the lock is acquired, we set a new value - * for the idempotency record in Redis with the appropriate time-to-live (TTL). + * The purpose of acquiring the lock is to prevent race conditions with other processes that might + * also be trying to handle the same orphan record. Once the lock is acquired, we set a new value + * for the idempotency record in Redis with the appropriate time-to-live (TTL). */ await this.#acquireLock(record.idempotencyKey); @@ -337,9 +336,9 @@ class RedisPersistenceLayer extends BasePersistenceLayer { /** * Attempt to acquire a lock for a specified resource name, with a default timeout. - * This method attempts to set a lock using Redis to prevent concurrent - * access to a resource identified by 'idempotencyKey'. It uses the 'NX' flag to ensure that - * the lock is only set if it does not already exist, thereby enforcing mutual exclusion. + * This method attempts to set a lock using Redis to prevent concurrent access to a resource + * identified by 'idempotencyKey'. It uses the 'NX' flag to ensure that the lock is only + * set if it does not already exist, thereby enforcing mutual exclusion. * * @param idempotencyKey - The key to create a lock for */ @@ -354,9 +353,10 @@ class RedisPersistenceLayer extends BasePersistenceLayer { }); if (acquired) return; - // If the lock acquisition fails, it suggests a race condition has occurred. In this case, instead of - // proceeding, we log the event and raise an error to indicate that the current operation should be - // retried after the lock is released by the process that currently holds it. + /** If the lock acquisition fails, it suggests a race condition has occurred. In this case, instead of + * proceeding, we log the event and raise an error to indicate that the current operation should be + * retried after the lock is released by the process that currently holds it. + */ console.debug('Lock acquisition failed, raise to retry'); throw new IdempotencyItemAlreadyExistsError( 'Lock acquisition failed, raise to retry' From 32ef8fbc7d604ccd755afdf905c5a24dccaae150 Mon Sep 17 00:00:00 2001 From: arnabrahman Date: Tue, 6 May 2025 10:43:02 +0600 Subject: [PATCH 35/51] fix: cast Redis client to RedisClientProtocol in usage example --- packages/idempotency/src/persistence/RedisPersistenceLayer.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/idempotency/src/persistence/RedisPersistenceLayer.ts b/packages/idempotency/src/persistence/RedisPersistenceLayer.ts index 1a06b84de..0f3565954 100644 --- a/packages/idempotency/src/persistence/RedisPersistenceLayer.ts +++ b/packages/idempotency/src/persistence/RedisPersistenceLayer.ts @@ -45,12 +45,13 @@ import RedisConnection from './RedisConnection.js'; * // Using your own Redis client * import { createClient } from '@redis/client'; * import { RedisPersistenceLayer } from '@aws-lambda-powertools/idempotency/redis'; + * import { RedisClientProtocol } from '@aws-lambda-powertools/idempotency/redis/types'; * * const redisClient = createClient({ url: 'redis://localhost:6379' }); * await redisClient.connect(); * * const persistence = new RedisPersistenceLayer({ - * client: redisClient, + * client: redisClient as RedisClientProtocol, * }); * ``` * From b8e8f83c8c08fe7387f7435f213a4bc865ad856c Mon Sep 17 00:00:00 2001 From: arnabrahman Date: Tue, 6 May 2025 10:53:35 +0600 Subject: [PATCH 36/51] fix: update documentation for RedisConnection class to clarify its purpose and functionality --- packages/idempotency/src/persistence/RedisConnection.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/idempotency/src/persistence/RedisConnection.ts b/packages/idempotency/src/persistence/RedisConnection.ts index afd32e675..4f6881f30 100644 --- a/packages/idempotency/src/persistence/RedisConnection.ts +++ b/packages/idempotency/src/persistence/RedisConnection.ts @@ -3,9 +3,10 @@ import type { RedisClientType, RedisClusterType } from '@redis/client'; import type { RedisConnectionConfig } from '../types/RedisPersistence.js'; /** - * RedisConnection class is responsible for creating a Redis client based on the provided configuration. - * It supports both standalone and cluster modes. - * The class takes a configuration object as a parameter and initializes the Redis client accordingly. + * A class for creating and managing Redis client connections. + * + * This class handles both standalone and cluster connection modes to Redis, + * providing an abstraction layer over the Redis client creation process. */ class RedisConnection { readonly #mode: 'standalone' | 'cluster'; From 2215c1f4cd79c0c09cde7edc131722f33e53510f Mon Sep 17 00:00:00 2001 From: arnabrahman Date: Wed, 7 May 2025 10:21:38 +0600 Subject: [PATCH 37/51] refactor: rename `RedisClientProtocol` to `RedisCompatibleClient` for clarity and consistency --- .../src/persistence/RedisPersistenceLayer.ts | 10 ++++----- .../idempotency/src/types/RedisPersistence.ts | 22 ++++++++++--------- packages/idempotency/src/types/index.ts | 2 +- 3 files changed, 18 insertions(+), 16 deletions(-) diff --git a/packages/idempotency/src/persistence/RedisPersistenceLayer.ts b/packages/idempotency/src/persistence/RedisPersistenceLayer.ts index 0f3565954..8eb5425b5 100644 --- a/packages/idempotency/src/persistence/RedisPersistenceLayer.ts +++ b/packages/idempotency/src/persistence/RedisPersistenceLayer.ts @@ -10,7 +10,7 @@ import { } from '../errors.js'; import type { IdempotencyRecordStatusValue } from '../types/IdempotencyRecord.js'; import type { - RedisClientProtocol, + RedisCompatibleClient, RedisPersistenceOptions, } from '../types/RedisPersistence.js'; import { BasePersistenceLayer } from './BasePersistenceLayer.js'; @@ -45,13 +45,13 @@ import RedisConnection from './RedisConnection.js'; * // Using your own Redis client * import { createClient } from '@redis/client'; * import { RedisPersistenceLayer } from '@aws-lambda-powertools/idempotency/redis'; - * import { RedisClientProtocol } from '@aws-lambda-powertools/idempotency/redis/types'; + * import { RedisCompatibleClient } from '@aws-lambda-powertools/idempotency/redis/types'; * * const redisClient = createClient({ url: 'redis://localhost:6379' }); * await redisClient.connect(); * * const persistence = new RedisPersistenceLayer({ - * client: redisClient as RedisClientProtocol, + * client: redisClient as RedisCompatibleClient, * }); * ``` * @@ -59,7 +59,7 @@ import RedisConnection from './RedisConnection.js'; * @category Persistence Layer */ class RedisPersistenceLayer extends BasePersistenceLayer { - readonly #client: RedisClientProtocol | RedisClientType | RedisClusterType; + readonly #client: RedisCompatibleClient | RedisClientType | RedisClusterType; readonly #dataAttr: string; readonly #expiryAttr: string; readonly #inProgressExpiryAttr: string; @@ -194,7 +194,7 @@ class RedisPersistenceLayer extends BasePersistenceLayer { * @param client - The Redis client to check */ #isDefaultRedisClient( - client: RedisClientProtocol | RedisClientType | RedisClusterType + client: RedisCompatibleClient | RedisClientType | RedisClusterType ): client is RedisClientType | RedisClusterType { return 'isOpen' in client; } diff --git a/packages/idempotency/src/types/RedisPersistence.ts b/packages/idempotency/src/types/RedisPersistence.ts index 870440b63..3f5085108 100644 --- a/packages/idempotency/src/types/RedisPersistence.ts +++ b/packages/idempotency/src/types/RedisPersistence.ts @@ -1,13 +1,13 @@ import type { JSONValue } from '@aws-lambda-powertools/commons/types'; /** - * Protocol defining the interface for a Redis client. + * Interface for clients compatible with Redis operations. * - * This protocol outlines the expected behavior of a Redis client, allowing for - * standardization among different implementations and allowing customers to extend it - * in their own implementation. + * This interface defines the minimum set of Redis operations that must be implemented + * by a client to be used with the Redis persistence layer. It supports basic key-value + * operations like get, set, and delete. */ -interface RedisClientProtocol { +interface RedisCompatibleClient { /** * Retrieves the value associated with the given key. * @param name The key to get the value for @@ -19,13 +19,15 @@ interface RedisClientProtocol { * @param name The key to set * @param value The value to set * @param options Optional parameters for setting the value + * @param options.EX Set the specified expire time, in seconds (a positive integer) + * @param options.NX Only set the key if it does not already exist */ set( name: string, value: JSONValue, options?: { - EX?: number; // Expiration time in seconds - NX?: boolean; // Only set the key if it does not already exist + EX?: number; + NX?: boolean; } ): Promise; @@ -86,10 +88,10 @@ interface RedisConnectionConfig { */ interface RedisPersistenceOptions extends RedisConnectionConfig { /** - * A Redis client that implements the RedisClientProtocol interface. + * A Redis client that implements the RedisCompatibleClient interface. * If provided, all other connection configuration options will be ignored. */ - client?: RedisClientProtocol; + client?: RedisCompatibleClient; /** * Redis JSON attribute name for expiry timestamp (default: 'expiration') @@ -119,7 +121,7 @@ interface RedisPersistenceOptions extends RedisConnectionConfig { } export type { - RedisClientProtocol, + RedisCompatibleClient, RedisConnectionConfig, RedisPersistenceOptions, }; diff --git a/packages/idempotency/src/types/index.ts b/packages/idempotency/src/types/index.ts index f7d8fe5d6..2652ca324 100644 --- a/packages/idempotency/src/types/index.ts +++ b/packages/idempotency/src/types/index.ts @@ -21,7 +21,7 @@ export type { DynamoDBPersistenceOptionsWithClientInstance, } from './DynamoDBPersistence.js'; export type { - RedisClientProtocol, + RedisCompatibleClient, RedisConnectionConfig, RedisPersistenceOptions, } from './RedisPersistence.js'; From 489bcfcd47d0e16b0f01e8992d897584ff3b30cc Mon Sep 17 00:00:00 2001 From: arnabrahman Date: Wed, 7 May 2025 19:00:09 +0600 Subject: [PATCH 38/51] refactor: remove `RedisConnectionConfig` interface and enforce client requirement in `RedisPersistenceOptions` --- .../idempotency/src/types/RedisPersistence.ts | 58 ++----------------- packages/idempotency/src/types/index.ts | 1 - 2 files changed, 4 insertions(+), 55 deletions(-) diff --git a/packages/idempotency/src/types/RedisPersistence.ts b/packages/idempotency/src/types/RedisPersistence.ts index 3f5085108..b93d91cdc 100644 --- a/packages/idempotency/src/types/RedisPersistence.ts +++ b/packages/idempotency/src/types/RedisPersistence.ts @@ -38,60 +38,14 @@ interface RedisCompatibleClient { del(keys: string[]): Promise; } -/** - * Redis client connection configuration parameters - */ -interface RedisConnectionConfig { - /** - * Redis host - */ - host?: string; - - /** - * Redis port (default: 6379) - */ - port?: number; - - /** - * Redis username - */ - username?: string; - - /** - * Redis password - */ - password?: string; - - /** - * Redis connection string URL, overrides host/port if provided - */ - url?: string; - - /** - * Redis database index (default: 0) - */ - dbIndex?: number; - - /** - * Redis client mode (default: 'standalone') - */ - mode?: 'standalone' | 'cluster'; - - /** - * Whether to use SSL for Redis connection (default: true) - */ - ssl?: boolean; -} - /** * Options for configuring the Redis persistence layer */ -interface RedisPersistenceOptions extends RedisConnectionConfig { +interface RedisPersistenceOptions { /** - * A Redis client that implements the RedisCompatibleClient interface. - * If provided, all other connection configuration options will be ignored. + * Redis client instance that implements the RedisCompatibleClient interface. */ - client?: RedisCompatibleClient; + client: RedisCompatibleClient; /** * Redis JSON attribute name for expiry timestamp (default: 'expiration') @@ -120,8 +74,4 @@ interface RedisPersistenceOptions extends RedisConnectionConfig { validationKeyAttr?: string; } -export type { - RedisCompatibleClient, - RedisConnectionConfig, - RedisPersistenceOptions, -}; +export type { RedisCompatibleClient, RedisPersistenceOptions }; diff --git a/packages/idempotency/src/types/index.ts b/packages/idempotency/src/types/index.ts index 2652ca324..70a799286 100644 --- a/packages/idempotency/src/types/index.ts +++ b/packages/idempotency/src/types/index.ts @@ -22,6 +22,5 @@ export type { } from './DynamoDBPersistence.js'; export type { RedisCompatibleClient, - RedisConnectionConfig, RedisPersistenceOptions, } from './RedisPersistence.js'; From 08286e1c4b5fde493af69701a512893290b8ff97 Mon Sep 17 00:00:00 2001 From: arnabrahman Date: Wed, 7 May 2025 19:08:54 +0600 Subject: [PATCH 39/51] refactor: remove `RedisConnection` class and related connection error handling from persistence layer --- packages/idempotency/src/errors.ts | 16 +--- .../src/persistence/RedisConnection.ts | 76 ------------------- .../src/persistence/RedisPersistenceLayer.ts | 52 +------------ 3 files changed, 3 insertions(+), 141 deletions(-) delete mode 100644 packages/idempotency/src/persistence/RedisConnection.ts diff --git a/packages/idempotency/src/errors.ts b/packages/idempotency/src/errors.ts index dbb9475d6..0acd58d57 100644 --- a/packages/idempotency/src/errors.ts +++ b/packages/idempotency/src/errors.ts @@ -113,24 +113,11 @@ class IdempotencyKeyError extends IdempotencyUnknownError { } } -/** - * Error connecting to the persistence layer - */ -class IdempotencyPersistenceConnectionError extends IdempotencyUnknownError { - public constructor(message: string, options?: ErrorOptions) { - super(message, options); - this.name = 'IdempotencyPersistenceConnectionError'; - } -} - /** * Error with the persistence layer's consistency, needs to be removed */ class IdempotencyPersistenceConsistencyError extends IdempotencyUnknownError { - public constructor( - message: string, - options?: ErrorOptions - ) { + public constructor(message: string, options?: ErrorOptions) { super(message, options); this.name = 'IdempotencyPersistenceConsistencyError'; } @@ -145,7 +132,6 @@ export { IdempotencyValidationError, IdempotencyInconsistentStateError, IdempotencyPersistenceLayerError, - IdempotencyPersistenceConnectionError, IdempotencyPersistenceConsistencyError, IdempotencyKeyError, }; diff --git a/packages/idempotency/src/persistence/RedisConnection.ts b/packages/idempotency/src/persistence/RedisConnection.ts deleted file mode 100644 index 4f6881f30..000000000 --- a/packages/idempotency/src/persistence/RedisConnection.ts +++ /dev/null @@ -1,76 +0,0 @@ -import { createClient, createCluster } from '@redis/client'; -import type { RedisClientType, RedisClusterType } from '@redis/client'; -import type { RedisConnectionConfig } from '../types/RedisPersistence.js'; - -/** - * A class for creating and managing Redis client connections. - * - * This class handles both standalone and cluster connection modes to Redis, - * providing an abstraction layer over the Redis client creation process. - */ -class RedisConnection { - readonly #mode: 'standalone' | 'cluster'; - readonly #host: string; - readonly #port: number; - readonly #username: string; - readonly #password: string; - readonly #url: string; - readonly #dbIndex: number; - readonly #ssl: boolean; - - public constructor(options: RedisConnectionConfig) { - this.#mode = options.mode ?? 'standalone'; - this.#host = options.host ?? ''; - this.#port = options.port ?? 6379; - this.#username = options.username ?? ''; - this.#password = options.password ?? ''; - this.#url = options.url ?? ''; - this.#dbIndex = options.dbIndex ?? 0; - this.#ssl = options.ssl ?? true; - } - - /** - * Returns a Redis client based on the connection mode. - * If the mode is 'cluster', it creates a Redis cluster client. - * If the mode is 'standalone', it creates a standalone Redis client. - */ - public getClient(): RedisClientType | RedisClusterType { - return this.#mode === 'cluster' - ? this.#createClusterClient() - : this.#createStandaloneClient(); - } - - /** - * Creates a Redis cluster client. - * The client will connect to the Redis cluster using the provided URL. - */ - #createClusterClient(): RedisClusterType { - return createCluster({ - rootNodes: [{ url: this.#url }], - }); - } - - /** - * Creates a standalone Redis client. - * The client will connect to the Redis server using the provided host, port, username, password, and database index. - * If a URL is provided, it will be used to create the client instead of the other parameters. - */ - #createStandaloneClient(): RedisClientType { - if (this.#url) { - return createClient({ url: this.#url }); - } - - return createClient({ - username: this.#username, - password: this.#password, - socket: { - host: this.#host, - port: this.#port, - ...(this.#ssl ? { tls: true } : {}), - }, - database: this.#dbIndex, - }); - } -} - -export default RedisConnection; diff --git a/packages/idempotency/src/persistence/RedisPersistenceLayer.ts b/packages/idempotency/src/persistence/RedisPersistenceLayer.ts index 8eb5425b5..cd50de16a 100644 --- a/packages/idempotency/src/persistence/RedisPersistenceLayer.ts +++ b/packages/idempotency/src/persistence/RedisPersistenceLayer.ts @@ -1,10 +1,8 @@ import type { JSONObject } from '@aws-lambda-powertools/commons/types'; -import type { RedisClientType, RedisClusterType } from '@redis/client'; import { IdempotencyRecordStatus } from '../constants.js'; import { IdempotencyItemAlreadyExistsError, IdempotencyItemNotFoundError, - IdempotencyPersistenceConnectionError, IdempotencyPersistenceConsistencyError, IdempotencyUnknownError, } from '../errors.js'; @@ -15,7 +13,6 @@ import type { } from '../types/RedisPersistence.js'; import { BasePersistenceLayer } from './BasePersistenceLayer.js'; import { IdempotencyRecord } from './IdempotencyRecord.js'; -import RedisConnection from './RedisConnection.js'; /** * Redis persistence layer for idempotency records. @@ -59,7 +56,7 @@ import RedisConnection from './RedisConnection.js'; * @category Persistence Layer */ class RedisPersistenceLayer extends BasePersistenceLayer { - readonly #client: RedisCompatibleClient | RedisClientType | RedisClusterType; + readonly #client: RedisCompatibleClient; readonly #dataAttr: string; readonly #expiryAttr: string; readonly #inProgressExpiryAttr: string; @@ -77,37 +74,7 @@ class RedisPersistenceLayer extends BasePersistenceLayer { this.#dataAttr = options.dataAttr ?? 'data'; this.#validationKeyAttr = options.validationKeyAttr ?? 'validation'; this.#orphanLockTimeout = Math.min(10, this.expiresAfterSeconds); - if (options.client) { - this.#client = options.client; - } else { - this.#client = new RedisConnection(options).getClient(); - } - } - - /** - * Initializes the Redis connection if it's the default Redis client and not already open. - * - * This method attempts to connect to Redis if necessary. If using a custom Redis client, - * it assumes the client is already connected. - * - * @throws {IdempotencyPersistenceConnectionError} When the connection to Redis fails - */ - public async init() { - if ( - this.#isDefaultRedisClient(this.#client) && - this.#client.isOpen === false - ) { - try { - await this.#client.connect(); - } catch (error) { - console.error('Failed to connect to Redis:', error); - throw new IdempotencyPersistenceConnectionError( - 'Could not connect to Redis', - error as Error - ); - } - } - return this; + this.#client = options.client; } /** @@ -184,21 +151,6 @@ class RedisPersistenceLayer extends BasePersistenceLayer { }); } - /** - * Determines if the provided Redis client is a default Redis client. - * - * This method checks if the provided client is an instance of the default Redis client - * by verifying the presence of the 'isOpen' property, which is specific to - * RedisClientType or RedisClusterType from the @redis/client package. - * - * @param client - The Redis client to check - */ - #isDefaultRedisClient( - client: RedisCompatibleClient | RedisClientType | RedisClusterType - ): client is RedisClientType | RedisClusterType { - return 'isOpen' in client; - } - /** * Put a record in the persistence store with a status of "INPROGRESS". * The method guards against concurrent execution by using Redis' conditional write operations. From d7401889fcb3a8af1cb5bb0712f97e73c48f4ea8 Mon Sep 17 00:00:00 2001 From: arnabrahman Date: Wed, 7 May 2025 19:09:06 +0600 Subject: [PATCH 40/51] refactor: remove `RedisConnection` references from `RedisPersistenceLayer` tests and streamline client handling --- .../unit/persistence/RedisConnection.test.ts | 153 ---- .../persistence/RedisPersistenceLayer.test.ts | 748 ++++++++---------- 2 files changed, 316 insertions(+), 585 deletions(-) delete mode 100644 packages/idempotency/tests/unit/persistence/RedisConnection.test.ts diff --git a/packages/idempotency/tests/unit/persistence/RedisConnection.test.ts b/packages/idempotency/tests/unit/persistence/RedisConnection.test.ts deleted file mode 100644 index 892d7c62c..000000000 --- a/packages/idempotency/tests/unit/persistence/RedisConnection.test.ts +++ /dev/null @@ -1,153 +0,0 @@ -import { createClient, createCluster } from '@redis/client'; -import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; -import RedisConnection from '../../../src/persistence/RedisConnection.js'; - -vi.mock('@redis/client', () => ({ - createClient: vi.fn(), - createCluster: vi.fn(), -})); - -describe('Class: RedisConnection', () => { - beforeEach(() => { - vi.clearAllMocks(); - }); - - afterEach(() => { - vi.resetAllMocks(); - }); - - describe('Method: constructor', () => { - it('creates an instance with default values', () => { - // Prepare & Act - const connection = new RedisConnection({}); - - // Assess - expect(connection).toBeInstanceOf(RedisConnection); - }); - - it('creates an instance with provided values', () => { - // Prepare & Act - const connection = new RedisConnection({ - mode: 'standalone', - host: 'localhost', - port: 6379, - username: 'user', - password: 'pass', - dbIndex: 1, - ssl: false, - }); - - // Assess - expect(connection).toBeInstanceOf(RedisConnection); - }); - }); - - describe('Method: getClient', () => { - const url = 'redis://localhost:6379'; - it('creates a standalone client by default', () => { - // Prepare - const connection = new RedisConnection({}); - - // Act - connection.getClient(); - - // Assess - expect(createClient).toHaveBeenCalledTimes(1); - expect(createCluster).not.toHaveBeenCalled(); - }); - - it('creates a standalone client with URL configuration', () => { - // Prepare - const connection = new RedisConnection({ url }); - - // Act - connection.getClient(); - - // Assess - expect(createClient).toHaveBeenCalledWith({ url }); - expect(createCluster).not.toHaveBeenCalled(); - }); - - it('creates a standalone client with host and port configuration', () => { - // Prepare - const connection = new RedisConnection({ - host: 'localhost', - port: 6380, - username: 'user', - password: 'pass', - dbIndex: 2, - }); - - // Act - connection.getClient(); - - // Assess - expect(createClient).toHaveBeenCalledWith({ - username: 'user', - password: 'pass', - socket: { - host: 'localhost', - port: 6380, - tls: true, - }, - database: 2, - }); - expect(createCluster).not.toHaveBeenCalled(); - }); - - it('creates a standalone client without `tls` when `ssl` is false', () => { - // Prepare - const connection = new RedisConnection({ - host: 'localhost', - port: 6380, - username: 'user', - password: 'pass', - dbIndex: 2, - ssl: false, - }); - - // Act - connection.getClient(); - - // Assess - expect(createClient).toHaveBeenCalledWith({ - username: 'user', - password: 'pass', - socket: { - host: 'localhost', - port: 6380, - }, - database: 2, - }); - expect(createCluster).not.toHaveBeenCalled(); - }); - - it('creates a cluster client when mode is set to cluster', () => { - // Prepare - const connection = new RedisConnection({ - mode: 'cluster', - url, - }); - - // Act - connection.getClient(); - - // Assess - expect(createCluster).toHaveBeenCalledTimes(1); - expect(createCluster).toHaveBeenCalledWith({ - rootNodes: [{ url }], - }); - expect(createClient).not.toHaveBeenCalled(); - }); - - it('prioritizes URL over host/port when both are provided', () => { - const connection = new RedisConnection({ - url, - host: 'localhost', - port: 6380, - }); - connection.getClient(); - expect(createClient).toHaveBeenCalledWith({ url }); - }); - }); -}); diff --git a/packages/idempotency/tests/unit/persistence/RedisPersistenceLayer.test.ts b/packages/idempotency/tests/unit/persistence/RedisPersistenceLayer.test.ts index 592841e25..a29d9cca5 100644 --- a/packages/idempotency/tests/unit/persistence/RedisPersistenceLayer.test.ts +++ b/packages/idempotency/tests/unit/persistence/RedisPersistenceLayer.test.ts @@ -1,9 +1,7 @@ import { - type Mock, afterAll, afterEach, beforeAll, - beforeEach, describe, expect, it, @@ -12,11 +10,9 @@ import { import { IdempotencyRecordStatus } from '../../../src/constants.js'; import { IdempotencyItemNotFoundError, - IdempotencyPersistenceConnectionError, IdempotencyPersistenceConsistencyError, } from '../../../src/errors.js'; import { IdempotencyRecord } from '../../../src/persistence/IdempotencyRecord.js'; -import RedisConnection from '../../../src/persistence/RedisConnection.js'; import { RedisPersistenceLayerTestClass } from '../../helpers/idempotencyUtils.js'; vi.mock('../../../src/persistence/RedisConnection.js'); @@ -27,36 +23,20 @@ const getFutureTimestampInMillis = (seconds: number): number => getFutureTimestamp(seconds) * 1000; const dummyKey = 'someKey'; +const client = { + get: vi.fn(), + set: vi.fn(), + del: vi.fn(), +}; +const persistenceLayer = new RedisPersistenceLayerTestClass({ + client, +}); describe('Class: RedisPersistenceLayerTestClass', () => { - const mockDefaultClientConnect = vi.fn().mockResolvedValue(true); - - const mockDefaultClient = { - isOpen: false, - connect: mockDefaultClientConnect, - set: vi.fn(), - get: vi.fn(), - del: vi.fn().mockResolvedValue(1), - }; - - const mockUserProvidedClient = { - get: vi.fn(), - set: vi.fn(), - del: vi.fn(), - }; - beforeAll(() => { vi.useFakeTimers().setSystemTime(new Date()); }); - beforeEach(() => { - vi.clearAllMocks(); - - (RedisConnection as unknown as Mock).mockImplementation(() => ({ - getClient: vi.fn().mockReturnValue(mockDefaultClient), - })); - }); - afterEach(() => { vi.clearAllMocks(); }); @@ -65,470 +45,374 @@ describe('Class: RedisPersistenceLayerTestClass', () => { vi.useRealTimers(); }); - describe('Method: init', () => { - it('should create a Redis connection and connect when no client is provided', async () => { + describe('Method: _putRecord', () => { + it('puts a record with INPROGRESS status into Redis', async () => { // Prepare - const layer = new RedisPersistenceLayerTestClass({}); + const record = new IdempotencyRecord({ + idempotencyKey: dummyKey, + status: IdempotencyRecordStatus.INPROGRESS, + expiryTimestamp: getFutureTimestamp(10), + }); + client.set.mockResolvedValue('OK'); // Act - await layer.init(); + await persistenceLayer._putRecord(record); // Assess - expect(RedisConnection).toHaveBeenCalledTimes(1); - expect(mockDefaultClientConnect).toHaveBeenCalledTimes(1); + expect(client.set).toHaveBeenCalledWith( + dummyKey, + JSON.stringify({ + status: IdempotencyRecordStatus.INPROGRESS, + expiration: record.expiryTimestamp, + }), + { EX: 10, NX: true } + ); }); - it('should throw IdempotencyPersistenceConnectionError when connection fails', async () => { + it('puts the record in Redis when using an in progress expiry timestamp', async () => { // Prepare - mockDefaultClientConnect.mockRejectedValueOnce( - new Error('Connection failed') - ); - const layer = new RedisPersistenceLayerTestClass({}); - const consoleErrorSpy = vi - .spyOn(console, 'error') - .mockImplementation(() => {}); + const status = IdempotencyRecordStatus.INPROGRESS; + const expiryTimestamp = getFutureTimestamp(10); + const inProgressExpiryTimestamp = getFutureTimestampInMillis(5); + const record = new IdempotencyRecord({ + idempotencyKey: dummyKey, + status, + expiryTimestamp, + inProgressExpiryTimestamp, + }); - // Act & Assess - await expect(layer.init()).rejects.toThrow( - new IdempotencyPersistenceConnectionError( - 'Could not connect to Redis', - new Error('Connection failed') - ) - ); - expect(mockDefaultClientConnect).toHaveBeenCalledTimes(1); - expect(consoleErrorSpy).toHaveBeenCalledWith( - 'Failed to connect to Redis:', - expect.any(Error) - ); + // Act + await persistenceLayer._putRecord(record); - // Cleanup - consoleErrorSpy.mockRestore(); + // Assess + expect(client.set).toHaveBeenCalledWith( + dummyKey, + JSON.stringify({ + status, + expiration: expiryTimestamp, + in_progress_expiration: inProgressExpiryTimestamp, + }), + { EX: 10, NX: true } + ); }); - it('should not attempt to connect if the client is already connected', async () => { + it('puts record in Redis when using payload validation', async () => { // Prepare - const connectedClient = { - isOpen: true, - connect: vi.fn().mockResolvedValue(undefined), - }; - - (RedisConnection as unknown as Mock).mockImplementation(() => ({ - getClient: vi.fn().mockReturnValue(connectedClient), - })); - - const layer = new RedisPersistenceLayerTestClass({}); + const persistenceLayerSpy = vi + .spyOn(persistenceLayer, 'isPayloadValidationEnabled') + .mockReturnValue(true); + const expiryTimestamp = getFutureTimestamp(10); + const record = new IdempotencyRecord({ + idempotencyKey: dummyKey, + status: IdempotencyRecordStatus.INPROGRESS, + expiryTimestamp, + payloadHash: 'someHash', + }); // Act - await layer.init(); + await persistenceLayer._putRecord(record); // Assess - expect(RedisConnection).toHaveBeenCalledTimes(1); - expect(connectedClient.connect).not.toHaveBeenCalled(); + expect(client.set).toHaveBeenCalledWith( + dummyKey, + JSON.stringify({ + status: IdempotencyRecordStatus.INPROGRESS, + expiration: expiryTimestamp, + validation: 'someHash', + }), + { EX: 10, NX: true } + ); + persistenceLayerSpy.mockRestore(); }); - it('will do nothing if a user-provided client is used', async () => { + it('puts record in Redis with default expiry timestamp', async () => { // Prepare - const layer = new RedisPersistenceLayerTestClass({ - client: mockUserProvidedClient, + const status = IdempotencyRecordStatus.INPROGRESS; + const record = new IdempotencyRecord({ + idempotencyKey: dummyKey, + status, }); // Act - await layer.init(); + await persistenceLayer._putRecord(record); // Assess - expect(RedisConnection).not.toHaveBeenCalled(); - expect(mockDefaultClientConnect).not.toHaveBeenCalled(); + expect(client.set).toHaveBeenCalledWith( + dummyKey, + JSON.stringify({ + status, + }), + { EX: 60 * 60, NX: true } + ); }); - }); - describe.each([ - { - name: 'when default Redis client is used', - client: mockDefaultClient, - getLayerConfig: () => ({}), - }, - { - name: 'when a user-provided client is used', - client: mockUserProvidedClient, - getLayerConfig: () => ({ client: mockUserProvidedClient }), - }, - ])('$name', ({ client, getLayerConfig }) => { - let layer: RedisPersistenceLayerTestClass; - - beforeEach(async () => { - layer = new RedisPersistenceLayerTestClass(getLayerConfig()); - await layer.init(); - }); + it('handles orphaned records by acquiring a lock and updating', async () => { + // Prepare + const record = new IdempotencyRecord({ + idempotencyKey: dummyKey, + status: IdempotencyRecordStatus.INPROGRESS, + expiryTimestamp: getFutureTimestamp(10), + }); - describe('Method: _putRecord', () => { - it('puts a record with INPROGRESS status into Redis', async () => { - // Prepare - const record = new IdempotencyRecord({ - idempotencyKey: dummyKey, + client.set.mockResolvedValueOnce(null).mockResolvedValueOnce('OK'); + client.get.mockResolvedValueOnce( + JSON.stringify({ status: IdempotencyRecordStatus.INPROGRESS, - expiryTimestamp: getFutureTimestamp(10), - }); - client.set.mockResolvedValue('OK'); - - // Act - await layer._putRecord(record); - - // Assess - expect(client.set).toHaveBeenCalledWith( - dummyKey, - JSON.stringify({ - status: IdempotencyRecordStatus.INPROGRESS, - expiration: record.expiryTimestamp, - }), - { EX: 10, NX: true } - ); - }); + expiration: getFutureTimestamp(-10), + }) + ); + const consoleDebugSpy = vi.spyOn(console, 'debug'); - it('puts the record in Redis when using an in progress expiry timestamp', async () => { - // Prepare - const status = IdempotencyRecordStatus.INPROGRESS; - const expiryTimestamp = getFutureTimestamp(10); - const inProgressExpiryTimestamp = getFutureTimestampInMillis(5); - const record = new IdempotencyRecord({ - idempotencyKey: dummyKey, - status, - expiryTimestamp, - inProgressExpiryTimestamp, - }); - - // Act - await layer._putRecord(record); - - // Assess - expect(client.set).toHaveBeenCalledWith( - dummyKey, - JSON.stringify({ - status, - expiration: expiryTimestamp, - in_progress_expiration: inProgressExpiryTimestamp, - }), - { EX: 10, NX: true } - ); - }); + // Act + await persistenceLayer._putRecord(record); - it('puts record in Redis when using payload validation', async () => { - // Prepare - const persistenceLayerSpy = vi - .spyOn(layer, 'isPayloadValidationEnabled') - .mockReturnValue(true); - const expiryTimestamp = getFutureTimestamp(10); - const record = new IdempotencyRecord({ - idempotencyKey: dummyKey, + // Assess + expect(consoleDebugSpy).toHaveBeenCalledWith( + 'Acquiring lock to overwrite orphan record' + ); + expect(client.set).toHaveBeenCalledWith( + `${dummyKey}:lock`, + 'true', + expect.objectContaining({ EX: 10, NX: true }) + ); + expect(consoleDebugSpy).toHaveBeenCalledWith( + 'Lock acquired, updating record' + ); + expect(client.set).toHaveBeenCalledWith( + dummyKey, + JSON.stringify({ status: IdempotencyRecordStatus.INPROGRESS, - expiryTimestamp, - payloadHash: 'someHash', - }); - - // Act - await layer._putRecord(record); - - // Assess - expect(client.set).toHaveBeenCalledWith( - dummyKey, - JSON.stringify({ - status: IdempotencyRecordStatus.INPROGRESS, - expiration: expiryTimestamp, - validation: 'someHash', - }), - { EX: 10, NX: true } - ); - persistenceLayerSpy.mockRestore(); - }); + expiration: record.expiryTimestamp, + }), + { EX: 10 } + ); + }); - it('puts record in Redis with default expiry timestamp', async () => { - // Prepare - const status = IdempotencyRecordStatus.INPROGRESS; - const record = new IdempotencyRecord({ - idempotencyKey: dummyKey, - status, - }); - - // Act - await layer._putRecord(record); - - // Assess - expect(client.set).toHaveBeenCalledWith( - dummyKey, - JSON.stringify({ - status, - }), - { EX: 60 * 60, NX: true } - ); + it('handles orphaned records by acquiring a lock but it fails and throw error', async () => { + // Prepare + const record = new IdempotencyRecord({ + idempotencyKey: dummyKey, + status: IdempotencyRecordStatus.INPROGRESS, + expiryTimestamp: getFutureTimestamp(10), }); - it('handles orphaned records by acquiring a lock and updating', async () => { - // Prepare - const record = new IdempotencyRecord({ - idempotencyKey: dummyKey, + client.set + .mockResolvedValueOnce(null) // First attempt to set fails + .mockResolvedValueOnce(null); // Lock acquisition fails + client.get.mockResolvedValueOnce( + JSON.stringify({ status: IdempotencyRecordStatus.INPROGRESS, - expiryTimestamp: getFutureTimestamp(10), - }); - - client.set.mockResolvedValueOnce(null).mockResolvedValueOnce('OK'); - client.get.mockResolvedValueOnce( - JSON.stringify({ - status: IdempotencyRecordStatus.INPROGRESS, - expiration: getFutureTimestamp(-10), - }) - ); - const consoleDebugSpy = vi.spyOn(console, 'debug'); - - // Act - await layer._putRecord(record); - - // Assess - expect(consoleDebugSpy).toHaveBeenCalledWith( - 'Acquiring lock to overwrite orphan record' - ); - expect(client.set).toHaveBeenCalledWith( - `${dummyKey}:lock`, - 'true', - expect.objectContaining({ EX: 10, NX: true }) - ); - expect(consoleDebugSpy).toHaveBeenCalledWith( - 'Lock acquired, updating record' - ); - expect(client.set).toHaveBeenCalledWith( - dummyKey, - JSON.stringify({ - status: IdempotencyRecordStatus.INPROGRESS, - expiration: record.expiryTimestamp, - }), - { EX: 10 } - ); - }); + expiration: getFutureTimestamp(-10), + }) + ); - it('handles orphaned records by acquiring a lock but it fails and throw error', async () => { - // Prepare - const record = new IdempotencyRecord({ - idempotencyKey: dummyKey, - status: IdempotencyRecordStatus.INPROGRESS, - expiryTimestamp: getFutureTimestamp(10), - }); - - client.set - .mockResolvedValueOnce(null) // First attempt to set fails - .mockResolvedValueOnce(null); // Lock acquisition fails - client.get.mockResolvedValueOnce( - JSON.stringify({ - status: IdempotencyRecordStatus.INPROGRESS, - expiration: getFutureTimestamp(-10), - }) - ); - - // Act & Assess - await expect(layer._putRecord(record)).rejects.toThrow( - 'Lock acquisition failed, raise to retry' - ); - }); + // Act & Assess + await expect(persistenceLayer._putRecord(record)).rejects.toThrow( + 'Lock acquisition failed, raise to retry' + ); + }); - it('throws error when item already exists and not expired', async () => { - // Prepare - const record = new IdempotencyRecord({ - idempotencyKey: dummyKey, - status: IdempotencyRecordStatus.INPROGRESS, - expiryTimestamp: getFutureTimestamp(10), - }); - client.set.mockResolvedValue(null); - client.get.mockResolvedValueOnce( - JSON.stringify({ - status: IdempotencyRecordStatus.COMPLETED, - expiration: getFutureTimestamp(10), - }) - ); - - // Act & Assess - await expect(layer._putRecord(record)).rejects.toThrow( - `Failed to put record for already existing idempotency key: ${dummyKey}` - ); + it('throws error when item already exists and not expired', async () => { + // Prepare + const record = new IdempotencyRecord({ + idempotencyKey: dummyKey, + status: IdempotencyRecordStatus.INPROGRESS, + expiryTimestamp: getFutureTimestamp(10), }); + client.set.mockResolvedValue(null); + client.get.mockResolvedValueOnce( + JSON.stringify({ + status: IdempotencyRecordStatus.COMPLETED, + expiration: getFutureTimestamp(10), + }) + ); - it('throws error when item is in progress', async () => { - // Prepare - const inProgressExpiryTimestamp = getFutureTimestampInMillis(10); - const record = new IdempotencyRecord({ - idempotencyKey: dummyKey, - status: IdempotencyRecordStatus.INPROGRESS, - expiryTimestamp: getFutureTimestamp(10), - }); - client.set.mockResolvedValue(null); - client.get.mockResolvedValueOnce( - JSON.stringify({ - status: IdempotencyRecordStatus.INPROGRESS, - in_progress_expiration: inProgressExpiryTimestamp, - }) - ); - - // Act & Assess - await expect(layer._putRecord(record)).rejects.toThrow( - `Failed to put record for in-progress idempotency key: ${dummyKey}` - ); + // Act & Assess + await expect(persistenceLayer._putRecord(record)).rejects.toThrow( + `Failed to put record for already existing idempotency key: ${dummyKey}` + ); + }); + + it('throws error when item is in progress', async () => { + // Prepare + const inProgressExpiryTimestamp = getFutureTimestampInMillis(10); + const record = new IdempotencyRecord({ + idempotencyKey: dummyKey, + status: IdempotencyRecordStatus.INPROGRESS, + expiryTimestamp: getFutureTimestamp(10), }); + client.set.mockResolvedValue(null); + client.get.mockResolvedValueOnce( + JSON.stringify({ + status: IdempotencyRecordStatus.INPROGRESS, + in_progress_expiration: inProgressExpiryTimestamp, + }) + ); - it('throws error when trying to put a non-INPROGRESS record', async () => { - // Prepare - const record = new IdempotencyRecord({ - idempotencyKey: dummyKey, - status: IdempotencyRecordStatus.COMPLETED, - expiryTimestamp: getFutureTimestamp(10), - }); + // Act & Assess + await expect(persistenceLayer._putRecord(record)).rejects.toThrow( + `Failed to put record for in-progress idempotency key: ${dummyKey}` + ); + }); - // Act & Assess - await expect(layer._putRecord(record)).rejects.toThrow( - 'Only INPROGRESS records can be inserted with _putRecord' - ); + it('throws error when trying to put a non-INPROGRESS record', async () => { + // Prepare + const record = new IdempotencyRecord({ + idempotencyKey: dummyKey, + status: IdempotencyRecordStatus.COMPLETED, + expiryTimestamp: getFutureTimestamp(10), }); + + // Act & Assess + await expect(persistenceLayer._putRecord(record)).rejects.toThrow( + 'Only INPROGRESS records can be inserted with _putRecord' + ); }); + }); - describe('Method: _deleteRecord', () => { - it('deletes a record from Redis', async () => { - // Prepare - const record = new IdempotencyRecord({ - idempotencyKey: dummyKey, - status: IdempotencyRecordStatus.COMPLETED, - expiryTimestamp: getFutureTimestamp(15), - }); - const consoleDebugSpy = vi.spyOn(console, 'debug'); - - // Act - await layer._deleteRecord(record); - - // Assess - expect(client.del).toHaveBeenCalledWith([dummyKey]); - expect(consoleDebugSpy).toHaveBeenCalledWith( - `Deleting record for idempotency key: ${record.idempotencyKey}` - ); + describe('Method: _deleteRecord', () => { + it('deletes a record from Redis', async () => { + // Prepare + const record = new IdempotencyRecord({ + idempotencyKey: dummyKey, + status: IdempotencyRecordStatus.COMPLETED, + expiryTimestamp: getFutureTimestamp(15), }); + const consoleDebugSpy = vi.spyOn(console, 'debug'); + + // Act + await persistenceLayer._deleteRecord(record); + + // Assess + expect(client.del).toHaveBeenCalledWith([dummyKey]); + expect(consoleDebugSpy).toHaveBeenCalledWith( + `Deleting record for idempotency key: ${record.idempotencyKey}` + ); }); + }); - describe('Method: _getRecord', () => { - it('gets a record from Redis', async () => { - // Prepare - const status = IdempotencyRecordStatus.INPROGRESS; - const expiryTimestamp = getFutureTimestamp(15); - const inProgressExpiryTimestamp = getFutureTimestampInMillis(15); - client.get.mockResolvedValue( - JSON.stringify({ - status, - expiration: expiryTimestamp, - in_progress_expiration: inProgressExpiryTimestamp, - validation: 'someHash', - data: { some: 'data' }, - }) - ); - - // Act - const record = await layer._getRecord(dummyKey); - - // Assess - expect(client.get).toHaveBeenCalledWith(dummyKey); - expect(record.getStatus()).toEqual(status); - expect(record.expiryTimestamp).toEqual(expiryTimestamp); - expect(record.inProgressExpiryTimestamp).toEqual( - inProgressExpiryTimestamp - ); - expect(record.payloadHash).toEqual('someHash'); - expect(record.getResponse()).toEqual({ some: 'data' }); - }); + describe('Method: _getRecord', () => { + it('gets a record from Redis', async () => { + // Prepare + const status = IdempotencyRecordStatus.INPROGRESS; + const expiryTimestamp = getFutureTimestamp(15); + const inProgressExpiryTimestamp = getFutureTimestampInMillis(15); + client.get.mockResolvedValue( + JSON.stringify({ + status, + expiration: expiryTimestamp, + in_progress_expiration: inProgressExpiryTimestamp, + validation: 'someHash', + data: { some: 'data' }, + }) + ); - it('throws IdempotencyItemNotFoundError when record does not exist', async () => { - // Prepare - client.get.mockResolvedValue(null); + // Act + const record = await persistenceLayer._getRecord(dummyKey); - // Act & Assess - await expect(layer._getRecord(dummyKey)).rejects.toThrow( - IdempotencyItemNotFoundError - ); - }); + // Assess + expect(client.get).toHaveBeenCalledWith(dummyKey); + expect(record.getStatus()).toEqual(status); + expect(record.expiryTimestamp).toEqual(expiryTimestamp); + expect(record.inProgressExpiryTimestamp).toEqual( + inProgressExpiryTimestamp + ); + expect(record.payloadHash).toEqual('someHash'); + expect(record.getResponse()).toEqual({ some: 'data' }); + }); - it('throws IdempotencyPersistenceConsistencyError when record is invalid JSON', async () => { - // Prepare - client.get.mockResolvedValue('invalid-json'); + it('throws IdempotencyItemNotFoundError when record does not exist', async () => { + // Prepare + client.get.mockResolvedValue(null); - // Act & Assess - await expect(layer._getRecord(dummyKey)).rejects.toThrow( - IdempotencyPersistenceConsistencyError - ); - }); + // Act & Assess + await expect(persistenceLayer._getRecord(dummyKey)).rejects.toThrow( + IdempotencyItemNotFoundError + ); }); - describe('Method: _updateRecord', () => { - it('updates a record in Redis', async () => { - // Prepare - const record = new IdempotencyRecord({ - idempotencyKey: dummyKey, - status: IdempotencyRecordStatus.COMPLETED, - expiryTimestamp: getFutureTimestamp(15), - }); - client.set.mockResolvedValue('OK'); - - // Act - await layer._updateRecord(record); - - // Assess - expect(client.set).toHaveBeenCalledWith( - dummyKey, - JSON.stringify({ - status: 'COMPLETED', - expiration: record.expiryTimestamp, - }), - expect.objectContaining({ EX: expect.any(Number) }) - ); + it('throws IdempotencyPersistenceConsistencyError when record is invalid JSON', async () => { + // Prepare + client.get.mockResolvedValue('invalid-json'); + + // Act & Assess + await expect(persistenceLayer._getRecord(dummyKey)).rejects.toThrow( + IdempotencyPersistenceConsistencyError + ); + }); + }); + + describe('Method: _updateRecord', () => { + it('updates a record in Redis', async () => { + // Prepare + const record = new IdempotencyRecord({ + idempotencyKey: dummyKey, + status: IdempotencyRecordStatus.COMPLETED, + expiryTimestamp: getFutureTimestamp(15), }); + client.set.mockResolvedValue('OK'); - it('updates a record with null responseData', async () => { - // Prepare - const record = new IdempotencyRecord({ - idempotencyKey: dummyKey, - status: IdempotencyRecordStatus.COMPLETED, - expiryTimestamp: getFutureTimestamp(15), - responseData: undefined, - }); - client.set.mockResolvedValue('OK'); - - // Act - await layer._updateRecord(record); - - // Assess - expect(client.set).toHaveBeenCalledWith( - dummyKey, - JSON.stringify({ - status: 'COMPLETED', - expiration: record.expiryTimestamp, - }), - expect.objectContaining({ EX: expect.any(Number) }) - ); + // Act + await persistenceLayer._updateRecord(record); + + // Assess + expect(client.set).toHaveBeenCalledWith( + dummyKey, + JSON.stringify({ + status: 'COMPLETED', + expiration: record.expiryTimestamp, + }), + expect.objectContaining({ EX: expect.any(Number) }) + ); + }); + + it('updates a record with null responseData', async () => { + // Prepare + const record = new IdempotencyRecord({ + idempotencyKey: dummyKey, + status: IdempotencyRecordStatus.COMPLETED, + expiryTimestamp: getFutureTimestamp(15), + responseData: undefined, }); + client.set.mockResolvedValue('OK'); - it('updates a record with valid responseData', async () => { - // Prepare - const record = new IdempotencyRecord({ - idempotencyKey: dummyKey, - status: IdempotencyRecordStatus.COMPLETED, - expiryTimestamp: getFutureTimestamp(15), - responseData: { key: 'value' }, - }); - client.set.mockResolvedValue('OK'); - - // Act - await layer._updateRecord(record); - - // Assess - expect(client.set).toHaveBeenCalledWith( - dummyKey, - JSON.stringify({ - status: 'COMPLETED', - expiration: record.expiryTimestamp, - data: record.responseData, - }), - expect.objectContaining({ EX: expect.any(Number) }) - ); + // Act + await persistenceLayer._updateRecord(record); + + // Assess + expect(client.set).toHaveBeenCalledWith( + dummyKey, + JSON.stringify({ + status: 'COMPLETED', + expiration: record.expiryTimestamp, + }), + expect.objectContaining({ EX: expect.any(Number) }) + ); + }); + + it('updates a record with valid responseData', async () => { + // Prepare + const record = new IdempotencyRecord({ + idempotencyKey: dummyKey, + status: IdempotencyRecordStatus.COMPLETED, + expiryTimestamp: getFutureTimestamp(15), + responseData: { key: 'value' }, }); + client.set.mockResolvedValue('OK'); + + // Act + await persistenceLayer._updateRecord(record); + + // Assess + expect(client.set).toHaveBeenCalledWith( + dummyKey, + JSON.stringify({ + status: 'COMPLETED', + expiration: record.expiryTimestamp, + data: record.responseData, + }), + expect.objectContaining({ EX: expect.any(Number) }) + ); }); }); }); From 8b73fda896b24656f13c283369ef890ab080ea53 Mon Sep 17 00:00:00 2001 From: arnabrahman Date: Wed, 7 May 2025 19:12:15 +0600 Subject: [PATCH 41/51] refactor: remove `console.debug` statements from `RedisPersistenceLayer` methods and tests --- .../src/persistence/RedisPersistenceLayer.ts | 9 --------- .../unit/persistence/RedisPersistenceLayer.test.ts | 11 ----------- 2 files changed, 20 deletions(-) diff --git a/packages/idempotency/src/persistence/RedisPersistenceLayer.ts b/packages/idempotency/src/persistence/RedisPersistenceLayer.ts index cd50de16a..2cf42a714 100644 --- a/packages/idempotency/src/persistence/RedisPersistenceLayer.ts +++ b/packages/idempotency/src/persistence/RedisPersistenceLayer.ts @@ -89,9 +89,6 @@ class RedisPersistenceLayer extends BasePersistenceLayer { * @param record */ protected async _deleteRecord(record: IdempotencyRecord): Promise { - console.debug( - `Deleting record for idempotency key: ${record.idempotencyKey}` - ); await this.#client.del([record.idempotencyKey]); } @@ -188,9 +185,6 @@ class RedisPersistenceLayer extends BasePersistenceLayer { * - SET see https://redis.io/commands/set/ */ - console.debug( - `Putting record on redis for idempotency key: ${record.idempotencyKey}` - ); const response = await this.#client.set( record.idempotencyKey, encodedItem, @@ -267,7 +261,6 @@ class RedisPersistenceLayer extends BasePersistenceLayer { */ await this.#acquireLock(record.idempotencyKey); - console.debug('Lock acquired, updating record'); await this.#client.set(record.idempotencyKey, encodedItem, { EX: ttl, }); @@ -299,7 +292,6 @@ class RedisPersistenceLayer extends BasePersistenceLayer { const lockKey = `${idempotencyKey}:lock`; const lockValue = 'true'; - console.debug('Acquiring lock to overwrite orphan record'); const acquired = await this.#client.set(lockKey, lockValue, { EX: this.#orphanLockTimeout, NX: true, @@ -310,7 +302,6 @@ class RedisPersistenceLayer extends BasePersistenceLayer { * proceeding, we log the event and raise an error to indicate that the current operation should be * retried after the lock is released by the process that currently holds it. */ - console.debug('Lock acquisition failed, raise to retry'); throw new IdempotencyItemAlreadyExistsError( 'Lock acquisition failed, raise to retry' ); diff --git a/packages/idempotency/tests/unit/persistence/RedisPersistenceLayer.test.ts b/packages/idempotency/tests/unit/persistence/RedisPersistenceLayer.test.ts index a29d9cca5..4ddffa57f 100644 --- a/packages/idempotency/tests/unit/persistence/RedisPersistenceLayer.test.ts +++ b/packages/idempotency/tests/unit/persistence/RedisPersistenceLayer.test.ts @@ -161,23 +161,16 @@ describe('Class: RedisPersistenceLayerTestClass', () => { expiration: getFutureTimestamp(-10), }) ); - const consoleDebugSpy = vi.spyOn(console, 'debug'); // Act await persistenceLayer._putRecord(record); // Assess - expect(consoleDebugSpy).toHaveBeenCalledWith( - 'Acquiring lock to overwrite orphan record' - ); expect(client.set).toHaveBeenCalledWith( `${dummyKey}:lock`, 'true', expect.objectContaining({ EX: 10, NX: true }) ); - expect(consoleDebugSpy).toHaveBeenCalledWith( - 'Lock acquired, updating record' - ); expect(client.set).toHaveBeenCalledWith( dummyKey, JSON.stringify({ @@ -278,16 +271,12 @@ describe('Class: RedisPersistenceLayerTestClass', () => { status: IdempotencyRecordStatus.COMPLETED, expiryTimestamp: getFutureTimestamp(15), }); - const consoleDebugSpy = vi.spyOn(console, 'debug'); // Act await persistenceLayer._deleteRecord(record); // Assess expect(client.del).toHaveBeenCalledWith([dummyKey]); - expect(consoleDebugSpy).toHaveBeenCalledWith( - `Deleting record for idempotency key: ${record.idempotencyKey}` - ); }); }); From 3b9d8a21ee9041f9971accd2c011bc7e01e80065 Mon Sep 17 00:00:00 2001 From: arnabrahman Date: Wed, 7 May 2025 19:14:35 +0600 Subject: [PATCH 42/51] fix: add `@valkey/valkey-glide` in dependencies and mark as optional --- packages/idempotency/package.json | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/idempotency/package.json b/packages/idempotency/package.json index 13afa787d..8dbc36d13 100644 --- a/packages/idempotency/package.json +++ b/packages/idempotency/package.json @@ -121,7 +121,8 @@ "@aws-sdk/client-dynamodb": ">=3.x", "@aws-sdk/lib-dynamodb": ">=3.x", "@middy/core": "4.x || 5.x || 6.x", - "@redis/client": "^5.0.1" + "@redis/client": "^5.0.1", + "@valkey/valkey-glide": "^1.3.4" }, "peerDependenciesMeta": { "@aws-sdk/client-dynamodb": { @@ -135,6 +136,9 @@ }, "@redis/client": { "optional": true + }, + "@valkey/valkey-glide": { + "optional": true } }, "keywords": [ From 8c792adee7856e1b1a12f28ee336d9cc87ff9936 Mon Sep 17 00:00:00 2001 From: arnabrahman Date: Wed, 7 May 2025 19:17:43 +0600 Subject: [PATCH 43/51] refactor: remove outdated example usage of `RedisPersistenceLayer` from documentation --- .../idempotency/src/persistence/RedisPersistenceLayer.ts | 8 -------- 1 file changed, 8 deletions(-) diff --git a/packages/idempotency/src/persistence/RedisPersistenceLayer.ts b/packages/idempotency/src/persistence/RedisPersistenceLayer.ts index 2cf42a714..99526185f 100644 --- a/packages/idempotency/src/persistence/RedisPersistenceLayer.ts +++ b/packages/idempotency/src/persistence/RedisPersistenceLayer.ts @@ -32,14 +32,6 @@ import { IdempotencyRecord } from './IdempotencyRecord.js'; * * @example * ```ts - * import { RedisPersistenceLayer } from '@aws-lambda-powertools/idempotency/redis'; - * - * const persistence = await new RedisPersistenceLayer({ url: 'redis://localhost:6379' }).init(); - * ``` - * - * @example - * ```ts - * // Using your own Redis client * import { createClient } from '@redis/client'; * import { RedisPersistenceLayer } from '@aws-lambda-powertools/idempotency/redis'; * import { RedisCompatibleClient } from '@aws-lambda-powertools/idempotency/redis/types'; From d0af3a151a2a14a26ff4ab62fbb42e19a91dea20 Mon Sep 17 00:00:00 2001 From: arnabrahman Date: Wed, 7 May 2025 19:29:34 +0600 Subject: [PATCH 44/51] refactor: update documentation for `RedisPersistenceLayer` and `RedisCompatibleClient` interface to clarify client initialization requirements --- .../idempotency/src/persistence/RedisPersistenceLayer.ts | 8 +++----- packages/idempotency/src/types/RedisPersistence.ts | 3 +++ 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/packages/idempotency/src/persistence/RedisPersistenceLayer.ts b/packages/idempotency/src/persistence/RedisPersistenceLayer.ts index 99526185f..d3d31bffe 100644 --- a/packages/idempotency/src/persistence/RedisPersistenceLayer.ts +++ b/packages/idempotency/src/persistence/RedisPersistenceLayer.ts @@ -17,15 +17,13 @@ import { IdempotencyRecord } from './IdempotencyRecord.js'; /** * Redis persistence layer for idempotency records. * - * This class uses Redis to write and read idempotency records. It supports both the default Redis client - * from @redis/client package as well as custom Redis clients. + * This class uses Redis to write and read idempotency records. It supports any Redis client that + * implements the RedisCompatibleClient interface. * * There are various options to configure the persistence layer, such as attribute names for storing * status, expiry, data, and validation keys in Redis. * - * With default configuration, you don't need to create the Redis client beforehand, the persistence layer - * will create it for you using the provided options. You can also bring your own Redis client by passing - * it through the `client` option. + * You must provide your own connected Redis client instance by passing it through the `client` option. * * See the {@link https://docs.powertools.aws.dev/lambda/typescript/latest/utilities/idempotency/ Idempotency documentation} * for more details on the Redis configuration and usage patterns. diff --git a/packages/idempotency/src/types/RedisPersistence.ts b/packages/idempotency/src/types/RedisPersistence.ts index b93d91cdc..375a63eef 100644 --- a/packages/idempotency/src/types/RedisPersistence.ts +++ b/packages/idempotency/src/types/RedisPersistence.ts @@ -44,6 +44,9 @@ interface RedisCompatibleClient { interface RedisPersistenceOptions { /** * Redis client instance that implements the RedisCompatibleClient interface. + * + * The client must be properly initialized and connected to a Redis server + * before being passed to the `RedisPersistenceLayer`. */ client: RedisCompatibleClient; From 70f3bebe70e3df9c73e9102e055da2149310fff2 Mon Sep 17 00:00:00 2001 From: arnabrahman Date: Wed, 7 May 2025 19:32:17 +0600 Subject: [PATCH 45/51] refactor: remove outdated reference to Redis client documentation from `RedisPersistenceLayer` --- packages/idempotency/src/persistence/RedisPersistenceLayer.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/idempotency/src/persistence/RedisPersistenceLayer.ts b/packages/idempotency/src/persistence/RedisPersistenceLayer.ts index d3d31bffe..bef680044 100644 --- a/packages/idempotency/src/persistence/RedisPersistenceLayer.ts +++ b/packages/idempotency/src/persistence/RedisPersistenceLayer.ts @@ -42,7 +42,6 @@ import { IdempotencyRecord } from './IdempotencyRecord.js'; * }); * ``` * - * @see https://github.com/redis/node-redis/tree/master/packages/client * @category Persistence Layer */ class RedisPersistenceLayer extends BasePersistenceLayer { From 78d3aba502e97374fa52c1f105972a64aaa83be4 Mon Sep 17 00:00:00 2001 From: arnabrahman Date: Wed, 7 May 2025 20:22:27 +0600 Subject: [PATCH 46/51] refactor: introduce `BasePersistenceOptions` interface for `Dynamodb` & `Redis` to extend --- .../src/types/BasePersistenceLayer.ts | 24 ++++++++++++- .../src/types/DynamoDBPersistence.ts | 23 ++++-------- .../idempotency/src/types/RedisPersistence.ts | 36 +++++-------------- packages/idempotency/src/types/index.ts | 1 + 4 files changed, 38 insertions(+), 46 deletions(-) diff --git a/packages/idempotency/src/types/BasePersistenceLayer.ts b/packages/idempotency/src/types/BasePersistenceLayer.ts index c3c7f19e5..8b14013d5 100644 --- a/packages/idempotency/src/types/BasePersistenceLayer.ts +++ b/packages/idempotency/src/types/BasePersistenceLayer.ts @@ -16,4 +16,26 @@ interface BasePersistenceLayerInterface { getRecord(data: unknown): Promise; } -export type { BasePersistenceLayerOptions, BasePersistenceLayerInterface }; +/** + * Base interface for persistence options i.e. DynamoDB, Redis, etc. + * + * @interface + * @property {string} [expiryAttr] - The attribute name for expiry timestamp. Defaults to 'expiration'. + * @property {string} [inProgressExpiryAttr] - The attribute name for in-progress expiry timestamp. Defaults to 'in_progress_expiration'. + * @property {string} [statusAttr] - The attribute name for status. Defaults to 'status'. + * @property {string} [dataAttr] - The attribute name for response data. Defaults to 'data'. + * @property {string} [validationKeyAttr] - The attribute name for hashed representation of the parts of the event used for validation. Defaults to 'validation'. + */ +interface BasePersistenceOptions { + expiryAttr?: string; + inProgressExpiryAttr?: string; + statusAttr?: string; + dataAttr?: string; + validationKeyAttr?: string; +} + +export type { + BasePersistenceLayerOptions, + BasePersistenceLayerInterface, + BasePersistenceOptions, +}; diff --git a/packages/idempotency/src/types/DynamoDBPersistence.ts b/packages/idempotency/src/types/DynamoDBPersistence.ts index 15a3f7ab4..5c1eefd64 100644 --- a/packages/idempotency/src/types/DynamoDBPersistence.ts +++ b/packages/idempotency/src/types/DynamoDBPersistence.ts @@ -2,29 +2,22 @@ import type { DynamoDBClient, DynamoDBClientConfig, } from '@aws-sdk/client-dynamodb'; +import type { BasePersistenceOptions } from './BasePersistenceLayer.js'; /** * Base interface for DynamoPersistenceOptions. * + * @see {@link BasePersistenceOptions} for full list of properties. + * * @interface * @property {string} tableName - The DynamoDB table name. * @property {string} [keyAttr] - The DynamoDB table key attribute name. Defaults to 'id'. - * @property {string} [expiryAttr] - The DynamoDB table expiry attribute name. Defaults to 'expiration'. - * @property {string} [inProgressExpiryAttr] - The DynamoDB table in progress expiry attribute name. Defaults to 'in_progress_expiry_attr'. - * @property {string} [statusAttr] - The DynamoDB table status attribute name. Defaults to 'status'. - * @property {string} [dataAttr] - The DynamoDB table data attribute name. Defaults to 'data'. - * @property {string} [validationKeyAttr] - The DynamoDB table validation key attribute name. Defaults to 'validation'. * @property {string} [sortKeyAttr] - The DynamoDB table sort key attribute name, use only when table has one. Defaults to undefined. * @property {string} [staticPkValue] - The DynamoDB table static partition key value, use only with sortKeyAttr. Defaults to `idempotency#{LAMBDA_FUNCTION_NAME}`. */ -interface DynamoDBPersistenceOptionsBase { +interface DynamoDBPersistenceOptionsBase extends BasePersistenceOptions { tableName: string; keyAttr?: string; - expiryAttr?: string; - inProgressExpiryAttr?: string; - statusAttr?: string; - dataAttr?: string; - validationKeyAttr?: string; sortKeyAttr?: string; staticPkValue?: string; } @@ -64,16 +57,12 @@ interface DynamoDBPersistenceOptionsWithClientInstance /** * Options for the {@link persistence/DynamoDBPersistenceLayer.DynamoDBPersistenceLayer | DynamoDBPersistenceLayer} class constructor. * - * @see {@link DynamoDBPersistenceOptionsBase}, {@link DynamoDBPersistenceOptionsWithClientConfig}, and {@link DynamoDBPersistenceOptionsWithClientInstance} for full list of properties. + * @see {@link BasePersistenceOptions}, {@link DynamoDBPersistenceOptionsBase}, {@link DynamoDBPersistenceOptionsWithClientConfig}, + * {@link DynamoDBPersistenceOptionsWithClientInstance} for full list of properties. * * @type DynamoDBPersistenceOptions * @property {string} tableName - The DynamoDB table name. * @property {string} [keyAttr] - The DynamoDB table key attribute name. Defaults to 'id'. - * @property {string} [expiryAttr] - The DynamoDB table expiry attribute name. Defaults to 'expiration'. - * @property {string} [inProgressExpiryAttr] - The DynamoDB table in progress expiry attribute name. Defaults to 'in_progress_expiry_attr'. - * @property {string} [statusAttr] - The DynamoDB table status attribute name. Defaults to 'status'. - * @property {string} [dataAttr] - The DynamoDB table data attribute name. Defaults to 'data'. - * @property {string} [validationKeyAttr] - The DynamoDB table validation key attribute name. Defaults to 'validation'. * @property {string} [sortKeyAttr] - The DynamoDB table sort key attribute name, use only when table has one. Defaults to undefined. * @property {string} [staticPkValue] - The DynamoDB table static partition key value, use only with sortKeyAttr. Defaults to `idempotency#{LAMBDA_FUNCTION_NAME}`. * @property {DynamoDBClientConfig} [clientConfig] - Optional configuration to pass during client initialization, e.g. AWS region. Mutually exclusive with awsSdkV3Client. diff --git a/packages/idempotency/src/types/RedisPersistence.ts b/packages/idempotency/src/types/RedisPersistence.ts index 375a63eef..2c836b078 100644 --- a/packages/idempotency/src/types/RedisPersistence.ts +++ b/packages/idempotency/src/types/RedisPersistence.ts @@ -1,4 +1,5 @@ import type { JSONValue } from '@aws-lambda-powertools/commons/types'; +import type { BasePersistenceOptions } from './BasePersistenceLayer.js'; /** * Interface for clients compatible with Redis operations. @@ -39,9 +40,14 @@ interface RedisCompatibleClient { } /** - * Options for configuring the Redis persistence layer + * Options for the {@link persistence/RedisPersistenceLayer.RedisPersistenceLayer | RedisPersistenceLayer} class constructor. + * + * @see {@link BasePersistenceOptions} for full list of properties. + * + * @interface + * @property {RedisCompatibleClient} client - The Redis client instance that implements the RedisCompatibleClient interface. */ -interface RedisPersistenceOptions { +interface RedisPersistenceOptions extends BasePersistenceOptions { /** * Redis client instance that implements the RedisCompatibleClient interface. * @@ -49,32 +55,6 @@ interface RedisPersistenceOptions { * before being passed to the `RedisPersistenceLayer`. */ client: RedisCompatibleClient; - - /** - * Redis JSON attribute name for expiry timestamp (default: 'expiration') - */ - expiryAttr?: string; - - /** - * Redis JSON attribute name for in-progress expiry timestamp (default: 'in_progress_expiration') - */ - inProgressExpiryAttr?: string; - - /** - * Redis JSON attribute name for status (default: 'status') - */ - statusAttr?: string; - - /** - * Redis JSON attribute name for response data (default: 'data') - */ - dataAttr?: string; - - /** - * Redis JSON attribute name for hashed representation of the parts of the event used for validation - * (default: 'validation') - */ - validationKeyAttr?: string; } export type { RedisCompatibleClient, RedisPersistenceOptions }; diff --git a/packages/idempotency/src/types/index.ts b/packages/idempotency/src/types/index.ts index 70a799286..c28bf81d8 100644 --- a/packages/idempotency/src/types/index.ts +++ b/packages/idempotency/src/types/index.ts @@ -5,6 +5,7 @@ export type { export type { BasePersistenceLayerInterface, BasePersistenceLayerOptions, + BasePersistenceOptions, } from './BasePersistenceLayer.js'; export type { IdempotencyConfigOptions, From b709f33ae0296526d2de077ba8279173e918e41b Mon Sep 17 00:00:00 2001 From: arnabrahman Date: Wed, 7 May 2025 20:25:08 +0600 Subject: [PATCH 47/51] refactor: update documentation for `RedisPersistenceOptions` to clarify client initialization requirements --- packages/idempotency/src/types/RedisPersistence.ts | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/packages/idempotency/src/types/RedisPersistence.ts b/packages/idempotency/src/types/RedisPersistence.ts index 2c836b078..92f5054f0 100644 --- a/packages/idempotency/src/types/RedisPersistence.ts +++ b/packages/idempotency/src/types/RedisPersistence.ts @@ -45,15 +45,9 @@ interface RedisCompatibleClient { * @see {@link BasePersistenceOptions} for full list of properties. * * @interface - * @property {RedisCompatibleClient} client - The Redis client instance that implements the RedisCompatibleClient interface. + * @property {RedisCompatibleClient} client - The client must be properly initialized and connected to a Redis server */ interface RedisPersistenceOptions extends BasePersistenceOptions { - /** - * Redis client instance that implements the RedisCompatibleClient interface. - * - * The client must be properly initialized and connected to a Redis server - * before being passed to the `RedisPersistenceLayer`. - */ client: RedisCompatibleClient; } From a509f7970fda99f49c851f6ca73723a36ac89032 Mon Sep 17 00:00:00 2001 From: arnabrahman Date: Wed, 7 May 2025 20:51:59 +0600 Subject: [PATCH 48/51] refactor: rename `BasePersistenceOptions` to `BasePersistenceAttributes` for clarity and consistency --- packages/idempotency/src/types/BasePersistenceLayer.ts | 4 ++-- packages/idempotency/src/types/DynamoDBPersistence.ts | 8 ++++---- packages/idempotency/src/types/RedisPersistence.ts | 6 +++--- packages/idempotency/src/types/index.ts | 2 +- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/packages/idempotency/src/types/BasePersistenceLayer.ts b/packages/idempotency/src/types/BasePersistenceLayer.ts index 8b14013d5..c36e2aee6 100644 --- a/packages/idempotency/src/types/BasePersistenceLayer.ts +++ b/packages/idempotency/src/types/BasePersistenceLayer.ts @@ -26,7 +26,7 @@ interface BasePersistenceLayerInterface { * @property {string} [dataAttr] - The attribute name for response data. Defaults to 'data'. * @property {string} [validationKeyAttr] - The attribute name for hashed representation of the parts of the event used for validation. Defaults to 'validation'. */ -interface BasePersistenceOptions { +interface BasePersistenceAttributes { expiryAttr?: string; inProgressExpiryAttr?: string; statusAttr?: string; @@ -37,5 +37,5 @@ interface BasePersistenceOptions { export type { BasePersistenceLayerOptions, BasePersistenceLayerInterface, - BasePersistenceOptions, + BasePersistenceAttributes, }; diff --git a/packages/idempotency/src/types/DynamoDBPersistence.ts b/packages/idempotency/src/types/DynamoDBPersistence.ts index 5c1eefd64..ab8f71394 100644 --- a/packages/idempotency/src/types/DynamoDBPersistence.ts +++ b/packages/idempotency/src/types/DynamoDBPersistence.ts @@ -2,12 +2,12 @@ import type { DynamoDBClient, DynamoDBClientConfig, } from '@aws-sdk/client-dynamodb'; -import type { BasePersistenceOptions } from './BasePersistenceLayer.js'; +import type { BasePersistenceAttributes } from './BasePersistenceLayer.js'; /** * Base interface for DynamoPersistenceOptions. * - * @see {@link BasePersistenceOptions} for full list of properties. + * @see {@link BasePersistenceAttributes} for full list of properties. * * @interface * @property {string} tableName - The DynamoDB table name. @@ -15,7 +15,7 @@ import type { BasePersistenceOptions } from './BasePersistenceLayer.js'; * @property {string} [sortKeyAttr] - The DynamoDB table sort key attribute name, use only when table has one. Defaults to undefined. * @property {string} [staticPkValue] - The DynamoDB table static partition key value, use only with sortKeyAttr. Defaults to `idempotency#{LAMBDA_FUNCTION_NAME}`. */ -interface DynamoDBPersistenceOptionsBase extends BasePersistenceOptions { +interface DynamoDBPersistenceOptionsBase extends BasePersistenceAttributes { tableName: string; keyAttr?: string; sortKeyAttr?: string; @@ -57,7 +57,7 @@ interface DynamoDBPersistenceOptionsWithClientInstance /** * Options for the {@link persistence/DynamoDBPersistenceLayer.DynamoDBPersistenceLayer | DynamoDBPersistenceLayer} class constructor. * - * @see {@link BasePersistenceOptions}, {@link DynamoDBPersistenceOptionsBase}, {@link DynamoDBPersistenceOptionsWithClientConfig}, + * @see {@link BasePersistenceAttributes}, {@link DynamoDBPersistenceOptionsBase}, {@link DynamoDBPersistenceOptionsWithClientConfig}, * {@link DynamoDBPersistenceOptionsWithClientInstance} for full list of properties. * * @type DynamoDBPersistenceOptions diff --git a/packages/idempotency/src/types/RedisPersistence.ts b/packages/idempotency/src/types/RedisPersistence.ts index 92f5054f0..76ce3cebd 100644 --- a/packages/idempotency/src/types/RedisPersistence.ts +++ b/packages/idempotency/src/types/RedisPersistence.ts @@ -1,5 +1,5 @@ import type { JSONValue } from '@aws-lambda-powertools/commons/types'; -import type { BasePersistenceOptions } from './BasePersistenceLayer.js'; +import type { BasePersistenceAttributes } from './BasePersistenceLayer.js'; /** * Interface for clients compatible with Redis operations. @@ -42,12 +42,12 @@ interface RedisCompatibleClient { /** * Options for the {@link persistence/RedisPersistenceLayer.RedisPersistenceLayer | RedisPersistenceLayer} class constructor. * - * @see {@link BasePersistenceOptions} for full list of properties. + * @see {@link BasePersistenceAttributes} for full list of properties. * * @interface * @property {RedisCompatibleClient} client - The client must be properly initialized and connected to a Redis server */ -interface RedisPersistenceOptions extends BasePersistenceOptions { +interface RedisPersistenceOptions extends BasePersistenceAttributes { client: RedisCompatibleClient; } diff --git a/packages/idempotency/src/types/index.ts b/packages/idempotency/src/types/index.ts index c28bf81d8..255ea7ecf 100644 --- a/packages/idempotency/src/types/index.ts +++ b/packages/idempotency/src/types/index.ts @@ -5,7 +5,7 @@ export type { export type { BasePersistenceLayerInterface, BasePersistenceLayerOptions, - BasePersistenceOptions, + BasePersistenceAttributes, } from './BasePersistenceLayer.js'; export type { IdempotencyConfigOptions, From 6e91573b41b3065edfebda001f521c93de80b665 Mon Sep 17 00:00:00 2001 From: arnabrahman Date: Wed, 7 May 2025 20:53:55 +0600 Subject: [PATCH 49/51] refactor: introduce DEFAULT_PERSISTENCE_LAYER_ATTRIBUTES for consistent attribute handling across persistence layers --- packages/idempotency/src/constants.ts | 21 ++++++++++++++++++- packages/idempotency/src/index.ts | 5 ++++- .../persistence/DynamoDBPersistenceLayer.ts | 21 +++++++++++++------ .../src/persistence/RedisPersistenceLayer.ts | 21 +++++++++++++------ .../src/types/BasePersistenceLayer.ts | 2 +- 5 files changed, 55 insertions(+), 15 deletions(-) diff --git a/packages/idempotency/src/constants.ts b/packages/idempotency/src/constants.ts index da6efbf7a..334634ecc 100644 --- a/packages/idempotency/src/constants.ts +++ b/packages/idempotency/src/constants.ts @@ -1,3 +1,4 @@ +import type { BasePersistenceAttributes } from './types/BasePersistenceLayer.js'; /** * Number of times to retry a request in case of `IdempotencyInconsistentStateError` * @@ -20,4 +21,22 @@ const IdempotencyRecordStatus = { EXPIRED: 'EXPIRED', } as const; -export { IdempotencyRecordStatus, MAX_RETRIES }; +/** + * Default persistence attribute key names for persistence layers + */ +const DEFAULT_PERSISTENCE_LAYER_ATTRIBUTES: Record< + keyof Required, + string +> = { + statusAttr: 'status', + expiryAttr: 'expiration', + inProgressExpiryAttr: 'in_progress_expiration', + dataAttr: 'data', + validationKeyAttr: 'validation', +} as const; + +export { + IdempotencyRecordStatus, + MAX_RETRIES, + DEFAULT_PERSISTENCE_LAYER_ATTRIBUTES, +}; diff --git a/packages/idempotency/src/index.ts b/packages/idempotency/src/index.ts index 9d96f9316..d75f41e08 100644 --- a/packages/idempotency/src/index.ts +++ b/packages/idempotency/src/index.ts @@ -12,4 +12,7 @@ export { export { IdempotencyConfig } from './IdempotencyConfig.js'; export { makeIdempotent } from './makeIdempotent.js'; export { idempotent } from './idempotencyDecorator.js'; -export { IdempotencyRecordStatus } from './constants.js'; +export { + IdempotencyRecordStatus, + DEFAULT_PERSISTENCE_LAYER_ATTRIBUTES, +} from './constants.js'; diff --git a/packages/idempotency/src/persistence/DynamoDBPersistenceLayer.ts b/packages/idempotency/src/persistence/DynamoDBPersistenceLayer.ts index 8354f6032..1d5cd5ed2 100644 --- a/packages/idempotency/src/persistence/DynamoDBPersistenceLayer.ts +++ b/packages/idempotency/src/persistence/DynamoDBPersistenceLayer.ts @@ -13,7 +13,10 @@ import { UpdateItemCommand, } from '@aws-sdk/client-dynamodb'; import { marshall, unmarshall } from '@aws-sdk/util-dynamodb'; -import { IdempotencyRecordStatus } from '../constants.js'; +import { + DEFAULT_PERSISTENCE_LAYER_ATTRIBUTES, + IdempotencyRecordStatus, +} from '../constants.js'; import { IdempotencyItemAlreadyExistsError, IdempotencyItemNotFoundError, @@ -65,12 +68,18 @@ class DynamoDBPersistenceLayer extends BasePersistenceLayer { this.tableName = config.tableName; this.keyAttr = config.keyAttr ?? 'id'; - this.statusAttr = config.statusAttr ?? 'status'; - this.expiryAttr = config.expiryAttr ?? 'expiration'; + this.statusAttr = + config.statusAttr ?? DEFAULT_PERSISTENCE_LAYER_ATTRIBUTES.statusAttr; + this.expiryAttr = + config.expiryAttr ?? DEFAULT_PERSISTENCE_LAYER_ATTRIBUTES.expiryAttr; this.inProgressExpiryAttr = - config.inProgressExpiryAttr ?? 'in_progress_expiration'; - this.dataAttr = config.dataAttr ?? 'data'; - this.validationKeyAttr = config.validationKeyAttr ?? 'validation'; + config.inProgressExpiryAttr ?? + DEFAULT_PERSISTENCE_LAYER_ATTRIBUTES.inProgressExpiryAttr; + this.dataAttr = + config.dataAttr ?? DEFAULT_PERSISTENCE_LAYER_ATTRIBUTES.dataAttr; + this.validationKeyAttr = + config.validationKeyAttr ?? + DEFAULT_PERSISTENCE_LAYER_ATTRIBUTES.validationKeyAttr; if (config.sortKeyAttr === this.keyAttr) { throw new Error( `keyAttr [${this.keyAttr}] and sortKeyAttr [${config.sortKeyAttr}] cannot be the same!` diff --git a/packages/idempotency/src/persistence/RedisPersistenceLayer.ts b/packages/idempotency/src/persistence/RedisPersistenceLayer.ts index bef680044..074e968cf 100644 --- a/packages/idempotency/src/persistence/RedisPersistenceLayer.ts +++ b/packages/idempotency/src/persistence/RedisPersistenceLayer.ts @@ -1,5 +1,8 @@ import type { JSONObject } from '@aws-lambda-powertools/commons/types'; -import { IdempotencyRecordStatus } from '../constants.js'; +import { + DEFAULT_PERSISTENCE_LAYER_ATTRIBUTES, + IdempotencyRecordStatus, +} from '../constants.js'; import { IdempotencyItemAlreadyExistsError, IdempotencyItemNotFoundError, @@ -56,12 +59,18 @@ class RedisPersistenceLayer extends BasePersistenceLayer { public constructor(options: RedisPersistenceOptions) { super(); - this.#statusAttr = options.statusAttr ?? 'status'; - this.#expiryAttr = options.expiryAttr ?? 'expiration'; + this.#statusAttr = + options.statusAttr ?? DEFAULT_PERSISTENCE_LAYER_ATTRIBUTES.statusAttr; + this.#expiryAttr = + options.expiryAttr ?? DEFAULT_PERSISTENCE_LAYER_ATTRIBUTES.expiryAttr; this.#inProgressExpiryAttr = - options.inProgressExpiryAttr ?? 'in_progress_expiration'; - this.#dataAttr = options.dataAttr ?? 'data'; - this.#validationKeyAttr = options.validationKeyAttr ?? 'validation'; + options.inProgressExpiryAttr ?? + DEFAULT_PERSISTENCE_LAYER_ATTRIBUTES.inProgressExpiryAttr; + this.#dataAttr = + options.dataAttr ?? DEFAULT_PERSISTENCE_LAYER_ATTRIBUTES.dataAttr; + this.#validationKeyAttr = + options.validationKeyAttr ?? + DEFAULT_PERSISTENCE_LAYER_ATTRIBUTES.validationKeyAttr; this.#orphanLockTimeout = Math.min(10, this.expiresAfterSeconds); this.#client = options.client; } diff --git a/packages/idempotency/src/types/BasePersistenceLayer.ts b/packages/idempotency/src/types/BasePersistenceLayer.ts index c36e2aee6..cef082140 100644 --- a/packages/idempotency/src/types/BasePersistenceLayer.ts +++ b/packages/idempotency/src/types/BasePersistenceLayer.ts @@ -17,7 +17,7 @@ interface BasePersistenceLayerInterface { } /** - * Base interface for persistence options i.e. DynamoDB, Redis, etc. + * Base attributes used by the persistence layer i.e. DynamoDB, Redis, etc. * * @interface * @property {string} [expiryAttr] - The attribute name for expiry timestamp. Defaults to 'expiration'. From 063edff65fff0807234ef437c6721743baf35dea Mon Sep 17 00:00:00 2001 From: arnabrahman Date: Wed, 7 May 2025 21:01:04 +0600 Subject: [PATCH 50/51] refactor: rename `DEFAULT_PERSISTENCE_LAYER_ATTRIBUTES` to `PERSISTENCE_ATTRIBUTE_KEY_MAPPINGS` for clarity and consistency --- packages/idempotency/src/constants.ts | 6 +++--- packages/idempotency/src/index.ts | 2 +- .../src/persistence/DynamoDBPersistenceLayer.ts | 12 ++++++------ .../src/persistence/RedisPersistenceLayer.ts | 12 ++++++------ 4 files changed, 16 insertions(+), 16 deletions(-) diff --git a/packages/idempotency/src/constants.ts b/packages/idempotency/src/constants.ts index 334634ecc..404a0bf95 100644 --- a/packages/idempotency/src/constants.ts +++ b/packages/idempotency/src/constants.ts @@ -22,9 +22,9 @@ const IdempotencyRecordStatus = { } as const; /** - * Default persistence attribute key names for persistence layers + * Base persistence attribute key names for persistence layers */ -const DEFAULT_PERSISTENCE_LAYER_ATTRIBUTES: Record< +const PERSISTENCE_ATTRIBUTE_KEY_MAPPINGS: Record< keyof Required, string > = { @@ -38,5 +38,5 @@ const DEFAULT_PERSISTENCE_LAYER_ATTRIBUTES: Record< export { IdempotencyRecordStatus, MAX_RETRIES, - DEFAULT_PERSISTENCE_LAYER_ATTRIBUTES, + PERSISTENCE_ATTRIBUTE_KEY_MAPPINGS, }; diff --git a/packages/idempotency/src/index.ts b/packages/idempotency/src/index.ts index d75f41e08..6a76aac37 100644 --- a/packages/idempotency/src/index.ts +++ b/packages/idempotency/src/index.ts @@ -14,5 +14,5 @@ export { makeIdempotent } from './makeIdempotent.js'; export { idempotent } from './idempotencyDecorator.js'; export { IdempotencyRecordStatus, - DEFAULT_PERSISTENCE_LAYER_ATTRIBUTES, + PERSISTENCE_ATTRIBUTE_KEY_MAPPINGS, } from './constants.js'; diff --git a/packages/idempotency/src/persistence/DynamoDBPersistenceLayer.ts b/packages/idempotency/src/persistence/DynamoDBPersistenceLayer.ts index 1d5cd5ed2..390c70ecf 100644 --- a/packages/idempotency/src/persistence/DynamoDBPersistenceLayer.ts +++ b/packages/idempotency/src/persistence/DynamoDBPersistenceLayer.ts @@ -14,8 +14,8 @@ import { } from '@aws-sdk/client-dynamodb'; import { marshall, unmarshall } from '@aws-sdk/util-dynamodb'; import { - DEFAULT_PERSISTENCE_LAYER_ATTRIBUTES, IdempotencyRecordStatus, + PERSISTENCE_ATTRIBUTE_KEY_MAPPINGS, } from '../constants.js'; import { IdempotencyItemAlreadyExistsError, @@ -69,17 +69,17 @@ class DynamoDBPersistenceLayer extends BasePersistenceLayer { this.tableName = config.tableName; this.keyAttr = config.keyAttr ?? 'id'; this.statusAttr = - config.statusAttr ?? DEFAULT_PERSISTENCE_LAYER_ATTRIBUTES.statusAttr; + config.statusAttr ?? PERSISTENCE_ATTRIBUTE_KEY_MAPPINGS.statusAttr; this.expiryAttr = - config.expiryAttr ?? DEFAULT_PERSISTENCE_LAYER_ATTRIBUTES.expiryAttr; + config.expiryAttr ?? PERSISTENCE_ATTRIBUTE_KEY_MAPPINGS.expiryAttr; this.inProgressExpiryAttr = config.inProgressExpiryAttr ?? - DEFAULT_PERSISTENCE_LAYER_ATTRIBUTES.inProgressExpiryAttr; + PERSISTENCE_ATTRIBUTE_KEY_MAPPINGS.inProgressExpiryAttr; this.dataAttr = - config.dataAttr ?? DEFAULT_PERSISTENCE_LAYER_ATTRIBUTES.dataAttr; + config.dataAttr ?? PERSISTENCE_ATTRIBUTE_KEY_MAPPINGS.dataAttr; this.validationKeyAttr = config.validationKeyAttr ?? - DEFAULT_PERSISTENCE_LAYER_ATTRIBUTES.validationKeyAttr; + PERSISTENCE_ATTRIBUTE_KEY_MAPPINGS.validationKeyAttr; if (config.sortKeyAttr === this.keyAttr) { throw new Error( `keyAttr [${this.keyAttr}] and sortKeyAttr [${config.sortKeyAttr}] cannot be the same!` diff --git a/packages/idempotency/src/persistence/RedisPersistenceLayer.ts b/packages/idempotency/src/persistence/RedisPersistenceLayer.ts index 074e968cf..9309c9d42 100644 --- a/packages/idempotency/src/persistence/RedisPersistenceLayer.ts +++ b/packages/idempotency/src/persistence/RedisPersistenceLayer.ts @@ -1,7 +1,7 @@ import type { JSONObject } from '@aws-lambda-powertools/commons/types'; import { - DEFAULT_PERSISTENCE_LAYER_ATTRIBUTES, IdempotencyRecordStatus, + PERSISTENCE_ATTRIBUTE_KEY_MAPPINGS, } from '../constants.js'; import { IdempotencyItemAlreadyExistsError, @@ -60,17 +60,17 @@ class RedisPersistenceLayer extends BasePersistenceLayer { super(); this.#statusAttr = - options.statusAttr ?? DEFAULT_PERSISTENCE_LAYER_ATTRIBUTES.statusAttr; + options.statusAttr ?? PERSISTENCE_ATTRIBUTE_KEY_MAPPINGS.statusAttr; this.#expiryAttr = - options.expiryAttr ?? DEFAULT_PERSISTENCE_LAYER_ATTRIBUTES.expiryAttr; + options.expiryAttr ?? PERSISTENCE_ATTRIBUTE_KEY_MAPPINGS.expiryAttr; this.#inProgressExpiryAttr = options.inProgressExpiryAttr ?? - DEFAULT_PERSISTENCE_LAYER_ATTRIBUTES.inProgressExpiryAttr; + PERSISTENCE_ATTRIBUTE_KEY_MAPPINGS.inProgressExpiryAttr; this.#dataAttr = - options.dataAttr ?? DEFAULT_PERSISTENCE_LAYER_ATTRIBUTES.dataAttr; + options.dataAttr ?? PERSISTENCE_ATTRIBUTE_KEY_MAPPINGS.dataAttr; this.#validationKeyAttr = options.validationKeyAttr ?? - DEFAULT_PERSISTENCE_LAYER_ATTRIBUTES.validationKeyAttr; + PERSISTENCE_ATTRIBUTE_KEY_MAPPINGS.validationKeyAttr; this.#orphanLockTimeout = Math.min(10, this.expiresAfterSeconds); this.#client = options.client; } From 42b3e9ee55c6b9aef2d9fe3f294e511f51a7e421 Mon Sep 17 00:00:00 2001 From: arnabrahman Date: Wed, 7 May 2025 21:02:13 +0600 Subject: [PATCH 51/51] refactor: remove unused `DynamoDBClientConfig` import and associated property from `DynamoDBPersistenceLayer` --- .../idempotency/src/persistence/DynamoDBPersistenceLayer.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/idempotency/src/persistence/DynamoDBPersistenceLayer.ts b/packages/idempotency/src/persistence/DynamoDBPersistenceLayer.ts index 390c70ecf..e72cb0a5e 100644 --- a/packages/idempotency/src/persistence/DynamoDBPersistenceLayer.ts +++ b/packages/idempotency/src/persistence/DynamoDBPersistenceLayer.ts @@ -7,7 +7,6 @@ import { ConditionalCheckFailedException, DeleteItemCommand, DynamoDBClient, - type DynamoDBClientConfig, GetItemCommand, PutItemCommand, UpdateItemCommand, @@ -52,7 +51,6 @@ import { IdempotencyRecord } from './IdempotencyRecord.js'; */ class DynamoDBPersistenceLayer extends BasePersistenceLayer { private client: DynamoDBClient; - private clientConfig: DynamoDBClientConfig = {}; private dataAttr: string; private expiryAttr: string; private inProgressExpiryAttr: string;