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

feat: lifecycle base class #3581

Merged
merged 1 commit into from
Apr 16, 2019
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
11 changes: 11 additions & 0 deletions agent.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
'use strict';

const BaseHookClass = require('./lib/core/base_hook_class');

class EggAgentHook extends BaseHookClass {
configDidLoad() {
this.agent._wrapMessenger();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

}
}

module.exports = EggAgentHook;
36 changes: 31 additions & 5 deletions index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@ import { HttpClient2, RequestOptions } from 'urllib';
import {
EggCoreBase,
FileLoaderOption,
EggLoader as CoreLoader,
EggCoreOptions as CoreOptions,
EggLoaderOptions as CoreLoaderOptions,
EggLoader as CoreLoader,
EggCoreOptions as CoreOptions,
EggLoaderOptions as CoreLoaderOptions,
BaseContextClass as CoreBaseContextClass,
} from 'egg-core';
import EggCookies = require('egg-cookies');
Expand Down Expand Up @@ -59,6 +59,32 @@ declare module 'egg' {
protected logger: EggLogger;
}

export class Boot {
/**
* logger
* @member {EggLogger}
*/
protected logger: EggLogger;

/**
* The configuration of application
* @member {EggAppConfig}
*/
protected config: EggAppConfig;

/**
* The instance of agent
* @member {Agent}
*/
protected agent: Agent;

/**
* The instance of app
* @member {Application}
*/
protected app: Application;
}

