From fea3292497756776d5f6bb1fb485bc43b2d561b9 Mon Sep 17 00:00:00 2001 From: bkurtz Date: Tue, 13 Apr 2021 09:04:59 -0700 Subject: [PATCH] Add option to raise an error when unstructured dict has extra keys Default is to maintain the old behavior. - Add `_cattr_forbid_extra_keys` arg to `make_dict_structure_fn` and implement new behavior - Update `GenConverter` with `forbid_extra_keys` option to enable this for all classes - Add tests and update docs --- HISTORY.rst | 2 + docs/customizing.rst | 34 ++++++++++++- src/cattr/converters.py | 10 +++- src/cattr/gen.py | 14 +++++- tests/metadata/test_genconverter.py | 75 ++++++++++++++++++++++++++++- 5 files changed, 131 insertions(+), 4 deletions(-) diff --git a/HISTORY.rst b/HISTORY.rst index 724aab47..58656a21 100644 --- a/HISTORY.rst +++ b/HISTORY.rst @@ -9,6 +9,8 @@ History * ``GenConverter`` has support for easy overriding of collection unstructuring types (for example, unstructure all sets to lists) through its ``unstruct_collection_overrides`` argument. (`#137 `_) * Unstructuring mappings with ``GenConverter`` is significantly faster. +* ``GenConverter`` supports strict handling of unexpected dictionary keys through its ``forbid_extra_keys`` argument. + (`#142 `_) 1.4.0 (2021-03-21) ------------------ diff --git a/docs/customizing.rst b/docs/customizing.rst index 99e334d2..f05a7099 100644 --- a/docs/customizing.rst +++ b/docs/customizing.rst @@ -33,7 +33,8 @@ a lot of ``cattrs`` machinery and be significantly faster than normal ``cattrs`` Another reason is that it's possible to override behavior on a per-attribute basis. Currently, the overrides only support generating dictionary un/structuring functions -(as opposed to tuples), and support ``omit_if_default`` and ``rename``. +(as opposed to tuples), and support ``omit_if_default``, ``forbid_extra_keys`` and +``rename``. ``omit_if_default`` ------------------- @@ -81,6 +82,37 @@ but not to the `DateTime` field. This override has no effect when generating structuring functions. +``forbid_extra_keys`` +--------------------- + +By default ``cattrs`` is lenient in accepting unstructured input. If extra +keys are present in a dictionary, they will be ignored when generating a +structured object. Sometimes it may be desirable to enforce a stricter +contract, and to raise an error when unknown keys are present - in particular +when fields have default values this may help with catching typos. +`forbid_extra_keys` can also be enabled (or disabled) on a per-class basis when +creating structure hooks with ``make_dict_structure_fn``. + +.. doctest:: + + >>> from cattr.gen import make_dict_structure_fn + >>> + >>> @attr.s + ... class TestClass: + ... number: int = attr.ib(default=1) + >>> + >>> c = cattr.GenConverter(forbid_extra_keys=True) + >>> c.structure({"nummber": 2}, TestClass) + Traceback (most recent call last): + ... + Exception: Extra fields in constructor for TestClass: nummber + >>> hook = make_dict_structure_fn(TestClass, c, _cattr_forbid_extra_keys=False) + >>> c.register_structure_hook(TestClass, hook) + >>> c.structure({"nummber": 2}, TestClass) + TestClass(number=1) + +This behavior can only be applied to classes or to the default for the +`GenConverter`, and has no effect when generating unstructuring functions. ``rename`` ---------- diff --git a/src/cattr/converters.py b/src/cattr/converters.py index 0c7d2e53..1160533f 100644 --- a/src/cattr/converters.py +++ b/src/cattr/converters.py @@ -489,6 +489,7 @@ class GenConverter(Converter): __slots__ = ( "omit_if_default", + "forbid_extra_keys", "type_overrides", "_unstruct_collection_overrides", ) @@ -498,6 +499,7 @@ def __init__( dict_factory: Callable[[], Any] = dict, unstruct_strat: UnstructureStrategy = UnstructureStrategy.AS_DICT, omit_if_default: bool = False, + forbid_extra_keys: bool = False, type_overrides: Mapping[Type, AttributeOverride] = {}, unstruct_collection_overrides: Mapping[Type, Callable] = {}, ): @@ -505,6 +507,7 @@ def __init__( dict_factory=dict_factory, unstruct_strat=unstruct_strat ) self.omit_if_default = omit_if_default + self.forbid_extra_keys = forbid_extra_keys self.type_overrides = dict(type_overrides) self._unstruct_collection_overrides = unstruct_collection_overrides @@ -635,7 +638,12 @@ def gen_structure_attrs_fromdict(self, cl: Type[T]) -> T: for a in attribs if a.type in self.type_overrides } - h = make_dict_structure_fn(cl, self, **attrib_overrides) + h = make_dict_structure_fn( + cl, + self, + _cattrs_forbid_extra_keys=self.forbid_extra_keys, + **attrib_overrides + ) self._structure_func.register_cls_list([(cl, h)], direct=True) # only direct dispatch so that subclasses get separately generated return h diff --git a/src/cattr/gen.py b/src/cattr/gen.py index cdbee81d..9b332a0b 100644 --- a/src/cattr/gen.py +++ b/src/cattr/gen.py @@ -109,7 +109,9 @@ def generate_mapping(cl: Type, old_mapping): return cls(**mapping) -def make_dict_structure_fn(cl: Type, converter, **kwargs): +def make_dict_structure_fn( + cl: Type, converter, _cattrs_forbid_extra_keys: bool = False, **kwargs +): """Generate a specialized dict structuring function for an attrs class.""" mapping = None @@ -181,6 +183,16 @@ def make_dict_structure_fn(cl: Type, converter, **kwargs): f" res['{ian}'] = {struct_handler_name}(o['{kn}'], __c_t_{an})" ) lines.append(" }") + if _cattrs_forbid_extra_keys: + allowed_fields = {a.name for a in attrs} + globs["__c_a"] = allowed_fields + post_lines += [ + " unknown_fields = set(o.keys()) - __c_a", + " if unknown_fields:", + " raise Exception(", + f" 'Extra fields in constructor for {cl_name}: ' + ', '.join(unknown_fields)" + " )", + ] total_lines = lines + post_lines + [" return __cl(**res)"] diff --git a/tests/metadata/test_genconverter.py b/tests/metadata/test_genconverter.py index 5439fbc8..a4ce4700 100644 --- a/tests/metadata/test_genconverter.py +++ b/tests/metadata/test_genconverter.py @@ -18,7 +18,7 @@ from cattr import GenConverter as Converter from cattr import UnstructureStrategy -from cattr.gen import override +from cattr.gen import make_dict_structure_fn, override from . import ( nested_typed_classes, @@ -59,6 +59,79 @@ def test_simple_roundtrip_defaults(attr_and_vals, strat): assert inst == converter.structure(converter.unstructure(inst), cl) +@given(simple_typed_classes() | simple_typed_dataclasses(), unstructure_strats) +def test_simple_roundtrip_with_extra_keys_forbidden(cls_and_vals, strat): + """ + Simple classes can be unstructured and restructured with forbid_extra_keys=True. + """ + converter = Converter(unstruct_strat=strat, forbid_extra_keys=True) + cl, vals = cls_and_vals + inst = cl(*vals) + unstructured = converter.unstructure(inst) + assert "Hyp" not in repr(unstructured) + assert inst == converter.structure(unstructured, cl) + + +@given(simple_typed_classes() | simple_typed_dataclasses()) +def test_forbid_extra_keys(cls_and_vals): + """ + Restructuring fails when extra keys are present (when configured) + """ + converter = Converter(forbid_extra_keys=True) + cl, vals = cls_and_vals + inst = cl(*vals) + unstructured = converter.unstructure(inst) + bad_key = list(unstructured)[0] + "A" if unstructured else "Hyp" + while bad_key in unstructured: + bad_key += "A" + unstructured[bad_key] = 1 + with pytest.raises(Exception): + converter.structure(unstructured, cl) + + +@given(simple_typed_attrs(defaults=True)) +def test_forbid_extra_keys_defaults(attr_and_vals): + """ + Restructuring fails when a dict key is renamed (if forbid_extra_keys set) + """ + a, _ = attr_and_vals + cl = make_class("HypClass", {"a": a}) + converter = Converter(forbid_extra_keys=True) + inst = cl() + unstructured = converter.unstructure(inst) + unstructured["aa"] = unstructured.pop("a") + with pytest.raises(Exception): + converter.structure(unstructured, cl) + + +def test_forbid_extra_keys_nested_override(): + @attr.s + class C: + a = attr.ib(type=int, default=1) + + @attr.s + class A: + c = attr.ib(type=C) + a = attr.ib(type=int, default=2) + + converter = Converter(forbid_extra_keys=True) + unstructured = {"a": 3, "c": {"a": 4}} + # at this point, structuring should still work + converter.structure(unstructured, A) + # if we break it in the subclass, we need it to raise + unstructured["c"]["aa"] = 5 + with pytest.raises(Exception): + converter.structure(unstructured, A) + # we can "fix" that by disabling forbid_extra_keys on the subclass + hook = make_dict_structure_fn(C, converter, _cattr_forbid_extra_keys=False) + converter.register_structure_hook(C, hook) + converter.structure(unstructured, A) + # but we should still raise at the top level + unstructured["b"] = 6 + with pytest.raises(Exception): + converter.structure(unstructured, A) + + @given( nested_typed_classes(defaults=True, min_attrs=1), unstructure_strats,