Merge pull request #232 from jimni1222/requestManager

Use setRequestManager instead of setProvider for adding data listener once only

Author: jimni1222

index.js

@@ -42,6 +42,8 @@ const helpers = require('./packages/caver-core-helpers')
 const { version } = require('./package.json')
 
 function Caver(provider, net) {
+    const _this = this
+
     this.use = middleware.registerMiddleware.bind(middleware)
     // sets _requestmanager etc
     packageInit(this, [provider, net])
@@ -60,7 +62,7 @@ function Caver(provider, net) {
     const setProvider = this.setProvider
     this.setProvider = (p, n) => {
         setProvider.apply(this, [p, n])
-        this.klay.setProvider(p, n)
+        _this.klay.setRequestManager(_this._requestManager)
         return true
     }
 }

package.json

@@ -121,6 +121,8 @@
     "rollup-plugin-progress": "^0.4.0",
     "rollup-plugin-replace": "^2.1.0",
     "rollup-plugin-uglify": "^6.0.0",
+    "sinon": "^9.0.0",
+    "sinon-chai": "^3.5.0",
     "vinyl-source-stream": "^2.0.0"
   }
 }

packages/caver-core/src/index.js

@@ -61,7 +61,7 @@ const extend = pkg => {
 }
 
 module.exports = {
-    packageInit: function(pkg, [provider, net]) {
+    packageInit: function(pkg, args) {
         if (!pkg) throw new Error('You need to instantiate using the "new" keyword.')
 
         // make property of pkg._provider, which can properly set providers
@@ -76,20 +76,28 @@ module.exports = {
             configurable: true,
         })
 
-        if (provider && provider._requestManager) {
-            pkg._requestManager = new Manager(provider.currentProvider)
-            // set requestmanager on package
+        // inherit from parent package or create a new RequestManager
+        if (args[0] && args[0]._requestManager) {
+            pkg._requestManager = args[0]._requestManager
         } else {
-            pkg._requestManager = new Manager(provider, net)
+            pkg._requestManager = new Manager(args[0], args[1])
         }
 
         pkg.providers = Manager.providers
-
         pkg._provider = pkg._requestManager.provider
 
         // add SETPROVIDER function (don't overwrite if already existing)
         if (!pkg.setProvider) {
-            pkg.setProvider = (p, n) => (pkg._provider = pkg._requestManager.setProvider(p, n).provider)
+            pkg.setProvider = function(provider, net) {
+                pkg._requestManager.setProvider(provider, net)
+                pkg._provider = pkg._requestManager.provider
+                return true
+            }
+        }
+
+        pkg.setRequestManager = function(manager) {
+            pkg._requestManager = manager
+            pkg._provider = manager.provider
         }
 
         // attach batch request creation

packages/caver-klay/caver-klay-contract/src/index.js

@@ -301,8 +301,8 @@ Contract.prototype._getCallback = function getCallback(args) {
  * @return {Object} the contract instance
  */
 /**
- * this._checkListener('newListener', subOptions.event.name, subOptions.callback);
- * this._checkListener('removeListener', subOptions.event.name, subOptions.callback);
+ * this._checkListener('newListener', subOptions.event.name);
+ * this._checkListener('removeListener', subOptions.event.name);
  */
 Contract.prototype._checkListener = function(type, event) {
     if (event === type) {
@@ -727,8 +727,8 @@ Contract.prototype._on = function() {
     const subOptions = this._generateEventOptions.apply(this, arguments)
 
     // prevent the event "newListener" and "removeListener" from being overwritten
-    this._checkListener('newListener', subOptions.event.name, subOptions.callback)
-    this._checkListener('removeListener', subOptions.event.name, subOptions.callback)
+    this._checkListener('newListener', subOptions.event.name)
+    this._checkListener('removeListener', subOptions.event.name)
 
     // TODO check if listener already exists? and reuse subscription if options are the same.
 

packages/caver-klay/caver-klay-personal/src/index.js

@@ -38,7 +38,7 @@ const Personal = function Personal(...args) {
     // sets _requestmanager
     core.packageInit(this, args)
 
-    this.net = new Net(this.currentProvider)
+    this.net = new Net(this)
 
     let defaultAccount = null
     let defaultBlock = 'latest'

packages/caver-klay/src/index.js

@@ -50,14 +50,25 @@ const Klay = function Klay(...args) {
     // sets _requestmanager
     core.packageInit(this, args)
 
+    // overwrite package setRequestManager
+    const setRequestManager = this.setRequestManager
+    this.setRequestManager = function(manager) {
+        setRequestManager(manager)
+
+        _this.net.setRequestManager(manager)
+        _this.personal.setRequestManager(manager)
+        _this.accounts.setRequestManager(manager)
+        _this.Contract._requestManager = _this._requestManager
+        _this.Contract.currentProvider = _this._provider
+
+        return true
+    }
+
     // overwrite setProvider
     const setProvider = this.setProvider
     this.setProvider = function(...arg) {
         setProvider.apply(_this, arg)
-        _this.net.setProvider.apply(_this, arg)
-        _this.personal.setProvider.apply(_this, arg)
-        _this.accounts.setProvider.apply(_this, arg)
-        _this.Contract.setProvider(_this.currentProvider, _this.accounts)
+        _this.setRequestManager(_this._requestManager)
     }
 
     let defaultAccount = null
@@ -113,24 +124,36 @@ const Klay = function Klay(...args) {
     this.decodeTransaction = decodeFromRawTransaction
 
     // add net
-    this.net = new Net(this.currentProvider)
+    this.net = new Net(this)
     // add chain detection
     this.net.getNetworkType = getNetworkType.bind(this)
 
     // add accounts
-    this.accounts = new Accounts(this.currentProvider)
+    this.accounts = new Accounts(this)
 
     // add personal
-    this.personal = new Personal(this.currentProvider)
+    this.personal = new Personal(this)
     this.personal.defaultAccount = this.defaultAccount
 
     // create a proxy Contract type for this instance, as a Contract's provider
     // is stored as a class member rather than an instance variable. If we do
     // not create this proxy type, changing the provider in one instance of
     // caver-klay would subsequently change the provider for _all_ contract
     // instances!
+    const self = this
     const Contract = function Contract() {
         BaseContract.apply(this, arguments)
+
+        // when Klay.setProvider is called, call packageInit
+        // on all contract instances instantiated via this Klay
+        // instances. This will update the currentProvider for
+        // the contract instances
+        const _this = this // eslint-disable-line no-shadow
+        const setProvider = self.setProvider // eslint-disable-line no-shadow
+        self.setProvider = function() {
+            setProvider.apply(self, arguments)
+            core.packageInit(_this, [self])
+        }
     }
 
     Contract.setProvider = function() {
@@ -146,12 +169,16 @@ const Klay = function Klay(...args) {
     this.Contract = Contract
     this.Contract.defaultAccount = this.defaultAccount
     this.Contract.defaultBlock = this.defaultBlock
-    this.Contract.setProvider(this.currentProvider, this.accounts)
+    this.Contract._requestManager = this._requestManager
+    this.Contract._klayAccounts = this.accounts
+    this.Contract.currentProvider = this._requestManager.provider
 
     this.KIP7 = KIP7
     this.KIP7.defaultAccount = this.defaultAccount
     this.KIP7.defaultBlock = this.defaultBlock
-    this.KIP7.setProvider(this.currentProvider, this.accounts)
+    this.KIP7._requestManager = this._requestManager
+    this.KIP7._klayAccounts = this.accounts
+    this.KIP7.currentProvider = this._requestManager.provider
 
     // add IBAN
     this.Iban = utils.Iban
@@ -240,7 +267,7 @@ const Klay = function Klay(...args) {
 
     methods.forEach(function(method) {
         method.attachToObject(_this)
-        // second param means is klay.accounts (necessary for wallet signing)
+        // second param is the klay.accounts module (necessary for signing transactions locally)
         method.setRequestManager(_this._requestManager, _this.accounts)
         method.defaultBlock = _this.defaultBlock
         method.defaultAccount = _this.defaultAccount

test/setRequestManager.js

@@ -0,0 +1,64 @@
+/*
+    Copyright 2020 The caver-js Authors
+    This file is part of the caver-js library.
+
+    The caver-js library is free software: you can redistribute it and/or modify
+    it under the terms of the GNU Lesser General Public License as published by
+    the Free Software Foundation, either version 3 of the License, or
+    (at your option) any later version.
+
+    The caver-js library is distributed in the hope that it will be useful,
+    but WITHOUT ANY WARRANTY; without even the implied warranty of
+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+    GNU Lesser General Public License for more details.
+
+    You should have received a copy of the GNU Lesser General Public License
+    along with the caver-js. If not, see <http://www.gnu.org/licenses/>.
+*/
+
+const chai = require('chai')
+const sinon = require('sinon')
+const sinonChai = require('sinon-chai')
+
+chai.use(sinonChai)
+
+const expect = chai.expect
+
+const Caver = require('../index.js')
+const testRPCURL = require('./testrpc')
+const { Manager } = require('../packages/caver-core-requestmanager/src/index')
+const { kip7JsonInterface } = require('../packages/caver-klay/caver-klay-kct/kctHelper')
+
+describe('setRequestManager', () => {
+    it('CAVERJS-UNIT-ETC-203: should call setRequestManager with each pacakge instead of setProvider', () => {
+        const setProviderSpy = sinon.spy(Manager.prototype, 'setProvider')
+
+        const originalProvider = new Caver.providers.HttpProvider(testRPCURL)
+        const caver = new Caver(originalProvider)
+
+        expect(caver).not.to.be.undefined
+        expect(setProviderSpy).to.have.been.calledOnce
+
+        const contract = new caver.klay.Contract(kip7JsonInterface)
+
+        expect(contract.currentProvider).to.deep.equals(originalProvider)
+        expect(caver.klay.currentProvider).to.deep.equals(originalProvider)
+
+        const setKlayRequestManager = sinon.spy(caver.klay, 'setRequestManager')
+        const setNetRequestManager = sinon.spy(caver.klay.net, 'setRequestManager')
+        const setPersonalRequestManager = sinon.spy(caver.klay.personal, 'setRequestManager')
+        const setAccountsRequestManager = sinon.spy(caver.klay.accounts, 'setRequestManager')
+        const setKlayProvider = sinon.spy(caver.klay, 'setProvider')
+
+        const newProvider = new Caver.providers.HttpProvider('https://api.baobab.klaytn.net:8651/')
+        caver.klay.setProvider(newProvider)
+
+        expect(setKlayRequestManager).to.have.been.calledOnce
+        expect(setNetRequestManager).to.have.been.calledOnce
+        expect(setPersonalRequestManager).to.have.been.calledOnce
+        expect(setAccountsRequestManager).to.have.been.calledOnce
+        expect(setKlayProvider).to.have.been.calledOnce
+        expect(contract.currentProvider).to.deep.equals(newProvider)
+        expect(caver.klay.currentProvider).to.deep.equals(newProvider)
+    })
+})