From d169bb2fbbc86335315619866b4134a25296f552 Mon Sep 17 00:00:00 2001 From: killa Date: Thu, 10 Oct 2024 15:21:09 +0800 Subject: [PATCH] fix: fix aop in constructor inject type (#247) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ##### Checklist - [ ] `npm test` passes - [ ] tests and/or benchmarks are included - [ ] documentation is changed or added - [ ] commit message follows commit guidelines ##### Affected core subsystem(s) ##### Description of change ## Summary by CodeRabbit - **New Features** - Enhanced AOP functionality with new hooks and advice injection methods. - Introduced a new module for constructor injection in AOP, improving dependency management. - **Bug Fixes** - Improved sequence of operations in module loading to ensure proper graph building. - **Tests** - Added a new test suite for constructor injection, increasing test coverage for AOP functionality. - **Documentation** - New `package.json` file created for the "aop-module" to define module configuration. --- core/aop-runtime/src/EggObjectAopHook.ts | 26 ++++- core/aop-runtime/test/aop-runtime.test.ts | 98 +++++++++++++++++++ .../modules/constructor_inject_aop/Hello.ts | 26 +++++ .../constructor_inject_aop/package.json | 6 ++ plugin/aop/app.ts | 11 ++- plugin/tegg/lib/EggModuleLoader.ts | 3 +- standalone/standalone/src/EggModuleLoader.ts | 2 +- standalone/standalone/src/Runner.ts | 11 ++- 8 files changed, 176 insertions(+), 7 deletions(-) create mode 100644 core/aop-runtime/test/fixtures/modules/constructor_inject_aop/Hello.ts create mode 100644 core/aop-runtime/test/fixtures/modules/constructor_inject_aop/package.json diff --git a/core/aop-runtime/src/EggObjectAopHook.ts b/core/aop-runtime/src/EggObjectAopHook.ts index 9f5a1243..2909ba7a 100644 --- a/core/aop-runtime/src/EggObjectAopHook.ts +++ b/core/aop-runtime/src/EggObjectAopHook.ts @@ -1,7 +1,10 @@ -import { ASPECT_LIST } from '@eggjs/tegg-types'; +import { ASPECT_LIST, InjectType } from '@eggjs/tegg-types'; import type { EggObject, EggObjectLifeCycleContext, LifecycleHook } from '@eggjs/tegg-types'; import { Aspect } from '@eggjs/aop-decorator'; import { AspectExecutor } from './AspectExecutor'; +import { PrototypeUtil } from '@eggjs/core-decorator'; +import assert from 'node:assert'; +import { EggContainerFactory } from '@eggjs/tegg-runtime'; export class EggObjectAopHook implements LifecycleHook { private hijackMethods(obj: any, aspectList: Array) { @@ -11,6 +14,25 @@ export class EggObjectAopHook implements LifecycleHook) { + if (eggObject.proto.getMetaData(PrototypeUtil.INJECT_TYPE) !== InjectType.CONSTRUCTOR) { + return; + } + for (const aspect of aspectList) { + for (const advice of aspect.adviceList) { + const injectObject = eggObject.proto.injectObjects.find(t => t.objName === advice.name); + assert(injectObject, `not found inject advice ${advice.name}`); + const adviceObj = EggContainerFactory.getEggObject(injectObject!.proto, advice.name); + Object.defineProperty(obj, advice.name, { + value: adviceObj.obj, + enumerable: false, + }); + } + } + } + async postCreate(_: EggObjectLifeCycleContext, eggObject: EggObject): Promise { const aspectList: Array | undefined = eggObject.proto.getMetaData(ASPECT_LIST); if (!aspectList || !aspectList.length) return; @@ -25,12 +47,14 @@ export class EggObjectAopHook implements LifecycleHook { describe('succeed call', () => { @@ -160,4 +161,101 @@ describe('test/aop-runtime.test.ts', () => { }, /Aop Advice\(PointcutAdvice\) not found in loadUnits/); }); }); + + describe('aop constructor should work', () => { + let modules: Array; + let crosscutAdviceFactory: CrosscutAdviceFactory; + let eggObjectAopHook: EggObjectAopHook; + let loadUnitAopHook: LoadUnitAopHook; + let eggPrototypeCrossCutHook: EggPrototypeCrossCutHook; + + beforeEach(async () => { + crosscutAdviceFactory = new CrosscutAdviceFactory(); + eggObjectAopHook = new EggObjectAopHook(); + loadUnitAopHook = new LoadUnitAopHook(crosscutAdviceFactory); + eggPrototypeCrossCutHook = new EggPrototypeCrossCutHook(crosscutAdviceFactory); + EggPrototypeLifecycleUtil.registerLifecycle(eggPrototypeCrossCutHook); + LoadUnitLifecycleUtil.registerLifecycle(loadUnitAopHook); + EggObjectLifecycleUtil.registerLifecycle(eggObjectAopHook); + + modules = await CoreTestHelper.prepareModules([ + path.join(__dirname, '..'), + path.join(__dirname, 'fixtures/modules/constructor_inject_aop'), + path.join(__dirname, 'fixtures/modules/hello_point_cut'), + path.join(__dirname, 'fixtures/modules/hello_cross_cut'), + ], [ + crossCutGraphHook, + pointCutGraphHook, + ]); + }); + + afterEach(async () => { + for (const module of modules) { + await LoadUnitFactory.destroyLoadUnit(module.loadUnit); + await LoadUnitInstanceFactory.destroyLoadUnitInstance(module); + } + EggPrototypeLifecycleUtil.deleteLifecycle(eggPrototypeCrossCutHook); + LoadUnitLifecycleUtil.deleteLifecycle(loadUnitAopHook); + EggObjectLifecycleUtil.deleteLifecycle(eggObjectAopHook); + }); + + it('should work', async () => { + await EggTestContext.mockContext(async () => { + const hello = await CoreTestHelper.getObject(HelloConstructorInject); + const callTrace = await CoreTestHelper.getObject(CallTrace); + const msg = await hello.hello('aop'); + const traceMsg = callTrace.msgs; + console.log('msg: ', msg, traceMsg); + assert.deepStrictEqual(msg, `withPointAroundResult(hello withPointAroundParam(aop)${JSON.stringify(pointcutAdviceParams)})`); + assert.deepStrictEqual(traceMsg, [ + { + className: 'PointcutAdvice', + methodName: 'beforeCall', + id: 233, + name: 'aop', + adviceParams: pointcutAdviceParams, + }, + { + className: 'PointcutAdvice', + methodName: 'afterReturn', + id: 233, + name: 'withPointAroundParam(aop)', + result: `withPointAroundResult(hello withPointAroundParam(aop)${JSON.stringify(pointcutAdviceParams)})`, + adviceParams: pointcutAdviceParams, + }, + { + className: 'PointcutAdvice', + methodName: 'afterFinally', + id: 233, + name: 'withPointAroundParam(aop)', + adviceParams: pointcutAdviceParams, + }, + ]); + + await assert.rejects(async () => { + await hello.helloWithException('foo'); + }, new Error('ops, exception for withPointAroundParam(foo)')); + assert.deepStrictEqual(callTrace.msgs[callTrace.msgs.length - 2], { + className: 'PointcutAdvice', + methodName: 'afterThrow', + id: 233, + name: 'withPointAroundParam(foo)', + result: 'ops, exception for withPointAroundParam(foo)', + adviceParams: pointcutAdviceParams, + }); + }); + }); + + it('mock should work', async () => { + await EggTestContext.mockContext(async () => { + const hello = await CoreTestHelper.getObject(HelloConstructorInject); + let helloMocked = false; + mm(HelloConstructorInject.prototype, 'hello', async () => { + helloMocked = true; + }); + await hello.hello('aop'); + assert(helloMocked); + }); + }); + }); }); diff --git a/core/aop-runtime/test/fixtures/modules/constructor_inject_aop/Hello.ts b/core/aop-runtime/test/fixtures/modules/constructor_inject_aop/Hello.ts new file mode 100644 index 00000000..2fa188d5 --- /dev/null +++ b/core/aop-runtime/test/fixtures/modules/constructor_inject_aop/Hello.ts @@ -0,0 +1,26 @@ +import { ContextProto, Inject, SingletonProto } from '@eggjs/core-decorator'; +import { Pointcut } from '@eggjs/aop-decorator'; +import { PointcutAdvice, pointcutAdviceParams } from '../hello_point_cut/HelloPointCut'; + +@SingletonProto() +export class Foo { +} + +@ContextProto() +export class HelloConstructorInject { + id = 233; + + constructor(@Inject() readonly foo: Foo) { + } + + @Pointcut(PointcutAdvice, { adviceParams: pointcutAdviceParams }) + async hello(name: string) { + return `hello ${name}`; + } + + @Pointcut(PointcutAdvice, { adviceParams: pointcutAdviceParams }) + async helloWithException(name: string) { + throw new Error(`ops, exception for ${name}`); + } + +} diff --git a/core/aop-runtime/test/fixtures/modules/constructor_inject_aop/package.json b/core/aop-runtime/test/fixtures/modules/constructor_inject_aop/package.json new file mode 100644 index 00000000..4dcd7bf5 --- /dev/null +++ b/core/aop-runtime/test/fixtures/modules/constructor_inject_aop/package.json @@ -0,0 +1,6 @@ +{ + "name": "aop-module", + "eggModule": { + "name": "aopModule" + } +} diff --git a/plugin/aop/app.ts b/plugin/aop/app.ts index c2623769..4062b0c8 100644 --- a/plugin/aop/app.ts +++ b/plugin/aop/app.ts @@ -1,7 +1,14 @@ import { Application } from 'egg'; import { CrosscutAdviceFactory } from '@eggjs/tegg/aop'; -import { EggObjectAopHook, EggPrototypeCrossCutHook, LoadUnitAopHook } from '@eggjs/tegg-aop-runtime'; +import { + crossCutGraphHook, + EggObjectAopHook, + EggPrototypeCrossCutHook, + LoadUnitAopHook, + pointCutGraphHook, +} from '@eggjs/tegg-aop-runtime'; import { AopContextHook } from './lib/AopContextHook'; +import { GlobalGraph } from '@eggjs/tegg-metadata'; export default class AopAppHook { private readonly app: Application; @@ -24,6 +31,8 @@ export default class AopAppHook { this.app.eggPrototypeLifecycleUtil.registerLifecycle(this.eggPrototypeCrossCutHook); this.app.loadUnitLifecycleUtil.registerLifecycle(this.loadUnitAopHook); this.app.eggObjectLifecycleUtil.registerLifecycle(this.eggObjectAopHook); + GlobalGraph.instance!.registerBuildHook(crossCutGraphHook); + GlobalGraph.instance!.registerBuildHook(pointCutGraphHook); } async didLoad() { diff --git a/plugin/tegg/lib/EggModuleLoader.ts b/plugin/tegg/lib/EggModuleLoader.ts index bd593585..702d42d7 100644 --- a/plugin/tegg/lib/EggModuleLoader.ts +++ b/plugin/tegg/lib/EggModuleLoader.ts @@ -40,12 +40,11 @@ export class EggModuleLoader { }); } const graph = GlobalGraph.create(moduleDescriptors); - graph.build(); return graph; } private async loadModule() { - this.buildAppGraph(); + this.globalGraph.build(); this.globalGraph.sort(); const moduleConfigList = this.globalGraph.moduleConfigList; for (const moduleConfig of moduleConfigList) { diff --git a/standalone/standalone/src/EggModuleLoader.ts b/standalone/standalone/src/EggModuleLoader.ts index 479ddb2c..3b11342d 100644 --- a/standalone/standalone/src/EggModuleLoader.ts +++ b/standalone/standalone/src/EggModuleLoader.ts @@ -35,12 +35,12 @@ export class EggModuleLoader { }); } const globalGraph = GlobalGraph.create(moduleDescriptors); - globalGraph.build(); return globalGraph; } async load(): Promise { const loadUnits: LoadUnit[] = []; + this.globalGraph.build(); this.globalGraph.sort(); const moduleConfigList = GlobalGraph.instance!.moduleConfigList; for (const moduleConfig of moduleConfigList) { diff --git a/standalone/standalone/src/Runner.ts b/standalone/standalone/src/Runner.ts index eb52b4f2..9bddde88 100644 --- a/standalone/standalone/src/Runner.ts +++ b/standalone/standalone/src/Runner.ts @@ -1,7 +1,7 @@ import { ModuleConfigUtil, ModuleReference, ReadModuleReferenceOptions, RuntimeConfig } from '@eggjs/tegg-common-util'; import { EggPrototype, - EggPrototypeLifecycleUtil, + EggPrototypeLifecycleUtil, GlobalGraph, LoadUnit, LoadUnitFactory, LoadUnitLifecycleUtil, @@ -26,7 +26,12 @@ import { } from '@eggjs/tegg'; import { StandaloneUtil, MainRunner } from '@eggjs/tegg/standalone'; import { CrosscutAdviceFactory } from '@eggjs/tegg/aop'; -import { EggObjectAopHook, EggPrototypeCrossCutHook, LoadUnitAopHook } from '@eggjs/tegg-aop-runtime'; +import { + crossCutGraphHook, + EggObjectAopHook, + EggPrototypeCrossCutHook, + LoadUnitAopHook, pointCutGraphHook, +} from '@eggjs/tegg-aop-runtime'; import { EggModuleLoader } from './EggModuleLoader'; import { InnerObject, StandaloneLoadUnit, StandaloneLoadUnitType } from './StandaloneLoadUnit'; @@ -143,6 +148,8 @@ export class Runner { logger: ((this.innerObjects.logger && this.innerObjects.logger[0])?.obj as Logger) || console, baseDir: this.cwd, }); + GlobalGraph.instance!.registerBuildHook(crossCutGraphHook); + GlobalGraph.instance!.registerBuildHook(pointCutGraphHook); const configSourceEggPrototypeHook = new ConfigSourceLoadUnitHook(); LoadUnitLifecycleUtil.registerLifecycle(configSourceEggPrototypeHook);