From b6e9a33f4c1870a891ef07a49b544c53f64fe4c0 Mon Sep 17 00:00:00 2001 From: "Martin K. Scherer" Date: Wed, 18 May 2022 20:59:50 +0200 Subject: [PATCH] [wxfile] raise KeyError for non-existent keys. (#759) * [wxfile] raise KeyError for non-existent keys. Fixes #757 Co-authored-by: Cagtay Fabry <43667554+CagtayFabry@users.noreply.github.com> --- CHANGELOG.rst | 4 ++ weldx/asdf/file.py | 2 +- weldx/asdf/util.py | 28 +++++++------- weldx/tests/asdf_tests/test_asdf_util.py | 45 +++++++++++++++++++++++ weldx/tests/asdf_tests/test_weldx_file.py | 8 ++-- 5 files changed, 70 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 10fea55f6..8241115c7 100644 --- a/CHANGELOG.rst +++ b/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 ===== diff --git a/weldx/asdf/file.py b/weldx/asdf/file.py index 1f92f1cd8..869b1ef7e 100644 --- a/weldx/asdf/file.py +++ b/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 ------- diff --git a/weldx/asdf/util.py b/weldx/asdf/util.py index 7a39c5e3f..f897d9917 100644 --- a/weldx/asdf/util.py +++ b/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.""" diff --git a/weldx/tests/asdf_tests/test_asdf_util.py b/weldx/tests/asdf_tests/test_asdf_util.py index db5cd6398..eb9338fb3 100644 --- a/weldx/tests/asdf_tests/test_asdf_util.py +++ b/weldx/tests/asdf_tests/test_asdf_util.py @@ -2,6 +2,7 @@ from __future__ import annotations import io +import unittest from dataclasses import dataclass import numpy as np @@ -9,6 +10,7 @@ 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 diff --git a/weldx/tests/asdf_tests/test_weldx_file.py b/weldx/tests/asdf_tests/test_weldx_file.py index 21acbb6bd..ac6911a59 100644 --- a/weldx/tests/asdf_tests/test_weldx_file.py +++ b/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