export type RequestArrayBody = any[];
export type RequestObjectBody = PlainObject;
export interface Request extends KoaApplication.Request { // tslint:disable-line
Expand Down Expand Up @@ -636,7 +662,7 @@ declare module 'egg' {

truncated: boolean;
}

interface GetFileStreamOptions {
requireFile?: boolean; // required file submit, default is true
defCharset?: string;
Expand Down Expand Up @@ -664,7 +690,7 @@ declare module 'egg' {
* special properties (e.g: encrypted). So we must remove this property and
* create our own with the same name.
* @see https://github.com/eggjs/egg/pull/2958
*
*
* However, the latest version of Koa has "[key: string]: any" on the
* context, and there'll be a type error for "keyof koa.Context".
* So we have to directly inherit from "KoaApplication.BaseContext" and
Expand Down
5 changes: 5 additions & 0 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,3 +63,8 @@ exports.Subscription = require('./lib/core/base_context_class');
* @since 1.2.0
*/
exports.BaseContextClass = require('./lib/core/base_context_class');

/**
* @member {Boot} Egg#Boot
*/
exports.Boot = require('./lib/core/base_hook_class');
2 changes: 0 additions & 2 deletions lib/agent.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,6 @@ class Agent extends EggApplication {
options.type = 'agent';
super(options);

this._wrapMessenger();

this.loader.load();

// dump config after loaded, ensure all the dynamic modifications will be recorded
Expand Down
31 changes: 31 additions & 0 deletions lib/core/base_hook_class.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
'use strict';

const assert = require('assert');
const INSTANCE = Symbol('BaseHookClass#instance');

class BaseHookClass {

constructor(instance) {
this[INSTANCE] = instance;
}

get logger() {
return this[INSTANCE].logger;
}

get config() {
return this[INSTANCE].config;
}

get app() {
assert(this[INSTANCE].type === 'application', 'agent boot should not use app instance');
return this[INSTANCE];
}

get agent() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这样实现的话,this.app 和 this.agent 都可以取到值?是不是可以根据 worker.type 判断一下,避免用户误用?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

直接在 constructor 里面根据类型来赋值吧?而不是 assert 报错。

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

根据类型来赋值,开发者取到 undefined,有个断言提醒会不会好一点?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

也行

assert(this[INSTANCE].type === 'agent', 'app boot should not use agent instance');
return this[INSTANCE];
}
}

module.exports = BaseHookClass;
15 changes: 14 additions & 1 deletion lib/egg.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ const createLoggers = require('./core/logger');
const Singleton = require('./core/singleton');
const utils = require('./core/utils');
const BaseContextClass = require('./core/base_context_class');
const BaseHookClass = require('./core/base_hook_class');

const HTTPCLIENT = Symbol('EggApplication#httpclient');
const LOGGERS = Symbol('EggApplication#loggers');
Expand Down Expand Up @@ -133,7 +134,7 @@ class EggApplication extends EggCore {

/**
* Retreive base context class
* @member {Controller} BaseContextClass
* @member {BaseContextClass} BaseContextClass
* @since 1.0.0
*/
this.BaseContextClass = BaseContextClass;
Expand All @@ -158,6 +159,18 @@ class EggApplication extends EggCore {
* @since 2.12.0
*/
this.Subscription = BaseContextClass;

/**
* Retreive base context class
* @member {BaseHookClass} BaseHookClass
*/
this.BaseHookClass = BaseHookClass;

/**
* Retreive base boot
* @member {Boot}
*/
this.Boot = BaseHookClass;
}

/**
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@
"lib",
"app",
"config",
"agent.js",
"index.d.ts"
],
"scripts": {
Expand Down
22 changes: 22 additions & 0 deletions test/agent.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
'use strict';

const assert = require('assert');
const mock = require('egg-mock');
const path = require('path');
const utils = require('./utils');

describe('test/agent.test.js', () => {
afterEach(mock.restore);
let app;

before(() => {
app = utils.app('apps/agent-logger-config');
return app.ready();
});
after(() => app.close());

it('agent logger config should work', () => {
const fileTransport = app._agent.logger.get('file');
assert(fileTransport.options.file === path.join('/tmp/foo', 'egg-agent.log'));
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
'use strict';

module.exports = {
logger: {
dir: '/tmp/foo',
},
};
3 changes: 3 additions & 0 deletions test/fixtures/apps/agent-logger-config/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"name": "agent-logger-config"
}
24 changes: 14 additions & 10 deletions test/fixtures/apps/boot-app/agent.js
Original file line number Diff line number Diff line change
@@ -1,39 +1,43 @@
'use strict';

const assert = require('assert');
const sleep = require('mz-modules/sleep');
const BaseHookClass = require('../../../../lib/core/base_hook_class');

module.exports = class {
constructor(app) {
app.bootLog = [];
this.app = app;
module.exports = class extends BaseHookClass {
atian25 marked this conversation as resolved.
Show resolved Hide resolved
constructor(agent) {
super(agent);
agent.bootLog = [];
assert(this.config);
}

configDidLoad() {
this.app.bootLog.push('configDidLoad');
this.agent.bootLog.push('configDidLoad');
}

async didLoad() {
await sleep(1);
this.app.bootLog.push('didLoad');
this.agent.bootLog.push('didLoad');
}

async willReady() {
await sleep(1);
this.app.bootLog.push('willReady');
this.agent.bootLog.push('willReady');
}

async didReady() {
await sleep(1);
this.app.bootLog.push('didReady');
this.agent.bootLog.push('didReady');
this.logger.info('agent is ready');
}

async beforeClose() {
await sleep(1);
this.app.bootLog.push('beforeClose');
this.agent.bootLog.push('beforeClose');
}

async serverDidReady() {
await sleep(1);
this.app.bootLog.push('serverDidReady');
this.agent.bootLog.push('serverDidReady');
}
};
8 changes: 6 additions & 2 deletions test/fixtures/apps/boot-app/app.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
'use strict';

const assert = require('assert');
const sleep = require('mz-modules/sleep');
const BaseHookClass = require('../../../../lib/core/base_hook_class');

module.exports = class {
module.exports = class extends BaseHookClass {
constructor(app) {
super(app);
app.bootLog = [];
this.app = app;
assert(this.config);
}

configDidLoad() {
Expand All @@ -25,6 +28,7 @@ module.exports = class {
async didReady() {
await sleep(1);
this.app.bootLog.push('didReady');
this.logger.info('app is ready');
}

async beforeClose() {
Expand Down
1 change: 1 addition & 0 deletions test/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ describe('test/index.test.js', () => {
'AppWorkerLoader',
'Application',
'BaseContextClass',
'Boot',
'Controller',
'Service',
'Subscription',
Expand Down
2 changes: 2 additions & 0 deletions test/lib/core/loader/load_boot.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,9 @@ describe('test/lib/core/loader/load_boot.test.js', () => {
});

it('should load app.js', async () => {
app.mockLog();
await app.close();
app.expectLog('app is ready');
assert.deepStrictEqual(app.bootLog, [ 'configDidLoad',
'didLoad',
'willReady',
Expand Down
2 changes: 1 addition & 1 deletion test/lib/egg.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ describe('test/lib/egg.test.js', () => {

it('should read timing data', function* () {
let json = readJson(path.join(baseDir, `run/agent_timing_${process.pid}.json`));
assert(json.length === 39);
assert(json.length === 40);
assert(json[0].name === 'Application Start');
assert(json[0].pid === process.pid);

Expand Down