Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: aspect bind missing ctx #694

Merged
merged 2 commits into from
Oct 31, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion packages/core/src/baseFramework.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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;
}
}
17 changes: 11 additions & 6 deletions packages/core/src/context/midwayContainer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ export class MidwayContainer
* 单个进程中上一次的 applicationContext 的 registry
*/
static parentDefinitionMetadata: Map<string, IObjectDefinitionMetadata[]>;
static wrapperAspect = false;

constructor(baseDir: string = process.cwd(), parent?: IApplicationContext) {
super(baseDir, parent);
Expand Down Expand Up @@ -522,19 +523,21 @@ export class MidwayContainer
);
descriptor.value = async function (...args) {
let error, result;
originMethod = originMethod.bind(this);
const newProceed = (...args) => {
return originMethod.apply(this, 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);
Expand All @@ -558,19 +561,21 @@ export class MidwayContainer
);
descriptor.value = function (...args) {
let error, result;
originMethod = originMethod.bind(this);
const newProceed = (...args) => {
return originMethod.apply(this, 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;
Expand Down
13 changes: 12 additions & 1 deletion packages/core/test/baseFramework.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
BaseFramework,
clearAllModule,
MidwayContainer,
MidwayRequestContainer,
} from '../src';
import * as mm from 'mm';
import sinon = require('sinon');
Expand Down Expand Up @@ -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 () => {
Expand Down Expand Up @@ -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 () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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'];
}
}
12 changes: 12 additions & 0 deletions packages/core/test/fixtures/base-app-aspect/src/aspect/c.ts
Original file line number Diff line number Diff line change
@@ -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');
}
}
}
18 changes: 15 additions & 3 deletions packages/core/test/fixtures/base-app-aspect/src/home.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Provide } from '@midwayjs/decorator';
import { Provide, Inject } from '@midwayjs/decorator';

class Parent {
ddd = 'ddd';
Expand All @@ -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');
}
}
12 changes: 11 additions & 1 deletion packages/core/test/loader.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down Expand Up @@ -628,9 +629,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 () => {
Expand Down
1 change: 1 addition & 0 deletions packages/mock/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"name": "ali-demo"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
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) {
const result = await point.proceed(...point.args); // 执行原方法
return result + ' world';
}
}

Original file line number Diff line number Diff line change
@@ -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,
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@

exports.hello = {
b: 4,
c: 3,
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
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() {
return this.ctx.query.text;
}
}
16 changes: 16 additions & 0 deletions packages/web/test/issue.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
});