From e164523df12db19c73e2e8d0225baa5821e3805f Mon Sep 17 00:00:00 2001 From: Shaanjot Gill Date: Wed, 29 Mar 2023 10:46:08 -0700 Subject: [PATCH 1/6] impl all requirements Signed-off-by: Shaanjot Gill --- aries_cloudagent/commands/upgrade.py | 176 +++++++++++++++------------ aries_cloudagent/config/argparse.py | 9 ++ aries_cloudagent/core/conductor.py | 40 ++++-- 3 files changed, 137 insertions(+), 88 deletions(-) diff --git a/aries_cloudagent/commands/upgrade.py b/aries_cloudagent/commands/upgrade.py index fb037c6b2e..56bb0e27a9 100644 --- a/aries_cloudagent/commands/upgrade.py +++ b/aries_cloudagent/commands/upgrade.py @@ -5,7 +5,7 @@ from configargparse import ArgumentParser from packaging import version as package_version -from typing import Callable, Sequence, Optional +from typing import Callable, Sequence, Optional, List from ..core.profile import Profile from ..config import argparse as arg @@ -36,23 +36,13 @@ class VersionUpgradeConfig: def __init__(self, config_path: str = None): """Initialize config for use during upgrade process.""" - self.function_map_config = {} + self.function_map_config = UPGRADE_EXISTING_RECORDS_FUNCTION_MAPPING self.upgrade_configs = {} - self.setup_executable_map_config(CONFIG_v7_3) if config_path: self.setup_version_upgrade_config(config_path) else: self.setup_version_upgrade_config(DEFAULT_UPGRADE_CONFIG_PATH) - def setup_executable_map_config(self, config_dict: dict): - """Set ups config with reference to functions mapped to versions.""" - for version, config in config_dict.items(): - self.function_map_config[version] = {} - if "update_existing_function_inst" in config: - self.function_map_config[version][ - "update_existing_function_inst" - ] = config.get("update_existing_function_inst") - def setup_version_upgrade_config(self, path: str): """Set ups config dict from the provided YML file.""" with open(path, "r") as stream: @@ -73,19 +63,20 @@ def setup_version_upgrade_config(self, path: str): "resave_records" ).get("base_exch_record_path") version_config_dict[version]["resave_records"] = recs_list - version_config_dict[version]["update_existing_records"] = ( - provided_config.get("update_existing_records") or False - ) + config_key_set = set(version_config_dict.get(version).keys()) + config_key_set.remove("resave_records") + for executable in config_key_set: + version_config_dict[version][executable] = ( + provided_config.get(executable) or False + ) if version_config_dict == {}: raise UpgradeError(f"No version configs found in {path}") self.upgrade_configs = version_config_dict - def get_update_existing_func(self, ver: str) -> Optional[Callable]: - """Return callable update_existing_records function for specific version.""" - if ver in self.function_map_config: - return self.function_map_config.get(ver).get( - "update_existing_function_inst" - ) + def get_callable(self, executable: str) -> Optional[Callable]: + """Return callable function for executable name.""" + if executable in self.function_map_config: + return self.function_map_config.get(executable) else: return None @@ -95,6 +86,47 @@ def init_argument_parser(parser: ArgumentParser): return arg.load_argument_groups(parser, *arg.group.get_registered(arg.CAT_UPGRADE)) +def get_upgrade_version_list( + from_version: str, + sorted_version_list: Optional[List] = None, + config_path: Optional[str] = None, +) -> List: + if not sorted_version_list and not config_path: + raise UpgradeError( + f"No sorted version list from config or path to config provided." + ) + if not sorted_version_list: + version_upgrade_config_inst = VersionUpgradeConfig(config_path) + upgrade_configs = version_upgrade_config_inst.upgrade_configs + versions_found_in_config = upgrade_configs.keys() + sorted_version_list = sorted( + versions_found_in_config, key=lambda x: package_version.parse(x) + ) + + version_list = [] + for version in sorted_version_list: + if package_version.parse(version) >= package_version.parse(from_version): + version_list.append(version) + return version_list + + +async def add_version_record(profile: Profile, version: str): + async with profile.session() as session: + storage = session.inject(BaseStorage) + version_storage_record = await storage.find_record( + type_filter=RECORD_TYPE_ACAPY_VERSION, tag_query={} + ) + if not version_storage_record: + await storage.add_record( + StorageRecord( + RECORD_TYPE_ACAPY_VERSION, + version, + ) + ) + else: + await storage.update_record(version_storage_record, version, {}) + + async def upgrade(settings: dict): """Perform upgradation steps.""" context_builder = DefaultContextBuilder(settings) @@ -104,7 +136,7 @@ async def upgrade(settings: dict): settings.get("upgrade.config_path") ) upgrade_configs = version_upgrade_config_inst.upgrade_configs - root_profile, public_did = await wallet_config(context) + root_profile, _ = await wallet_config(context) version_storage_record = None upgrade_to_version = f"v{__version__}" versions_found_in_config = upgrade_configs.keys() @@ -136,68 +168,64 @@ async def upgrade(settings: dict): f"{upgrade_from_version} as --from-version from " "the config." ) - if upgrade_from_version == upgrade_to_version: + upgrade_version_in_config = get_upgrade_version_list( + sorted_versions_found_in_config, upgrade_from_version + ) + force_upgrade_flag = root_profile.settings.get("upgrade.force_upgrade") or False + if upgrade_from_version == upgrade_to_version and not force_upgrade_flag: print( f"Version {upgrade_from_version} to upgrade from and " f"current version to upgrade to {upgrade_to_version} " - "are same." + "are same. If you still wish to run upgrade then plese " + " run ACA-Py with --force-upgrade argument." ) else: - if upgrade_from_version not in sorted_versions_found_in_config: - raise UpgradeError( - f"No upgrade configuration found for {upgrade_from_version}" - ) - upgrade_from_version_index = sorted_versions_found_in_config.index( - upgrade_from_version - ) - for config_from_version in sorted_versions_found_in_config[ - upgrade_from_version_index: - ]: + resave_record_path_sets = set() + executables_called = set() + for config_from_version in upgrade_version_in_config: print(f"Running upgrade process for {config_from_version}") upgrade_config = upgrade_configs.get(config_from_version) # Step 1 re-saving all BaseRecord and BaseExchangeRecord if "resave_records" in upgrade_config: resave_record_paths = upgrade_config.get("resave_records") for record_path in resave_record_paths: - try: - rec_type = ClassLoader.load_class(record_path) - except ClassNotFoundError as err: - raise UpgradeError( - f"Unknown Record type {record_path}" - ) from err - if not issubclass(rec_type, BaseRecord): - raise UpgradeError( - f"Only BaseRecord can be resaved, found: {str(rec_type)}" - ) - async with root_profile.session() as session: - all_records = await rec_type.query(session) - for record in all_records: - await record.save( - session, - reason="re-saving record during the upgrade process", - ) - if len(all_records) == 0: - print(f"No records of {str(rec_type)} found") - else: - print( - f"All recs of {str(rec_type)} successfully re-saved" - ) + resave_record_path_sets.add(record_path) + # Step 2 Update existing records, if required - if ( - "update_existing_records" in upgrade_config - and upgrade_config.get("update_existing_records") is True - ): - update_existing_recs_callable = ( - version_upgrade_config_inst.get_update_existing_func( - config_from_version - ) + config_key_set = set(upgrade_config.keys()) + config_key_set.remove("resave_records") + for executable in list(config_key_set): + if ( + upgrade_config.get(executable) is False + or executable in executables_called + ): + continue + + _callable = version_upgrade_config_inst.get_callable(executable) + if not _callable: + raise UpgradeError(f"No function specified for {executable}") + executables_called.add(executable) + await _callable(root_profile) + for record_path in resave_record_path_sets: + try: + rec_type = ClassLoader.load_class(record_path) + except ClassNotFoundError as err: + raise UpgradeError(f"Unknown Record type {record_path}") from err + if not issubclass(rec_type, BaseRecord): + raise UpgradeError( + f"Only BaseRecord can be resaved, found: {str(rec_type)}" ) - if not update_existing_recs_callable: - raise UpgradeError( - "No update_existing_records function " - f"specified for {config_from_version}" + async with root_profile.session() as session: + all_records = await rec_type.query(session) + for record in all_records: + await record.save( + session, + reason="re-saving record during the upgrade process", ) - await update_existing_recs_callable(root_profile) + if len(all_records) == 0: + print(f"No records of {str(rec_type)} found") + else: + print(f"All recs of {str(rec_type)} successfully re-saved") # Update storage version async with root_profile.session() as session: @@ -247,12 +275,8 @@ def main(): execute() -# Update every release -CONFIG_v7_3 = { - "v0.7.2": { - "update_existing_function_inst": update_existing_records, - }, +UPGRADE_EXISTING_RECORDS_FUNCTION_MAPPING = { + "update_existing_records": update_existing_records } - main() diff --git a/aries_cloudagent/config/argparse.py b/aries_cloudagent/config/argparse.py index 2951e6969f..0b1f20b5e4 100644 --- a/aries_cloudagent/config/argparse.py +++ b/aries_cloudagent/config/argparse.py @@ -2053,6 +2053,13 @@ def add_arguments(self, parser: ArgumentParser): ), ) + parser.add_argument( + "--force-upgrade", + action="store_true", + env_var="ACAPY_UPGRADE_FORCE_UPGRADE", + help="Brute force the upgrade process.", + ) + def get_settings(self, args: Namespace) -> dict: """Extract ACA-Py upgrade process settings.""" settings = {} @@ -2060,4 +2067,6 @@ def get_settings(self, args: Namespace) -> dict: settings["upgrade.config_path"] = args.upgrade_config_path if args.from_version: settings["upgrade.from_version"] = args.from_version + if args.force_upgrade: + settings["upgrade.force_upgrade"] = args.force_upgrade return settings diff --git a/aries_cloudagent/core/conductor.py b/aries_cloudagent/core/conductor.py index 1f09dbc13a..1e50a83f6e 100644 --- a/aries_cloudagent/core/conductor.py +++ b/aries_cloudagent/core/conductor.py @@ -26,6 +26,7 @@ from ..config.logging import LoggingConfigurator from ..config.provider import ClassProvider from ..config.wallet import wallet_config +from ..commands.upgrade import get_upgrade_version_list, add_version_record, upgrade from ..core.profile import Profile from ..indy.verifier import IndyVerifier @@ -311,26 +312,41 @@ async def start(self) -> None: ) # record ACA-Py version in Wallet, if needed + from_version = None + agent_version = f"v{__version__}" async with self.root_profile.session() as session: storage: BaseStorage = session.context.inject(BaseStorage) - agent_version = f"v{__version__}" try: record = await storage.find_record( type_filter=RECORD_TYPE_ACAPY_VERSION, tag_query={}, ) - if record.value != agent_version: - LOGGER.exception( - ( - f"Wallet storage version {record.value} " - "does not match this ACA-Py agent " - f"version {agent_version}. Run aca-py " - "upgrade command to fix this." - ) - ) - raise + from_version = record.value except StorageNotFoundError: - pass + LOGGER.exception(("Wallet version storage record not found.")) + from_version = from_version or self.root_profile.settings.get( + "upgrade.config_path" + ) + if from_version: + config_available_list = get_upgrade_version_list( + config_path=self.root_profile.settings.get("upgrade.config_path"), + from_version=from_version, + ) + if len(config_available_list) >= 1 and ( + from_version != agent_version + or self.root_profile.settings.get("upgrade.force_upgrade") + ): + await upgrade(self.root_profile.settings) + else: + LOGGER.exception( + ( + "Wallet storage version not found. " + "Run aca-py upgrade command with " + "--from-version to fix this." + ) + ) + raise + await add_version_record(self.root_profile, agent_version) # Create a static connection for use by the test-suite if context.settings.get("debug.test_suite_endpoint"): From 3e6d8afeb8e8c7a640fb264b1f01bf607a66cb49 Mon Sep 17 00:00:00 2001 From: Shaanjot Gill Date: Wed, 29 Mar 2023 13:33:53 -0700 Subject: [PATCH 2/6] updates based on feedback Signed-off-by: Shaanjot Gill --- aries_cloudagent/commands/upgrade.py | 128 +++++++++++++++++---------- aries_cloudagent/config/argparse.py | 5 +- aries_cloudagent/core/conductor.py | 16 ++-- 3 files changed, 92 insertions(+), 57 deletions(-) diff --git a/aries_cloudagent/commands/upgrade.py b/aries_cloudagent/commands/upgrade.py index 56bb0e27a9..1ceec9564a 100644 --- a/aries_cloudagent/commands/upgrade.py +++ b/aries_cloudagent/commands/upgrade.py @@ -1,6 +1,7 @@ """Upgrade command for handling breaking changes when updating ACA-PY versions.""" import asyncio +import logging import yaml from configargparse import ArgumentParser @@ -25,6 +26,7 @@ DEFAULT_UPGRADE_CONFIG_PATH = ( "./aries_cloudagent/commands/default_version_upgrade_config.yml" ) +LOGGER = logging.getLogger(__name__) class UpgradeError(BaseError): @@ -125,6 +127,7 @@ async def add_version_record(profile: Profile, version: str): ) else: await storage.update_record(version_storage_record, version, {}) + LOGGER.info(f"{RECORD_TYPE_ACAPY_VERSION} storage record set to {version}") async def upgrade(settings: dict): @@ -143,47 +146,68 @@ async def upgrade(settings: dict): sorted_versions_found_in_config = sorted( versions_found_in_config, key=lambda x: package_version.parse(x) ) + upgrade_from_version_config = None + upgrade_from_version_setting = None + upgrade_from_version = None async with root_profile.session() as session: storage = session.inject(BaseStorage) try: version_storage_record = await storage.find_record( type_filter=RECORD_TYPE_ACAPY_VERSION, tag_query={} ) - upgrade_from_version = version_storage_record.value - if "upgrade.from_version" in settings: - print( - ( - f"version {upgrade_from_version} found in storage" - ", --from-version will be ignored." - ) - ) + upgrade_from_version_config = version_storage_record.value except StorageNotFoundError: - if "upgrade.from_version" in settings: - upgrade_from_version = settings.get("upgrade.from_version") - else: - upgrade_from_version = sorted_versions_found_in_config[-1] - print( - "No ACA-Py version found in wallet storage and " - "no --from-version specified. Selecting " - f"{upgrade_from_version} as --from-version from " - "the config." + LOGGER.info("No ACA-Py version found in wallet storage.") + + if "upgrade.from_version" in settings: + upgrade_from_version_setting = settings.get("upgrade.from_version") + LOGGER.info( + ( + f"Selecting {upgrade_from_version_setting} as " + "--from-version from the config." ) + ) + + force_upgrade_flag = root_profile.settings.get("upgrade.force_upgrade") or False + if upgrade_from_version_config and upgrade_from_version_setting: + if ( + package_version.parse(upgrade_from_version_config) + > package_version.parse(upgrade_from_version_setting) + ) and force_upgrade_flag: + upgrade_from_version = upgrade_from_version_setting + else: + upgrade_from_version = upgrade_from_version_config + if ( + not upgrade_from_version + and not upgrade_from_version_config + and upgrade_from_version_setting + ): + upgrade_from_version = upgrade_from_version_setting + if ( + not upgrade_from_version + and upgrade_from_version_config + and not upgrade_from_version_setting + ): + upgrade_from_version = upgrade_from_version_config + upgrade_version_in_config = get_upgrade_version_list( sorted_versions_found_in_config, upgrade_from_version ) - force_upgrade_flag = root_profile.settings.get("upgrade.force_upgrade") or False - if upgrade_from_version == upgrade_to_version and not force_upgrade_flag: - print( - f"Version {upgrade_from_version} to upgrade from and " - f"current version to upgrade to {upgrade_to_version} " - "are same. If you still wish to run upgrade then plese " - " run ACA-Py with --force-upgrade argument." + to_update_flag = False + if upgrade_from_version == upgrade_to_version: + LOGGER.info( + ( + f"Version {upgrade_from_version} to upgrade from and " + f"current version to upgrade to {upgrade_to_version} " + "are same. If you still wish to run upgrade then please " + " run ACA-Py with --force-upgrade argument." + ) ) else: resave_record_path_sets = set() - executables_called = set() + executables_call_set = set() for config_from_version in upgrade_version_in_config: - print(f"Running upgrade process for {config_from_version}") + LOGGER.info(f"Running upgrade process for {config_from_version}") upgrade_config = upgrade_configs.get(config_from_version) # Step 1 re-saving all BaseRecord and BaseExchangeRecord if "resave_records" in upgrade_config: @@ -194,18 +218,13 @@ async def upgrade(settings: dict): # Step 2 Update existing records, if required config_key_set = set(upgrade_config.keys()) config_key_set.remove("resave_records") - for executable in list(config_key_set): - if ( - upgrade_config.get(executable) is False - or executable in executables_called - ): + for callable_name in list(config_key_set): + if upgrade_config.get(callable_name) is False: continue + executables_call_set.add(callable_name) - _callable = version_upgrade_config_inst.get_callable(executable) - if not _callable: - raise UpgradeError(f"No function specified for {executable}") - executables_called.add(executable) - await _callable(root_profile) + if len(resave_record_path_sets) >= 1 or len(executables_call_set) >= 1: + to_update_flag = True for record_path in resave_record_path_sets: try: rec_type = ClassLoader.load_class(record_path) @@ -223,23 +242,34 @@ async def upgrade(settings: dict): reason="re-saving record during the upgrade process", ) if len(all_records) == 0: - print(f"No records of {str(rec_type)} found") + LOGGER.info(f"No records of {str(rec_type)} found") else: - print(f"All recs of {str(rec_type)} successfully re-saved") + LOGGER.info( + f"All recs of {str(rec_type)} successfully re-saved" + ) + for callable_name in executables_call_set: + _callable = version_upgrade_config_inst.get_callable(callable_name) + if not _callable: + raise UpgradeError(f"No function specified for {callable_name}") + await _callable(root_profile) # Update storage version - async with root_profile.session() as session: - storage = session.inject(BaseStorage) - if not version_storage_record: - await storage.add_record( - StorageRecord( - RECORD_TYPE_ACAPY_VERSION, - upgrade_to_version, + if to_update_flag: + async with root_profile.session() as session: + storage = session.inject(BaseStorage) + if not version_storage_record: + await storage.add_record( + StorageRecord( + RECORD_TYPE_ACAPY_VERSION, + upgrade_to_version, + ) ) - ) - else: - await storage.update_record( - version_storage_record, upgrade_to_version, {} + else: + await storage.update_record( + version_storage_record, upgrade_to_version, {} + ) + LOGGER.info( + f"{RECORD_TYPE_ACAPY_VERSION} storage record set to {upgrade_to_version}" ) await root_profile.close() except BaseError as e: diff --git a/aries_cloudagent/config/argparse.py b/aries_cloudagent/config/argparse.py index 0b1f20b5e4..ff26a55071 100644 --- a/aries_cloudagent/config/argparse.py +++ b/aries_cloudagent/config/argparse.py @@ -2057,7 +2057,10 @@ def add_arguments(self, parser: ArgumentParser): "--force-upgrade", action="store_true", env_var="ACAPY_UPGRADE_FORCE_UPGRADE", - help="Brute force the upgrade process.", + help=( + "Forces the '—from-version' argument to override the version retrieved from " + "secure storage when calculating upgrades to be run." + ), ) def get_settings(self, args: Namespace) -> dict: diff --git a/aries_cloudagent/core/conductor.py b/aries_cloudagent/core/conductor.py index 1e50a83f6e..79033974ca 100644 --- a/aries_cloudagent/core/conductor.py +++ b/aries_cloudagent/core/conductor.py @@ -323,7 +323,7 @@ async def start(self) -> None: ) from_version = record.value except StorageNotFoundError: - LOGGER.exception(("Wallet version storage record not found.")) + LOGGER.warning(("Wallet version storage record not found.")) from_version = from_version or self.root_profile.settings.get( "upgrade.config_path" ) @@ -338,15 +338,17 @@ async def start(self) -> None: ): await upgrade(self.root_profile.settings) else: - LOGGER.exception( + LOGGER.warning( ( - "Wallet storage version not found. " - "Run aca-py upgrade command with " - "--from-version to fix this." + "No upgrade from version was found from wallet or via" + " --from-version startup argument. " + f"{RECORD_TYPE_ACAPY_VERSION} storage record will be " + f"set to {agent_version}. You can run run the upgrade " + "command with --from-version and --force-upgrade to " + "upgrade later." ) ) - raise - await add_version_record(self.root_profile, agent_version) + await add_version_record(self.root_profile, agent_version) # Create a static connection for use by the test-suite if context.settings.get("debug.test_suite_endpoint"): From 7ce4ce2719e371fd987c350e5432bd3376d4ef6f Mon Sep 17 00:00:00 2001 From: Shaanjot Gill Date: Wed, 29 Mar 2023 13:40:22 -0700 Subject: [PATCH 3/6] correct logging statement Signed-off-by: Shaanjot Gill --- aries_cloudagent/commands/upgrade.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/aries_cloudagent/commands/upgrade.py b/aries_cloudagent/commands/upgrade.py index 1ceec9564a..61e4c672c3 100644 --- a/aries_cloudagent/commands/upgrade.py +++ b/aries_cloudagent/commands/upgrade.py @@ -199,8 +199,10 @@ async def upgrade(settings: dict): ( f"Version {upgrade_from_version} to upgrade from and " f"current version to upgrade to {upgrade_to_version} " - "are same. If you still wish to run upgrade then please " - " run ACA-Py with --force-upgrade argument." + "are same. You can apply upgrade from a lower " + "version by running the upgrade command with " + f"--from-version [< {upgrade_to_version}] and " + "--force-upgrade" ) ) else: From 7c3959a8b5ca29b7b48fc1793c75a232f4c3871d Mon Sep 17 00:00:00 2001 From: Shaanjot Gill Date: Wed, 29 Mar 2023 16:13:31 -0700 Subject: [PATCH 4/6] fixes Signed-off-by: Shaanjot Gill --- .../commands/tests/test_upgrade.py | 35 ++++------------ aries_cloudagent/commands/upgrade.py | 24 +++++++---- aries_cloudagent/core/conductor.py | 8 +++- aries_cloudagent/core/tests/test_conductor.py | 41 ++----------------- 4 files changed, 34 insertions(+), 74 deletions(-) diff --git a/aries_cloudagent/commands/tests/test_upgrade.py b/aries_cloudagent/commands/tests/test_upgrade.py index 704f4cb43b..258fe8b1bf 100644 --- a/aries_cloudagent/commands/tests/test_upgrade.py +++ b/aries_cloudagent/commands/tests/test_upgrade.py @@ -167,11 +167,13 @@ async def test_upgrade_missing_from_version(self): ), async_mock.patch.object( ConnRecord, "save", async_mock.CoroutineMock() ): - await test_module.upgrade( - { - "upgrade.config_path": "./aries_cloudagent/commands/default_version_upgrade_config.yml", - } - ) + with self.assertRaises(UpgradeError) as ctx: + await test_module.upgrade( + { + "upgrade.config_path": "./aries_cloudagent/commands/default_version_upgrade_config.yml", + } + ) + assert "No upgrade from version found in wallet or" in str(ctx.exception) async def test_upgrade_x_callable_not_set(self): with async_mock.patch.object( @@ -196,7 +198,7 @@ async def test_upgrade_x_callable_not_set(self): }, "update_existing_records": True, }, - "v0.6.0": {"update_existing_records": True}, + "v0.6.0": {"update_existing_records_b": True}, } ), ): @@ -206,7 +208,7 @@ async def test_upgrade_x_callable_not_set(self): "upgrade.from_version": "v0.6.0", } ) - assert "No update_existing_records function specified" in str(ctx.exception) + assert "No function specified for" in str(ctx.exception) async def test_upgrade_x_class_not_found(self): with async_mock.patch.object( @@ -315,25 +317,6 @@ async def test_upgrade_x_invalid_config(self): await test_module.upgrade({}) assert "No version configs found in" in str(ctx.exception) - async def test_upgrade_x_from_version_not_in_config(self): - with async_mock.patch.object( - test_module, - "wallet_config", - async_mock.CoroutineMock( - return_value=( - self.profile, - async_mock.CoroutineMock(did="public DID", verkey="verkey"), - ) - ), - ): - with self.assertRaises(UpgradeError) as ctx: - await test_module.upgrade( - { - "upgrade.from_version": "v1.2.3", - } - ) - assert "No upgrade configuration found for" in str(ctx.exception) - def test_main(self): with async_mock.patch.object( test_module, "__name__", "__main__" diff --git a/aries_cloudagent/commands/upgrade.py b/aries_cloudagent/commands/upgrade.py index 61e4c672c3..22fd3a6ac8 100644 --- a/aries_cloudagent/commands/upgrade.py +++ b/aries_cloudagent/commands/upgrade.py @@ -65,8 +65,11 @@ def setup_version_upgrade_config(self, path: str): "resave_records" ).get("base_exch_record_path") version_config_dict[version]["resave_records"] = recs_list - config_key_set = set(version_config_dict.get(version).keys()) - config_key_set.remove("resave_records") + config_key_set = set(provided_config.keys()) + try: + config_key_set.remove("resave_records") + except KeyError: + pass for executable in config_key_set: version_config_dict[version][executable] = ( provided_config.get(executable) or False @@ -93,10 +96,6 @@ def get_upgrade_version_list( sorted_version_list: Optional[List] = None, config_path: Optional[str] = None, ) -> List: - if not sorted_version_list and not config_path: - raise UpgradeError( - f"No sorted version list from config or path to config provided." - ) if not sorted_version_list: version_upgrade_config_inst = VersionUpgradeConfig(config_path) upgrade_configs = version_upgrade_config_inst.upgrade_configs @@ -189,9 +188,13 @@ async def upgrade(settings: dict): and not upgrade_from_version_setting ): upgrade_from_version = upgrade_from_version_config - + if not upgrade_from_version: + raise UpgradeError( + "No upgrade from version found in wallet or settings [--from-version]" + ) upgrade_version_in_config = get_upgrade_version_list( - sorted_versions_found_in_config, upgrade_from_version + sorted_version_list=sorted_versions_found_in_config, + from_version=upgrade_from_version, ) to_update_flag = False if upgrade_from_version == upgrade_to_version: @@ -219,7 +222,10 @@ async def upgrade(settings: dict): # Step 2 Update existing records, if required config_key_set = set(upgrade_config.keys()) - config_key_set.remove("resave_records") + try: + config_key_set.remove("resave_records") + except KeyError: + pass for callable_name in list(config_key_set): if upgrade_config.get(callable_name) is False: continue diff --git a/aries_cloudagent/core/conductor.py b/aries_cloudagent/core/conductor.py index 79033974ca..3f3816b996 100644 --- a/aries_cloudagent/core/conductor.py +++ b/aries_cloudagent/core/conductor.py @@ -26,7 +26,11 @@ from ..config.logging import LoggingConfigurator from ..config.provider import ClassProvider from ..config.wallet import wallet_config -from ..commands.upgrade import get_upgrade_version_list, add_version_record, upgrade +from ..commands.upgrade import ( + get_upgrade_version_list, + add_version_record, + upgrade, +) from ..core.profile import Profile from ..indy.verifier import IndyVerifier @@ -348,7 +352,7 @@ async def start(self) -> None: "upgrade later." ) ) - await add_version_record(self.root_profile, agent_version) + await add_version_record(profile=self.root_profile, version=agent_version) # Create a static connection for use by the test-suite if context.settings.get("debug.test_suite_endpoint"): diff --git a/aries_cloudagent/core/tests/test_conductor.py b/aries_cloudagent/core/tests/test_conductor.py index d4e73b4a8f..4d373745e0 100644 --- a/aries_cloudagent/core/tests/test_conductor.py +++ b/aries_cloudagent/core/tests/test_conductor.py @@ -1370,43 +1370,6 @@ async def test_setup_ledger_only_base(self): await conductor.setup() mock_genesis_load.assert_called_once() - async def test_startup_x_version_mismatch(self): - builder: ContextBuilder = StubContextBuilder(self.test_settings) - conductor = test_module.Conductor(builder) - - with async_mock.patch.object( - test_module, "InboundTransportManager", autospec=True - ) as mock_inbound_mgr, async_mock.patch.object( - test_module, "OutboundTransportManager", autospec=True - ) as mock_outbound_mgr, async_mock.patch.object( - test_module, "LOGGER" - ) as mock_logger, async_mock.patch.object( - BaseStorage, - "find_record", - async_mock.AsyncMock(return_value=async_mock.MagicMock(value=f"v0.6.0")), - ): - mock_outbound_mgr.return_value.registered_transports = { - "test": async_mock.MagicMock(schemes=["http"]) - } - await conductor.setup() - - session = await conductor.root_profile.session() - - wallet = session.inject(BaseWallet) - await wallet.create_public_did( - SOV, - ED25519, - ) - - mock_inbound_mgr.return_value.setup.assert_awaited_once() - mock_outbound_mgr.return_value.setup.assert_awaited_once() - - mock_inbound_mgr.return_value.registered_transports = {} - mock_outbound_mgr.return_value.registered_transports = {} - with self.assertRaises(RuntimeError): - await conductor.start() - mock_logger.exception.assert_called_once() - async def test_startup_x_no_storage_version(self): builder: ContextBuilder = StubContextBuilder(self.test_settings) conductor = test_module.Conductor(builder) @@ -1421,6 +1384,10 @@ async def test_startup_x_no_storage_version(self): BaseStorage, "find_record", async_mock.AsyncMock(side_effect=StorageNotFoundError()), + ), async_mock.patch.object( + test_module, + "add_version_record", + async_mock.AsyncMock(), ): mock_outbound_mgr.return_value.registered_transports = { "test": async_mock.MagicMock(schemes=["http"]) From d305f538d454c5a24de6e8bbed294ca763cb211f Mon Sep 17 00:00:00 2001 From: Shaanjot Gill Date: Thu, 30 Mar 2023 06:03:07 -0700 Subject: [PATCH 5/6] fixes Signed-off-by: Shaanjot Gill --- aries_cloudagent/commands/upgrade.py | 16 +++++++++++----- aries_cloudagent/config/argparse.py | 5 +++-- 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/aries_cloudagent/commands/upgrade.py b/aries_cloudagent/commands/upgrade.py index 22fd3a6ac8..1d7dec5d61 100644 --- a/aries_cloudagent/commands/upgrade.py +++ b/aries_cloudagent/commands/upgrade.py @@ -96,6 +96,7 @@ def get_upgrade_version_list( sorted_version_list: Optional[List] = None, config_path: Optional[str] = None, ) -> List: + """Get available versions from the upgrade config.""" if not sorted_version_list: version_upgrade_config_inst = VersionUpgradeConfig(config_path) upgrade_configs = version_upgrade_config_inst.upgrade_configs @@ -112,11 +113,15 @@ def get_upgrade_version_list( async def add_version_record(profile: Profile, version: str): + """Add an acapy_version storage record for provided version.""" async with profile.session() as session: storage = session.inject(BaseStorage) - version_storage_record = await storage.find_record( - type_filter=RECORD_TYPE_ACAPY_VERSION, tag_query={} - ) + try: + version_storage_record = await storage.find_record( + type_filter=RECORD_TYPE_ACAPY_VERSION, tag_query={} + ) + except StorageNotFoundError: + version_storage_record = None if not version_storage_record: await storage.add_record( StorageRecord( @@ -139,7 +144,6 @@ async def upgrade(settings: dict): ) upgrade_configs = version_upgrade_config_inst.upgrade_configs root_profile, _ = await wallet_config(context) - version_storage_record = None upgrade_to_version = f"v{__version__}" versions_found_in_config = upgrade_configs.keys() sorted_versions_found_in_config = sorted( @@ -157,6 +161,7 @@ async def upgrade(settings: dict): upgrade_from_version_config = version_storage_record.value except StorageNotFoundError: LOGGER.info("No ACA-Py version found in wallet storage.") + version_storage_record = None if "upgrade.from_version" in settings: upgrade_from_version_setting = settings.get("upgrade.from_version") @@ -277,7 +282,8 @@ async def upgrade(settings: dict): version_storage_record, upgrade_to_version, {} ) LOGGER.info( - f"{RECORD_TYPE_ACAPY_VERSION} storage record set to {upgrade_to_version}" + f"{RECORD_TYPE_ACAPY_VERSION} storage record " + f"set to {upgrade_to_version}" ) await root_profile.close() except BaseError as e: diff --git a/aries_cloudagent/config/argparse.py b/aries_cloudagent/config/argparse.py index ff26a55071..f91f99f592 100644 --- a/aries_cloudagent/config/argparse.py +++ b/aries_cloudagent/config/argparse.py @@ -2058,8 +2058,9 @@ def add_arguments(self, parser: ArgumentParser): action="store_true", env_var="ACAPY_UPGRADE_FORCE_UPGRADE", help=( - "Forces the '—from-version' argument to override the version retrieved from " - "secure storage when calculating upgrades to be run." + "Forces the '—from-version' argument to override the version " + "retrieved from secure storage when calculating upgrades to " + "be run." ), ) From ccce11f2d588e062c6067852575185145a88958b Mon Sep 17 00:00:00 2001 From: Shaanjot Gill Date: Thu, 30 Mar 2023 08:09:39 -0700 Subject: [PATCH 6/6] unit test coverage Signed-off-by: Shaanjot Gill --- .../commands/tests/test_upgrade.py | 83 +++++++++++++++++-- aries_cloudagent/commands/upgrade.py | 2 +- .../config/tests/test_argparse.py | 2 + aries_cloudagent/core/tests/test_conductor.py | 64 +++++++++++++- 4 files changed, 139 insertions(+), 12 deletions(-) diff --git a/aries_cloudagent/commands/tests/test_upgrade.py b/aries_cloudagent/commands/tests/test_upgrade.py index 258fe8b1bf..f57a77b1cc 100644 --- a/aries_cloudagent/commands/tests/test_upgrade.py +++ b/aries_cloudagent/commands/tests/test_upgrade.py @@ -17,10 +17,7 @@ class TestUpgrade(AsyncTestCase): async def setUp(self): self.session = InMemoryProfile.test_session() self.profile = self.session.profile - - self.session_storage = InMemoryProfile.test_session() - self.profile_storage = self.session_storage.profile - self.storage = self.session_storage.inject(BaseStorage) + self.storage = self.session.inject(BaseStorage) record = StorageRecord( "acapy_version", "v0.7.2", @@ -37,7 +34,7 @@ async def test_upgrade_storage_from_version_included(self): "wallet_config", async_mock.CoroutineMock( return_value=( - self.profile_storage, + self.profile, async_mock.CoroutineMock(did="public DID", verkey="verkey"), ) ), @@ -61,7 +58,7 @@ async def test_upgrade_storage_missing_from_version(self): "wallet_config", async_mock.CoroutineMock( return_value=( - self.profile_storage, + self.profile, async_mock.CoroutineMock(did="public DID", verkey="verkey"), ) ), @@ -98,6 +95,10 @@ async def test_upgrade_from_version(self): ) async def test_upgrade_callable(self): + version_storage_record = await self.storage.find_record( + type_filter="acapy_version", tag_query={} + ) + await self.storage.delete_record(version_storage_record) with async_mock.patch.object( test_module, "wallet_config", @@ -139,7 +140,7 @@ async def test_upgrade_x_same_version(self): "wallet_config", async_mock.CoroutineMock( return_value=( - self.profile_storage, + self.profile, async_mock.CoroutineMock(did="public DID", verkey="verkey"), ) ), @@ -151,6 +152,10 @@ async def test_upgrade_x_same_version(self): ) async def test_upgrade_missing_from_version(self): + version_storage_record = await self.storage.find_record( + type_filter="acapy_version", tag_query={} + ) + await self.storage.delete_record(version_storage_record) with async_mock.patch.object( test_module, "wallet_config", @@ -176,6 +181,10 @@ async def test_upgrade_missing_from_version(self): assert "No upgrade from version found in wallet or" in str(ctx.exception) async def test_upgrade_x_callable_not_set(self): + version_storage_record = await self.storage.find_record( + type_filter="acapy_version", tag_query={} + ) + await self.storage.delete_record(version_storage_record) with async_mock.patch.object( test_module, "wallet_config", @@ -270,7 +279,8 @@ async def test_execute(self): "--upgrade-config", "./aries_cloudagent/config/tests/test-acapy-upgrade-config.yaml", "--from-version", - "v0.7.2", + "v0.7.0", + "--force-upgrade", ] ) @@ -307,6 +317,63 @@ async def test_upgrade_x_invalid_record_type(self): ) assert "Only BaseRecord can be resaved" in str(ctx.exception) + async def test_upgrade_force(self): + with async_mock.patch.object( + test_module, + "wallet_config", + async_mock.CoroutineMock( + return_value=( + self.profile, + async_mock.CoroutineMock(did="public DID", verkey="verkey"), + ) + ), + ), async_mock.patch.object( + test_module.yaml, + "safe_load", + async_mock.MagicMock( + return_value={ + "v0.7.2": { + "resave_records": { + "base_record_path": [ + "aries_cloudagent.connections.models.conn_record.ConnRecord" + ], + }, + "update_existing_records": True, + }, + "v0.7.3": { + "update_existing_records": True, + }, + "v0.7.1": { + "update_existing_records": False, + }, + } + ), + ): + await test_module.upgrade( + { + "upgrade.from_version": "v0.7.0", + "upgrade.force_upgrade": True, + } + ) + + async def test_get_upgrade_version_list(self): + assert len(test_module.get_upgrade_version_list(from_version="v0.7.2")) >= 1 + + async def test_add_version_record(self): + await test_module.add_version_record(self.profile, "v0.7.4") + version_storage_record = await self.storage.find_record( + type_filter="acapy_version", tag_query={} + ) + assert version_storage_record.value == "v0.7.4" + await self.storage.delete_record(version_storage_record) + with self.assertRaises(test_module.StorageNotFoundError): + await self.storage.find_record(type_filter="acapy_version", tag_query={}) + await test_module.add_version_record(self.profile, "v0.7.5") + version_storage_record = await self.storage.find_record( + type_filter="acapy_version", tag_query={} + ) + assert version_storage_record.value == "v0.7.5" + async def test_upgrade_x_invalid_config(self): with async_mock.patch.object( test_module.yaml, diff --git a/aries_cloudagent/commands/upgrade.py b/aries_cloudagent/commands/upgrade.py index 1d7dec5d61..be98d043ab 100644 --- a/aries_cloudagent/commands/upgrade.py +++ b/aries_cloudagent/commands/upgrade.py @@ -172,7 +172,7 @@ async def upgrade(settings: dict): ) ) - force_upgrade_flag = root_profile.settings.get("upgrade.force_upgrade") or False + force_upgrade_flag = settings.get("upgrade.force_upgrade") or False if upgrade_from_version_config and upgrade_from_version_setting: if ( package_version.parse(upgrade_from_version_config) diff --git a/aries_cloudagent/config/tests/test_argparse.py b/aries_cloudagent/config/tests/test_argparse.py index e9303b8987..044bb217ad 100644 --- a/aries_cloudagent/config/tests/test_argparse.py +++ b/aries_cloudagent/config/tests/test_argparse.py @@ -111,6 +111,7 @@ async def test_upgrade_config(self): "./aries_cloudagent/config/tests/test-acapy-upgrade-config.yml", "--from-version", "v0.7.2", + "--force-upgrade", ] ) @@ -118,6 +119,7 @@ async def test_upgrade_config(self): result.upgrade_config_path == "./aries_cloudagent/config/tests/test-acapy-upgrade-config.yml" ) + assert result.force_upgrade is True settings = group.get_settings(result) diff --git a/aries_cloudagent/core/tests/test_conductor.py b/aries_cloudagent/core/tests/test_conductor.py index 4d373745e0..5ce750cb4a 100644 --- a/aries_cloudagent/core/tests/test_conductor.py +++ b/aries_cloudagent/core/tests/test_conductor.py @@ -101,7 +101,7 @@ async def build_context(self) -> InjectionContext: class TestConductor(IsolatedAsyncioTestCase, Config, TestDIDs): - async def test_startup(self): + async def test_startup_version_record_exists(self): builder: ContextBuilder = StubContextBuilder(self.test_settings) conductor = test_module.Conductor(builder) @@ -114,9 +114,67 @@ async def test_startup(self): ) as mock_logger, async_mock.patch.object( BaseStorage, "find_record", - async_mock.AsyncMock( - return_value=async_mock.MagicMock(value=f"v{__version__}") + async_mock.AsyncMock(return_value=async_mock.MagicMock(value="v0.7.3")), + ), async_mock.patch.object( + test_module, + "get_upgrade_version_list", + async_mock.MagicMock( + return_value=["v0.7.4", "0.7.5", "v0.8.0rc1", "v8.0.0"] ), + ), async_mock.patch.object( + test_module, + "upgrade", + async_mock.AsyncMock(), + ): + mock_outbound_mgr.return_value.registered_transports = { + "test": async_mock.MagicMock(schemes=["http"]) + } + await conductor.setup() + + session = await conductor.root_profile.session() + + wallet = session.inject(BaseWallet) + await wallet.create_public_did( + SOV, + ED25519, + ) + + mock_inbound_mgr.return_value.setup.assert_awaited_once() + mock_outbound_mgr.return_value.setup.assert_awaited_once() + + mock_inbound_mgr.return_value.registered_transports = {} + mock_outbound_mgr.return_value.registered_transports = {} + + await conductor.start() + + mock_inbound_mgr.return_value.start.assert_awaited_once_with() + mock_outbound_mgr.return_value.start.assert_awaited_once_with() + + mock_logger.print_banner.assert_called_once() + + await conductor.stop() + + mock_inbound_mgr.return_value.stop.assert_awaited_once_with() + mock_outbound_mgr.return_value.stop.assert_awaited_once_with() + + async def test_startup_version_record_not_exists(self): + builder: ContextBuilder = StubContextBuilder(self.test_settings) + conductor = test_module.Conductor(builder) + + with async_mock.patch.object( + test_module, "InboundTransportManager", autospec=True + ) as mock_inbound_mgr, async_mock.patch.object( + test_module, "OutboundTransportManager", autospec=True + ) as mock_outbound_mgr, async_mock.patch.object( + test_module, "LoggingConfigurator", autospec=True + ) as mock_logger, async_mock.patch.object( + BaseStorage, + "find_record", + async_mock.AsyncMock(side_effect=StorageNotFoundError()), + ), async_mock.patch.object( + test_module, + "add_version_record", + async_mock.AsyncMock(), ): mock_outbound_mgr.return_value.registered_transports = { "test": async_mock.MagicMock(schemes=["http"])