Fix error when removing a wallet on askar-profile

[baegjae]

This PR fixes error when removing a wallet on askar-profile

Environment

askar-profile

Problem

Error occurred when removing a wallet.

Fix

Unlike other options, in askar-profile, the profiles of sub wallet are not stored in self._instances.
However, current code always try to delete the profile in self._instances when removing a wallet.
This PR resolves the above problem.

Thanks!

Signed-off-by: Ethan Sung [email protected]

[codecov-commenter]

Codecov Report

Merging #1518 (56dc521) into main (04a0264) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main    #1518   +/-   ##
=======================================
  Coverage   95.70%   95.70%           
=======================================
  Files         521      521           
  Lines       32160    32165    +5     
=======================================
+ Hits        30779    30784    +5     
  Misses       1381     1381           

[ianco]

I don't think this is correct. There are two separate instances of the multitenant manager that derive from base - one is MultitenantManager and one is AskarProfileMultitenantManager. Both add loaded sub-wallets to the self._instances array.

I suspect that the wrong instance type of the manager is getting loaded when running in askar mode (i.e. MultitenantManager instead of AskarProfileMultitenantManager) and that's why self._instances seems to be missing the sub-wallet profile.

[ianco]

Askar sub-wallets get added to self._instances here:

https://github.com/hyperledger/aries-cloudagent-python/blob/main/aries_cloudagent/multitenant/askar_profile_manager.py#L68

[baegjae]

@ianco
Thanks for the comment.
I tested with three cases.

Case 1. indy - basic
--wallet-type indy, --multitenancy-config Omit

Case 2. askar - basic
--wallet-type askar, --multitenancy-config Omit

Case 3. askar - askar-profile
--wallet-type askar, --multitenancy-config {"wallet_type":"askar-profile", "wallet_name":"askar-profile-name"}

Case 1 and 2 use get_wallet_profile of MultitenantManager.
In get_wallet_profile, sub-wallets are added to self._instances.

Case 3 uses get_wallet_profile of AskarProfileMultitenantManager.
In get_wallet_profile, sub-wallets are added to multitenant_wallet.
In here, self._instances contains a single profile with key multitenant_wallet_name.
https://github.com/hyperledger/aries-cloudagent-python/blob/main/aries_cloudagent/multitenant/askar_profile_manager.py#L45

The single profile (multitenant_wallet) is used to store sub-wallets and to get the template of a sub-wallet context.

This PR handles the Case 3.

Thanks!

[ianco]

Hi @baegjae thanks for the info!

Since the behaviour of the MultitenantManager and AskarProfileMultitenantManager is different, I suggest to refactor the code to move the del self._instances[wallet_id] operation into the MultitenantManager class. (I don't like base classes having dependencies on their sub-classes).

Maybe define a virtual method in the base class called cleanup_local_wallet_instances() (or something like that) and in the MultitenantManager sub-class it can delete from the local self._instances and in the Askar sub-class it can be a no-op

[baegjae]

Hi @ianco
I agree your comment.
I added a virtual method named remove_wallet_profile.
In MultitenantManager, the method removes the profile from self._instances and removes the profile.
In AskarProfileMultitenantManager, the method only removes the profile.

Also, I updated and added the unit test for this.

Thanks!