Merge pull request #2195 from matiu/bug/walletids

better walletIds storage

index.html

@@ -65,7 +65,7 @@
             <section class="left-small">
               <a class="left-off-canvas-toggle menu-icon" ><span></span></a>
             </section>
-            <section class="right-small" ng-show="$root.iden && $root.iden.listWallets().length >1">
+            <section class="right-small" ng-show="$root.iden && $root.iden.getWallets().length >1">
               <a class="right-off-canvas-toggle p10"><i class="icon-wallet size-24"></i></a>
             </section>
 
@@ -80,7 +80,7 @@
             <div ng-include="'views/includes/sidebar-mobile.html'"></div>
           </nav>
 
-          <nav class="right-off-canvas-menu" ng-show="$root.iden && $root.iden.listWallets().length >1">
+          <nav class="right-off-canvas-menu" ng-show="$root.iden && $root.iden.getWallets().length >1">
             <div ng-include="'views/includes/walletbar-mobile.html'"></div>
           </nav>
           <div 

js/controllers/profile.js

@@ -28,7 +28,7 @@ angular.module('copayApp.controllers').controller('ProfileController', function(
   $scope.setWallets = function() {
     if (!$rootScope.iden) return;
 
-    var wallets = $rootScope.iden.listWallets();
+    var wallets = $rootScope.iden.getWallets();
     var max = $rootScope.quotaPerItem;
 
     _.each(wallets, function(w) {

js/controllers/sidebar.js

@@ -85,7 +85,7 @@ angular.module('copayApp.controllers').controller('SidebarController', function(
 
   $scope.setWallets = function() {
     if (!$rootScope.iden) return;
-    var ret = _.filter($rootScope.iden.listWallets(), function(w) {
+    var ret = _.filter($rootScope.iden.getWallets(), function(w) {
       return w;
     });
     $scope.wallets = _.sortBy(ret, 'name');

js/controllers/walletForPayment.js

@@ -14,7 +14,7 @@ angular.module('copayApp.controllers').controller('walletForPaymentController',
     });
 
     $scope.setWallets = function() {
-      $scope.wallets = $rootScope.iden.listWallets();
+      $scope.wallets = $rootScope.iden.getWallets();
     };
 
     $scope.ok = function(w) {

js/models/Identity.js

@@ -58,7 +58,7 @@ function Identity(opts) {
   this.walletDefaults = opts.walletDefaults || {};
   this.version = opts.version || version;
 
-  this.walletIds = opts.walletIds || {};
+  this.walletIds = opts.walletIds || [];
   this.wallets = opts.wallets || {};
   this.focusedTimestamps = opts.focusedTimestamps || {};
   this.backupNeeded = opts.backupNeeded || false;
@@ -139,6 +139,59 @@ Identity.open = function(opts, cb) {
   });
 };
 
+/**
+ * @param {string} walletId
+ * @returns {Wallet}
+ */
+Identity.prototype.getWalletById = function(walletId) {
+  return this.wallets[walletId];
+};
+
+/**
+ * @returns {Wallet[]}
+ */
+Identity.prototype.getWallets = function() {
+  return _.values(this.wallets);
+};
+
+/**
+ * addWallet
+ *
+ * @param w
+ */
+Identity.prototype.addWallet = function(w) {
+  this.wallets[w.getId()] = w;
+  this.walletIds = _.union(this.walletIds, [w.getId()]);
+};
+
+/**
+ * @desc Deletes a wallet. This involves removing it from the storage instance
+ *
+ * @param {string} walletId
+ * @callback cb
+ * @return {err}
+ */
+Identity.prototype.deleteWallet = function(walletId, cb) {
+  preconditions.checkArgument(_.isString(walletId));
+
+  var self = this;
+
+
+
+  var w = this.getWalletById(walletId);
+  w.close();
+
+  delete this.wallets[walletId];
+  delete this.focusedTimestamps[walletId];
+  this.walletIds = _.without(this.walletIds, walletId);
+
+  this.storage.removeItem(Wallet.getStorageKey(walletId), function(err) {
+    if (err) return cb(err);
+    self.emitAndKeepAlive('walletDeleted', walletId);
+    self.store(null, cb);
+  });
+};
+
 
 /**
  * readAndBindWallet
@@ -151,7 +204,7 @@ Identity.prototype.readAndBindWallet = function(walletId, cb) {
   var self = this;
   self.retrieveWalletFromStorage(walletId, {}, function(error, wallet) {
     if (!error) {
-      self.wallets[wallet.getId()] = wallet;
+      self.addWallet(wallet);
       self.bindWallet(wallet);
     }
     return cb(error);
@@ -275,10 +328,7 @@ Identity.storeWalletDebounced = _.debounce(function(identity, wallet, cb) {
 
 
 Identity.prototype.toObj = function() {
-  return _.extend({
+  return _.pick(this, 'walletIds', 'version', 'fullName', 'password', 'email', 'backupNeeded', 'focusedTimestamps');
-      walletIds: _.isEmpty(this.wallets) ? this.walletsIds : _.keys(this.wallets),
-    },
-    _.pick(this, 'version', 'fullName', 'password', 'email', 'backupNeeded', 'focusedTimestamps'));
 };
 
 Identity.prototype.exportEncryptedWithWalletInfo = function(opts) {
@@ -303,7 +353,7 @@ Identity.prototype.setBackupDone = function() {
 
 Identity.prototype.exportWithWalletInfo = function(opts) {
   return _.extend({
-      wallets: _.map(this.wallets, function(wallet) {
+      wallets: _.map(this.getWallets(), function(wallet) {
         return wallet.toObj();
       })
     },
@@ -329,7 +379,7 @@ Identity.prototype.store = function(opts, cb) {
     if (opts.noWallets)
       return cb();
 
-    async.each(_.values(self.wallets), function(wallet, in_cb) {
+    async.each(self.getWallets(), function(wallet, in_cb) {
       self.storeWallet(wallet, in_cb);
     }, cb);
   });
@@ -345,7 +395,7 @@ Identity.prototype.remove = function(opts, cb) {
   var self = this;
   opts = opts || {};
 
-  async.each(_.values(self.wallets), function(w, cb) {
+  async.each(self.getWallets(), function(w, cb) {
     w.close();
     self.storage.removeItem(Wallet.getStorageKey(w.getId()), function(err) {
       if (err) return cb(err);
@@ -357,7 +407,7 @@ Identity.prototype.remove = function(opts, cb) {
     self.storage.removeItem(self.getId(), function(err) {
       if (err) return cb(err);
 
-      self.storage.clear(function (err) {
+      self.storage.clear(function(err) {
         if (err) return cb(err);
 
         self.emitAndKeepAlive('closed');
@@ -368,7 +418,7 @@ Identity.prototype.remove = function(opts, cb) {
 };
 
 Identity.prototype._cleanUp = function() {
-  _.each(this.wallets, function(w){
+  _.each(this.getWallets(), function(w) {
     w.close();
   });
 };
@@ -402,7 +452,7 @@ Identity.prototype.importWalletFromObj = function(obj, opts, cb) {
   log.debug('Updating Indexes for wallet:' + w.getName());
   w.updateIndexes(function(err) {
     log.debug('Adding wallet to profile:' + w.getName());
-    self.wallets[w.getId()] = w;
+    self.addWallet(w);
     self.updateFocusedTimestamp(w.getId());
     self.bindWallet(w);
     self.storeWallet(w, cb);
@@ -489,7 +539,7 @@ Identity.importFromFullJson = function(str, password, opts, cb) {
  * @emits newWallet  (walletId)
  */
 Identity.prototype.bindWallet = function(w) {
-  preconditions.checkArgument(w && this.wallets[w.getId()]);
+  preconditions.checkArgument(w && this.getWalletById(w.getId()));
 
   var self = this;
   log.debug('Binding wallet:' + w.getName());
@@ -593,10 +643,10 @@ Identity.prototype.createWallet = function(opts, cb) {
   var self = this;
   var w = new walletClass(opts);
 
-  if (self.wallets[w.getId()]) {
+  if (self.getWalletById(w.getId())) {
     return cb('walletAlreadyExists');
   }
-  self.wallets[w.getId()] = w;
+  self.addWallet(w);
   self.updateFocusedTimestamp(w.getId());
   self.bindWallet(w);
   self.storeWallet(w, function(err) {
@@ -633,43 +683,6 @@ Identity.prototype._checkVersion = function(inVersion) {
   }
 };
 
-/**
- * @param {string} walletId
- * @returns {Wallet}
- */
-Identity.prototype.getWalletById = function(walletId) {
-  return this.wallets[walletId];
-};
-
-/**
- * @returns {Wallet[]}
- */
-Identity.prototype.listWallets = function() {
-  return _.values(this.wallets);
-};
-
-/**
- * @desc Deletes a wallet. This involves removing it from the storage instance
- *
- * @param {string} walletId
- * @callback cb
- * @return {err}
- */
-Identity.prototype.deleteWallet = function(walletId, cb) {
-  var self = this;
-
-  var w = this.getWalletById(walletId);
-  w.close();
-
-  delete this.wallets[walletId];
-  delete this.focusedTimestamps[walletId];
-
-  this.storage.removeItem(Wallet.getStorageKey(walletId), function(err) {
-    if (err) return cb(err);
-    self.emitAndKeepAlive('walletDeleted', walletId);
-    self.store(null, cb);
-  });
-};
 
 /**
  * @desc Pass through to {@link Wallet#secret}
@@ -701,7 +714,7 @@ Identity.prototype.getLastFocusedWalletId = function() {
 };
 
 Identity.prototype.updateFocusedTimestamp = function(wid) {
-  preconditions.checkArgument(wid && this.wallets[wid]);
+  preconditions.checkArgument(wid && this.getWalletById(wid));
   this.focusedTimestamps[wid] = Date.now();
 };
 

js/services/go.js

@@ -37,7 +37,7 @@ angular.module('copayApp.services').factory('go', function($window, $rootScope,
         hideSidebars();
       }
       else {
-        if ($rootScope.iden && $rootScope.iden.listWallets().length >1) {
+        if ($rootScope.iden && $rootScope.iden.getWallets().length >1) {
           elem.addClass('move-left');
         }
       }

test/Identity.js

@@ -11,6 +11,7 @@ var Insight = require('../js/models/Insight');
 var Identity = copay.Identity;
 var Wallet = copay.Wallet;
 var Passphrase = copay.Passphrase;
+var version = copay.version;
 
 var FakeBlockchain = require('./mocks/FakeBlockchain');
 
@@ -78,6 +79,7 @@ describe('Identity model', function() {
   }
 
   var wid = 0;
+
   function getNewWallet(args) {
     var w = sinon.stub();
     w.getId = sinon.stub().returns('wid' + (++wid));
@@ -167,7 +169,7 @@ describe('Identity model', function() {
   });
 
   describe('#remove', function(done) {
-    it('should remove empty profile', function (done) {
+    it('should remove empty profile', function(done) {
       var storage = sinon.stub();
       storage.setCredentials = sinon.stub();
       storage.removeItem = sinon.stub().yields(null);
@@ -188,7 +190,7 @@ describe('Identity model', function() {
       };
 
       var iden = new Identity(opts);
-      iden.remove(null, function (err, res) {
+      iden.remove(null, function(err, res) {
         should.not.exist(err);
         storage.removeItem.calledOnce.should.be.true;
         storage.removeItem.getCall(0).args[0].should.equal(iden.getId());
@@ -264,33 +266,41 @@ describe('Identity model', function() {
       iden = new Identity(opts);
     });
 
-    it('should store a simple wallet', function (done) {
+    it('should store a simple wallet', function(done) {
       storage.setItem = sinon.stub().yields(null);
       var w = {
-        toObj: sinon.stub().returns({ key1: 'val1' }),
+        toObj: sinon.stub().returns({
+          key1: 'val1'
+        }),
         getStorageKey: sinon.stub().returns('storage_key'),
         getName: sinon.stub().returns('name'),
         setVersion: sinon.spy(),
       };
-      iden.storeWallet(w, function (err) {
+      iden.storeWallet(w, function(err) {
         should.not.exist(err);
         storage.setItem.calledOnce.should.be.true;
-        storage.setItem.calledWith('storage_key', { key1: 'val1' });
+        storage.setItem.calledWith('storage_key', {
+          key1: 'val1'
+        });
         done();
       });
     });
-    it('should change wallet version when storing', function (done) {
+    it('should change wallet version when storing', function(done) {
       storage.setItem = sinon.stub().yields(null);
       var w = {
-        toObj: sinon.stub().returns({ key1: 'val1' }),
+        toObj: sinon.stub().returns({
+          key1: 'val1'
+        }),
         getStorageKey: sinon.stub().returns('storage_key'),
         getName: sinon.stub().returns('name'),
         setVersion: sinon.spy(),
         version: '1.0',
-        opts: { version: '1.0' },
+        opts: {
+          version: '1.0'
+        },
       };
       iden.version = '2.0';
-      iden.storeWallet(w, function (err) {
+      iden.storeWallet(w, function(err) {
         should.not.exist(err);
         w.setVersion.calledWith('2.0').should.be.true;
         done();
@@ -554,4 +564,130 @@ describe('Identity model', function() {
       });
     });
   });
+
+
+  describe('add / delete / list Wallets', function() {
+    var iden, w;
+    beforeEach(function() {
+      var storage = sinon.stub();
+      storage.setCredentials = sinon.stub();
+      storage.removeItem = sinon.stub().yields(null);
+      storage.clear = sinon.stub().yields();
+
+      var opts = {
+        email: 'test@test.com',
+        password: '123',
+        network: {
+          testnet: {
+            url: 'https://test-insight.bitpay.com:443'
+          },
+          livenet: {
+            url: 'https://insight.bitpay.com:443'
+          },
+        },
+        storage: storage,
+      };
+      iden = new Identity(opts);
+
+      w = {
+        getId: sinon.stub().returns('32'),
+        getName: sinon.stub().returns('treintaydos'),
+        close: sinon.stub(),
+      };
+    });
+
+    it('should add wallet', function() {
+      iden.addWallet(w);
+      iden.getWalletById('32').getName().should.equal('treintaydos');
+      iden.walletIds.should.deep.equal(['32']);
+
+      _.find(iden.getWallets(), function(w) {
+        return w.getName() == 'treintaydos';
+      }).should.deep.equal(w);
+
+    });
+
+    it('should not add same wallet twice', function() {
+      iden.addWallet(w);
+      iden.addWallet(w);
+      iden.getWalletById('32').getName().should.equal('treintaydos');
+      iden.walletIds.should.deep.equal(['32']);
+      _.find(iden.getWallets(), function(w) {
+        return w.getName() == 'treintaydos';
+      }).should.deep.equal(w);
+    });
+
+ 
+
+    it('should delete wallet', function(done) {
+      iden.addWallet(w);
+      iden.getWalletById('32').getName().should.equal('treintaydos');
+      iden.deleteWallet('32', function(err) {
+        should.not.exist(iden.getWalletById('32'));
+        iden.walletIds.should.deep.equal([]);
+
+        should.not.exist(_.find(iden.getWallets(), function(w) {
+          return w.getName() == 'treintaydos';
+        }));
+        done();
+      });
+    });
+  });
+
+  describe('toObj', function() {
+    var iden, w, w2;
+    beforeEach(function() {
+      var storage = sinon.stub();
+      storage.setCredentials = sinon.stub();
+      storage.removeItem = sinon.stub().yields(null);
+      storage.clear = sinon.stub().yields();
+
+      var opts = {
+        email: 'test@test.com',
+        password: '123',
+        network: {
+          testnet: {
+            url: 'https://test-insight.bitpay.com:443'
+          },
+          livenet: {
+            url: 'https://insight.bitpay.com:443'
+          },
+        },
+        storage: storage,
+      };
+      iden = new Identity(opts);
+
+      w = {
+        getId: sinon.stub().returns('32'),
+        getName: sinon.stub().returns('treintaydos'),
+        close: sinon.stub(),
+      };
+      w2 = {
+        getId: sinon.stub().returns('33'),
+        getName: sinon.stub().returns('treintaytres'),
+        close: sinon.stub(),
+      };
+    });
+
+    it('should include wallets', function() {
+      iden.addWallet(w);
+      var obj = iden.toObj();
+      _.indexOf(obj.walletIds,'32').should.be.above(-1);
+    });
+
+    it('should set version to actual version', function() {
+      var obj = iden.toObj();
+      obj.version.should.equal(version);
+    });
+
+
+
+    it('should include 2 wallets', function() {
+      iden.addWallet(w);
+      iden.addWallet(w2);
+      var obj = iden.toObj();
+      _.indexOf(obj.walletIds,'32').should.be.above(-1);
+      _.indexOf(obj.walletIds,'33').should.be.above(-1);
+    });
+  });
 });

test/unit/controllers/controllersSpec.js

@@ -108,7 +108,7 @@ describe("Unit: Controllers", function() {
 
     var iden = {};
     iden.getLastFocusedWallet = sinon.stub().returns(null);
-    iden.listWallets = sinon.stub().returns([w]);
+    iden.getWallets = sinon.stub().returns([w]);
     iden.getWalletById = sinon.stub().returns(w);
     iden.getName = sinon.stub().returns('name');
     iden.deleteWallet = sinon.stub();