Fix issue with admin importCard API (hyperledger-archives#4252)

cardStore.has returns a promise The setup for the wallet needs to be done in case removeIdentity is called. closes #4248 Signed-off-by: Dave Kelsey <[email protected]>
avinashtgoje · Jul 16, 2018 · 4f979d4 · 4f979d4
1 parent 37fdef8
commit 4f979d4
Show file tree

Hide file tree

Showing 2 changed files with 20 additions and 6 deletions.
diff --git a/packages/composer-admin/lib/adminconnection.js b/packages/composer-admin/lib/adminconnection.js
@@ -84,14 +84,14 @@ class AdminConnection {
         const connectionProfileData = card.getConnectionProfile();
         connectionProfileData.cardName = name;
         const connectionManager = await this.connectionProfileManager.getConnectionManagerByType(connectionProfileData['x-type']);
-        const exists = this.cardStore.has(name);
+        connectionProfileData.wallet = await this.cardStore.getWallet(name);
+        const exists = await this.cardStore.has(name);
         if (exists) {
             await connectionManager.removeIdentity(connectionProfileData.name, connectionProfileData, card.getUserName());
         }
         // if we have a certificate and optionally a privateKey we should ask the connection manager to import
         const certificate = card.getCredentials().certificate;
         const privateKey = card.getCredentials().privateKey;
-        connectionProfileData.wallet = await this.cardStore.getWallet(name);
         if (certificate){
             await connectionManager.importIdentity(connectionProfileData.name, connectionProfileData, card.getUserName(), certificate, privateKey);
         }

diff --git a/packages/composer-admin/test/adminconnection.js b/packages/composer-admin/test/adminconnection.js
@@ -749,7 +749,8 @@ describe('AdminConnection', () => {
 
             it('should import a new card to card store', function() {
                 const cardName = 'conga';
-                sandbox.stub(cardStore, 'has').returns(false);
+                sandbox.stub(cardStore, 'has').resolves(false);
+                sandbox.stub(cardStore, 'getWallet').resolves('something');
                 return adminConnection.importCard(cardName, userCard)
                 .then((updated) => {
                     updated.should.be.false;
@@ -762,10 +763,16 @@ describe('AdminConnection', () => {
                 const cardName = 'conga';
                 let expectedConnection = Object.assign({}, connection);
                 expectedConnection.cardName = cardName;
-                sandbox.stub(cardStore, 'has').returns(true);
+                expectedConnection.wallet = 'something';
+                sandbox.stub(cardStore, 'has').resolves(true);
+                sandbox.stub(cardStore, 'getWallet').resolves('something');
                 return adminConnection.importCard(cardName, userCard)
                 .then((updated) => {
                     sinon.assert.calledOnce(mockConnectionManager.removeIdentity);
+                    // A bug in sinon here: the object passed to removeIdentity is held as a reference by sinon
+                    // when recording the call was invoked. Such that if the object is updated later in the method
+                    // under test, it may look as though it was called with an object with the expected properties
+                    // but actually it wasn't.
                     sinon.assert.calledWith(mockConnectionManager.removeIdentity, 'connectionName', sinon.match(expectedConnection), 'user');
                     updated.should.be.true;
                     return cardStore.get(cardName).should.eventually.deep.equal(userCard);
@@ -783,11 +790,18 @@ describe('AdminConnection', () => {
             it('should import identity if card contains credentials', function () {
                 const certificate = 'CERTIFICATE_DATA';
                 const privateKey = 'PRIVATE_KEY_DATA';
+                const cardName = 'conga';
                 userCard.setCredentials({certificate : certificate, privateKey : privateKey});
-                return adminConnection.importCard('conga', userCard).then(() => {
+                let expectedConnection = Object.assign({}, connection);
+                expectedConnection.cardName = cardName;
+                expectedConnection.wallet = 'something';
+                sandbox.stub(cardStore, 'has').resolves(true);
+                sandbox.stub(cardStore, 'getWallet').resolves('something');
+
+                return adminConnection.importCard(cardName, userCard).then(() => {
                     return sinon.assert.calledWith(mockConnectionManager.importIdentity,
                         userCard.getConnectionProfile().name,
-                        sinon.match.object,
+                        sinon.match(expectedConnection),
                         userCard.getUserName(),
                         certificate,
                         privateKey