From 4b602d037554b72c8261b7abb7efd94f8f59f3fe Mon Sep 17 00:00:00 2001 From: Yiyu He Date: Thu, 3 May 2018 10:25:27 +0800 Subject: [PATCH] fix: make singleton work for unextensible or frozen instance (#2472) --- lib/core/singleton.js | 28 +++++++--- test/lib/core/singleton.test.js | 98 +++++++++++++++++++++++++++++++++ 2 files changed, 118 insertions(+), 8 deletions(-) diff --git a/lib/core/singleton.js b/lib/core/singleton.js index 75ea4242b0..28df173bca 100644 --- a/lib/core/singleton.js +++ b/lib/core/singleton.js @@ -30,10 +30,7 @@ class Singleton { if (options.client) { const client = this.createInstance(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); + this._extendDynamicMethods(client); return; } @@ -60,10 +57,7 @@ class Singleton { if (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); + this._extendDynamicMethods(client); return; } @@ -99,6 +93,24 @@ class Singleton { config = Object.assign({}, this.options.default, config); return await this.create(config, this.app); } + + _extendDynamicMethods(client) { + assert(!client.createInstance, 'singleton instance should not have createInstance method'); + assert(!client.createInstanceAsync, 'singleton instance should not have createInstanceAsync method'); + + try { + let extendable = client; + // Object.preventExtensions() or Object.freeze() + if (!Object.isExtensible(client) || Object.isFrozen(client)) { + // eslint-disable-next-line no-proto + extendable = client.__proto__ || client; + } + extendable.createInstance = this.createInstance.bind(this); + extendable.createInstanceAsync = this.createInstanceAsync.bind(this); + } catch (err) { + this.app.logger.warn('egg:singleton %s dynamic create is disabled because of client is unextensible', this.name); + } + } } module.exports = Singleton; diff --git a/test/lib/core/singleton.test.js b/test/lib/core/singleton.test.js index 6ad52118df..cbbb7b54ca 100644 --- a/test/lib/core/singleton.test.js +++ b/test/lib/core/singleton.test.js @@ -25,6 +25,11 @@ async function asyncCreate(config) { } describe('test/lib/core/singleton.test.js', () => { + afterEach(() => { + delete DataService.prototype.createInstance; + delete DataService.prototype.createInstanceAsync; + }); + it('should init with client', async () => { const name = 'dataService'; @@ -163,6 +168,99 @@ describe('test/lib/core/singleton.test.js', () => { assert(app.dataService.config.foo === 'bar'); }); + it('should work with unextensible', async () => { + function create(config) { + const d = new DataService(config); + Object.preventExtensions(d); + return d; + } + const app = { + config: { + dataService: { + client: { foo: 'bar' }, + default: { foo: 'bar' }, + }, + }, + }; + const name = 'dataService'; + + const singleton = new Singleton({ + name, + app, + create, + }); + singleton.init(); + const dataService = await app.dataService.createInstanceAsync({ foo1: 'bar1' }); + assert(dataService instanceof DataService); + assert(dataService.config.foo1 === 'bar1'); + assert(dataService.config.foo === 'bar'); + }); + + it('should work with frozen', async () => { + function create(config) { + const d = new DataService(config); + Object.freeze(d); + return d; + } + const app = { + config: { + dataService: { + client: { foo: 'bar' }, + default: { foo: 'bar' }, + }, + }, + }; + const name = 'dataService'; + + const singleton = new Singleton({ + name, + app, + create, + }); + singleton.init(); + + const dataService = await app.dataService.createInstanceAsync({ foo1: 'bar1' }); + assert(dataService instanceof DataService); + assert(dataService.config.foo1 === 'bar1'); + assert(dataService.config.foo === 'bar'); + }); + + + it('should work with no prototype and frozen', async () => { + let warn = false; + function create() { + const d = Object.create(null); + Object.freeze(d); + return d; + } + const app = { + config: { + dataService: { + client: { foo: 'bar' }, + default: { foo: 'bar' }, + }, + }, + logger: { + warn(msg, name) { + assert(name === 'dataService'); + warn = true; + }, + }, + }; + const name = 'dataService'; + + const singleton = new Singleton({ + name, + app, + create, + }); + singleton.init(); + + assert(!app.dataService.createInstance); + assert(!app.dataService.createInstanceAsync); + assert(warn); + }); + describe('async create', () => { it('should init with client', async () => { const name = 'dataService';