From 7a2e341383743e61d349e4a2c76da404e35328ce 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 e96bf4a1d..8f58456d8 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])