Unsubscribe error on ShapeShift modal close (#4005)

* unsubscribe in onClose (state available)

* Revert "unsubscribe in onClose (state available)"

This reverts commit 1da0a7447563e3cb0d9149b0b9898ec93b483982.

* Fix shapeshift double unsubscribe

* Swap multiple list test addresses

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
 };

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);
     });
   });
 });

js/src/3rdparty/shapeshift/shapeshift.js

@@ -15,8 +15,9 @@
 // along with Parity.  If not, see <http://www.gnu.org/licenses/>.
 
 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 = _subscriptions.filter((sub) => sub.depositAddress !== depositAddress);
 
-    subscriptions = newSubscriptions;
-
-    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,

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 <http://www.gnu.org/licenses/>.
 
+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,80 @@ 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);
+        expect(shapeshift._getSubscriptions()[0].depositAddress).to.equal(ADDRESS);
+        shapeshift.unsubscribe(ADDRESS);
+        expect(shapeshift._getSubscriptions()[0].depositAddress).to.equal(ADDRESS2);
+      });
+    });
+  });
 });