diff --git a/packages/idempotency/src/IdempotencyHandler.ts b/packages/idempotency/src/IdempotencyHandler.ts index f84d5768fc..0fcd4f6ded 100644 --- a/packages/idempotency/src/IdempotencyHandler.ts +++ b/packages/idempotency/src/IdempotencyHandler.ts @@ -61,9 +61,9 @@ export class IdempotencyHandler { */ readonly #idempotencyConfig: IdempotencyConfig; /** - * Custom prefix to be used when generating the idempotency key. + * Key prefix resolved by the persistence store for this operation. */ - readonly #keyPrefix: string | undefined; + readonly #resolvedKeyPrefix: string; /** * Persistence layer used to store the idempotency records. */ @@ -88,7 +88,6 @@ export class IdempotencyHandler { this.#functionToMakeIdempotent = functionToMakeIdempotent; this.#functionPayloadToBeHashed = functionPayloadToBeHashed; this.#idempotencyConfig = idempotencyConfig; - this.#keyPrefix = keyPrefix; this.#functionArguments = functionArguments; this.#thisArg = thisArg; @@ -96,8 +95,9 @@ export class IdempotencyHandler { this.#persistenceStore.configure({ config: this.#idempotencyConfig, - keyPrefix: this.#keyPrefix, + keyPrefix, }); + this.#resolvedKeyPrefix = this.#persistenceStore.idempotencyKeyPrefix; } /** @@ -345,6 +345,16 @@ export class IdempotencyHandler { return false; } + /** + * Restore this operation's key prefix on the persistence store. + * + * Operations sharing a store reconfigure its prefix on construction, so it + * is re-applied immediately before each call that hashes a key. + */ + readonly #applyKeyPrefix = (): void => { + this.#persistenceStore.idempotencyKeyPrefix = this.#resolvedKeyPrefix; + }; + /** * Delete an in progress record from the idempotency store. * @@ -386,6 +396,7 @@ export class IdempotencyHandler { result: undefined, }; try { + this.#applyKeyPrefix(); // Resolve the identity before attempting acquisition so that it is also // available when the function runs without acquiring a record (replay). this.#recordIdentity = this.#persistenceStore.getRecordIdentity( @@ -415,6 +426,7 @@ export class IdempotencyHandler { // If the error doesn't include the existing record, we need to fetch // it from the persistence layer. In doing so, we also call the processExistingRecord // method to validate the record and cache it in memory. + this.#applyKeyPrefix(); idempotencyRecord = await this.#persistenceStore.getRecord( this.#functionPayloadToBeHashed ); diff --git a/packages/idempotency/src/persistence/BasePersistenceLayer.ts b/packages/idempotency/src/persistence/BasePersistenceLayer.ts index de15c25627..8f1cb38bd9 100644 --- a/packages/idempotency/src/persistence/BasePersistenceLayer.ts +++ b/packages/idempotency/src/persistence/BasePersistenceLayer.ts @@ -68,6 +68,8 @@ abstract class BasePersistenceLayer implements BasePersistenceLayerInterface { this.idempotencyKeyPrefix = keyPrefix.trim(); } else if (functionName?.trim()) { this.idempotencyKeyPrefix = `${this.#keyPrefixBase}.${functionName.trim()}`; + } else { + this.idempotencyKeyPrefix = this.#keyPrefixBase; } // Prevent reconfiguration diff --git a/packages/idempotency/tests/unit/makeIdempotent.test.ts b/packages/idempotency/tests/unit/makeIdempotent.test.ts index a3740fe4b3..232aec8d9f 100644 --- a/packages/idempotency/tests/unit/makeIdempotent.test.ts +++ b/packages/idempotency/tests/unit/makeIdempotent.test.ts @@ -698,6 +698,72 @@ describe('Function: makeIdempotent', () => { expect(saveSuccessSpy).toHaveBeenCalledWith(event, '123456', withIdentity); }); + it('completes nested operations under their own key prefix when sharing a persistence store', async () => { + // Prepare + const persistenceStore = new PersistenceLayerTestClass(); + const config = new IdempotencyConfig({}); + config.registerLambdaContext(context); + const inner = makeIdempotent(async (_event: unknown) => 'inner', { + persistenceStore, + config, + keyPrefix: 'inner', + }); + const outer = makeIdempotent( + async (event: unknown) => `outer:${await inner(event)}`, + { persistenceStore, config, keyPrefix: 'outer' } + ); + const event = { id: 'order-1' }; + + // Act + const result = await outer(event); + + // Assess + expect(result).toBe('outer:inner'); + const putKeys = persistenceStore._putRecord.mock.calls.map( + ([record]) => record.idempotencyKey + ); + const updateKeys = persistenceStore._updateRecord.mock.calls.map( + ([record]) => record.idempotencyKey + ); + expect(putKeys).toEqual([ + expect.stringMatching(/^outer#/), + expect.stringMatching(/^inner#/), + ]); + expect(updateKeys).toEqual([putKeys[1], putKeys[0]]); + }); + + it('completes a nested operation without a key prefix under the default prefix', async () => { + // Prepare + const persistenceStore = new PersistenceLayerTestClass(); + const config = new IdempotencyConfig({}); + config.registerLambdaContext(context); + const inner = makeIdempotent(async (_event: unknown) => 'inner', { + persistenceStore, + config, + }); + const outer = makeIdempotent( + async (event: unknown) => `outer:${await inner(event)}`, + { persistenceStore, config, keyPrefix: 'outer' } + ); + + // Act + const result = await outer({ id: 'order-1' }); + + // Assess + expect(result).toBe('outer:inner'); + const putKeys = persistenceStore._putRecord.mock.calls.map( + ([record]) => record.idempotencyKey + ); + const updateKeys = persistenceStore._updateRecord.mock.calls.map( + ([record]) => record.idempotencyKey + ); + expect(putKeys).toEqual([ + expect.stringMatching(/^outer#/), + expect.stringMatching(/^my-lambda-function#/), + ]); + expect(updateKeys).toEqual([putKeys[1], putKeys[0]]); + }); + it('uses the specified argument as payload when wrapping an arbitrary function', async () => { // Prepare const config = new IdempotencyConfig({}); diff --git a/packages/idempotency/tests/unit/persistence/BasePersistenceLayer.test.ts b/packages/idempotency/tests/unit/persistence/BasePersistenceLayer.test.ts index e4967e3ad1..3e848ee7e7 100644 --- a/packages/idempotency/tests/unit/persistence/BasePersistenceLayer.test.ts +++ b/packages/idempotency/tests/unit/persistence/BasePersistenceLayer.test.ts @@ -99,6 +99,19 @@ describe('Class: BasePersistenceLayer', () => { ); }); + it('resets the idempotency key prefix when configured without a prefix or function name', () => { + // Prepare + const config = new IdempotencyConfig({}); + const persistenceLayer = new PersistenceLayerTestClass(); + persistenceLayer.configure({ config, keyPrefix: 'custom' }); + + // Act + persistenceLayer.configure({ config }); + + // Assess + expect(persistenceLayer.idempotencyKeyPrefix).toBe('my-lambda-function'); + }); + it('trims the function name before appending as key prefix', () => { // Prepare const config = new IdempotencyConfig({}); @@ -652,6 +665,26 @@ describe('Class: BasePersistenceLayer', () => { ); }); + it('hashes the idempotency key before yielding to the event loop', async () => { + // Prepare + const persistenceLayer = new PersistenceLayerTestClass(); + persistenceLayer.configure({ + config: new IdempotencyConfig({}), + keyPrefix: 'first', + }); + const putRecordSpy = vi.spyOn(persistenceLayer, '_putRecord'); + + // Act + const pending = persistenceLayer.saveInProgress({ foo: 'bar' }, 2000); + persistenceLayer.idempotencyKeyPrefix = 'second'; + await pending; + + // Assess + expect(putRecordSpy).toHaveBeenCalledWith( + expect.objectContaining({ idempotencyKey: 'first#mocked-hash' }) + ); + }); + it('logs a warning when unable to call remainingTimeInMillis() from the context', async () => { // Prepare const persistenceLayer = new PersistenceLayerTestClass();