From 886313e5f46d47db119fe7a4b189bddec8f9675e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Leon=20M=C3=B6ller?= Date: Sat, 4 Nov 2023 16:13:27 +0100 Subject: [PATCH] model.base: fix `ConstrainedList.clear()` atomicity The default inherited `clear()` implementation repeatedly deletes the last item of the list until the list is empty. If the last item can be deleted successfully, but an item in front of it that will be deleted later cannot, this makes `clear()` non-atomic. Thus, the `clear()` method is now overriden in an atomic way. Furthermore, the ConstrainedList atomicity test is fixed to correctly test for this as well. --- basyx/aas/model/base.py | 4 ++++ test/model/test_base.py | 10 +++++++++- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/basyx/aas/model/base.py b/basyx/aas/model/base.py index a39f48fe9..895197fc8 100644 --- a/basyx/aas/model/base.py +++ b/basyx/aas/model/base.py @@ -1311,6 +1311,10 @@ def extend(self, values: Iterable[_T]) -> None: self._item_add_hook(v, self._list + v_list[:idx]) self._list = self._list + v_list + def clear(self) -> None: + # clear() repeatedly deletes the last item by default, making it not atomic + del self[:] + @overload def __getitem__(self, index: int) -> _T: ... diff --git a/test/model/test_base.py b/test/model/test_base.py index ef894de50..aa40e0199 100644 --- a/test/model/test_base.py +++ b/test/model/test_base.py @@ -1213,7 +1213,15 @@ def hook(itm: int, _list: List[int]) -> None: self.assertEqual(c_list, [1, 2, 3]) with self.assertRaises(ValueError): c_list.clear() - self.assertEqual(c_list, [1, 2, 3]) + # the default clear() implementation seems to repeatedly delete the last item until the list is empty + # in this case, the last item is 3, which cannot be deleted because it is > 2, thus leaving it unclear whether + # clear() really is atomic. to work around this, the list is reversed, making 1 the last item, and attempting + # to clear again. + c_list.reverse() + with self.assertRaises(ValueError): + c_list.clear() + self.assertEqual(c_list, [3, 2, 1]) + c_list.reverse() del c_list[0:2] self.assertEqual(c_list, [3])