From 937d653dd399c8fe7b2a9151d12051680e730923 Mon Sep 17 00:00:00 2001 From: David Feltell Date: Fri, 9 May 2025 12:18:48 +0100 Subject: [PATCH] [Core] Add relationship access and version overrides Part of testing TheFoundryVisionmongers/KatanaOpenAssetIO#4. BAL would error when performing relationship queries with a non-`kRead` access mode. However, such queries are valid for publishing workflows and so must be supported in order to test host applications. So add an optional `"access"` flag that can be added to relationship definitions in the JSON database, overriding the default `kRead` access, and limiting that relationship to specific access modes. Hence no longer error when responding to relationship queries for non-`kRead` access modes (the default response instead being an empty set of entity references). Additionally, if a version relationship trait set is specified in the JSON database, then prefer that over the built-in versioning behaviour. A long term goal would be to remove the built-in versioning behaviour in favour of leveraging generic relationship queries (#88). This at least gets us part way there to support some use cases. Signed-off-by: David Feltell --- RELEASE_NOTES.md | 4 + .../BasicAssetLibraryInterface.py | 30 ++--- plugin/openassetio_manager_bal/bal.py | 4 + schema.json | 3 + tests/bal_business_logic_suite.py | 115 +++++++++++------- .../resources/library_apiComplianceSuite.json | 13 ++ 6 files changed, 103 insertions(+), 66 deletions(-) diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index 3c1d764..042b374 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -11,6 +11,10 @@ v1.0.0-beta.x.y value presenting BAL's in-memory library as serialised JSON. [#51](https://github.com/OpenAssetIO/OpenAssetIO-Manager-BAL/issues/51) +- Added support for access mode specific relationships and overriding + the special-case version relationship in the JSON database. Querying a + relationship with a non-`kRead` access mode is no longer an error. + [KatanaOpenAssetIO#4](https://github.com/TheFoundryVisionmongers/KatanaOpenAssetIO/pull/4) v1.0.0-beta.1.0 --------------- diff --git a/plugin/openassetio_manager_bal/BasicAssetLibraryInterface.py b/plugin/openassetio_manager_bal/BasicAssetLibraryInterface.py index e2298ad..f326dc5 100644 --- a/plugin/openassetio_manager_bal/BasicAssetLibraryInterface.py +++ b/plugin/openassetio_manager_bal/BasicAssetLibraryInterface.py @@ -33,7 +33,6 @@ ResolveAccess, EntityTraitsAccess, PublishingAccess, - RelationsAccess, kAccessNames, ) from openassetio.errors import BatchElementError, ConfigurationException @@ -532,14 +531,6 @@ def getWithRelationship( ) return - if not self.__validate_access( - "relationship query", - (RelationsAccess.kRead,), - access, - entityReferences, - errorCallback, - ): - return for idx, entity_ref in enumerate(entityReferences): try: entity_info = self.__parse_entity_ref(entity_ref.toString(), access) @@ -584,14 +575,6 @@ def getWithRelationships( ) return - if not self.__validate_access( - "relationship query", - (RelationsAccess.kRead,), - access, - relationshipTraitsDatas, - errorCallback, - ): - return for idx, relationship in enumerate(relationshipTraitsDatas): try: entity_info = self.__parse_entity_ref(entityReference.toString(), access) @@ -616,6 +599,15 @@ def __get_relations(self, entity_info, relationship_traits_data, result_trait_se """ relationship_trait_set = relationship_traits_data.traitSet() + # Prefer relations defined in the library before going on to + # query versions, in case versioning relationship is overridden + # explicitly in the library. + relations_from_library = self.__get_relations_from_library( + entity_info, relationship_traits_data, result_trait_set + ) + if relations_from_library: + return relations_from_library + # We don't use issuperset as otherwise we'd end up responding to # any more specialized relationship definitions that may be # added in the future, with incorrect results. @@ -631,9 +623,7 @@ def __get_relations(self, entity_info, relationship_traits_data, result_trait_se # Remove dynamic behaviour from the reference return self.__get_relation_stable(entity_info) - return self.__get_relations_from_library( - entity_info, relationship_traits_data, result_trait_set - ) + return [] def __get_relations_entity_versions(self, entity_info, relationship_traits_data): """ diff --git a/plugin/openassetio_manager_bal/bal.py b/plugin/openassetio_manager_bal/bal.py index fe74fe0..a6ab3ea 100644 --- a/plugin/openassetio_manager_bal/bal.py +++ b/plugin/openassetio_manager_bal/bal.py @@ -71,6 +71,7 @@ class Relation: traits: Dict[str, Dict] entity_infos: List[EntityInfo] + access: str def load_library(path: str) -> dict: @@ -147,6 +148,7 @@ def entity(entity_info: EntityInfo, library: dict) -> Entity: EntityInfo(name, version=None, access=entity_info.access) for name in relation["entities"] ], + access=relation.get("access", "read"), ) for relation in entity_dict.get("relations", []) ] @@ -223,6 +225,8 @@ def related_references( entity_data = entity(entity_info, library) for relation in entity_data.relations: + if not relation.access == entity_info.access: + continue # Check if this relation contains the requested traits if not _dict_has_traits(relation.traits, requested_relation_traits): continue diff --git a/schema.json b/schema.json index f25e836..0b61104 100644 --- a/schema.json +++ b/schema.json @@ -502,6 +502,9 @@ "type": "object", "description": "The definition of a particular relationship", "properties": { + "access": { + "description": "Access mode that this relationship applies to (default is 'read')" + }, "traits": { "description": "Traits and their associated properties that describe the nature of the relationship itself (not the related entities).", "type": "object", diff --git a/tests/bal_business_logic_suite.py b/tests/bal_business_logic_suite.py index 12f4277..db7ddb4 100644 --- a/tests/bal_business_logic_suite.py +++ b/tests/bal_business_logic_suite.py @@ -103,7 +103,7 @@ class GetWithRelationshipTestCase(FixtureAugmentedTestCase): relatiionship query methods. """ - def assertExpectedRefs(self, ref, relationship, expected_refs): + def assertExpectedRefs(self, ref, relationship, expected_refs, access=RelationsAccess.kRead): """ Fetches all relations and asserts that they match expectations. """ @@ -113,7 +113,23 @@ def assertExpectedRefs(self, ref, relationship, expected_refs): [ref], relationship, 1000, - RelationsAccess.kRead, + access, + self.createTestContext(), + lambda idx, pager: result_refs.extend(pager.get()), + lambda _, err: self.fail(f"Failed to query relationships: {err.code} {err.message}"), + ) + + self.assertEqual( + [r.toString() for r in result_refs], [r.toString() for r in expected_refs] + ) + + result_refs = [] + + self._manager.getWithRelationships( + ref, + [relationship], + 1000, + access, self.createTestContext(), lambda idx, pager: result_refs.extend(pager.get()), lambda _, err: self.fail(f"Failed to query relationships: {err.code} {err.message}"), @@ -1243,52 +1259,42 @@ def assertVersioning(self, entity_name, specified_tag, expected_stable): self.assertEqual(version_trait.getStableTag(), expected_stable) -class Test_getWithRelationship_All(FixtureAugmentedTestCase): - # pylint: disable=cell-var-from-loop,unbalanced-tuple-unpacking - def test_when_unsupported_access_then_kEntityAccessError_returned(self): - entity_reference = self._manager.createEntityReference("bal:///entity/original") - relationship = TraitsData({"proxy"}) - context = self.createTestContext() - - expected = [ - BatchElementError( - BatchElementError.ErrorCode.kEntityAccessError, - "Unsupported access mode for relationship query", - ) +class Test_getWithRelationship_access(GetWithRelationshipTestCase): + def test_when_access_mode_matches_default_then_expected_relations_returned(self): + input_ref = self._manager.createEntityReference("bal:///entity/original") + expected_refs = [ + self._manager.createEntityReference("bal:///entity/proxy/1"), + self._manager.createEntityReference("bal:///entity/proxy/2"), + self._manager.createEntityReference("bal:///entity/proxy/3"), ] + # No "access" provided for "proxy" relationship in library, so kRead assumed. + self.assertExpectedRefs( + input_ref, TraitsData({"proxy"}), expected_refs, access=RelationsAccess.kRead + ) - for access in (RelationsAccess.kWrite, RelationsAccess.kCreateRelated): - with self.subTest("getWithRelationship", access=access): - actual = [None] - self._manager.getWithRelationship( - [entity_reference], - relationship, - 10, - access, - context, - lambda _idx, _refs: self.fail("Unexpected success callback"), - lambda idx, batch_element_error: operator.setitem( - actual, idx, batch_element_error - ), - ) - - self.assertListEqual(actual, expected) - - with self.subTest("getWithRelationships", access=access): - actual = [None] - self._manager.getWithRelationships( - entity_reference, - [relationship], - 10, - access, - context, - lambda _idx, _refs: self.fail("Unexpected success callback"), - lambda idx, batch_element_error: operator.setitem( - actual, idx, batch_element_error - ), - ) - - self.assertListEqual(actual, expected) + def test_when_access_mode_doesnt_match_default_then_no_relations_returned(self): + input_ref = self._manager.createEntityReference("bal:///entity/original") + expected_refs = [] + # No "access" provided for "proxy" relationship in library, so kRead assumed. + self.assertExpectedRefs( + input_ref, TraitsData({"proxy"}), expected_refs, access=RelationsAccess.kWrite + ) + + def test_when_access_mode_matches_override_then_expected_relation_returned(self): + input_ref = self._manager.createEntityReference("bal:///entity/original") + expected_refs = [self._manager.createEntityReference("bal:///anAsset⭐︎")] + # Explicit kWrite "access" provided for "publishable" relationship in library. + self.assertExpectedRefs( + input_ref, TraitsData({"publishable"}), expected_refs, access=RelationsAccess.kWrite + ) + + def test_when_access_mode_doesnt_match_override_then_no_relations_returned(self): + input_ref = self._manager.createEntityReference("bal:///entity/original") + expected_refs = [] + # Explicit kWrite "access" provided for "publishable" relationship in library. + self.assertExpectedRefs( + input_ref, TraitsData({"publishable"}), expected_refs, access=RelationsAccess.kRead + ) class Test_preflight(FixtureAugmentedTestCase): @@ -1679,5 +1685,22 @@ def test_when_querying_v1_reference_then_v1_ref_returned(self): ) +class Test_getWithRelationship_override_version(GetWithRelationshipTestCase): + + def test_when_relationship_overrides_default_versioning_behaviour_then_override_returned(self): + """ + By default, BAL handles version relationships as a special case (for better or worse). But + we offer a way to override that behaviour using the data-driven approach used for other + types of query, i.e. by explicitly defining a version relationship in the JSON library. + """ + input_ref = self._manager.createEntityReference("bal:///entity/original") + expected_refs = [self._manager.createEntityReference("bal:///another 𝓐𝓼𝓼𝓼𝓮𝔱")] + self.assertExpectedRefs( + input_ref, + EntityVersionsRelationshipSpecification.create().traitsData(), + expected_refs, + ) + + def resources_path(): return os.path.join(os.path.dirname(__file__), "resources") diff --git a/tests/resources/library_apiComplianceSuite.json b/tests/resources/library_apiComplianceSuite.json index 258e891..a950bbe 100644 --- a/tests/resources/library_apiComplianceSuite.json +++ b/tests/resources/library_apiComplianceSuite.json @@ -181,6 +181,19 @@ { "traits": {"missing": {}}, "entities": ["missingEntity"] + }, + { + "access": "write", + "traits": {"publishable": {}}, + "entities": ["anAsset⭐︎"] + }, + { + "traits": { + "openassetio-mediacreation:lifecycle.Version": {}, + "openassetio-mediacreation:relationship.Unbounded": {}, + "openassetio-mediacreation:usage.Relationship": {} + }, + "entities": ["another 𝓐𝓼𝓼𝓼𝓮𝔱"] } ] },