Added support for v1 connectors with wallets #1029
Conversation
Signed-off-by: RosieMurphy0 <[email protected]>
| * @param {string} privateKey The identity private key | ||
| */ | ||
| async import(mspId, identityName, certificate, privateKey) { | ||
| const exists = await this.wallet.export(identityName); |
There was a problem hiding this comment.
there is an exists call on the wallet that would be better just in case export throws an error if not found
const exists = await this.wallet.exists(identityName)
| * @returns {[string]} all the identity names in the wallet | ||
| */ | ||
| async getAllIdentityNames() { | ||
| return await this.wallet.list(); |
There was a problem hiding this comment.
In the case of a v1 wallet, this.wallet.list doesn't return the list of identity names. It returns a list of IdentityInfo objects. The getAllLabels api should return just a list of identity names which is what we need.
| * @param {[string]} walletPath optional path to a file system wallet | ||
| */ | ||
| async create(walletPath) { | ||
| const walletFacade = new WalletFacade(); |
There was a problem hiding this comment.
Now that intiialize is not required, this can be simplified to
async create(walletPath) {
return new WalletFacade(walletPath);
}
| chai.should(); | ||
| const mockery = require('mockery'); | ||
|
|
||
| /** |
| * simulate a node sdk v1 wallet | ||
| */ | ||
| class StubWallet { | ||
| /** |
| this.map = new Map(); | ||
| } | ||
|
|
||
| /** |
| this.map.set(key, value); | ||
| } | ||
|
|
||
| /** |
| } | ||
|
|
||
| /** | ||
| * |
| return this.map.get(key); | ||
| } | ||
|
|
||
| /** |
| /** | ||
| * | ||
| */ | ||
| async list() { |
There was a problem hiding this comment.
change this from list to getAllLabels as that api is the one we want to be using in the real wallet
| } | ||
| } | ||
|
|
||
| /** |
| */ | ||
| class InMemoryWallet extends StubWallet {} | ||
|
|
||
| /** |
| async export(identityName) { | ||
| const exported = await this.wallet.export(identityName); | ||
| if (exported) { | ||
| return new ExportedIdentity(exported.mspid, exported.credentials.cert, exported.credentials.key); |
There was a problem hiding this comment.
In a v1 wallet the format returned by the export is
{
mspId,
certificate,
privateKey
}
So this line should read
return new ExportedIdentity(exported.mspId, exported.certificate, exported.privateKey);
| */ | ||
| class FileSystemWallet extends StubWallet {} | ||
|
|
||
| /** |
| * x509walletmixin class | ||
| */ | ||
| class X509WalletMixin { | ||
| /** |
| * @param {*} key b | ||
| * @return {*} identity b | ||
| */ | ||
| static createIdentity(mspid, cert, key){ |
There was a problem hiding this comment.
The code from createIdentity can be almost directly copied from the 1.4 node sdk which is
static createIdentity(mspId, certificate, privateKey) {
logger.debug('in createIdentity: mspId = ' + mspId);
return {
type: 'X509',
mspId,
certificate,
privateKey
};
}just remove the logger.debug line
|
|
||
| describe('When testing a V1 Wallet Facade Implementation', () => { | ||
|
|
||
| beforeEach(() => { |
There was a problem hiding this comment.
we don't need this as a beforeEach, so can be removed
| mockery.registerMock('fabric-network', {FileSystemWallet, InMemoryWallet, X509WalletMixin}); | ||
| }); | ||
|
|
||
| afterEach(() => { |
There was a problem hiding this comment.
change this from afterEach to after by doing this it means the beforeEach can be removed
| mockery.deregisterAll(); | ||
| mockery.disable(); | ||
| }); | ||
| it('A Wallet Facade Factory should create a wallet facade', async () => { |
There was a problem hiding this comment.
Needs a new line before the it statement
| const exported = await walletFacade.export('label'); | ||
| exported.should.deep.equal({mspid: 'mspid', certificate: 'cert', privateKey: 'key'}); | ||
| }); | ||
|
|
There was a problem hiding this comment.
I'm going to add this test in as well to the v2 tests so could you add this extra test in here as well
it('A wallet facade should return null on export if the identity does not exist', async () => {
const walletFacade = await new WalletFacadeFactory().create();
const exported = await walletFacade.export('label');
should.equal(exported, null);
});
Signed-off-by: RosieMurphy0 <[email protected]>
| async export(identityName) { | ||
| const exported = await this.wallet.export(identityName); | ||
| if (exported) { | ||
| return new ExportedIdentity(exported.mspId, exported.credentials.certificate, exported.credentials.privateKey); |
There was a problem hiding this comment.
The information in exported is not in the format you are expecting here. If you look at the code for createIdentity in node-sdk 1.4 X509Wallet looks like this
static createIdentity(mspId, certificate, privateKey) {
return {
type: 'X509',
mspId,
certificate,
privateKey
};
}You can ignore the type property but you see there is no credentials property.
| class FileSystemWallet extends StubWallet {} | ||
|
|
||
| class X509WalletMixin { | ||
| static createIdentity(mspId, certificate, privateKey){ |
There was a problem hiding this comment.
This should match the 1.4 implementation, as shown in the comment above
Signed-off-by: RosieMurphy0 <[email protected]>
* added wallet support for the v1 connectors Signed-off-by: RosieMurphy0 <[email protected]> * added wallet support for the v1 connectors Signed-off-by: RosieMurphy0 <[email protected]> * added wallet support for the v1 connectors Signed-off-by: RosieMurphy0 <[email protected]>
Signed-off-by: RosieMurphy0 [email protected]
Adds support for v1 connectors through 2 files:
WalletFacadeandWalletFacadeFactory.Contributes to #940