From ecdaff76ae72cc5871b178fb802e6fd6befc97fd Mon Sep 17 00:00:00 2001 From: Daniel <d.hornung@indiscale.com> Date: Fri, 1 Dec 2023 18:37:30 +0100 Subject: [PATCH] FIX: Better detection of dependencies when deleting Containers --- CHANGELOG.md | 2 + src/linkahead/common/models.py | 85 ++++++++++++++++++++-------------- unittests/test_container.py | 26 +++++++++-- 3 files changed, 76 insertions(+), 37 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ff6a2d58..62b74939 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed ### +- [#113](https://gitlab.com/linkahead/linkahead-pylib/-/issues/113) Container could fail to delete when there were reference properties. + ### Security ### ### Documentation ### diff --git a/src/linkahead/common/models.py b/src/linkahead/common/models.py index 75ee469b..be6acf3c 100644 --- a/src/linkahead/common/models.py +++ b/src/linkahead/common/models.py @@ -63,6 +63,7 @@ from ..exceptions import (AmbiguousEntityError, AuthorizationError, UniqueNamesError, UnqualifiedParentsError, UnqualifiedPropertiesError) from .datatype import (BOOLEAN, DATETIME, DOUBLE, INTEGER, TEXT, + get_list_datatype, is_list_datatype, is_reference) from .state import State from .timezone import TimeZone @@ -3250,14 +3251,19 @@ class Container(list): return sync_dict - def _test_dependencies_in_container(self, container): - """This function returns those elements of a given container that are a dependency of another element of the same container. + @staticmethod + def _find_dependencies_in_container(container): + """Find elements in a container that are a dependency of another element of the same. - Args: - container (Container): a linkahead container + Parameters + ---------- + container : Container + A LinkAhead container. - Returns: - [set]: a set of unique elements that are a dependency of another element of `container` + Returns + ------- + out : set + A set of IDs of unique elements that are a dependency of another element of ``container``. """ item_id = set() is_parent = set() @@ -3274,28 +3280,40 @@ class Container(list): for parents in container_item.get_parents(): is_parent.add(parents.id) - for references in container_item.get_properties(): - if is_reference(references.datatype): - # add only if it is a reference, not a property - - if references.value is None: - continue - elif isinstance(references.value, int): - is_being_referenced.add(references.value) - elif is_list_datatype(references.datatype): - for list_item in references.value: - if isinstance(list_item, int): - is_being_referenced.add(list_item) - else: - is_being_referenced.add(list_item.id) - else: - try: - is_being_referenced.add(references.value.id) - except AttributeError: + for prop in container_item.get_properties(): + prop_dt = prop.datatype + if is_reference(prop_dt): + # add only if it is a reference, not a simple property + # Step 1: look for prop.value + if prop.value is not None: + if isinstance(prop.value, int): + is_being_referenced.add(prop.value) + elif is_list_datatype(prop_dt): + for list_item in prop.value: + if isinstance(list_item, int): + is_being_referenced.add(list_item) + else: + is_being_referenced.add(list_item.id) + else: + try: + is_being_referenced.add(prop.value.id) + except AttributeError: + pass + # Step 2: Reference properties + if prop.is_reference(): + if is_list_datatype(prop_dt): + ref_name = get_list_datatype(prop_dt) + try: + is_being_referenced.add(container.get_entity_by_name(ref_name).id) + except KeyError: + pass + elif isinstance(prop_dt, str): pass + else: + is_being_referenced.add(prop_dt.id) - if hasattr(references, 'id'): - is_property.add(references.id) + if hasattr(prop, 'id'): + is_property.add(prop.id) dependent_parents = item_id.intersection(is_parent) dependent_properties = item_id.intersection(is_property) @@ -3312,19 +3330,18 @@ class Container(list): name otherwise. If any entity has no id and no name a TransactionError will be raised. - Note: If only a name is given this could lead to ambiguities. If - this happens, none of them will be deleted. It occurs an error - instead. + Note: If only a name is given this could lead to ambiguities. If this happens, none of them + will be deleted. An error is raised instead. + """ item_count = len(self) # Split Container in 'chunk_size'-sized containers (if necessary) to avoid error 414 Request-URI Too Long if item_count > chunk_size: - dependencies = self._test_dependencies_in_container(self) - ''' - If there are as many dependencies as entities in the container and it is larger than chunk_size it cannot be split and deleted. - This case cannot be handled at the moment. - ''' + dependencies = Container._find_dependencies_in_container(self) + + # If there are as many dependencies as entities in the container and it is larger than + # chunk_size it cannot be split and deleted. This case cannot be handled at the moment. if len(dependencies) == item_count: if raise_exception_on_error: diff --git a/unittests/test_container.py b/unittests/test_container.py index 113dd622..5833fb3f 100644 --- a/unittests/test_container.py +++ b/unittests/test_container.py @@ -23,7 +23,7 @@ # ** end header # """Tests for the Container class.""" -from __future__ import absolute_import +import pytest import linkahead as db @@ -125,7 +125,7 @@ def test_container_dependencies_for_deletion(): record_with_parent, record_with_property_which_is_not_a_record ]) - assert (db.Container()._test_dependencies_in_container(container) + assert (db.Container._find_dependencies_in_container(container) == {2002, 1005, 1007}) @@ -143,4 +143,24 @@ def test_container_dependencies_for_deletion_with_lists(): container = db.Container() container.extend([record_with_list, record_referenced]) - assert db.Container()._test_dependencies_in_container(container) == {2001} + assert db.Container._find_dependencies_in_container(container) == {2001} + + +# @pytest.mark.xfail +def test_container_deletion_with_references(): + """Test if dependencies are checked correctly. + """ + + RT1 = db.RecordType(name="RT1") + RT2 = db.RecordType(name="RT2").add_property(name="prop2", datatype=RT1) + RT3 = db.RecordType(name="RT3").add_property(name="prop3", datatype="LIST<RT1>") + cont12 = db.Container().extend([RT1, RT2]) + cont13 = db.Container().extend([RT1, RT3]) + cont12.to_xml() + cont13.to_xml() + + deps12 = db.Container._find_dependencies_in_container(cont12) + deps13 = db.Container._find_dependencies_in_container(cont13) + + assert len(deps12) == 1 and deps12.pop() == -1 + assert len(deps13) == 1 and deps13.pop() == -1 -- GitLab