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(singleton): support async create function #2382

Merged
merged 6 commits into from
Apr 14, 2018
Merged
Show file tree
Hide file tree
Changes from 3 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
27 changes: 22 additions & 5 deletions lib/core/singleton.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
'use strict';

const assert = require('assert');
const is = require('is-type-of');

class Singleton {
constructor(options = {}) {
Expand All @@ -16,25 +17,28 @@ class Singleton {
this.options = options.app.config[this.name] || {};
}

init() {
async init() {
const options = this.options;
assert(!(options.client && options.clients),
`egg:singleton ${this.name} can not set options.client and options.clients both`);

// alias app[name] as client, but still support createInstance method
if (options.client) {
const client = this.createInstance(options.client);
const client = await this.createInstanceAsync(options.client);
this.app[this.name] = client;
assert(!client.createInstance, 'singleton instance should not have createInstance method');
assert(!client.createInstanceAsync, 'singleton instance should not have createInstanceAsync method');
client.createInstance = this.createInstance.bind(this);
client.createInstanceAsync = this.createInstanceAsync.bind(this);
return;
}

// multi clent, use app[name].getInstance(id)
if (options.clients) {
for (const id in options.clients) {
this.clients.set(id, this.createInstance(options.clients[id]));
}
await Promise.all(Object.keys(options.clients).map(id => {
return this.createInstanceAsync(options.clients[id])
.then(client => this.clients.set(id, client));
}));
this.app[this.name] = this;
return;
}
Expand All @@ -48,10 +52,23 @@ class Singleton {
}

createInstance(config) {
// async creator only support createInstanceAsync
assert(!is.asyncFunction(this.create),
Copy link
Member Author

Choose a reason for hiding this comment

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

有一个可以讨论的地方,其实可以判断 this.create 来决定 createInstance 方法是返回创建后的实例还是返回一个 promise,这样就不需要 createInstanceAsync 方法了,但是 createInstance 方法就不统一了,也不好。所以我倾向于新增一个 createInstanceAsync 方法来应对这种特殊场景。

Copy link
Member

Choose a reason for hiding this comment

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

主要还是兼容问题,如果没有问题我倾向于 createInstance 就是一个 async function

Copy link
Member Author

Choose a reason for hiding this comment

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

就是因为有兼容问题才这样的,createInstance 不能直接返回一个 async function

`egg:singleton ${this.name} only support create asynchronous, please use createInstanceAsync`);
// options.default will be merge in to options.clients[id]
config = Object.assign({}, this.options.default, config);
return this.create(config, this.app);
}

async createInstanceAsync(config) {
if (typeof config === 'function') {
// support config to be an async function or a normal function
config = await config();
}
// options.default will be merge in to options.clients[id]
config = Object.assign({}, this.options.default, config);
return await this.create(config, this.app);
}
}

module.exports = Singleton;
5 changes: 3 additions & 2 deletions lib/egg.js
Original file line number Diff line number Diff line change
Expand Up @@ -384,15 +384,16 @@ class EggApplication extends EggCore {
/**
* create a singleton instance
* @param {String} name - unique name for singleton
* @param {Object} create - method will be invoked when singleton instance create
* @param {Function|AsyncFunction} create - method will be invoked when singleton instance create
*/
addSingleton(name, create) {
const options = {};
options.name = name;
options.create = create;
options.app = this;
const singleton = new Singleton(options);
singleton.init();
const done = this.readyCallback(`singleton:${name}`);
Copy link
Member

Choose a reason for hiding this comment

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

use beforeStart?

Copy link
Member Author

Choose a reason for hiding this comment

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

改了

singleton.init().then(done);
}

_patchClusterClient(client) {
Expand Down
19 changes: 19 additions & 0 deletions test/app/extend/agent.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,25 @@ describe('test/app/extend/agent.test.js', () => {
const ds = app.agent.dataService.createInstance({ foo: 'barrr' });
config = await ds.getConfig();
assert(config.foo === 'barrr');

const ds2 = await app.agent.dataService.createInstanceAsync({ foo: 'barrr' });
config = await ds2.getConfig();
assert(config.foo === 'barrr');

config = await app.agent.dataServiceAsync.get('second').getConfig();
assert(config.foo === 'bar');
assert(config.foo2 === 'bar2');

try {
app.agent.dataServiceAsync.createInstance({ foo: 'barrr' });
throw new Error('should not execute');
} catch (err) {
assert(err.message === 'egg:singleton dataServiceAsync only support create asynchronous, please use createInstanceAsync');
}

const ds4 = await app.agent.dataServiceAsync.createInstanceAsync({ foo: 'barrr' });
config = await ds4.getConfig();
assert(config.foo === 'barrr');
});
});
});
19 changes: 19 additions & 0 deletions test/app/extend/application.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,25 @@ describe('test/app/extend/application.test.js', () => {
const ds = app.dataService.createInstance({ foo: 'barrr' });
config = await ds.getConfig();
assert(config.foo === 'barrr');

const ds2 = await app.dataService.createInstanceAsync({ foo: 'barrr' });
config = await ds2.getConfig();
assert(config.foo === 'barrr');

config = await app.dataServiceAsync.get('first').getConfig();
assert(config.foo === 'bar');
assert(config.foo1 === 'bar1');

try {
app.dataServiceAsync.createInstance({ foo: 'barrr' });
throw new Error('should not execute');
} catch (err) {
assert(err.message === 'egg:singleton dataServiceAsync only support create asynchronous, please use createInstanceAsync');
}

const ds4 = await app.dataServiceAsync.createInstanceAsync({ foo: 'barrr' });
config = await ds4.getConfig();
assert(config.foo === 'barrr');
});
});

Expand Down
4 changes: 3 additions & 1 deletion test/fixtures/apps/singleton-demo/agent.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
'use strict';

const createDataService = require('./create');
const createDataService = require('./create').sync;
const createDataServiceAsync = require('./create').async;

module.exports = agent => {
agent.addSingleton('dataService', createDataService);
agent.addSingleton('dataServiceAsync', createDataServiceAsync);
};
4 changes: 3 additions & 1 deletion test/fixtures/apps/singleton-demo/app.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
'use strict';

const createDataService = require('./create');
const createDataService = require('./create').sync;
const createDataServiceAsync = require('./create').async;

module.exports = app => {
app.addSingleton('dataService', createDataService);
app.addSingleton('dataServiceAsync', createDataServiceAsync);
};
17 changes: 16 additions & 1 deletion test/fixtures/apps/singleton-demo/config/config.default.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,22 @@
exports.dataService = {
clients: {
first: { foo1: 'bar1' },
second: { foo2: 'bar2' },
second: async () => {
return { foo2: 'bar2' };
},
},

default: {
foo: 'bar',
}
};

exports.dataServiceAsync = {
clients: {
first: { foo1: 'bar1' },
second: async () => {
return { foo2: 'bar2' };
},
},

default: {
Expand Down
10 changes: 9 additions & 1 deletion test/fixtures/apps/singleton-demo/create.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,15 @@ class DataService {
}

let count = 0;
module.exports = function create(config, app) {

exports.sync = (config, app) => {
const done = app.readyCallback(`DataService-${count++}`);
const dataService = new DataService(config);
dataService.ready(done);
return dataService;
};

exports.async = async (config, app) => {
const done = app.readyCallback(`DataService-${count++}`);
const dataService = new DataService(config);
dataService.ready(done);
Expand Down
Loading