From 580b64bc4108463630ddadbf78410dd7e086f494 Mon Sep 17 00:00:00 2001 From: Harry Chen Date: Fri, 30 Oct 2020 00:55:11 +0800 Subject: [PATCH 1/2] fix: aspect bind missing ctx --- packages/core/src/context/midwayContainer.ts | 17 +++++++++----- .../fixtures/base-app-aspect/src/aspect/a.ts | 2 +- .../fixtures/base-app-aspect/src/aspect/c.ts | 12 ++++++++++ .../test/fixtures/base-app-aspect/src/home.ts | 18 ++++++++++++--- packages/core/test/loader.test.ts | 11 +++++++++- .../issue/base-app-aspect-throw/package.json | 3 +++ .../src/aspect/testThrow.ts | 22 +++++++++++++++++++ .../src/config/config.default.ts | 13 +++++++++++ .../src/config/config.unittest.ts | 5 +++++ .../src/controller/User.ts | 19 ++++++++++++++++ packages/web/test/issue.test.ts | 16 ++++++++++++++ 11 files changed, 127 insertions(+), 11 deletions(-) create mode 100644 packages/core/test/fixtures/base-app-aspect/src/aspect/c.ts create mode 100644 packages/web/test/fixtures/issue/base-app-aspect-throw/package.json create mode 100644 packages/web/test/fixtures/issue/base-app-aspect-throw/src/aspect/testThrow.ts create mode 100644 packages/web/test/fixtures/issue/base-app-aspect-throw/src/config/config.default.ts create mode 100644 packages/web/test/fixtures/issue/base-app-aspect-throw/src/config/config.unittest.ts create mode 100755 packages/web/test/fixtures/issue/base-app-aspect-throw/src/controller/User.ts diff --git a/packages/core/src/context/midwayContainer.ts b/packages/core/src/context/midwayContainer.ts index 1dd750f06926..6833499ae2aa 100644 --- a/packages/core/src/context/midwayContainer.ts +++ b/packages/core/src/context/midwayContainer.ts @@ -522,19 +522,22 @@ export class MidwayContainer ); descriptor.value = async function (...args) { let error, result; - originMethod = originMethod.bind(this); + const self = this; + const newProceed = (...args) => { + return originMethod.call(self, args); + } const joinPoint = { methodName: name, target: this, args: args, - proceed: originMethod, + proceed: newProceed, }; try { await aspectIns.before?.(joinPoint); if (aspectIns.around) { result = await aspectIns.around(joinPoint); } else { - result = await originMethod(...joinPoint.args); + result = await originMethod.apply(this, joinPoint.args); } joinPoint.proceed = undefined; const resultTemp = await aspectIns.afterReturn?.(joinPoint, result); @@ -558,19 +561,21 @@ export class MidwayContainer ); descriptor.value = function (...args) { let error, result; - originMethod = originMethod.bind(this); + const newProceed = (...args) => { + return originMethod.call(self, args); + } const joinPoint = { methodName: name, target: this, args: args, - proceed: originMethod, + proceed: newProceed, }; try { aspectIns.before?.(joinPoint); if (aspectIns.around) { result = aspectIns.around(joinPoint); } else { - result = originMethod(...joinPoint.args); + result = originMethod.apply(this, joinPoint.args); } const resultTemp = aspectIns.afterReturn?.(joinPoint, result); result = typeof resultTemp === 'undefined' ? result : resultTemp; diff --git a/packages/core/test/fixtures/base-app-aspect/src/aspect/a.ts b/packages/core/test/fixtures/base-app-aspect/src/aspect/a.ts index 23a9c0e0aa36..d53ce8a6e211 100644 --- a/packages/core/test/fixtures/base-app-aspect/src/aspect/a.ts +++ b/packages/core/test/fixtures/base-app-aspect/src/aspect/a.ts @@ -6,6 +6,6 @@ import { Home } from '../home'; export class MyAspect1 implements IMethodAspect { before(point: JoinPoint): any { console.log('change args in aspect1'); - point.args = ['ddd'] + point.args = ['ddd', 'cccc']; } } diff --git a/packages/core/test/fixtures/base-app-aspect/src/aspect/c.ts b/packages/core/test/fixtures/base-app-aspect/src/aspect/c.ts new file mode 100644 index 000000000000..7d037c24274d --- /dev/null +++ b/packages/core/test/fixtures/base-app-aspect/src/aspect/c.ts @@ -0,0 +1,12 @@ +import { Aspect, IMethodAspect, JoinPoint, Provide } from '@midwayjs/decorator'; +import { UserController } from '../home'; + +@Provide() +@Aspect([UserController]) +export class MyAspect3 implements IMethodAspect { + afterThrow(point: JoinPoint, err): any { + if (err.message === 'bbb') { + throw new Error('ccc'); + } + } +} diff --git a/packages/core/test/fixtures/base-app-aspect/src/home.ts b/packages/core/test/fixtures/base-app-aspect/src/home.ts index de58368e7725..f8c4ba889f7c 100644 --- a/packages/core/test/fixtures/base-app-aspect/src/home.ts +++ b/packages/core/test/fixtures/base-app-aspect/src/home.ts @@ -1,4 +1,4 @@ -import { Provide } from '@midwayjs/decorator'; +import { Provide, Inject } from '@midwayjs/decorator'; class Parent { ddd = 'ddd'; @@ -19,11 +19,23 @@ export class Home extends Parent { ccc: string; - hello(data1: string = 'ggg', data2 = 'fff') { - return 'hello world' + data1 + data2; + hello(data1: string = 'ggg', data2: string = 'aaa', data3 = 'fff') { + return 'hello world' + data1 + data2 + data3; } async hello2(data1: string = 'ggg', data2 = 'fff') { return 'hello world' + data1 + data2; } } + + +@Provide() +export class UserController { + + @Inject() + ctx; + + async getUser() { + throw new Error('bbb'); + } +} diff --git a/packages/core/test/loader.test.ts b/packages/core/test/loader.test.ts index 9d66586f20cc..a123ee3a611a 100644 --- a/packages/core/test/loader.test.ts +++ b/packages/core/test/loader.test.ts @@ -628,9 +628,18 @@ describe('/test/loader.test.ts', () => { await loader.refresh(); const home: any = await loader.getApplicationContext().getAsync('home'); - expect(home.hello()).toEqual('hello worlddddfff'); + expect(home.hello()).toEqual('hello worlddddccccfff'); expect(await home.hello1()).toEqual('hello world 1'); expect(await home.hello2()).toEqual('hello worldcccppp'); + + const ctx1 = {id: 1}; + const requestContext = new MidwayRequestContainer(ctx1, loader.getApplicationContext()); + const userController1: any = await requestContext.getAsync('userController'); + try { + await userController1.getUser(); + } catch (err) { + expect(err.message).toMatch('ccc'); + } }); it('should inject global value in component', async () => { diff --git a/packages/web/test/fixtures/issue/base-app-aspect-throw/package.json b/packages/web/test/fixtures/issue/base-app-aspect-throw/package.json new file mode 100644 index 000000000000..621cdc6a4174 --- /dev/null +++ b/packages/web/test/fixtures/issue/base-app-aspect-throw/package.json @@ -0,0 +1,3 @@ +{ + "name": "ali-demo" +} diff --git a/packages/web/test/fixtures/issue/base-app-aspect-throw/src/aspect/testThrow.ts b/packages/web/test/fixtures/issue/base-app-aspect-throw/src/aspect/testThrow.ts new file mode 100644 index 000000000000..72c61451fe02 --- /dev/null +++ b/packages/web/test/fixtures/issue/base-app-aspect-throw/src/aspect/testThrow.ts @@ -0,0 +1,22 @@ +import { Aspect, IMethodAspect, JoinPoint, Provide } from '@midwayjs/decorator'; +import { UserController } from '../controller/User'; + +@Provide() +@Aspect(UserController, 'api') +export class ReportInfo implements IMethodAspect { + async afterThrow(point: JoinPoint, err) { + point.target.ctx.status = 200; + point.target.ctx.body = 'hello'; + } +} + +@Provide() +@Aspect(UserController, 'do*') +export class ReportInfo1 implements IMethodAspect { + async around(point: JoinPoint) { + console.log('---'); + const result = await point.proceed(...point.args); // 执行原方法 + return result + ' world'; + } +} + diff --git a/packages/web/test/fixtures/issue/base-app-aspect-throw/src/config/config.default.ts b/packages/web/test/fixtures/issue/base-app-aspect-throw/src/config/config.default.ts new file mode 100644 index 000000000000..ad227ab0d36b --- /dev/null +++ b/packages/web/test/fixtures/issue/base-app-aspect-throw/src/config/config.default.ts @@ -0,0 +1,13 @@ +'use strict'; + +export const keys = 'key'; + +export const hello = { + a: 1, + b: 2, + d: [1, 2, 3], +}; + +export const plugins = { + bucLogin: false, +}; diff --git a/packages/web/test/fixtures/issue/base-app-aspect-throw/src/config/config.unittest.ts b/packages/web/test/fixtures/issue/base-app-aspect-throw/src/config/config.unittest.ts new file mode 100644 index 000000000000..764021a2a8b6 --- /dev/null +++ b/packages/web/test/fixtures/issue/base-app-aspect-throw/src/config/config.unittest.ts @@ -0,0 +1,5 @@ + +exports.hello = { + b: 4, + c: 3, +}; diff --git a/packages/web/test/fixtures/issue/base-app-aspect-throw/src/controller/User.ts b/packages/web/test/fixtures/issue/base-app-aspect-throw/src/controller/User.ts new file mode 100755 index 000000000000..2c1aa44ce793 --- /dev/null +++ b/packages/web/test/fixtures/issue/base-app-aspect-throw/src/controller/User.ts @@ -0,0 +1,19 @@ +import { Controller, Get, Provide, Inject } from '@midwayjs/decorator'; + +@Provide() +@Controller('/api/user') +export class UserController { + @Inject() + ctx: any; + + @Get('/info') + async api() { + throw new Error('bbb'); + } + + @Get('/ctx_bind') + async doTestCtxBind() { + console.log(this.ctx.path); + return this.ctx.query.text; + } +} diff --git a/packages/web/test/issue.test.ts b/packages/web/test/issue.test.ts index 9c3b4c16aeb8..3f3799fa964a 100644 --- a/packages/web/test/issue.test.ts +++ b/packages/web/test/issue.test.ts @@ -35,5 +35,21 @@ describe('/test/issue.test.ts', () => { expect(result.text).toEqual('hello world'); await closeApp(app); }); + + it('test #683 issue to change ctx correct', async () => { + const app = await creatApp('issue/base-app-aspect-throw'); + let result = await createHttpRequest(app).get('/api/user/info'); + expect(result.status).toEqual(200); + expect(result.text).toEqual('hello'); + + let result2 = await createHttpRequest(app).get('/api/user/info'); + expect(result2.status).toEqual(200); + expect(result2.text).toEqual('hello'); + + let result3 = await createHttpRequest(app).get('/api/user/ctx_bind').query({text: 'hello'}); + expect(result3.status).toEqual(200); + expect(result3.text).toEqual('hello world'); + await closeApp(app); + }); }); From 7336f01d01d6b42c17dcb0e989119c2f93faf021 Mon Sep 17 00:00:00 2001 From: Harry Chen Date: Sat, 31 Oct 2020 12:20:51 +0800 Subject: [PATCH 2/2] fix: run aspect wrapper once in a process --- packages/core/src/baseFramework.ts | 7 ++++++- packages/core/src/context/midwayContainer.ts | 6 +++--- packages/core/test/baseFramework.test.ts | 13 ++++++++++++- packages/core/test/loader.test.ts | 1 + packages/mock/src/utils.ts | 1 + .../base-app-aspect-throw/src/aspect/testThrow.ts | 1 - .../base-app-aspect-throw/src/controller/User.ts | 1 - 7 files changed, 23 insertions(+), 7 deletions(-) diff --git a/packages/core/src/baseFramework.ts b/packages/core/src/baseFramework.ts index 7b3d056a4cca..693e769e793c 100644 --- a/packages/core/src/baseFramework.ts +++ b/packages/core/src/baseFramework.ts @@ -289,6 +289,9 @@ export abstract class BaseFramework< * @private */ private async loadAspect() { + // 每个进程只执行一次拦截器 + if(MidwayContainer.wrapperAspect) return; + // for aop implementation const aspectModules = listModule(ASPECT_KEY); // sort for aspect target @@ -312,7 +315,9 @@ export abstract class BaseFramework< const aspectIns = await this.getApplicationContext().getAsync< IMethodAspect >(aspectData.aspectModule); - await this.getApplicationContext().addAspect(aspectIns, aspectData); + await this.applicationContext.addAspect(aspectIns, aspectData); } + + MidwayContainer.wrapperAspect = true; } } diff --git a/packages/core/src/context/midwayContainer.ts b/packages/core/src/context/midwayContainer.ts index 6833499ae2aa..2e8b6119f0cc 100644 --- a/packages/core/src/context/midwayContainer.ts +++ b/packages/core/src/context/midwayContainer.ts @@ -80,6 +80,7 @@ export class MidwayContainer * 单个进程中上一次的 applicationContext 的 registry */ static parentDefinitionMetadata: Map; + static wrapperAspect = false; constructor(baseDir: string = process.cwd(), parent?: IApplicationContext) { super(baseDir, parent); @@ -522,9 +523,8 @@ export class MidwayContainer ); descriptor.value = async function (...args) { let error, result; - const self = this; const newProceed = (...args) => { - return originMethod.call(self, args); + return originMethod.apply(this, args); } const joinPoint = { methodName: name, @@ -562,7 +562,7 @@ export class MidwayContainer descriptor.value = function (...args) { let error, result; const newProceed = (...args) => { - return originMethod.call(self, args); + return originMethod.apply(this, args); } const joinPoint = { methodName: name, diff --git a/packages/core/test/baseFramework.test.ts b/packages/core/test/baseFramework.test.ts index af4e3b536ddf..6982cf9c7489 100644 --- a/packages/core/test/baseFramework.test.ts +++ b/packages/core/test/baseFramework.test.ts @@ -11,6 +11,7 @@ import { BaseFramework, clearAllModule, MidwayContainer, + MidwayRequestContainer, } from '../src'; import * as mm from 'mm'; import sinon = require('sinon'); @@ -51,6 +52,7 @@ describe('/test/baseFramework.test.ts', () => { beforeEach(() => { clearAllModule(); MidwayContainer.parentDefinitionMetadata = null; + MidwayContainer.wrapperAspect = false; }); it.skip('should load js directory and auto disable', async () => { @@ -476,9 +478,18 @@ describe('/test/baseFramework.test.ts', () => { }); const home: any = await framework.getApplicationContext().getAsync('home'); - expect(home.hello()).toEqual('hello worlddddfff'); + expect(home.hello()).toEqual('hello worlddddccccfff'); expect(await home.hello1()).toEqual('hello world 1'); expect(await home.hello2()).toEqual('hello worldcccppp'); + + const ctx1 = {id: 1}; + const requestContext = new MidwayRequestContainer(ctx1, framework.getApplicationContext()); + const userController1: any = await requestContext.getAsync('userController'); + try { + await userController1.getUser(); + } catch (err) { + expect(err.message).toMatch('ccc'); + } }); it('should inject global value in component', async () => { diff --git a/packages/core/test/loader.test.ts b/packages/core/test/loader.test.ts index a123ee3a611a..1c890fa70946 100644 --- a/packages/core/test/loader.test.ts +++ b/packages/core/test/loader.test.ts @@ -21,6 +21,7 @@ describe('/test/loader.test.ts', () => { beforeEach(() => { clearAllModule(); MidwayContainer.parentDefinitionMetadata = null; + MidwayContainer.wrapperAspect = false; }); it('should create new loader', async () => { const loader = new ContainerLoader({ diff --git a/packages/mock/src/utils.ts b/packages/mock/src/utils.ts index 7430f873a203..552e718652b7 100644 --- a/packages/mock/src/utils.ts +++ b/packages/mock/src/utils.ts @@ -34,6 +34,7 @@ export async function create< process.env.MIDWAY_TS_MODE = 'true'; clearAllModule(); MidwayContainer.parentDefinitionMetadata = null; + MidwayContainer.wrapperAspect = false; let framework: T = null; let DefaultFramework = null; diff --git a/packages/web/test/fixtures/issue/base-app-aspect-throw/src/aspect/testThrow.ts b/packages/web/test/fixtures/issue/base-app-aspect-throw/src/aspect/testThrow.ts index 72c61451fe02..d0dcf0960505 100644 --- a/packages/web/test/fixtures/issue/base-app-aspect-throw/src/aspect/testThrow.ts +++ b/packages/web/test/fixtures/issue/base-app-aspect-throw/src/aspect/testThrow.ts @@ -14,7 +14,6 @@ export class ReportInfo implements IMethodAspect { @Aspect(UserController, 'do*') export class ReportInfo1 implements IMethodAspect { async around(point: JoinPoint) { - console.log('---'); const result = await point.proceed(...point.args); // 执行原方法 return result + ' world'; } diff --git a/packages/web/test/fixtures/issue/base-app-aspect-throw/src/controller/User.ts b/packages/web/test/fixtures/issue/base-app-aspect-throw/src/controller/User.ts index 2c1aa44ce793..2b69559774a7 100755 --- a/packages/web/test/fixtures/issue/base-app-aspect-throw/src/controller/User.ts +++ b/packages/web/test/fixtures/issue/base-app-aspect-throw/src/controller/User.ts @@ -13,7 +13,6 @@ export class UserController { @Get('/ctx_bind') async doTestCtxBind() { - console.log(this.ctx.path); return this.ctx.query.text; } }