From ebfb278009679f4f458c4362e97540abdca7a16b Mon Sep 17 00:00:00 2001 From: Moritz Sommer Date: Mon, 23 Feb 2026 17:55:38 +0100 Subject: [PATCH] sdk: Fix empty-cache handling in LocalFileIdentifiableStore Previously, the `_object_cache` of the `LocalFileIdentifiableStore` was initialised empty. Calling `discard()` immediately after store initialisation raised a `KeyError` if the Identifiable to delete existed on disk but had not yet been loaded into the in-memory cache. This adds explicit handling of missing cache entries in `discard()`, preventing errors when a key is not present in the in-memory cache. Moreover, a corresponding regression test is added. Finally, the interim empty-cache workaround in `sync()` is removed. Fixes #438 --- sdk/basyx/aas/backend/local_file.py | 2 +- sdk/basyx/aas/model/provider.py | 14 -------------- sdk/test/backend/test_local_file.py | 10 ++++++++++ 3 files changed, 11 insertions(+), 15 deletions(-) diff --git a/sdk/basyx/aas/backend/local_file.py b/sdk/basyx/aas/backend/local_file.py index 1983eb08..4008497a 100644 --- a/sdk/basyx/aas/backend/local_file.py +++ b/sdk/basyx/aas/backend/local_file.py @@ -123,7 +123,7 @@ def discard(self, x: model.Identifiable) -> None: except FileNotFoundError as e: raise KeyError("No AAS object with id {} exists in local file database".format(x.id)) from e with self._object_cache_lock: - del self._object_cache[x.id] + self._object_cache.pop(x.id, None) def __contains__(self, x: object) -> bool: """ diff --git a/sdk/basyx/aas/model/provider.py b/sdk/basyx/aas/model/provider.py index 425344f7..c48342c6 100644 --- a/sdk/basyx/aas/model/provider.py +++ b/sdk/basyx/aas/model/provider.py @@ -78,20 +78,6 @@ def sync(self, other: Iterable[_VALUE], overwrite: bool) -> Tuple[int, int, int] for value in other: if value in self: if overwrite: - - # TODO: This is a quick fix. Yes it works. The underlying problem with the subclass - # `LocalFileIdentifiableStore` will be solved in a separate issue - # (https://github.com/eclipse-basyx/basyx-python-sdk/issues/438). - # Think of this as pythonic duct tape. - # - # The problem is that the `_object_cache` isn't initialised together with the - # `LocalFileIdentifiableStore`, leading to an error when `discard()` is called on the empty cache. - # The for-loop calls `__iter__` calls `get_identifiable_by_hash()` calls - # `self._object_cache[obj.id] = obj`, adding all identifiables to the cache and therefore avoiding - # the error. - for element in self: - pass - self.discard(value) self.add(value) overwritten += 1 diff --git a/sdk/test/backend/test_local_file.py b/sdk/test/backend/test_local_file.py index 9d72c73c..02526c80 100644 --- a/sdk/test/backend/test_local_file.py +++ b/sdk/test/backend/test_local_file.py @@ -106,3 +106,13 @@ def test_key_errors(self) -> None: self.identifiable_store.discard(retrieved_submodel) self.assertEqual("'No AAS object with id https://acplt.org/Test_Submodel exists in " "local file database'", str(cm.exception)) + + def test_reload_discard(self) -> None: + # Load example submodel + example_submodel = create_example_submodel() + self.identifiable_store.add(example_submodel) + + # Reload the DictIdentifiableStore and discard the example submodel + self.identifiable_store = local_file.LocalFileIdentifiableStore(store_path) + self.identifiable_store.discard(example_submodel) + self.assertNotIn(example_submodel, self.identifiable_store)