Skip to content

Commit 6084f69

Browse files
committed
sdk: Fix LocalFileIdentifiableStore mutation persistence
Previously, mutations to objects retrieved from `LocalFileIdentifiableStore` were never written back to disk. Only `add()` wrote to the file store; subsequent in-memory changes were invisible to other processes (e.g. different uWSGI workers) once the `WeakValueDictionary` cache was evicted. This was an issue introduced in #370, which removed `Referable.commit()` and the `LocalFileBackend` class without replacing the write-back mechanism. Two fixes are made here: - `AbstractObjectStore.commit()` is added as a no-op default. `LocalFileIdentifiableStore` overrides it to re-serialize the object to its `.json` file. - `get_item()` now returns the cached object directly on a cache hit, instead of always re-reading from disk and overwriting in-memory state. This was a secondary bug that would silently discard mutations within the same process if the same object was retrieved more than once. A regression test (`test_mutation_persistence`) is added that mutates a stored object, evicts the cache via `gc.collect()`, and verifies the updated value is read back from disk.
1 parent efea046 commit 6084f69

3 files changed

Lines changed: 55 additions & 8 deletions

File tree

sdk/basyx/aas/backend/local_file.py

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -68,21 +68,14 @@ def get_identifiable_by_hash(self, hash_: str) -> model.Identifiable:
6868
6969
:raises KeyError: If the respective file could not be found
7070
"""
71-
# Try to get the correct file
7271
try:
7372
with open("{}/{}.json".format(self.directory_path, hash_), "r") as file:
7473
data = json.load(file, cls=json_deserialization.AASFromJsonDecoder)
7574
obj = data["data"]
7675
except FileNotFoundError as e:
7776
raise KeyError("No Identifiable with hash {} found in local file database".format(hash_)) from e
78-
# If we still have a local replication of that object (since it is referenced from anywhere else), update that
79-
# replication and return it.
8077
with self._object_cache_lock:
81-
if obj.id in self._object_cache:
82-
old_obj = self._object_cache[obj.id]
83-
old_obj.update_from(obj)
84-
return old_obj
85-
self._object_cache[obj.id] = obj
78+
self._object_cache[obj.id] = obj
8679
return obj
8780

8881
def get_item(self, identifier: model.Identifier) -> model.Identifiable:
@@ -91,6 +84,9 @@ def get_item(self, identifier: model.Identifier) -> model.Identifiable:
9184
9285
:raises KeyError: If the respective file could not be found
9386
"""
87+
with self._object_cache_lock:
88+
if identifier in self._object_cache:
89+
return self._object_cache[identifier]
9490
try:
9591
return self.get_identifiable_by_hash(self._transform_id(identifier))
9692
except KeyError as e:
@@ -110,6 +106,18 @@ def add(self, x: model.Identifiable) -> None:
110106
with self._object_cache_lock:
111107
self._object_cache[x.id] = x
112108

109+
def commit(self, x: model.Identifiable) -> None:
110+
"""
111+
Write the current in-memory state of a stored object back to its file.
112+
113+
:param x: The object to persist
114+
:raises KeyError: If the object is not present in the store
115+
"""
116+
if not os.path.exists("{}/{}.json".format(self.directory_path, self._transform_id(x.id))):
117+
raise KeyError("No AAS object with id {} exists in local file database".format(x.id))
118+
with open("{}/{}.json".format(self.directory_path, self._transform_id(x.id)), "w") as file:
119+
json.dump({"data": x}, file, cls=json_serialization.AASToJsonEncoder, indent=4)
120+
113121
def discard(self, x: model.Identifiable) -> None:
114122
"""
115123
Delete an :class:`~basyx.aas.model.base.Identifiable` AAS object from the local file store

sdk/basyx/aas/model/provider.py

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,18 @@ class AbstractObjectStore(AbstractObjectProvider[_KEY, _VALUE], MutableSet[_VALU
5959
def __init__(self):
6060
pass
6161

62+
def commit(self, x: _VALUE) -> None:
63+
"""
64+
Persist an in-memory mutation of a stored object back to the underlying storage.
65+
66+
The default implementation is a no-op, suitable for in-memory stores where the object
67+
is the storage. Persistent backends (e.g. file-based or database-backed stores) must
68+
override this to write the updated object back to storage.
69+
70+
:param x: The object whose current in-memory state should be persisted
71+
"""
72+
pass
73+
6274
def update(self, other: Iterable[_VALUE]) -> None:
6375
for x in other:
6476
self.add(x)

sdk/test/backend/test_local_file.py

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
# the LICENSE file of this project.
55
#
66
# SPDX-License-Identifier: MIT
7+
import gc
78
import os.path
89
import shutil
910

@@ -134,6 +135,32 @@ def test_iter_ignores_non_json_files(self) -> None:
134135
self.assertEqual(5, len(items))
135136
os.remove(stray)
136137

138+
def test_mutation_persistence(self) -> None:
139+
submodel = model.Submodel(
140+
id_='https://example.org/MutationTest',
141+
submodel_element={
142+
model.Property(id_short='Prop', value_type=model.datatypes.String, value='before')
143+
}
144+
)
145+
self.identifiable_store.add(submodel)
146+
147+
retrieved = self.identifiable_store.get_item('https://example.org/MutationTest')
148+
assert isinstance(retrieved, model.Submodel)
149+
prop = retrieved.get_referable(['Prop'])
150+
assert isinstance(prop, model.Property)
151+
prop.update_from(model.Property(id_short='Prop', value_type=model.datatypes.String, value='after'))
152+
self.identifiable_store.commit(retrieved)
153+
154+
# Drop all strong references to evict the WeakValueDictionary cache
155+
del submodel, retrieved, prop
156+
gc.collect()
157+
158+
fresh = self.identifiable_store.get_item('https://example.org/MutationTest')
159+
assert isinstance(fresh, model.Submodel)
160+
fresh_prop = fresh.get_referable(['Prop'])
161+
assert isinstance(fresh_prop, model.Property)
162+
self.assertEqual('after', fresh_prop.value)
163+
137164
def test_reload_discard(self) -> None:
138165
# Load example submodel
139166
example_submodel = create_example_submodel()

0 commit comments

Comments
 (0)