From b20eab5eb7eec945d598aefda9ff57f5e4b2a1ef Mon Sep 17 00:00:00 2001 From: Fabian Raab Date: Fri, 25 Mar 2022 17:04:54 +0100 Subject: [PATCH 1/4] For ``forbid_extra_keys`` raise custom ``ForbiddenExtraKeyError`` instead of generic ``Exception``. --- HISTORY.rst | 3 ++- docs/customizing.rst | 2 +- docs/structuring.rst | 2 +- src/cattrs/errors.py | 4 ++++ src/cattrs/gen.py | 3 ++- tests/metadata/test_genconverter.py | 9 +++++---- 6 files changed, 15 insertions(+), 8 deletions(-) diff --git a/HISTORY.rst b/HISTORY.rst index f048f1b7..e4f7673b 100644 --- a/HISTORY.rst +++ b/HISTORY.rst @@ -18,7 +18,8 @@ History (`#231 `_) * Fix unstructuring all tuples - unannotated, variable-length, homogenous and heterogenous - to `list`. (`#226 `_) - +* For ``forbid_extra_keys`` raise custom ``ForbiddenExtraKeyError`` instead of generic ``Exception``. + (`#255 `_) 1.10.0 (2022-01-04) ------------------- diff --git a/docs/customizing.rst b/docs/customizing.rst index f0e7b4ee..e9927f5b 100644 --- a/docs/customizing.rst +++ b/docs/customizing.rst @@ -108,7 +108,7 @@ creating structure hooks with ``make_dict_structure_fn``. >>> c.structure({"nummber": 2}, TestClass) Traceback (most recent call last): ... - Exception: Extra fields in constructor for TestClass: nummber + ForbiddenExtraKeyError: Extra fields in constructor for TestClass: nummber >>> hook = make_dict_structure_fn(TestClass, c, _cattrs_forbid_extra_keys=False) >>> c.register_structure_hook(TestClass, hook) >>> c.structure({"nummber": 2}, TestClass) diff --git a/docs/structuring.rst b/docs/structuring.rst index dfa9489a..a16ed490 100644 --- a/docs/structuring.rst +++ b/docs/structuring.rst @@ -473,7 +473,7 @@ Here's a small example showing how to use factory hooks to apply the `forbid_ext >>> c.structure({"an_int": 1, "else": 2}, E) Traceback (most recent call last): ... - Exception: Extra fields in constructor for E: else + ForbiddenExtraKeyError: Extra fields in constructor for E: else A complex use case for hook factories is described over at :ref:`Using factory hooks`. diff --git a/src/cattrs/errors.py b/src/cattrs/errors.py index a85af868..7e588737 100644 --- a/src/cattrs/errors.py +++ b/src/cattrs/errors.py @@ -33,3 +33,7 @@ class ClassValidationError(BaseValidationError): """Raised when validating a class if any attributes are invalid.""" pass + + +class ForbiddenExtraKeyError(Exception): + """Raised when `forbid_extra_keys` is activated and such an extra key is detected during structuring.""" diff --git a/src/cattrs/gen.py b/src/cattrs/gen.py index 3027cf30..c0456ddb 100644 --- a/src/cattrs/gen.py +++ b/src/cattrs/gen.py @@ -447,10 +447,11 @@ def make_dict_structure_fn( if _cattrs_forbid_extra_keys: globs["__c_a"] = allowed_fields + globs["ForbiddenExtraKeyError"] = ForbiddenExtraKeyError post_lines += [ " unknown_fields = set(o.keys()) - __c_a", " if unknown_fields:", - " raise Exception(", + " raise ForbiddenExtraKeyError(", f" 'Extra fields in constructor for {cl_name}: ' + ', '.join(unknown_fields)" " )", ] diff --git a/tests/metadata/test_genconverter.py b/tests/metadata/test_genconverter.py index 419aaf59..fe21e63c 100644 --- a/tests/metadata/test_genconverter.py +++ b/tests/metadata/test_genconverter.py @@ -19,6 +19,7 @@ from cattr import GenConverter as Converter from cattr import UnstructureStrategy from cattr._compat import is_py39_plus, is_py310_plus +from cattrs.errors import ForbiddenExtraKeyError from cattr.gen import make_dict_structure_fn, override from . import ( @@ -87,7 +88,7 @@ def test_forbid_extra_keys(cls_and_vals): while bad_key in unstructured: bad_key += "A" unstructured[bad_key] = 1 - with pytest.raises(Exception): + with pytest.raises(ForbiddenExtraKeyError): converter.structure(unstructured, cl) @@ -102,7 +103,7 @@ def test_forbid_extra_keys_defaults(attr_and_vals): inst = cl() unstructured = converter.unstructure(inst) unstructured["aa"] = unstructured.pop("a") - with pytest.raises(Exception): + with pytest.raises(ForbiddenExtraKeyError): converter.structure(unstructured, cl) @@ -122,7 +123,7 @@ class A: converter.structure(unstructured, A) # if we break it in the subclass, we need it to raise unstructured["c"]["aa"] = 5 - with pytest.raises(Exception): + with pytest.raises(ForbiddenExtraKeyError): converter.structure(unstructured, A) # we can "fix" that by disabling forbid_extra_keys on the subclass hook = make_dict_structure_fn(C, converter, _cattrs_forbid_extra_keys=False) @@ -130,7 +131,7 @@ class A: converter.structure(unstructured, A) # but we should still raise at the top level unstructured["b"] = 6 - with pytest.raises(Exception): + with pytest.raises(ForbiddenExtraKeyError): converter.structure(unstructured, A) From 92fe330d89ccd3157b1db4db4152743d01ca5227 Mon Sep 17 00:00:00 2001 From: Fabian Raab Date: Fri, 25 Mar 2022 18:54:32 +0100 Subject: [PATCH 2/4] The exception ``ForbiddenExtraKeysError`` gets cl and extra_keys attributes. --- src/cattrs/errors.py | 24 +++++++++++++++++++++--- src/cattrs/gen.py | 12 +++++++----- tests/metadata/test_genconverter.py | 16 +++++++++++----- tests/test_gen_dict.py | 3 ++- 4 files changed, 41 insertions(+), 14 deletions(-) diff --git a/src/cattrs/errors.py b/src/cattrs/errors.py index 7e588737..a627c207 100644 --- a/src/cattrs/errors.py +++ b/src/cattrs/errors.py @@ -1,4 +1,4 @@ -from typing import Type +from typing import Optional, Set, Type from cattr._compat import ExceptionGroup @@ -35,5 +35,23 @@ class ClassValidationError(BaseValidationError): pass -class ForbiddenExtraKeyError(Exception): - """Raised when `forbid_extra_keys` is activated and such an extra key is detected during structuring.""" +class ForbiddenExtraKeysError(Exception): + """Raised when `forbid_extra_keys` is activated and such extra keys are detected during structuring. + + The attribute `extra_fields` is a sequence of those extra keys, which were the cause of this error, + and `cl` is the class which was structured with those extra keys. + """ + + def __init__( + self, message: Optional[str], cl: Type, extra_fields: Set[str] + ) -> None: + self.cl = cl + self.extra_fields = extra_fields + + msg = ( + message + if message + else f"Extra fields in constructor for {cl.__name__}: {', '.join(extra_fields)}" + ) + + super().__init__(msg) diff --git a/src/cattrs/gen.py b/src/cattrs/gen.py index c0456ddb..4e73fb62 100644 --- a/src/cattrs/gen.py +++ b/src/cattrs/gen.py @@ -17,7 +17,11 @@ is_generic, ) from cattr._generics import deep_copy_with -from cattrs.errors import ClassValidationError, IterableValidationError +from cattrs.errors import ( + ClassValidationError, + ForbiddenExtraKeysError, + IterableValidationError, +) if TYPE_CHECKING: # pragma: no cover from cattr.converters import Converter @@ -447,13 +451,11 @@ def make_dict_structure_fn( if _cattrs_forbid_extra_keys: globs["__c_a"] = allowed_fields - globs["ForbiddenExtraKeyError"] = ForbiddenExtraKeyError + globs["__c_feke"] = ForbiddenExtraKeysError post_lines += [ " unknown_fields = set(o.keys()) - __c_a", " if unknown_fields:", - " raise ForbiddenExtraKeyError(", - f" 'Extra fields in constructor for {cl_name}: ' + ', '.join(unknown_fields)" - " )", + " raise __c_feke('', __cl, unknown_fields)", ] # At the end, we create the function header. diff --git a/tests/metadata/test_genconverter.py b/tests/metadata/test_genconverter.py index fe21e63c..f79749d5 100644 --- a/tests/metadata/test_genconverter.py +++ b/tests/metadata/test_genconverter.py @@ -19,8 +19,8 @@ from cattr import GenConverter as Converter from cattr import UnstructureStrategy from cattr._compat import is_py39_plus, is_py310_plus -from cattrs.errors import ForbiddenExtraKeyError from cattr.gen import make_dict_structure_fn, override +from cattrs.errors import ForbiddenExtraKeysError from . import ( nested_typed_classes, @@ -88,9 +88,12 @@ def test_forbid_extra_keys(cls_and_vals): while bad_key in unstructured: bad_key += "A" unstructured[bad_key] = 1 - with pytest.raises(ForbiddenExtraKeyError): + with pytest.raises(ForbiddenExtraKeysError) as feke: converter.structure(unstructured, cl) + assert feke.value.cl is cl + assert feke.value.extra_fields == {bad_key} + @given(simple_typed_attrs(defaults=True)) def test_forbid_extra_keys_defaults(attr_and_vals): @@ -103,9 +106,12 @@ def test_forbid_extra_keys_defaults(attr_and_vals): inst = cl() unstructured = converter.unstructure(inst) unstructured["aa"] = unstructured.pop("a") - with pytest.raises(ForbiddenExtraKeyError): + with pytest.raises(ForbiddenExtraKeysError) as feke: converter.structure(unstructured, cl) + assert feke.value.cl is cl + assert feke.value.extra_fields == {"aa"} + def test_forbid_extra_keys_nested_override(): @attr.s @@ -123,7 +129,7 @@ class A: converter.structure(unstructured, A) # if we break it in the subclass, we need it to raise unstructured["c"]["aa"] = 5 - with pytest.raises(ForbiddenExtraKeyError): + with pytest.raises(ForbiddenExtraKeysError): converter.structure(unstructured, A) # we can "fix" that by disabling forbid_extra_keys on the subclass hook = make_dict_structure_fn(C, converter, _cattrs_forbid_extra_keys=False) @@ -131,7 +137,7 @@ class A: converter.structure(unstructured, A) # but we should still raise at the top level unstructured["b"] = 6 - with pytest.raises(ForbiddenExtraKeyError): + with pytest.raises(ForbiddenExtraKeysError): converter.structure(unstructured, A) diff --git a/tests/test_gen_dict.py b/tests/test_gen_dict.py index a56e430c..e7e431b7 100644 --- a/tests/test_gen_dict.py +++ b/tests/test_gen_dict.py @@ -7,6 +7,7 @@ from cattr._compat import adapted_fields, fields from cattrs import Converter +from cattrs.errors import ForbiddenExtraKeysError from cattrs.gen import make_dict_structure_fn, make_dict_unstructure_fn, override from . import nested_classes, simple_classes @@ -226,7 +227,7 @@ class A: assert new_inst == A(1, "str") - with pytest.raises(Exception): + with pytest.raises(ForbiddenExtraKeysError): converter.structure({"b": 1, "c": "str"}, A) From b2e47b58d49c0c4bd00d5ed181ec06616fe5342d Mon Sep 17 00:00:00 2001 From: Fabian Raab Date: Fri, 25 Mar 2022 19:32:24 +0100 Subject: [PATCH 3/4] Make sure that ForbiddenExtraKeysError is created before ClassValidationError is raised. --- src/cattrs/gen.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cattrs/gen.py b/src/cattrs/gen.py index 4e73fb62..d906b515 100644 --- a/src/cattrs/gen.py +++ b/src/cattrs/gen.py @@ -452,7 +452,7 @@ def make_dict_structure_fn( if _cattrs_forbid_extra_keys: globs["__c_a"] = allowed_fields globs["__c_feke"] = ForbiddenExtraKeysError - post_lines += [ + lines += [ " unknown_fields = set(o.keys()) - __c_a", " if unknown_fields:", " raise __c_feke('', __cl, unknown_fields)", From fd5a9b842da2185b3d04046c1fa38aa533e209da Mon Sep 17 00:00:00 2001 From: Fabian Raab Date: Fri, 25 Mar 2022 20:40:38 +0100 Subject: [PATCH 4/4] Add ForbiddenExtraKeysError to ClassValidationError if used. --- src/cattrs/gen.py | 26 ++++++++++++++------- tests/metadata/test_genconverter.py | 35 +++++++++++++++++++++-------- tests/test_gen_dict.py | 10 +++++++-- 3 files changed, 52 insertions(+), 19 deletions(-) diff --git a/src/cattrs/gen.py b/src/cattrs/gen.py index d906b515..84a2e2fc 100644 --- a/src/cattrs/gen.py +++ b/src/cattrs/gen.py @@ -262,6 +262,10 @@ def make_dict_structure_fn( resolve_types(cl) allowed_fields = set() + if _cattrs_forbid_extra_keys: + globs["__c_a"] = allowed_fields + globs["__c_feke"] = ForbiddenExtraKeysError + if _cattrs_detailed_validation: lines.append(" res = {}") lines.append(" errors = []") @@ -327,6 +331,14 @@ def make_dict_structure_fn( f"{i}e.__note__ = 'Structuring class {cl.__qualname__} @ attribute {an}'" ) lines.append(f"{i}errors.append(e)") + + if _cattrs_forbid_extra_keys: + post_lines += [ + " unknown_fields = set(o.keys()) - __c_a", + " if unknown_fields:", + " errors.append(__c_feke('', __cl, unknown_fields))", + ] + post_lines.append( f" if errors: raise __c_cve('While structuring {cl.__name__}', errors, __cl)" ) @@ -449,14 +461,12 @@ def make_dict_structure_fn( [" return __cl("] + [f" {line}" for line in invocation_lines] + [" )"] ) - if _cattrs_forbid_extra_keys: - globs["__c_a"] = allowed_fields - globs["__c_feke"] = ForbiddenExtraKeysError - lines += [ - " unknown_fields = set(o.keys()) - __c_a", - " if unknown_fields:", - " raise __c_feke('', __cl, unknown_fields)", - ] + if _cattrs_forbid_extra_keys: + post_lines += [ + " unknown_fields = set(o.keys()) - __c_a", + " if unknown_fields:", + " raise __c_feke('', __cl, unknown_fields)", + ] # At the end, we create the function header. internal_arg_line = ", ".join([f"{i}={i}" for i in internal_arg_parts]) diff --git a/tests/metadata/test_genconverter.py b/tests/metadata/test_genconverter.py index f79749d5..c56ae43f 100644 --- a/tests/metadata/test_genconverter.py +++ b/tests/metadata/test_genconverter.py @@ -20,7 +20,7 @@ from cattr import UnstructureStrategy from cattr._compat import is_py39_plus, is_py310_plus from cattr.gen import make_dict_structure_fn, override -from cattrs.errors import ForbiddenExtraKeysError +from cattrs.errors import ClassValidationError, ForbiddenExtraKeysError from . import ( nested_typed_classes, @@ -88,11 +88,13 @@ def test_forbid_extra_keys(cls_and_vals): while bad_key in unstructured: bad_key += "A" unstructured[bad_key] = 1 - with pytest.raises(ForbiddenExtraKeysError) as feke: + with pytest.raises(ClassValidationError) as cve: converter.structure(unstructured, cl) - assert feke.value.cl is cl - assert feke.value.extra_fields == {bad_key} + assert len(cve.value.exceptions) == 1 + assert isinstance(cve.value.exceptions[0], ForbiddenExtraKeysError) + assert cve.value.exceptions[0].cl is cl + assert cve.value.exceptions[0].extra_fields == {bad_key} @given(simple_typed_attrs(defaults=True)) @@ -106,11 +108,13 @@ def test_forbid_extra_keys_defaults(attr_and_vals): inst = cl() unstructured = converter.unstructure(inst) unstructured["aa"] = unstructured.pop("a") - with pytest.raises(ForbiddenExtraKeysError) as feke: + with pytest.raises(ClassValidationError) as cve: converter.structure(unstructured, cl) - assert feke.value.cl is cl - assert feke.value.extra_fields == {"aa"} + assert len(cve.value.exceptions) == 1 + assert isinstance(cve.value.exceptions[0], ForbiddenExtraKeysError) + assert cve.value.exceptions[0].cl is cl + assert cve.value.exceptions[0].extra_fields == {"aa"} def test_forbid_extra_keys_nested_override(): @@ -129,17 +133,30 @@ class A: converter.structure(unstructured, A) # if we break it in the subclass, we need it to raise unstructured["c"]["aa"] = 5 - with pytest.raises(ForbiddenExtraKeysError): + with pytest.raises(ClassValidationError) as cve: converter.structure(unstructured, A) + + assert len(cve.value.exceptions) == 1 + assert isinstance(cve.value.exceptions[0], ClassValidationError) + assert len(cve.value.exceptions[0].exceptions) == 1 + assert isinstance(cve.value.exceptions[0].exceptions[0], ForbiddenExtraKeysError) + assert cve.value.exceptions[0].exceptions[0].cl is C + assert cve.value.exceptions[0].exceptions[0].extra_fields == {"aa"} + # we can "fix" that by disabling forbid_extra_keys on the subclass hook = make_dict_structure_fn(C, converter, _cattrs_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(ForbiddenExtraKeysError): + with pytest.raises(ClassValidationError) as cve: converter.structure(unstructured, A) + assert len(cve.value.exceptions) == 1 + assert isinstance(cve.value.exceptions[0], ForbiddenExtraKeysError) + assert cve.value.exceptions[0].cl is A + assert cve.value.exceptions[0].extra_fields == {"b"} + @given(nested_typed_classes(defaults=True, min_attrs=1), unstructure_strats, booleans()) def test_nested_roundtrip(cls_and_vals, strat, omit_if_default): diff --git a/tests/test_gen_dict.py b/tests/test_gen_dict.py index e7e431b7..3f7f0701 100644 --- a/tests/test_gen_dict.py +++ b/tests/test_gen_dict.py @@ -7,7 +7,7 @@ from cattr._compat import adapted_fields, fields from cattrs import Converter -from cattrs.errors import ForbiddenExtraKeysError +from cattrs.errors import ClassValidationError, ForbiddenExtraKeysError from cattrs.gen import make_dict_structure_fn, make_dict_unstructure_fn, override from . import nested_classes, simple_classes @@ -227,9 +227,15 @@ class A: assert new_inst == A(1, "str") - with pytest.raises(ForbiddenExtraKeysError): + with pytest.raises(ClassValidationError) as cve: converter.structure({"b": 1, "c": "str"}, A) + assert len(cve.value.exceptions) == 2 + assert isinstance(cve.value.exceptions[0], KeyError) + assert isinstance(cve.value.exceptions[1], ForbiddenExtraKeysError) + assert cve.value.exceptions[1].cl is A + assert cve.value.exceptions[1].extra_fields == {"c"} + def test_omitting(): converter = Converter()