IdCard API changes (hyperledger-archives#2118)

* IdCard API changes * Remove IdCard.getName function * Add IdCard.getUserName function * Change semantics of IdCard.getEnrollmentCredentials function * Migrate previous format ID card archives on load Signed-off-by: Mark S. Lewis <[email protected]> * Fix mocks to match the code. * Update card definition in docker-composer-playground.yml * Guard against null return from IdCard.getEnrollmentCredentials
pchaozhong · Sep 18, 2017 · 9d20612 · 9d20612
1 parent 81b8ab1
commit 9d20612
Show file tree

Hide file tree

Showing 39 changed files with 383 additions and 300 deletions.
diff --git a/packages/composer-common/api.txt b/packages/composer-common/api.txt
@@ -48,7 +48,7 @@ class FileWallet extends Wallet {
    + Promise remove(string) 
 }
 class IdCard {
-   + String getName() 
+   + String getUserName() 
    + String getDescription() 
    + String getBusinessNetworkName() 
    + Object getConnectionProfile() 

diff --git a/packages/composer-common/changelog.txt b/packages/composer-common/changelog.txt
@@ -11,6 +11,11 @@
 #
 # Note that the latest public API is documented using JSDocs and is available in api.txt.
 #
+Version 0.12.3 {4258e987144410b121e40d2dd9801e87} 2017-09-12
+- Remove IdCard.getName function
+- Add IdCard.getUserName function
+- Change semantics of IdCard.getEnrollmentCredentials function
+
 Version 0.11.4 {6f0338e1a0a12cea4676d1093fd436ea} 2017-08-17
 - Add IdCard.setCredentials function
 

diff --git a/packages/composer-common/lib/idcard.js b/packages/composer-common/lib/idcard.js
@@ -23,6 +23,8 @@ const CONNECTION_FILENAME = 'connection.json';
 const METADATA_FILENAME = 'metadata.json';
 const CREDENTIALS_DIRNAME = 'credentials';
 
+const CURRENT_VERSION = 1;
+
 /**
  * An ID card. Encapsulates credentials and other information required to connect to a specific business network
  * as a specific user.
@@ -46,28 +48,32 @@ class IdCard {
         const method = 'constructor';
         LOG.entry(method);
 
-        if (!(metadata && metadata.name)) {
-            throw Error('Required metadata field not found: name');
+        if (!(metadata && metadata.userName)) {
+            throw new Error('Required metadata field not found: userName');
         }
         if (!(connectionProfile && connectionProfile.name)) {
-            throw Error('Required connection field not found: name');
+            throw new Error('Required connection field not found: name');
         }
-
-        this.metadata = metadata;
+        this.metadata = Object.assign({ version: CURRENT_VERSION }, metadata);
         this.connectionProfile = connectionProfile;
         this.credentials = { };
 
+        if (this.metadata.version !== CURRENT_VERSION) {
+            throw new Error(`Incompatible card version ${this.metadata.version}. Current version is ${CURRENT_VERSION}`);
+        }
+
         LOG.exit(method);
     }
 
     /**
-     * Name of the card. This is typically used for display purposes, and is not a unique identifier.
+     * Name of the user identity associated with the card. This should be unique within the scope of a given
+     * business network and connection profile.
      * <p>
      * This is a mandatory field.
-     * @return {String} name of the card.
+     * @return {String} Name of the user identity.
      */
-    getName() {
-        return this.metadata.name;
+    getUserName() {
+        return this.metadata.userName;
     }
 
     /**
@@ -129,27 +135,20 @@ class IdCard {
      * enroll with a business network and obtain certificates.
      * <p>
      * For an ID/secret enrollment scheme, the credentials are expected to be of the form:
-     * <em>{ id: String, secret: String }</em>.
+     * <em>{ secret: String }</em>.
      * @return {Object} enrollment credentials, or {@link null} if none exist.
      */
     getEnrollmentCredentials() {
-        let result = null;
-        const id = this.metadata.enrollmentId;
         const secret = this.metadata.enrollmentSecret;
-        if (id || secret) {
-            result = { };
-            result.id = id;
-            result.secret = secret;
-        }
-        return result;
+        return secret ? { secret: secret } : null;
     }
 
     /**
      * Special roles for which this ID can be used, which can include:
      * <ul>
-     *   <li>peerAdmin</li>
-     *   <li>channelAdmin</li>
-     *   <li>issuer</li>
+     *   <li>PeerAdmin</li>
+     *   <li>ChannelAdmin</li>
+     *   <li>Issuer</li>
      * </ul>
      * @return {String[]} roles.
      */
@@ -218,6 +217,14 @@ class IdCard {
             loadDirectoryToObject(CREDENTIALS_DIRNAME, credentials);
 
             return promise.then(() => {
+                // First cut of ID cards did not have a version so migrate them to version 1
+                if (!metadata.version) {
+                    metadata.userName = metadata.enrollmentId;
+                    delete metadata.enrollmentId;
+                    delete metadata.name;
+                    metadata.version = 1;
+                }
+
                 const idCard = new IdCard(metadata, connection);
                 idCard.setCredentials(credentials);
                 LOG.exit(method, idCard.toString());

diff --git a/packages/composer-common/test/data/id-cards/invalid-metadata-image/metadata.json b/packages/composer-common/test/data/id-cards/invalid-metadata-image/metadata.json
diff --git a/packages/composer-common/test/data/id-cards/minimal/metadata.json b/packages/composer-common/test/data/id-cards/minimal/metadata.json
@@ -1,3 +1,4 @@
 {
-    "name": "Minimal"
+    "userName": "minimal",
+    "version": 1
 }
diff --git a/packages/composer-common/test/data/id-cards/missing-connection-name/metadata.json b/packages/composer-common/test/data/id-cards/missing-connection-name/metadata.json
@@ -1,3 +1,4 @@
 {
-    "name": "Minimal"
+    "userName": "minimal",
+    "version": 1
 }
diff --git a/packages/composer-common/test/data/id-cards/missing-connection/metadata.json b/packages/composer-common/test/data/id-cards/missing-connection/metadata.json
@@ -1,3 +1,4 @@
 {
-    "name": "Minimal"
+    "userName": "minimal",
+    "version": 1
 }
diff --git a/packages/composer-common/test/data/id-cards/missing-metadata-name/metadata.json b/packages/composer-common/test/data/id-cards/missing-metadata-name/metadata.json
diff --git a/...rds/missing-metadata-name/connection.json → ...missing-metadata-username/connection.json b/...rds/missing-metadata-name/connection.json → ...missing-metadata-username/connection.json
diff --git a/packages/composer-common/test/data/id-cards/missing-metadata-username/metadata.json b/packages/composer-common/test/data/id-cards/missing-metadata-username/metadata.json
@@ -0,0 +1,3 @@
+{
+    "version": 1
+}
diff --git a/...ds/invalid-metadata-image/connection.json → ...st/data/id-cards/valid-v0/connection.json b/...ds/invalid-metadata-image/connection.json → ...st/data/id-cards/valid-v0/connection.json
@@ -1,4 +1,5 @@
 {
+    "name": "hlfv1",
     "type": "hlfv1",
     "orderers": [
         {

diff --git a/packages/composer-common/test/data/id-cards/valid-v0/metadata.json b/packages/composer-common/test/data/id-cards/valid-v0/metadata.json
@@ -0,0 +1,7 @@
+{
+    "name": "name",
+    "description": "A valid ID card",
+    "businessNetwork": "org-acme-biznet",
+    "enrollmentId": "conga",
+    "enrollmentSecret": "super-secret-passphrase"
+}
diff --git a/packages/composer-common/test/data/id-cards/valid-with-enrollment/metadata.json b/packages/composer-common/test/data/id-cards/valid-with-enrollment/metadata.json
@@ -1,8 +1,8 @@
 {
-    "name": "Conga",
+    "version": 1,
+    "userName": "conga",
     "description": "A valid ID card",
     "businessNetwork": "org-acme-biznet",
     "image": "images/conga.png",
-    "enrollmentId": "conga",
     "enrollmentSecret": "super-secret-passphrase"
 }
diff --git a/packages/composer-common/test/data/id-cards/valid-with-roles/metadata.json b/packages/composer-common/test/data/id-cards/valid-with-roles/metadata.json
@@ -1,11 +1,12 @@
 {
-    "name": "Conga",
+    "version": 1,
+    "userName": "conga",
     "description": "A valid ID card",
     "businessNetwork": "org-acme-biznet",
     "image": "images/conga.png",
     "roles": [
-        "peerAdmin",
-        "channelAdmin",
-        "issuer"
+        "PeerAdmin",
+        "ChannelAdmin",
+        "Issuer"
     ]
 }
diff --git a/packages/composer-common/test/data/id-cards/valid/metadata.json b/packages/composer-common/test/data/id-cards/valid/metadata.json
@@ -1,5 +1,6 @@
 {
-    "name": "Conga",
+    "userName": "conga",
+    "version": 1,
     "description": "A valid ID card",
     "businessNetwork": "org-acme-biznet",
     "image": "images/conga.png"

diff --git a/packages/composer-common/test/idcard.js b/packages/composer-common/test/idcard.js
@@ -74,14 +74,16 @@ class DirectoryVisitor {
 }
 
 describe('IdCard', function() {
+    let minimalMetadata;
+    let minimalConnectionProfile;
     let emptyCredentials;
     let validCredentials;
     let minimalCard;
     let credentialsCard;
 
     beforeEach(function() {
-        const minimalMetadata = { name: 'minimal'};
-        const minimalConnectionProfile = { name: 'minimal' };
+        minimalMetadata = { userName: 'minimal' };
+        minimalConnectionProfile = { name: 'minimal' };
 
         emptyCredentials = { };
         validCredentials = {
@@ -94,6 +96,16 @@ describe('IdCard', function() {
         credentialsCard.setCredentials(validCredentials);
     });
 
+    describe('#constructor', function() {
+        it('should reject newer card versions', function() {
+            const metadata = minimalCard.metadata;
+            metadata.version++;
+            should.throw(() => {
+                new IdCard(metadata, minimalConnectionProfile);
+            }, new RegExp(metadata.version.toString(10)));
+        });
+    });
+
     describe('#fromArchive', function() {
         it('should load a minimal card file without error', function() {
             return readIdCardAsync('minimal').then((readBuffer) => {
@@ -121,17 +133,17 @@ describe('IdCard', function() {
             }).should.be.rejectedWith(/metadata.json/);
         });
 
-        it('should throw error on missing name field in metadata', function() {
-            return readIdCardAsync('missing-metadata-name').then((readBuffer) => {
+        it('should throw error on missing userName field in metadata', function() {
+            return readIdCardAsync('missing-metadata-username').then((readBuffer) => {
                 return IdCard.fromArchive(readBuffer);
-            }).should.be.rejectedWith(/name/);
+            }).should.be.rejectedWith(/userName/);
         });
 
-        it('should load name', function() {
+        it('should load userName', function() {
             return readIdCardAsync('valid').then((readBuffer) => {
                 return IdCard.fromArchive(readBuffer);
             }).then(card => {
-                card.getName().should.equal('Conga');
+                card.getUserName().should.equal('conga');
             });
         });
 
@@ -199,8 +211,7 @@ describe('IdCard', function() {
                 return IdCard.fromArchive(readBuffer);
             }).then(card => {
                 const credentials = card.getEnrollmentCredentials();
-                credentials.id.should.equal('conga');
-                credentials.secret.should.equal('super-secret-passphrase');
+                credentials.should.deep.equal({ secret: 'super-secret-passphrase' });
             });
         });
 
@@ -217,7 +228,7 @@ describe('IdCard', function() {
                 return IdCard.fromArchive(readBuffer);
             }).then(card => {
                 const roles = card.getRoles();
-                roles.should.have.members(['peerAdmin', 'channelAdmin', 'issuer']);
+                roles.should.have.members(['PeerAdmin', 'ChannelAdmin', 'Issuer']);
             });
         });
 
@@ -229,6 +240,16 @@ describe('IdCard', function() {
                 roles.should.be.empty;
             });
         });
+
+        it('should migrate a version 0 card', function() {
+            return readIdCardAsync('valid-v0').then((readBuffer) => {
+                return IdCard.fromArchive(readBuffer);
+            }).then(card => {
+                card.getUserName().should.equal('conga');
+                should.not.exist(card.metadata.name);
+                should.not.exist(card.metadata.enrollmentId);
+            });
+        });
     });
 
     describe('#toArchive', function() {

diff --git a/packages/composer-playground/src/app/common/identity-card/identity-card.component.html b/packages/composer-playground/src/app/common/identity-card/identity-card.component.html
@@ -16,7 +16,7 @@
                 </svg>
             </div>
         </div>
-        <div class="user-name">{{identity.getName()}}</div>
+        <div class="user-name">{{identity.getUserName()}}</div>
     </section>
     <section *ngIf="!preview" class="actions">
         <button [disabled]="indestructible" type="button" class="action circular delete"

diff --git a/packages/composer-playground/src/app/common/identity-card/identity-card.component.spec.ts b/packages/composer-playground/src/app/common/identity-card/identity-card.component.spec.ts
@@ -24,7 +24,7 @@ describe(`IdentityCardComponent`, () => {
         };
 
         mockIdCard = sinon.createStubInstance(IdCard);
-        mockIdCard.getName.returns('pedantic-owl');
+        mockIdCard.getUserName.returns('pedantic-owl');
         mockIdCard.getBusinessNetworkName.returns('conga-network');
         mockIdCard.getConnectionProfile.returns(mockConnectionProfile);
         mockIdCard.getRoles.returns([]);
@@ -101,7 +101,7 @@ describe(`IdentityCardComponent`, () => {
 
     describe('#getInitials', () => {
         it('should get one initial', () => {
-            mockIdCard.getName.returns('admin');
+            mockIdCard.getUserName.returns('admin');
             component.identity = mockIdCard;
 
             let result = component.getInitials();
@@ -110,7 +110,7 @@ describe(`IdentityCardComponent`, () => {
         });
 
         it('should get two initials', () => {
-            mockIdCard.getName.returns('pedantic owl');
+            mockIdCard.getUserName.returns('pedantic owl');
             component.identity = mockIdCard;
 
             let result = component.getInitials();
@@ -119,7 +119,7 @@ describe(`IdentityCardComponent`, () => {
         });
 
         it('should get maximum of two initials', () => {
-            mockIdCard.getName.returns('eat conga repeat');
+            mockIdCard.getUserName.returns('eat conga repeat');
             component.identity = mockIdCard;
 
             let result = component.getInitials();
@@ -128,7 +128,7 @@ describe(`IdentityCardComponent`, () => {
         });
 
         it('should get non-ascii \'initials\'', () => {
-            mockIdCard.getName.returns('黄 丽');
+            mockIdCard.getUserName.returns('黄 丽');
             component.identity = mockIdCard;
 
             let result = component.getInitials();
@@ -137,7 +137,7 @@ describe(`IdentityCardComponent`, () => {
         });
 
         it('should smile if there are no initials', () => {
-            mockIdCard.getName.returns(' ');
+            mockIdCard.getUserName.returns(' ');
             component.identity = mockIdCard;
 
             let result = component.getInitials();

diff --git a/packages/composer-playground/src/app/common/identity-card/identity-card.component.ts b/packages/composer-playground/src/app/common/identity-card/identity-card.component.ts
@@ -37,24 +37,24 @@ export class IdentityCardComponent {
     onExport: EventEmitter<string> = new EventEmitter<string>();
 
     connect() {
-        this.onConnect.emit(this.identity.getName());
+        this.onConnect.emit(this.identity.getUserName());
     }
 
     dismiss() {
-        this.onDismiss.emit(this.identity.getName());
+        this.onDismiss.emit(this.identity.getUserName());
     }
 
     delete() {
-        this.onDelete.emit(this.identity.getName());
+        this.onDelete.emit(this.identity.getUserName());
     }
 
     export() {
-        this.onExport.emit(this.identity.getName());
+        this.onExport.emit(this.identity.getUserName());
     }
 
     getInitials(): string {
         let result;
-        let userId = this.identity.getName();
+        let userId = this.identity.getUserName();
         let regexp = /^(\S)\S*\s*(\S?)/i;
         let matches = regexp.exec(userId);
 

diff --git a/...es/composer-playground/src/app/identity/identity-issued/identity-issued.component.spec.ts b/...es/composer-playground/src/app/identity/identity-issued/identity-issued.component.spec.ts
@@ -95,8 +95,8 @@ describe('IdentityIssuedComponent', () => {
 
             tick();
 
-            component['newCard'].getName().should.equal('dan');
-            component['newCard'].getEnrollmentCredentials().should.deep.equal({id: 'dan', secret: 'wotnodolphin'});
+            component['newCard'].getUserName().should.equal('dan');
+            component['newCard'].getEnrollmentCredentials().should.deep.equal({secret: 'wotnodolphin'});
             component['newCard'].getBusinessNetworkName().should.equal('dan-net');
             component['newCard'].getConnectionProfile().should.deep.equal({name: 'dan-profile'});
         }));