From a9020556e834ce0193ece9f11a1d509e8a2b3a38 Mon Sep 17 00:00:00 2001 From: Jasmine/kimjimin Date: Tue, 3 Mar 2020 16:40:40 +0900 Subject: [PATCH 1/2] Use setRequestManager instead of setProvider for adding data listener once only --- index.js | 4 +- packages/caver-core/src/index.js | 22 +++++++--- .../caver-klay-contract/src/index.js | 8 ++-- .../caver-klay-personal/src/index.js | 2 +- packages/caver-klay/src/index.js | 43 ++++++++++++++----- 5 files changed, 56 insertions(+), 23 deletions(-) diff --git a/index.js b/index.js index 93c49771..ffc0e577 100644 --- a/index.js +++ b/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 } } diff --git a/packages/caver-core/src/index.js b/packages/caver-core/src/index.js index 97ea3d16..1c6f853f 100644 --- a/packages/caver-core/src/index.js +++ b/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 diff --git a/packages/caver-klay/caver-klay-contract/src/index.js b/packages/caver-klay/caver-klay-contract/src/index.js index 44793440..6c072058 100644 --- a/packages/caver-klay/caver-klay-contract/src/index.js +++ b/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. diff --git a/packages/caver-klay/caver-klay-personal/src/index.js b/packages/caver-klay/caver-klay-personal/src/index.js index be37abe5..c2fcb98d 100644 --- a/packages/caver-klay/caver-klay-personal/src/index.js +++ b/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' diff --git a/packages/caver-klay/src/index.js b/packages/caver-klay/src/index.js index 93e91a9a..d5139372 100644 --- a/packages/caver-klay/src/index.js +++ b/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,15 +124,15 @@ 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 @@ -129,8 +140,16 @@ const Klay = function Klay(...args) { // 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) + + 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 +165,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 +263,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 eth.accounts module (necessary for signing transactions locally) method.setRequestManager(_this._requestManager, _this.accounts) method.defaultBlock = _this.defaultBlock method.defaultAccount = _this.defaultAccount From 0607a402154e3381497e68e27dd6024d1d47844d Mon Sep 17 00:00:00 2001 From: Jasmine/kimjimin Date: Wed, 4 Mar 2020 09:59:14 +0900 Subject: [PATCH 2/2] Implemented test code for using setRequestManger --- package.json | 2 ++ test/setRequestManager.js | 55 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+) create mode 100644 test/setRequestManager.js diff --git a/package.json b/package.json index 37ed1f31..75cf7cf2 100644 --- a/package.json +++ b/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" } } diff --git a/test/setRequestManager.js b/test/setRequestManager.js new file mode 100644 index 00000000..499fcdcb --- /dev/null +++ b/test/setRequestManager.js @@ -0,0 +1,55 @@ +/* + 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 . +*/ + +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') + +describe('setRequestManager', () => { + it('CAVERJS-UNIT-ETC-203: should call setRequestManager with each pacakge instead of setProvider', () => { + const setProviderSpy = sinon.spy(Manager.prototype, 'setProvider') + + const caver = new Caver(testRPCURL) + + expect(caver).not.to.be.undefined + expect(setProviderSpy).to.have.been.calledOnce + + 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.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).not.to.have.been.calledOnce + }) +})