Skip to content

Commit

Permalink
fix(idempotency): preserve scope of decorated class (#2693)
Browse files Browse the repository at this point in the history
  • Loading branch information
dreamorosi authored Jun 26, 2024
1 parent 266b19c commit 22ec1f0
Show file tree
Hide file tree
Showing 6 changed files with 73 additions and 10 deletions.
19 changes: 17 additions & 2 deletions packages/idempotency/src/IdempotencyHandler.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import type { Handler } from 'aws-lambda';
import type {
JSONValue,
MiddyLikeRequest,
Expand Down Expand Up @@ -53,6 +54,12 @@ export class IdempotencyHandler<Func extends AnyFunction> {
* Persistence layer used to store the idempotency records.
*/
readonly #persistenceStore: BasePersistenceLayer;
/**
* The `this` context to be used when calling the function.
*
* When decorating a class method, this will be the instance of the class.
*/
readonly #thisArg?: Handler;

public constructor(options: IdempotencyHandlerOptions) {
const {
Expand All @@ -61,11 +68,13 @@ export class IdempotencyHandler<Func extends AnyFunction> {
idempotencyConfig,
functionArguments,
persistenceStore,
thisArg,
} = options;
this.#functionToMakeIdempotent = functionToMakeIdempotent;
this.#functionPayloadToBeHashed = functionPayloadToBeHashed;
this.#idempotencyConfig = idempotencyConfig;
this.#functionArguments = functionArguments;
this.#thisArg = thisArg;

this.#persistenceStore = persistenceStore;

Expand Down Expand Up @@ -121,7 +130,10 @@ export class IdempotencyHandler<Func extends AnyFunction> {
public async getFunctionResult(): Promise<ReturnType<Func>> {
let result;
try {
result = await this.#functionToMakeIdempotent(...this.#functionArguments);
result = await this.#functionToMakeIdempotent.apply(
this.#thisArg,
this.#functionArguments
);
} catch (error) {
await this.#deleteInProgressRecord();
throw error;
Expand Down Expand Up @@ -149,7 +161,10 @@ export class IdempotencyHandler<Func extends AnyFunction> {
public async handle(): Promise<ReturnType<Func>> {
// early return if we should skip idempotency completely
if (this.shouldSkipIdempotency()) {
return await this.#functionToMakeIdempotent(...this.#functionArguments);
return await this.#functionToMakeIdempotent.apply(
this.#thisArg,
this.#functionArguments
);
}

let e;
Expand Down
6 changes: 5 additions & 1 deletion packages/idempotency/src/idempotencyDecorator.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import type { Handler } from 'aws-lambda';
import {
AnyFunction,
ItempotentFunctionOptions,
Expand Down Expand Up @@ -65,7 +66,10 @@ const idempotent = function (
descriptor: PropertyDescriptor
) {
const childFunction = descriptor.value;
descriptor.value = makeIdempotent(childFunction, options);

descriptor.value = async function (this: Handler, ...args: unknown[]) {
return makeIdempotent(childFunction, options).bind(this)(...args);
};

return descriptor;
};
Expand Down
10 changes: 6 additions & 4 deletions packages/idempotency/src/makeIdempotent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,16 +72,17 @@ const isOptionsWithDataIndexArgument = (
*
* ```
*/
const makeIdempotent = <Func extends AnyFunction>(
// eslint-disable-next-line func-style
function makeIdempotent<Func extends AnyFunction>(
fn: Func,
options: ItempotentFunctionOptions<Parameters<Func>>
): ((...args: Parameters<Func>) => ReturnType<Func>) => {
): (...args: Parameters<Func>) => ReturnType<Func> {
const { persistenceStore, config } = options;
const idempotencyConfig = config ? config : new IdempotencyConfig({});

if (!idempotencyConfig.isEnabled()) return fn;

return (...args: Parameters<Func>): ReturnType<Func> => {
return function (this: Handler, ...args: Parameters<Func>): ReturnType<Func> {
let functionPayloadToBeHashed;

if (isFnHandler(fn, args)) {
Expand All @@ -101,8 +102,9 @@ const makeIdempotent = <Func extends AnyFunction>(
persistenceStore: persistenceStore,
functionArguments: args,
functionPayloadToBeHashed,
thisArg: this,
}).handle() as ReturnType<Func>;
};
};
}

export { makeIdempotent };
8 changes: 7 additions & 1 deletion packages/idempotency/src/types/IdempotencyOptions.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import type { Context } from 'aws-lambda';
import type { Context, Handler } from 'aws-lambda';
import type { BasePersistenceLayer } from '../persistence/BasePersistenceLayer.js';
import type { IdempotencyConfig } from '../IdempotencyConfig.js';
import type { JSONValue } from '@aws-lambda-powertools/commons/types';
Expand Down Expand Up @@ -139,6 +139,12 @@ type IdempotencyHandlerOptions = {
* Persistence layer used to store the idempotency records.
*/
persistenceStore: BasePersistenceLayer;
/**
* The `this` context to be used when calling the function.
*
* When decorating a class method, this will be the instance of the class.
*/
thisArg?: Handler;
};

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,14 @@ const dynamoDBPersistenceLayerCustomized = new DynamoDBPersistenceLayer({
const config = new IdempotencyConfig({});

class DefaultLambda implements LambdaInterface {
private readonly message = 'Got test event:';

@idempotent({ persistenceStore: dynamoDBPersistenceLayer })
public async handler(
_event: Record<string, unknown>,
_context: Context
): Promise<void> {
logger.info(`Got test event: ${JSON.stringify(_event)}`);
logger.info(`${this.message} ${JSON.stringify(_event)}`);
// sleep to enforce error with parallel execution
await new Promise((resolve) => setTimeout(resolve, 1000));

Expand Down
36 changes: 35 additions & 1 deletion packages/idempotency/tests/unit/idempotencyDecorator.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import type { IdempotencyRecordOptions } from '../../src/types/index.js';
import { Context } from 'aws-lambda';
import context from '@aws-lambda-powertools/testing-utils/context';
import { IdempotencyRecordStatus } from '../../src/constants.js';
import type { LambdaInterface } from '@aws-lambda-powertools/commons/types';

const mockSaveInProgress = jest
.spyOn(BasePersistenceLayer.prototype, 'saveInProgress')
Expand Down Expand Up @@ -86,7 +87,10 @@ describe('Given a class with a function to decorate', (classWithLambdaHandler =
testingKey: keyValueToBeSaved,
otherKey: 'thisWillNot',
};
beforeEach(() => jest.clearAllMocks());
beforeEach(() => {
jest.clearAllMocks();
jest.resetAllMocks();
});

describe('When wrapping a function with no previous executions', () => {
beforeEach(async () => {
Expand Down Expand Up @@ -313,4 +317,34 @@ describe('Given a class with a function to decorate', (classWithLambdaHandler =
delete process.env.POWERTOOLS_IDEMPOTENCY_DISABLED;
});
});

it('maintains the scope of the decorated function', async () => {
// Prepare
class TestClass implements LambdaInterface {
private readonly foo = 'foo';

@idempotent({
persistenceStore: new PersistenceLayerTestClass(),
})
public async handler(
_event: unknown,
_context: Context
): Promise<string> {
return this.privateMethod();
}

public privateMethod(): string {
return `private ${this.foo}`;
}
}

const handlerClass = new TestClass();
const handler = handlerClass.handler.bind(handlerClass);

// Act
const result = await handler({}, context);

// Assess
expect(result).toBe('private foo');
});
});

0 comments on commit 22ec1f0

Please sign in to comment.