fix: make singleton work for unextensible or frozen instance (#2472)

eggjs · May 3, 2018 · 4b602d0 · 4b602d0
1 parent 824200c
commit 4b602d0
Show file tree

Hide file tree

Showing 2 changed files with 118 additions and 8 deletions.
diff --git 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
@@ -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';