From cf87d461d3feb170213f6118bb81270b5b2a137b Mon Sep 17 00:00:00 2001 From: David Feltell Date: Tue, 17 Sep 2024 23:10:58 +0100 Subject: [PATCH] [Core] Add `"library_json"` setting Closes #51. Presently, BAL can only be configured using a JSON library file. This causes several problems with the authoring of automated tests within host integrations: * It is difficult to single-source inputs and expected values without authoring a JSON file in the test case. * It requires additional side-car data files to accompany the tests which may not be desirable. So add a new setting, `"library_json"`. This allows hosts to query for the current internal BAL library via `settings()`, and then mutate it and update BAL via a re-`initialize()`. The JSON must be passed from/to the manager in serialised form, since the settings dictionary must be coerced to/from a (C++) `InfoDictionary`, which is a simple key-value map, rather than an arbitrarily nested dictionary. In addition, calculated `"variables"` (e.g. `"bal_library_dir_url"`) cannot be provided when the library is given as a JSON string, since they are based on the location of the `library_path`. If both `library_path` and `library_json` are provided, then `library_json` takes precedence. This allows a JSON library file to be provided initially, then the library mutated during test cases. Recall that the OpenAssetIO `initialize` method can accept partial settings updates, meaning the manager retains any previous settings that are not explicitly overridden in the new settings dict. However, this does not apply to the special `library_json` setting here. This is a bit of an abuse for convenience. E.g. if we re-initialize with an empty settings dict, the initialization process falls back to using the previously-set `library_path` (if available) and resets the library to match the file. This compromise is somewhat justified by analogy to current behaviour when BAL is re-initialised after entities have been published. In this case the new entities only exist in memory, and are lost when re-initialising, since the library JSON file is re-read and overwrites any changes. This is similar to how the `library_json` changes are lost by default when re-initializing. Signed-off-by: David Feltell --- RELEASE_NOTES.md | 11 ++ .../BasicAssetLibraryInterface.py | 53 +++-- plugin/openassetio_manager_bal/bal.py | 9 +- tests/bal_business_logic_suite.py | 185 ++++++++++++++++++ tests/fixtures.py | 1 + 5 files changed, 243 insertions(+), 16 deletions(-) diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index 445c100..3c1d764 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -1,6 +1,17 @@ Release Notes ============= +v1.0.0-beta.x.y +--------------- + +### New features + +- Added new setting `"library_json"` to provide easier control of BAL's + internal library database when used in host test suites. It is a string + value presenting BAL's in-memory library as serialised JSON. + [#51](https://github.com/OpenAssetIO/OpenAssetIO-Manager-BAL/issues/51) + + v1.0.0-beta.1.0 --------------- diff --git a/plugin/openassetio_manager_bal/BasicAssetLibraryInterface.py b/plugin/openassetio_manager_bal/BasicAssetLibraryInterface.py index 93fa5dc..e2298ad 100644 --- a/plugin/openassetio_manager_bal/BasicAssetLibraryInterface.py +++ b/plugin/openassetio_manager_bal/BasicAssetLibraryInterface.py @@ -19,6 +19,7 @@ """ A single-class module, providing the BasicAssetLibraryInterface class. """ +import json import os import re import time @@ -57,6 +58,7 @@ DEFAULT_IDENTIFIER = "org.openassetio.examples.manager.bal" ENV_VAR_IDENTIFIER_OVERRIDE = "OPENASSETIO_BAL_IDENTIFIER" SETTINGS_KEY_LIBRARY_PATH = "library_path" +SETTINGS_KEY_LIBRARY_JSON = "library_json" SETTINGS_KEY_SIMULATED_QUERY_LATENCY = "simulated_query_latency_ms" SETTINGS_KEY_ENTITY_REFERENCE_URL_SCHEME = "entity_reference_url_scheme" @@ -118,7 +120,9 @@ def info(self): return {constants.kInfoKey_EntityReferencesMatchPrefix: self.__entity_refrence_prefix()} def settings(self, hostSession): - return self.__settings.copy() + augmented_settings = self.__settings.copy() + augmented_settings[SETTINGS_KEY_LIBRARY_JSON] = json.dumps(self.__library) + return augmented_settings def hasCapability(self, capability): """ @@ -168,37 +172,44 @@ def simulated_latency(self): def initialize(self, managerSettings, hostSession): self.__validate_settings(managerSettings) - + logger = hostSession.logger() # Settings updates can be partial, so make sure we keep any # existing path. existing_library_path = self.__settings.get("library_path") library_path = managerSettings.get("library_path", existing_library_path) if not library_path: - hostSession.logger().log( - hostSession.logger().Severity.kDebug, + logger.log( + logger.Severity.kDebug, "'library_path' not in settings or is empty, checking " f"{self.__lib_path_envvar_name}", ) - library_path = os.environ.get(self.__lib_path_envvar_name) + library_path = os.environ.get(self.__lib_path_envvar_name, "") - if not library_path: + # Pop from dictionary so it doesn't get merged into persistent + # settings, since the library will be serialised on-demand in + # `settings()`. + library_json = managerSettings.pop("library_json", None) + + if not library_path and not library_json: raise ConfigurationException( - f"'library_path'/{self.__lib_path_envvar_name} not set or is empty" + f"'library_json'/'library_path'/{self.__lib_path_envvar_name} not set or is empty" ) self.__settings.update(managerSettings) self.__settings["library_path"] = library_path - self.__library = {} - hostSession.logger().log( - hostSession.logger().Severity.kDebug, - f"Loading library from '{library_path}'", - ) - self.__library = bal.load_library(library_path) + if library_json is not None: + if logger.isSeverityLogged(logger.Severity.kDebug): + logger.log(logger.Severity.kDebug, f"Parsing library from '{library_json}'") + self.__library = bal.parse_library(library_json) - hostSession.logger().log( - hostSession.logger().Severity.kDebug, + else: + logger.log(logger.Severity.kDebug, f"Loading library from '{library_path}'") + self.__library = bal.load_library(library_path) + + logger.log( + logger.Severity.kDebug, f"Running with simulated query latency of " f"{self.__settings[SETTINGS_KEY_SIMULATED_QUERY_LATENCY]}ms", ) @@ -799,6 +810,7 @@ def __handle_exception(exc, idx, error_callback): def __make_default_settings() -> dict: """ Generates a default settings dict for BAL. + Note: as a library is required, the default settings are not enough to initialize the manager. """ @@ -814,10 +826,21 @@ def __validate_settings(settings: dict): Parses the supplied settings dict, raising if there are any unrecognized keys present. """ + # pylint: disable=too-many-branches if SETTINGS_KEY_LIBRARY_PATH in settings: if not isinstance(settings[SETTINGS_KEY_LIBRARY_PATH], str): raise ValueError(f"{SETTINGS_KEY_LIBRARY_PATH} must be a str") + if SETTINGS_KEY_LIBRARY_JSON in settings: + if not isinstance(settings[SETTINGS_KEY_LIBRARY_JSON], str): + raise ValueError(f"{SETTINGS_KEY_LIBRARY_JSON} must be a str") + try: + json.loads(settings[SETTINGS_KEY_LIBRARY_JSON]) + except json.decoder.JSONDecodeError as err: + raise ValueError( + f"{SETTINGS_KEY_LIBRARY_JSON} must be a valid JSON string" + ) from err + if SETTINGS_KEY_SIMULATED_QUERY_LATENCY in settings: query_latency = settings[SETTINGS_KEY_SIMULATED_QUERY_LATENCY] # This bool check is because bools are also ints as far as diff --git a/plugin/openassetio_manager_bal/bal.py b/plugin/openassetio_manager_bal/bal.py index 921bb68..fe74fe0 100644 --- a/plugin/openassetio_manager_bal/bal.py +++ b/plugin/openassetio_manager_bal/bal.py @@ -98,6 +98,13 @@ def load_library(path: str) -> dict: return library +def parse_library(library_json: str): + """ + Parse the library from a JSON string. + """ + return json.loads(library_json) + + def exists(entity_info: EntityInfo, library: dict) -> bool: """ Determines if the supplied entity exists in the library @@ -358,7 +365,7 @@ def _copy_and_expand_trait_properties(entity_version_dict: dict, library: dict) # append the other vars as kwarg. Fortunately this has # exactly the precedence behaviour we want. trait_data[prop] = string.Template(value).safe_substitute( - os.environ, **library["variables"] + os.environ, **library.get("variables", {}) ) subbed_val = trait_data[prop] diff --git a/tests/bal_business_logic_suite.py b/tests/bal_business_logic_suite.py index 12668a6..12f4277 100644 --- a/tests/bal_business_logic_suite.py +++ b/tests/bal_business_logic_suite.py @@ -21,6 +21,7 @@ # pylint: disable=invalid-name, missing-function-docstring, missing-class-docstring, # pylint: disable=too-few-public-methods,too-many-lines +import json import operator import os import pathlib @@ -85,6 +86,10 @@ def setUp(self): "resources", self._library, ) + # library_json takes precedence, so remove library_json to + # ensure library_path is used. + del new_settings["library_json"] + self.addCleanup(self.cleanUp) self._manager.initialize(new_settings) @@ -225,6 +230,186 @@ def initialize_and_assert_scheme(self, scheme=None): self.assertTrue(str(published_refs[0]).startswith(prefix)) +class Test_initialize_library_as_json_string(LibraryOverrideTestCase): + # Override library just to ensure the cleanup step gets added, + # restoring the library back to its original state. See base class. + _library = "library_apiComplianceSuite.json" + + def test_when_library_loaded_from_file_then_library_setting_contains_file_contents(self): + settings = self._manager.settings() + library_path = pathlib.Path(settings["library_path"]) + expected_library = json.loads(library_path.read_text(encoding="utf-8")) + actual_library = json.loads(settings["library_json"]) + + # For simplicity, strip dynamically calculated values. + del actual_library["variables"] + self.assertDictEqual(expected_library, actual_library) + + def test_when_library_json_updated_then_settings_updated(self): + expected_library = {"managementPolicy": {"read": {"default": {"some.policy": {}}}}} + + self._manager.initialize({"library_json": json.dumps(expected_library)}) + + actual_library = json.loads(self._manager.settings()["library_json"]) + + self.assertDictEqual(actual_library, expected_library) + + def test_when_library_json_is_invalid_primitive_value_then_raises(self): + with self.assertRaises(ValueError) as err: + self._manager.initialize({"library_json": ""}) + + self.assertEqual("library_json must be a valid JSON string", str(err.exception)) + + def test_when_library_json_is_invalid_object_then_raises(self): + with self.assertRaises(TypeError) as err: + self._manager.initialize({"library_json": {"variables": {"a": "b"}}}) + + # Error comes from pybind11 trying to coerce dict. + self.assertIn("incompatible function arguments", str(err.exception)) + + def test_when_library_json_provided_and_library_path_blank_then_settings_updated(self): + # Test to ensure we don't error on a blank library_path if + # library_json is given + + expected_library = {"managementPolicy": {"read": {"default": {"some.policy": {}}}}} + + self._manager.initialize( + {"library_json": json.dumps(expected_library), "library_path": ""} + ) + + actual_library = json.loads(self._manager.settings()["library_json"]) + + self.assertDictEqual(actual_library, expected_library) + + def test_when_no_library_json_and_library_path_blank_then_raises(self): + expected_error = "'library_json'/'library_path'/BAL_LIBRARY_PATH not set or is empty" + + with self.assertRaises(ConfigurationException) as exc: + self._manager.initialize({"library_path": ""}) + + self.assertEqual(str(exc.exception), expected_error) + + def test_when_library_provided_as_json_and_as_file_then_json_takes_precedence(self): + library_path = self._manager.settings()["library_path"] + self.assertGreater(len(library_path), 0) # Confidence check. + expected_library = {"variables": {"a": "b"}} + + self._manager.initialize( + {"library_json": json.dumps(expected_library), "library_path": library_path} + ) + actual_library = json.loads(self._manager.settings()["library_json"]) + + self.assertDictEqual(expected_library, actual_library) + + def test_when_initialised_with_no_library_json_then_resets_to_library_file(self): + # Read in initial library file. + library_path = pathlib.Path(self._manager.settings()["library_path"]) + expected_library = json.loads(library_path.read_text(encoding="utf-8")) + self.assertGreater(len(expected_library), 0) # Confidence check. + + # Mutate library (to empty dict). + self._manager.initialize({"library_json": "{}"}) + self.assertEqual("{}", self._manager.settings()["library_json"]) # Confidence check. + + # Re-`initialize` with an empty settings dict, triggering a + # reset of the library to use the previous `library_path` file. + self._manager.initialize({}) + + actual_library = json.loads(self._manager.settings()["library_json"]) + + # For simplicity, strip dynamically calculated values. + del actual_library["variables"] + self.assertDictEqual(expected_library, actual_library) + + def test_when_in_memory_library_is_updated_then_library_json_is_updated(self): + # Publish a new entity that is not in the initial JSON library. + # This will mutate BAL's in-memory library. + self._manager.register( + self._manager.createEntityReference("bal:///new_entity"), + TraitsData(), + PublishingAccess.kWrite, + self.createTestContext(), + ) + + library = json.loads(self._manager.settings()["library_json"]) + + self.assertIn("new_entity", library["entities"]) + + def test_when_library_uses_undefined_substitution_variables_then_variables_not_substituted( + self, + ): + # Test illustrating that implicit variables for interpolation + # are not available when library is given as a JSON string, + # unlike for library files. + + # setup + + expected_library_json = json.dumps( + { + "entities": { + "some_entity": { + "versions": [ + {"traits": {"some.trait": {"some_key": "${bal_library_path}"}}} + ] + } + } + } + ) + + # action + + self._manager.initialize({"library_json": expected_library_json}) + + # confirm + + traits_data = self._manager.resolve( + self._manager.createEntityReference("bal:///some_entity"), + {"some.trait"}, + ResolveAccess.kRead, + self.createTestContext(), + ) + self.assertEqual( + traits_data.getTraitProperty("some.trait", "some_key"), "${bal_library_path}" + ) + + def test_when_library_uses_defined_substitution_variables_then_variables_are_substituted( + self, + ): + # Test illustrating that variables for interpolation must be + # explicitly provided when library is given as a JSON string. + # I.e. there are no implicit variables, unlike when the library + # is given as a JSON file. + + # setup + + expected_library_json = json.dumps( + { + "variables": {"bal_library_path": "/some/path"}, + "entities": { + "some_entity": { + "versions": [ + {"traits": {"some.trait": {"some_key": "${bal_library_path}"}}} + ] + } + }, + } + ) + + # action + + self._manager.initialize({"library_json": expected_library_json}) + + # confirm + + traits_data = self._manager.resolve( + self._manager.createEntityReference("bal:///some_entity"), + {"some.trait"}, + ResolveAccess.kRead, + self.createTestContext(), + ) + self.assertEqual(traits_data.getTraitProperty("some.trait", "some_key"), "/some/path") + + class Test_managementPolicy_missing_completely(LibraryOverrideTestCase): """ Tests error case when BAL library managementPolicy is missing. diff --git a/tests/fixtures.py b/tests/fixtures.py index 4ba56de..f88dcb9 100644 --- a/tests/fixtures.py +++ b/tests/fixtures.py @@ -101,6 +101,7 @@ "test_when_settings_have_all_keys_then_all_settings_updated": { "some_settings_with_all_keys": { "library_path": blank_library_path, + "library_json": json.dumps({"variables": {"a": "b"}}), "simulated_query_latency_ms": 0, "entity_reference_url_scheme": "thingy", }