From 8f9a030229390156bc9455e91cf7215266353fe3 Mon Sep 17 00:00:00 2001 From: Andrea Amorosi Date: Sat, 24 May 2025 18:00:10 +0200 Subject: [PATCH 1/3] fix(event-handler): fix decorated scope in appsync events --- .../appsync-events/AppSyncEventsResolver.ts | 14 +-- .../src/appsync-events/Router.ts | 97 ++++++++++++++++++- .../AppSyncEventsResolver.test.ts | 89 +++++++++++++++++ 3 files changed, 189 insertions(+), 11 deletions(-) diff --git a/packages/event-handler/src/appsync-events/AppSyncEventsResolver.ts b/packages/event-handler/src/appsync-events/AppSyncEventsResolver.ts index 457ddbc30..3e54d7452 100644 --- a/packages/event-handler/src/appsync-events/AppSyncEventsResolver.ts +++ b/packages/event-handler/src/appsync-events/AppSyncEventsResolver.ts @@ -114,11 +114,11 @@ class AppSyncEventsResolver extends Router { if (aggregate) { try { return { - events: await (handler as OnPublishHandlerAggregateFn).apply(this, [ + events: await (handler as OnPublishHandlerAggregateFn)( event.events, event, - context, - ]), + context + ), }; } catch (error) { this.logger.error(`An error occurred in handler ${path}`, error); @@ -131,11 +131,11 @@ class AppSyncEventsResolver extends Router { event.events.map(async (message) => { const { id, payload } = message; try { - const result = await (handler as OnPublishHandlerFn).apply(this, [ + const result = await (handler as OnPublishHandlerFn)( payload, event, - context, - ]); + context + ); return { id, payload: result, @@ -173,7 +173,7 @@ class AppSyncEventsResolver extends Router { } const { handler } = routeHandlerOptions; try { - await (handler as OnSubscribeHandler).apply(this, [event, context]); + await (handler as OnSubscribeHandler)(event, context); } catch (error) { this.logger.error(`An error occurred in handler ${path}`, error); if (error instanceof UnauthorizedException) throw error; diff --git a/packages/event-handler/src/appsync-events/Router.ts b/packages/event-handler/src/appsync-events/Router.ts index f9402b53c..32a9c267b 100644 --- a/packages/event-handler/src/appsync-events/Router.ts +++ b/packages/event-handler/src/appsync-events/Router.ts @@ -9,6 +9,9 @@ import type { } from '../types/appsync-events.js'; import { RouteHandlerRegistry } from './RouteHandlerRegistry.js'; +// Simple global approach - store the last instance per router +const routerInstanceMap = new WeakMap(); + /** * Class for registering routes for the `onPublish` and `onSubscribe` events in AWS AppSync Events APIs. */ @@ -194,11 +197,22 @@ class Router { return; } - return (_target, _propertyKey, descriptor: PropertyDescriptor) => { + return (target, _propertyKey, descriptor: PropertyDescriptor) => { const routeOptions = isRecord(handler) ? handler : options; + const originalMethod = descriptor?.value; + const routerInstance = this; + + this.#bindResolveMethodScope(target); + + // Create a handler that uses the captured instance + const boundHandler = (...args: unknown[]) => { + const instance = routerInstanceMap.get(routerInstance); + return originalMethod?.apply(instance, args); + }; + this.onPublishRegistry.register({ path, - handler: descriptor.value, + handler: boundHandler, aggregate: (routeOptions?.aggregate ?? false) as T, }); return descriptor; @@ -273,14 +287,89 @@ class Router { return; } - return (_target, _propertyKey, descriptor: PropertyDescriptor) => { + return (target, propertyKey, descriptor: PropertyDescriptor) => { + const originalMethod = descriptor?.value; + const routerInstance = this; + + // Patch any method that might call resolve() to capture instance + this.#bindResolveMethodScope(target); + + // Create a handler that uses the captured instance + const boundHandler = (...args: unknown[]) => { + const instance = routerInstanceMap.get(routerInstance); + return originalMethod?.apply(instance, args); + }; + this.onSubscribeRegistry.register({ path, - handler: descriptor.value, + handler: boundHandler, }); return descriptor; }; } + + /** + * Binds the resolve method scope to the target object. + * + * We patch any method that might call `resolve()` to ensure that + * the class instance is captured correctly when the method is resolved. We need + * to do this because when a method is decorated, it loses its context and + * the `this` keyword inside the method no longer refers to the class instance of the decorated method. + * + * We need to apply this two-step process because the decorator is applied to the method + * before the class instance is created, so we cannot capture the instance directly. + * + * @param target - The target object whose methods will be patched to capture the instance scope + */ + #bindResolveMethodScope(target: object) { + const routerInstance = this; + + // Patch any method that might call resolve() to capture instance + if (!target.constructor.prototype._powertoolsPatched) { + target.constructor.prototype._powertoolsPatched = true; + + // Get all method names from the prototype + const proto = target.constructor.prototype; + const methodNames = Object.getOwnPropertyNames(proto); + + for (const methodName of methodNames) { + if (methodName === 'constructor') continue; + + const methodDescriptor = Object.getOwnPropertyDescriptor( + proto, + methodName + ); + if ( + methodDescriptor?.value && + typeof methodDescriptor.value === 'function' + ) { + const originalMethodRef = methodDescriptor.value; + const methodSource = originalMethodRef.toString(); + + // Check if this method calls .resolve() on our router instance + if ( + methodSource.includes('.resolve(') || + methodSource.includes('.resolve ') + ) { + const patchedMethod = function (this: unknown, ...args: unknown[]) { + // Capture instance when any method that calls resolve is called + if (this && typeof this === 'object') { + routerInstanceMap.set(routerInstance, this); + } + return originalMethodRef.apply(this, args); + }; + + Object.defineProperty(proto, methodName, { + value: patchedMethod, + writable: true, + configurable: true, + enumerable: true, + }); + } + } + } + } + } } export { Router }; diff --git a/packages/event-handler/tests/unit/appsync-events/AppSyncEventsResolver.test.ts b/packages/event-handler/tests/unit/appsync-events/AppSyncEventsResolver.test.ts index d75bede42..0e153da91 100644 --- a/packages/event-handler/tests/unit/appsync-events/AppSyncEventsResolver.test.ts +++ b/packages/event-handler/tests/unit/appsync-events/AppSyncEventsResolver.test.ts @@ -1,9 +1,11 @@ import context from '@aws-lambda-powertools/testing-utils/context'; +import type { Context } from 'aws-lambda'; import { beforeEach, describe, expect, it, vi } from 'vitest'; import { AppSyncEventsResolver, UnauthorizedException, } from '../../../src/appsync-events/index.js'; +import type { AppSyncEventsSubscribeEvent } from '../../../src/types/appsync-events.js'; import { onPublishEventFactory, onSubscribeEventFactory, @@ -63,6 +65,93 @@ describe('Class: AppSyncEventsResolver', () => { }); }); + it('preserves the scope when decorating methods', async () => { + // Prepare + const app = new AppSyncEventsResolver({ logger: console }); + + class Lambda { + public scope = 'scoped'; + + @app.onPublish('/foo') + public async handleFoo(payload: string) { + return `${this.scope} ${payload}`; + } + + public async handler(event: unknown, context: Context) { + return this.stuff(event, context); + } + + async stuff(event: unknown, context: Context) { + return app.resolve(event, context); + } + } + const lambda = new Lambda(); + const handler = lambda.handler.bind(lambda); + + // Act + const result = await handler( + onPublishEventFactory( + [ + { + id: '1', + payload: 'foo', + }, + ], + { + path: '/foo', + segments: ['foo'], + } + ), + context + ); + + // Assess + expect(result).toEqual({ + events: [ + { + id: '1', + payload: 'scoped foo', + }, + ], + }); + }); + + it('preserves the scope when decorating methods', async () => { + // Prepare + const app = new AppSyncEventsResolver({ logger: console }); + + class Lambda { + public scope = 'scoped'; + + @app.onSubscribe('/foo') + public async handleFoo(payload: AppSyncEventsSubscribeEvent) { + console.debug(`${this.scope} ${payload.info.channel.path}`); + } + + public async handler(event: unknown, context: Context) { + return this.stuff(event, context); + } + + async stuff(event: unknown, context: Context) { + return app.resolve(event, context); + } + } + const lambda = new Lambda(); + const handler = lambda.handler.bind(lambda); + + // Act + await handler( + onSubscribeEventFactory({ + path: '/foo', + segments: ['foo'], + }), + context + ); + + // Assess + expect(console.debug).toHaveBeenCalledWith('scoped /foo'); + }); + it('returns null if there are no onSubscribe handlers', async () => { // Prepare const app = new AppSyncEventsResolver({ logger: console }); From 84cf6c49f590bca6c6b34595a91192cdf65ab606 Mon Sep 17 00:00:00 2001 From: Andrea Amorosi Date: Sat, 24 May 2025 18:04:09 +0200 Subject: [PATCH 2/3] chore: fix imports --- .../tests/unit/appsync-events/AppSyncEventsResolver.test.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/event-handler/tests/unit/appsync-events/AppSyncEventsResolver.test.ts b/packages/event-handler/tests/unit/appsync-events/AppSyncEventsResolver.test.ts index 0e153da91..9896b091b 100644 --- a/packages/event-handler/tests/unit/appsync-events/AppSyncEventsResolver.test.ts +++ b/packages/event-handler/tests/unit/appsync-events/AppSyncEventsResolver.test.ts @@ -4,12 +4,12 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; import { AppSyncEventsResolver, UnauthorizedException, -} from '../../../src/appsync-events/index.js'; -import type { AppSyncEventsSubscribeEvent } from '../../../src/types/appsync-events.js'; +} from '../../src/appsync-events/index.js'; +import type { AppSyncEventsSubscribeEvent } from '../../src/types/appsync-events.js'; import { onPublishEventFactory, onSubscribeEventFactory, -} from '../../helpers/factories.js'; +} from '../helpers/factories.js'; describe('Class: AppSyncEventsResolver', () => { beforeEach(() => { From 15c47c806c055cdd9ed484639df5a77e80f850ef Mon Sep 17 00:00:00 2001 From: Andrea Amorosi Date: Mon, 26 May 2025 18:56:45 +0200 Subject: [PATCH 3/3] chore: fix imports --- .../tests/unit/appsync-events/AppSyncEventsResolver.test.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/event-handler/tests/unit/appsync-events/AppSyncEventsResolver.test.ts b/packages/event-handler/tests/unit/appsync-events/AppSyncEventsResolver.test.ts index 9896b091b..0e153da91 100644 --- a/packages/event-handler/tests/unit/appsync-events/AppSyncEventsResolver.test.ts +++ b/packages/event-handler/tests/unit/appsync-events/AppSyncEventsResolver.test.ts @@ -4,12 +4,12 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; import { AppSyncEventsResolver, UnauthorizedException, -} from '../../src/appsync-events/index.js'; -import type { AppSyncEventsSubscribeEvent } from '../../src/types/appsync-events.js'; +} from '../../../src/appsync-events/index.js'; +import type { AppSyncEventsSubscribeEvent } from '../../../src/types/appsync-events.js'; import { onPublishEventFactory, onSubscribeEventFactory, -} from '../helpers/factories.js'; +} from '../../helpers/factories.js'; describe('Class: AppSyncEventsResolver', () => { beforeEach(() => {