From 7d88b235e225cc158b54b8f8e4f438c5f55f751a Mon Sep 17 00:00:00 2001 From: Daniel Bluhm Date: Tue, 22 Mar 2022 23:02:54 -0400 Subject: [PATCH 01/28] feat: add caching for multi-tenant profiles Signed-off-by: Timo Glastra Signed-off-by: Daniel Bluhm --- aries_cloudagent/multitenant/cache.py | 94 ++++++++++++++++ .../multitenant/tests/test_cache.py | 100 ++++++++++++++++++ 2 files changed, 194 insertions(+) create mode 100644 aries_cloudagent/multitenant/cache.py create mode 100644 aries_cloudagent/multitenant/tests/test_cache.py diff --git a/aries_cloudagent/multitenant/cache.py b/aries_cloudagent/multitenant/cache.py new file mode 100644 index 0000000000..b228878160 --- /dev/null +++ b/aries_cloudagent/multitenant/cache.py @@ -0,0 +1,94 @@ +"""Cache for multitenancy profiles.""" + +import logging +import sys +from collections import OrderedDict +from typing import Optional + +from ..core.profile import Profile + +LOGGER = logging.getLogger(__name__) + + +class ProfileCache: + """Profile cache that caches based on LRU strategy.""" + + def __init__(self, capacity: int): + """Initialize ProfileCache. + + Args: + capacity: The capacity of the cache. If capacity is exceeded + profiles are closed. + """ + + self.profiles: OrderedDict[str, Profile] = OrderedDict() + self.capacity = capacity + + async def _cleanup(self): + for (key, profile) in self.profiles.items(): + # When ref count is 4 we can assume the profile is not referenced + # 1 = profiles dict + # 2 = self.profiles.items() + # 3 = profile above + # 4 = sys.getrefcount + if sys.getrefcount(profile) <= 4: + LOGGER.debug(f"closing profile with id {key}") + del self.profiles[key] + await profile.close() + + if len(self.profiles) <= self.capacity: + break + + def get(self, key: str) -> Optional[Profile]: + """Get profile with associated key from cache. + + Args: + key (str): the key to get the profile for. + + Returns: + Optional[Profile]: Profile if found in cache. + + """ + if key not in self.profiles: + return None + else: + self.profiles.move_to_end(key) + return self.profiles[key] + + def has(self, key: str) -> bool: + """Check whether there is a profile with associated key in the cache. + + Args: + key (str): the key to check for a profile + + Returns: + bool: Whether the key exists in the cache + + """ + return key in self.profiles + + async def put(self, key: str, value: Profile) -> None: + """Add profile with associated key to the cache. + + If new profile exceeds the cache capacity least recently used profiles + that are not used will be removed from the cache. + + Args: + key (str): the key to set + value (Profile): the profile to set + """ + self.profiles[key] = value + self.profiles.move_to_end(key) + LOGGER.debug(f"setting profile with id {key} in profile cache") + + if len(self.profiles) > self.capacity: + LOGGER.debug(f"profile limit of {self.capacity} reached. cleaning...") + await self._cleanup() + + def remove(self, key: str): + """Remove profile with associated key from the cache. + + Args: + key (str): The key to remove from the cache. + """ + del self.profiles[key] diff --git a/aries_cloudagent/multitenant/tests/test_cache.py b/aries_cloudagent/multitenant/tests/test_cache.py new file mode 100644 index 0000000000..9d2f1f8279 --- /dev/null +++ b/aries_cloudagent/multitenant/tests/test_cache.py @@ -0,0 +1,100 @@ +import sys +from asynctest import TestCase as AsyncTestCase +from asynctest import mock as async_mock + +from ..cache import ProfileCache + + +class TestProfileCache(AsyncTestCase): + async def setUp(self): + pass + + async def test_cache_cleanup_capacity_reached(self): + with async_mock.patch.object(ProfileCache, "_cleanup") as _cleanup: + cache = ProfileCache(1) + + await cache.put("1", async_mock.MagicMock()) + _cleanup.assert_not_called() + + await cache.put("2", async_mock.MagicMock()) + _cleanup.assert_called_once() + + async def test_get_not_in_cache(self): + cache = ProfileCache(1) + + assert cache.get("1") is None + + async def test_put_get_in_cache(self): + cache = ProfileCache(1) + + profile = async_mock.MagicMock() + await cache.put("1", profile) + + assert cache.get("1") is profile + + async def test_remove(self): + cache = ProfileCache(1) + + profile = async_mock.MagicMock() + await cache.put("1", profile) + + assert cache.get("1") is profile + + cache.remove("1") + + assert cache.get("1") is None + + async def test_has_true(self): + cache = ProfileCache(1) + + profile = async_mock.MagicMock() + + assert cache.has("1") is False + await cache.put("1", profile) + assert cache.has("1") is True + + async def test_cleanup(self): + cache = ProfileCache(1) + + with async_mock.patch.object(sys, "getrefcount") as getrefcount: + getrefcount.return_value = 4 + + profile1 = async_mock.MagicMock(close=async_mock.CoroutineMock()) + profile2 = async_mock.MagicMock(close=async_mock.CoroutineMock()) + + await cache.put("1", profile1) + + assert len(cache.profiles) == 1 + + await cache.put("2", profile2) + + assert len(cache.profiles) == 1 + assert cache.get("1") == None + profile1.close.assert_called_once() + + async def test_cleanup_reference(self): + cache = ProfileCache(3) + + with async_mock.patch.object(sys, "getrefcount") as getrefcount: + getrefcount.side_effect = [6, 4] + + profile1 = async_mock.MagicMock(close=async_mock.CoroutineMock()) + profile2 = async_mock.MagicMock(close=async_mock.CoroutineMock()) + profile3 = async_mock.MagicMock(close=async_mock.CoroutineMock()) + profile4 = async_mock.MagicMock(close=async_mock.CoroutineMock()) + + await cache.put("1", profile1) + await cache.put("2", profile2) + await cache.put("3", profile3) + + assert len(cache.profiles) == 3 + + await cache.put("4", profile4) + + assert len(cache.profiles) == 3 + assert cache.get("1") == profile1 + assert cache.get("2") == None + assert cache.get("3") == profile3 + assert cache.get("4") == profile4 + + profile2.close.assert_called_once() From 076935189fb7de603474a4b9426cd3f385b2b628 Mon Sep 17 00:00:00 2001 From: Daniel Bluhm Date: Wed, 23 Mar 2022 09:41:31 -0400 Subject: [PATCH 02/28] feat: add finalizers to profile impls Signed-off-by: Daniel Bluhm --- aries_cloudagent/askar/profile.py | 16 ++++- aries_cloudagent/core/profile.py | 13 ++++ aries_cloudagent/indy/sdk/profile.py | 17 ++++- aries_cloudagent/multitenant/cache.py | 24 ++----- .../multitenant/tests/test_cache.py | 64 +++++++------------ 5 files changed, 71 insertions(+), 63 deletions(-) diff --git a/aries_cloudagent/askar/profile.py b/aries_cloudagent/askar/profile.py index 4b72d20a57..b36d151fca 100644 --- a/aries_cloudagent/askar/profile.py +++ b/aries_cloudagent/askar/profile.py @@ -6,8 +6,8 @@ # import traceback -from typing import Any, Mapping -from weakref import ref +from typing import Any, Mapping, Optional +from weakref import finalize, ref from aries_askar import AskarError, Session, Store @@ -146,6 +146,18 @@ async def close(self): await self.opened.close() self.opened = None + def finalizer(self) -> Optional[finalize]: + """Return a finalizer for this profile. + + See docs for weakref.finalize for more details on behavior of finalizers. + """ + + def _finalize(opened: Optional[AskarOpenStore]): + if opened: + asyncio.get_event_loop().run_until_complete(opened.close()) + + return finalize(self, _finalize, self.opened) + class AskarProfileSession(ProfileSession): """An active connection to the profile management backend.""" diff --git a/aries_cloudagent/core/profile.py b/aries_cloudagent/core/profile.py index e5de864d6e..c8682aa222 100644 --- a/aries_cloudagent/core/profile.py +++ b/aries_cloudagent/core/profile.py @@ -4,6 +4,7 @@ from abc import ABC, abstractmethod from typing import Any, Mapping, Optional, Type +from weakref import finalize from .event_bus import EventBus, Event from ..config.base import InjectionError @@ -115,6 +116,18 @@ def inject_or( async def close(self): """Close the profile instance.""" + def finalizer(self) -> Optional[finalize]: + """Create and return a finalizer for the profile or None. + + None is returned if no special handling is required to close the profile. + + Finalizers enable automatic clean up of wallet profiles when all references to + the profile expire. + + See docs for weakref.finalize for more details on the behavior of finalizers. + """ + return None + async def remove(self): """Remove the profile.""" diff --git a/aries_cloudagent/indy/sdk/profile.py b/aries_cloudagent/indy/sdk/profile.py index 24badc748a..b5fb130b3d 100644 --- a/aries_cloudagent/indy/sdk/profile.py +++ b/aries_cloudagent/indy/sdk/profile.py @@ -1,9 +1,10 @@ """Manage Indy-SDK profile interaction.""" +import asyncio import logging -from typing import Any, Mapping -from weakref import ref +from typing import Any, Mapping, Optional +from weakref import finalize, ref from ...config.injection_context import InjectionContext from ...config.provider import ClassProvider @@ -116,6 +117,18 @@ async def close(self): await self.opened.close() self.opened = None + def finalizer(self) -> Optional[finalize]: + """Return a finalizer for this profile. + + See docs for weakref.finalize for more details on behavior of finalizers. + """ + + def _finalize(opened: Optional[IndyOpenWallet]): + if opened: + asyncio.get_event_loop().run_until_complete(opened.close()) + + return finalize(self, _finalize, self.opened) + async def remove(self): """Remove the profile associated with this instance.""" if not self.opened: diff --git a/aries_cloudagent/multitenant/cache.py b/aries_cloudagent/multitenant/cache.py index b228878160..b2f0141085 100644 --- a/aries_cloudagent/multitenant/cache.py +++ b/aries_cloudagent/multitenant/cache.py @@ -1,7 +1,6 @@ """Cache for multitenancy profiles.""" import logging -import sys from collections import OrderedDict from typing import Optional @@ -24,21 +23,6 @@ def __init__(self, capacity: int): self.profiles: OrderedDict[str, Profile] = OrderedDict() self.capacity = capacity - async def _cleanup(self): - for (key, profile) in self.profiles.items(): - # When ref count is 4 we can assume the profile is not referenced - # 1 = profiles dict - # 2 = self.profiles.items() - # 3 = profile above - # 4 = sys.getrefcount - if sys.getrefcount(profile) <= 4: - LOGGER.debug(f"closing profile with id {key}") - del self.profiles[key] - await profile.close() - - if len(self.profiles) <= self.capacity: - break - def get(self, key: str) -> Optional[Profile]: """Get profile with associated key from cache. @@ -77,13 +61,17 @@ async def put(self, key: str, value: Profile) -> None: key (str): the key to set value (Profile): the profile to set """ + value.finalizer() self.profiles[key] = value self.profiles.move_to_end(key) LOGGER.debug(f"setting profile with id {key} in profile cache") if len(self.profiles) > self.capacity: - LOGGER.debug(f"profile limit of {self.capacity} reached. cleaning...") - await self._cleanup() + LOGGER.debug( + f"Profile limit of {self.capacity} reached." + " Evicting least recently used profile..." + ) + self.profiles.popitem(last=False) def remove(self, key: str): """Remove profile with associated key from the cache. diff --git a/aries_cloudagent/multitenant/tests/test_cache.py b/aries_cloudagent/multitenant/tests/test_cache.py index 9d2f1f8279..8a805122c5 100644 --- a/aries_cloudagent/multitenant/tests/test_cache.py +++ b/aries_cloudagent/multitenant/tests/test_cache.py @@ -1,4 +1,3 @@ -import sys from asynctest import TestCase as AsyncTestCase from asynctest import mock as async_mock @@ -9,16 +8,6 @@ class TestProfileCache(AsyncTestCase): async def setUp(self): pass - async def test_cache_cleanup_capacity_reached(self): - with async_mock.patch.object(ProfileCache, "_cleanup") as _cleanup: - cache = ProfileCache(1) - - await cache.put("1", async_mock.MagicMock()) - _cleanup.assert_not_called() - - await cache.put("2", async_mock.MagicMock()) - _cleanup.assert_called_once() - async def test_get_not_in_cache(self): cache = ProfileCache(1) @@ -56,45 +45,38 @@ async def test_has_true(self): async def test_cleanup(self): cache = ProfileCache(1) - with async_mock.patch.object(sys, "getrefcount") as getrefcount: - getrefcount.return_value = 4 + profile1 = async_mock.MagicMock() + profile2 = async_mock.MagicMock() - profile1 = async_mock.MagicMock(close=async_mock.CoroutineMock()) - profile2 = async_mock.MagicMock(close=async_mock.CoroutineMock()) + await cache.put("1", profile1) - await cache.put("1", profile1) + assert len(cache.profiles) == 1 - assert len(cache.profiles) == 1 + await cache.put("2", profile2) - await cache.put("2", profile2) + assert len(cache.profiles) == 1 + assert cache.get("1") == None - assert len(cache.profiles) == 1 - assert cache.get("1") == None - profile1.close.assert_called_once() - - async def test_cleanup_reference(self): + async def test_cleanup_lru(self): cache = ProfileCache(3) - with async_mock.patch.object(sys, "getrefcount") as getrefcount: - getrefcount.side_effect = [6, 4] - - profile1 = async_mock.MagicMock(close=async_mock.CoroutineMock()) - profile2 = async_mock.MagicMock(close=async_mock.CoroutineMock()) - profile3 = async_mock.MagicMock(close=async_mock.CoroutineMock()) - profile4 = async_mock.MagicMock(close=async_mock.CoroutineMock()) + profile1 = async_mock.MagicMock() + profile2 = async_mock.MagicMock() + profile3 = async_mock.MagicMock() + profile4 = async_mock.MagicMock() - await cache.put("1", profile1) - await cache.put("2", profile2) - await cache.put("3", profile3) + await cache.put("1", profile1) + await cache.put("2", profile2) + await cache.put("3", profile3) - assert len(cache.profiles) == 3 + assert len(cache.profiles) == 3 - await cache.put("4", profile4) + cache.get("1") - assert len(cache.profiles) == 3 - assert cache.get("1") == profile1 - assert cache.get("2") == None - assert cache.get("3") == profile3 - assert cache.get("4") == profile4 + await cache.put("4", profile4) - profile2.close.assert_called_once() + assert len(cache.profiles) == 3 + assert cache.get("1") == profile1 + assert cache.get("2") == None + assert cache.get("3") == profile3 + assert cache.get("4") == profile4 From fd057e25a766383b9b22f4677cb27874f5ec0e13 Mon Sep 17 00:00:00 2001 From: Daniel Bluhm Date: Fri, 25 Mar 2022 11:42:57 -0400 Subject: [PATCH 03/28] feat: profile cache only on non askar-profile remove usage of self._instances in AskarProfileMultitenantManager add profile_id to AskarProfile init (to clearly differentiate between AskarProfiles where default profile is used and alternate profiles are used) base update_wallet only updates records; managers where profiles are held open are responsible for updating currently open profiles Signed-off-by: Daniel Bluhm --- aries_cloudagent/askar/profile.py | 23 ++++++--- .../multitenant/askar_profile_manager.py | 47 ++++++++++++++----- aries_cloudagent/multitenant/base.py | 25 ++++------ aries_cloudagent/multitenant/manager.py | 47 +++++++++++++++++-- 4 files changed, 102 insertions(+), 40 deletions(-) diff --git a/aries_cloudagent/askar/profile.py b/aries_cloudagent/askar/profile.py index b36d151fca..6277a936df 100644 --- a/aries_cloudagent/askar/profile.py +++ b/aries_cloudagent/askar/profile.py @@ -36,11 +36,18 @@ class AskarProfile(Profile): BACKEND_NAME = "askar" - def __init__(self, opened: AskarOpenStore, context: InjectionContext = None): + def __init__( + self, + opened: AskarOpenStore, + context: InjectionContext = None, + *, + profile_id: str = None + ): """Create a new AskarProfile instance.""" super().__init__(context=context, name=opened.name, created=opened.created) self.opened = opened self.ledger_pool: IndyVdrLedgerPool = None + self.profile_id = profile_id self.init_ledger_pool() self.bind_providers() @@ -56,8 +63,8 @@ def store(self) -> Store: async def remove(self): """Remove the profile.""" - if self.settings.get("multitenant.wallet_type") == "askar-profile": - await self.store.remove_profile(self.settings.get("wallet.askar_profile")) + if self.profile_id: + await self.store.remove_profile(self.profile_id) def init_ledger_pool(self): """Initialize the ledger pool.""" @@ -151,6 +158,11 @@ def finalizer(self) -> Optional[finalize]: See docs for weakref.finalize for more details on behavior of finalizers. """ + # Askar Profiles (not to be confused with AskarProfiles) don't need + # additional clean up + + if self.profile_id: + return None def _finalize(opened: Optional[AskarOpenStore]): if opened: @@ -172,11 +184,10 @@ def __init__( ): """Create a new IndySdkProfileSession instance.""" super().__init__(profile=profile, context=context, settings=settings) - profile_id = profile.context.settings.get("wallet.askar_profile") if is_txn: - self._opener = self.profile.store.transaction(profile_id) + self._opener = self.profile.store.transaction(profile.profile_id) else: - self._opener = self.profile.store.session(profile_id) + self._opener = self.profile.store.session(profile.profile_id) self._handle: Session = None self._acquire_start: float = None self._acquire_end: float = None diff --git a/aries_cloudagent/multitenant/askar_profile_manager.py b/aries_cloudagent/multitenant/askar_profile_manager.py index c692b04ea8..d7d13b964c 100644 --- a/aries_cloudagent/multitenant/askar_profile_manager.py +++ b/aries_cloudagent/multitenant/askar_profile_manager.py @@ -1,5 +1,6 @@ """Manager for askar profile multitenancy mode.""" +from typing import Iterable, Optional, cast from ..core.profile import ( Profile, ) @@ -15,13 +16,23 @@ class AskarProfileMultitenantManager(BaseMultitenantManager): DEFAULT_MULTIENANT_WALLET_NAME = "multitenant_sub_wallet" - def __init__(self, profile: Profile): + def __init__(self, profile: Profile, multitenant_profile: AskarProfile = None): """Initialize askar profile multitenant Manager. Args: profile: The base profile for this manager """ super().__init__(profile) + self._multitenant_profile: Optional[AskarProfile] = multitenant_profile + + @property + def open_profiles(self) -> Iterable[Profile]: + """Return iterator over open profiles. + + Only the core multitenant profile is considered open. + """ + if self._multitenant_profile: + yield self._multitenant_profile async def get_wallet_profile( self, @@ -33,6 +44,13 @@ async def get_wallet_profile( ) -> Profile: """Get Askar profile for a wallet record. + An object of type AskarProfile is returned but this should not be + confused with the underlying profile mechanism provided by Askar that + enables multiple "profiles" to share a wallet. Usage of this mechanism + is what causes this implementation of BaseMultitenantManager.get_wallet_profile + to look different from others, especially since no explicit clean up is + required for profiles that are no longer in use. + Args: base_context: Base context to extend from wallet_record: Wallet record to get the context for @@ -42,12 +60,10 @@ async def get_wallet_profile( Profile: Profile for the wallet record """ - multitenant_wallet_name = ( - base_context.settings.get("multitenant.wallet_name") - or self.DEFAULT_MULTIENANT_WALLET_NAME - ) - - if multitenant_wallet_name not in self._instances: + if not self._multitenant_profile: + multitenant_wallet_name = base_context.settings.get( + "multitenant.wallet_name", self.DEFAULT_MULTIENANT_WALLET_NAME + ) context = base_context.copy() sub_wallet_settings = { "wallet.recreate": False, @@ -65,13 +81,14 @@ async def get_wallet_profile( context.settings = context.settings.extend(sub_wallet_settings) profile, _ = await wallet_config(context, provision=False) - self._instances[multitenant_wallet_name] = profile + self._multitenant_profile = cast(AskarProfile, profile) - multitenant_wallet = self._instances[multitenant_wallet_name] - profile_context = multitenant_wallet.context.copy() + profile_context = self._multitenant_profile.context.copy() if provision: - await multitenant_wallet.store.create_profile(wallet_record.wallet_id) + await self._multitenant_profile.store.create_profile( + wallet_record.wallet_id + ) extra_settings = { "admin.webhook_urls": self.get_webhook_urls(base_context, wallet_record), @@ -82,7 +99,13 @@ async def get_wallet_profile( wallet_record.settings ).extend(extra_settings) - return AskarProfile(multitenant_wallet.opened, profile_context) + assert self._multitenant_profile.opened + + return AskarProfile( + self._multitenant_profile.opened, + profile_context, + profile_id=wallet_record.wallet_id, + ) async def remove_wallet_profile(self, profile: Profile): """Remove the wallet profile instance. diff --git a/aries_cloudagent/multitenant/base.py b/aries_cloudagent/multitenant/base.py index 5bdd9a9787..6788f325a5 100644 --- a/aries_cloudagent/multitenant/base.py +++ b/aries_cloudagent/multitenant/base.py @@ -1,10 +1,10 @@ """Manager for multitenancy.""" import logging -from abc import abstractmethod +from abc import abstractmethod, ABC, abstractproperty import jwt -from typing import List, Optional, cast +from typing import Iterable, List, Optional, cast from ..core.profile import ( Profile, @@ -34,7 +34,7 @@ class MultitenantManagerError(BaseError): """Generic multitenant error.""" -class BaseMultitenantManager: +class BaseMultitenantManager(ABC): """Base class for handling multitenancy.""" def __init__(self, profile: Profile): @@ -47,7 +47,10 @@ def __init__(self, profile: Profile): if not profile: raise MultitenantManagerError("Missing profile") - self._instances: dict[str, Profile] = {} + @abstractproperty + def open_profiles(self) -> Iterable[Profile]: + """Return iterator over open profiles.""" + ... async def get_default_mediator(self) -> Optional[MediationRecord]: """Retrieve the default mediator used for subwallet routing. @@ -211,7 +214,7 @@ async def update_wallet( wallet_id: str, new_settings: dict, ) -> WalletRecord: - """Update a existing wallet and wallet record. + """Update an existing wallet record. Args: wallet_id: The wallet id of the wallet record @@ -227,18 +230,6 @@ async def update_wallet( wallet_record.update_settings(new_settings) await wallet_record.save(session) - # update profile only if loaded - if wallet_id in self._instances: - profile = self._instances[wallet_id] - profile.settings.update(wallet_record.settings) - - extra_settings = { - "admin.webhook_urls": self.get_webhook_urls( - self._profile.context, wallet_record - ), - } - profile.settings.update(extra_settings) - return wallet_record async def remove_wallet(self, wallet_id: str, wallet_key: str = None): diff --git a/aries_cloudagent/multitenant/manager.py b/aries_cloudagent/multitenant/manager.py index 5bbbcc6632..e3e4418186 100644 --- a/aries_cloudagent/multitenant/manager.py +++ b/aries_cloudagent/multitenant/manager.py @@ -1,5 +1,6 @@ """Manager for multitenancy.""" +from typing import Iterable from ..core.profile import ( Profile, ) @@ -8,6 +9,8 @@ from ..wallet.models.wallet_record import WalletRecord from ..multitenant.base import BaseMultitenantManager +from .cache import ProfileCache + class MultitenantManager(BaseMultitenantManager): """Class for handling multitenancy.""" @@ -19,6 +22,11 @@ def __init__(self, profile: Profile): profile: The profile for this manager """ super().__init__(profile) + self._profiles = ProfileCache(100) + + def open_profiles(self) -> Iterable[Profile]: + """Return iterator over open profiles.""" + yield from self._profiles.profiles.values() async def get_wallet_profile( self, @@ -40,7 +48,8 @@ async def get_wallet_profile( """ wallet_id = wallet_record.wallet_id - if wallet_id not in self._instances: + profile = self._profiles.get(wallet_id) + if not profile: # Extend base context context = base_context.copy() @@ -68,9 +77,37 @@ async def get_wallet_profile( # MTODO: add ledger config profile, _ = await wallet_config(context, provision=provision) - self._instances[wallet_id] = profile + self._profiles.put(wallet_id, profile) + + return profile + + async def update_wallet(self, wallet_id: str, new_settings: dict) -> WalletRecord: + """Update an existing wallet and wallet record. + + Args: + wallet_id: The wallet id of the wallet record + new_settings: The context settings to be updated for this wallet + + Returns: + WalletRecord: The updated wallet record + + """ + wallet_record = await super().update_wallet(wallet_id, new_settings) + + # Wallet record has been updated but profile settings in memory must + # also be refreshed; update profile only if loaded + profile = self._profiles.get(wallet_id) + if profile: + profile.settings.update(wallet_record.settings) + + extra_settings = { + "admin.webhook_urls": self.get_webhook_urls( + self._profile.context, wallet_record + ), + } + profile.settings.update(extra_settings) - return self._instances[wallet_id] + return wallet_record async def remove_wallet_profile(self, profile: Profile): """Remove the wallet profile instance. @@ -79,6 +116,6 @@ async def remove_wallet_profile(self, profile: Profile): profile: The wallet profile instance """ - wallet_id = profile.settings.get("wallet.id") - del self._instances[wallet_id] + wallet_id = profile.settings.get_str("wallet.id") + self._profiles.remove(wallet_id) await profile.remove() From fdac67cc53923b4d3ce7840e91f701b7fbd5c7a6 Mon Sep 17 00:00:00 2001 From: Daniel Bluhm Date: Fri, 25 Mar 2022 11:58:23 -0400 Subject: [PATCH 04/28] refactor: conductor iterates through open_profiles Signed-off-by: Daniel Bluhm --- aries_cloudagent/core/conductor.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/aries_cloudagent/core/conductor.py b/aries_cloudagent/core/conductor.py index 6ed174545f..1af7f32a76 100644 --- a/aries_cloudagent/core/conductor.py +++ b/aries_cloudagent/core/conductor.py @@ -495,7 +495,7 @@ async def stop(self, timeout=1.0): # close multitenant profiles multitenant_mgr = self.context.inject_or(BaseMultitenantManager) if multitenant_mgr: - for profile in multitenant_mgr._instances.values(): + for profile in multitenant_mgr.open_profiles: shutdown.run(profile.close()) if self.root_profile: From 22e05f6144e9315bf2d1c3238ea5e0570a1abbe3 Mon Sep 17 00:00:00 2001 From: Daniel Bluhm Date: Fri, 25 Mar 2022 11:59:55 -0400 Subject: [PATCH 05/28] fix: multitenant manager tests Signed-off-by: Daniel Bluhm --- .../admin/tests/test_admin_server.py | 3 +- aries_cloudagent/core/tests/test_conductor.py | 16 ++- aries_cloudagent/multitenant/cache.py | 2 +- .../tests/test_askar_profile_manager.py | 126 ++++++++---------- .../multitenant/tests/test_base.py | 53 +++++--- .../multitenant/tests/test_cache.py | 102 +++++++------- .../multitenant/tests/test_manager.py | 45 ++++++- 7 files changed, 188 insertions(+), 159 deletions(-) diff --git a/aries_cloudagent/admin/tests/test_admin_server.py b/aries_cloudagent/admin/tests/test_admin_server.py index 7707315e12..8f5429dead 100644 --- a/aries_cloudagent/admin/tests/test_admin_server.py +++ b/aries_cloudagent/admin/tests/test_admin_server.py @@ -198,10 +198,9 @@ async def test_import_routes_multitenant_middleware(self): context = InjectionContext() context.injector.bind_instance(ProtocolRegistry, ProtocolRegistry()) context.injector.bind_instance(GoalCodeRegistry, GoalCodeRegistry()) - profile = InMemoryProfile.test_profile() context.injector.bind_instance( test_module.BaseMultitenantManager, - test_module.BaseMultitenantManager(profile), + async_mock.MagicMock(spec=test_module.BaseMultitenantManager), ) await DefaultContextBuilder().load_plugins(context) server = self.get_admin_server( diff --git a/aries_cloudagent/core/tests/test_conductor.py b/aries_cloudagent/core/tests/test_conductor.py index d3f17d499b..7eb87b12c9 100644 --- a/aries_cloudagent/core/tests/test_conductor.py +++ b/aries_cloudagent/core/tests/test_conductor.py @@ -918,15 +918,19 @@ async def test_shutdown_multitenant_profiles(self): await conductor.setup() multitenant_mgr = conductor.context.inject(BaseMultitenantManager) - multitenant_mgr._instances = { - "test1": async_mock.MagicMock(close=async_mock.CoroutineMock()), - "test2": async_mock.MagicMock(close=async_mock.CoroutineMock()), - } + multitenant_mgr._profiles.put( + "test1", + async_mock.MagicMock(close=async_mock.CoroutineMock()), + ) + multitenant_mgr._profiles.put( + "test2", + async_mock.MagicMock(close=async_mock.CoroutineMock()), + ) await conductor.stop() - multitenant_mgr._instances["test1"].close.assert_called_once_with() - multitenant_mgr._instances["test2"].close.assert_called_once_with() + multitenant_mgr._profiles.profiles["test1"].close.assert_called_once_with() + multitenant_mgr._profiles.profiles["test2"].close.assert_called_once_with() def get_invite_store_mock( diff --git a/aries_cloudagent/multitenant/cache.py b/aries_cloudagent/multitenant/cache.py index b2f0141085..edaed40b8b 100644 --- a/aries_cloudagent/multitenant/cache.py +++ b/aries_cloudagent/multitenant/cache.py @@ -51,7 +51,7 @@ def has(self, key: str) -> bool: """ return key in self.profiles - async def put(self, key: str, value: Profile) -> None: + def put(self, key: str, value: Profile) -> None: """Add profile with associated key to the cache. If new profile exceeds the cache capacity least recently used profiles diff --git a/aries_cloudagent/multitenant/tests/test_askar_profile_manager.py b/aries_cloudagent/multitenant/tests/test_askar_profile_manager.py index 6bad949cb4..b93930f2b0 100644 --- a/aries_cloudagent/multitenant/tests/test_askar_profile_manager.py +++ b/aries_cloudagent/multitenant/tests/test_askar_profile_manager.py @@ -43,79 +43,61 @@ async def test_get_wallet_profile_should_open_store_and_return_profile_with_wall with async_mock.patch( "aries_cloudagent.multitenant.askar_profile_manager.wallet_config" - ) as wallet_config: - with async_mock.patch( - "aries_cloudagent.multitenant.askar_profile_manager.AskarProfile" - ) as AskarProfile: - sub_wallet_profile_context = InjectionContext() - sub_wallet_profile = AskarProfile(None, None) - sub_wallet_profile.context.copy.return_value = ( - sub_wallet_profile_context - ) + ) as wallet_config, async_mock.patch( + "aries_cloudagent.multitenant.askar_profile_manager.AskarProfile", + ) as AskarProfile: + sub_wallet_profile_context = InjectionContext() + sub_wallet_profile = AskarProfile(None, None) + sub_wallet_profile.context.copy.return_value = sub_wallet_profile_context - def side_effect(context, provision): - sub_wallet_profile.name = askar_profile_mock_name - return sub_wallet_profile, None + def side_effect(context, provision): + sub_wallet_profile.name = askar_profile_mock_name + return sub_wallet_profile, None - wallet_config.side_effect = side_effect + wallet_config.side_effect = side_effect - profile = await self.manager.get_wallet_profile( - self.profile.context, wallet_record - ) + profile = await self.manager.get_wallet_profile( + self.profile.context, wallet_record + ) - assert profile.name == askar_profile_mock_name - wallet_config.assert_called_once() - wallet_config_settings_argument = wallet_config.call_args[0][0].settings - assert ( - wallet_config_settings_argument.get("wallet.name") - == self.DEFAULT_MULTIENANT_WALLET_NAME - ) - assert wallet_config_settings_argument.get("wallet.id") == None - assert wallet_config_settings_argument.get("auto_provision") == True - assert wallet_config_settings_argument.get("wallet.type") == "askar" - AskarProfile.assert_called_with( - sub_wallet_profile.opened, sub_wallet_profile_context - ) - assert ( - sub_wallet_profile_context.settings.get("wallet.seed") - == "test_seed" - ) - assert ( - sub_wallet_profile_context.settings.get("wallet.rekey") - == "test_rekey" - ) - assert ( - sub_wallet_profile_context.settings.get("wallet.name") - == "test_name" - ) - assert ( - sub_wallet_profile_context.settings.get("wallet.type") - == "test_type" - ) - assert sub_wallet_profile_context.settings.get("mediation.open") == True - assert ( - sub_wallet_profile_context.settings.get("mediation.invite") - == "http://invite.com" - ) - assert ( - sub_wallet_profile_context.settings.get("mediation.default_id") - == "24a96ef5" - ) - assert ( - sub_wallet_profile_context.settings.get("mediation.clear") == True - ) - assert ( - sub_wallet_profile_context.settings.get("wallet.id") - == wallet_record.wallet_id - ) - assert ( - sub_wallet_profile_context.settings.get("wallet.name") - == "test_name" - ) - assert ( - sub_wallet_profile_context.settings.get("wallet.askar_profile") - == wallet_record.wallet_id - ) + assert profile.name == askar_profile_mock_name + wallet_config.assert_called_once() + wallet_config_settings_argument = wallet_config.call_args[0][0].settings + assert ( + wallet_config_settings_argument.get("wallet.name") + == self.DEFAULT_MULTIENANT_WALLET_NAME + ) + assert wallet_config_settings_argument.get("wallet.id") == None + assert wallet_config_settings_argument.get("auto_provision") == True + assert wallet_config_settings_argument.get("wallet.type") == "askar" + AskarProfile.assert_called_with( + sub_wallet_profile.opened, sub_wallet_profile_context, profile_id="test" + ) + assert sub_wallet_profile_context.settings.get("wallet.seed") == "test_seed" + assert ( + sub_wallet_profile_context.settings.get("wallet.rekey") == "test_rekey" + ) + assert sub_wallet_profile_context.settings.get("wallet.name") == "test_name" + assert sub_wallet_profile_context.settings.get("wallet.type") == "test_type" + assert sub_wallet_profile_context.settings.get("mediation.open") == True + assert ( + sub_wallet_profile_context.settings.get("mediation.invite") + == "http://invite.com" + ) + assert ( + sub_wallet_profile_context.settings.get("mediation.default_id") + == "24a96ef5" + ) + assert sub_wallet_profile_context.settings.get("mediation.clear") == True + assert ( + sub_wallet_profile_context.settings.get("wallet.id") + == wallet_record.wallet_id + ) + assert sub_wallet_profile_context.settings.get("wallet.name") == "test_name" + assert ( + sub_wallet_profile_context.settings.get("wallet.askar_profile") + == wallet_record.wallet_id + ) async def test_get_wallet_profile_should_create_profile(self): wallet_record = WalletRecord(wallet_id="test", settings={}) @@ -128,9 +110,7 @@ async def test_get_wallet_profile_should_create_profile(self): sub_wallet_profile = AskarProfile(None, None) sub_wallet_profile.context.copy.return_value = InjectionContext() sub_wallet_profile.store.create_profile.return_value = create_profile_stub - self.manager._instances[ - self.DEFAULT_MULTIENANT_WALLET_NAME - ] = sub_wallet_profile + self.manager._multitenant_profile = sub_wallet_profile await self.manager.get_wallet_profile( self.profile.context, wallet_record, provision=True @@ -172,7 +152,7 @@ def side_effect(context, provision): ) async def test_remove_wallet_profile(self): - test_profile = InMemoryProfile.test_profile() + test_profile = InMemoryProfile.test_profile({"wallet.id": "test"}) with async_mock.patch.object(InMemoryProfile, "remove") as profile_remove: await self.manager.remove_wallet_profile(test_profile) diff --git a/aries_cloudagent/multitenant/tests/test_base.py b/aries_cloudagent/multitenant/tests/test_base.py index 4f6cb8dabf..b37392c014 100644 --- a/aries_cloudagent/multitenant/tests/test_base.py +++ b/aries_cloudagent/multitenant/tests/test_base.py @@ -23,6 +23,25 @@ from ..error import WalletKeyMissingError +class MockMultitenantManager(BaseMultitenantManager): + async def get_wallet_profile( + self, + base_context, + wallet_record: WalletRecord, + extra_settings: dict = ..., + *, + provision=False + ): + """Do nothing.""" + + async def remove_wallet_profile(self, profile): + """Do nothing.""" + + @property + def open_profiles(self): + """Do nothing.""" + + class TestBaseMultitenantManager(AsyncTestCase): async def setUp(self): self.profile = InMemoryProfile.test_profile() @@ -31,11 +50,11 @@ async def setUp(self): self.responder = async_mock.CoroutineMock(send=async_mock.CoroutineMock()) self.context.injector.bind_instance(BaseResponder, self.responder) - self.manager = BaseMultitenantManager(self.profile) + self.manager = MockMultitenantManager(self.profile) async def test_init_throws_no_profile(self): with self.assertRaises(MultitenantManagerError): - BaseMultitenantManager(None) + MockMultitenantManager(None) async def test_get_default_mediator(self): with async_mock.patch.object( @@ -158,7 +177,7 @@ async def test_get_wallet_by_key(self): async def test_create_wallet_removes_key_only_unmanaged_mode(self): with async_mock.patch.object( - BaseMultitenantManager, "get_wallet_profile" + self.manager, "get_wallet_profile" ) as get_wallet_profile: get_wallet_profile.return_value = InMemoryProfile.test_profile() @@ -174,7 +193,7 @@ async def test_create_wallet_removes_key_only_unmanaged_mode(self): async def test_create_wallet_fails_if_wallet_name_exists(self): with async_mock.patch.object( - BaseMultitenantManager, "_wallet_name_exists" + self.manager, "_wallet_name_exists" ) as _wallet_name_exists: _wallet_name_exists.return_value = True @@ -191,9 +210,9 @@ async def test_create_wallet_saves_wallet_record_creates_profile(self): with async_mock.patch.object( WalletRecord, "save" ) as wallet_record_save, async_mock.patch.object( - BaseMultitenantManager, "get_wallet_profile" + self.manager, "get_wallet_profile" ) as get_wallet_profile, async_mock.patch.object( - BaseMultitenantManager, "add_key" + self.manager, "add_key" ) as add_key: get_wallet_profile.return_value = InMemoryProfile.test_profile() @@ -227,9 +246,9 @@ async def test_create_wallet_adds_wallet_route(self): with async_mock.patch.object( WalletRecord, "save" ) as wallet_record_save, async_mock.patch.object( - BaseMultitenantManager, "get_wallet_profile" + self.manager, "get_wallet_profile" ) as get_wallet_profile, async_mock.patch.object( - BaseMultitenantManager, "add_key" + self.manager, "add_key" ) as add_key, async_mock.patch.object( InMemoryWallet, "get_public_did" ) as get_public_did: @@ -257,15 +276,13 @@ async def test_create_wallet_adds_wallet_route(self): assert wallet_record.key_management_mode == WalletRecord.MODE_MANAGED assert wallet_record.wallet_key == "test_key" - async def test_update_wallet_update_wallet_profile(self): + async def test_update_wallet(self): with async_mock.patch.object( WalletRecord, "retrieve_by_id" ) as retrieve_by_id, async_mock.patch.object( WalletRecord, "save" ) as wallet_record_save: wallet_id = "test-wallet-id" - wallet_profile = InMemoryProfile.test_profile() - self.manager._instances["test-wallet-id"] = wallet_profile retrieve_by_id.return_value = WalletRecord( wallet_id=wallet_id, settings={ @@ -285,10 +302,6 @@ async def test_update_wallet_update_wallet_profile(self): assert isinstance(wallet_record, WalletRecord) assert wallet_record.wallet_webhook_urls == ["new-webhook-url"] assert wallet_record.wallet_dispatch_type == "default" - assert wallet_profile.settings.get("wallet.webhook_urls") == [ - "new-webhook-url" - ] - assert wallet_profile.settings.get("wallet.dispatch_type") == "default" async def test_remove_wallet_fails_no_wallet_key_but_required(self): with async_mock.patch.object(WalletRecord, "retrieve_by_id") as retrieve_by_id: @@ -305,9 +318,9 @@ async def test_remove_wallet_removes_profile_wallet_storage_records(self): with async_mock.patch.object( WalletRecord, "retrieve_by_id" ) as retrieve_by_id, async_mock.patch.object( - BaseMultitenantManager, "get_wallet_profile" + self.manager, "get_wallet_profile" ) as get_wallet_profile, async_mock.patch.object( - BaseMultitenantManager, "remove_wallet_profile" + self.manager, "remove_wallet_profile" ) as remove_wallet_profile, async_mock.patch.object( WalletRecord, "delete_record" ) as wallet_delete_record, async_mock.patch.object( @@ -483,7 +496,7 @@ async def test_get_profile_for_token_managed_wallet(self): ).decode() with async_mock.patch.object( - BaseMultitenantManager, "get_wallet_profile" + self.manager, "get_wallet_profile" ) as get_wallet_profile: mock_profile = InMemoryProfile.test_profile() get_wallet_profile.return_value = mock_profile @@ -517,7 +530,7 @@ async def test_get_profile_for_token_unmanaged_wallet(self): ).decode() with async_mock.patch.object( - BaseMultitenantManager, "get_wallet_profile" + self.manager, "get_wallet_profile" ) as get_wallet_profile: mock_profile = InMemoryProfile.test_profile() get_wallet_profile.return_value = mock_profile @@ -557,7 +570,7 @@ async def test_get_wallets_by_message(self): ] with async_mock.patch.object( - BaseMultitenantManager, "_get_wallet_by_key" + self.manager, "_get_wallet_by_key" ) as get_wallet_by_key: get_wallet_by_key.side_effect = return_wallets diff --git a/aries_cloudagent/multitenant/tests/test_cache.py b/aries_cloudagent/multitenant/tests/test_cache.py index 8a805122c5..451e465051 100644 --- a/aries_cloudagent/multitenant/tests/test_cache.py +++ b/aries_cloudagent/multitenant/tests/test_cache.py @@ -1,82 +1,82 @@ -from asynctest import TestCase as AsyncTestCase from asynctest import mock as async_mock from ..cache import ProfileCache -class TestProfileCache(AsyncTestCase): - async def setUp(self): - pass +def test_get_not_in_cache(): + cache = ProfileCache(1) - async def test_get_not_in_cache(self): - cache = ProfileCache(1) + assert cache.get("1") is None - assert cache.get("1") is None - async def test_put_get_in_cache(self): - cache = ProfileCache(1) +def test_put_get_in_cache(): + cache = ProfileCache(1) - profile = async_mock.MagicMock() - await cache.put("1", profile) + profile = async_mock.MagicMock() + cache.put("1", profile) - assert cache.get("1") is profile + assert cache.get("1") is profile - async def test_remove(self): - cache = ProfileCache(1) - profile = async_mock.MagicMock() - await cache.put("1", profile) +def test_remove(): + cache = ProfileCache(1) - assert cache.get("1") is profile + profile = async_mock.MagicMock() + cache.put("1", profile) - cache.remove("1") + assert cache.get("1") is profile - assert cache.get("1") is None + cache.remove("1") - async def test_has_true(self): - cache = ProfileCache(1) + assert cache.get("1") is None - profile = async_mock.MagicMock() - assert cache.has("1") is False - await cache.put("1", profile) - assert cache.has("1") is True +def test_has_true(): + cache = ProfileCache(1) - async def test_cleanup(self): - cache = ProfileCache(1) + profile = async_mock.MagicMock() - profile1 = async_mock.MagicMock() - profile2 = async_mock.MagicMock() + assert cache.has("1") is False + cache.put("1", profile) + assert cache.has("1") is True - await cache.put("1", profile1) - assert len(cache.profiles) == 1 +def test_cleanup(): + cache = ProfileCache(1) - await cache.put("2", profile2) + profile1 = async_mock.MagicMock() + profile2 = async_mock.MagicMock() - assert len(cache.profiles) == 1 - assert cache.get("1") == None + cache.put("1", profile1) - async def test_cleanup_lru(self): - cache = ProfileCache(3) + assert len(cache.profiles) == 1 - profile1 = async_mock.MagicMock() - profile2 = async_mock.MagicMock() - profile3 = async_mock.MagicMock() - profile4 = async_mock.MagicMock() + cache.put("2", profile2) - await cache.put("1", profile1) - await cache.put("2", profile2) - await cache.put("3", profile3) + assert len(cache.profiles) == 1 + assert cache.get("1") == None - assert len(cache.profiles) == 3 - cache.get("1") +def test_cleanup_lru(): + cache = ProfileCache(3) - await cache.put("4", profile4) + profile1 = async_mock.MagicMock() + profile2 = async_mock.MagicMock() + profile3 = async_mock.MagicMock() + profile4 = async_mock.MagicMock() - assert len(cache.profiles) == 3 - assert cache.get("1") == profile1 - assert cache.get("2") == None - assert cache.get("3") == profile3 - assert cache.get("4") == profile4 + cache.put("1", profile1) + cache.put("2", profile2) + cache.put("3", profile3) + + assert len(cache.profiles) == 3 + + cache.get("1") + + cache.put("4", profile4) + + assert len(cache.profiles) == 3 + assert cache.get("1") == profile1 + assert cache.get("2") == None + assert cache.get("3") == profile3 + assert cache.get("4") == profile4 diff --git a/aries_cloudagent/multitenant/tests/test_manager.py b/aries_cloudagent/multitenant/tests/test_manager.py index 7e01959f95..c851369c11 100644 --- a/aries_cloudagent/multitenant/tests/test_manager.py +++ b/aries_cloudagent/multitenant/tests/test_manager.py @@ -19,7 +19,7 @@ async def setUp(self): async def test_get_wallet_profile_returns_from_cache(self): wallet_record = WalletRecord(wallet_id="test") - self.manager._instances["test"] = InMemoryProfile.test_profile() + self.manager._profiles.put("test", InMemoryProfile.test_profile()) with async_mock.patch( "aries_cloudagent.config.wallet.wallet_config" @@ -27,12 +27,12 @@ async def test_get_wallet_profile_returns_from_cache(self): profile = await self.manager.get_wallet_profile( self.profile.context, wallet_record ) - assert profile is self.manager._instances["test"] + assert profile is self.manager._profiles.get("test") wallet_config.assert_not_called() async def test_get_wallet_profile_not_in_cache(self): wallet_record = WalletRecord(wallet_id="test", settings={}) - self.manager._instances["test"] = InMemoryProfile.test_profile() + self.manager._profiles.put("test", InMemoryProfile.test_profile()) self.profile.context.update_settings( {"admin.webhook_urls": ["http://localhost:8020"]} ) @@ -43,7 +43,7 @@ async def test_get_wallet_profile_not_in_cache(self): profile = await self.manager.get_wallet_profile( self.profile.context, wallet_record ) - assert profile is self.manager._instances["test"] + assert profile is self.manager._profiles.get("test") wallet_config.assert_not_called() async def test_get_wallet_profile_settings(self): @@ -174,13 +174,46 @@ def side_effect(context, provision): assert profile.settings.get("mediation.default_id") == "24a96ef5" assert profile.settings.get("mediation.clear") == True + async def test_update_wallet_update_wallet_profile(self): + with async_mock.patch.object( + WalletRecord, "retrieve_by_id" + ) as retrieve_by_id, async_mock.patch.object( + WalletRecord, "save" + ) as wallet_record_save: + wallet_id = "test-wallet-id" + wallet_profile = InMemoryProfile.test_profile() + self.manager._profiles.put("test-wallet-id", wallet_profile) + retrieve_by_id.return_value = WalletRecord( + wallet_id=wallet_id, + settings={ + "wallet.webhook_urls": ["test-webhook-url"], + "wallet.dispatch_type": "both", + }, + ) + + new_settings = { + "wallet.webhook_urls": ["new-webhook-url"], + "wallet.dispatch_type": "default", + } + wallet_record = await self.manager.update_wallet(wallet_id, new_settings) + + wallet_record_save.assert_called_once() + + assert isinstance(wallet_record, WalletRecord) + assert wallet_record.wallet_webhook_urls == ["new-webhook-url"] + assert wallet_record.wallet_dispatch_type == "default" + assert wallet_profile.settings.get("wallet.webhook_urls") == [ + "new-webhook-url" + ] + assert wallet_profile.settings.get("wallet.dispatch_type") == "default" + async def test_remove_wallet_profile(self): test_profile = InMemoryProfile.test_profile( settings={"wallet.id": "test"}, ) - self.manager._instances["test"] = test_profile + self.manager._profiles.put("test", test_profile) with async_mock.patch.object(InMemoryProfile, "remove") as profile_remove: await self.manager.remove_wallet_profile(test_profile) - assert "test" not in self.manager._instances + assert not self.manager._profiles.has("test") profile_remove.assert_called_once_with() From ac585c98eca0bcd4330e1ef163eb4aa84192a591 Mon Sep 17 00:00:00 2001 From: Daniel Bluhm Date: Fri, 25 Mar 2022 12:09:26 -0400 Subject: [PATCH 06/28] fix: askar profile tests Signed-off-by: Daniel Bluhm --- aries_cloudagent/askar/tests/test_profile.py | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/aries_cloudagent/askar/tests/test_profile.py b/aries_cloudagent/askar/tests/test_profile.py index f01da0d2fe..f943ca6a76 100644 --- a/aries_cloudagent/askar/tests/test_profile.py +++ b/aries_cloudagent/askar/tests/test_profile.py @@ -28,7 +28,7 @@ async def test_remove_success(self, AskarOpenStore): "wallet.askar_profile": profile_id, "ledger.genesis_transactions": mock.MagicMock(), } - askar_profile = AskarProfile(openStore, context) + askar_profile = AskarProfile(openStore, context, profile_id=profile_id) remove_profile_stub = asyncio.Future() remove_profile_stub.set_result(True) openStore.store.remove_profile.return_value = remove_profile_stub @@ -63,9 +63,6 @@ async def test_profile_manager_transaction(self): transactionProfile = test_module.AskarProfileSession(askar_profile, True) assert transactionProfile._opener == askar_profile_transaction - askar_profile.context.settings.get.assert_called_once_with( - "wallet.askar_profile" - ) askar_profile.store.transaction.assert_called_once_with(profile) @pytest.mark.asyncio @@ -81,7 +78,4 @@ async def test_profile_manager_store(self): sessionProfile = test_module.AskarProfileSession(askar_profile, False) assert sessionProfile._opener == askar_profile_session - askar_profile.context.settings.get.assert_called_once_with( - "wallet.askar_profile" - ) askar_profile.store.session.assert_called_once_with(profile) From 6f613ea9148f7980008dd2d6ad9c2664764b71b3 Mon Sep 17 00:00:00 2001 From: Daniel Bluhm Date: Fri, 25 Mar 2022 12:09:43 -0400 Subject: [PATCH 07/28] fix: open_profiles should be property Signed-off-by: Daniel Bluhm --- aries_cloudagent/core/tests/test_conductor.py | 2 ++ aries_cloudagent/multitenant/manager.py | 1 + 2 files changed, 3 insertions(+) diff --git a/aries_cloudagent/core/tests/test_conductor.py b/aries_cloudagent/core/tests/test_conductor.py index 7eb87b12c9..a35744efdd 100644 --- a/aries_cloudagent/core/tests/test_conductor.py +++ b/aries_cloudagent/core/tests/test_conductor.py @@ -27,6 +27,7 @@ ) from ...resolver.did_resolver import DIDResolver, DIDResolverRegistry from ...multitenant.base import BaseMultitenantManager +from ...multitenant.manager import MultitenantManager from ...storage.base import BaseStorage from ...storage.error import StorageNotFoundError from ...transport.inbound.message import InboundMessage @@ -917,6 +918,7 @@ async def test_shutdown_multitenant_profiles(self): await conductor.setup() multitenant_mgr = conductor.context.inject(BaseMultitenantManager) + assert isinstance(multitenant_mgr, MultitenantManager) multitenant_mgr._profiles.put( "test1", diff --git a/aries_cloudagent/multitenant/manager.py b/aries_cloudagent/multitenant/manager.py index e3e4418186..210d799a06 100644 --- a/aries_cloudagent/multitenant/manager.py +++ b/aries_cloudagent/multitenant/manager.py @@ -24,6 +24,7 @@ def __init__(self, profile: Profile): super().__init__(profile) self._profiles = ProfileCache(100) + @property def open_profiles(self) -> Iterable[Profile]: """Return iterator over open profiles.""" yield from self._profiles.profiles.values() From ce7ccf17da91f2567a1e49729960d3a09417a11a Mon Sep 17 00:00:00 2001 From: Daniel Bluhm Date: Fri, 25 Mar 2022 12:14:13 -0400 Subject: [PATCH 08/28] fix: askar profile tests, profile passed to sesh, txn Signed-off-by: Daniel Bluhm --- aries_cloudagent/askar/tests/test_profile.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/aries_cloudagent/askar/tests/test_profile.py b/aries_cloudagent/askar/tests/test_profile.py index f943ca6a76..4c4c646a84 100644 --- a/aries_cloudagent/askar/tests/test_profile.py +++ b/aries_cloudagent/askar/tests/test_profile.py @@ -55,10 +55,10 @@ async def test_profile_manager_transaction(self): profile = "profileId" with mock.patch("aries_cloudagent.askar.profile.AskarProfile") as AskarProfile: - askar_profile = AskarProfile(None, True) + askar_profile = AskarProfile(None, True, profile_id=profile) + askar_profile.profile_id = profile askar_profile_transaction = mock.MagicMock() askar_profile.store.transaction.return_value = askar_profile_transaction - askar_profile.context.settings.get.return_value = profile transactionProfile = test_module.AskarProfileSession(askar_profile, True) @@ -70,10 +70,10 @@ async def test_profile_manager_store(self): profile = "profileId" with mock.patch("aries_cloudagent.askar.profile.AskarProfile") as AskarProfile: - askar_profile = AskarProfile(None, False) + askar_profile = AskarProfile(None, False, profile_id=profile) + askar_profile.profile_id = profile askar_profile_session = mock.MagicMock() askar_profile.store.session.return_value = askar_profile_session - askar_profile.context.settings.get.return_value = profile sessionProfile = test_module.AskarProfileSession(askar_profile, False) From a2e306e9e41e10e2d95422da5aeec043a25f9ece Mon Sep 17 00:00:00 2001 From: Daniel Bluhm Date: Mon, 28 Mar 2022 20:38:19 -0400 Subject: [PATCH 09/28] feat: profile cache track open profiles via weakref Signed-off-by: Daniel Bluhm --- aries_cloudagent/multitenant/cache.py | 40 ++++++++++++++++++++------- 1 file changed, 30 insertions(+), 10 deletions(-) diff --git a/aries_cloudagent/multitenant/cache.py b/aries_cloudagent/multitenant/cache.py index edaed40b8b..8a00120ca8 100644 --- a/aries_cloudagent/multitenant/cache.py +++ b/aries_cloudagent/multitenant/cache.py @@ -3,6 +3,7 @@ import logging from collections import OrderedDict from typing import Optional +from weakref import WeakValueDictionary from ..core.profile import Profile @@ -21,11 +22,27 @@ def __init__(self, capacity: int): """ self.profiles: OrderedDict[str, Profile] = OrderedDict() + self._open_profiles: WeakValueDictionary[str, Profile] = WeakValueDictionary() self.capacity = capacity + def _cleanup(self): + """Prune cache until size matches defined capacity.""" + if len(self.profiles) > self.capacity: + LOGGER.debug( + f"Profile limit of {self.capacity} reached." + " Evicting least recently used profiles..." + ) + while len(self.profiles) > self.capacity: + key, _ = self.profiles.popitem(last=False) + LOGGER.debug(f"Evicted profile with key {key}") + def get(self, key: str) -> Optional[Profile]: """Get profile with associated key from cache. + If a profile is open but has been evicted from the cache, this will + reinsert the profile back into the cache. This prevents attempting to + open a profile that is already open. Triggers clean up. + Args: key (str): the key to get the profile for. @@ -33,11 +50,19 @@ def get(self, key: str) -> Optional[Profile]: Optional[Profile]: Profile if found in cache. """ - if key not in self.profiles: + if key not in self._open_profiles: return None else: + value = self._open_profiles[key] + if key not in self.profiles: + LOGGER.debug( + f"Rescuing profile {key} from eviction from cache; profile " + "will be reinserted into cache" + ) + self.profiles[key] = value self.profiles.move_to_end(key) - return self.profiles[key] + self._cleanup() + return value def has(self, key: str) -> bool: """Check whether there is a profile with associated key in the cache. @@ -62,16 +87,11 @@ def put(self, key: str, value: Profile) -> None: value (Profile): the profile to set """ value.finalizer() + self._open_profiles[key] = value self.profiles[key] = value + LOGGER.debug(f"Setting profile with id {key} in profile cache") self.profiles.move_to_end(key) - LOGGER.debug(f"setting profile with id {key} in profile cache") - - if len(self.profiles) > self.capacity: - LOGGER.debug( - f"Profile limit of {self.capacity} reached." - " Evicting least recently used profile..." - ) - self.profiles.popitem(last=False) + self._cleanup() def remove(self, key: str): """Remove profile with associated key from the cache. From c32d9079fc287c2cd15b7760e4f4d390e84f5f08 Mon Sep 17 00:00:00 2001 From: Daniel Bluhm Date: Thu, 7 Apr 2022 09:44:54 -0400 Subject: [PATCH 10/28] fix: create task for profile finalizer Signed-off-by: Daniel Bluhm --- aries_cloudagent/askar/profile.py | 3 +- aries_cloudagent/config/argparse.py | 15 ++++-- aries_cloudagent/indy/sdk/profile.py | 3 +- aries_cloudagent/multitenant/cache.py | 40 +++++++++------ aries_cloudagent/multitenant/manager.py | 17 ++++--- .../multitenant/tests/test_cache.py | 49 ++++++++++--------- 6 files changed, 75 insertions(+), 52 deletions(-) diff --git a/aries_cloudagent/askar/profile.py b/aries_cloudagent/askar/profile.py index 6277a936df..a03c7bbd3e 100644 --- a/aries_cloudagent/askar/profile.py +++ b/aries_cloudagent/askar/profile.py @@ -166,7 +166,8 @@ def finalizer(self) -> Optional[finalize]: def _finalize(opened: Optional[AskarOpenStore]): if opened: - asyncio.get_event_loop().run_until_complete(opened.close()) + LOGGER.debug("Profile finalizer called; closing wallet") + asyncio.get_event_loop().create_task(opened.close()) return finalize(self, _finalize, self.opened) diff --git a/aries_cloudagent/config/argparse.py b/aries_cloudagent/config/argparse.py index a8b6ae5110..dfcba82dd1 100644 --- a/aries_cloudagent/config/argparse.py +++ b/aries_cloudagent/config/argparse.py @@ -1645,18 +1645,23 @@ def get_settings(self, args: Namespace): settings["multitenant.admin_enabled"] = True if args.multitenancy_config: - multitenancyConfig = json.loads(args.multitenancy_config) + multitenancy_config = json.loads(args.multitenancy_config) - if multitenancyConfig.get("wallet_type"): - settings["multitenant.wallet_type"] = multitenancyConfig.get( + if multitenancy_config.get("wallet_type"): + settings["multitenant.wallet_type"] = multitenancy_config.get( "wallet_type" ) - if multitenancyConfig.get("wallet_name"): - settings["multitenant.wallet_name"] = multitenancyConfig.get( + if multitenancy_config.get("wallet_name"): + settings["multitenant.wallet_name"] = multitenancy_config.get( "wallet_name" ) + if multitenancy_config.get("cache_size"): + settings["multitenant.cache_size"] = multitenancy_config.get( + "cache_size" + ) + return settings diff --git a/aries_cloudagent/indy/sdk/profile.py b/aries_cloudagent/indy/sdk/profile.py index b5fb130b3d..5034408938 100644 --- a/aries_cloudagent/indy/sdk/profile.py +++ b/aries_cloudagent/indy/sdk/profile.py @@ -125,7 +125,8 @@ def finalizer(self) -> Optional[finalize]: def _finalize(opened: Optional[IndyOpenWallet]): if opened: - asyncio.get_event_loop().run_until_complete(opened.close()) + LOGGER.debug("Profile finalizer called; closing wallet") + asyncio.get_event_loop().create_task(opened.close()) return finalize(self, _finalize, self.opened) diff --git a/aries_cloudagent/multitenant/cache.py b/aries_cloudagent/multitenant/cache.py index 8a00120ca8..7fb636843b 100644 --- a/aries_cloudagent/multitenant/cache.py +++ b/aries_cloudagent/multitenant/cache.py @@ -21,19 +21,21 @@ def __init__(self, capacity: int): profiles are closed. """ - self.profiles: OrderedDict[str, Profile] = OrderedDict() - self._open_profiles: WeakValueDictionary[str, Profile] = WeakValueDictionary() + LOGGER.debug(f"Profile cache initialized with capacity {capacity}") + + self._cache: OrderedDict[str, Profile] = OrderedDict() + self.profiles: WeakValueDictionary[str, Profile] = WeakValueDictionary() self.capacity = capacity def _cleanup(self): """Prune cache until size matches defined capacity.""" - if len(self.profiles) > self.capacity: + if len(self._cache) > self.capacity: LOGGER.debug( f"Profile limit of {self.capacity} reached." " Evicting least recently used profiles..." ) - while len(self.profiles) > self.capacity: - key, _ = self.profiles.popitem(last=False) + while len(self._cache) > self.capacity: + key, _ = self._cache.popitem(last=False) LOGGER.debug(f"Evicted profile with key {key}") def get(self, key: str) -> Optional[Profile]: @@ -50,19 +52,18 @@ def get(self, key: str) -> Optional[Profile]: Optional[Profile]: Profile if found in cache. """ - if key not in self._open_profiles: - return None - else: - value = self._open_profiles[key] - if key not in self.profiles: + value = self.profiles.get(key) + if value: + if key not in self._cache: LOGGER.debug( f"Rescuing profile {key} from eviction from cache; profile " "will be reinserted into cache" ) - self.profiles[key] = value - self.profiles.move_to_end(key) + self._cache[key] = value + self._cache.move_to_end(key) self._cleanup() - return value + + return value def has(self, key: str) -> bool: """Check whether there is a profile with associated key in the cache. @@ -86,11 +87,19 @@ def put(self, key: str, value: Profile) -> None: key (str): the key to set value (Profile): the profile to set """ + + # Close the profile when it falls out of scope value.finalizer() - self._open_profiles[key] = value + + # Keep track of currently opened profiles using weak references self.profiles[key] = value + + # Strong reference to profile to hold open until evicted LOGGER.debug(f"Setting profile with id {key} in profile cache") - self.profiles.move_to_end(key) + self._cache[key] = value + + # Refresh profile livliness + self._cache.move_to_end(key) self._cleanup() def remove(self, key: str): @@ -100,3 +109,4 @@ def remove(self, key: str): key (str): The key to remove from the cache. """ del self.profiles[key] + del self._cache[key] diff --git a/aries_cloudagent/multitenant/manager.py b/aries_cloudagent/multitenant/manager.py index 210d799a06..e7bf2d9447 100644 --- a/aries_cloudagent/multitenant/manager.py +++ b/aries_cloudagent/multitenant/manager.py @@ -1,16 +1,17 @@ """Manager for multitenancy.""" +import logging from typing import Iterable -from ..core.profile import ( - Profile, -) -from ..config.wallet import wallet_config + from ..config.injection_context import InjectionContext -from ..wallet.models.wallet_record import WalletRecord +from ..config.wallet import wallet_config +from ..core.profile import Profile from ..multitenant.base import BaseMultitenantManager - +from ..wallet.models.wallet_record import WalletRecord from .cache import ProfileCache +LOGGER = logging.getLogger(__name__) + class MultitenantManager(BaseMultitenantManager): """Class for handling multitenancy.""" @@ -22,7 +23,9 @@ def __init__(self, profile: Profile): profile: The profile for this manager """ super().__init__(profile) - self._profiles = ProfileCache(100) + self._profiles = ProfileCache( + profile.settings.get_int("multitenant.cache_size") or 100 + ) @property def open_profiles(self) -> Iterable[Profile]: diff --git a/aries_cloudagent/multitenant/tests/test_cache.py b/aries_cloudagent/multitenant/tests/test_cache.py index 451e465051..ce90488512 100644 --- a/aries_cloudagent/multitenant/tests/test_cache.py +++ b/aries_cloudagent/multitenant/tests/test_cache.py @@ -1,8 +1,19 @@ -from asynctest import mock as async_mock +from ...core.profile import Profile from ..cache import ProfileCache +class MockProfile(Profile): + def session(self, context = None): + ... + + def transaction(self, context = None): + ... + + def finalizer(self): + return None + + def test_get_not_in_cache(): cache = ProfileCache(1) @@ -12,7 +23,7 @@ def test_get_not_in_cache(): def test_put_get_in_cache(): cache = ProfileCache(1) - profile = async_mock.MagicMock() + profile = MockProfile() cache.put("1", profile) assert cache.get("1") is profile @@ -21,7 +32,7 @@ def test_put_get_in_cache(): def test_remove(): cache = ProfileCache(1) - profile = async_mock.MagicMock() + profile = MockProfile() cache.put("1", profile) assert cache.get("1") is profile @@ -34,7 +45,7 @@ def test_remove(): def test_has_true(): cache = ProfileCache(1) - profile = async_mock.MagicMock() + profile = MockProfile() assert cache.has("1") is False cache.put("1", profile) @@ -44,14 +55,11 @@ def test_has_true(): def test_cleanup(): cache = ProfileCache(1) - profile1 = async_mock.MagicMock() - profile2 = async_mock.MagicMock() - - cache.put("1", profile1) + cache.put("1", MockProfile()) assert len(cache.profiles) == 1 - cache.put("2", profile2) + cache.put("2", MockProfile()) assert len(cache.profiles) == 1 assert cache.get("1") == None @@ -60,23 +68,18 @@ def test_cleanup(): def test_cleanup_lru(): cache = ProfileCache(3) - profile1 = async_mock.MagicMock() - profile2 = async_mock.MagicMock() - profile3 = async_mock.MagicMock() - profile4 = async_mock.MagicMock() - - cache.put("1", profile1) - cache.put("2", profile2) - cache.put("3", profile3) + cache.put("1", MockProfile()) + cache.put("2", MockProfile()) + cache.put("3", MockProfile()) assert len(cache.profiles) == 3 cache.get("1") - cache.put("4", profile4) + cache.put("4", MockProfile()) - assert len(cache.profiles) == 3 - assert cache.get("1") == profile1 - assert cache.get("2") == None - assert cache.get("3") == profile3 - assert cache.get("4") == profile4 + assert len(cache._cache) == 3 + assert cache.get("1") + assert cache.get("2") is None + assert cache.get("3") + assert cache.get("4") From 352840921f89c69dd389a1feb63f94c4afcd2048 Mon Sep 17 00:00:00 2001 From: Daniel Bluhm Date: Thu, 7 Apr 2022 09:47:42 -0400 Subject: [PATCH 11/28] style: black fixes Signed-off-by: Daniel Bluhm --- aries_cloudagent/multitenant/tests/test_cache.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/aries_cloudagent/multitenant/tests/test_cache.py b/aries_cloudagent/multitenant/tests/test_cache.py index ce90488512..7350cdc1f0 100644 --- a/aries_cloudagent/multitenant/tests/test_cache.py +++ b/aries_cloudagent/multitenant/tests/test_cache.py @@ -4,10 +4,10 @@ class MockProfile(Profile): - def session(self, context = None): + def session(self, context=None): ... - def transaction(self, context = None): + def transaction(self, context=None): ... def finalizer(self): From f424dbe8568781bb55b56173e26bdc04bd77b1ef Mon Sep 17 00:00:00 2001 From: Daniel Bluhm Date: Mon, 11 Apr 2022 20:52:05 -0400 Subject: [PATCH 12/28] fix: prevent circular references from breaking cache Signed-off-by: Daniel Bluhm --- aries_cloudagent/admin/server.py | 17 +++++++++++++--- aries_cloudagent/core/dispatcher.py | 30 ++++++++++++++++++++--------- 2 files changed, 35 insertions(+), 12 deletions(-) diff --git a/aries_cloudagent/admin/server.py b/aries_cloudagent/admin/server.py index 3043f10ca4..096f0fd680 100644 --- a/aries_cloudagent/admin/server.py +++ b/aries_cloudagent/admin/server.py @@ -7,6 +7,7 @@ from typing import Callable, Coroutine import uuid import warnings +import weakref from aiohttp import web from aiohttp_apispec import ( @@ -115,7 +116,11 @@ def __init__( """ super().__init__(**kwargs) - self._profile = profile + # Weakly hold the profile so this reference doesn't prevent profiles + # from being cleaned up when appropriate. + # Binding this AdminResponder to the profile's context creates a circular + # reference. + self._profile = weakref.ref(profile) self._send = send async def send_outbound(self, message: OutboundMessage) -> OutboundSendStatus: @@ -125,7 +130,10 @@ async def send_outbound(self, message: OutboundMessage) -> OutboundSendStatus: Args: message: The `OutboundMessage` to be sent """ - return await self._send(self._profile, message) + profile = self._profile() + if not profile: + raise RuntimeError("weakref to profile has expired") + return await self._send(profile, message) async def send_webhook(self, topic: str, payload: dict): """ @@ -139,7 +147,10 @@ async def send_webhook(self, topic: str, payload: dict): "responder.send_webhook is deprecated; please use the event bus instead.", DeprecationWarning, ) - await self._profile.notify("acapy::webhook::" + topic, payload) + profile = self._profile() + if not profile: + raise RuntimeError("weakref to profile has expired") + await profile.notify("acapy::webhook::" + topic, payload) @property def send_fn(self) -> Coroutine: diff --git a/aries_cloudagent/core/dispatcher.py b/aries_cloudagent/core/dispatcher.py index 98df420472..f7663e64dd 100644 --- a/aries_cloudagent/core/dispatcher.py +++ b/aries_cloudagent/core/dispatcher.py @@ -11,6 +11,7 @@ import warnings from typing import Callable, Coroutine, Union +import weakref from aiohttp.web import HTTPException @@ -272,7 +273,10 @@ def __init__( """ super().__init__(**kwargs) - self._context = context + # Weakly hold the context so it can be properly garbage collected. + # Binding this DispatcherResponder into the context creates a circular + # reference. + self._context = weakref.ref(context) self._inbound_message = inbound_message self._send = send_outbound @@ -285,13 +289,13 @@ async def create_outbound( Args: message: The message payload """ - if isinstance(message, AgentMessage) and self._context.settings.get( - "timing.enabled" - ): + context = self._context() + if not context: + raise RuntimeError("weakref to context has expired") + + if isinstance(message, AgentMessage) and context.settings.get("timing.enabled"): # Inject the timing decorator - in_time = ( - self._context.message_receipt and self._context.message_receipt.in_time - ) + in_time = context.message_receipt and context.message_receipt.in_time if not message._decorators.get("timing"): message._decorators["timing"] = { "in_time": in_time, @@ -307,7 +311,11 @@ async def send_outbound(self, message: OutboundMessage) -> OutboundSendStatus: Args: message: The `OutboundMessage` to be sent """ - return await self._send(self._context.profile, message, self._inbound_message) + context = self._context() + if not context: + raise RuntimeError("weakref to context has expired") + + return await self._send(context.profile, message, self._inbound_message) async def send_webhook(self, topic: str, payload: dict): """ @@ -321,4 +329,8 @@ async def send_webhook(self, topic: str, payload: dict): "responder.send_webhook is deprecated; please use the event bus instead.", DeprecationWarning, ) - await self._context.profile.notify("acapy::webhook::" + topic, payload) + context = self._context() + if not context: + raise RuntimeError("weakref to context has expired") + + await context.profile.notify("acapy::webhook::" + topic, payload) From a091c63190f88d2479026010a0933c1741e9511c Mon Sep 17 00:00:00 2001 From: Daniel Bluhm Date: Mon, 11 Apr 2022 21:13:11 -0400 Subject: [PATCH 13/28] feat: rework multitenancy-config arg Signed-off-by: Daniel Bluhm --- aries_cloudagent/config/argparse.py | 50 +++++++++++++++++------------ 1 file changed, 30 insertions(+), 20 deletions(-) diff --git a/aries_cloudagent/config/argparse.py b/aries_cloudagent/config/argparse.py index 5551a85389..1bdb3396dd 100644 --- a/aries_cloudagent/config/argparse.py +++ b/aries_cloudagent/config/argparse.py @@ -1629,12 +1629,13 @@ def add_arguments(self, parser: ArgumentParser): parser.add_argument( "--multitenancy-config", type=str, - metavar="", + nargs="+", + metavar="key=value", env_var="ACAPY_MULTITENANCY_CONFIGURATION", help=( - 'Specify multitenancy configuration ("wallet_type" and "wallet_name"). ' - 'For example: "{"wallet_type":"askar-profile","wallet_name":' - '"askar-profile-name"}"' + "Specify multitenancy configuration in key=value pairs. " + 'For example: "wallet_type=askar-profile wallet_name=askar-profile-name" ' + "Possible values: wallet_name, wallet_key, cache_size. " '"wallet_name" is only used when "wallet_type" is "askar-profile"' ), ) @@ -1656,22 +1657,31 @@ def get_settings(self, args: Namespace): settings["multitenant.admin_enabled"] = True if args.multitenancy_config: - multitenancy_config = json.loads(args.multitenancy_config) - - if multitenancy_config.get("wallet_type"): - settings["multitenant.wallet_type"] = multitenancy_config.get( - "wallet_type" - ) - - if multitenancy_config.get("wallet_name"): - settings["multitenant.wallet_name"] = multitenancy_config.get( - "wallet_name" - ) - - if multitenancy_config.get("cache_size"): - settings["multitenant.cache_size"] = multitenancy_config.get( - "cache_size" - ) + # Legacy support + if ( + len(args.multitenancy_config) == 1 + and args.multitenancy_config[0][0] == "{" + ): + multitenancy_config = json.loads(args.multitenancy_config) + if multitenancy_config.get("wallet_type"): + settings["multitenant.wallet_type"] = multitenancy_config.get( + "wallet_type" + ) + + if multitenancy_config.get("wallet_name"): + settings["multitenant.wallet_name"] = multitenancy_config.get( + "wallet_name" + ) + + if multitenancy_config.get("cache_size"): + settings["multitenant.cache_size"] = multitenancy_config.get( + "cache_size" + ) + else: + for value_str in chain(*args.multitenancy_config): + key, value = value_str.split("=", maxsplit=1) + value = yaml.safe_load(value) + settings[f"multitenant.{key}"] = value return settings From 2ffd52163e826ce6e022c37979b19df08bea751d Mon Sep 17 00:00:00 2001 From: Daniel Bluhm Date: Mon, 11 Apr 2022 21:22:44 -0400 Subject: [PATCH 14/28] fix: parsing multiple multitenancy-config options Signed-off-by: Daniel Bluhm --- aries_cloudagent/config/argparse.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/aries_cloudagent/config/argparse.py b/aries_cloudagent/config/argparse.py index 1bdb3396dd..2d9b2f6b13 100644 --- a/aries_cloudagent/config/argparse.py +++ b/aries_cloudagent/config/argparse.py @@ -1678,7 +1678,7 @@ def get_settings(self, args: Namespace): "cache_size" ) else: - for value_str in chain(*args.multitenancy_config): + for value_str in args.multitenancy_config: key, value = value_str.split("=", maxsplit=1) value = yaml.safe_load(value) settings[f"multitenant.{key}"] = value From b5b09371392583f105e69c11c89b4d1c094f25ff Mon Sep 17 00:00:00 2001 From: Daniel Bluhm Date: Tue, 12 Apr 2022 09:16:29 -0400 Subject: [PATCH 15/28] fix: multitenancy config json parsing and tests Signed-off-by: Daniel Bluhm --- aries_cloudagent/config/argparse.py | 2 +- aries_cloudagent/config/tests/test_argparse.py | 18 ++++++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/aries_cloudagent/config/argparse.py b/aries_cloudagent/config/argparse.py index 2d9b2f6b13..3abfa363f2 100644 --- a/aries_cloudagent/config/argparse.py +++ b/aries_cloudagent/config/argparse.py @@ -1662,7 +1662,7 @@ def get_settings(self, args: Namespace): len(args.multitenancy_config) == 1 and args.multitenancy_config[0][0] == "{" ): - multitenancy_config = json.loads(args.multitenancy_config) + multitenancy_config = json.loads(args.multitenancy_config[0]) if multitenancy_config.get("wallet_type"): settings["multitenant.wallet_type"] = multitenancy_config.get( "wallet_type" diff --git a/aries_cloudagent/config/tests/test_argparse.py b/aries_cloudagent/config/tests/test_argparse.py index 4e76a681d2..e8672f7a1e 100644 --- a/aries_cloudagent/config/tests/test_argparse.py +++ b/aries_cloudagent/config/tests/test_argparse.py @@ -263,6 +263,24 @@ async def test_multitenancy_settings(self): assert settings.get("multitenant.wallet_type") == "askar" assert settings.get("multitenant.wallet_name") == "test" + result = parser.parse_args( + [ + "--multitenant", + "--jwt-secret", + "secret", + "--multitenancy-config", + "wallet_type=askar", + "wallet_name=test", + ] + ) + + settings = group.get_settings(result) + + assert settings.get("multitenant.enabled") == True + assert settings.get("multitenant.jwt_secret") == "secret" + assert settings.get("multitenant.wallet_type") == "askar" + assert settings.get("multitenant.wallet_name") == "test" + async def test_endorser_settings(self): """Test required argument parsing.""" From 1c218041a843f9428926f712f06f46278af43c0f Mon Sep 17 00:00:00 2001 From: Daniel Bluhm Date: Thu, 14 Apr 2022 15:27:26 -0400 Subject: [PATCH 16/28] test: askar profile finalizer And make tests follow pytest convention Signed-off-by: Daniel Bluhm --- aries_cloudagent/askar/tests/test_profile.py | 153 ++++++++++--------- 1 file changed, 85 insertions(+), 68 deletions(-) diff --git a/aries_cloudagent/askar/tests/test_profile.py b/aries_cloudagent/askar/tests/test_profile.py index 4c4c646a84..319f0b64c7 100644 --- a/aries_cloudagent/askar/tests/test_profile.py +++ b/aries_cloudagent/askar/tests/test_profile.py @@ -1,7 +1,8 @@ import asyncio +import logging import pytest -from asynctest import TestCase as AsyncTestCase, mock +from asynctest import mock from ...askar.profile import AskarProfile from ...config.injection_context import InjectionContext @@ -9,73 +10,89 @@ from .. import profile as test_module -class TestProfile(AsyncTestCase): - @mock.patch("aries_cloudagent.askar.store.AskarOpenStore") - async def test_init_success(self, AskarOpenStore): +@pytest.fixture +def open_store(): + yield mock.MagicMock() + + +async def test_init_success(open_store): + askar_profile = AskarProfile( + open_store, + ) + + assert askar_profile.opened == open_store + + +async def test_remove_success(open_store): + openStore = open_store + context = InjectionContext() + profile_id = "profile_id" + context.settings = { + "multitenant.wallet_type": "askar-profile", + "wallet.askar_profile": profile_id, + "ledger.genesis_transactions": mock.MagicMock(), + } + askar_profile = AskarProfile(openStore, context, profile_id=profile_id) + remove_profile_stub = asyncio.Future() + remove_profile_stub.set_result(True) + openStore.store.remove_profile.return_value = remove_profile_stub + + await askar_profile.remove() + + openStore.store.remove_profile.assert_called_once_with(profile_id) + + +async def test_remove_profile_not_removed_if_wallet_type_not_askar_profile(open_store): + openStore = open_store + context = InjectionContext() + context.settings = {"multitenant.wallet_type": "basic"} + askar_profile = AskarProfile(openStore, context) + + await askar_profile.remove() + + openStore.store.remove_profile.assert_not_called() + + +@pytest.mark.asyncio +async def test_profile_manager_transaction(): + profile = "profileId" + + with mock.patch("aries_cloudagent.askar.profile.AskarProfile") as AskarProfile: + askar_profile = AskarProfile(None, True, profile_id=profile) + askar_profile.profile_id = profile + askar_profile_transaction = mock.MagicMock() + askar_profile.store.transaction.return_value = askar_profile_transaction + + transactionProfile = test_module.AskarProfileSession(askar_profile, True) + + assert transactionProfile._opener == askar_profile_transaction + askar_profile.store.transaction.assert_called_once_with(profile) + + +@pytest.mark.asyncio +async def test_profile_manager_store(): + profile = "profileId" + + with mock.patch("aries_cloudagent.askar.profile.AskarProfile") as AskarProfile: + askar_profile = AskarProfile(None, False, profile_id=profile) + askar_profile.profile_id = profile + askar_profile_session = mock.MagicMock() + askar_profile.store.session.return_value = askar_profile_session + + sessionProfile = test_module.AskarProfileSession(askar_profile, False) + + assert sessionProfile._opener == askar_profile_session + askar_profile.store.session.assert_called_once_with(profile) + + +def test_finalizer(open_store, caplog): + def _smaller_scope(): askar_profile = AskarProfile( - AskarOpenStore, + open_store, ) + askar_profile.finalizer() + + with caplog.at_level(logging.DEBUG): + _smaller_scope() - assert askar_profile.opened == AskarOpenStore - - @mock.patch("aries_cloudagent.askar.store.AskarOpenStore") - async def test_remove_success(self, AskarOpenStore): - openStore = AskarOpenStore - context = InjectionContext() - profile_id = "profile_id" - context.settings = { - "multitenant.wallet_type": "askar-profile", - "wallet.askar_profile": profile_id, - "ledger.genesis_transactions": mock.MagicMock(), - } - askar_profile = AskarProfile(openStore, context, profile_id=profile_id) - remove_profile_stub = asyncio.Future() - remove_profile_stub.set_result(True) - openStore.store.remove_profile.return_value = remove_profile_stub - - await askar_profile.remove() - - openStore.store.remove_profile.assert_called_once_with(profile_id) - - @mock.patch("aries_cloudagent.askar.store.AskarOpenStore") - async def test_remove_profile_not_removed_if_wallet_type_not_askar_profile( - self, AskarOpenStore - ): - openStore = AskarOpenStore - context = InjectionContext() - context.settings = {"multitenant.wallet_type": "basic"} - askar_profile = AskarProfile(openStore, context) - - await askar_profile.remove() - - openStore.store.remove_profile.assert_not_called() - - @pytest.mark.asyncio - async def test_profile_manager_transaction(self): - profile = "profileId" - - with mock.patch("aries_cloudagent.askar.profile.AskarProfile") as AskarProfile: - askar_profile = AskarProfile(None, True, profile_id=profile) - askar_profile.profile_id = profile - askar_profile_transaction = mock.MagicMock() - askar_profile.store.transaction.return_value = askar_profile_transaction - - transactionProfile = test_module.AskarProfileSession(askar_profile, True) - - assert transactionProfile._opener == askar_profile_transaction - askar_profile.store.transaction.assert_called_once_with(profile) - - @pytest.mark.asyncio - async def test_profile_manager_store(self): - profile = "profileId" - - with mock.patch("aries_cloudagent.askar.profile.AskarProfile") as AskarProfile: - askar_profile = AskarProfile(None, False, profile_id=profile) - askar_profile.profile_id = profile - askar_profile_session = mock.MagicMock() - askar_profile.store.session.return_value = askar_profile_session - - sessionProfile = test_module.AskarProfileSession(askar_profile, False) - - assert sessionProfile._opener == askar_profile_session - askar_profile.store.session.assert_called_once_with(profile) + assert "finalizer called" in caplog.text From 108ecabc0e1fa309417154411f069d90f3d27e4c Mon Sep 17 00:00:00 2001 From: Daniel Bluhm Date: Thu, 14 Apr 2022 15:30:20 -0400 Subject: [PATCH 17/28] refactor: indy test profile by pytest convention Signed-off-by: Daniel Bluhm --- .../indy/sdk/tests/test_profile.py | 122 +++++++++--------- 1 file changed, 61 insertions(+), 61 deletions(-) diff --git a/aries_cloudagent/indy/sdk/tests/test_profile.py b/aries_cloudagent/indy/sdk/tests/test_profile.py index 6db4425574..aed2f93ef6 100644 --- a/aries_cloudagent/indy/sdk/tests/test_profile.py +++ b/aries_cloudagent/indy/sdk/tests/test_profile.py @@ -25,65 +25,65 @@ async def profile(): ) -class TestIndySdkProfile: - @pytest.mark.asyncio - async def test_properties(self, profile): - assert profile.name == "test-profile" - assert profile.backend == "indy" - assert profile.wallet and profile.wallet.handle == 1 - - assert "IndySdkProfile" in str(profile) - assert profile.created - assert profile.wallet.created - assert profile.wallet.master_secret_id == "master-secret" - - with async_mock.patch.object(profile, "opened", False): - with pytest.raises(ProfileError): - await profile.remove() - - with async_mock.patch.object( - profile.opened, "close", async_mock.CoroutineMock() - ): +@pytest.mark.asyncio +async def test_properties(profile): + assert profile.name == "test-profile" + assert profile.backend == "indy" + assert profile.wallet and profile.wallet.handle == 1 + + assert "IndySdkProfile" in str(profile) + assert profile.created + assert profile.wallet.created + assert profile.wallet.master_secret_id == "master-secret" + + with async_mock.patch.object(profile, "opened", False): + with pytest.raises(ProfileError): await profile.remove() - assert profile.opened is None - - def test_settings_genesis_transactions(self): - context = InjectionContext( - settings={"ledger.genesis_transactions": async_mock.MagicMock()} - ) - context.injector.bind_instance(IndySdkLedgerPool, IndySdkLedgerPool("name")) - profile = IndySdkProfile( - IndyOpenWallet( - config=IndyWalletConfig({"name": "test-profile"}), - created=True, - handle=1, - master_secret_id="master-secret", - ), - context, - ) - - def test_settings_ledger_config(self): - context = InjectionContext(settings={"ledger.ledger_config_list": True}) - context.injector.bind_instance(IndySdkLedgerPool, IndySdkLedgerPool("name")) - profile = IndySdkProfile( - IndyOpenWallet( - config=IndyWalletConfig({"name": "test-profile"}), - created=True, - handle=1, - master_secret_id="master-secret", - ), - context, - ) - - def test_read_only(self): - context = InjectionContext(settings={"ledger.read_only": True}) - context.injector.bind_instance(IndySdkLedgerPool, IndySdkLedgerPool("name")) - ro_profile = IndySdkProfile( - IndyOpenWallet( - config=IndyWalletConfig({"name": "test-profile"}), - created=True, - handle=1, - master_secret_id="master-secret", - ), - context, - ) + + with async_mock.patch.object(profile.opened, "close", async_mock.CoroutineMock()): + await profile.remove() + assert profile.opened is None + + +def test_settings_genesis_transactions(): + context = InjectionContext( + settings={"ledger.genesis_transactions": async_mock.MagicMock()} + ) + context.injector.bind_instance(IndySdkLedgerPool, IndySdkLedgerPool("name")) + profile = IndySdkProfile( + IndyOpenWallet( + config=IndyWalletConfig({"name": "test-profile"}), + created=True, + handle=1, + master_secret_id="master-secret", + ), + context, + ) + + +def test_settings_ledger_config(): + context = InjectionContext(settings={"ledger.ledger_config_list": True}) + context.injector.bind_instance(IndySdkLedgerPool, IndySdkLedgerPool("name")) + profile = IndySdkProfile( + IndyOpenWallet( + config=IndyWalletConfig({"name": "test-profile"}), + created=True, + handle=1, + master_secret_id="master-secret", + ), + context, + ) + + +def test_read_only(): + context = InjectionContext(settings={"ledger.read_only": True}) + context.injector.bind_instance(IndySdkLedgerPool, IndySdkLedgerPool("name")) + ro_profile = IndySdkProfile( + IndyOpenWallet( + config=IndyWalletConfig({"name": "test-profile"}), + created=True, + handle=1, + master_secret_id="master-secret", + ), + context, + ) From 2e80b2991d89b4696b6567af21b0e8bccf6e5526 Mon Sep 17 00:00:00 2001 From: Daniel Bluhm Date: Thu, 14 Apr 2022 15:35:56 -0400 Subject: [PATCH 18/28] test: indy sdk profile finalizer Signed-off-by: Daniel Bluhm --- aries_cloudagent/askar/tests/test_profile.py | 4 +- .../indy/sdk/tests/test_profile.py | 70 ++++++++----------- 2 files changed, 31 insertions(+), 43 deletions(-) diff --git a/aries_cloudagent/askar/tests/test_profile.py b/aries_cloudagent/askar/tests/test_profile.py index 319f0b64c7..cc484b109b 100644 --- a/aries_cloudagent/askar/tests/test_profile.py +++ b/aries_cloudagent/askar/tests/test_profile.py @@ -87,9 +87,7 @@ async def test_profile_manager_store(): def test_finalizer(open_store, caplog): def _smaller_scope(): - askar_profile = AskarProfile( - open_store, - ) + askar_profile = AskarProfile(open_store) askar_profile.finalizer() with caplog.at_level(logging.DEBUG): diff --git a/aries_cloudagent/indy/sdk/tests/test_profile.py b/aries_cloudagent/indy/sdk/tests/test_profile.py index aed2f93ef6..7372cd1fea 100644 --- a/aries_cloudagent/indy/sdk/tests/test_profile.py +++ b/aries_cloudagent/indy/sdk/tests/test_profile.py @@ -1,3 +1,4 @@ +import logging import pytest from asynctest import mock as async_mock @@ -10,19 +11,21 @@ from ..wallet_setup import IndyWalletConfig, IndyOpenWallet +@pytest.fixture +async def open_wallet(): + yield IndyOpenWallet( + config=IndyWalletConfig({"name": "test-profile"}), + created=True, + handle=1, + master_secret_id="master-secret", + ) + + @pytest.fixture() -async def profile(): +async def profile(open_wallet): context = InjectionContext() context.injector.bind_instance(IndySdkLedgerPool, IndySdkLedgerPool("name")) - yield IndySdkProfile( - IndyOpenWallet( - config=IndyWalletConfig({"name": "test-profile"}), - created=True, - handle=1, - master_secret_id="master-secret", - ), - context, - ) + yield IndySdkProfile(open_wallet, context) @pytest.mark.asyncio @@ -45,45 +48,32 @@ async def test_properties(profile): assert profile.opened is None -def test_settings_genesis_transactions(): +def test_settings_genesis_transactions(open_wallet): context = InjectionContext( settings={"ledger.genesis_transactions": async_mock.MagicMock()} ) context.injector.bind_instance(IndySdkLedgerPool, IndySdkLedgerPool("name")) - profile = IndySdkProfile( - IndyOpenWallet( - config=IndyWalletConfig({"name": "test-profile"}), - created=True, - handle=1, - master_secret_id="master-secret", - ), - context, - ) + profile = IndySdkProfile(open_wallet, context) -def test_settings_ledger_config(): +def test_settings_ledger_config(open_wallet): context = InjectionContext(settings={"ledger.ledger_config_list": True}) context.injector.bind_instance(IndySdkLedgerPool, IndySdkLedgerPool("name")) - profile = IndySdkProfile( - IndyOpenWallet( - config=IndyWalletConfig({"name": "test-profile"}), - created=True, - handle=1, - master_secret_id="master-secret", - ), - context, - ) + profile = IndySdkProfile(open_wallet, context) -def test_read_only(): +def test_read_only(open_wallet): context = InjectionContext(settings={"ledger.read_only": True}) context.injector.bind_instance(IndySdkLedgerPool, IndySdkLedgerPool("name")) - ro_profile = IndySdkProfile( - IndyOpenWallet( - config=IndyWalletConfig({"name": "test-profile"}), - created=True, - handle=1, - master_secret_id="master-secret", - ), - context, - ) + ro_profile = IndySdkProfile(open_wallet, context) + + +def test_finalizer(open_wallet, caplog): + def _smaller_scope(): + profile = IndySdkProfile(open_wallet) + profile.finalizer() + + with caplog.at_level(logging.DEBUG): + _smaller_scope() + + assert "finalizer called" in caplog.text From 9cfbcda9053357a83f987f83f12091c8613074e5 Mon Sep 17 00:00:00 2001 From: Daniel Bluhm Date: Thu, 14 Apr 2022 15:43:54 -0400 Subject: [PATCH 19/28] test: cache_size parsing Signed-off-by: Daniel Bluhm --- aries_cloudagent/config/tests/test_argparse.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/aries_cloudagent/config/tests/test_argparse.py b/aries_cloudagent/config/tests/test_argparse.py index e8672f7a1e..605621857f 100644 --- a/aries_cloudagent/config/tests/test_argparse.py +++ b/aries_cloudagent/config/tests/test_argparse.py @@ -252,7 +252,7 @@ async def test_multitenancy_settings(self): "--jwt-secret", "secret", "--multitenancy-config", - '{"wallet_type":"askar","wallet_name":"test"}', + '{"wallet_type":"askar","wallet_name":"test", "cache_size": 10}', ] ) @@ -271,6 +271,7 @@ async def test_multitenancy_settings(self): "--multitenancy-config", "wallet_type=askar", "wallet_name=test", + "cache_size=10", ] ) From 0422d8aeb43cdf09d15403df3cb9828b8e340cba Mon Sep 17 00:00:00 2001 From: Daniel Bluhm Date: Thu, 14 Apr 2022 15:44:58 -0400 Subject: [PATCH 20/28] test: dispatcher responder context weak ref Signed-off-by: Daniel Bluhm --- aries_cloudagent/core/tests/test_dispatcher.py | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/aries_cloudagent/core/tests/test_dispatcher.py b/aries_cloudagent/core/tests/test_dispatcher.py index 3b5e245367..b9f00564b1 100644 --- a/aries_cloudagent/core/tests/test_dispatcher.py +++ b/aries_cloudagent/core/tests/test_dispatcher.py @@ -404,3 +404,20 @@ async def test_create_enc_outbound(self): ) as mock_send_outbound: await responder.send(message) assert mock_send_outbound.called_once() + + async def test_expired_context_x(self): + def _smaller_scope(): + profile = make_profile() + context = RequestContext(profile) + message = b"abc123xyz7890000" + return test_module.DispatcherResponder(context, message, None) + + responder = _smaller_scope() + with self.assertRaises(RuntimeError): + await responder.create_outbound(b"test") + + with self.assertRaises(RuntimeError): + await responder.send_outbound(None) + + with self.assertRaises(RuntimeError): + await responder.send_webhook("test", {}) From 94b0c3e6f797d5b471bab580e7434dfa12fc6f4d Mon Sep 17 00:00:00 2001 From: Daniel Bluhm Date: Thu, 14 Apr 2022 15:49:22 -0400 Subject: [PATCH 21/28] test: rescuing still open profile from eviction Signed-off-by: Daniel Bluhm --- .../multitenant/tests/test_cache.py | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/aries_cloudagent/multitenant/tests/test_cache.py b/aries_cloudagent/multitenant/tests/test_cache.py index 7350cdc1f0..7acaaba318 100644 --- a/aries_cloudagent/multitenant/tests/test_cache.py +++ b/aries_cloudagent/multitenant/tests/test_cache.py @@ -83,3 +83,28 @@ def test_cleanup_lru(): assert cache.get("2") is None assert cache.get("3") assert cache.get("4") + + +def test_rescue_open_profile(): + cache = ProfileCache(3) + + cache.put("1", MockProfile()) + cache.put("2", MockProfile()) + cache.put("3", MockProfile()) + + assert len(cache.profiles) == 3 + + held = cache.profiles["1"] + cache.put("4", MockProfile()) + + assert len(cache.profiles) == 4 + assert len(cache._cache) == 3 + + cache.get("1") + + assert len(cache.profiles) == 3 + assert len(cache._cache) == 3 + assert cache.get("1") + assert cache.get("2") is None + assert cache.get("3") + assert cache.get("4") From a9402f47eaec0c96b03f2a97b5a23823d69839d8 Mon Sep 17 00:00:00 2001 From: Daniel Bluhm Date: Thu, 14 Apr 2022 15:54:16 -0400 Subject: [PATCH 22/28] test: askar profile manager open profiles Signed-off-by: Daniel Bluhm --- .../tests/test_askar_profile_manager.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/aries_cloudagent/multitenant/tests/test_askar_profile_manager.py b/aries_cloudagent/multitenant/tests/test_askar_profile_manager.py index b93930f2b0..5bbc1d926c 100644 --- a/aries_cloudagent/multitenant/tests/test_askar_profile_manager.py +++ b/aries_cloudagent/multitenant/tests/test_askar_profile_manager.py @@ -157,3 +157,18 @@ async def test_remove_wallet_profile(self): with async_mock.patch.object(InMemoryProfile, "remove") as profile_remove: await self.manager.remove_wallet_profile(test_profile) profile_remove.assert_called_once_with() + + async def test_open_profiles(self): + assert len(list(self.manager.open_profiles)) == 0 + + create_profile_stub = asyncio.Future() + create_profile_stub.set_result("") + with async_mock.patch( + "aries_cloudagent.multitenant.askar_profile_manager.AskarProfile" + ) as AskarProfile: + sub_wallet_profile = AskarProfile(None, None) + sub_wallet_profile.context.copy.return_value = InjectionContext() + sub_wallet_profile.store.create_profile.return_value = create_profile_stub + self.manager._multitenant_profile = sub_wallet_profile + + assert len(list(self.manager.open_profiles)) == 1 From 4a5f65ddcc37adf0879e036ec69cf99489d8e26f Mon Sep 17 00:00:00 2001 From: Daniel Bluhm Date: Thu, 14 Apr 2022 17:23:03 -0400 Subject: [PATCH 23/28] fix: self.manager for mocking methods in tests Signed-off-by: Daniel Bluhm --- aries_cloudagent/multitenant/tests/test_base.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/aries_cloudagent/multitenant/tests/test_base.py b/aries_cloudagent/multitenant/tests/test_base.py index 2a73a3143b..2832fc74ef 100644 --- a/aries_cloudagent/multitenant/tests/test_base.py +++ b/aries_cloudagent/multitenant/tests/test_base.py @@ -556,7 +556,7 @@ async def test_get_profile_for_token_managed_wallet_iat(self): ).decode() with async_mock.patch.object( - BaseMultitenantManager, "get_wallet_profile" + self.manager, "get_wallet_profile" ) as get_wallet_profile: mock_profile = InMemoryProfile.test_profile() get_wallet_profile.return_value = mock_profile @@ -594,7 +594,7 @@ async def test_get_profile_for_token_managed_wallet_x_iat_no_match(self): ).decode() with async_mock.patch.object( - BaseMultitenantManager, "get_wallet_profile" + self.manager, "get_wallet_profile" ) as get_wallet_profile, self.assertRaises( MultitenantManagerError, msg="Token not valid" ): From 4bf26e58845839514d670564897d7043d8f19ab4 Mon Sep 17 00:00:00 2001 From: Daniel Bluhm Date: Thu, 14 Apr 2022 17:57:44 -0400 Subject: [PATCH 24/28] test: admin responder profile expired Signed-off-by: Daniel Bluhm --- aries_cloudagent/admin/tests/test_admin_server.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/aries_cloudagent/admin/tests/test_admin_server.py b/aries_cloudagent/admin/tests/test_admin_server.py index 8f5429dead..6482371c9e 100644 --- a/aries_cloudagent/admin/tests/test_admin_server.py +++ b/aries_cloudagent/admin/tests/test_admin_server.py @@ -485,3 +485,17 @@ async def test_on_record_event(server, event_topic, webhook_topic): ) as mock_send_webhook: await server._on_record_event(profile, Event(event_topic, None)) mock_send_webhook.assert_called_once_with(profile, webhook_topic, None) + + +@pytest.mark.asyncio +async def test_admin_responder_profile_expired_x(): + def _smaller_scope(): + profile = InMemoryProfile.test_profile() + return test_module.AdminResponder(profile, None) + + responder = _smaller_scope() + with pytest.raises(RuntimeError): + await responder.send_outbound(None) + + with pytest.raises(RuntimeError): + await responder.send_webhook("test", {}) From e84b3f826db44377d8ed2190dbae44387e297819 Mon Sep 17 00:00:00 2001 From: Daniel Bluhm Date: Tue, 3 May 2022 11:44:07 -0400 Subject: [PATCH 25/28] feat: askar profiles don't need finalizers Signed-off-by: Daniel Bluhm --- aries_cloudagent/askar/profile.py | 22 ++----------------- aries_cloudagent/askar/tests/test_profile.py | 11 ---------- aries_cloudagent/core/profile.py | 13 ----------- aries_cloudagent/indy/sdk/profile.py | 9 ++++++-- .../indy/sdk/tests/test_profile.py | 2 +- aries_cloudagent/multitenant/cache.py | 5 +++-- .../multitenant/tests/test_cache.py | 3 --- 7 files changed, 13 insertions(+), 52 deletions(-) diff --git a/aries_cloudagent/askar/profile.py b/aries_cloudagent/askar/profile.py index a03c7bbd3e..d992f845df 100644 --- a/aries_cloudagent/askar/profile.py +++ b/aries_cloudagent/askar/profile.py @@ -6,8 +6,8 @@ # import traceback -from typing import Any, Mapping, Optional -from weakref import finalize, ref +from typing import Any, Mapping +from weakref import ref from aries_askar import AskarError, Session, Store @@ -153,24 +153,6 @@ async def close(self): await self.opened.close() self.opened = None - def finalizer(self) -> Optional[finalize]: - """Return a finalizer for this profile. - - See docs for weakref.finalize for more details on behavior of finalizers. - """ - # Askar Profiles (not to be confused with AskarProfiles) don't need - # additional clean up - - if self.profile_id: - return None - - def _finalize(opened: Optional[AskarOpenStore]): - if opened: - LOGGER.debug("Profile finalizer called; closing wallet") - asyncio.get_event_loop().create_task(opened.close()) - - return finalize(self, _finalize, self.opened) - class AskarProfileSession(ProfileSession): """An active connection to the profile management backend.""" diff --git a/aries_cloudagent/askar/tests/test_profile.py b/aries_cloudagent/askar/tests/test_profile.py index cc484b109b..7855ae1188 100644 --- a/aries_cloudagent/askar/tests/test_profile.py +++ b/aries_cloudagent/askar/tests/test_profile.py @@ -83,14 +83,3 @@ async def test_profile_manager_store(): assert sessionProfile._opener == askar_profile_session askar_profile.store.session.assert_called_once_with(profile) - - -def test_finalizer(open_store, caplog): - def _smaller_scope(): - askar_profile = AskarProfile(open_store) - askar_profile.finalizer() - - with caplog.at_level(logging.DEBUG): - _smaller_scope() - - assert "finalizer called" in caplog.text diff --git a/aries_cloudagent/core/profile.py b/aries_cloudagent/core/profile.py index c8682aa222..e5de864d6e 100644 --- a/aries_cloudagent/core/profile.py +++ b/aries_cloudagent/core/profile.py @@ -4,7 +4,6 @@ from abc import ABC, abstractmethod from typing import Any, Mapping, Optional, Type -from weakref import finalize from .event_bus import EventBus, Event from ..config.base import InjectionError @@ -116,18 +115,6 @@ def inject_or( async def close(self): """Close the profile instance.""" - def finalizer(self) -> Optional[finalize]: - """Create and return a finalizer for the profile or None. - - None is returned if no special handling is required to close the profile. - - Finalizers enable automatic clean up of wallet profiles when all references to - the profile expire. - - See docs for weakref.finalize for more details on the behavior of finalizers. - """ - return None - async def remove(self): """Remove the profile.""" diff --git a/aries_cloudagent/indy/sdk/profile.py b/aries_cloudagent/indy/sdk/profile.py index 5034408938..c0d3d4adc2 100644 --- a/aries_cloudagent/indy/sdk/profile.py +++ b/aries_cloudagent/indy/sdk/profile.py @@ -31,13 +31,18 @@ class IndySdkProfile(Profile): BACKEND_NAME = "indy" - def __init__(self, opened: IndyOpenWallet, context: InjectionContext = None): + def __init__( + self, + opened: IndyOpenWallet, + context: InjectionContext = None, + ): """Create a new IndyProfile instance.""" super().__init__(context=context, name=opened.name, created=opened.created) self.opened = opened self.ledger_pool: IndySdkLedgerPool = None self.init_ledger_pool() self.bind_providers() + self._finalizer = self._make_finalizer() @property def name(self) -> str: @@ -117,7 +122,7 @@ async def close(self): await self.opened.close() self.opened = None - def finalizer(self) -> Optional[finalize]: + def _make_finalizer(self) -> finalize: """Return a finalizer for this profile. See docs for weakref.finalize for more details on behavior of finalizers. diff --git a/aries_cloudagent/indy/sdk/tests/test_profile.py b/aries_cloudagent/indy/sdk/tests/test_profile.py index 7372cd1fea..6bd97474e6 100644 --- a/aries_cloudagent/indy/sdk/tests/test_profile.py +++ b/aries_cloudagent/indy/sdk/tests/test_profile.py @@ -71,7 +71,7 @@ def test_read_only(open_wallet): def test_finalizer(open_wallet, caplog): def _smaller_scope(): profile = IndySdkProfile(open_wallet) - profile.finalizer() + assert profile with caplog.at_level(logging.DEBUG): _smaller_scope() diff --git a/aries_cloudagent/multitenant/cache.py b/aries_cloudagent/multitenant/cache.py index 7fb636843b..1fb3f37e3c 100644 --- a/aries_cloudagent/multitenant/cache.py +++ b/aries_cloudagent/multitenant/cache.py @@ -88,8 +88,9 @@ def put(self, key: str, value: Profile) -> None: value (Profile): the profile to set """ - # Close the profile when it falls out of scope - value.finalizer() + # Profiles are responsible for cleaning up after themselves when they + # fall out of scope. Previously the cache needed to create a finalizer. + # value.finalzer() # Keep track of currently opened profiles using weak references self.profiles[key] = value diff --git a/aries_cloudagent/multitenant/tests/test_cache.py b/aries_cloudagent/multitenant/tests/test_cache.py index 7acaaba318..ae7dbcc303 100644 --- a/aries_cloudagent/multitenant/tests/test_cache.py +++ b/aries_cloudagent/multitenant/tests/test_cache.py @@ -10,9 +10,6 @@ def session(self, context=None): def transaction(self, context=None): ... - def finalizer(self): - return None - def test_get_not_in_cache(): cache = ProfileCache(1) From 20d753b83ab7787f60c3fd8d86737979761cba41 Mon Sep 17 00:00:00 2001 From: Daniel Bluhm Date: Thu, 2 Jun 2022 15:24:52 -0400 Subject: [PATCH 26/28] fix: oddities and typos Signed-off-by: Daniel Bluhm --- aries_cloudagent/indy/sdk/profile.py | 15 +++++++-------- .../multitenant/askar_profile_manager.py | 4 ++-- aries_cloudagent/multitenant/base.py | 6 +++--- 3 files changed, 12 insertions(+), 13 deletions(-) diff --git a/aries_cloudagent/indy/sdk/profile.py b/aries_cloudagent/indy/sdk/profile.py index c0d3d4adc2..d8151a5050 100644 --- a/aries_cloudagent/indy/sdk/profile.py +++ b/aries_cloudagent/indy/sdk/profile.py @@ -3,7 +3,7 @@ import asyncio import logging -from typing import Any, Mapping, Optional +from typing import Any, Mapping from weakref import finalize, ref from ...config.injection_context import InjectionContext @@ -42,7 +42,7 @@ def __init__( self.ledger_pool: IndySdkLedgerPool = None self.init_ledger_pool() self.bind_providers() - self._finalizer = self._make_finalizer() + self._finalizer = self._make_finalizer(opened) @property def name(self) -> str: @@ -122,18 +122,17 @@ async def close(self): await self.opened.close() self.opened = None - def _make_finalizer(self) -> finalize: + def _make_finalizer(self, opened: IndyOpenWallet) -> finalize: """Return a finalizer for this profile. See docs for weakref.finalize for more details on behavior of finalizers. """ - def _finalize(opened: Optional[IndyOpenWallet]): - if opened: - LOGGER.debug("Profile finalizer called; closing wallet") - asyncio.get_event_loop().create_task(opened.close()) + def _finalize(opened: IndyOpenWallet): + LOGGER.debug("Profile finalizer called; closing wallet") + asyncio.get_event_loop().create_task(opened.close()) - return finalize(self, _finalize, self.opened) + return finalize(self, _finalize, opened) async def remove(self): """Remove the profile associated with this instance.""" diff --git a/aries_cloudagent/multitenant/askar_profile_manager.py b/aries_cloudagent/multitenant/askar_profile_manager.py index d7d13b964c..83135cfe8e 100644 --- a/aries_cloudagent/multitenant/askar_profile_manager.py +++ b/aries_cloudagent/multitenant/askar_profile_manager.py @@ -14,7 +14,7 @@ class AskarProfileMultitenantManager(BaseMultitenantManager): """Class for handling askar profile multitenancy.""" - DEFAULT_MULTIENANT_WALLET_NAME = "multitenant_sub_wallet" + DEFAULT_MULTITENANT_WALLET_NAME = "multitenant_sub_wallet" def __init__(self, profile: Profile, multitenant_profile: AskarProfile = None): """Initialize askar profile multitenant Manager. @@ -62,7 +62,7 @@ async def get_wallet_profile( """ if not self._multitenant_profile: multitenant_wallet_name = base_context.settings.get( - "multitenant.wallet_name", self.DEFAULT_MULTIENANT_WALLET_NAME + "multitenant.wallet_name", self.DEFAULT_MULTITENANT_WALLET_NAME ) context = base_context.copy() sub_wallet_settings = { diff --git a/aries_cloudagent/multitenant/base.py b/aries_cloudagent/multitenant/base.py index b2c03afe65..6c1a0fb13e 100644 --- a/aries_cloudagent/multitenant/base.py +++ b/aries_cloudagent/multitenant/base.py @@ -2,7 +2,7 @@ from datetime import datetime import logging -from abc import abstractmethod, ABC, abstractproperty +from abc import abstractmethod, ABC import jwt from typing import Iterable, List, Optional, cast @@ -48,10 +48,10 @@ def __init__(self, profile: Profile): if not profile: raise MultitenantManagerError("Missing profile") - @abstractproperty + @property + @abstractmethod def open_profiles(self) -> Iterable[Profile]: """Return iterator over open profiles.""" - ... async def get_default_mediator(self) -> Optional[MediationRecord]: """Retrieve the default mediator used for subwallet routing. From da236fd34813171190c46f6c02e5ca7ad5cf98ac Mon Sep 17 00:00:00 2001 From: Daniel Bluhm Date: Thu, 2 Jun 2022 15:30:20 -0400 Subject: [PATCH 27/28] fix: add key_derivation_method help text back in Signed-off-by: Daniel Bluhm --- aries_cloudagent/config/argparse.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/aries_cloudagent/config/argparse.py b/aries_cloudagent/config/argparse.py index a00c049f38..510d3b54c7 100644 --- a/aries_cloudagent/config/argparse.py +++ b/aries_cloudagent/config/argparse.py @@ -1626,8 +1626,9 @@ def add_arguments(self, parser: ArgumentParser): help=( "Specify multitenancy configuration in key=value pairs. " 'For example: "wallet_type=askar-profile wallet_name=askar-profile-name" ' - "Possible values: wallet_name, wallet_key, cache_size. " - '"wallet_name" is only used when "wallet_type" is "askar-profile"' + "Possible values: wallet_name, wallet_key, cache_size, " + 'key_derivation_method. "wallet_name" is only used when ' + '"wallet_type" is "askar-profile"' ), ) From 9575d070369cd97d2e06347e0be8aa2623e6faa2 Mon Sep 17 00:00:00 2001 From: Daniel Bluhm Date: Thu, 2 Jun 2022 15:49:39 -0400 Subject: [PATCH 28/28] fix: missing async marks for pytest Signed-off-by: Daniel Bluhm --- aries_cloudagent/askar/tests/test_profile.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/aries_cloudagent/askar/tests/test_profile.py b/aries_cloudagent/askar/tests/test_profile.py index 7855ae1188..aef94fa481 100644 --- a/aries_cloudagent/askar/tests/test_profile.py +++ b/aries_cloudagent/askar/tests/test_profile.py @@ -15,6 +15,7 @@ def open_store(): yield mock.MagicMock() +@pytest.mark.asyncio async def test_init_success(open_store): askar_profile = AskarProfile( open_store, @@ -23,6 +24,7 @@ async def test_init_success(open_store): assert askar_profile.opened == open_store +@pytest.mark.asyncio async def test_remove_success(open_store): openStore = open_store context = InjectionContext() @@ -42,6 +44,7 @@ async def test_remove_success(open_store): openStore.store.remove_profile.assert_called_once_with(profile_id) +@pytest.mark.asyncio async def test_remove_profile_not_removed_if_wallet_type_not_askar_profile(open_store): openStore = open_store context = InjectionContext()