Skip to content

Commit

Permalink
fix: fix aop in constructor inject type (#247)
Browse files Browse the repository at this point in the history
<!--
Thank you for your pull request. Please review below requirements.
Bug fixes and new features should include tests and possibly benchmarks.
Contributors guide:
https://github.com/eggjs/egg/blob/master/CONTRIBUTING.md

感谢您贡献代码。请确认下列 checklist 的完成情况。
Bug 修复和新功能必须包含测试,必要时请附上性能测试。
Contributors guide:
https://github.com/eggjs/egg/blob/master/CONTRIBUTING.md
-->

##### Checklist
<!-- Remove items that do not apply. For completed items, change [ ] to
[x]. -->

- [ ] `npm test` passes
- [ ] tests and/or benchmarks are included
- [ ] documentation is changed or added
- [ ] commit message follows commit guidelines

##### Affected core subsystem(s)
<!-- Provide affected core subsystem(s). -->


##### Description of change
<!-- Provide a description of the change below this comment. -->

<!--
- any feature?
- close https://github.com/eggjs/egg/ISSUE_URL
-->

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## 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.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
  • Loading branch information
killagu authored Oct 10, 2024
1 parent ae42093 commit d169bb2
Show file tree
Hide file tree
Showing 8 changed files with 176 additions and 7 deletions.
26 changes: 25 additions & 1 deletion core/aop-runtime/src/EggObjectAopHook.ts
Original file line number Diff line number Diff line change
@@ -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<EggObjectLifeCycleContext, EggObject> {
private hijackMethods(obj: any, aspectList: Array<Aspect>) {
Expand All @@ -11,6 +14,25 @@ export class EggObjectAopHook implements LifecycleHook<EggObjectLifeCycleContext
}
}

// constructor inject only paas obj to constructor
// should manually define obj to property
private injectAdvice(eggObject: EggObject, obj: any, aspectList: Array<Aspect>) {
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<void> {
const aspectList: Array<Aspect> | undefined = eggObject.proto.getMetaData(ASPECT_LIST);
if (!aspectList || !aspectList.length) return;
Expand All @@ -25,12 +47,14 @@ export class EggObjectAopHook implements LifecycleHook<EggObjectLifeCycleContext
if (!obj) {
obj = Reflect.apply(propertyDesc.get!, eggObject, []);
self.hijackMethods(obj, aspectList);
self.injectAdvice(eggObject, obj, aspectList);
}
return obj;
},
});
} else {
this.hijackMethods(eggObject.obj, aspectList);
this.injectAdvice(eggObject, eggObject.obj, aspectList);
}
}
}
98 changes: 98 additions & 0 deletions core/aop-runtime/test/aop-runtime.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import { EggPrototypeCrossCutHook } from '../src/EggPrototypeCrossCutHook';
import { crossCutGraphHook } from '../src/CrossCutGraphHook';
import { pointCutGraphHook } from '../src/PointCutGraphHook';
import { CallTrace } from './fixtures/modules/hello_cross_cut/CallTrace';
import { HelloConstructorInject } from './fixtures/modules/constructor_inject_aop/Hello';

describe('test/aop-runtime.test.ts', () => {
describe('succeed call', () => {
Expand Down Expand Up @@ -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<LoadUnitInstance>;
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);
});
});
});
});
Original file line number Diff line number Diff line change
@@ -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}`);
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"name": "aop-module",
"eggModule": {
"name": "aopModule"
}
}
11 changes: 10 additions & 1 deletion plugin/aop/app.ts
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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() {
Expand Down
3 changes: 1 addition & 2 deletions plugin/tegg/lib/EggModuleLoader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion standalone/standalone/src/EggModuleLoader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,12 @@ export class EggModuleLoader {
});
}
const globalGraph = GlobalGraph.create(moduleDescriptors);
globalGraph.build();
return globalGraph;
}

async load(): Promise<LoadUnit[]> {
const loadUnits: LoadUnit[] = [];
this.globalGraph.build();
this.globalGraph.sort();
const moduleConfigList = GlobalGraph.instance!.moduleConfigList;
for (const moduleConfig of moduleConfigList) {
Expand Down
11 changes: 9 additions & 2 deletions standalone/standalone/src/Runner.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { ModuleConfigUtil, ModuleReference, ReadModuleReferenceOptions, RuntimeConfig } from '@eggjs/tegg-common-util';
import {
EggPrototype,
EggPrototypeLifecycleUtil,
EggPrototypeLifecycleUtil, GlobalGraph,
LoadUnit,
LoadUnitFactory,
LoadUnitLifecycleUtil,
Expand All @@ -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';
Expand Down Expand Up @@ -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);

Expand Down

0 comments on commit d169bb2

Please sign in to comment.