diff --git a/js/src/api/api.js b/js/src/api/api.js index c82129772..9768b9acb 100644 --- a/js/src/api/api.js +++ b/js/src/api/api.js @@ -90,8 +90,8 @@ export default class Api { return this._subscriptions.subscribe(subscriptionName, callback); } - unsubscribe (subscriptionName, subscriptionId) { - return this._subscriptions.unsubscribe(subscriptionName, subscriptionId); + unsubscribe (subscriptionId) { + return this._subscriptions.unsubscribe(subscriptionId); } pollMethod (method, input, validate) { diff --git a/js/src/api/subscriptions/manager.js b/js/src/api/subscriptions/manager.js index 6f9b7cf7e..61e06499e 100644 --- a/js/src/api/subscriptions/manager.js +++ b/js/src/api/subscriptions/manager.js @@ -29,17 +29,14 @@ const events = { 'personal_requestsToConfirm': { module: 'signer' } }; -let nextSubscriptionId = 0; - export default class Manager { constructor (api) { this._api = api; - this.subscriptions = {}; + this.subscriptions = []; this.values = {}; Object.keys(events).forEach((subscriptionName) => { - this.subscriptions[subscriptionName] = {}; this.values[subscriptionName] = { error: null, data: null @@ -71,61 +68,59 @@ export default class Manager { return; } - const subscriptionId = nextSubscriptionId++; + const subscriptionId = this.subscriptions.length; const { error, data } = this.values[subscriptionName]; const engine = this[`_${subscription.module}`]; - this.subscriptions[subscriptionName][subscriptionId] = callback; + this.subscriptions[subscriptionId] = { + name: subscriptionName, + id: subscriptionId, + callback + }; if (!engine.isStarted) { engine.start(); } else { - this._sendData(subscriptionName, subscriptionId, callback, error, data); + this._sendData(subscriptionId, error, data); } resolve(subscriptionId); }); } - unsubscribe (subscriptionName, subscriptionId) { + unsubscribe (subscriptionId) { return new Promise((resolve, reject) => { - const subscription = this._validateType(subscriptionName); - - if (isError(subscription)) { - reject(subscription); + if (!this.subscriptions[subscriptionId]) { + reject(new Error(`Cannot find subscription ${subscriptionId}`)); return; } - if (!this.subscriptions[subscriptionName][subscriptionId]) { - reject(new Error(`Cannot find subscription ${subscriptionId} for type ${subscriptionName}`)); - return; - } - - delete this.subscriptions[subscriptionName][subscriptionId]; + delete this.subscriptions[subscriptionId]; resolve(); }); } - _sendData (subscriptionName, subscriptionId, callback, error, data) { + _sendData (subscriptionId, error, data) { + const { callback } = this.subscriptions[subscriptionId]; + try { callback(error, data); } catch (error) { - console.error(`Unable to update callback for ${subscriptionName}, subscriptionId ${subscriptionId}`, error); - this.unsubscribe(subscriptionName, subscriptionId); + console.error(`Unable to update callback for subscriptionId ${subscriptionId}`, error); + this.unsubscribe(subscriptionId); } } _updateSubscriptions = (subscriptionName, error, data) => { - if (!this.subscriptions[subscriptionName]) { - throw new Error(`Cannot find entry point for subscriptions of type ${subscriptionName}`); - } + const subscriptions = this.subscriptions + .filter(subscription => subscription.name === subscriptionName); this.values[subscriptionName] = { error, data }; - Object.keys(this.subscriptions[subscriptionName]).forEach((subscriptionId) => { - const callback = this.subscriptions[subscriptionName][subscriptionId]; - this._sendData(subscriptionName, subscriptionId, callback, error, data); - }); + subscriptions + .forEach((subscription) => { + this._sendData(subscription.id, error, data); + }); } } diff --git a/js/src/api/subscriptions/manager.spec.js b/js/src/api/subscriptions/manager.spec.js index 4b6b84b84..5e434efec 100644 --- a/js/src/api/subscriptions/manager.spec.js +++ b/js/src/api/subscriptions/manager.spec.js @@ -20,6 +20,7 @@ import Manager, { events } from './manager'; function newStub () { const start = () => manager._updateSubscriptions(manager.__test, null, 'test'); + const manager = new Manager({ transport: { isConnected: true @@ -53,7 +54,7 @@ describe('api/subscriptions/manager', () => { describe('constructor', () => { it('sets up the subscription types & defaults', () => { - expect(Object.keys(manager.subscriptions)).to.deep.equal(Object.keys(events)); + expect(manager.subscriptions).to.be.an.array; expect(Object.keys(manager.values)).to.deep.equal(Object.keys(events)); }); }); @@ -74,6 +75,7 @@ describe('api/subscriptions/manager', () => { manager.__test = eventName; cb = sinon.stub(); sinon.spy(engine, 'start'); + return manager .subscribe(eventName, cb) .then((_subscriptionId) => { @@ -86,7 +88,7 @@ describe('api/subscriptions/manager', () => { }); it('returns a subscriptionId', () => { - expect(subscriptionId).to.be.ok; + expect(subscriptionId).to.be.a.number; }); it('calls the subscription callback with updated values', () => { @@ -95,4 +97,38 @@ describe('api/subscriptions/manager', () => { }); }); }); + + describe('unsubscriptions', () => { + Object + .keys(events) + .filter((eventName) => eventName.indexOf('_') !== -1) + .forEach((eventName) => { + const { module } = events[eventName]; + let engine; + let cb; + + describe(eventName, () => { + beforeEach(() => { + engine = manager[`_${module}`]; + manager.__test = eventName; + cb = sinon.stub(); + sinon.spy(engine, 'start'); + + return manager + .subscribe(eventName, cb) + .then((_subscriptionId) => { + manager.unsubscribe(_subscriptionId); + }) + .then(() => { + manager._updateSubscriptions(manager.__test, null, 'test2'); + }); + }); + + it('does not call the callback after unsibscription', () => { + expect(cb).to.have.been.calledWith(null, 'test'); + expect(cb).to.not.have.been.calledWith(null, 'test2'); + }); + }); + }); + }); }); diff --git a/js/src/ui/TxHash/txHash.js b/js/src/ui/TxHash/txHash.js index 79714cf9b..7b2080463 100644 --- a/js/src/ui/TxHash/txHash.js +++ b/js/src/ui/TxHash/txHash.js @@ -50,7 +50,7 @@ class TxHash extends Component { const { api } = this.context; const { subscriptionId } = this.state; - api.unsubscribe('eth_blockNumber', subscriptionId); + api.unsubscribe(subscriptionId); } render () { diff --git a/js/src/views/Contract/contract.js b/js/src/views/Contract/contract.js index 7a1889c19..ec12c3ac9 100644 --- a/js/src/views/Contract/contract.js +++ b/js/src/views/Contract/contract.js @@ -87,7 +87,7 @@ class Contract extends Component { const { api } = this.context; const { subscriptionId, blockSubscriptionId, contract } = this.state; - api.unsubscribe('eth_blockNumber', blockSubscriptionId); + api.unsubscribe(blockSubscriptionId); contract.unsubscribe(subscriptionId); }