From 1da0a7447563e3cb0d9149b0b9898ec93b483982 Mon Sep 17 00:00:00 2001 From: Jaco Greeff Date: Fri, 30 Dec 2016 15:54:50 +0100 Subject: [PATCH 1/4] unsubscribe in onClose (state available) --- js/src/modals/Shapeshift/shapeshift.js | 34 ++++++++++++++------------ 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/js/src/modals/Shapeshift/shapeshift.js b/js/src/modals/Shapeshift/shapeshift.js index fc76a896851..8d01ce68d47 100644 --- a/js/src/modals/Shapeshift/shapeshift.js +++ b/js/src/modals/Shapeshift/shapeshift.js @@ -56,23 +56,14 @@ export default class Shapeshift extends Component { exchangeInfo: null, error: {}, hasAccepted: false, - shifting: false + shifting: false, + subscribed: false } componentDidMount () { this.retrieveCoins(); } - componentWillUnmount () { - this.unsubscribe(); - } - - unsubscribe () { - // Unsubscribe from Shapeshit - const { depositAddress } = this.state; - shapeshift.unsubscribe(depositAddress); - } - render () { const { error, stage } = this.state; @@ -196,6 +187,7 @@ export default class Shapeshift extends Component { } onClose = () => { + this.unsubscribe(); this.setStage(0); this.props.onClose && this.props.onClose(); } @@ -215,12 +207,12 @@ export default class Shapeshift extends Component { console.log('onShift', result); const depositAddress = result.deposit; - if (this.state.depositAddress) { - this.unsubscribe(); - } - + this.unsubscribe(); shapeshift.subscribe(depositAddress, this.onExchangeInfo); - this.setState({ depositAddress }); + this.setState({ + depositAddress, + subscribed: true + }); }) .catch((error) => { console.error('onShift', error); @@ -312,6 +304,16 @@ export default class Shapeshift extends Component { }); } + unsubscribe () { + const { depositAddress, subscribed } = this.state; + + if (!subscribed || !depositAddress || !depositAddress.length) { + return; + } + + shapeshift.unsubscribe(depositAddress); + } + newError (error) { const { store } = this.context; From 88edf314560253365e7b7ecad9c0dad058473e8e Mon Sep 17 00:00:00 2001 From: Jaco Greeff Date: Sat, 31 Dec 2016 12:10:08 +0100 Subject: [PATCH 2/4] Revert "unsubscribe in onClose (state available)" This reverts commit 1da0a7447563e3cb0d9149b0b9898ec93b483982. --- js/src/modals/Shapeshift/shapeshift.js | 34 ++++++++++++-------------- 1 file changed, 16 insertions(+), 18 deletions(-) diff --git a/js/src/modals/Shapeshift/shapeshift.js b/js/src/modals/Shapeshift/shapeshift.js index 8d01ce68d47..fc76a896851 100644 --- a/js/src/modals/Shapeshift/shapeshift.js +++ b/js/src/modals/Shapeshift/shapeshift.js @@ -56,14 +56,23 @@ export default class Shapeshift extends Component { exchangeInfo: null, error: {}, hasAccepted: false, - shifting: false, - subscribed: false + shifting: false } componentDidMount () { this.retrieveCoins(); } + componentWillUnmount () { + this.unsubscribe(); + } + + unsubscribe () { + // Unsubscribe from Shapeshit + const { depositAddress } = this.state; + shapeshift.unsubscribe(depositAddress); + } + render () { const { error, stage } = this.state; @@ -187,7 +196,6 @@ export default class Shapeshift extends Component { } onClose = () => { - this.unsubscribe(); this.setStage(0); this.props.onClose && this.props.onClose(); } @@ -207,12 +215,12 @@ export default class Shapeshift extends Component { console.log('onShift', result); const depositAddress = result.deposit; - this.unsubscribe(); + if (this.state.depositAddress) { + this.unsubscribe(); + } + shapeshift.subscribe(depositAddress, this.onExchangeInfo); - this.setState({ - depositAddress, - subscribed: true - }); + this.setState({ depositAddress }); }) .catch((error) => { console.error('onShift', error); @@ -304,16 +312,6 @@ export default class Shapeshift extends Component { }); } - unsubscribe () { - const { depositAddress, subscribed } = this.state; - - if (!subscribed || !depositAddress || !depositAddress.length) { - return; - } - - shapeshift.unsubscribe(depositAddress); - } - newError (error) { const { store } = this.context; From a3fdad11a1f285304daad96f2d0a54c3f7685319 Mon Sep 17 00:00:00 2001 From: Jaco Greeff Date: Sat, 31 Dec 2016 12:09:37 +0100 Subject: [PATCH 3/4] Fix shapeshift double unsubscribe --- js/src/3rdparty/shapeshift/helpers.spec.js | 18 +-- js/src/3rdparty/shapeshift/rpc.spec.js | 19 ++- js/src/3rdparty/shapeshift/shapeshift.js | 77 ++++++++----- js/src/3rdparty/shapeshift/shapeshift.spec.js | 109 ++++++++++++++++-- 4 files changed, 167 insertions(+), 56 deletions(-) diff --git a/js/src/3rdparty/shapeshift/helpers.spec.js b/js/src/3rdparty/shapeshift/helpers.spec.js index 8ccec679183..d5e9994b85d 100644 --- a/js/src/3rdparty/shapeshift/helpers.spec.js +++ b/js/src/3rdparty/shapeshift/helpers.spec.js @@ -16,16 +16,10 @@ const nock = require('nock'); -const ShapeShift = require('./'); -const initShapeshift = (ShapeShift.default || ShapeShift); - const APIKEY = '0x123454321'; -const shapeshift = initShapeshift(APIKEY); -const rpc = shapeshift.getRpc(); - -function mockget (requests) { - let scope = nock(rpc.ENDPOINT); +function mockget (shapeshift, requests) { + let scope = nock(shapeshift.getRpc().ENDPOINT); requests.forEach((request) => { scope = scope @@ -38,8 +32,8 @@ function mockget (requests) { return scope; } -function mockpost (requests) { - let scope = nock(rpc.ENDPOINT); +function mockpost (shapeshift, requests) { + let scope = nock(shapeshift.getRpc().ENDPOINT); requests.forEach((request) => { scope = scope @@ -58,7 +52,5 @@ function mockpost (requests) { module.exports = { APIKEY, mockget, - mockpost, - shapeshift, - rpc + mockpost }; diff --git a/js/src/3rdparty/shapeshift/rpc.spec.js b/js/src/3rdparty/shapeshift/rpc.spec.js index 582b77a53bd..d561fbe7d51 100644 --- a/js/src/3rdparty/shapeshift/rpc.spec.js +++ b/js/src/3rdparty/shapeshift/rpc.spec.js @@ -16,12 +16,21 @@ const helpers = require('./helpers.spec.js'); -const APIKEY = helpers.APIKEY; +const ShapeShift = require('./'); +const initShapeshift = (ShapeShift.default || ShapeShift); + const mockget = helpers.mockget; const mockpost = helpers.mockpost; -const rpc = helpers.rpc; describe('shapeshift/rpc', () => { + let rpc; + let shapeshift; + + beforeEach(() => { + shapeshift = initShapeshift(helpers.APIKEY); + rpc = shapeshift.getRpc(); + }); + describe('GET', () => { const REPLY = { test: 'this is some result' }; @@ -29,7 +38,7 @@ describe('shapeshift/rpc', () => { let result; beforeEach(() => { - scope = mockget([{ path: 'test', reply: REPLY }]); + scope = mockget(shapeshift, [{ path: 'test', reply: REPLY }]); return rpc .get('test') @@ -54,7 +63,7 @@ describe('shapeshift/rpc', () => { let result; beforeEach(() => { - scope = mockpost([{ path: 'test', reply: REPLY }]); + scope = mockpost(shapeshift, [{ path: 'test', reply: REPLY }]); return rpc .post('test', { input: 'stuff' }) @@ -76,7 +85,7 @@ describe('shapeshift/rpc', () => { }); it('passes the apikey specified', () => { - expect(scope.body.test.apiKey).to.equal(APIKEY); + expect(scope.body.test.apiKey).to.equal(helpers.APIKEY); }); }); }); diff --git a/js/src/3rdparty/shapeshift/shapeshift.js b/js/src/3rdparty/shapeshift/shapeshift.js index 39b8365ecf2..c98ef3eca03 100644 --- a/js/src/3rdparty/shapeshift/shapeshift.js +++ b/js/src/3rdparty/shapeshift/shapeshift.js @@ -15,8 +15,9 @@ // along with Parity. If not, see . export default function (rpc) { - let subscriptions = []; - let pollStatusIntervalId = null; + let _subscriptions = []; + let _pollStatusIntervalId = null; + let _subscriptionPromises = null; function getCoins () { return rpc.get('getcoins'); @@ -36,75 +37,93 @@ export default function (rpc) { function shift (toAddress, returnAddress, pair) { return rpc.post('shift', { - withdrawal: toAddress, - pair: pair, - returnAddress: returnAddress + pair, + returnAddress, + withdrawal: toAddress }); } function subscribe (depositAddress, callback) { - const idx = subscriptions.length; + if (!depositAddress || !callback) { + return; + } - subscriptions.push({ - depositAddress, + const index = _subscriptions.length; + + _subscriptions.push({ callback, - idx + depositAddress, + index }); - // Only poll if there are subscriptions... - if (!pollStatusIntervalId) { - pollStatusIntervalId = setInterval(_pollStatus, 2000); + if (_pollStatusIntervalId === null) { + _pollStatusIntervalId = setInterval(_pollStatus, 2000); } } function unsubscribe (depositAddress) { - const newSubscriptions = [] - .concat(subscriptions) - .filter((sub) => sub.depositAddress !== depositAddress); - - subscriptions = newSubscriptions; + _subscriptions = _subscriptions.filter((sub) => sub.depositAddress !== depositAddress); - if (subscriptions.length === 0) { - clearInterval(pollStatusIntervalId); - pollStatusIntervalId = null; + if (_subscriptions.length === 0) { + clearInterval(_pollStatusIntervalId); + _pollStatusIntervalId = null; } + + return true; } function _getSubscriptionStatus (subscription) { if (!subscription) { - return; + return Promise.resolve(); } - getStatus(subscription.depositAddress) + return getStatus(subscription.depositAddress) .then((result) => { switch (result.status) { case 'no_deposits': case 'received': subscription.callback(null, result); - return; + return true; case 'complete': subscription.callback(null, result); - subscriptions[subscription.idx] = null; - return; + unsubscribe(subscription.depositAddress); + return true; case 'failed': subscription.callback({ message: status.error, fatal: true }); - subscriptions[subscription.idx] = null; - return; + unsubscribe(subscription.depositAddress); + return true; } }) - .catch(subscription.callback); + .catch(() => { + return true; + }); } function _pollStatus () { - subscriptions.forEach(_getSubscriptionStatus); + _subscriptionPromises = Promise.all(_subscriptions.map(_getSubscriptionStatus)); + } + + function _getSubscriptions () { + return _subscriptions; + } + + function _getSubscriptionPromises () { + return _subscriptionPromises; + } + + function _isPolling () { + return _pollStatusIntervalId !== null; } return { + _getSubscriptions, + _getSubscriptionPromises, + _isPolling, getCoins, getMarketInfo, getRpc, diff --git a/js/src/3rdparty/shapeshift/shapeshift.spec.js b/js/src/3rdparty/shapeshift/shapeshift.spec.js index 9fe2ca1f07c..57bfdfb39b5 100644 --- a/js/src/3rdparty/shapeshift/shapeshift.spec.js +++ b/js/src/3rdparty/shapeshift/shapeshift.spec.js @@ -14,13 +14,29 @@ // You should have received a copy of the GNU General Public License // along with Parity. If not, see . +const sinon = require('sinon'); + +const ShapeShift = require('./'); +const initShapeshift = (ShapeShift.default || ShapeShift); + const helpers = require('./helpers.spec.js'); const mockget = helpers.mockget; const mockpost = helpers.mockpost; -const shapeshift = helpers.shapeshift; describe('shapeshift/calls', () => { + let clock; + let shapeshift; + + beforeEach(() => { + clock = sinon.useFakeTimers(); + shapeshift = initShapeshift(helpers.APIKEY); + }); + + afterEach(() => { + clock.restore(); + }); + describe('getCoins', () => { const REPLY = { BTC: { @@ -39,8 +55,8 @@ describe('shapeshift/calls', () => { let scope; - before(() => { - scope = mockget([{ path: 'getcoins', reply: REPLY }]); + beforeEach(() => { + scope = mockget(shapeshift, [{ path: 'getcoins', reply: REPLY }]); return shapeshift.getCoins(); }); @@ -61,8 +77,8 @@ describe('shapeshift/calls', () => { let scope; - before(() => { - scope = mockget([{ path: 'marketinfo/btc_ltc', reply: REPLY }]); + beforeEach(() => { + scope = mockget(shapeshift, [{ path: 'marketinfo/btc_ltc', reply: REPLY }]); return shapeshift.getMarketInfo('btc_ltc'); }); @@ -80,8 +96,8 @@ describe('shapeshift/calls', () => { let scope; - before(() => { - scope = mockget([{ path: 'txStat/0x123', reply: REPLY }]); + beforeEach(() => { + scope = mockget(shapeshift, [{ path: 'txStat/0x123', reply: REPLY }]); return shapeshift.getStatus('0x123'); }); @@ -101,8 +117,8 @@ describe('shapeshift/calls', () => { let scope; - before(() => { - scope = mockpost([{ path: 'shift', reply: REPLY }]); + beforeEach(() => { + scope = mockpost(shapeshift, [{ path: 'shift', reply: REPLY }]); return shapeshift.shift('0x456', '1BTC', 'btc_eth'); }); @@ -125,4 +141,79 @@ describe('shapeshift/calls', () => { }); }); }); + + describe('subscriptions', () => { + const ADDRESS = '0123456789abcdef'; + const REPLY = { + status: 'complete', + address: ADDRESS + }; + + let callback; + + beforeEach(() => { + mockget(shapeshift, [{ path: `txStat/${ADDRESS}`, reply: REPLY }]); + callback = sinon.stub(); + shapeshift.subscribe(ADDRESS, callback); + }); + + describe('subscribe', () => { + it('adds the depositAddress to the list', () => { + const subscriptions = shapeshift._getSubscriptions(); + + expect(subscriptions.length).to.equal(1); + expect(subscriptions[0].depositAddress).to.equal(ADDRESS); + }); + + it('starts the polling timer', () => { + expect(shapeshift._isPolling()).to.be.true; + }); + + it('calls the callback once the timer has elapsed', () => { + clock.tick(2222); + + return shapeshift._getSubscriptionPromises().then(() => { + expect(callback).to.have.been.calledWith(null, REPLY); + }); + }); + + it('auto-unsubscribes on completed', () => { + clock.tick(2222); + + return shapeshift._getSubscriptionPromises().then(() => { + expect(shapeshift._getSubscriptions().length).to.equal(0); + }); + }); + }); + + describe('unsubscribe', () => { + it('unbsubscribes when requested', () => { + expect(shapeshift._getSubscriptions().length).to.equal(1); + shapeshift.unsubscribe(ADDRESS); + expect(shapeshift._getSubscriptions().length).to.equal(0); + }); + + it('clears the polling on no subscriptions', () => { + shapeshift.unsubscribe(ADDRESS); + expect(shapeshift._isPolling()).to.be.false; + }); + + it('handles unsubscribe of auto-unsubscribe', () => { + clock.tick(2222); + + return shapeshift._getSubscriptionPromises().then(() => { + expect(shapeshift.unsubscribe(ADDRESS)).to.be.true; + }); + }); + + it('handles unsubscribe when multiples listed', () => { + const ADDRESS2 = 'abcdef0123456789'; + + shapeshift.subscribe(ADDRESS2, sinon.stub()); + expect(shapeshift._getSubscriptions().length).to.equal(2); + shapeshift.unsubscribe(ADDRESS2); + expect(shapeshift._getSubscriptions()[0].depositAddress).to.equal(ADDRESS); + }); + }); + }); }); From e777f4d5c79853315c7be81ad070f08c29c7d556 Mon Sep 17 00:00:00 2001 From: Jaco Greeff Date: Sat, 31 Dec 2016 12:19:39 +0100 Subject: [PATCH 4/4] Swap multiple list test addresses --- js/src/3rdparty/shapeshift/shapeshift.spec.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/js/src/3rdparty/shapeshift/shapeshift.spec.js b/js/src/3rdparty/shapeshift/shapeshift.spec.js index 57bfdfb39b5..1897178b3c5 100644 --- a/js/src/3rdparty/shapeshift/shapeshift.spec.js +++ b/js/src/3rdparty/shapeshift/shapeshift.spec.js @@ -211,8 +211,9 @@ describe('shapeshift/calls', () => { shapeshift.subscribe(ADDRESS2, sinon.stub()); expect(shapeshift._getSubscriptions().length).to.equal(2); - shapeshift.unsubscribe(ADDRESS2); expect(shapeshift._getSubscriptions()[0].depositAddress).to.equal(ADDRESS); + shapeshift.unsubscribe(ADDRESS); + expect(shapeshift._getSubscriptions()[0].depositAddress).to.equal(ADDRESS2); }); }); });