[wxfile] raise KeyError for non-existent keys. (#759)

* [wxfile] raise KeyError for non-existent keys.

Fixes #757

Co-authored-by: Cagtay Fabry <[email protected]>

CHANGELOG.rst

@@ -15,6 +15,10 @@ removed
 changes
 =======
 
+-  `WeldxFile` now raises a `KeyError`, if the user tries to directly read or manipulate a protected ASDF keyword
+   within the file. [:pull:`759`]
+
+
 fixes
 =====
 

weldx/asdf/file.py

@@ -629,7 +629,7 @@ def update(self, mapping: Union[Mapping, Iterable] = (), **kwargs: Any):
         super().update(mapping, **kwargs)
 
     def items(self) -> Set[tuple[Any, Any]]:
-        """Return a set-like object providing a view on this files items.
+        """Return a set-like object providing a view on this file's items.
 
         Returns
         -------

weldx/asdf/util.py

@@ -587,29 +587,31 @@ def _get_instance_units(
 
 
 class _ProtectedViewDict(MutableMapping):
+    """A mutable mapping which protects given keys from manipulation."""
+
     def __init__(self, protected_keys, data=None):
         super(_ProtectedViewDict, self).__init__()
-        self.__data = data
+        self.__data = data if data is not None else dict()
         self.protected_keys = protected_keys
 
-    @property
-    def _data(self):
-        return self
+    def _wrap_protected_non_existent(self, key, method: str):
+        if key in self.protected_keys:
+            self._warn_protected_keys()
+            raise KeyError(f"'{key}' is protected.")
+        elif key not in self.__data:
+            raise KeyError(f"'{key}' not contained.")
+
+        method_obj = getattr(self.__data, method)
+        return method_obj(key)
 
     def __len__(self) -> int:
         return len(self.keys())
 
     def __getitem__(self, key):
-        if key in self.protected_keys:
-            self._warn_protected_keys()
-            raise KeyError
-        return self.__data.get(key)
+        return self._wrap_protected_non_existent(key, "__getitem__")
 
     def __delitem__(self, key):
-        if key in self.protected_keys:
-            self._warn_protected_keys()
-            return
-        del self.__data[key]
+        return self._wrap_protected_non_existent(key, "__delitem__")
 
     def __setitem__(self, key, value):
         if key in self.protected_keys:
@@ -643,7 +645,7 @@ def popitem(self) -> tuple[Hashable, Any]:
             if k not in self.protected_keys:
                 return k, self.pop(k)
 
-        raise KeyError
+        raise KeyError("empty")
 
     def clear(self):
         """Clear all data except the protected keys."""

weldx/tests/asdf_tests/test_asdf_util.py

@@ -2,13 +2,15 @@
 from __future__ import annotations
 
 import io
+import unittest
 from dataclasses import dataclass
 
 import numpy as np
 import pytest
 
 from weldx.asdf.file import WeldxFile
 from weldx.asdf.util import (
+    _ProtectedViewDict,
     dataclass_serialization_class,
     get_highest_tag_version,
     get_schema_tree,
@@ -186,3 +188,46 @@ def test_get_highest_tag_version():
 def test_get_schema_tree():
     d = get_schema_tree("single_pass_weld-0.1.0")
     assert isinstance(d, dict)
+
+
+class TestProtectedView(unittest.TestCase):
+    def setUp(self):
+        data = dict(foo="blub", bar=42)
+        self.protected_key = "bar"
+        self.view = _ProtectedViewDict(protected_keys=[self.protected_key], data=data)
+
+    def test_protected_keys_hidden(self):
+        assert self.protected_key not in self.view.keys()
+        assert self.protected_key not in self.view
+        assert (self.protected_key, 42) not in self.view.items()
+
+    def test_allowed_access(self):
+        assert self.view["foo"] == "blub"
+
+    def test_illegal_access(self):
+        with pytest.raises(KeyError):
+            _ = self.view["bar"]
+
+        with pytest.raises(KeyError):
+            del self.view["bar"]
+
+    def test_access_non_existent(self):
+        with pytest.raises(KeyError):
+            _ = self.view["no"]
+
+    def test_update(self):
+        expected_match = "manipulate an ASDF internal structure"
+        warning_type = UserWarning
+
+        with pytest.warns(warning_type, match=expected_match):
+            self.view.update(dict(bar=None), foo=1)
+        assert self.view["foo"] == 1
+
+    def test_popitem(self):
+        k, v = self.view.popitem()
+        assert k == "foo"
+        assert v == "blub"
+
+    def test_clear(self):
+        self.view.clear()
+        assert len(self.view) == 0

weldx/tests/asdf_tests/test_weldx_file.py

@@ -594,10 +594,12 @@ def test_cannot_update_del_protected_keys(self, protected_key):
         expected_match = "manipulate an ASDF internal structure"
         warning_type = UserWarning
 
+        # reading is also forbidden
+        with pytest.raises(KeyError):
+            _ = self.fh[protected_key]
+
         with pytest.warns(warning_type, match=expected_match):
             self.fh.update({protected_key: None})
-        with pytest.warns(warning_type, match=expected_match):
-            del self.fh[protected_key]
         with pytest.warns(warning_type, match=expected_match):
             self.fh.pop(protected_key)
         with pytest.warns(warning_type, match=expected_match):
@@ -612,7 +614,7 @@ def test_popitem_remain_protected_keys(self):
             keys.append(key)
         assert keys == [META_ATTR]
 
-    def test_len_proteced_keys(self):
+    def test_len_protected_keys(self):
         """Should only contain key 'wx_metadata'."""
         assert len(self.fh) == 1